From 796b8229b92f95678f85130515861c3e4579b76a Mon Sep 17 00:00:00 2001 From: Ravi Mistry Date: Tue, 9 May 2017 08:45:07 -0400 Subject: [PATCH] Display total comments and unresolved comments in patchset dropdown Bug: Issue 5987 Change-Id: I9270345aa1cb10b7f04b6b871239fb3c7014c5cb --- .../change/gr-change-view/gr-change-view.html | 1 + .../change/gr-change-view/gr-change-view.js | 21 ++++++++++ .../gr-change-view/gr-change-view_test.html | 39 +++++++++++++++++++ .../change/gr-file-list/gr-file-list.js | 17 +++++--- .../gr-file-list/gr-file-list_test.html | 4 ++ 5 files changed, 76 insertions(+), 6 deletions(-) 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 2c89e5cf0f..ff3f8b5239 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 @@ -425,6 +425,7 @@ limitations under the License. [[patchNum.num]] / [[computeLatestPatchNum(_allPatchSets)]] + [[_computePatchSetCommentsString(_comments, patchNum.num)]] [[_computePatchSetDescription(_change, patchNum.num)]] 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 4a50dbb1f5..488a20d7fb 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 @@ -1030,6 +1030,27 @@ rev.description.substring(0, PATCH_DESC_MAX_LENGTH) : ''; }, + _computePatchSetCommentsString: function(allComments, patchNum) { + var numComments = 0; + var numUnresolved = 0; + for (var file in allComments) { + var comments = allComments[file]; + numComments += this.$.fileList.getCommentsForPath( + allComments, patchNum, file).length; + numUnresolved += this.$.fileList.computeUnresolvedNum( + allComments, {}, patchNum, file); + } + var commentsStr = ''; + if (numComments > 0) { + commentsStr = '(' + numComments + ' comments'; + if (numUnresolved > 0) { + commentsStr += ', ' + numUnresolved + ' unresolved'; + } + commentsStr += ')'; + } + return commentsStr; + }, + _computeDescriptionPlaceholder: function(readOnly) { return (readOnly ? 'No' : 'Add a') + ' patch set description'; }, 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 59b3d41ead..cec1c2365c 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 @@ -197,6 +197,45 @@ limitations under the License. assert.equal(result, 'R=\u200Btest@google.com\nR=\u200Btest@google.com'); }), + test('_computePatchSetCommentsString', function() { + // Test string with unresolved comments. + comments = { + 'foo': 'foo comments', + 'bar': 'bar comments', + 'xyz': 'xyz comments', + }; + sandbox.stub(element.$.fileList, 'getCommentsForPath', function(c, p, f) { + if (f == 'foo') { + return ['comment1', 'comment2']; + } else if (f == 'bar') { + return ['comment1']; + } else { + return []; + } + }); + sandbox.stub( + element.$.fileList, 'computeUnresolvedNum', function (c, d, p, f) { + if (f == 'foo') { + return 0; + } else if (f == 'bar') { + return 1; + } else { + return 0; + } + }); + assert.equal(element._computePatchSetCommentsString(comments, 1), + '(3 comments, 1 unresolved)'); + + // Test string with no unresolved comments. + delete comments['bar'] + assert.equal(element._computePatchSetCommentsString(comments, 1), + '(2 comments)'); + + // Test string with no comments. + delete comments['foo'] + assert.equal(element._computePatchSetCommentsString(comments, 1), ''); + }); + test('_handleDescriptionChanged', function() { var putDescStub = sandbox.stub(element.$.restAPI, 'setDescription') .returns(Promise.resolve({ok: true})); 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 307b3479f6..91d5eca885 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 @@ -294,7 +294,7 @@ return commentCount ? commentCount + 'c' : ''; }, - _getCommentsForPath: function(comments, patchNum, path) { + getCommentsForPath: function(comments, patchNum, path) { return (comments[path] || []).filter(function(c) { return parseInt(c.patch_set, 10) === parseInt(patchNum, 10); }); @@ -303,7 +303,7 @@ _computeCountString: function(comments, patchNum, path, opt_noun) { if (!comments) { return ''; } - var patchComments = this._getCommentsForPath(comments, patchNum, path); + var patchComments = this.getCommentsForPath(comments, patchNum, path); var num = patchComments.length; if (num === 0) { return ''; } if (!opt_noun) { return num; } @@ -322,8 +322,14 @@ * @return {string} */ _computeUnresolvedString: function(comments, drafts, patchNum, path) { - comments = this._getCommentsForPath(comments, patchNum, path); - drafts = this._getCommentsForPath(drafts, patchNum, path); + var unresolvedNum = this.computeUnresolvedNum( + comments, drafts, patchNum, path); + return unresolvedNum === 0 ? '' : '(' + unresolvedNum + ' unresolved)'; + }, + + computeUnresolvedNum: function(comments, drafts, patchNum, path) { + comments = this.getCommentsForPath(comments, patchNum, path); + drafts = this.getCommentsForPath(drafts, patchNum, path); comments = comments.concat(drafts); // Create an object where every comment ID is the key of an unresolved @@ -346,8 +352,7 @@ return idMap[key]; }); - return unresolvedLeaves.length === 0 ? - '' : '(' + unresolvedLeaves.length + ' unresolved)'; + return unresolvedLeaves.length; }, _computeReviewed: function(file, _reviewed) { 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 4abd68a31b..e51c33aa6d 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 @@ -521,9 +521,13 @@ limitations under the License. 'unresolved.file', 'comment'), '3 comments'); assert.equal( element._computeUnresolvedString(comments, [], 2, 'myfile.txt'), ''); + assert.equal( + element.computeUnresolvedNum(comments, [], 2, 'myfile.txt'), 0); assert.equal( element._computeUnresolvedString(comments, [], 2, 'unresolved.file'), '(1 unresolved)'); + assert.equal( + element.computeUnresolvedNum(comments, [], 2, 'unresolved.file'), 1); assert.equal( element._computeUnresolvedString(comments, drafts, 2, 'unresolved.file'), '');