Fix range issue when there is a base patch that is not parent
Change-Id: I24d1f063bb6abacd1fdcfc7289762d5061a8337e
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index f78f893..9007888 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -727,6 +727,29 @@
opt_patchNum, opt_path);
},
+ _setRange: function(comments, comment) {
+ if (comment.in_reply_to && !comment.range) {
+ for (var i = 0; i < comments.length; i++) {
+ if (comments[i].id === comment.in_reply_to) {
+ comment.range = comments[i].range;
+ break;
+ }
+ }
+ }
+ return comment;
+ },
+
+ _setRanges: function(comments) {
+ comments = comments || [];
+ comments.sort(function(a, b) {
+ return util.parseDate(a.updated) - util.parseDate(b.updated);
+ });
+ comments.forEach(function(comment) {
+ this._setRange(comments, comment);
+ }.bind(this));
+ return comments;
+ },
+
_getDiffComments: function(changeNum, endpoint, opt_basePatchNum,
opt_patchNum, opt_path) {
if (!opt_basePatchNum && !opt_patchNum && !opt_path) {
@@ -751,19 +774,7 @@
// Sort comments by date so that parent ranges can be propagated in a
// single pass.
- comments = comments.sort(function(a, b) {
- return a.updated > b.updated;
- });
- comments.forEach(function(comment){
- if (comment.in_reply_to && !comment.range) {
- for (var i = 0; i < comments.length; i++) {
- if (comments[i].id === comment.in_reply_to) {
- comment.range = comments[i].range;
- break;
- }
- }
- }
- });
+ comments = this._setRanges(comments);
if (opt_basePatchNum == PARENT_PATCH_NUM) {
baseComments = comments.filter(onlyParent);
@@ -779,8 +790,11 @@
opt_basePatchNum);
promises.push(this.fetchJSON(baseURL).then(function(response) {
baseComments = (response[opt_path] || []).filter(withoutParent);
+
+ baseComments = this._setRanges(baseComments);
+
baseComments.forEach(setPath);
- }));
+ }.bind(this)));
}
return Promise.all(promises).then(function() {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index 0d6d40d..19e3151 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -20,6 +20,7 @@
<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<script src="../../../scripts/util.js"></script>
<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="gr-rest-api-interface.html">
@@ -115,7 +116,7 @@
sandbox.stub(window, 'fetch', function() {
return Promise.resolve({
body: {
- cancel: function() { cancelCalled = true; }
+ cancel: function() { cancelCalled = true; },
},
});
});
@@ -133,11 +134,13 @@
'/COMMIT_MSG': [],
'sieve.go': [
{
+ updated: '2017-02-03 22:32:28.000000000',
message: 'this isn’t quite right',
},
{
side: 'PARENT',
message: 'how did this work in the first place?',
+ updated: '2017-02-03 22:33:28.000000000',
},
],
});
@@ -149,16 +152,123 @@
side: 'PARENT',
message: 'how did this work in the first place?',
path: 'sieve.go',
+ updated: '2017-02-03 22:33:28.000000000',
});
assert.equal(obj.comments.length, 1);
assert.deepEqual(obj.comments[0], {
message: 'this isn’t quite right',
path: 'sieve.go',
+ updated: '2017-02-03 22:32:28.000000000',
});
done();
});
});
+ test('_setRange', function() {
+ var comments = [
+ {
+ id: 1,
+ side: 'PARENT',
+ message: 'how did this work in the first place?',
+ updated: '2017-02-03 22:32:28.000000000',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 2,
+ end_character: 1,
+ },
+ },
+ {
+ id: 2,
+ in_reply_to: 1,
+ message: 'this isn’t quite right',
+ updated: '2017-02-03 22:33:28.000000000',
+ },
+ ];
+ var expectedResult = {
+ id: 2,
+ in_reply_to: 1,
+ message: 'this isn’t quite right',
+ updated: '2017-02-03 22:33:28.000000000',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 2,
+ end_character: 1,
+ },
+ };
+ var comment = comments[1];
+ assert.deepEqual(element._setRange(comments, comment), expectedResult);
+ });
+
+ test('_setRanges', function() {
+ var comments = [
+ {
+ id: 3,
+ in_reply_to: 2,
+ message: 'this isn’t quite right either',
+ updated: '2017-02-03 22:34:28.000000000',
+ },
+ {
+ id: 2,
+ in_reply_to: 1,
+ message: 'this isn’t quite right',
+ updated: '2017-02-03 22:33:28.000000000',
+ },
+ {
+ id: 1,
+ side: 'PARENT',
+ message: 'how did this work in the first place?',
+ updated: '2017-02-03 22:32:28.000000000',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 2,
+ end_character: 1,
+ },
+ },
+ ];
+ var expectedResult = [
+ {
+ id: 1,
+ side: 'PARENT',
+ message: 'how did this work in the first place?',
+ updated: '2017-02-03 22:32:28.000000000',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 2,
+ end_character: 1,
+ },
+ },
+ {
+ id: 2,
+ in_reply_to: 1,
+ message: 'this isn’t quite right',
+ updated: '2017-02-03 22:33:28.000000000',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 2,
+ end_character: 1,
+ },
+ },
+ {
+ id: 3,
+ in_reply_to: 2,
+ message: 'this isn’t quite right either',
+ updated: '2017-02-03 22:34:28.000000000',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 2,
+ end_character: 1,
+ },
+ },
+ ];
+ assert.deepEqual(element._setRanges(comments), expectedResult);
+ });
+
test('differing patch diff comments are properly grouped', function(done) {
sandbox.stub(element, 'fetchJSON', function(url) {
if (url == '/changes/42/revisions/1') {
@@ -167,10 +277,12 @@
'sieve.go': [
{
message: 'this isn’t quite right',
+ updated: '2017-02-03 22:32:28.000000000',
},
{
side: 'PARENT',
message: 'how did this work in the first place?',
+ updated: '2017-02-03 22:33:28.000000000',
},
],
});
@@ -180,13 +292,16 @@
'sieve.go': [
{
message: 'What on earth are you thinking, here?',
+ updated: '2017-02-03 22:32:28.000000000',
},
{
side: 'PARENT',
message: 'Yeah not sure how this worked either?',
+ updated: '2017-02-03 22:33:28.000000000',
},
{
message: '¯\\_(ツ)_/¯',
+ updated: '2017-02-04 22:33:28.000000000',
},
],
});
@@ -198,15 +313,18 @@
assert.deepEqual(obj.baseComments[0], {
message: 'this isn’t quite right',
path: 'sieve.go',
+ updated: '2017-02-03 22:32:28.000000000',
});
assert.equal(obj.comments.length, 2);
assert.deepEqual(obj.comments[0], {
message: 'What on earth are you thinking, here?',
path: 'sieve.go',
+ updated: '2017-02-03 22:32:28.000000000',
});
assert.deepEqual(obj.comments[1], {
message: '¯\\_(ツ)_/¯',
path: 'sieve.go',
+ updated: '2017-02-04 22:33:28.000000000',
});
done();
});
@@ -303,12 +421,12 @@
{
ok: false,
status: 403,
- text: function() { return Promise.resolve(); }
+ text: function() { return Promise.resolve(); },
},
{
ok: true,
status: 200,
- text: function() { return Promise.resolve(')]}\'{}'); }
+ text: function() { return Promise.resolve(')]}\'{}'); },
},
];
sandbox.stub(window, 'fetch', function(url) {
