Merge "PolyGerrit: Fix adding and deleting members in groups"
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
index 2d8da15..ae28a05 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
@@ -60,6 +60,10 @@
         type: Boolean,
         value: false,
       },
+      _isAdmin: {
+        type: Boolean,
+        value: false,
+      },
       _showGroup: Boolean,
       _showGroupAuditLog: Boolean,
       _showGroupList: Boolean,
@@ -155,7 +159,7 @@
               },
             ],
           };
-          if (this._groupOwner) {
+          if (this._isAdmin || this._groupOwner) {
             linkCopy.subsection.children.push(
                 {
                   name: 'Audit Log',
@@ -242,16 +246,20 @@
 
     _computeGroupName(groupId) {
       if (!groupId) { return ''; }
+      const promises = [];
       this.$.restAPI.getGroupConfig(groupId).then(group => {
         this._groupName = group.name;
         this.reload();
-        this.$.restAPI.getIsGroupOwner(group.name).then(
+        promises.push(this.$.restAPI.getIsAdmin().then(isAdmin => {
+          this._isAdmin = isAdmin;
+        }));
+        promises.push(this.$.restAPI.getIsGroupOwner(group.name).then(
             isOwner => {
-              if (isOwner) {
-                this._groupOwner = true;
-                this.reload();
-              }
-            });
+              this._groupOwner = isOwner;
+            }));
+        return Promise.all(promises).then(() => {
+          this.reload();
+        });
       });
     },
 
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
index 9371b17..9e08381 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
@@ -67,9 +67,18 @@
         font-family: var(--font-family-bold);
         text-align: left;
       }
+      .canModify #groupMemberSearchInput,
+      .canModify #saveGroupMember,
+      .canModify .deleteHeader,
+      .canModify .deleteColumn,
+      .canModify #includedGroupSearchInput,
+      .canModify #saveIncludedGroups,
+      .canModify .deleteIncludedHeader,
+      .canModify #saveIncludedGroups {
+        display: none;
+      }
     </style>
-    <style include="gr-form-styles"></style>
-    <main class="gr-form-styles">
+    <main class$="gr-form-styles [[_computeHideItemClass(_groupOwner, _isAdmin)]]">
       <div id="loading" class$="[[_computeLoadingClass(_loading)]]">
         Loading...
       </div>
@@ -84,15 +93,13 @@
                     id="groupMemberSearchInput"
                     text="{{_groupMemberSearch}}"
                     query="[[_queryMembers]]"
-                    placeholder="Name Or Email"
-                    hidden$="[[!_groupOwner]]">
+                    placeholder="Name Or Email">
                 </gr-autocomplete>
               </span>
               <gr-button
                   id="saveGroupMember"
                   on-tap="_handleSavingGroupMember"
-                  disabled="[[!_groupMemberSearch]]"
-                  hidden$="[[!_groupOwner]]">
+                  disabled="[[!_groupMemberSearch]]">
                 Add
               </gr-button>
               <div class="gr-form-styles">
@@ -100,9 +107,7 @@
                   <tr class="headerRow">
                     <th class="nameHeader">Name</th>
                     <th class="emailAddressHeader">Email Address</th>
-                    <th class="deleteHeader" hidden$="[[!_groupOwner]]">
-                      Delete Member
-                    </th>
+                    <th class="deleteHeader">Delete Member</th>
                   </tr>
                   <tbody>
                     <template is="dom-repeat" items="[[_groupMembers]]">
@@ -111,7 +116,7 @@
                           <gr-account-link account="[[item]]"></gr-account-link>
                         </td>
                         <td>[[item.email]]</td>
-                        <td hidden$="[[!_groupOwner]]">
+                        <td class="deleteColumn">
                           <gr-button
                               class="deleteButton"
                               on-tap="_handleDeleteMember">
@@ -133,15 +138,13 @@
                     id="includedGroupSearchInput"
                     text="{{_includedGroupSearch}}"
                     query="[[_queryIncludedGroup]]"
-                    placeholder="Group Name"
-                    hidden$="[[!_groupOwner]]">
+                    placeholder="Group Name">
                 </gr-autocomplete>
               </span>
               <gr-button
                   id="saveIncludedGroups"
                   on-tap="_handleSavingIncludedGroups"
-                  disabled="[[!_includedGroupSearch]]"
-                  hidden$="[[!_groupOwner]]">
+                  disabled="[[!_includedGroupSearch]]">
                 Add
               </gr-button>
               <div class="gr-form-styles">
@@ -149,7 +152,7 @@
                   <tr class="headerRow">
                     <th class="groupNameHeader">Group Name</th>
                     <th class="descriptionHeader">Description</th>
-                    <th class="deleteIncludedHeader" hidden$="[[!_groupOwner]]">
+                    <th class="deleteIncludedHeader">
                       Delete Included Group
                     </th>
                   </tr>
@@ -162,7 +165,7 @@
                           </a>
                         </td>
                         <td>[[item.description]]</td>
-                        <td hidden$="[[!_groupOwner]]">
+                        <td class="deleteColumn">
                           <gr-button
                               class="deleteIncludedGroupButton"
                               on-tap="_handleDeleteIncludedGroup">
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
index cde6c66..81bb902 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
@@ -48,6 +48,10 @@
         type: Boolean,
         value: false,
       },
+      _isAdmin: {
+        type: Boolean,
+        value: false,
+      },
     },
 
     behaviors: [
@@ -68,17 +72,29 @@
 
       return this.$.restAPI.getGroupConfig(this.groupId).then(
           config => {
+            if (!config.name) { return; }
+
             this._groupName = config.name;
+
+            promises.push(this.$.restAPI.getIsAdmin().then(isAdmin => {
+              this._isAdmin = isAdmin ? true : false;
+            }));
+
             promises.push(this.$.restAPI.getIsGroupOwner(config.name)
-                .then(isOwner => { this._groupOwner = isOwner; }));
+                .then(isOwner => {
+                  this._groupOwner = isOwner ? true : false;
+                }));
+
             promises.push(this.$.restAPI.getGroupMembers(config.name).then(
                 members => {
                   this._groupMembers = members;
                 }));
+
             promises.push(this.$.restAPI.getIncludedGroup(config.name)
                 .then(includedGroup => {
                   this._includedGroups = includedGroup;
                 }));
+
             return Promise.all(promises).then(() => {
               this._loading = false;
             });
@@ -223,5 +239,9 @@
             return groups;
           });
     },
+
+    _computeHideItemClass(owner, admin) {
+      return admin || owner ? '' : 'canModify';
+    },
   });
 })();
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.html b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.html
index 76a182b..ef89e37 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.html
@@ -152,5 +152,23 @@
         done();
       });
     });
+
+    test('_computeHideItemClass returns string for admin', () => {
+      const admin = true;
+      const owner = false;
+      assert.equal(element._computeHideItemClass(owner, admin), '');
+    });
+
+    test('_computeHideItemClass returns hideItem for admin and owner', () => {
+      const admin = false;
+      const owner = false;
+      assert.equal(element._computeHideItemClass(owner, admin), 'canModify');
+    });
+
+    test('_computeHideItemClass returns string for owner', () => {
+      const admin = false;
+      const owner = true;
+      assert.equal(element._computeHideItemClass(owner, admin), '');
+    });
   });
 </script>
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
index 2f3d59a..0449ffa 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
@@ -71,13 +71,15 @@
                 <gr-autocomplete
                     id="groupNameInput"
                     text="{{_groupConfig.name}}"
-                    disabled="[[!_groupOwner]]"></gr-autocomplete>
+                    disabled="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]"></gr-autocomplete>
               </span>
-              <gr-button
-                  id="inputUpdateNameBtn"
-                  on-tap="_handleSaveName"
-                  disabled="[[_computeButtonDisabled(_groupOwner, _rename)]]">
-                Rename Group</gr-button>
+              <span class="value" disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
+                <gr-button
+                    id="inputUpdateNameBtn"
+                    on-tap="_handleSaveName"
+                    disabled="[[!_rename]]">
+                  Rename Group</gr-button>
+              </span>
             </fieldset>
             <h3 class$="[[_computeHeaderClass(_owner)]]">
               Owners
@@ -87,13 +89,15 @@
                 <gr-autocomplete
                     text="{{_groupConfig.owner}}"
                     query="[[_query]]"
-                    disabled$="[[!_groupOwner]]">
+                    disabled="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
                 </gr-autocomplete>
               </span>
-              <gr-button
-                  on-tap="_handleSaveOwner"
-                  disabled="[[_computeButtonDisabled(_groupOwner, _owner)]]">
-                Change Owners</gr-button>
+              <span class="value" disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
+                <gr-button
+                    on-tap="_handleSaveOwner"
+                    disabled="[[!_owner]]">
+                  Change Owners</gr-button>
+              </span>
             </fieldset>
             <h3 class$="[[_computeHeaderClass(_description)]]">
               Description
@@ -104,14 +108,15 @@
                     class="description"
                     autocomplete="on"
                     bind-value="{{_groupConfig.description}}"
-                    disabled="[[!_groupOwner]]"></iron-autogrow-textarea>
+                    disabled="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]"></iron-autogrow-textarea>
               </div>
-              <gr-button
-                  on-tap="_handleSaveDescription"
-                  disabled=
-                      "[[_computeButtonDisabled(_groupOwner, _description)]]">
-                Save Description
-              </gr-button>
+              <span class="value" disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
+                <gr-button
+                    on-tap="_handleSaveDescription"
+                    disabled="[[!_description]]">
+                  Save Description
+                </gr-button>
+              </span>
             </fieldset>
             <h3 id="options" class$="[[_computeHeaderClass(_options)]]">
               Group Options
@@ -124,7 +129,7 @@
                 <span class="value">
                   <gr-select
                       bind-value="{{_groupConfig.options.visible_to_all}}">
-                    <select disabled$="[[!_groupOwner]]">
+                    <select disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
                       <template is="dom-repeat" items="[[_submitTypes]]">
                         <option value="[[item.value]]">[[item.label]]</option>
                       </template>
@@ -132,12 +137,13 @@
                   </gr-select>
                 </span>
               </section>
-              <gr-button
-                  on-tap="_handleSaveOptions"
-                  disabled=
-                         "[[_computeButtonDisabled(_groupOwner, _options)]]">
-                Save Group Options
-              </gr-button>
+              <span class="value" disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
+                <gr-button
+                    on-tap="_handleSaveOptions"
+                    disabled="[[!_options]]">
+                  Save Group Options
+                </gr-button>
+              </span>
             </fieldset>
           </fieldset>
         </div>
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.js b/polygerrit-ui/app/elements/admin/gr-group/gr-group.js
index 55bf3b0..9e0e8c6 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.js
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.js
@@ -56,11 +56,6 @@
         type: Boolean,
         value: true,
       },
-      _loggedIn: {
-        type: Boolean,
-        value: false,
-        observer: '_loggedInChanged',
-      },
       /** @type {?} */
       _groupConfig: Object,
       _groupName: Object,
@@ -80,6 +75,10 @@
           return this._getGroupSuggestions.bind(this);
         },
       },
+      _isAdmin: {
+        type: Boolean,
+        value: false,
+      },
     },
 
     observers: [
@@ -98,12 +97,22 @@
 
       return this.$.restAPI.getGroupConfig(this.groupId).then(
           config => {
-            this._groupConfig = config;
             this._groupName = config.name;
-            this.fire('title-change', {title: config.name});
-            this._loading = false;
+
+            this.$.restAPI.getIsAdmin().then(isAdmin => {
+              this._isAdmin = isAdmin ? true : false;
+            });
+
             this.$.restAPI.getIsGroupOwner(config.name)
-                .then(isOwner => { this._groupOwner = isOwner; });
+                .then(isOwner => {
+                  this._groupOwner = isOwner ? true : false;
+                });
+
+            this._groupConfig = config;
+
+            this.fire('title-change', {title: config.name});
+
+            this._loading = false;
           });
     },
 
@@ -111,18 +120,10 @@
       return loading ? 'loading' : '';
     },
 
-    _loggedInChanged(_loggedIn) {
-      if (!_loggedIn) { return; }
-    },
-
     _isLoading() {
       return this._loading || this._loading === undefined;
     },
 
-    _getLoggedIn() {
-      return this.$.restAPI.getLoggedIn();
-    },
-
     _handleSaveName() {
       return this.$.restAPI.saveGroupName(this.groupId, this._groupConfig.name)
           .then(config => {
@@ -182,10 +183,6 @@
       this._options = true;
     },
 
-    _computeButtonDisabled(options, option) {
-      return !options || !option;
-    },
-
     _computeHeaderClass(configChanged) {
       return configChanged ? 'edited' : '';
     },
@@ -204,5 +201,9 @@
             return groups;
           });
     },
+
+    _computeGroupDisabled(owner, admin) {
+      return admin || owner ? false : true;
+    },
   });
 })();
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group_test.html b/polygerrit-ui/app/elements/admin/gr-group/gr-group_test.html
index 75bd905..3c3b938 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group_test.html
@@ -121,5 +121,29 @@
             done();
           });
     });
+
+    test('_computeGroupDisabled return false for admin', () => {
+      const admin = true;
+      const owner = false;
+      assert.equal(element._computeGroupDisabled(owner, admin), false);
+    });
+
+    test('_computeGroupDisabled return true for admin', () => {
+      const admin = false;
+      const owner = false;
+      assert.equal(element._computeGroupDisabled(owner, admin), true);
+    });
+
+    test('_computeGroupDisabled return false for owner', () => {
+      const admin = false;
+      const owner = true;
+      assert.equal(element._computeGroupDisabled(owner, admin), false);
+    });
+
+    test('_computeGroupDisabled return true for owner', () => {
+      const admin = false;
+      const owner = false;
+      assert.equal(element._computeGroupDisabled(owner, admin), true);
+    });
   });
 </script>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index acc9d4f..3186c33 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -447,7 +447,8 @@
       this._mapRoute(RoutePattern.GROUP_AUDIT_LOG, '_handleGroupAuditLogRoute',
           true);
 
-      this._mapRoute(RoutePattern.GROUP_MEMBERS, '_handleGroupMembersRoute');
+      this._mapRoute(RoutePattern.GROUP_MEMBERS, '_handleGroupMembersRoute',
+          true);
 
       this._mapRoute(RoutePattern.GROUP_LIST_OFFSET,
           '_handleGroupListOffsetRoute', true);
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
index a9231b6..3a9d2d0 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -133,6 +133,7 @@
         '_handleGroupListFilterOffsetRoute',
         '_handleGroupListFilterRoute',
         '_handleGroupListOffsetRoute',
+        '_handleGroupMembersRoute',
         '_handleGroupRoute',
         '_handlePluginListFilterOffsetRoute',
         '_handlePluginListFilterRoute',
@@ -153,7 +154,6 @@
         '_handleDefaultRoute',
         '_handleChangeLegacyRoute',
         '_handleDiffLegacyRoute',
-        '_handleGroupMembersRoute',
         '_handleImproperlyEncodedPlusRoute',
         '_handlePassThroughRoute',
         '_handleProjectAccessRoute',
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 210c42f..ab90585 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -274,7 +274,7 @@
 
     getGroupConfig(group) {
       const encodeName = encodeURIComponent(group);
-      return this._fetchSharedCacheURL('/groups/' + encodeName + '/detail');
+      return this.fetchJSON(`/groups/${encodeName}/detail`);
     },
 
     /**
@@ -348,8 +348,8 @@
      */
     getIsGroupOwner(groupName) {
       const encodeName = encodeURIComponent(groupName);
-      return this._fetchSharedCacheURL('/groups/?owned&q=' + encodeName)
-          .then(configs => configs.hasOwnProperty(encodeName));
+      return this._fetchSharedCacheURL(`/groups/?owned&q=${encodeName}`)
+          .then(configs => configs.hasOwnProperty(groupName));
     },
 
     getGroupMembers(groupName) {