From f44c80e33aec5659925c27b412a31dfe45d67fb0 Mon Sep 17 00:00:00 2001
From: Becky Siegel <beckysiegel@google.com>
Date: Fri, 12 Jan 2018 13:48:07 -0800
Subject: [PATCH] Show and allow modification of exclusive bit in gr-permission

Bug: Issue 8034
Change-Id: Ia66c8308c242e7f5a09dc796cbd02944a3b02aa6
---
 .../admin/gr-permission/gr-permission.html    |  21 ++-
 .../admin/gr-permission/gr-permission.js      |  31 ++++
 .../gr-permission/gr-permission_test.html     |  27 +++
 .../gr-repo-access/gr-repo-access_test.html   | 168 +++++++++++-------
 .../gr-messages-list/gr-messages-list.html    |   4 -
 polygerrit-ui/app/styles/shared-styles.html   |   4 +
 6 files changed, 187 insertions(+), 68 deletions(-)

diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.html
index 98a1a6105b..4013b37dc0 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.html
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.html
@@ -16,6 +16,7 @@ limitations under the License.
 
 <link rel="import" href="../../../bower_components/polymer/polymer.html">
 <link rel="import" href="../../../behaviors/gr-access-behavior/gr-access-behavior.html">
+<link rel="import" href="../../../bower_components/paper-toggle-button/paper-toggle-button.html">
 <link rel="import" href="../../../styles/gr-form-styles.html">
 <link rel="import" href="../../../styles/gr-menu-page-styles.html">
 <link rel="import" href="../../../styles/shared-styles.html">
@@ -52,8 +53,13 @@ limitations under the License.
       #removeBtn {
         display: none;
       }
+      .right {
+        display: flex;
+        align-items: center;
+      }
       .editing #removeBtn {
         display: block;
+        margin-left: 1.5em;
       }
       .editing #addRule {
         display: block;
@@ -82,10 +88,17 @@ limitations under the License.
       <div id="mainContainer">
         <div class="header">
           <span class="title">[[name]]</span>
-          <gr-button
-              link
-              id="removeBtn"
-              on-tap="_handleRemovePermission">Remove</gr-button>
+          <div class="right">
+            <paper-toggle-button
+                id="exclusiveToggle"
+                checked="{{permission.value.exclusive}}"
+                on-change="_handleValueChange"
+                disabled$="[[!editing]]"></paper-toggle-button>Exclusive
+            <gr-button
+                link
+                id="removeBtn"
+                on-tap="_handleRemovePermission">Remove</gr-button>
+          </div>
         </div><!-- end header -->
         <div class="rules">
           <template
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
index 8a0403da0d..f9c04e600e 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
@@ -58,6 +58,7 @@
         type: Boolean,
         value: false,
       },
+      _originalExclusiveValue: Boolean,
     },
 
     behaviors: [
@@ -68,6 +69,26 @@
       '_handleRulesChanged(_rules.splices)',
     ],
 
+    listeners: {
+      'access-saved': '_handleAccessSaved',
+    },
+
+    ready() {
+      this._setupValues();
+    },
+
+    _setupValues() {
+      if (!this.permission) { return; }
+      this._originalExclusiveValue = !!this.permission.value.exclusive;
+      Polymer.dom.flush();
+    },
+
+    _handleAccessSaved() {
+      // Set a new 'original' value to keep track of after the value has been
+      // saved.
+      this._setupValues();
+    },
+
     _handleEditingChanged(editing, editingOld) {
       // Ignore when editing gets set initially.
       if (!editingOld) { return; }
@@ -76,9 +97,19 @@
         this._deleted = false;
         this._groupFilter = '';
         this._rules = this._rules.filter(rule => !rule.value.added);
+
+        // Restore exclusive bit to original.
+        this.set(['permission', 'value', 'exclusive'],
+            this._originalExclusiveValue);
       }
     },
 
+    _handleValueChange() {
+      this.permission.value.modified = true;
+      // Allows overall access page to know a change has been made.
+      this.dispatchEvent(new CustomEvent('access-modified', {bubbles: true}));
+    },
+
     _handleRemovePermission() {
       this._deleted = true;
       this.permission.value.deleted = true;
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
index 88a7472c2f..b67d705117 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
@@ -288,6 +288,7 @@ limitations under the License.
             },
           },
         };
+        element._setupValues();
         flushAsynchronousOperations();
       });
 
@@ -326,6 +327,32 @@ limitations under the License.
         assert.isFalse(element.$.permission.classList.contains('deleted'));
         assert.isFalse(element._deleted);
       });
+
+      test('modify a permission', () => {
+        element.editing = true;
+        element.name = 'Priority';
+        element.section = 'refs/*';
+
+        assert.isFalse(element._originalExclusiveValue);
+        assert.isNotOk(element.permission.value.modified);
+        MockInteractions.tap(element.$.exclusiveToggle);
+        flushAsynchronousOperations();
+        assert.isTrue(element.permission.value.exclusive);
+        assert.isTrue(element.permission.value.modified);
+        assert.isFalse(element._originalExclusiveValue);
+        element.editing = false;
+        assert.isFalse(element.permission.value.exclusive);
+      });
+
+      test('_handleValueChange', () => {
+        const modifiedHandler = sandbox.stub();
+        element.permission = {value: {rules: {}}};
+        element.addEventListener('access-modified', modifiedHandler);
+        assert.isNotOk(element.permission.value.modified);
+        element._handleValueChange();
+        assert.isTrue(element.permission.value.modified);
+        assert.isTrue(modifiedHandler.called);
+      });
     });
   });
 </script>
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
index 256c59f5a9..b26121c1ec 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
@@ -76,30 +76,6 @@ limitations under the License.
         'Code-Review': {},
       },
     };
-    const repoAccessInput = {
-      add: {
-        'refs/*': {
-          permissions: {
-            owner: {
-              rules: {
-                123: {action: 'DENY', modified: true},
-              },
-            },
-          },
-        },
-      },
-      remove: {
-        'refs/*': {
-          permissions: {
-            owner: {
-              rules: {
-                123: null,
-              },
-            },
-          },
-        },
-      },
-    };
     setup(() => {
       sandbox = sinon.sandbox.create();
       element = fixture('basic');
@@ -126,14 +102,13 @@ limitations under the License.
           name: 'Create Account',
         },
       };
-
       const accessStub = sandbox.stub(element.$.restAPI,
           'getRepoAccessRights');
 
-
-      accessStub.withArgs('New Repo').returns(Promise.resolve(accessRes));
+      accessStub.withArgs('New Repo').returns(
+          Promise.resolve(JSON.parse(JSON.stringify(accessRes))));
       accessStub.withArgs('Another New Repo')
-          .returns(Promise.resolve(accessRes2));
+          .returns(Promise.resolve(JSON.parse(JSON.stringify(accessRes2))));
       const capabilitiesStub = sandbox.stub(element.$.restAPI,
           'getCapabilities');
       capabilitiesStub.returns(Promise.resolve(capabilitiesRes));
@@ -168,26 +143,13 @@ limitations under the License.
           name: 'Access Database',
         },
       };
-      const accessRes = {
-        local: {
-          GLOBAL_CAPABILITIES: {
-            permissions: {
-              accessDatabase: {
-                rules: {
-                  123: {},
-                },
-              },
-            },
-          },
-        },
-      };
       const repoRes = {
         labels: {
           'Code-Review': {},
         },
       };
-      const accessStub = sandbox.stub(element.$.restAPI,
-          'getRepoAccessRights').returns(Promise.resolve(accessRes));
+      const accessStub = sandbox.stub(element.$.restAPI, 'getRepoAccessRights')
+          .returns(Promise.resolve(JSON.parse(JSON.stringify(accessRes2))));
       const capabilitiesStub = sandbox.stub(element.$.restAPI,
           'getCapabilities').returns(Promise.resolve(capabilitiesRes));
       const repoStub = sandbox.stub(element.$.restAPI, 'getRepo').returns(
@@ -228,7 +190,8 @@ limitations under the License.
 
     suite('with defined sections', () => {
       setup(() => {
-        element._sections = element.toSortedArray(accessRes.local);
+        element._sections =
+            element.toSortedArray(JSON.parse(JSON.stringify(accessRes.local)));
         flushAsynchronousOperations();
       });
 
@@ -268,7 +231,7 @@ limitations under the License.
         element._local = JSON.parse(JSON.stringify(accessRes.local));
         assert.deepEqual(element._computeAddAndRemove(), {add: {}, remove: {}});
         element._local['refs/*'].permissions.owner.rules[123].deleted = true;
-        const expectedInput = {
+        let expectedInput = {
           add: {},
           remove: {
             'refs/*': {
@@ -285,19 +248,26 @@ limitations under the License.
         assert.deepEqual(element._computeAddAndRemove(), expectedInput);
         delete element._local['refs/*'].permissions.owner.rules[123].deleted;
         element._local['refs/*'].permissions.owner.rules[123].modified = true;
-        assert.deepEqual(element._computeAddAndRemove(), repoAccessInput);
-      });
-
-      test('_computeAddAndRemove permissions', () => {
-        element._local = JSON.parse(JSON.stringify(accessRes.local));
-        assert.deepEqual(element._computeAddAndRemove(), {add: {}, remove: {}});
-        element._local['refs/*'].permissions.owner.deleted = true;
-        const expectedInput = {
-          add: {},
+        expectedInput = {
+          add: {
+            'refs/*': {
+              permissions: {
+                owner: {
+                  rules: {
+                    123: {action: 'DENY', modified: true},
+                  },
+                },
+              },
+            },
+          },
           remove: {
             'refs/*': {
               permissions: {
-                owner: {rules: {}},
+                owner: {
+                  rules: {
+                    123: null,
+                  },
+                },
               },
             },
           },
@@ -309,7 +279,7 @@ limitations under the License.
         element._local = JSON.parse(JSON.stringify(accessRes.local));
         assert.deepEqual(element._computeAddAndRemove(), {add: {}, remove: {}});
         element._local['refs/*'].permissions.owner.deleted = true;
-        const expectedInput = {
+        let expectedInput = {
           add: {},
           remove: {
             'refs/*': {
@@ -320,6 +290,31 @@ limitations under the License.
           },
         };
         assert.deepEqual(element._computeAddAndRemove(), expectedInput);
+        delete element._local['refs/*'].permissions.owner.deleted;
+        element._local['refs/*'].permissions.owner.modified = true;
+        expectedInput = {
+          add: {
+            'refs/*': {
+              permissions: {
+                owner: {
+                  modified: true,
+                  rules: {
+                    234: {action: 'ALLOW'},
+                    123: {action: 'DENY'},
+                  },
+                },
+              },
+            },
+          },
+          remove: {
+            'refs/*': {
+              permissions: {
+                owner: {rules: {}},
+              },
+            },
+          },
+        };
+        assert.deepEqual(element._computeAddAndRemove(), expectedInput);
       });
 
       test('_computeAddAndRemove combinations', () => {
@@ -368,11 +363,65 @@ limitations under the License.
           },
         };
         assert.deepEqual(element._computeAddAndRemove(), expectedInput);
+        // Modify both permissions with an exclusive bit. Owner is still
+        // deleted.
+        element._local['refs/*'].permissions.owner.exclusive = true;
+        element._local['refs/*'].permissions.owner.modified = true;
+        element._local['refs/*'].permissions.read.exclusive = true;
+        element._local['refs/*'].permissions.read.modified = true;
+        expectedInput = {
+          add: {
+            'refs/*': {
+              permissions: {
+                read: {
+                  exclusive: true,
+                  modified: true,
+                  rules: {
+                    234: {action: 'ALLOW'},
+                  },
+                },
+              },
+            },
+          },
+          remove: {
+            'refs/*': {
+              permissions: {
+                owner: {rules: {}},
+                read: {rules: {}},
+              },
+            },
+          },
+        };
+        assert.deepEqual(element._computeAddAndRemove(), expectedInput);
       });
 
       test('_handleSaveForReview', done => {
-        sandbox.stub(element.$.restAPI, 'getRepoAccessRights')
-            .returns(Promise.resolve(accessRes));
+        const repoAccessInput = {
+          add: {
+            'refs/*': {
+              permissions: {
+                owner: {
+                  rules: {
+                    123: {action: 'DENY', modified: true},
+                  },
+                },
+              },
+            },
+          },
+          remove: {
+            'refs/*': {
+              permissions: {
+                owner: {
+                  rules: {
+                    123: null,
+                  },
+                },
+              },
+            },
+          },
+        };
+        sandbox.stub(element.$.restAPI, 'getRepoAccessRights').returns(
+            Promise.resolve(JSON.parse(JSON.stringify(accessRes))));
         sandbox.stub(element.$.restAPI, 'getRepo')
             .returns(Promise.resolve({}));
         sandbox.stub(Gerrit.Nav, 'navigateToChange');
@@ -381,8 +430,7 @@ limitations under the License.
             .returns(Promise.resolve({_number: 1}));
 
         element.repo = 'test-repo';
-        sandbox.stub(element, '_computeAddAndRemove')
-            .returns(repoAccessInput);
+        sandbox.stub(element, '_computeAddAndRemove').returns(repoAccessInput);
 
         element._handleSaveForReview().then(() => {
           assert.isTrue(saveForReviewStub.called);
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
index f8d4ba8fa7..df9ce54abf 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
@@ -62,10 +62,6 @@ limitations under the License.
         align-items: center;
         display: flex;
       }
-      paper-toggle-button {
-        --paper-toggle-button-checked-bar-color: var(--color-link);
-        --paper-toggle-button-checked-button-color: var(--color-link);
-      }
     </style>
     <div class="header">
       <h3>Messages</h3>
diff --git a/polygerrit-ui/app/styles/shared-styles.html b/polygerrit-ui/app/styles/shared-styles.html
index c6e179f3a8..a3cf24738a 100644
--- a/polygerrit-ui/app/styles/shared-styles.html
+++ b/polygerrit-ui/app/styles/shared-styles.html
@@ -96,6 +96,10 @@ limitations under the License.
       .separator.transparent {
         background-color: transparent;
       }
+      paper-toggle-button {
+        --paper-toggle-button-checked-bar-color: var(--color-link);
+        --paper-toggle-button-checked-button-color: var(--color-link);
+      }
     </style>
   </template>
 </dom-module>