From 75f3312fcae1bb7df7b80c82318baf608ed75b44 Mon Sep 17 00:00:00 2001
From: Ben Rohlfs <brohlfs@google.com>
Date: Tue, 4 Feb 2020 11:29:34 +0100
Subject: [PATCH] Fix rendering of removed votes in Change Log

Change 251800 was too aggressive with removing information on
"Patch set $N: ...: lines. For normal votes the removal was fine,
because it was redundant information. But the dedicated vote chips
did not work for vote removals. So with this change we keep being
aggressive and remove the entire "Patch Set $N: ..." line, but we
create vote chips also for vote removals.

Change-Id: Ia721552f08528c76bde469ed85b76d12783b761b
---
 .../change/gr-message/gr-message.html         |  5 +++-
 .../elements/change/gr-message/gr-message.js  | 28 ++++++++++++++-----
 .../change/gr-message/gr-message_test.html    | 23 +++++++++++++--
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index a972565827..2ea1134b4a 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -156,6 +156,9 @@ limitations under the License.
       .commentsIcon {
         vertical-align: top;
       }
+      .score.removed {
+        background-color: var(--vote-color-neutral);
+      }
       .score.negative {
         background-color: var(--vote-color-disliked);
       }
@@ -195,7 +198,7 @@ limitations under the License.
             on behalf of
           </span>
           <gr-account-label account="[[author]]" class="authorLabel"></gr-account-label>
-          <template is="dom-repeat" items="[[_getScores(message)]]" as="score">
+          <template is="dom-repeat" items="[[_getScores(message, labelExtremes)]]" as="score">
             <span class$="score [[_computeScoreClass(score, labelExtremes)]]">
               [[score.label]] [[score.value]]
             </span>
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 9709f1ceef..d5505a488c 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -18,7 +18,7 @@
   'use strict';
 
   const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+:\s*(.*)/;
-  const LABEL_TITLE_SCORE_PATTERN = /^([A-Za-z0-9-]+)([+-]\d+)$/;
+  const LABEL_TITLE_SCORE_PATTERN = /^(-?)([A-Za-z0-9-]+?)([+-]\d+)?$/;
 
   /**
    * @appliesMixin Gerrit.FireMixin
@@ -274,17 +274,28 @@
       return event.type === 'REVIEWER_UPDATE';
     }
 
-    _getScores(message) {
-      if (!message.message) { return []; }
+    _getScores(message, labelExtremes) {
+      if (!message || !message.message || !labelExtremes) {
+        return [];
+      }
       const line = message.message.split('\n', 1)[0];
       const patchSetPrefix = PATCH_SET_PREFIX_PATTERN;
-      if (!line.match(patchSetPrefix)) { return []; }
+      if (!line.match(patchSetPrefix)) {
+        return [];
+      }
       const scoresRaw = line.split(patchSetPrefix)[1];
-      if (!scoresRaw) { return []; }
+      if (!scoresRaw) {
+        return [];
+      }
       return scoresRaw.split(' ')
           .map(s => s.match(LABEL_TITLE_SCORE_PATTERN))
-          .filter(ms => ms && ms.length === 3)
-          .map(ms => { return {label: ms[1], value: ms[2]}; });
+          .filter(ms =>
+            ms && ms.length === 4 && labelExtremes.hasOwnProperty(ms[2]))
+          .map(ms => {
+            const label = ms[2];
+            const value = ms[1] === '-' ? 'removed' : ms[3];
+            return {label, value};
+          });
     }
 
     _computeScoreClass(score, labelExtremes) {
@@ -292,6 +303,9 @@
       if ([score, labelExtremes].some(arg => arg === undefined)) {
         return '';
       }
+      if (score.value === 'removed') {
+        return 'removed';
+      }
       const classes = [];
       if (score.value > 0) {
         classes.push('positive');
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
index c4ae24920c..20d87d2edb 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
@@ -244,12 +244,12 @@ limitations under the License.
       element.message = {
         author: {},
         expanded: false,
-        message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Ready+1',
+        message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Label3+1 Blub+1',
       };
       element.labelExtremes = {
         'Verified': {max: 1, min: -1},
         'Code-Review': {max: 2, min: -2},
-        'Trybot-Ready': {max: 3, min: 0},
+        'Trybot-Label3': {max: 3, min: 0},
       };
       flushAsynchronousOperations();
       const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
@@ -265,6 +265,25 @@ limitations under the License.
       assert.isFalse(scoreChips[2].classList.contains('min'));
     });
 
+    test('removed votes', () => {
+      element.message = {
+        author: {},
+        expanded: false,
+        message: 'Patch Set 1: Verified+1 -Code-Review -Commit-Queue',
+      };
+      element.labelExtremes = {
+        'Verified': {max: 1, min: -1},
+        'Code-Review': {max: 2, min: -2},
+        'Commit-Queue': {max: 3, min: 0},
+      };
+      flushAsynchronousOperations();
+      const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
+      assert.equal(scoreChips.length, 3);
+
+      assert.isTrue(scoreChips[1].classList.contains('removed'));
+      assert.isTrue(scoreChips[2].classList.contains('removed'));
+    });
+
     test('false negative vote', () => {
       element.message = {
         author: {},