Merge "Add handling for ported comments with side=Parent"
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 166f27e..5210616 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -30,6 +30,7 @@
NumericChangeId,
PathToCommentsInfoMap,
FileInfo,
+ ParentPatchSetNum,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {
@@ -367,6 +368,10 @@
* does not return the range of the ported thread and it becomes a file level
* thread.
*
+ * If a comment was created with Side=PARENT, then we only show this ported
+ * comment if Base is part of the patch range, always on the left side of
+ * the diff.
+ *
* @return only the ported threads for the specified file and patch range
*/
_getPortedCommentThreads(
@@ -405,15 +410,22 @@
return false;
}
- // TODO(dhruvsri): Add handling for thread.commentSide = PARENT
- if (thread.commentSide === CommentSide.PARENT) return false;
+ thread.diffSide = Side.RIGHT;
+ if (thread.commentSide === CommentSide.PARENT) {
+ // TODO(dhruvsri): Add handling for merge parents
+ if (
+ patchRange.basePatchNum !== ParentPatchSetNum ||
+ !!thread.mergeParentNum
+ )
+ return false;
+ thread.diffSide = Side.LEFT;
+ }
if (!isUnresolved(thread) && !isDraftThread(thread)) return false;
thread.range = portedComment.range;
thread.line = portedComment.line;
thread.ported = true;
- thread.diffSide = Side.RIGHT;
return true;
});
}
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
index dd36613..e107c16 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
@@ -20,6 +20,7 @@
import {ChangeComments} from './gr-comment-api.js';
import {isInRevisionOfPatchRange, isInBaseOfPatchRange, isDraftThread, isUnresolved, createCommentThreads} from '../../../utils/comment-util.js';
import {createDraft, createComment, createChangeComments, createCommentThread} from '../../../test/test-data-generators.js';
+import {CommentSide, Side} from '../../../constants/constants.js';
const basicFixture = fixtureFromElement('gr-comment-api');
@@ -153,7 +154,7 @@
const comment1 = {
...createComment(),
unresolved: true,
- id: 'db977012_e1f13818',
+ id: '1',
line: 136,
patch_set: 2,
range: {
@@ -167,10 +168,22 @@
const comment2 = {
...createComment(),
patch_set: 2,
- id: 'ecf0b9fa_fe1a5f62',
+ id: '2',
line: 5,
};
+ const comment3 = {
+ ...createComment(),
+ side: CommentSide.PARENT,
+ line: 10,
+ unresolved: true,
+ };
+
+ const comment4 = {
+ ...comment3,
+ parent: -2,
+ };
+
const draft1 = {
...createDraft(),
id: 'db977012_e1f13828',
@@ -277,6 +290,74 @@
).length, 0);
});
+ test('comments with side=PARENT are ported over', () => {
+ changeComments = new ChangeComments(
+ {/* comments */
+ // comment left on Base
+ 'karma.conf.js': [comment3],
+ },
+ {}/* robot comments */,
+ {/* drafts */
+ 'karma.conf.js': [draft2],
+ },
+ {/* ported comments */
+ 'karma.conf.js': [{
+ ...comment3,
+ line: 31,
+ patch_set: 4,
+ }],
+ },
+ {}/* ported drafts */
+ );
+
+ const portedThreads = changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'});
+ assert.equal(portedThreads.length, 1);
+ assert.equal(portedThreads[0].line, 31);
+ assert.equal(portedThreads[0].diffSide, Side.LEFT);
+
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: -2}
+ ).length, 0);
+
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 2}
+ ).length, 0);
+ });
+
+ test('comments left on merge parent is not ported over', () => {
+ changeComments = new ChangeComments(
+ {/* comments */
+ // comment left on Base
+ 'karma.conf.js': [comment4],
+ },
+ {}/* robot comments */,
+ {/* drafts */
+ 'karma.conf.js': [draft2],
+ },
+ {/* ported comments */
+ 'karma.conf.js': [{
+ ...comment4,
+ line: 31,
+ patch_set: 4,
+ }],
+ },
+ {}/* ported drafts */
+ );
+
+ const portedThreads = changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'});
+ assert.equal(portedThreads.length, 0);
+
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: -2}
+ ).length, 0);
+
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 2}
+ ).length, 0);
+ });
+
test('ported comments contribute to comment count', () => {
assert.equal(changeComments.computeCommentsString(
{basePatchNum: 'PARENT', patchNum: 2}, 'karma.conf.js',
