Merge pull request #11422 from grafana/dashboard-acl-ux2

improved ux for permission list

pkg/api/dashboard_permission.go

@@ -29,6 +29,11 @@ func GetDashboardPermissionList(c *m.ReqContext) Response {
 	}
 
 	for _, perm := range acl {
+		perm.UserAvatarUrl = dtos.GetGravatarUrl(perm.UserEmail)
+
+		if perm.TeamId > 0 {
+			perm.TeamAvatarUrl = dtos.GetGravatarUrlWithDefault(perm.TeamEmail, perm.Team)
+		}
 		if perm.Slug != "" {
 			perm.Url = m.GetDashboardFolderUrl(perm.IsFolder, perm.Uid, perm.Slug)
 		}

pkg/api/dashboard_permission_test.go

@@ -143,7 +143,7 @@ func TestDashboardPermissionApiEndpoint(t *testing.T) {
 			})
 		})
 
-		Convey("When trying to override inherited permissions with lower presedence", func() {
+		Convey("When trying to override inherited permissions with lower precedence", func() {
 			origNewGuardian := guardian.New
 			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
 				CanAdminValue:                    true,

pkg/api/folder_permission.go

@@ -33,6 +33,12 @@ func GetFolderPermissionList(c *m.ReqContext) Response {
 		perm.FolderId = folder.Id
 		perm.DashboardId = 0
 
+		perm.UserAvatarUrl = dtos.GetGravatarUrl(perm.UserEmail)
+
+		if perm.TeamId > 0 {
+			perm.TeamAvatarUrl = dtos.GetGravatarUrlWithDefault(perm.TeamEmail, perm.Team)
+		}
+
 		if perm.Slug != "" {
 			perm.Url = m.GetDashboardFolderUrl(perm.IsFolder, perm.Uid, perm.Slug)
 		}

pkg/models/dashboard_acl.go

@@ -56,7 +56,10 @@ type DashboardAclInfoDTO struct {
 	UserId         int64          `json:"userId"`
 	UserLogin      string         `json:"userLogin"`
 	UserEmail      string         `json:"userEmail"`
+	UserAvatarUrl  string         `json:"userAvatarUrl"`
 	TeamId         int64          `json:"teamId"`
+	TeamEmail      string         `json:"teamEmail"`
+	TeamAvatarUrl  string         `json:"teamAvatarUrl"`
 	Team           string         `json:"team"`
 	Role           *RoleType      `json:"role,omitempty"`
 	Permission     PermissionType `json:"permission"`

pkg/services/sqlstore/dashboard_acl.go

@@ -92,6 +92,7 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error {
 				u.login AS user_login,
 				u.email AS user_email,
 				ug.name AS team,
+				ug.email AS team_email,
 				d.title,
 				d.slug,
 				d.uid,

public/app/core/components/Permissions/AddPermissions.tsx

@@ -39,7 +39,7 @@ class AddPermissions extends Component<IProps, any> {
       permissions.newItem.setUser(null, null);
       return;
     }
-    return permissions.newItem.setUser(user.id, user.login);
+    return permissions.newItem.setUser(user.id, user.login, user.avatarUrl);
   }
 
   teamPicked(team: Team) {
@@ -48,7 +48,7 @@ class AddPermissions extends Component<IProps, any> {
       permissions.newItem.setTeam(null, null);
       return;
     }
-    return permissions.newItem.setTeam(team.id, team.name);
+    return permissions.newItem.setTeam(team.id, team.name, team.avatarUrl);
   }
 
   permissionPicked(permission: OptionWithDescription) {

public/app/core/components/Permissions/DisabledPermissionsListItem.tsx

@@ -1,4 +1,4 @@
-import React, { Component } from 'react';
+import React, { Component } from 'react';
 import DescriptionPicker from 'app/core/components/Picker/DescriptionPicker';
 import { permissionOptions } from 'app/stores/PermissionsStore/PermissionsStore';
 
@@ -12,9 +12,12 @@ export default class DisabledPermissionListItem extends Component<IProps, any> {
 
     return (
       <tr className="gf-form-disabled">
-        <td style={{ width: '100%' }}>
-          <i className={`fa--permissions-list ${item.icon}`} />
-          <span dangerouslySetInnerHTML={{ __html: item.nameHtml }} />
+        <td style={{ width: '1%' }}>
+          <i style={{ width: '25px', height: '25px' }} className="gicon gicon-shield" />
+        </td>
+        <td style={{ width: '90%' }}>
+          {item.name}
+          <span className="filter-table__weak-italic"> (Role)</span>
         </td>
         <td />
         <td className="query-keyword">Can</td>

public/app/core/components/Permissions/Permissions.tsx

@@ -15,9 +15,8 @@ export interface DashboardAcl {
   permissionName?: string;
   role?: string;
   icon?: string;
-  nameHtml?: string;
+  name?: string;
   inherited?: boolean;
-  sortName?: string;
   sortRank?: number;
 }
 

public/app/core/components/Permissions/PermissionsList.tsx

@@ -1,4 +1,4 @@
-import React, { Component } from 'react';
+import React, { Component } from 'react';
 import PermissionsListItem from './PermissionsListItem';
 import DisabledPermissionsListItem from './DisabledPermissionsListItem';
 import { observer } from 'mobx-react';
@@ -23,7 +23,7 @@ class PermissionsList extends Component<IProps, any> {
           <DisabledPermissionsListItem
             key={0}
             item={{
-              nameHtml: 'Everyone with <span class="query-keyword">Admin</span> Role',
+              name: 'Admin',
               permission: 4,
               icon: 'fa fa-fw fa-street-view',
             }}

public/app/core/components/Permissions/PermissionsListItem.tsx

@@ -1,4 +1,4 @@
-import React from 'react';
+import React from 'react';
 import { observer } from 'mobx-react';
 import DescriptionPicker from 'app/core/components/Picker/DescriptionPicker';
 import { permissionOptions } from 'app/stores/PermissionsStore/PermissionsStore';
@@ -7,6 +7,30 @@ const setClassNameHelper = inherited => {
   return inherited ? 'gf-form-disabled' : '';
 };
 
+function ItemAvatar({ item }) {
+  if (item.userAvatarUrl) {
+    return <img className="filter-table__avatar" src={item.userAvatarUrl} />;
+  }
+  if (item.teamAvatarUrl) {
+    return <img className="filter-table__avatar" src={item.teamAvatarUrl} />;
+  }
+  if (item.role === 'Editor') {
+    return <i style={{ width: '25px', height: '25px' }} className="gicon gicon-editor" />;
+  }
+
+  return <i style={{ width: '25px', height: '25px' }} className="gicon gicon-viewer" />;
+}
+
+function ItemDescription({ item }) {
+  if (item.userId) {
+    return <span className="filter-table__weak-italic">(User)</span>;
+  }
+  if (item.teamId) {
+    return <span className="filter-table__weak-italic">(Team)</span>;
+  }
+  return <span className="filter-table__weak-italic">(Role)</span>;
+}
+
 export default observer(({ item, removeItem, permissionChanged, itemIndex, folderInfo }) => {
   const handleRemoveItem = evt => {
     evt.preventDefault();
@@ -21,9 +45,11 @@ export default observer(({ item, removeItem, permissionChanged, itemIndex, folde
 
   return (
     <tr className={setClassNameHelper(item.inherited)}>
-      <td style={{ width: '100%' }}>
-        <i className={`fa--permissions-list ${item.icon}`} />
-        <span dangerouslySetInnerHTML={{ __html: item.nameHtml }} />
+      <td style={{ width: '1%' }}>
+        <ItemAvatar item={item} />
+      </td>
+      <td style={{ width: '90%' }}>
+        {item.name} <ItemDescription item={item} />
       </td>
       <td>
         {item.inherited &&

public/app/stores/PermissionsStore/PermissionsStore.jest.ts

@@ -15,7 +15,23 @@ describe('PermissionsStore', () => {
           permission: 1,
           permissionName: 'View',
           teamId: 1,
-          teamName: 'MyTestTeam',
+          team: 'MyTestTeam',
+        },
+        {
+          id: 5,
+          dashboardId: 1,
+          permission: 1,
+          permissionName: 'View',
+          userId: 1,
+          userLogin: 'MyTestUser',
+        },
+        {
+          id: 6,
+          dashboardId: 1,
+          permission: 1,
+          permissionName: 'Edit',
+          teamId: 2,
+          team: 'MyTestTeam2',
         },
       ])
     );
@@ -48,15 +64,24 @@ describe('PermissionsStore', () => {
   });
 
   it('should save removed permissions automatically', async () => {
-    expect(store.items.length).toBe(3);
+    expect(store.items.length).toBe(5);
 
     await store.removeStoreItem(2);
 
-    expect(store.items.length).toBe(2);
+    expect(store.items.length).toBe(4);
     expect(backendSrv.post.mock.calls.length).toBe(1);
     expect(backendSrv.post.mock.calls[0][0]).toBe('/api/dashboards/id/1/permissions');
   });
 
+  it('should be sorted by sort rank and alphabetically', async () => {
+    expect(store.items[0].name).toBe('MyTestTeam');
+    expect(store.items[0].dashboardId).toBe(10);
+    expect(store.items[1].name).toBe('Editor');
+    expect(store.items[2].name).toBe('Viewer');
+    expect(store.items[3].name).toBe('MyTestTeam2');
+    expect(store.items[4].name).toBe('MyTestUser');
+  });
+
   describe('when one inherited and one not inherited team permission are added', () => {
     beforeEach(async () => {
       const overridingItemForChildDashboard = {
@@ -73,7 +98,18 @@ describe('PermissionsStore', () => {
     });
 
     it('should add new overriding permission', () => {
-      expect(store.items.length).toBe(4);
+      expect(store.items.length).toBe(6);
+    });
+
+    it('should be sorted by sort rank and alphabetically', async () => {
+      expect(store.items[0].name).toBe('MyTestTeam');
+      expect(store.items[0].dashboardId).toBe(10);
+      expect(store.items[1].name).toBe('Editor');
+      expect(store.items[2].name).toBe('Viewer');
+      expect(store.items[3].name).toBe('MyTestTeam');
+      expect(store.items[3].dashboardId).toBe(1);
+      expect(store.items[4].name).toBe('MyTestTeam2');
+      expect(store.items[5].name).toBe('MyTestUser');
     });
   });
 });

public/app/stores/PermissionsStore/PermissionsStore.ts

@@ -30,6 +30,8 @@ export const NewPermissionsItem = types
     ),
     userId: types.maybe(types.number),
     userLogin: types.maybe(types.string),
+    userAvatarUrl: types.maybe(types.string),
+    teamAvatarUrl: types.maybe(types.string),
     teamId: types.maybe(types.number),
     team: types.maybe(types.string),
     permission: types.optional(types.number, 1),
@@ -50,17 +52,19 @@ export const NewPermissionsItem = types
     },
   }))
   .actions(self => ({
-    setUser(userId: number, userLogin: string) {
+    setUser(userId: number, userLogin: string, userAvatarUrl: string) {
       self.userId = userId;
       self.userLogin = userLogin;
+      self.userAvatarUrl = userAvatarUrl;
       self.teamId = null;
       self.team = null;
     },
-    setTeam(teamId: number, team: string) {
+    setTeam(teamId: number, team: string, teamAvatarUrl: string) {
       self.userId = null;
       self.userLogin = null;
       self.teamId = teamId;
       self.team = team;
+      self.teamAvatarUrl = teamAvatarUrl;
     },
     setPermission(permission: number) {
       self.permission = permission;
@@ -121,16 +125,20 @@ export const PermissionsStore = types
           teamId: undefined,
           userLogin: undefined,
           userId: undefined,
+          userAvatarUrl: undefined,
+          teamAvatarUrl: undefined,
           role: undefined,
         };
         switch (self.newItem.type) {
           case aclTypeValues.GROUP.value:
             item.team = self.newItem.team;
             item.teamId = self.newItem.teamId;
+            item.teamAvatarUrl = self.newItem.teamAvatarUrl;
             break;
           case aclTypeValues.USER.value:
             item.userLogin = self.newItem.userLogin;
             item.userId = self.newItem.userId;
+            item.userAvatarUrl = self.newItem.userAvatarUrl;
             break;
           case aclTypeValues.VIEWER.value:
           case aclTypeValues.EDITOR.value:
@@ -147,6 +155,8 @@ export const PermissionsStore = types
         try {
           yield updateItems(self, updatedItems);
           self.items.push(newItem);
+          let sortedItems = self.items.sort((a, b) => b.sortRank - a.sortRank || a.name.localeCompare(b.name));
+          self.items = sortedItems;
           resetNewTypeInternal();
         } catch {}
         yield Promise.resolve();
@@ -206,9 +216,11 @@ const updateItems = (self, items) => {
 };
 
 const prepareServerResponse = (response, dashboardId: number, isFolder: boolean, isInRoot: boolean) => {
-  return response.map(item => {
-    return prepareItem(item, dashboardId, isFolder, isInRoot);
-  });
+  return response
+    .map(item => {
+      return prepareItem(item, dashboardId, isFolder, isInRoot);
+    })
+    .sort((a, b) => b.sortRank - a.sortRank || a.name.localeCompare(b.name));
 };
 
 const prepareItem = (item, dashboardId: number, isFolder: boolean, isInRoot: boolean) => {
@@ -216,21 +228,16 @@ const prepareItem = (item, dashboardId: number, isFolder: boolean, isInRoot: boo
 
   item.sortRank = 0;
   if (item.userId > 0) {
-    item.icon = 'fa fa-fw fa-user';
-    item.nameHtml = item.userLogin;
-    item.sortName = item.userLogin;
+    item.name = item.userLogin;
     item.sortRank = 10;
   } else if (item.teamId > 0) {
-    item.icon = 'fa fa-fw fa-users';
-    item.nameHtml = item.team;
-    item.sortName = item.team;
+    item.name = item.team;
     item.sortRank = 20;
   } else if (item.role) {
     item.icon = 'fa fa-fw fa-street-view';
-    item.nameHtml = `Everyone with <span class="query-keyword">${item.role}</span> Role`;
-    item.sortName = item.role;
+    item.name = item.role;
     item.sortRank = 30;
-    if (item.role === 'Viewer') {
+    if (item.role === 'Editor') {
       item.sortRank += 1;
     }
   }

public/app/stores/PermissionsStore/PermissionsStoreItem.ts

@@ -14,8 +14,9 @@ export const PermissionsStoreItem = types
     inherited: types.maybe(types.boolean),
     sortRank: types.maybe(types.number),
     icon: types.maybe(types.string),
-    nameHtml: types.maybe(types.string),
-    sortName: types.maybe(types.string),
+    name: types.maybe(types.string),
+    teamAvatarUrl: types.maybe(types.string),
+    userAvatarUrl: types.maybe(types.string),
   })
   .actions(self => ({
     updateRole: role => {

public/img/icons_dark_theme/icon_editor.svg

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Generator: Adobe Illustrator 19.0.1, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
+	 width="64px" height="64px" viewBox="-479 353 64 64" style="enable-background:new -479 353 64 64;" xml:space="preserve">
+<style type="text/css">
+	.st0{fill:#E3E2E2;}
+</style>
+<g>
+	<path class="st0" d="M-470.4,410h34.4c4.7,0,8.6-3.8,8.6-8.6v-17.3l-4.2,4.2v13.1c0,2.4-1.9,4.3-4.3,4.3h-34.4
+		c-2.4,0-4.3-1.9-4.3-4.3V376c0-2.4,1.9-4.3,4.3-4.3h32.1l4.2-4.2h-36.3c-4.7,0-8.6,3.8-8.6,8.6v25.5
+		C-479,406.2-475.2,410-470.4,410z"/>
+	
+		<rect x="-438.3" y="364.5" transform="matrix(-0.7071 -0.7071 0.7071 -0.7071 -1008.7032 339.9824)" class="st0" width="8.7" height="28.8"/>
+	<path class="st0" d="M-425.5,364.3l6.2,6.2l1.4-1.4l1.6-1.6c1.7-1.7,1.7-4.5,0-6.2c-1.7-1.7-4.5-1.7-6.2,0l-1.6,1.6L-425.5,364.3z"
+		/>
+	<polygon class="st0" points="-444.8,393.9 -442.3,393.5 -448.5,387.3 -448.9,389.8 -449.8,394.8 	"/>
+</g>
+</svg>

public/img/icons_dark_theme/icon_viewer.svg

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Generator: Adobe Illustrator 19.0.1, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
+	 width="64px" height="64px" viewBox="-479 353 64 64" style="enable-background:new -479 353 64 64;" xml:space="preserve">
+<style type="text/css">
+	.st0{fill:#E2E2E2;}
+</style>
+<path class="st0" d="M-415.1,384c-0.4-0.7-9.5-16.6-31.6-16.6c0,0-0.1,0-0.1,0c0,0,0,0,0,0c0,0-0.1,0-0.1,0
+	c-22,0.1-31.3,15.9-31.6,16.6c-0.3,0.6-0.3,1.3,0,1.9c0.4,0.7,9.6,16.5,31.6,16.6c0,0,0.1,0,0.1,0c0,0,0,0,0,0c0,0,0.1,0,0.1,0
+	c22.2,0,31.2-16,31.6-16.6C-414.8,385.3-414.8,384.6-415.1,384z M-446.9,399.3c-7.9,0-14.3-6.4-14.3-14.3c0-7.9,6.4-14.3,14.3-14.3
+	c7.9,0,14.3,6.4,14.3,14.3C-432.6,392.9-439,399.3-446.9,399.3z"/>
+<g>
+	<path class="st0" d="M-446.9,378.3c-0.9,0-1.8,0.2-2.6,0.5c1.2,0.4,2,1.5,2,2.9c0,1.7-1.4,3-3,3c-1.2,0-2.2-0.7-2.7-1.7
+		c-0.2,0.6-0.3,1.3-0.3,2c0,3.7,3,6.7,6.7,6.7c3.7,0,6.7-3,6.7-6.7S-443.2,378.3-446.9,378.3z"/>
+</g>
+</svg>

public/img/icons_light_theme/icon_editor.svg

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Generator: Adobe Illustrator 19.0.1, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
+	 width="64px" height="64px" viewBox="-479 353 64 64" style="enable-background:new -479 353 64 64;" xml:space="preserve">
+<style type="text/css">
+	.st0{fill:#52545C;}
+</style>
+<g>
+	<path class="st0" d="M-470.4,410h34.4c4.7,0,8.6-3.8,8.6-8.6v-17.3l-4.2,4.2v13.1c0,2.4-1.9,4.3-4.3,4.3h-34.4
+		c-2.4,0-4.3-1.9-4.3-4.3V376c0-2.4,1.9-4.3,4.3-4.3h32.1l4.2-4.2h-36.3c-4.7,0-8.6,3.8-8.6,8.6v25.5
+		C-479,406.2-475.2,410-470.4,410z"/>
+	
+		<rect x="-438.3" y="364.5" transform="matrix(-0.7071 -0.7071 0.7071 -0.7071 -1008.7032 339.9824)" class="st0" width="8.7" height="28.8"/>
+	<path class="st0" d="M-425.5,364.3l6.2,6.2l1.4-1.4l1.6-1.6c1.7-1.7,1.7-4.5,0-6.2c-1.7-1.7-4.5-1.7-6.2,0l-1.6,1.6L-425.5,364.3z"
+		/>
+	<polygon class="st0" points="-444.8,393.9 -442.3,393.5 -448.5,387.3 -448.9,389.8 -449.8,394.8 	"/>
+</g>
+</svg>

public/img/icons_light_theme/icon_viewer.svg

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Generator: Adobe Illustrator 19.0.1, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
+	 width="64px" height="64px" viewBox="-479 353 64 64" style="enable-background:new -479 353 64 64;" xml:space="preserve">
+<style type="text/css">
+	.st0{fill:#52545C;}
+</style>
+<path class="st0" d="M-415.1,384c-0.4-0.7-9.5-16.6-31.6-16.6c0,0-0.1,0-0.1,0c0,0,0,0,0,0c0,0-0.1,0-0.1,0
+	c-22,0.1-31.3,15.9-31.6,16.6c-0.3,0.6-0.3,1.3,0,1.9c0.4,0.7,9.6,16.5,31.6,16.6c0,0,0.1,0,0.1,0c0,0,0,0,0,0c0,0,0.1,0,0.1,0
+	c22.2,0,31.2-16,31.6-16.6C-414.8,385.3-414.8,384.6-415.1,384z M-446.9,399.3c-7.9,0-14.3-6.4-14.3-14.3c0-7.9,6.4-14.3,14.3-14.3
+	c7.9,0,14.3,6.4,14.3,14.3C-432.6,392.9-439,399.3-446.9,399.3z"/>
+<g>
+	<path class="st0" d="M-446.9,378.3c-0.9,0-1.8,0.2-2.6,0.5c1.2,0.4,2,1.5,2,2.9c0,1.7-1.4,3-3,3c-1.2,0-2.2-0.7-2.7-1.7
+		c-0.2,0.6-0.3,1.3-0.3,2c0,3.7,3,6.7,6.7,6.7c3.7,0,6.7-3,6.7-6.7S-443.2,378.3-446.9,378.3z"/>
+</g>
+</svg>

public/sass/base/_icons.scss

@@ -120,6 +120,10 @@
   background-image: url('../img/icons_#{$theme-name}_theme/icon_data_sources.svg');
 }
 
+.gicon-editor {
+  background-image: url('../img/icons_#{$theme-name}_theme/icon_editor.svg');
+}
+
 .gicon-folder-new {
   background-image: url('../img/icons_#{$theme-name}_theme/icon_add_folder.svg');
 }
@@ -180,6 +184,10 @@
   background-image: url('../img/icons_#{$theme-name}_theme/icon_variable.svg');
 }
 
+.gicon-viewer {
+  background-image: url('../img/icons_#{$theme-name}_theme/icon_viewer.svg');
+}
+
 .gicon-zoom-out {
   background-image: url('../img/icons_#{$theme-name}_theme/icon_zoom_out.svg');
 }

public/sass/components/_filter-table.scss

@@ -85,3 +85,7 @@
     }
   }
 }
+.filter-table__weak-italic {
+  font-style: italic;
+  color: $text-color-weak;
+}