Add ability to remove and add back permission in repo access

Bug: Issue 8033
Change-Id: Ib6290c8b50a7c085d0c37a0b268d2a043ecc2ade

polygerrit-ui/app/elements/admin/gr-permission/gr-permission.html

@@ -37,10 +37,6 @@ limitations under the License.
         justify-content: space-between;
         margin: .3em .7em;
       }
-      #deletedContainer {
-        border: 1px solid #d1d2d3;
-        padding: .7em;
-      }
       .rules {
         background: #fafafa;
         border: 1px solid #d1d2d3;
@@ -57,11 +53,11 @@ limitations under the License.
       #removeBtn {
         display: none;
       }
-      /* TODO @beckysiegel add back */
+      .editing #removeBtn {
-      /* .editing #removeBtn {
         display: block;
       }
-      .editing #addRule {
+      /* TODO @beckysiegel add back */
+      /* .editing #addRule {
         display: block;
         padding: .7em;
       } */
@@ -69,7 +65,13 @@ limitations under the License.
       .deleted #mainContainer {
         display: none;
       }
-      .deleted #deletedContainer,
+      .deleted #deletedContainer {
+        align-items: baseline;
+        border: 1px solid #d1d2d3;
+        display: flex;
+        justify-content: space-between;
+        padding: .7em;
+      }
       #mainContainer {
         display: block;
       }
@@ -83,6 +85,7 @@ limitations under the License.
         <div class="header">
           <span class="title">[[name]]</span>
           <gr-button
+              link
               id="removeBtn"
               on-tap="_handleRemovePermission">Remove</gr-button>
         </div><!-- end header -->
@@ -112,8 +115,9 @@ limitations under the License.
         </div> <!-- end rules -->
       </div><!-- end mainContainer -->
       <div id="deletedContainer">
-        [[name]] was deleted
+        <span>[[name]] was deleted</span>
         <gr-button
+            link
             id="undoRemoveBtn"
             on-tap="_handleUndoRemove">Undo</gr-button>
       </div><!-- end deletedContainer -->

polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js

@@ -16,6 +16,12 @@
 
   const MAX_AUTOCOMPLETE_RESULTS = 20;
 
+  /**
+   * Fired when the permission has been modified or removed.
+   *
+   * @event access-modified
+   */
+
   Polymer({
     is: 'gr-permission',
 
@@ -33,6 +39,7 @@
       editing: {
         type: Boolean,
         value: false,
+        observer: '_handleEditingChanged',
       },
       _label: {
         type: Object,
@@ -61,6 +68,21 @@
       '_handleRulesChanged(_rules.splices)',
     ],
 
+    _handleEditingChanged(editing, editingOld) {
+      // Ignore when editing gets set initially.
+      if (!editingOld) { return; }
+      // Restore original values if no longer editing.
+      if (!editing) {
+        this._deleted = false;
+      }
+    },
+
+    _handleRemovePermission() {
+      this._deleted = true;
+      this.permission.value.deleted = true;
+      this.dispatchEvent(new CustomEvent('access-modified', {bubbles: true}));
+    },
+
     _handleRulesChanged(changeRecord) {
       // Update the groups to exclude in the autocomplete.
       this._groupsWithRules = this._computeGroupsWithRules(this._rules);
@@ -70,11 +92,6 @@
       this._rules = this.toSortedArray(permission.value.rules);
     },
 
-    _handleRemovePermission() {
-      this._deleted = true;
-      this.set('permission.value.deleted', true);
-    },
-
     _computeSectionClass(editing, deleted) {
       const classList = [];
       if (editing) {

polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.html

@@ -50,8 +50,9 @@ limitations under the License.
     </style>
     <style include="gr-menu-page-styles"></style>
     <main class$="[[_computeAdminClass(_isAdmin)]]">
-      <div class="gwtLink">This is currently in read only mode.  To modify content, go to the
+      <div class="gwtLink">Editing access in the new UI is a work in progress. Visit the
         <a href$="[[computeGwtUrl(path)]]" rel="external">Old UI</a>
+        if you need a feature that is not yet supported.
       </div>
       <template is="dom-if" if="[[_inheritsFrom]]">
         <h3 id="inheritsFrom">Rights Inherit From

polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js

@@ -156,7 +156,14 @@
       for (const item of path) {
         if (!curPos[item]) {
           if (item === path[path.length - 1] && type === 'remove') {
+            // TODO(beckysiegel) This if statement should be removed when
+            // https://gerrit-review.googlesource.com/c/gerrit/+/150851
+            // is live.
+            if (path[path.length - 2] === 'permissions') {
+              curPos[item] = {rules: {}};
+            } else {
               curPos[item] = null;
+            }
           } else if (item === path[path.length - 1] && type === 'add') {
             curPos[item] = opt_value;
           } else {

polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html

@@ -100,22 +100,6 @@ limitations under the License.
         },
       },
     };
-
-    const repoAccessInputRemoved = {
-      add: {},
-      remove: {
-        'refs/*': {
-          permissions: {
-            owner: {
-              rules: {
-                123: null,
-              },
-            },
-          },
-        },
-      },
-    };
-
     setup(() => {
       sandbox = sinon.sandbox.create();
       element = fixture('basic');
@@ -280,17 +264,112 @@ limitations under the License.
         assert.isTrue(element._handleAccessModified.called);
       });
 
-      test('_computeAddAndRemove', () => {
+      test('_computeAddAndRemove rules', () => {
-        // With nothing modified
+        element._local = JSON.parse(JSON.stringify(accessRes.local));
-        element._local = accessRes.local;
         assert.deepEqual(element._computeAddAndRemove(), {add: {}, remove: {}});
         element._local['refs/*'].permissions.owner.rules[123].deleted = true;
-        assert.deepEqual(element._computeAddAndRemove(), repoAccessInputRemoved);
+        const expectedInput = {
+          add: {},
+          remove: {
+            'refs/*': {
+              permissions: {
+                owner: {
+                  rules: {
+                    123: null,
+                  },
+                },
+              },
+            },
+          },
+        };
+        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: {},
+          remove: {
+            'refs/*': {
+              permissions: {
+                owner: {rules: {}},
+              },
+            },
+          },
+        };
+        assert.deepEqual(element._computeAddAndRemove(), expectedInput);
+      });
+
+      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: {},
+          remove: {
+            'refs/*': {
+              permissions: {
+                owner: {rules: {}},
+              },
+            },
+          },
+        };
+        assert.deepEqual(element._computeAddAndRemove(), expectedInput);
+      });
+
+      test('_computeAddAndRemove combinations', () => {
+        // Modify rule and delete permission that it is inside of.
+        element._local = JSON.parse(JSON.stringify(accessRes.local));
+        element._local['refs/*'].permissions.owner.rules[123].modified = true;
+        element._local['refs/*'].permissions.owner.deleted = true;
+        let expectedInput = {
+          add: {},
+          remove: {
+            'refs/*': {
+              permissions: {
+                owner: {rules: {}},
+              },
+            },
+          },
+        };
+        assert.deepEqual(element._computeAddAndRemove(), expectedInput);
+        // Delete rule and delete permission that it is inside of.
+        element._local['refs/*'].permissions.owner.rules[123].modified = false;
+        element._local['refs/*'].permissions.owner.rules[123].deleted = true;
+        assert.deepEqual(element._computeAddAndRemove(), expectedInput);
+
+        // Also modify a different rule inside of another permission.
+        element._local['refs/*'].permissions.read.modified = true;
+        expectedInput = {
+          add: {
+            'refs/*': {
+              permissions: {
+                read: {
+                  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));