Coalesce request for drafts with login check
Because requests for a user's draft diff comments is always nested
within a check for whether a user is logged in, these can be combined
into the same REST API interface call. The updated call is given a JSDoc
to describe the additional functionality and call sites are simplified.
Change-Id: Idc0cc6298c7287579ba76e7cc35701e62142bca3
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 81e5371..d33ed23 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
@@ -51,11 +51,7 @@
.then(comments => { this._comments = comments; }));
promises.push(this.$.restAPI.getDiffRobotComments(changeNum)
.then(robotComments => { this._robotComments = robotComments; }));
- promises.push(this.$.restAPI.getLoggedIn()
- .then(loggedIn => {
- if (!loggedIn) { return Promise.resolve({}); }
- return this.$.restAPI.getDiffDrafts(changeNum);
- })
+ promises.push(this.$.restAPI.getDiffDrafts(changeNum)
.then(drafts => { this._drafts = drafts; }));
return Promise.all(promises);
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 1fdc872..09403a4 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
@@ -57,14 +57,16 @@
}));
sandbox.stub(element.$.restAPI, 'getDiffRobotComments')
.returns(Promise.resolve({'foo.c': [{id: '321', message: 'done'}]}));
- sandbox.stub(element.$.restAPI, 'getDiffDrafts');
+ sandbox.stub(element.$.restAPI, 'getDiffDrafts')
+ .returns(Promise.resolve({}));
return element.loadAll(changeNum).then(() => {
assert.isTrue(element.$.restAPI.getDiffComments.calledWithExactly(
changeNum));
assert.isTrue(element.$.restAPI.getDiffRobotComments.calledWithExactly(
changeNum));
- assert.isFalse(element.$.restAPI.getDiffDrafts.called);
+ assert.isTrue(element.$.restAPI.getDiffDrafts.calledWithExactly(
+ changeNum));
assert.isOk(element._comments);
assert.isOk(element._robotComments);
assert.deepEqual(element._drafts, {});
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 e4901f5..8655aa0 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
@@ -730,10 +730,7 @@
},
_getDiffDrafts() {
- return this._getLoggedIn().then(loggedIn => {
- if (!loggedIn) { return Promise.resolve({}); }
- return this.$.restAPI.getDiffDrafts(this._changeNum);
- });
+ return this.$.restAPI.getDiffDrafts(this._changeNum);
},
_computeCommentSkips(commentMap, fileList, path) {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 1865c4a..0ba6ec1 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -935,9 +935,23 @@
patchNum, opt_path);
},
+ /**
+ * If the user is logged in, fetch the user's draft diff comments. If there
+ * is no logged in user, the request is not made and the promise yields an
+ * empty object.
+ *
+ * @param {number|string} changeNum
+ * @param {number|string} opt_basePatchNum
+ * @param {number|string} opt_patchNum
+ * @param {string} opt_path
+ * @return {Promise<Object>}
+ */
getDiffDrafts(changeNum, opt_basePatchNum, opt_patchNum, opt_path) {
- return this._getDiffComments(changeNum, '/drafts', opt_basePatchNum,
- opt_patchNum, opt_path);
+ return this.getLoggedIn().then(loggedIn => {
+ if (!loggedIn) { return Promise.resolve({}); }
+ return this._getDiffComments(changeNum, '/drafts', opt_basePatchNum,
+ opt_patchNum, opt_path);
+ });
},
_setRange(comments, comment) {