opendev/gerrit
Code Issues Proposed changes

Correct patch range in comment links for indexed parents

Browse Source 
When the comment list generated links for the diff lines where comments
are located, it did not take the parent index into account, and instead
linked to the diff against the "AutoMerge" patch base. With this change,
the parent index is included in the URL's patch range.

Bug: Issue 7806
Change-Id: Iada8ab2aca733897c28af958cffbda5613d75065
This commit is contained in:
Wyatt Allen
2017-11-22 11:03:59 -08:00
parent a744e2303f
commit df4ae70005
2 changed files with 32 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
View File

@@ -46,8 +46,11 @@
},
_computeDiffLineURL(file, changeNum, patchNum, comment) {
const basePatchNum = comment.hasOwnProperty('parent') ?
-comment.parent : null;
return Gerrit.Nav.getUrlForDiffById(this.changeNum, this.projectName,
file, patchNum, null, comment.line, this._isOnParent(comment));
file, patchNum, basePatchNum, comment.line,
this._isOnParent(comment));
},
_computeCommentsForFile(comments, file) {

28
polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
View File

@@ -34,11 +34,15 @@ limitations under the License.
<script>
suite('gr-comment-list tests', () => {
let element;
let sandbox;
setup(() => {
element = fixture('basic');
sandbox = sinon.sandbox.create();
});
teardown(() => { sandbox.restore(); });
test('_computeFilesFromComments w/ special file path sorting', () => {
const comments = {
'file_b.html': [],
@@ -94,5 +98,29 @@ limitations under the License.
assert.deepEqual(formattedText.config,
element.projectConfig.commentlinks);
});
test('_computeDiffLineURL', () => {
const getUrlStub = sandbox.stub(Gerrit.Nav, 'getUrlForDiffById');
element.projectName = 'proj';
element.changeNum = 123;
const comment = {line: 456};
element._computeDiffLineURL('foo.cc', 123, 4, comment);
assert.isTrue(getUrlStub.calledOnce);
assert.deepEqual(getUrlStub.lastCall.args,
[123, 'proj', 'foo.cc', 4, null, 456, false]);
comment.side = 'PARENT';
element._computeDiffLineURL('foo.cc', 123, 4, comment);
assert.isTrue(getUrlStub.calledTwice);
assert.deepEqual(getUrlStub.lastCall.args,
[123, 'proj', 'foo.cc', 4, null, 456, true]);
comment.parent = 12;
element._computeDiffLineURL('foo.cc', 123, 4, comment);
assert.isTrue(getUrlStub.calledThrice);
assert.deepEqual(getUrlStub.lastCall.args,
[123, 'proj', 'foo.cc', 4, -12, 456, true]);
});
});
</script>