Move comment patchset logic to gr-diff-host This is a step towards making gr-diff oblivious of the specifics of the selected PatchRange. This makes it easier to reuse gr-diff independently of the underlying versioning system. A comment's patchset (and commentSide, already in gr-diff-host) together determine which patch set a comment is stored on. This is usually the patchset the user left the comment on, except in the case of PARENT and merge parent versions which are not patchsets and cannot store comments. Change-Id: I5e7e30f93ee5a74fe8f2c2e6b225282246c51bd9
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts index 7b58eb6..2fc8197 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts +++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -32,6 +32,7 @@ import {appContext} from '../../../services/app-context'; import { getParentIndex, + isAParent, isMergeParent, isNumber, } from '../../../utils/patch-set-util'; @@ -729,8 +730,26 @@ } _handleCreateComment(e: CustomEvent) { - const {lineNum, side, patchNum, range, path} = e.detail; - const commentSide = this._sideToCommentSide(side); + if (!this.patchRange) throw Error('patch range not set'); + + const {lineNum, side, range, path} = e.detail; + + // Usually, the comment is stored on the patchset shown on the side the + // user added the comment on, and the commentSide will be REVISION. + // However, if the comment is added on the left side of the diff and the + // version shown there is not a patchset that is part the change, but + // instead a base (a PARENT or a merge parent commit), the comment is + // stored on the patchset shown on the right, and commentSide=PARENT + // indicates that the comment should still be shown on the left side. + const patchNum = + side === Side.LEFT && !isAParent(this.patchRange.basePatchNum) + ? this.patchRange.basePatchNum + : this.patchRange.patchNum; + const commentSide = + side === Side.LEFT && isAParent(this.patchRange.basePatchNum) + ? CommentSide.PARENT + : CommentSide.REVISION; + const threadEl = this._getOrCreateThread( patchNum, lineNum, @@ -744,15 +763,6 @@ this.reporting.recordDraftInteraction(); } - _sideToCommentSide(side: Side): CommentSide { - if (!this.patchRange) throw Error('patch range not set'); - return side === Side.LEFT && - (this.patchRange.basePatchNum === 'PARENT' || - isMergeParent(this.patchRange.basePatchNum)) - ? CommentSide.PARENT - : CommentSide.REVISION; - } - /** * Gets or creates a comment thread at a given location. * May provide a range, to get/create a range comment.
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 ad46bcd..0c1a92d 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
@@ -966,7 +966,6 @@ element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 2, lineNum: 3, side: diffSide, path: '/p', @@ -989,10 +988,13 @@ end_line: 1, end_character: 3, }; + element.patchRange = { + basePatchNum: 'PARENT', + patchNum: 3, + }; element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 3, lineNum: 1, side: diffSide, path: '/p', @@ -1018,7 +1020,6 @@ element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 2, side: Side.RIGHT, }, })); @@ -1037,7 +1038,6 @@ element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 3, side: Side.LEFT, }, })); @@ -1056,7 +1056,6 @@ element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 3, side: Side.LEFT, }, })); @@ -1075,7 +1074,6 @@ element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 3, side: Side.LEFT, }, })); @@ -1097,8 +1095,6 @@ element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 2, - lineNum: 3, side: diffSide, path: '/p', }, @@ -1123,8 +1119,6 @@ element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 2, - lineNum: 3, side: diffSide, path: '/p', }, @@ -1149,11 +1143,8 @@ element.dispatchEvent(new CustomEvent('create-comment', { detail: { - patchNum: 2, - lineNum: 3, side: diffSide, path: '/p', - range: undefined, }, }));