Update sorting for threads

Threads are sorted by:
1. unresolved & has drafts should show up first
2. then sort by file path
3. then file level comments first
4. then sort by line
5. then sort by updated time
6. then sort by id

This should avoid most of the re-arrangements
from previous sort by date solution.
The re-arrangement will still happen if you change a comment
from unresolved to resolved or vice versa

Change-Id: I2e09211d5ab3c861c2346668d6c2b64118301972
This commit is contained in:
Tao Zhou 2020-03-30 09:35:34 +02:00
parent a3f0c6b640
commit a0d86ae964
2 changed files with 46 additions and 23 deletions
polygerrit-ui/app/elements/change/gr-thread-list

@ -116,6 +116,12 @@ class GrThreadList extends GestureEventListeners(
_updateSortedThreads(threads) {
this._sortedThreads =
threads.map(this._getThreadWithSortInfo).sort((c1, c2) => {
// threads will be sorted by:
// - unresolved first
// - with drafts
// - file path
// - line
// - updated time
const c1Date = c1.__date || util.parseDate(c1.updated);
const c2Date = c2.__date || util.parseDate(c2.updated);
const dateCompare = c2Date - c1Date;
@ -123,11 +129,28 @@ class GrThreadList extends GestureEventListeners(
if (!c1.unresolved) { return 1; }
if (!c2.unresolved) { return -1; }
}
if (c2.hasDraft || c1.hasDraft) {
if (!c1.hasDraft) { return 1; }
if (!c2.hasDraft) { return -1; }
}
// TODO: Update here once we introduce patchset level comments
// they may not have or have a special line or path attribute
if (c1.thread.path !== c2.thread.path) {
return c1.thread.path.localeCompare(c2.thread.path);
}
// File level comments (no `line` property)
// should always show before any lines
if ([c1, c2].some(c => c.thread.line === undefined)) {
if (!c1.thread.line) { return -1; }
if (!c2.thread.line) { return 1; }
} else if (c1.thread.line !== c2.thread.line) {
return c1.thread.line - c2.thread.line;
}
if (dateCompare === 0 && (!c1.id || !c1.id.localeCompare)) {
return 0;
}

@ -114,7 +114,6 @@ suite('gr-thread-list tests', () => {
},
patch_set: 2,
id: '8caddf38_44770ec1',
line: 4,
updated: '2018-02-13 22:48:40.000000000',
message: 'Another unresolved comment',
unresolved: true,
@ -122,7 +121,6 @@ suite('gr-thread-list tests', () => {
],
patchNum: 2,
path: '/COMMIT_MSG',
line: 4,
rootId: '8caddf38_44770ec1',
start_datetime: '2018-02-13 22:48:40.000000000',
},
@ -205,7 +203,7 @@ suite('gr-thread-list tests', () => {
},
patch_set: 4,
id: 'rc2',
line: 5,
line: 7,
updated: '2019-03-08 18:49:18.000000000',
message: 'test',
unresolved: true,
@ -228,7 +226,7 @@ suite('gr-thread-list tests', () => {
],
patchNum: 4,
path: '/COMMIT_MSG',
line: 5,
line: 7,
rootId: 'rc2',
start_datetime: '2019-03-08 18:49:18.000000000',
},
@ -259,25 +257,26 @@ suite('gr-thread-list tests', () => {
test('_computeSortedThreads', () => {
assert.equal(element._sortedThreads.length, 7);
// Draft and unresolved
// Draft and unresolved for commit-msg at line 5
assert.equal(element._sortedThreads[0].thread.rootId,
'ecf0b9fa_fe1a5f62');
// Unresolved robot comment
// /COMMIT_MSG
// unresolved no draft and file level
assert.equal(element._sortedThreads[1].thread.rootId,
'rc2');
// Unresolved robot comment
assert.equal(element._sortedThreads[2].thread.rootId,
'rc1');
// unresolved
assert.equal(element._sortedThreads[3].thread.rootId,
'scaddf38_44770ec1');
// unresolved
assert.equal(element._sortedThreads[4].thread.rootId,
'8caddf38_44770ec1');
// resolved and draft
// unresolved no draft at line 4
assert.equal(element._sortedThreads[2].thread.rootId,
'scaddf38_44770ec1');
// unresolved no draft at line 5
assert.equal(element._sortedThreads[3].thread.rootId,
'rc1');
// Unresolved no draft at line 7
assert.equal(element._sortedThreads[4].thread.rootId,
'rc2');
// resolved and draft on COMMIT_MSG
assert.equal(element._sortedThreads[5].thread.rootId,
'zcf0b9fa_fe1a5f62');
// resolved
// resolved and on file test.txt
assert.equal(element._sortedThreads[6].thread.rootId,
'09a9fb0a_1484e6cf');
});
@ -298,19 +297,20 @@ suite('gr-thread-list tests', () => {
assert.equal(element._sortedThreads.length, 6);
assert.equal(element._sortedThreads[0].thread.rootId,
'ecf0b9fa_fe1a5f62');
// Unresolved robot comment
// /COMMIT_MSG
// unresolved no draft and file level
assert.equal(element._sortedThreads[1].thread.rootId,
'rc1');
// unresolved
'8caddf38_44770ec1');
// unresolved no draft at line 4
assert.equal(element._sortedThreads[2].thread.rootId,
'scaddf38_44770ec1');
// unresolved
// unresolved no draft at line 5
assert.equal(element._sortedThreads[3].thread.rootId,
'8caddf38_44770ec1');
'rc1');
// resolved and draft
assert.equal(element._sortedThreads[4].thread.rootId,
'zcf0b9fa_fe1a5f62');
// resolved
// resolved and on file test.txt
assert.equal(element._sortedThreads[5].thread.rootId,
'09a9fb0a_1484e6cf');
});