From ca3c497c8ceff2a1734975c23fa6c9fa9b0aae0e Mon Sep 17 00:00:00 2001
From: Logan Hanks <logan@google.com>
Date: Wed, 14 Sep 2016 16:25:56 -0700
Subject: [PATCH] Only send changed labels with review

If the user hasn't changed their vote on a label, don't include it in
the review request.

Feature: Issue 4536
Change-Id: Icba0960dcceffd77b14582200d1619a1717f8fe2
---
 .../change/gr-reply-dialog/gr-reply-dialog.js | 28 ++++++++++++-------
 .../gr-reply-dialog/gr-reply-dialog_test.html | 18 ++++++++++++
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index b54fc85f3b..00fa05eecf 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -157,7 +157,16 @@
 
         var selectedVal = selectorEl.selectedItem.getAttribute('data-value');
         selectedVal = parseInt(selectedVal, 10);
-        obj.labels[label] = selectedVal;
+
+        // Only send the selection if the user changed it.
+        var prevVal = this._getVoteForAccount(this.labels, label,
+            this._account);
+        if (prevVal !== null) {
+          prevVal = parseInt(prevVal, 10);
+        }
+        if (selectedVal !== prevVal) {
+          obj.labels[label] = selectedVal;
+        }
       }
       if (this.draft != null) {
         obj.message = this.draft;
@@ -280,23 +289,22 @@
       return Object.keys(labelsObj).sort();
     },
 
-    _computeIndexOfLabelValue: function(
-        labels, permittedLabels, labelName, account) {
-      var t = labels[labelName];
-      if (!t) { return null; }
-      var labelValue = null;
-
-      // Is there an existing vote for the current user? If so, use that.
+    _getVoteForAccount: function(labels, labelName, account) {
       var votes = labels[labelName];
       if (votes.all && votes.all.length > 0) {
         for (var i = 0; i < votes.all.length; i++) {
           if (votes.all[i]._account_id == account._account_id) {
-            labelValue = votes.all[i].value;
-            break;
+            return votes.all[i].value;
           }
         }
       }
+      return null;
+    },
 
+    _computeIndexOfLabelValue: function(
+        labels, permittedLabels, labelName, account) {
+      if (!labels[labelName]) { return null; }
+      var labelValue = this._getVoteForAccount(labels, labelName, account);
       var len = permittedLabels[labelName] != null ?
           permittedLabels[labelName].length : 0;
       for (var i = 0; i < len; i++) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 639aeef161..1ef372bcc4 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -380,5 +380,23 @@ limitations under the License.
       assert.strictEqual(
           element._chooseFocusTarget(), element.FocusTarget.BODY);
     });
+
+    test('only send labels that have changed', function(done) {
+      var saveReviewStub = sinon.stub(element, '_saveReview',
+          function(review) {
+        assert.deepEqual(review.labels, {Verified: -1});
+        return Promise.resolve({ok: true});
+      });
+
+      element.addEventListener('send', function() {
+        saveReviewStub.restore();
+        done();
+      });
+
+      MockInteractions.tap(element.$$(
+          'iron-selector[data-label="Verified"] > ' +
+          'gr-button[data-value="-1"]'));
+      MockInteractions.tap(element.$$('.send'));
+    });
   });
 </script>