From 3df4b92863c923d96d64a136470777f61a987fc2 Mon Sep 17 00:00:00 2001
From: Paladox none <thomasmulhall410@yahoo.com>
Date: Fri, 4 May 2018 16:29:24 +0000
Subject: [PATCH] Bug fixes for gr-group-audit-log

* This uses gr-account-link where possible except if the rest api returns id
  which only groups return.

* This will not add a url if it is not a user or does not include url in
  the rest api.

* This makes gr-group-audit-log show users avatars thus makes the page
  look alot better.

Change-Id: I1d227953a997ebe7ab72ce0dd12cbf6c5105bbc5
---
 .../gr-group-audit-log.html                   | 18 +++--
 .../gr-group-audit-log/gr-group-audit-log.js  | 45 +++++++-----
 .../gr-group-audit-log_test.html              | 72 ++++++-------------
 3 files changed, 60 insertions(+), 75 deletions(-)

diff --git a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.html b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.html
index 48f4ecc1f6..eb6a70887b 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.html
@@ -19,6 +19,7 @@ limitations under the License.
 <link rel="import" href="../../../bower_components/polymer/polymer.html">
 <link rel="import" href="../../../styles/gr-table-styles.html">
 <link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
 <link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
 <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
 
@@ -47,11 +48,20 @@ limitations under the License.
           </td>
           <td class="type">[[itemType(item.type)]]</td>
           <td class="member">
-            <a href$="[[_computeGroupUrl(item.member.group_id)]]">
-              [[_getNameForMember(item.member)]]
-            </a>
+            <template is="dom-if" if="[[_isGroupEvent(item.type)]]">
+              <a href$="[[_computeGroupUrl(item.member)]]">
+                [[_getNameForGroup(item.member)]]
+              </a>
+            </template>
+            <template is="dom-if" if="[[!_isGroupEvent(item.type)]]">
+              <gr-account-link account="[[item.member]]"></gr-account-link>
+              [[_getIdForUser(item.member)]]
+            </template>
+          </td>
+          <td class="by-user">
+            <gr-account-link account="[[item.user]]"></gr-account-link>
+            [[_getIdForUser(item.user)]]
           </td>
-          <td class="by-user">[[_getNameForUser(item.user)]]</td>
         </tr>
       </template>
     </table>
diff --git a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.js b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.js
index b150d68397..bc6a5d03b4 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.js
+++ b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.js
@@ -17,12 +17,14 @@
 (function() {
   'use strict';
 
+  const GROUP_EVENTS = ['ADD_GROUP', 'REMOVE_GROUP'];
+
   Polymer({
     is: 'gr-group-audit-log',
 
     properties: {
-      groupId: Object,
-      _auditLog: Object,
+      groupId: String,
+      _auditLog: Array,
       _loading: {
         type: Boolean,
         value: true,
@@ -63,12 +65,6 @@
       return item.disabled ? 'Disabled' : 'Enabled';
     },
 
-    _computeGroupUrl(id) {
-      if (!id) { return ''; }
-
-      return this.getBaseUrl() + '/admin/groups/' + id;
-    },
-
     itemType(type) {
       let item;
       switch (type) {
@@ -86,20 +82,31 @@
       return item;
     },
 
-    _getNameForUser(account) {
-      const accountId = account._account_id ? ' (' +
-        account._account_id + ')' : '';
-      return this._getNameForMember(account) + accountId;
+    _isGroupEvent(type) {
+      return GROUP_EVENTS.indexOf(type) !== -1;
     },
 
-    _getNameForMember(account) {
-      if (account && account.name) {
-        return account.name;
-      } else if (account && account.username) {
-        return account.username;
-      } else if (account && account.email) {
-        return account.email.split('@')[0];
+    _computeGroupUrl(group) {
+      if (group && group.url && group.id) {
+        return Gerrit.Nav.getUrlForGroup(group.id);
       }
+
+      return '';
+    },
+
+    _getIdForUser(account) {
+      return account._account_id ? ' (' + account._account_id + ')' : '';
+    },
+
+    _getNameForGroup(group) {
+      if (group && group.name) {
+        return group.name;
+      } else if (group && group.id) {
+        // The URL encoded id of the member
+        return decodeURIComponent(group.id);
+      }
+
+      return '';
     },
   });
 })();
diff --git a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log_test.html b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log_test.html
index 86e8a2585c..59a665b9b7 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log_test.html
@@ -47,81 +47,49 @@ limitations under the License.
     });
 
     suite('members', () => {
-      test('test getNameForMember', () => {
-        let account = {
-          member: {
-            username: 'test-user',
-            _account_id: 12,
-          },
-        };
-        assert.equal(element._getNameForMember(account.member, false),
-            'test-user');
-
-        account = {
+      test('test _getNameForGroup', () => {
+        let group = {
           member: {
             name: 'test-name',
-            _account_id: 12,
           },
         };
-        assert.equal(element._getNameForMember(account.member), 'test-name');
+        assert.equal(element._getNameForGroup(group.member), 'test-name');
 
-        account = {
-          user: {
-            email: 'test-email@gmail.com',
+        group = {
+          member: {
+            id: 'test-id',
           },
         };
-        assert.equal(element._getNameForMember(account.user), 'test-email');
+        assert.equal(element._getNameForGroup(group.member), 'test-id');
+      });
+
+      test('test _isGroupEvent', () => {
+        assert.isTrue(element._isGroupEvent('ADD_GROUP'));
+        assert.isTrue(element._isGroupEvent('REMOVE_GROUP'));
+
+        assert.isFalse(element._isGroupEvent('ADD_USER'));
+        assert.isFalse(element._isGroupEvent('REMOVE_USER'));
       });
     });
 
     suite('users', () => {
-      test('test _getName', () => {
-        let account = {
+      test('test _getIdForUser', () => {
+        const account = {
           user: {
             username: 'test-user',
             _account_id: 12,
           },
         };
-        assert.equal(element._getNameForUser(account.user), 'test-user (12)');
-
-        account = {
-          user: {
-            name: 'test-name',
-            _account_id: 12,
-          },
-        };
-        assert.equal(element._getNameForUser(account.user), 'test-name (12)');
-
-        account = {
-          user: {
-            email: 'test-email@gmail.com',
-            _account_id: 12,
-          },
-        };
-        assert.equal(element._getNameForUser(account.user), 'test-email (12)');
+        assert.equal(element._getIdForUser(account.user), ' (12)');
       });
 
       test('test _account_id not present', () => {
-        let account = {
+        const account = {
           user: {
             username: 'test-user',
           },
         };
-        assert.equal(element._getNameForUser(account.user), 'test-user');
-
-        account = {
-          user: {
-            name: 'test-name',
-          },
-        };
-        assert.equal(element._getNameForUser(account.user), 'test-name');
-
-        account = {
-          user: {
-            email: 'test-email@gmail.com',
-          },
-        };
-        assert.equal(element._getNameForUser(account.user), 'test-email');
+        assert.equal(element._getIdForUser(account.user), '');
       });
     });