opendev/gerrit
Code Issues Proposed changes

A few updates to gr-diff-comment-thread-group

Browse Source 
- getPatchNum should check patch_set rather than patchNum. This fixes a
   previous bug due to checking the wrong attribute that is surfaced
   when threads in a unified view's thread group come from both the
   patch and base patch num.
 - Adds logic for sorting drafts, which should always be at the end,
   regardless of date
 - Use the draftID as rootId if it's the first (only) comment in a thread

Change-Id: I93c92dc77e158e245697e2f0ae01cde0af3a7ae1
This commit is contained in:
Becky Siegel
2018-02-28 16:43:12 -08:00
parent d14334fe2d
commit 3ae53d1ab8
2 changed files with 45 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
View File

@@ -138,12 +138,15 @@
* there are unsaved drafts.
*/
_getPatchNum(comment) {
return comment.patchNum || this.patchForNewThreads;
return comment.patch_set || this.patchForNewThreads;
},
_getThreads(comments) {
const sortedComments = comments.slice(0).sort((a, b) =>
util.parseDate(a.updated) - util.parseDate(b.updated));
const sortedComments = comments.slice(0).sort((a, b) => {
if (b.__draft && !a.__draft ) { return 0; }
if (a.__draft && !b.__draft ) { return 1; }
return util.parseDate(a.updated) - util.parseDate(b.updated);
});
const threads = [];
for (const comment of sortedComments) {
@@ -164,7 +167,7 @@
comments: [comment],
commentSide: comment.__commentSide,
patchNum: this._getPatchNum(comment),
rootId: comment.id,
rootId: comment.id || comment.__draftID,
};
if (comment.range) {
newThread.range = Object.assign({}, comment.range);

39
polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
View File

@@ -65,6 +65,13 @@ limitations under the License.
__commentSide: 'left',
in_reply_to: 'sallys_confession',
},
{
__draftID: 'new_draft',
message: 'i do not like either of you',
__commentSide: 'left',
__draft: true,
updated: '2015-12-20 15:01:20.396000000',
},
];
let expectedThreadGroups = [
@@ -86,6 +93,21 @@ limitations under the License.
rootId: 'sallys_confession',
patchNum: 3,
},
{
start_datetime: '2015-12-20 15:01:20.396000000',
commentSide: 'left',
comments: [
{
__draftID: 'new_draft',
message: 'i do not like either of you',
__commentSide: 'left',
__draft: true,
updated: '2015-12-20 15:01:20.396000000',
},
],
rootId: 'new_draft',
patchNum: 3,
},
];
assert.deepEqual(element._getThreads(comments), expectedThreadGroups);
@@ -147,6 +169,21 @@ limitations under the License.
end_character: 2,
},
},
{
start_datetime: '2015-12-20 15:01:20.396000000',
commentSide: 'left',
comments: [
{
__draftID: 'new_draft',
message: 'i do not like either of you',
__commentSide: 'left',
__draft: true,
updated: '2015-12-20 15:01:20.396000000',
},
],
rootId: 'new_draft',
patchNum: 3,
},
];
assert.deepEqual(element._getThreads(comments), expectedThreadGroups);
@@ -266,7 +303,7 @@ limitations under the License.
updated: '2015-12-23 15:00:20.396000000',
};
assert.equal(element._getPatchNum(comment), 3);
comment.patchNum = 4;
comment.patch_set = 4;
assert.equal(element._getPatchNum(comment), 4);
});