Merge "Remove diffSide property from UIComment"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index 8f484e9..61218f5 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -29,6 +29,7 @@
import {TestKeyboardShortcutBinder} from '../../../test/test-utils.js';
import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api.js';
+import {createCommentThreads} from '../../../utils/comment-util.js';
const commentApiMock = createCommentApiMockWithTemplateElement(
'gr-file-list-comment-api-mock', html`
@@ -106,8 +107,6 @@
// been initialized.
commentApiWrapper.loadComments().then(() => {
sinon.stub(element.changeComments, 'getPaths').returns({});
- sinon.stub(element.changeComments, 'getCommentsBySideForPath')
- .returns({meta: {}, left: [], right: []});
done();
});
element._loading = false;
@@ -1527,17 +1526,8 @@
];
async function setupDiff(diff) {
- diff.comments = {
- left: diff.path === '/COMMIT_MSG' ? commitMsgComments : [],
- right: [],
- meta: {
- changeNum: 1,
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 2,
- },
- },
- };
+ diff.threads = diff.path === '/COMMIT_MSG' ?
+ createCommentThreads(commitMsgComments) : [];
diff.prefs = {
context: 10,
tab_size: 8,
@@ -1556,7 +1546,7 @@
};
diff.diff = getMockDiffResponse();
commentApiWrapper.loadComments().then(() => {
- sinon.stub(element.changeComments, 'getCommentsBySideForPath')
+ sinon.stub(element.changeComments, 'getCommentsForPath')
.withArgs('/COMMIT_MSG', {
basePatchNum: 'PARENT',
patchNum: 2,
@@ -1608,8 +1598,6 @@
// been initialized.
commentApiWrapper.loadComments().then(() => {
sinon.stub(element.changeComments, 'getPaths').returns({});
- sinon.stub(element.changeComments, 'getCommentsBySideForPath')
- .returns({meta: {}, left: [], right: []});
done();
});
element._loading = false;
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 a1d4421..4f6dc3a 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
@@ -18,16 +18,10 @@
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-comment-api_html';
-import {
- getParentIndex,
- isMergeParent,
- patchNumEquals,
- CURRENT,
-} from '../../../utils/patch-set-util';
+import {patchNumEquals, CURRENT} from '../../../utils/patch-set-util';
import {customElement, property} from '@polymer/decorators';
import {
CommentBasics,
- ParentPatchSetNum,
PatchRange,
PatchSetNum,
PathToRobotCommentsInfoMap,
@@ -37,7 +31,6 @@
RevisionId,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
-import {CommentSide, Side} from '../../../constants/constants';
import {
Comment,
CommentMap,
@@ -49,6 +42,7 @@
UIHuman,
UIRobot,
createCommentThreads,
+ isInPatchRange,
} from '../../../utils/comment-util';
import {PatchSetFile, PatchNumOnly, isPatchSetFile} from '../../../types/types';
import {appContext} from '../../../services/app-context';
@@ -57,11 +51,6 @@
[urlEncodedCommentId: string]: CommentThread;
};
-export interface TwoSidesComments {
- left: UIComment[];
- right: UIComment[];
-}
-
export class ChangeComments {
private readonly _comments: {[path: string]: UIHuman[]};
@@ -160,7 +149,7 @@
if (!patchRange) {
return true;
}
- return this._isInPatchRange(c, patchRange);
+ return isInPatchRange(c, patchRange);
})
) {
commentMap[path] = true;
@@ -300,25 +289,11 @@
return allDrafts;
}
- _addCommentSide(comments: TwoSidesComments) {
- const allComments = [];
- for (const side of [Side.LEFT, Side.RIGHT]) {
- // This is needed by the threading.
- for (const comment of comments[side]) {
- comment.diffSide = side;
- }
- allComments.push(...comments[side]);
- }
- return allComments;
- }
-
getThreadsBySideForPath(
path: string,
patchRange: PatchRange
): CommentThread[] {
- return createCommentThreads(
- this._addCommentSide(this.getCommentsBySideForPath(path, patchRange))
- );
+ return createCommentThreads(this.getCommentsForPath(path, patchRange));
}
/**
@@ -331,10 +306,7 @@
* @param projectConfig Optional project config object to
* include in the meta sub-object.
*/
- getCommentsBySideForPath(
- path: string,
- patchRange: PatchRange
- ): TwoSidesComments {
+ getCommentsForPath(path: string, patchRange: PatchRange): Comment[] {
let comments: Comment[] = [];
let drafts: DraftInfo[] = [];
let robotComments: RobotCommentInfo[] = [];
@@ -352,33 +324,20 @@
d.__draft = true;
});
- const all: Comment[] = comments
+ return comments
.concat(drafts)
.concat(robotComments)
+ .filter(c => isInPatchRange(c, patchRange))
.map(c => {
return {...c};
});
-
- const baseComments = all.filter(c =>
- this._isInBaseOfPatchRange(c, patchRange)
- );
- const revisionComments = all.filter(c =>
- this._isInRevisionOfPatchRange(c, patchRange)
- );
-
- return {
- left: baseComments,
- right: revisionComments,
- };
}
getThreadsBySideForFile(
file: PatchSetFile,
patchRange: PatchRange
): CommentThread[] {
- return createCommentThreads(
- this._addCommentSide(this.getCommentsBySideForFile(file, patchRange))
- );
+ return createCommentThreads(this.getCommentsForFile(file, patchRange));
}
/**
@@ -393,19 +352,10 @@
* @param projectConfig Optional project config object to
* include in the meta sub-object.
*/
- getCommentsBySideForFile(
- file: PatchSetFile,
- patchRange: PatchRange
- ): TwoSidesComments {
- const comments = this.getCommentsBySideForPath(file.path, patchRange);
+ getCommentsForFile(file: PatchSetFile, patchRange: PatchRange): Comment[] {
+ const comments = this.getCommentsForPath(file.path, patchRange);
if (file.basePath) {
- const commentsForBasePath = this.getCommentsBySideForPath(
- file.basePath,
- patchRange
- );
- // merge in the left and right
- comments.left = comments.left.concat(commentsForBasePath.left);
- comments.right = comments.right.concat(commentsForBasePath.right);
+ comments.push(...this.getCommentsForPath(file.basePath, patchRange));
}
return comments;
}
@@ -471,58 +421,6 @@
const comments = this._commentObjToArray(this.getAllComments(true));
return createCommentThreads(comments);
}
-
- /**
- * Whether the given comment should be included in the base side of the
- * given patch range.
- */
- _isInBaseOfPatchRange(comment: CommentBasics, range: PatchRange) {
- // If the base of the patch range is a parent of a merge, and the comment
- // appears on a specific parent then only show the comment if the parent
- // index of the comment matches that of the range.
- if (comment.parent && comment.side === CommentSide.PARENT) {
- return (
- isMergeParent(range.basePatchNum) &&
- comment.parent === getParentIndex(range.basePatchNum)
- );
- }
-
- // If the base of the range is the parent of the patch:
- if (
- range.basePatchNum === ParentPatchSetNum &&
- comment.side === CommentSide.PARENT &&
- patchNumEquals(comment.patch_set, range.patchNum)
- ) {
- return true;
- }
- // If the base of the range is not the parent of the patch:
- return (
- range.basePatchNum !== ParentPatchSetNum &&
- comment.side !== CommentSide.PARENT &&
- patchNumEquals(comment.patch_set, range.basePatchNum)
- );
- }
-
- /**
- * Whether the given comment should be included in the revision side of the
- * given patch range.
- */
- _isInRevisionOfPatchRange(comment: CommentBasics, range: PatchRange) {
- return (
- comment.side !== CommentSide.PARENT &&
- patchNumEquals(comment.patch_set, range.patchNum)
- );
- }
-
- /**
- * Whether the given comment should be included in the given patch range.
- */
- _isInPatchRange(comment: CommentBasics, range: PatchRange): boolean {
- return (
- this._isInBaseOfPatchRange(comment, range) ||
- this._isInRevisionOfPatchRange(comment, range)
- );
- }
}
// TODO(TS): move findCommentById out of class
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 4bea2f1d..6b6756e 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
@@ -19,6 +19,7 @@
import './gr-comment-api.js';
import {ChangeComments} from './gr-comment-api.js';
import {CommentSide} from '../../../constants/constants.js';
+import {isInRevisionOfPatchRange, isInBaseOfPatchRange} from '../../../utils/comment-util.js';
const basicFixture = fixtureFromElement('gr-comment-api');
@@ -146,74 +147,50 @@
});
});
- test('_isInBaseOfPatchRange', () => {
+ test('isInBaseOfPatchRange', () => {
const comment = {patch_set: 1};
const patchRange = {basePatchNum: 1, patchNum: 2};
- assert.isTrue(element._changeComments._isInBaseOfPatchRange(comment,
+ assert.isTrue(isInBaseOfPatchRange(comment,
patchRange));
patchRange.basePatchNum = PARENT;
- assert.isFalse(element._changeComments._isInBaseOfPatchRange(comment,
+ assert.isFalse(isInBaseOfPatchRange(comment,
patchRange));
comment.side = PARENT;
- assert.isFalse(element._changeComments._isInBaseOfPatchRange(comment,
+ assert.isFalse(isInBaseOfPatchRange(comment,
patchRange));
comment.patch_set = 2;
- assert.isTrue(element._changeComments._isInBaseOfPatchRange(comment,
+ assert.isTrue(isInBaseOfPatchRange(comment,
patchRange));
patchRange.basePatchNum = -2;
comment.side = PARENT;
comment.parent = 1;
- assert.isFalse(element._changeComments._isInBaseOfPatchRange(comment,
+ assert.isFalse(isInBaseOfPatchRange(comment,
patchRange));
comment.parent = 2;
- assert.isTrue(element._changeComments._isInBaseOfPatchRange(comment,
+ assert.isTrue(isInBaseOfPatchRange(comment,
patchRange));
});
- test('_isInRevisionOfPatchRange', () => {
+ test('isInRevisionOfPatchRange', () => {
const comment = {patch_set: 123};
const patchRange = {basePatchNum: 122, patchNum: 124};
- assert.isFalse(element._changeComments._isInRevisionOfPatchRange(
+ assert.isFalse(isInRevisionOfPatchRange(
comment, patchRange));
patchRange.patchNum = 123;
- assert.isTrue(element._changeComments._isInRevisionOfPatchRange(
+ assert.isTrue(isInRevisionOfPatchRange(
comment, patchRange));
comment.side = PARENT;
- assert.isFalse(element._changeComments._isInRevisionOfPatchRange(
+ assert.isFalse(isInRevisionOfPatchRange(
comment, patchRange));
});
- test('_isInPatchRange', () => {
- const patchRange1 = {basePatchNum: 122, patchNum: 124};
- const patchRange2 = {basePatchNum: 123, patchNum: 125};
- const patchRange3 = {basePatchNum: 124, patchNum: 125};
-
- const isInBasePatchStub = sinon.stub(element._changeComments,
- '_isInBaseOfPatchRange');
- const isInRevisionPatchStub = sinon.stub(element._changeComments,
- '_isInRevisionOfPatchRange');
-
- isInBasePatchStub.withArgs({}, patchRange1).returns(true);
- isInBasePatchStub.withArgs({}, patchRange2).returns(false);
- isInBasePatchStub.withArgs({}, patchRange3).returns(false);
-
- isInRevisionPatchStub.withArgs({}, patchRange1).returns(false);
- isInRevisionPatchStub.withArgs({}, patchRange2).returns(true);
- isInRevisionPatchStub.withArgs({}, patchRange3).returns(false);
-
- assert.isTrue(element._changeComments._isInPatchRange({}, patchRange1));
- assert.isTrue(element._changeComments._isInPatchRange({}, patchRange2));
- assert.isFalse(element._changeComments._isInPatchRange({},
- patchRange3));
- });
-
suite('comment ranges and paths', () => {
function makeTime(mins) {
return `2013-02-26 15:0${mins}:43.986000000`;
@@ -349,32 +326,40 @@
assert.property(paths, 'file/four');
});
- test('getCommentsBySideForPath', () => {
+ test('getCommentsForPath', () => {
const patchRange = {basePatchNum: 1, patchNum: 3};
let path = 'file/one';
- let comments = element._changeComments.getCommentsBySideForPath(path,
+ let comments = element._changeComments.getCommentsForPath(path,
patchRange);
- assert.equal(comments.left.length, 0);
- assert.equal(comments.right.length, 0);
+ assert.equal(comments.filter(c => isInBaseOfPatchRange(c, patchRange))
+ .length, 0);
+ assert.equal(comments.filter(c => isInRevisionOfPatchRange(c,
+ patchRange)).length, 0);
path = 'file/two';
- comments = element._changeComments.getCommentsBySideForPath(path,
+ comments = element._changeComments.getCommentsForPath(path,
patchRange);
- assert.equal(comments.left.length, 0);
- assert.equal(comments.right.length, 2);
+ assert.equal(comments.filter(c => isInBaseOfPatchRange(c, patchRange))
+ .length, 0);
+ assert.equal(comments.filter(c => isInRevisionOfPatchRange(c,
+ patchRange)).length, 2);
patchRange.basePatchNum = 2;
- comments = element._changeComments.getCommentsBySideForPath(path,
+ comments = element._changeComments.getCommentsForPath(path,
patchRange);
- assert.equal(comments.left.length, 1);
- assert.equal(comments.right.length, 2);
+ assert.equal(comments.filter(c => isInBaseOfPatchRange(c,
+ patchRange)).length, 1);
+ assert.equal(comments.filter(c => isInRevisionOfPatchRange(c,
+ patchRange)).length, 2);
patchRange.basePatchNum = PARENT;
path = 'file/three';
- comments = element._changeComments.getCommentsBySideForPath(path,
+ comments = element._changeComments.getCommentsForPath(path,
patchRange);
- assert.equal(comments.left.length, 0);
- assert.equal(comments.right.length, 1);
+ assert.equal(comments.filter(c => isInBaseOfPatchRange(c, patchRange))
+ .length, 0);
+ assert.equal(comments.filter(c => isInRevisionOfPatchRange(c,
+ patchRange)).length, 1);
});
test('getAllCommentsForPath', () => {
@@ -539,7 +524,6 @@
path: 'file/one',
},
],
- diffSide: undefined,
commentSide: 'PARENT',
patchNum: 2,
path: 'file/one',
@@ -566,7 +550,6 @@
path: 'file/one',
line: 2,
range: undefined,
- diffSide: undefined,
commentSide: CommentSide.PARENT,
rootId: '03',
}, {
@@ -602,7 +585,6 @@
line: 1,
rootId: '04',
range: undefined,
- diffSide: undefined,
commentSide: CommentSide.REVISION,
}, {
comments: [
@@ -619,7 +601,6 @@
line: 2,
rootId: '05',
range: undefined,
- diffSide: undefined,
commentSide: CommentSide.REVISION,
}, {
comments: [
@@ -636,7 +617,6 @@
line: 2,
rootId: '06',
range: undefined,
- diffSide: undefined,
commentSide: CommentSide.REVISION,
}, {
comments: [
@@ -666,7 +646,6 @@
line: 1,
rootId: '07',
range: undefined,
- diffSide: undefined,
}, {
comments: [
{
@@ -682,7 +661,6 @@
line: 1,
rootId: '09',
range: undefined,
- diffSide: undefined,
commentSide: CommentSide.REVISION,
}, {
comments: [
@@ -701,7 +679,6 @@
line: 1,
rootId: '10',
range: undefined,
- diffSide: undefined,
}, {
comments: [
{
@@ -717,7 +694,6 @@
path: 'file/four',
line: 1,
range: undefined,
- diffSide: undefined,
commentSide: CommentSide.REVISION,
}, {
comments: [
@@ -735,7 +711,6 @@
path: 'file/two',
line: 1,
range: undefined,
- diffSide: undefined,
commentSide: CommentSide.REVISION,
}, {
comments: [
@@ -755,7 +730,6 @@
path: 'file/one',
line: 1,
range: undefined,
- diffSide: undefined,
},
];
const threads = element._changeComments.getAllThreadsForChange();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index 4c5bcb4..81a3604 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -973,14 +973,12 @@
message: 'i like you, jack',
updated: '2015-12-23 15:00:20.396000000',
line: 1,
- diffSide: Side.LEFT,
patch_set: 1,
path: 'some/path',
}, {
id: 'jacks_reply',
message: 'i like you, too',
updated: '2015-12-24 15:01:20.396000000',
- diffSide: Side.LEFT,
line: 1,
in_reply_to: 'sallys_confession',
patch_set: 1,
@@ -989,7 +987,6 @@
{
id: 'new_draft',
message: 'i do not like either of you',
- diffSide: Side.LEFT,
__draft: true,
updated: '2015-12-20 15:01:20.396000000',
patch_set: 1,
@@ -997,7 +994,8 @@
},
];
- const actualThreads = createCommentThreads(comments);
+ const actualThreads = createCommentThreads(comments,
+ {basePatchNum: 1, patchNum: 4});
assert.equal(actualThreads.length, 2);
@@ -1015,7 +1013,7 @@
assert.equal(actualThreads[1].line, FILE);
});
- test('_createThreads inherits patchNum and range', () => {
+ test('_createThreads derives patchNum and range', () => {
const comments = [{
id: 'betsys_confession',
message: 'i like you, jack',
@@ -1028,7 +1026,6 @@
},
patch_set: 5,
path: '/p',
- diffSide: Side.LEFT,
line: 1,
}];
@@ -1050,7 +1047,6 @@
end_character: 2,
},
patch_set: 5,
- diffSide: Side.LEFT,
line: 1,
}],
patchNum: 5,
@@ -1065,7 +1061,7 @@
];
assert.deepEqual(
- createCommentThreads(comments),
+ createCommentThreads(comments, {basePatchNum: 5, patchNum: 10}),
expectedThreads);
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 390c636..6656ff1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -92,7 +92,7 @@
import {GrApplyFixDialog} from '../gr-apply-fix-dialog/gr-apply-fix-dialog';
import {LineOfInterest} from '../gr-diff/gr-diff';
import {RevisionInfo as RevisionInfoObj} from '../../shared/revision-info/revision-info';
-import {CommentMap} from '../../../utils/comment-util';
+import {CommentMap, isInBaseOfPatchRange} from '../../../utils/comment-util';
import {AppElementParams} from '../../gr-app-types';
import {CustomKeyboardEvent, OpenFixPreviewEvent} from '../../../types/events';
import {PORTING_COMMENTS_DIFF_LATENCY_LABEL} from '../../../services/gr-reporting/gr-reporting';
@@ -973,7 +973,7 @@
patchNum: latestPatchNum,
basePatchNum: ParentPatchSetNum,
};
- leftSide = comment.diffSide === Side.LEFT;
+ leftSide = isInBaseOfPatchRange(comment, this._patchRange);
} else {
this._patchRange = {
patchNum: latestPatchNum,
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 365cc6e..b5efff6 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -507,7 +507,6 @@
if (rootComment.patch_set !== undefined)
d.patch_set = rootComment.patch_set;
if (rootComment.side !== undefined) d.side = rootComment.side;
- if (rootComment.diffSide !== undefined) d.diffSide = rootComment.diffSide;
if (rootComment.line !== undefined) d.line = rootComment.line;
if (rootComment.range !== undefined) d.range = rootComment.range;
if (rootComment.parent !== undefined) d.parent = rootComment.parent;
@@ -516,7 +515,6 @@
d.path = this.path;
d.patch_set = this.patchNum;
d.side = this._getSide(this.isOnParent);
- d.diffSide = this.diffSide;
if (lineNum && lineNum !== FILE) {
d.line = lineNum;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
index cd274fb..64be155 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
@@ -117,7 +117,6 @@
draft="[[_isDraft(comment)]]"
show-actions="[[_showActions]]"
show-patchset="[[showPatchset]]"
- diff-side="[[comment.diffSide]]"
side="[[comment.side]]"
project-config="[[_projectConfig]]"
on-create-fix-comment="_handleCommentFix"
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index a6ead90..175311e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -348,7 +348,6 @@
path: '/path/to/file.txt',
unresolved: true,
patch_set: 3 as PatchSetNum,
- diffSide: Side.LEFT,
},
];
flush();
@@ -833,7 +832,6 @@
test('_newDraft with root', () => {
const draft = element._newDraft();
- assert.equal(draft.diffSide, Side.LEFT);
assert.equal(draft.patch_set, 3 as PatchSetNum);
});
@@ -842,7 +840,6 @@
element.diffSide = Side.RIGHT;
element.patchNum = 2 as PatchSetNum;
const draft = element._newDraft();
- assert.equal(draft.diffSide, Side.RIGHT);
assert.equal(draft.patch_set, 2 as PatchSetNum);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 9e4de906a..73f3614 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -50,7 +50,6 @@
import {GrButton} from '../gr-button/gr-button';
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
import {GrDialog} from '../gr-dialog/gr-dialog';
-import {Side} from '../../../constants/constants';
import {
isDraft,
UIComment,
@@ -218,9 +217,6 @@
_messageText = '';
@property({type: String})
- diffSide?: Side;
-
- @property({type: String})
side?: string;
@property({type: Boolean})
@@ -449,7 +445,6 @@
if (this.comment?.__draftID) {
resComment.__draftID = this.comment.__draftID;
}
- resComment.diffSide = this.diffSide;
this.comment = resComment;
this._fireSave();
return obj;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
index fa092f1..3026f4a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
@@ -910,7 +910,6 @@
assert.deepEqual(dispatchEventStub.lastCall.args[0].detail, {
comment: {
- diffSide: Side.RIGHT,
__draft: true,
__draftID: 'temp_draft_id',
id: 'baf0414d_40572e03',
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index c338b00..3340820 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -22,11 +22,14 @@
Timestamp,
UrlEncodedCommentId,
CommentRange,
+ PatchRange,
+ ParentPatchSetNum,
} from '../types/common';
import {CommentSide, Side} from '../constants/constants';
import {parseDate} from './date-util';
import {LineNumber} from '../elements/diff/gr-diff/gr-diff-line';
import {CommentIdToCommentThreadMap} from '../elements/diff/gr-comment-api/gr-comment-api';
+import {isMergeParent, getParentIndex, patchNumEquals} from './patch-set-util';
export interface DraftCommentProps {
__draft?: boolean;
@@ -42,9 +45,6 @@
export type Comment = DraftInfo | CommentInfo | RobotCommentInfo;
export interface UIStateCommentProps {
- // diffSide is used by gr-diff to decide which side(left/right) to show
- // the comment
- diffSide?: Side;
collapsed?: boolean;
// TODO(TS): Consider allowing this only for drafts.
__editing?: boolean;
@@ -97,7 +97,10 @@
});
}
-export function createCommentThreads(comments: UIComment[]) {
+export function createCommentThreads(
+ comments: UIComment[],
+ patchRange?: PatchRange
+) {
const sortedComments = sortComments(comments);
const threads: CommentThread[] = [];
const idThreadMap: CommentIdToCommentThreadMap = {};
@@ -126,8 +129,14 @@
line: comment.line,
range: comment.range,
rootId: comment.id,
- diffSide: comment.diffSide,
};
+ if (patchRange) {
+ if (isInBaseOfPatchRange(comment, patchRange))
+ newThread.diffSide = Side.LEFT;
+ else if (isInRevisionOfPatchRange(comment, patchRange))
+ newThread.diffSide = Side.RIGHT;
+ else throw new Error('comment does not belong in given patchrange');
+ }
if (!comment.line && !comment.range) {
newThread.line = 'FILE';
}
@@ -162,3 +171,64 @@
export function isDraftThread(thread?: CommentThread): boolean {
return isDraft(getLastComment(thread));
}
+
+/**
+ * Whether the given comment should be included in the base side of the
+ * given patch range.
+ */
+export function isInBaseOfPatchRange(
+ comment: CommentBasics,
+ range: PatchRange
+) {
+ // If the base of the patch range is a parent of a merge, and the comment
+ // appears on a specific parent then only show the comment if the parent
+ // index of the comment matches that of the range.
+ if (comment.parent && comment.side === CommentSide.PARENT) {
+ return (
+ isMergeParent(range.basePatchNum) &&
+ comment.parent === getParentIndex(range.basePatchNum)
+ );
+ }
+
+ // If the base of the range is the parent of the patch:
+ if (
+ range.basePatchNum === ParentPatchSetNum &&
+ comment.side === CommentSide.PARENT &&
+ patchNumEquals(comment.patch_set, range.patchNum)
+ ) {
+ return true;
+ }
+ // If the base of the range is not the parent of the patch:
+ return (
+ range.basePatchNum !== ParentPatchSetNum &&
+ comment.side !== CommentSide.PARENT &&
+ patchNumEquals(comment.patch_set, range.basePatchNum)
+ );
+}
+
+/**
+ * Whether the given comment should be included in the revision side of the
+ * given patch range.
+ */
+export function isInRevisionOfPatchRange(
+ comment: CommentBasics,
+ range: PatchRange
+) {
+ return (
+ comment.side !== CommentSide.PARENT &&
+ patchNumEquals(comment.patch_set, range.patchNum)
+ );
+}
+
+/**
+ * Whether the given comment should be included in the given patch range.
+ */
+export function isInPatchRange(
+ comment: CommentBasics,
+ range: PatchRange
+): boolean {
+ return (
+ isInBaseOfPatchRange(comment, range) ||
+ isInRevisionOfPatchRange(comment, range)
+ );
+}
