Merge "Associate messages and comment by id"

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

@@ -279,9 +279,9 @@ class GrMessagesList extends mixinBehaviors( [
   }
 
   /**
-   * Computes message author's file comments for change's message.
+   * Computes message author's file comments for change's message. The backend
-   * Method uses this.messages to find next message and relies on messages
+   * sets comment.change_message_id for matching, so this computation is fairly
-   * to be sorted by date field descending.
+   * straightforward.
    *
    * @param {!Object} changeComments changeComment object, which includes
    *     a method to get all published comments (including robot comments),
@@ -297,39 +297,12 @@ class GrMessagesList extends mixinBehaviors( [
     if (message._index === undefined || !comments || !this.messages) {
       return [];
     }
-    const messages = this.messages || [];
+
-    const index = message._index;
+    const idFilter = comment => comment.change_message_id === message.id;
-    const authorId = message.author && message.author._account_id;
-    const mDate = util.parseDate(message.date).getTime();
-    // NB: Messages array has oldest messages first.
-    let nextMDate;
-    if (index > 0) {
-      for (let i = index - 1; i >= 0; i--) {
-        if (messages[i] && messages[i].author &&
-            messages[i].author._account_id === authorId) {
-          nextMDate = util.parseDate(messages[i].date).getTime();
-          break;
-        }
-      }
-    }
     const msgComments = {};
     for (const file in comments) {
       if (!comments.hasOwnProperty(file)) { continue; }
-      const fileComments = comments[file];
+      msgComments[file] = comments[file].filter(idFilter);
-      for (let i = 0; i < fileComments.length; i++) {
-        if (fileComments[i].author &&
-            fileComments[i].author._account_id !== authorId) {
-          continue;
-        }
-        const cDate = util.parseDate(fileComments[i].updated).getTime();
-        if (cDate <= mDate) {
-          if (nextMDate && cDate <= nextMDate) {
-            continue;
-          }
-          msgComments[file] = msgComments[file] || [];
-          msgComments[file].push(fileComments[i]);
-        }
-      }
     }
     return msgComments;
   }

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

@@ -78,6 +78,10 @@ suite('gr-messages-list tests', () => {
     return dom(element.root).querySelectorAll('gr-message');
   };
 
+  const MESSAGE_ID_0 = '1234ccc949c6d482b061be6a28e10782abf0e7af';
+  const MESSAGE_ID_1 = '8c19ccc949c6d482b061be6a28e10782abf0e7af';
+  const MESSAGE_ID_2 = 'e7bfdbc842f6b6d8064bc68e0f52b673f40c0ca5';
+
   const author = {
     _account_id: 42,
     name: 'Marvin the Paranoid Android',
@@ -88,6 +92,7 @@ suite('gr-messages-list tests', () => {
     file1: [
       {
         message: 'message text',
+        change_message_id: MESSAGE_ID_0,
         updated: '2016-09-27 00:18:03.000000000',
         in_reply_to: '6505d749_f0bec0aa',
         line: 62,
@@ -100,6 +105,7 @@ suite('gr-messages-list tests', () => {
       },
       {
         message: 'message text',
+        change_message_id: MESSAGE_ID_1,
         updated: '2016-09-27 00:18:03.000000000',
         in_reply_to: 'c5912363_6b820105',
         line: 42,
@@ -109,6 +115,7 @@ suite('gr-messages-list tests', () => {
       },
       {
         message: 'message text',
+        change_message_id: MESSAGE_ID_1,
         updated: '2016-09-27 00:18:03.000000000',
         in_reply_to: '6505d749_f0bec0aa',
         line: 62,
@@ -116,10 +123,20 @@ suite('gr-messages-list tests', () => {
         patch_set: 2,
         author,
       },
+      {
+        message: 'message text',
+        change_message_id: MESSAGE_ID_2,
+        updated: '2016-09-27 00:18:03.000000000',
+        line: 64,
+        id: '34ed05d749_10ed44b2',
+        patch_set: 2,
+        author,
+      },
     ],
     file2: [
       {
         message: 'message text',
+        change_message_id: MESSAGE_ID_1,
         updated: '2016-09-27 00:18:03.000000000',
         in_reply_to: 'c5912363_4b7d450a',
         line: 132,
@@ -389,8 +406,8 @@ suite('gr-messages-list tests', () => {
           }
       );
       element.messages = messages;
-      const isAuthor = function(author, message) {
+      const isAuthor = function(author, comment) {
-        return message.author._account_id === author._account_id;
+        return comment.author._account_id === author._account_id;
       };
       const isMarvin = isAuthor.bind(null, author);
       flushAsynchronousOperations();
@@ -399,10 +416,14 @@ suite('gr-messages-list tests', () => {
       assert.deepEqual(messageElements[1].message, messages[1]);
       assert.deepEqual(messageElements[2].message, messages[2]);
       assert.deepEqual(messageElements[1].comments.file1,
-          comments.file1.filter(isMarvin));
+          comments.file1.filter(isMarvin).filter(
+              c => c.change_message_id === messages[1].id));
       assert.deepEqual(messageElements[1].comments.file2,
-          comments.file2.filter(isMarvin));
+          comments.file2.filter(isMarvin).filter(
-      assert.deepEqual(messageElements[2].comments, {});
+              c => c.change_message_id === messages[1].id));
+      assert.deepEqual(messageElements[2].comments.file1,
+          comments.file1.filter(isMarvin).filter(
+              c => c.change_message_id === messages[2].id));
     });
 
     test('messages without author do not throw', () => {