Add reloadDrafts function in gr-comment-api This function also re-initializes a ChangeComment object, but uses the old values for comments and robot comments, and only makes a request for new drafts. It's response is identical to loadAll for the purposes of elements using these functions, but will reduce unneeded API calls. Change-Id: I3c0872b86a0e24c9c378acf3311fe8cc3e078eda
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html index 2975300..fa53bfe 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -469,7 +469,7 @@ file-list-increment="{{_numFilesShown}}" on-files-shown-changed="_setShownFiles" on-file-action-tap="_handleFileActionTap" - on-reload-comments="_reloadCommentsWithCallback"></gr-file-list> + on-reload-drafts="_reloadDraftsWithCallback"></gr-file-list> </section> <gr-endpoint-decorator name="change-view-integration"> </gr-endpoint-decorator>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js index ffabb37..0790c3e 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -259,7 +259,7 @@ }); this.addEventListener('comment-save', this._handleCommentSave.bind(this)); - this.addEventListener('comment-refresh', this._reloadComments.bind(this)); + this.addEventListener('comment-refresh', this._reloadDrafts.bind(this)); this.addEventListener('comment-discard', this._handleCommentDiscard.bind(this)); this.addEventListener('editable-content-save', @@ -1037,12 +1037,16 @@ }); }, - _reloadCommentsWithCallback(e) { - return this._reloadComments().then(() => { + _reloadDraftsWithCallback(e) { + return this._reloadDrafts().then(() => { return e.detail.resolve(); }); }, + /** + * Fetches a new changeComment object, and data for all types of comments + * (comments, robot comments, draft comments) is requested. + */ _reloadComments() { return this.$.commentAPI.loadAll(this._changeNum) .then(comments => { @@ -1051,6 +1055,18 @@ }); }, + /** + * Fetches a new changeComment object, but only updated data for drafts is + * requested. + */ + _reloadDrafts() { + return this.$.commentAPI.reloadDrafts(this._changeNum) + .then(comments => { + this._changeComments = comments; + this._diffDrafts = Object.assign({}, this._changeComments.drafts); + }); + }, + _reload() { this._loading = true; this._relatedChangesCollapsed = true;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html index a455c18..09860ec 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -241,40 +241,41 @@ }); }); - test('comments are reloaded when reload-comments fired', done => { - const reloadStub = sandbox.stub(element.$.commentAPI, 'loadAll') + suite('reloading drafts', () => { + let reloadStub; + const drafts = { + 'testfile.txt': [ + { + patch_set: 5, + id: 'dd2982f5_c01c9e6a', + line: 1, + updated: '2017-11-08 18:47:45.000000000', + message: 'test', + unresolved: true, + }, + ], + }; + setup(() => { + reloadStub = sandbox.stub(element.$.commentAPI, 'reloadDrafts') .returns(Promise.resolve({ - drafts: { - 'testfile.txt': [ - { - patch_set: 5, - id: 'dd2982f5_c01c9e6a', - line: 1, - updated: '2017-11-08 18:47:45.000000000', - message: 'test', - unresolved: true, - }, - ], - }, + drafts, } )); - element.$.fileList.fire('reload-comments', { - resolve: () => { - assert.isTrue(reloadStub.called); - assert.deepEqual(element._diffDrafts, { - 'testfile.txt': [ - { - patch_set: 5, - id: 'dd2982f5_c01c9e6a', - line: 1, - updated: '2017-11-08 18:47:45.000000000', - message: 'test', - unresolved: true, - }, - ], - }); - done(); - }, + }); + + test('drafts are reloaded when reload-drafts fired', done => { + element.$.fileList.fire('reload-drafts', { + resolve: () => { + assert.isTrue(reloadStub.called); + assert.deepEqual(element._diffDrafts, drafts); + done(); + }, + }); + }); + + test('drafts are reloaded when comment-refresh fired', () => { + element.fire('comment-refresh'); + assert.isTrue(reloadStub.called); }); });
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js index 38ba921..6e71a80 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -33,9 +33,9 @@ is: 'gr-file-list', /** - * Fired when a comment refresh should get triggered + * Fired when a draft refresh should get triggered * - * @event reload-comments + * @event reload-drafts */ properties: { @@ -876,7 +876,7 @@ let iter = 0; return (new Promise(resolve => { - this.fire('reload-comments', {resolve}); + this.fire('reload-drafts', {resolve}); })).then(() => { return this.asyncForeach(paths, path => { iter++;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html index ed7b665..2cdb3c4 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -34,7 +34,7 @@ <template> <gr-file-list id="fileList" change-comments="[[_changeComments]]" - on-reload-comments="_reloadCommentsWithCallback"></gr-file-list> + on-reload-drafts="_reloadDraftsWithCallback"></gr-file-list> <gr-comment-api id="commentAPI"></gr-comment-api> </template> <script src="../../diff/gr-comment-api/gr-comment-api-mock.js"></script>
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api-mock.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api-mock.js index cfcafd3..3e554c6 100644 --- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api-mock.js +++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api-mock.js
@@ -25,7 +25,13 @@ return this._reloadComments(); }, - _reloadCommentsWithCallback(e) { + /** + * For the purposes of the mock, _reloadDrafts is not included because its + * response is the same type as reloadComments, just makes less API + * requests. Since this is for test purposes/mocked data anyway, keep this + * file simpler by just using _reloadComments here instead. + */ + _reloadDraftsWithCallback(e) { return this._reloadComments().then(() => { return e.detail.resolve(); });
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.js index 8fc3a64..c41c731 100644 --- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.js +++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.js
@@ -200,7 +200,7 @@ }, listeners: { - 'reload-comments': 'loadAll', + 'reload-drafts': 'reloadDrafts', }, behaviors: [ @@ -213,7 +213,7 @@ * does not yield the comment data. * * @param {number} changeNum - * @return {!Promise} + * @return {!Promise<!Object>} */ loadAll(changeNum) { const promises = []; @@ -227,5 +227,25 @@ return this._changeComments; }); }, + + + /** + * Re-initialize _changeComments with a new ChangeComments object, that + * uses the previous values for comments and robot comments, but fetches + * updated draft comments. + * + * @param {number} changeNum + * @return {!Promise<!Object>} + */ + reloadDrafts(changeNum) { + if (!this._changeComments) { + return this.loadAll(changeNum); + } + return this.$.restAPI.getDiffDrafts(changeNum).then(drafts => { + this._changeComments = new ChangeComments(this._changeComments.comments, + this._changeComments.robotComments, drafts, changeNum); + return this._changeComments; + }); + }, }); })();
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.html b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.html index eae249d..d7ce77e 100644 --- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.html +++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.html
@@ -100,6 +100,50 @@ }); }); + suite('reloadDrafts', () => { + let commentStub; + let robotCommentStub; + let draftStub; + setup(() => { + commentStub = sandbox.stub(element.$.restAPI, 'getDiffComments') + .returns(Promise.resolve({})); + robotCommentStub = sandbox.stub(element.$.restAPI, + 'getDiffRobotComments').returns(Promise.resolve({})); + draftStub = sandbox.stub(element.$.restAPI, 'getDiffDrafts') + .returns(Promise.resolve({})); + }); + + test('without loadAll first', done => { + assert.isNotOk(element._changeComments); + sandbox.spy(element, 'loadAll'); + element.reloadDrafts().then(() => { + assert.isTrue(element.loadAll.called); + assert.isOk(element._changeComments); + assert.equal(commentStub.callCount, 1); + assert.equal(robotCommentStub.callCount, 1); + assert.equal(draftStub.callCount, 1); + done(); + }); + }); + + test('with loadAll first', done => { + assert.isNotOk(element._changeComments); + element.loadAll().then(() => { + assert.isOk(element._changeComments); + assert.equal(commentStub.callCount, 1); + assert.equal(robotCommentStub.callCount, 1); + assert.equal(draftStub.callCount, 1); + return element.reloadDrafts(); + }).then(() => { + assert.isOk(element._changeComments); + assert.equal(commentStub.callCount, 1); + assert.equal(robotCommentStub.callCount, 1); + assert.equal(draftStub.callCount, 2); + done(); + }); + }); + }); + suite('_changeComment methods', () => { setup(done => { const changeNum = 1234;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js index 7c46b58..5431a78 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -751,8 +751,8 @@ }, _loadComments() { - return this.$.commentAPI.loadAll(this._changeNum).then(() => { - this._changeComments = this.$.commentAPI._changeComments; + return this.$.commentAPI.loadAll(this._changeNum).then(comments => { + this._changeComments = comments; this._commentMap = this._getPaths(this._patchRange); this._commentsForDiff = this._getCommentsForPath(this._path,