From 9c8cfe5619fafc459184b593f0eafea66d330f45 Mon Sep 17 00:00:00 2001
From: Kasper Nilsson <kaspern@google.com>
Date: Wed, 2 Aug 2017 14:10:12 -0700
Subject: [PATCH 1/2] Add _editLoaded property to gr-change-view

Computed flag for use in enabling edit-specific actions, like
publishing, discarding, and modifying edits.

First use for this feature is computing whether edit specific buttons
like "Publish" and "Discard" ought to be shown/hidden.

Bug: Issue 4437
Change-Id: I5f9e92d49255f56b49113d4f33d5a61a743db4ea
---
 .../app/elements/change/gr-change-view/gr-change-view.js | 9 +++++++++
 .../change/gr-change-view/gr-change-view_test.html       | 8 ++++++++
 2 files changed, 17 insertions(+)

diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 8866ce5294..1d8025e409 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -182,6 +182,10 @@
       },
       _updateCheckTimerHandle: Number,
       _sortedRevisions: Array,
+      _editLoaded: {
+        type: Boolean,
+        computed: '_computeEditLoaded(_patchRange.*)',
+      },
     },
 
     behaviors: [
@@ -1319,5 +1323,10 @@
     _computeHeaderClass(change) {
       return change.work_in_progress ? 'header wip' : 'header';
     },
+
+    _computeEditLoaded(patchRangeRecord) {
+      const patchRange = patchRangeRecord.base || {};
+      return this.patchNumEquals(patchRange.patchNum, this.EDIT_NAME);
+    },
   });
 })();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 25f1192c5f..ddbbd5f000 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -1288,6 +1288,14 @@ limitations under the License.
       assert.isTrue(element.$.relatedChanges.reload.calledOnce);
     });
 
+    test('_computeEditLoaded', () => {
+      const callCompute = range => element._computeEditLoaded({base: range});
+      assert.isFalse(callCompute({}));
+      assert.isFalse(callCompute({basePatchNum: 'PARENT', patchNum: 1}));
+      assert.isFalse(callCompute({basePatchNum: 'edit', patchNum: 1}));
+      assert.isTrue(callCompute({basePatchNum: 1, patchNum: 'edit'}));
+    });
+
     suite('_upgradeUrl calls', () => {
       let upgradeStub;
       const mockChange = {project: 'test'};

From 5b57a3685727d74d55c18c41fdc10ab26e2d32ba Mon Sep 17 00:00:00 2001
From: Kasper Nilsson <kaspern@google.com>
Date: Mon, 7 Aug 2017 10:58:19 -0700
Subject: [PATCH 2/2] Hide reviewed checkbox when edit is loaded

Modifying the reviewed state of an edit patchset is an illogical action,
and should be disabled.

This change utilizes the _editLoaded property of gr-change-view to
conditionally hide the reviewed checkbox from the file list. In
addition, attempts to modify the reviewed state (e.g. with a hotkey)
are treated as a no-op, and the user is shown a toast.

TODO: Further utilize this approach to conditionally show buttons for
modifying an edit patchset. Will be done in a later change.

Bug: Issue 4437
Change-Id: Iee33d7ebbe0a7182cb91ca6168813154775d9ab4
---
 .../change/gr-change-view/gr-change-view.html |  1 +
 .../change/gr-file-list/gr-file-list.html     | 10 ++++++--
 .../change/gr-file-list/gr-file-list.js       | 11 ++++++++
 .../gr-file-list/gr-file-list_test.html       | 25 ++++++++++++++++++-
 4 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 4e0e07c26c..009e673da9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -516,6 +516,7 @@ limitations under the License.
             project-config="[[_projectConfig]]"
             selected-index="{{viewState.selectedFileIndex}}"
             diff-view-mode="{{viewState.diffMode}}"
+            edit-loaded="[[_editLoaded]]"
             num-files-shown="{{_numFilesShown}}"
             file-list-increment="{{_numFilesShown}}"></gr-file-list>
       </section>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 8c5855975b..e633327967 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -168,6 +168,9 @@ limitations under the License.
       .mobile {
         display: none;
       }
+      #container.editLoaded .hideOnEdit {
+        display: none;
+      }
       @media screen and (max-width: 50em) {
         .desktop {
           display: none;
@@ -245,7 +248,10 @@ limitations under the License.
         </label>
       </div>
     </header>
-    <div on-tap="_handleFileListTap">
+    <div
+        id="container"
+        class$="[[_computeContainerClass(editLoaded)]]"
+        on-tap="_handleFileListTap">
       <template is="dom-repeat"
           items="[[_shownFiles]]"
           id="files"
@@ -253,7 +259,7 @@ limitations under the License.
           initial-count="[[fileListIncrement]]"
           target-framerate="1">
         <div class="file-row row" data-path$="[[file.__path]]" tabindex="-1">
-          <div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
+          <div class="reviewed hideOnEdit" hidden$="[[!_loggedIn]]" hidden>
             <input
                 type="checkbox"
                 checked="[[file.isReviewed]]"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index a7025f5e8d..d24eb564a3 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -14,6 +14,8 @@
 (function() {
   'use strict';
 
+  const ERR_EDIT_LOADED = 'You cannot change the review status of an edit.';
+
   // Maximum length for patch set descriptions.
   const PATCH_DESC_MAX_LENGTH = 500;
   const WARN_SHOW_ALL_THRESHOLD = 1000;
@@ -58,6 +60,7 @@
         notify: true,
         observer: '_updateDiffPreferences',
       },
+      editLoaded: Boolean,
       _files: {
         type: Array,
         observer: '_filesChanged',
@@ -387,6 +390,10 @@
     },
 
     _reviewFile(path) {
+      if (this.editLoaded) {
+        this.fire('show-alert', {message: ERR_EDIT_LOADED});
+        return;
+      }
       const index = this._reviewed.indexOf(path);
       const reviewed = index !== -1;
       if (reviewed) {
@@ -943,5 +950,9 @@
         this.classList.toggle('loading', loading && this._files.length);
       }, LOADING_DEBOUNCE_INTERVAL);
     },
+
+    _computeContainerClass(editLoaded) {
+      return editLoaded ? 'editLoaded' : '';
+    },
   });
 })();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index c9ad238a9e..2017460e3a 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -1267,7 +1267,30 @@ limitations under the License.
       element._handleEscKey(mockEvent);
       assert.isFalse(element._displayLine);
     });
-  });
 
+    suite('editLoaded behavior', () => {
+      const isVisible = el => {
+        assert.ok(el);
+        return getComputedStyle(el).getPropertyValue('display') !== 'none';
+      };
+
+      test('reviewed checkbox', () => {
+        const alertStub = sandbox.stub();
+        element.addEventListener('show-alert', alertStub);
+        element.editLoaded = false;
+        // Reviewed checkbox should be shown.
+        assert.isTrue(isVisible(element.$$('.reviewed')));
+        MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
+        assert.isFalse(alertStub.called);
+
+        element.editLoaded = true;
+        flushAsynchronousOperations();
+
+        assert.isFalse(isVisible(element.$$('.reviewed')));
+        MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
+        assert.isTrue(alertStub.called);
+      });
+    });
+  });
   a11ySuite('basic');
 </script>