Merge "Convert comment counts to comment thread counts around the UI" into stable-3.3
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 3243af0..e188254 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -637,17 +637,17 @@
patchNum: patchRange.patchNum,
path,
});
- const commentCount =
- changeComments.computeCommentCount({
+ const commentThreadCount =
+ changeComments.computeCommentThreadCount({
patchNum: patchRange.basePatchNum,
path,
}) +
- changeComments.computeCommentCount({
+ changeComments.computeCommentThreadCount({
patchNum: patchRange.patchNum,
path,
});
const commentString = GrCountStringFormatter.computePluralString(
- commentCount,
+ commentThreadCount,
'comment'
);
const unresolvedString = GrCountStringFormatter.computeString(
@@ -733,16 +733,16 @@
) {
return '';
}
- const commentCount =
- changeComments.computeCommentCount({
+ const commentThreadCount =
+ changeComments.computeCommentThreadCount({
patchNum: patchRange.basePatchNum,
path,
}) +
- changeComments.computeCommentCount({
+ changeComments.computeCommentThreadCount({
patchNum: patchRange.patchNum,
path,
});
- return GrCountStringFormatter.computeShortString(commentCount, 'c');
+ return GrCountStringFormatter.computeShortString(commentThreadCount, 'c');
}
private _reviewFile(path: string, reviewed?: boolean) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index 5cb9f15..d85ae4d 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -355,36 +355,66 @@
test('comment filtering', () => {
const comments = {
'/COMMIT_MSG': [
- {patch_set: 1, message: 'Done', updated: '2017-02-08 16:40:49'},
- {patch_set: 1, message: 'oh hay', updated: '2017-02-09 16:40:49'},
- {patch_set: 2, message: 'hello', updated: '2017-02-10 16:40:49'},
+ {
+ patch_set: 1,
+ message: 'Done',
+ updated: '2017-02-08 16:40:49',
+ id: '1',
+ },
+ {
+ patch_set: 1,
+ message: 'oh hay',
+ updated: '2017-02-09 16:40:49',
+ id: '2',
+ },
+ {
+ patch_set: 2,
+ message: 'hello',
+ updated: '2017-02-10 16:40:49',
+ id: '3',
+ },
],
'myfile.txt': [
- {patch_set: 1, message: 'good news!', updated: '2017-02-08 16:40:49'},
- {patch_set: 2, message: 'wat!?', updated: '2017-02-09 16:40:49'},
- {patch_set: 2, message: 'hi', updated: '2017-02-10 16:40:49'},
+ {
+ patch_set: 1,
+ message: 'good news!',
+ updated: '2017-02-08 16:40:49',
+ id: '4',
+ },
+ {
+ patch_set: 2,
+ message: 'wat!?',
+ updated: '2017-02-09 16:40:49',
+ id: '5',
+ },
+ {
+ patch_set: 2,
+ message: 'hi',
+ updated: '2017-02-10 16:40:49',
+ id: '6',
+ },
],
'unresolved.file': [
{
patch_set: 2,
message: 'wat!?',
updated: '2017-02-09 16:40:49',
- id: '1',
+ id: '7',
unresolved: true,
},
{
patch_set: 2,
message: 'hi',
updated: '2017-02-10 16:40:49',
- id: '2',
- in_reply_to: '1',
+ id: '8',
+ in_reply_to: '7',
unresolved: false,
},
{
patch_set: 2,
message: 'good news!',
updated: '2017-02-08 16:40:49',
- id: '3',
+ id: '9',
unresolved: true,
},
],
@@ -395,14 +425,14 @@
patch_set: 1,
message: 'hi',
updated: '2017-02-15 16:40:49',
- id: '5',
+ id: '10',
unresolved: true,
},
{
patch_set: 1,
message: 'fyi',
updated: '2017-02-15 16:40:49',
- id: '6',
+ id: '11',
unresolved: false,
},
],
@@ -411,7 +441,7 @@
patch_set: 1,
message: 'hi',
updated: '2017-02-11 16:40:49',
- id: '4',
+ id: '12',
unresolved: false,
},
],
@@ -570,10 +600,10 @@
'file_added_in_rev2.txt', 'comment'), '');
assert.equal(
element._computeCommentsString(element.changeComments, parentTo2,
- 'unresolved.file', 'comment'), '3 comments (1 unresolved)');
+ 'unresolved.file', 'comment'), '2 comments (1 unresolved)');
assert.equal(
element._computeCommentsString(element.changeComments, _1To2,
- 'unresolved.file', 'comment'), '3 comments (1 unresolved)');
+ 'unresolved.file', 'comment'), '2 comments (1 unresolved)');
});
test('_reviewedTitle', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 8dce63f..367bbd6 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -436,14 +436,19 @@
}
/**
- * Computes a string counting the number of commens in a given file.
+ * Computes the number of comment threads in a given file or patch.
*/
- computeCommentCount(file: PatchSetFile | PatchNumOnly) {
+ computeCommentThreadCount(file: PatchSetFile | PatchNumOnly) {
+ let comments: Comment[] = [];
if (isPatchSetFile(file)) {
- return this.getAllCommentsForFile(file).length;
+ comments = this.getAllCommentsForFile(file);
+ } else {
+ comments = this._commentObjToArray(
+ this.getAllPublishedComments(file.patchNum)
+ );
}
- const allComments = this.getAllPublishedComments(file.patchNum);
- return this._commentObjToArray(allComments).length;
+
+ return this.getCommentThreads(comments).length;
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
index 2cf9bc1..be6f646 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
@@ -220,14 +220,14 @@
const drafts = {
'file/one': [
{
- id: '11',
+ id: '12',
patch_set: 2,
side: PARENT,
line: 1,
updated: makeTime(3),
},
{
- id: '12',
+ id: '13',
in_reply_to: '04',
patch_set: 2,
line: 1,
@@ -289,21 +289,30 @@
id: '07',
patch_set: 2,
side: PARENT,
- unresolved: true,
+ unresolved: false,
line: 1,
updated: makeTime(1),
},
- {id: '08', patch_set: 3, line: 1, updated: makeTime(1)},
+ {
+ id: '08',
+ patch_set: 2,
+ side: PARENT,
+ unresolved: true,
+ in_reply_to: '07',
+ line: 1,
+ updated: makeTime(1),
+ },
+ {id: '09', patch_set: 3, line: 1, updated: makeTime(1)},
],
'file/four': [
{
- id: '09',
+ id: '10',
patch_set: 5,
side: PARENT,
line: 1,
updated: makeTime(1),
},
- {id: '10', patch_set: 5, line: 1, updated: makeTime(1)},
+ {id: '11', patch_set: 5, line: 1, updated: makeTime(1)},
],
};
element._changeComments =
@@ -439,19 +448,19 @@
element._changeComments.computeUnresolvedNum(1, 'path'), 0);
});
- test('computeCommentCount', () => {
+ test('computeCommentThreadCount', () => {
assert.equal(element._changeComments
- .computeCommentCount({
+ .computeCommentThreadCount({
patchNum: 2,
path: 'file/one',
- }), 4);
+ }), 3);
assert.equal(element._changeComments
- .computeCommentCount({
+ .computeCommentThreadCount({
patchNum: 1,
path: 'file/one',
}), 0);
assert.equal(element._changeComments
- .computeCommentCount({
+ .computeCommentThreadCount({
patchNum: 2,
path: 'file/three',
}), 1);
@@ -572,7 +581,7 @@
updated: '2013-02-26 15:03:43.986000000',
},
{
- id: '12',
+ id: '13',
in_reply_to: '04',
patch_set: 2,
line: 1,
@@ -622,6 +631,17 @@
id: '07',
patch_set: 2,
side: 'PARENT',
+ unresolved: false,
+ line: 1,
+ path: 'file/three',
+ __path: 'file/three',
+ updated: '2013-02-26 15:01:43.986000000',
+ },
+ {
+ id: '08',
+ in_reply_to: '07',
+ patch_set: 2,
+ side: 'PARENT',
unresolved: true,
line: 1,
path: 'file/three',
@@ -637,7 +657,7 @@
}, {
comments: [
{
- id: '08',
+ id: '09',
patch_set: 3,
line: 1,
path: 'file/three',
@@ -648,11 +668,11 @@
patchNum: 3,
path: 'file/three',
line: 1,
- rootId: '08',
+ rootId: '09',
}, {
comments: [
{
- id: '09',
+ id: '10',
patch_set: 5,
side: 'PARENT',
line: 1,
@@ -665,11 +685,11 @@
patchNum: 5,
path: 'file/four',
line: 1,
- rootId: '09',
+ rootId: '10',
}, {
comments: [
{
- id: '10',
+ id: '11',
patch_set: 5,
line: 1,
path: 'file/four',
@@ -677,7 +697,7 @@
updated: '2013-02-26 15:01:43.986000000',
},
],
- rootId: '10',
+ rootId: '11',
patchNum: 5,
path: 'file/four',
line: 1,
@@ -700,7 +720,7 @@
}, {
comments: [
{
- id: '11',
+ id: '12',
patch_set: 2,
side: 'PARENT',
line: 1,
@@ -710,7 +730,7 @@
updated: '2013-02-26 15:03:43.986000000',
},
],
- rootId: '11',
+ rootId: '12',
commentSide: 'PARENT',
patchNum: 2,
path: 'file/one',
@@ -745,7 +765,7 @@
__path: 'file/one',
path: 'file/one',
__draft: true,
- id: '12',
+ id: '13',
in_reply_to: '04',
patch_set: 2,
line: 1,
@@ -756,7 +776,7 @@
expectedComments);
expectedComments = [{
- id: '11',
+ id: '12',
patch_set: 2,
side: 'PARENT',
line: 1,
@@ -766,7 +786,7 @@
updated: '2013-02-26 15:03:43.986000000',
}];
- assert.deepEqual(element._changeComments.getCommentsForThread('11'),
+ assert.deepEqual(element._changeComments.getCommentsForThread('12'),
expectedComments);
assert.deepEqual(element._changeComments.getCommentsForThread('1000'),
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index f749f0e..d0be880 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -1369,9 +1369,12 @@
patchNum,
path,
});
- const commentCount = changeComments.computeCommentCount({patchNum, path});
- const commentString = GrCountStringFormatter.computePluralString(
- commentCount,
+ const commentThreadCount = changeComments.computeCommentThreadCount({
+ patchNum,
+ path,
+ });
+ const commentThreadString = GrCountStringFormatter.computePluralString(
+ commentThreadCount,
'comment'
);
const unresolvedString = GrCountStringFormatter.computeString(
@@ -1381,7 +1384,7 @@
const unmodifiedString = changeFileInfo.status === 'U' ? 'no changes' : '';
- return [unmodifiedString, commentString, unresolvedString]
+ return [unmodifiedString, commentThreadString, unresolvedString]
.filter(v => v && v.length > 0)
.join(', ');
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 569e6df..9a08723 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -148,7 +148,7 @@
path: '/COMMIT_MSG',
},
]},
- computeCommentCount: () => {},
+ computeCommentThreadCount: () => {},
computeUnresolvedNum: () => {},
getPaths: () => {},
getCommentsBySideForPath: () => {},
@@ -903,14 +903,14 @@
test('_computeCommentString', done => {
const path = '/test';
element.$.commentAPI.loadAll().then(comments => {
- const commentCountStub =
- sinon.stub(comments, 'computeCommentCount');
+ const commentThreadCountStub =
+ sinon.stub(comments, 'computeCommentThreadCount');
const unresolvedCountStub =
sinon.stub(comments, 'computeUnresolvedNum');
- commentCountStub.withArgs({patchNum: 1, path}).returns(0);
- commentCountStub.withArgs({patchNum: 2, path}).returns(1);
- commentCountStub.withArgs({patchNum: 3, path}).returns(2);
- commentCountStub.withArgs({patchNum: 4, path}).returns(0);
+ commentThreadCountStub.withArgs({patchNum: 1, path}).returns(0);
+ commentThreadCountStub.withArgs({patchNum: 2, path}).returns(1);
+ commentThreadCountStub.withArgs({patchNum: 3, path}).returns(2);
+ commentThreadCountStub.withArgs({patchNum: 4, path}).returns(0);
unresolvedCountStub.withArgs({patchNum: 1, path}).returns(1);
unresolvedCountStub.withArgs({patchNum: 2, path}).returns(0);
unresolvedCountStub.withArgs({patchNum: 3, path}).returns(2);
@@ -957,8 +957,8 @@
basePatchNum: PARENT,
patchNum: 10,
};
- // computeCommentCount is an empty function hence stubbing function
- // that depends on it's return value
+ // computeCommentThreadCount is an empty function hence stubbing
+ // function that depends on it's return value
sinon.stub(element, '_computeCommentString').returns('');
element._change = {_number: 42};
element._files = getFilesFromFileList(
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index 2b997fd..2a9fe54 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -375,9 +375,11 @@
return;
}
- const commentCount = changeComments.computeCommentCount({patchNum});
- const commentString = GrCountStringFormatter.computePluralString(
- commentCount,
+ const commentThreadCount = changeComments.computeCommentThreadCount({
+ patchNum,
+ });
+ const commentThreadString = GrCountStringFormatter.computePluralString(
+ commentThreadCount,
'comment'
);
@@ -387,14 +389,14 @@
'unresolved'
);
- if (!commentString.length && !unresolvedString.length) {
+ if (!commentThreadString.length && !unresolvedString.length) {
return '';
}
return (
- ` (${commentString}` +
- // Add a comma + space if both comments and unresolved
- (commentString && unresolvedString ? ', ' : '') +
+ ` (${commentThreadString}` +
+ // Add a comma + space if both comment threads and unresolved
+ (commentThreadString && unresolvedString ? ', ' : '') +
`${unresolvedString})`
);
}
@@ -437,7 +439,7 @@
previous: detail.patchNum,
current: e.detail.value,
latest: latestPatchNum,
- commentCount: this.changeComments?.computeCommentCount({
+ commentCount: this.changeComments?.computeCommentThreadCount({
patchNum: e.detail.value as PatchSetNum,
}),
});
@@ -447,7 +449,7 @@
this.reporting.reportInteraction('left-patchset-changed', {
previous: detail.basePatchNum,
current: e.detail.value,
- commentCount: this.changeComments?.computeCommentCount({
+ commentCount: this.changeComments?.computeCommentThreadCount({
patchNum: patchSetValue,
}),
});
