Merge "Remove tracking of comments updates in gr-diff-host"
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 8c08084..aa769c6 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
@@ -31,13 +31,7 @@
isMergeParent,
isNumber,
} from '../../../utils/patch-set-util';
-import {
- Comment,
- isDraft,
- UIComment,
- CommentThread,
- createCommentThreads,
-} from '../../../utils/comment-util';
+import {CommentThread, createCommentThreads} from '../../../utils/comment-util';
import {TwoSidesComments} from '../gr-comment-api/gr-comment-api';
import {customElement, observe, property} from '@polymer/decorators';
import {
@@ -276,11 +270,12 @@
'create-comment',
e => this._handleCreateComment(e)
);
- this.addEventListener('comment-discard', e =>
- this._handleCommentDiscard(e)
+ this.addEventListener('comment-discard', () =>
+ this._handleCommentSaveOrDiscard()
);
- this.addEventListener('comment-update', e => this._handleCommentUpdate(e));
- this.addEventListener('comment-save', e => this._handleCommentSave(e));
+ this.addEventListener('comment-save', () =>
+ this._handleCommentSaveOrDiscard()
+ );
this.addEventListener('render-start', () => this._handleRenderStart());
this.addEventListener('render-content', () => this._handleRenderContent());
this.addEventListener('normalize-range', event =>
@@ -932,79 +927,12 @@
: null;
}
- _handleCommentSave(e: CustomEvent) {
- const comment = e.detail.comment;
- const side = e.detail.comment.__commentSide;
- const idx = this._findDraftIndex(comment, side);
- this.set(['comments', side, idx], comment);
- this._handleCommentSaveOrDiscard();
- }
-
- _handleCommentDiscard(e: CustomEvent) {
- const comment = e.detail.comment;
- this._removeComment(comment);
- this._handleCommentSaveOrDiscard();
- }
-
- _handleCommentUpdate(e: CustomEvent) {
- const comment = e.detail.comment;
- const side = e.detail.comment.__commentSide;
- let idx = this._findCommentIndex(comment, side);
- if (idx === -1) {
- idx = this._findDraftIndex(comment, side);
- }
- if (idx !== -1) {
- // Update draft or comment.
- this.set(['comments', side, idx], comment);
- } else {
- // Create new draft.
- this.push(['comments', side], comment);
- }
- }
-
_handleCommentSaveOrDiscard() {
this.dispatchEvent(
new CustomEvent('diff-comments-modified', {bubbles: true, composed: true})
);
}
- _removeComment(comment: UIComment) {
- const side = comment.__commentSide;
- if (!side) throw new Error('Missing required "side" in comment.');
- this._removeCommentFromSide(comment, side);
- }
-
- _removeCommentFromSide(comment: Comment, side: Side) {
- let idx = this._findCommentIndex(comment, side);
- if (idx === -1) {
- idx = this._findDraftIndex(comment, side);
- }
- if (idx !== -1) {
- this.splice('comments.' + side, idx, 1);
- }
- }
-
- _findCommentIndex(comment: Comment, side: Side) {
- if (!comment.id || !this.comments || !this.comments[side]) {
- return -1;
- }
- return this.comments[side].findIndex(item => item.id === comment.id);
- }
-
- _findDraftIndex(comment: Comment, side: Side) {
- if (
- !isDraft(comment) ||
- !comment.__draftID ||
- !this.comments ||
- !this.comments[side]
- ) {
- return -1;
- }
- return this.comments[side].findIndex(
- item => isDraft(item) && item.__draftID === comment.__draftID
- );
- }
-
_isSyntaxHighlightingEnabled(
preferenceChangeRecord?: PolymerDeepPropertyChange<
DiffPreferencesInfo,
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 bf4d912..402cb52 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
@@ -62,198 +62,6 @@
});
});
- suite('handle comment-update', () => {
- setup(() => {
- sinon.stub(element, '_commentsChanged');
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
- });
-
- test('creating a draft', () => {
- const comment = {__draft: true, __draftID: 'tempID', side: 'PARENT',
- __commentSide: 'left'};
- element.dispatchEvent(
- new CustomEvent('comment-update', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- assert.include(element.comments.left, comment);
- });
-
- test('discarding a draft', () => {
- const draftID = 'tempID';
- const id = 'savedID';
- const comment = {
- __draft: true,
- __draftID: draftID,
- side: 'PARENT',
- __commentSide: 'left',
- };
- const diffCommentsModifiedStub = sinon.stub();
- element.addEventListener('diff-comments-modified',
- diffCommentsModifiedStub);
- element.comments.left.push(comment);
- comment.id = id;
- element.dispatchEvent(
- new CustomEvent('comment-discard', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- const drafts = element.comments.left
- .filter(item => item.__draftID === draftID);
- assert.equal(drafts.length, 0);
- assert.isTrue(diffCommentsModifiedStub.called);
- });
-
- test('saving a draft', () => {
- const draftID = 'tempID';
- const id = 'savedID';
- const comment = {
- __draft: true,
- __draftID: draftID,
- side: 'PARENT',
- __commentSide: 'left',
- };
- const diffCommentsModifiedStub = sinon.stub();
- element.addEventListener('diff-comments-modified',
- diffCommentsModifiedStub);
- element.comments.left.push(comment);
- comment.id = id;
- element.dispatchEvent(
- new CustomEvent('comment-save', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- const drafts = element.comments.left
- .filter(item => item.__draftID === draftID);
- assert.equal(drafts.length, 1);
- assert.equal(drafts[0].id, id);
- assert.isTrue(diffCommentsModifiedStub.called);
- });
- });
-
- test('remove comment', () => {
- sinon.stub(element, '_commentsChanged');
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
-
- // Using JSON.stringify because Safari 9.1 (11601.5.17.1) doesn’t seem
- // to believe that one object deepEquals another even when they do :-/.
- assert.equal(JSON.stringify(element.comments), JSON.stringify({
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- }));
-
- element._removeComment({id: 'bc2', side: 'PARENT',
- __commentSide: 'left'});
- assert.deepEqual(element.comments, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- });
-
- element._removeComment({id: 'd2', __commentSide: 'right'});
- assert.deepEqual(element.comments, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- ],
- });
- });
-
test('thread-discard handling', () => {
const threads = createCommentThreads([
{
