From b84bf91c22ce5e225d8463377ba3bcc69c2a3434 Mon Sep 17 00:00:00 2001
From: Dhruv Srivastava <dhruvsri@google.com>
Date: Mon, 3 Feb 2020 19:05:59 +0100
Subject: [PATCH] Ensure all drafts are moved after comments during sorting

* The comment threads are formed after processing the comments in a
sorted order by timestamp and moving all drafts to the end. Each comment
is checked to see if it can be attached to an existing thread or not.
If not, then a new comment thread is formed.
The current sorting method had a bug where the drafts would not move to
the end of the array. This change fixes that bug.

Change-Id: I882cf804cee10b1713798da5dc130acfa68fa773
---
 .../diff/gr-diff-host/gr-diff-host.js         | 15 ++++++----
 .../diff/gr-diff-host/gr-diff-host_test.html  | 30 +++++++++++++++++++
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index a15ca37c1f..da22f6bbcc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -640,17 +640,20 @@
       }
     }
 
+    _sortComments(comments) {
+      return comments.slice(0).sort((a, b) => {
+        if (b.__draft && !a.__draft ) { return -1; }
+        if (a.__draft && !b.__draft ) { return 1; }
+        return util.parseDate(a.updated) - util.parseDate(b.updated);
+      });
+    }
+
     /**
      * @param {!Array<!Object>} comments
      * @return {!Array<!Object>} Threads for the given comments.
      */
     _createThreads(comments) {
-      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 sortedComments = this._sortComments(comments);
       const threads = [];
       for (const comment of sortedComments) {
         // If the comment is in reply to another comment, find that comment's
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index d28f2a342f..5aee282d62 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -1102,6 +1102,36 @@ limitations under the License.
       });
     });
 
+    test('comments sorting', () => {
+      const comments = [
+        {
+          id: 'new_draft',
+          message: 'i do not like either of you',
+          __commentSide: 'left',
+          __draft: true,
+          updated: '2015-12-20 15:01:20.396000000',
+        },
+        {
+          id: 'sallys_confession',
+          message: 'i like you, jack',
+          updated: '2015-12-23 15:00:20.396000000',
+          line: 1,
+          __commentSide: 'left',
+        }, {
+          id: 'jacks_reply',
+          message: 'i like you, too',
+          updated: '2015-12-24 15:01:20.396000000',
+          __commentSide: 'left',
+          line: 1,
+          in_reply_to: 'sallys_confession',
+        },
+      ];
+      const sortedComments = element._sortComments(comments);
+      assert.equal(sortedComments[0], comments[1]);
+      assert.equal(sortedComments[1], comments[2]);
+      assert.equal(sortedComments[2], comments[0]);
+    });
+
     test('_createThreads', () => {
       const comments = [
         {