Show and allow modification of exclusive bit in gr-permission

Bug: Issue 8034 Change-Id: Ia66c8308c242e7f5a09dc796cbd02944a3b02aa6
2018-01-12 13:48:07 -08:00
parent 8d4403eb28
commit f44c80e33a
6 changed files with 187 additions and 68 deletions
--- 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
--- 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;
--- 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>
--- 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);
--- 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>
--- 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>