From 3cf31451ec06c9f5bdc2ee07ae55161af2175fb3 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Fri, 4 Feb 2022 14:54:42 +0100 Subject: [PATCH] Access control: Reduce number of API calls for role picker (#44905) * Restucture state for TeamRolePicker and UserRolePicker Co-authored-by: Alex Khomenko --- .../components/RolePicker/TeamRolePicker.tsx | 19 ++++--------- .../components/RolePicker/UserRolePicker.tsx | 28 ++++--------------- public/app/features/admin/UserOrgs.tsx | 20 +++++++++++++ .../serviceaccounts/ServiceAccountsTable.tsx | 20 +++++++------ public/app/features/teams/TeamList.tsx | 4 +-- public/app/features/users/UsersTable.tsx | 11 ++------ 6 files changed, 46 insertions(+), 56 deletions(-) diff --git a/public/app/core/components/RolePicker/TeamRolePicker.tsx b/public/app/core/components/RolePicker/TeamRolePicker.tsx index c04b76180ac..e7f7d6cbd5c 100644 --- a/public/app/core/components/RolePicker/TeamRolePicker.tsx +++ b/public/app/core/components/RolePicker/TeamRolePicker.tsx @@ -1,38 +1,29 @@ import React, { FC, useState } from 'react'; import { useAsync } from 'react-use'; -import { contextSrv } from 'app/core/core'; -import { AccessControlAction, Role } from 'app/types'; +import { Role } from 'app/types'; import { RolePicker } from './RolePicker'; -import { fetchRoleOptions, fetchTeamRoles, updateTeamRoles } from './api'; +import { fetchTeamRoles, updateTeamRoles } from './api'; export interface Props { teamId: number; orgId?: number; - getRoleOptions?: () => Promise; + roleOptions: Role[]; disabled?: boolean; builtinRolesDisabled?: boolean; } -export const TeamRolePicker: FC = ({ teamId, orgId, getRoleOptions, disabled, builtinRolesDisabled }) => { - const [roleOptions, setRoleOptions] = useState([]); +export const TeamRolePicker: FC = ({ teamId, orgId, roleOptions, disabled, builtinRolesDisabled }) => { const [appliedRoles, setAppliedRoles] = useState([]); const { loading } = useAsync(async () => { try { - if (contextSrv.hasPermission(AccessControlAction.ActionRolesList)) { - let options = await (getRoleOptions ? getRoleOptions() : fetchRoleOptions(orgId)); - setRoleOptions(options.filter((option) => !option.name?.startsWith('managed:'))); - } else { - setRoleOptions([]); - } - const teamRoles = await fetchTeamRoles(teamId, orgId); setAppliedRoles(teamRoles); } catch (e) { // TODO handle error console.error('Error loading options'); } - }, [getRoleOptions, orgId, teamId]); + }, [orgId, teamId]); return ( void; - getRoleOptions?: () => Promise; - getBuiltinRoles?: () => Promise<{ [key: string]: Role[] }>; + roleOptions: Role[]; + builtInRoles?: { [key: string]: Role[] }; disabled?: boolean; builtinRolesDisabled?: boolean; } @@ -21,31 +21,15 @@ export const UserRolePicker: FC = ({ userId, orgId, onBuiltinRoleChange, - getRoleOptions, - getBuiltinRoles, + roleOptions, + builtInRoles, disabled, builtinRolesDisabled, }) => { - const [roleOptions, setRoleOptions] = useState([]); const [appliedRoles, setAppliedRoles] = useState([]); - const [builtInRoles, setBuiltinRoles] = useState>({}); const { loading } = useAsync(async () => { try { - if (contextSrv.hasPermission(AccessControlAction.ActionRolesList)) { - let options = await (getRoleOptions ? getRoleOptions() : fetchRoleOptions(orgId)); - setRoleOptions(options.filter((option) => !option.name?.startsWith('managed:'))); - } else { - setRoleOptions([]); - } - - if (contextSrv.hasPermission(AccessControlAction.ActionBuiltinRolesList)) { - const builtInRoles = await (getBuiltinRoles ? getBuiltinRoles() : fetchBuiltinRoles(orgId)); - setBuiltinRoles(builtInRoles); - } else { - setBuiltinRoles({}); - } - if (contextSrv.hasPermission(AccessControlAction.ActionUserRolesList)) { const userRoles = await fetchUserRoles(userId, orgId); setAppliedRoles(userRoles); @@ -56,7 +40,7 @@ export const UserRolePicker: FC = ({ // TODO handle error console.error('Error loading options'); } - }, [getBuiltinRoles, getRoleOptions, orgId, userId]); + }, [orgId, userId]); return ( { state = { currentRole: this.props.org.role, isChangingRole: false, + roleOptions: [], + builtInRoles: {}, }; + componentDidMount() { + if (contextSrv.licensedAccessControlEnabled()) { + if (contextSrv.hasPermission(AccessControlAction.ActionRolesList)) { + fetchRoleOptions(this.props.org.orgId) + .then((roles) => this.setState({ roleOptions: roles })) + .catch((e) => console.error(e)); + } + if (contextSrv.hasPermission(AccessControlAction.ActionBuiltinRolesList)) { + fetchRoleOptions(this.props.org.orgId) + .then((roles) => this.setState({ builtInRoles: roles })) + .catch((e) => console.error(e)); + } + } + } + onOrgRemove = () => { const { org } = this.props; this.props.onOrgRemove(org.orgId); @@ -184,6 +202,8 @@ class UnThemedOrgRow extends PureComponent { userId={user?.id || 0} orgId={org.orgId} builtInRole={org.role} + roleOptions={this.state.roleOptions} + builtInRoles={this.state.builtInRoles} onBuiltinRoleChange={this.onBuiltinRoleChange} builtinRolesDisabled={rolePickerDisabled} /> diff --git a/public/app/features/serviceaccounts/ServiceAccountsTable.tsx b/public/app/features/serviceaccounts/ServiceAccountsTable.tsx index c602db7b1c0..9e35adef38e 100644 --- a/public/app/features/serviceaccounts/ServiceAccountsTable.tsx +++ b/public/app/features/serviceaccounts/ServiceAccountsTable.tsx @@ -28,10 +28,15 @@ const ServiceAccountsTable: FC = (props) => { useEffect(() => { async function fetchOptions() { try { - let options = await fetchRoleOptions(orgId); - setRoleOptions(options); - const builtInRoles = await fetchBuiltinRoles(orgId); - setBuiltinRoles(builtInRoles); + if (contextSrv.hasPermission(AccessControlAction.ActionRolesList)) { + let options = await fetchRoleOptions(orgId); + setRoleOptions(options); + } + + if (contextSrv.hasPermission(AccessControlAction.ActionBuiltinRolesList)) { + const builtInRoles = await fetchBuiltinRoles(orgId); + setBuiltinRoles(builtInRoles); + } } catch (e) { console.error('Error loading options'); } @@ -41,9 +46,6 @@ const ServiceAccountsTable: FC = (props) => { } }, [orgId]); - const getRoleOptions = async () => roleOptions; - const getBuiltinRoles = async () => builtinRoles; - return ( <> @@ -89,8 +91,8 @@ const ServiceAccountsTable: FC = (props) => { orgId={orgId} builtInRole={serviceAccount.role} onBuiltinRoleChange={(newRole) => onRoleChange(newRole, serviceAccount)} - getRoleOptions={getRoleOptions} - getBuiltinRoles={getBuiltinRoles} + roleOptions={roleOptions} + builtInRoles={builtinRoles} disabled={rolePickerDisabled} /> ) : ( diff --git a/public/app/features/teams/TeamList.tsx b/public/app/features/teams/TeamList.tsx index 83894dc5e46..a584db108f2 100644 --- a/public/app/features/teams/TeamList.tsx +++ b/public/app/features/teams/TeamList.tsx @@ -43,7 +43,7 @@ export class TeamList extends PureComponent { componentDidMount() { this.fetchTeams(); - if (contextSrv.licensedAccessControlEnabled()) { + if (contextSrv.licensedAccessControlEnabled() && contextSrv.hasPermission(AccessControlAction.ActionRolesList)) { this.fetchRoleOptions(); } } @@ -95,7 +95,7 @@ export class TeamList extends PureComponent { {contextSrv.licensedAccessControlEnabled() && ( )}
- this.state.roleOptions} /> + diff --git a/public/app/features/users/UsersTable.tsx b/public/app/features/users/UsersTable.tsx index 97ae76998d5..c5c05f1faf2 100644 --- a/public/app/features/users/UsersTable.tsx +++ b/public/app/features/users/UsersTable.tsx @@ -26,15 +26,11 @@ const UsersTable: FC = (props) => { if (contextSrv.hasPermission(AccessControlAction.ActionRolesList)) { let options = await fetchRoleOptions(orgId); setRoleOptions(options); - } else { - setRoleOptions([]); } if (contextSrv.hasPermission(AccessControlAction.ActionBuiltinRolesList)) { const builtInRoles = await fetchBuiltinRoles(orgId); setBuiltinRoles(builtInRoles); - } else { - setBuiltinRoles({}); } } catch (e) { console.error('Error loading options'); @@ -45,9 +41,6 @@ const UsersTable: FC = (props) => { } }, [orgId]); - const getRoleOptions = async () => roleOptions; - const getBuiltinRoles = async () => builtinRoles; - return ( <> @@ -94,8 +87,8 @@ const UsersTable: FC = (props) => { orgId={orgId} builtInRole={user.role} onBuiltinRoleChange={(newRole) => onRoleChange(newRole, user)} - getRoleOptions={getRoleOptions} - getBuiltinRoles={getBuiltinRoles} + roleOptions={roleOptions} + builtInRoles={builtinRoles} disabled={!contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersRoleUpdate, user)} /> ) : (