Merge "Update sorting for threads"

polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js

@@ -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;
           }

polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html

@@ -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');
   });