From de5cf9db8a97414241e84b5b51853bb77f9e878c Mon Sep 17 00:00:00 2001 From: Novak Zaballa <41410593+novakzaballa@users.noreply.github.com> Date: Thu, 11 Jan 2024 09:18:02 -0400 Subject: [PATCH] fix: Reading role permissions generates 500 errors (#3009) --- api/users/serializers.py | 5 +- frontend/common/services/useGroupWithRole.ts | 74 +++++++++ frontend/common/services/useRolePermission.ts | 10 +- .../common/services/useRolePermissionGroup.ts | 12 +- frontend/common/services/useRolesUser.ts | 15 +- frontend/common/services/useUserWithRole.ts | 73 +++++++++ frontend/common/types/requests.ts | 4 + frontend/common/types/responses.ts | 2 + frontend/web/components/EditPermissions.tsx | 140 ++++++++++++++---- frontend/web/components/base/forms/Tabs.js | 4 +- frontend/web/components/modals/CreateRole.tsx | 1 + .../pages/OrganisationSettingsPage.js | 2 + 12 files changed, 299 insertions(+), 43 deletions(-) create mode 100644 frontend/common/services/useGroupWithRole.ts create mode 100644 frontend/common/services/useUserWithRole.ts diff --git a/api/users/serializers.py b/api/users/serializers.py index 789b1b1412f5..2f8f2fa088ce 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -58,7 +58,10 @@ class UserListSerializer(serializers.ModelSerializer): join_date = serializers.SerializerMethodField(read_only=True) default_fields = ("id", "email", "first_name", "last_name", "last_login") - organisation_users_fields = ("role", "date_joined") + organisation_users_fields = ( + "role", + "date_joined", + ) class Meta: model = FFAdminUser diff --git a/frontend/common/services/useGroupWithRole.ts b/frontend/common/services/useGroupWithRole.ts new file mode 100644 index 000000000000..73a496f3c016 --- /dev/null +++ b/frontend/common/services/useGroupWithRole.ts @@ -0,0 +1,74 @@ +import { Res } from 'common/types/responses' +import { Req } from 'common/types/requests' +import { service } from 'common/service' + +export const groupWithRoleService = service + .enhanceEndpoints({ addTagTypes: ['GroupWithRole', 'RolePermissionGroup'] }) + .injectEndpoints({ + endpoints: (builder) => ({ + deleteGroupWithRole: builder.mutation< + Res['groupWithRole'], + Req['deleteGroupWithRole'] + >({ + invalidatesTags: [{ type: 'GroupWithRole' }, { type: 'RolePermissionGroup' }], + query: (query: Req['deleteGroupWithRole']) => ({ + body: query, + method: 'DELETE', + url: `organisations/${query.org_id}/groups/${query.group_id}/roles/${query.role_id}/`, + }), + transformResponse: () => { + toast('Group role was removed') + }, + }), + getGroupWithRole: builder.query< + Res['groupWithRole'], + Req['getGroupWithRole'] + >({ + providesTags: (result, error, group) => { + const tags = result ? [{ id: group.id, type: 'GroupWithRole' }] : [] + return tags + }, + query: (query: Req['getGroupWithRole']) => ({ + url: `organisations/${query.org_id}/groups/${query.group_id}/roles/`, + }), + }), + // END OF ENDPOINTS + }), + }) + +export async function deleteGroupWithRole( + store: any, + data: Req['deleteGroupWithRole'], + options?: Parameters< + typeof groupWithRoleService.endpoints.deleteGroupWithRole.initiate + >[1], +) { + return store.dispatch( + groupWithRoleService.endpoints.deleteGroupWithRole.initiate(data, options), + ) +} +export async function getGroupWithRole( + store: any, + data: Req['getGroupWithRole'], + options?: Parameters< + typeof groupWithRoleService.endpoints.getGroupWithRole.initiate + >[1], +) { + return store.dispatch( + groupWithRoleService.endpoints.getGroupWithRole.initiate(data, options), + ) +} + +// END OF FUNCTION_EXPORTS + +export const { + useDeleteGroupWithRoleMutation, + useGetGroupWithRoleQuery, + // END OF EXPORTS +} = groupWithRoleService + +/* Usage examples: +const { data, isLoading } = useGetGroupWithRoleQuery({ id: 2 }, {}) //get hook +const [createGroupWithRole, { isLoading, data, isSuccess }] = useCreateGroupWithRoleMutation() //create hook +groupWithRoleService.endpoints.getGroupWithRole.select({id: 2})(store.getState()) //access data from any function +*/ diff --git a/frontend/common/services/useRolePermission.ts b/frontend/common/services/useRolePermission.ts index 535ee22c30a4..137ee4bbd967 100644 --- a/frontend/common/services/useRolePermission.ts +++ b/frontend/common/services/useRolePermission.ts @@ -12,7 +12,7 @@ export const rolePermissionService = service >({ invalidatesTags: (res) => [ { id: 'LIST', type: 'rolePermission' }, - { id: res?.id, type: 'rolePermission' }, + { type: 'rolePermission' }, ], query: (query: Req['updateRolePermission']) => ({ body: query.body, @@ -46,7 +46,7 @@ export const rolePermissionService = service Res['rolePermission'], Req['getRolePermission'] >({ - providesTags: (res) => [{ id: res?.id, type: 'RolePermission' }], + providesTags: (res) => [{ id: res?.id, type: 'rolePermission' }], query: (query: Req['getRolePermission']) => ({ url: `organisations/${query.organisation_id}/roles/${query.role_id}/projects-permissions/?project=${query.project_id}`, }), @@ -56,7 +56,7 @@ export const rolePermissionService = service Res['rolePermission'], Req['getRolePermission'] >({ - providesTags: (res) => [{ id: res?.id, type: 'RolePermission' }], + providesTags: (res) => [{ id: res?.id, type: 'rolePermission' }], query: (query: Req['getRolePermission']) => ({ url: `organisations/${query.organisation_id}/roles/${query.role_id}/environments-permissions/?environment=${query.env_id}`, }), @@ -66,7 +66,7 @@ export const rolePermissionService = service Res['rolePermission'], Req['getRolePermission'] >({ - providesTags: (res) => [{ id: res?.id, type: 'RolePermission' }], + providesTags: (res) => [{ id: res?.id, type: 'rolePermission' }], query: (query: Req['getRolePermission']) => ({ url: `organisations/${query.organisation_id}/roles/${query.role_id}/projects-permissions/?project=${query.project_id}`, }), @@ -76,7 +76,7 @@ export const rolePermissionService = service Res['rolePermission'], Req['updateRolePermission'] >({ - invalidatesTags: [{ id: 'LIST', type: 'rolePermission' }], + invalidatesTags: [{ type: 'rolePermission' }], query: (query: Req['updateRolePermission']) => ({ body: query.body, method: 'PUT', diff --git a/frontend/common/services/useRolePermissionGroup.ts b/frontend/common/services/useRolePermissionGroup.ts index 85389dbd4862..00903f8dc5dd 100644 --- a/frontend/common/services/useRolePermissionGroup.ts +++ b/frontend/common/services/useRolePermissionGroup.ts @@ -3,14 +3,17 @@ import { Req } from 'common/types/requests' import { service } from 'common/service' export const rolePermissionGroupService = service - .enhanceEndpoints({ addTagTypes: ['RolePermissionGroup'] }) + .enhanceEndpoints({ addTagTypes: ['RolePermissionGroup', 'GroupWithRole'] }) .injectEndpoints({ endpoints: (builder) => ({ createRolePermissionGroup: builder.mutation< Res['rolePermissionGroup'], Req['createRolePermissionGroup'] >({ - invalidatesTags: [{ id: 'LIST', type: 'RolePermissionGroup' }], + invalidatesTags: [ + { id: 'LIST', type: 'RolePermissionGroup' }, + { type: 'GroupWithRole' }, + ], query: (query: Req['createRolePermissionGroup']) => ({ body: query.data, method: 'POST', @@ -21,7 +24,10 @@ export const rolePermissionGroupService = service Res['rolePermissionGroup'], Req['deleteRolePermissionGroup'] >({ - invalidatesTags: [{ id: 'LIST', type: 'RolePermissionGroup' }], + invalidatesTags: [ + { type: 'RolePermissionGroup' }, + { type: 'GroupWithRole' }, + ], query: (query: Req['deleteRolePermissionGroup']) => ({ body: query, method: 'DELETE', diff --git a/frontend/common/services/useRolesUser.ts b/frontend/common/services/useRolesUser.ts index 4f007877ddcb..36df5ece6a49 100644 --- a/frontend/common/services/useRolesUser.ts +++ b/frontend/common/services/useRolesUser.ts @@ -3,14 +3,17 @@ import { Req } from 'common/types/requests' import { service } from 'common/service' export const rolesUserService = service - .enhanceEndpoints({ addTagTypes: ['RolesUser'] }) + .enhanceEndpoints({ addTagTypes: ['RolesUser', 'User-role'] }) .injectEndpoints({ endpoints: (builder) => ({ createRolesPermissionUsers: builder.mutation< Res['rolesUsers'], Req['createRolesPermissionUsers'] >({ - invalidatesTags: [{ id: 'LIST', type: 'RolesUser' }], + invalidatesTags: [ + { type: 'User-role' }, + { id: 'LIST', type: 'RolesUser' }, + ], query: (query: Req['createRolesPermissionUsers']) => ({ body: query.data, method: 'POST', @@ -21,12 +24,18 @@ export const rolesUserService = service Res['rolesUsers'], Req['deleteRolesPermissionUsers'] >({ - invalidatesTags: [{ id: 'LIST', type: 'RolesUser' }], + invalidatesTags: [ + { type: 'User-role' }, + { type: 'RolesUser' }, + ], query: (query: Req['deleteRolesPermissionUsers']) => ({ body: query, method: 'DELETE', url: `organisations/${query.organisation_id}/roles/${query.role_id}/users/${query.user_id}/`, }), + transformResponse: () => { + toast('User role was removed') + }, }), getRolesPermissionUsers: builder.query< Res['rolesUsers'], diff --git a/frontend/common/services/useUserWithRole.ts b/frontend/common/services/useUserWithRole.ts new file mode 100644 index 000000000000..36b8df906095 --- /dev/null +++ b/frontend/common/services/useUserWithRole.ts @@ -0,0 +1,73 @@ +import { Res } from 'common/types/responses' +import { Req } from 'common/types/requests' +import { service } from 'common/service' + +export const userWithRolesService = service + .enhanceEndpoints({ addTagTypes: ['User-role', 'RolesUser'] }) + .injectEndpoints({ + endpoints: (builder) => ({ + deleteUserWithRoles: builder.mutation< + Res['User-role'], + Req['deleteUserWithRoles'] + >({ + invalidatesTags: [{ type: 'User-role' }, { type: 'RolesUser' }], + query: (query: Req['deleteUserWithRoles']) => ({ + method: 'DELETE', + url: `organisations/${query.org_id}/users/${query.user_id}/roles/${query.role_id}/`, + }), + transformResponse: () => { + toast('User role was removed') + }, + }), + getUserWithRoles: builder.query< + Res['userWithRoles'], + Req['getUserWithRoles'] + >({ + invalidatesTags: [{ type: 'User-role' }], + providesTags: (result, error, userId) => { + const tags = result ? [{ id: userId, type: 'User-role' }] : [] + return tags + }, + query: (query: Req['getUserWithRoles']) => ({ + url: `organisations/${query.org_id}/users/${query.user_id}/roles/`, + }), + }), + // END OF ENDPOINTS + }), + }) + +export async function getUserWithRoles( + store: any, + data: Req['getUserWithRoles'], + options?: Parameters< + typeof userWithRolesService.endpoints.getUserWithRoles.initiate + >[1], +) { + return store.dispatch( + userWithRolesService.endpoints.getUserWithRoles.initiate(data, options), + ) +} + +export async function deleteUserRole( + store: any, + data: Req['deleteUserWithRoles'], + options?: Parameters< + typeof UserRoleService.endpoints.deleteUserWithRoles.initiate + >[1], +) { + return store.dispatch( + UserRoleService.endpoints.deleteUserWithRoles.initiate(data, options), + ) +} +// END OF FUNCTION_EXPORTS + +export const { + useDeleteUserWithRolesMutation, + useGetUserWithRolesQuery, + // END OF EXPORTS +} = userWithRolesService + +/* Usage examples: +const { data, isLoading } = useUserWithRolesQuery({ id: 2 }, {}) //get hook +userWithRolesService.endpoints.getUserWithRoles.select({id: 2})(store.getState()) //access data from any function +*/ diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index dc15afe41b5e..ba5e0745747a 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -133,6 +133,10 @@ export type Req = { createLaunchDarklyProjectImport: { project_id: string } getLaunchDarklyProjectImport: { project_id: string } getLaunchDarklyProjectsImport: { project_id: string; import_id: string } + getUserWithRoles: { org_id: string; user_id: string } + deleteUserWihRole: { org_id: string; user_id: string; role_id: string } + getGroupWithRole: { org_id: string; group_id: string } + deleteGroupWithRole: { org_id: string; group_id: string; role_id: string } getChangeRequests: PagedRequest<{ search?: string environmentId: string diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 34b36a1edf91..ff4ac3ea343f 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -393,6 +393,8 @@ export type Res = { environment: Environment launchDarklyProjectImport: LaunchDarklyProjectImport launchDarklyProjectsImport: LaunchDarklyProjectImport[] + userWithRoles: PagedResponse + groupWithRole: PagedResponse changeRequests: PagedResponse groupSummaries: UserGroupSummary[] // END OF TYPES diff --git a/frontend/web/components/EditPermissions.tsx b/frontend/web/components/EditPermissions.tsx index 48026328cb96..ef2f15da4daa 100644 --- a/frontend/web/components/EditPermissions.tsx +++ b/frontend/web/components/EditPermissions.tsx @@ -50,6 +50,16 @@ import { useDeleteRolePermissionGroupMutation, } from 'common/services/useRolePermissionGroup' +import { + useGetUserWithRolesQuery, + useDeleteUserWithRolesMutation, +} from 'common/services/useUserWithRole' + +import { + useGetGroupWithRoleQuery, + useDeleteGroupWithRoleMutation, +} from 'common/services/useGroupWithRole' + import MyRoleSelect from './MyRoleSelect' const OrganisationProvider = require('common/providers/OrganisationProvider') const Project = require('common/project') @@ -70,6 +80,8 @@ type EditPermissionModalType = { role?: Role permissionChanged: () => void editPermissionsFromSettings?: boolean + isEditUserPermission?: boolean + isEditGroupPermission?: boolean } type EditPermissionsType = Omit & { @@ -93,12 +105,16 @@ const _EditPermissionsModal: FC = forwardRef( const [parentError, setParentError] = useState(false) const [saving, setSaving] = useState(false) const [showRoles, setShowRoles] = useState(false) + const [permissionWasCreated, setPermissionWasCreated] = + useState(false) const [rolesSelected, setRolesSelected] = useState([]) const { editPermissionsFromSettings, envId, group, id, + isEditGroupPermission, + isEditUserPermission, isGroup, level, name, @@ -137,6 +153,46 @@ const _EditPermissionsModal: FC = forwardRef( [], ) const { data: permissions } = useGetAvailablePermissionsQuery({ level }) + const { data: userWithRolesData, isSuccess: userWithRolesDataSuccesfull } = + useGetUserWithRolesQuery( + { + org_id: id, + user_id: user?.id, + }, + { skip: level !== 'organisation' || !user?.id }, + ) + + const { + data: groupWithRolesData, + isSuccess: groupWithRolesDataSuccesfull, + } = useGetGroupWithRoleQuery( + { + group_id: group?.id, + org_id: id, + }, + { skip: level !== 'organisation' || !group?.id }, + ) + + useEffect(() => { + if (user && userWithRolesDataSuccesfull) { + const resultArray = userWithRolesData?.results?.map((userRole) => ({ + role: userRole.id, + user_role_id: user?.id, + })) + setRolesSelected(resultArray) + } + }, [userWithRolesDataSuccesfull]) + + useEffect(() => { + if (group && groupWithRolesDataSuccesfull) { + const resultArray = groupWithRolesData?.results?.map((groupRole) => ({ + group_role_id: group?.id, + role: groupRole.id, + })) + setRolesSelected(resultArray) + } + }, [groupWithRolesDataSuccesfull]) + const processResults = (results: (UserPermission & GroupPermission)[]) => { let entityPermissions: | (Omit & { @@ -167,7 +223,8 @@ const _EditPermissionsModal: FC = forwardRef( ] = useCreateRolesPermissionUsersMutation() const [deleteRolePermissionUser] = useDeleteRolesPermissionUsersMutation() - + const [deleteUserWithRoles] = useDeleteUserWithRolesMutation() + const [deleteGroupWithRoles] = useDeleteGroupWithRoleMutation() const [ createRolePermissionGroup, { data: groupsData, isSuccess: groupAdded }, @@ -199,6 +256,7 @@ const _EditPermissionsModal: FC = forwardRef( setSaving(true) } if (isRolePermCreated || isRolePermUpdated) { + setPermissionWasCreated(true) toast( `${level.charAt(0).toUpperCase() + level.slice(1)} permissions Saved`, ) @@ -227,7 +285,7 @@ const _EditPermissionsModal: FC = forwardRef( organisation_id: role?.organisation, role_id: role?.id, }, - { skip: !role }, + { skip: !role || level !== 'organisation' }, ) const { data: projectPermissions, isLoading: projectIsLoading } = @@ -241,7 +299,10 @@ const _EditPermissionsModal: FC = forwardRef( skip: !id || envId || - !Utils.getFlagsmithHasFeature('show_role_management'), + // TODO: https://github.com/Flagsmith/flagsmith/issues/3020 + !role?.organisation || + !Utils.getFlagsmithHasFeature('show_role_management') || + level !== 'project', }, ) @@ -256,9 +317,11 @@ const _EditPermissionsModal: FC = forwardRef( skip: !role || !id || - !Utils.getFlagsmithHasFeature('show_role_management'), + !Utils.getFlagsmithHasFeature('show_role_management') || + level !== 'environment', }, ) + useEffect(() => { if ( !organisationIsLoading && @@ -302,7 +365,7 @@ const _EditPermissionsModal: FC = forwardRef( (v) => v === `VIEW_${parentLevel.toUpperCase()}`, ) ) { - // e.g. trying to set an environment permission but don't have view_projec + // e.g. trying to set an environment permission but don't have view_project setParentError(true) } else { setParentError(false) @@ -373,7 +436,7 @@ const _EditPermissionsModal: FC = forwardRef( body.admin = entityPermissions.admin body.environment = envId || id } - if (entityId) { + if (entityId || permissionWasCreated) { updateRolePermissions({ body, id: entityId, @@ -456,22 +519,37 @@ const _EditPermissionsModal: FC = forwardRef( const roleSelected = rolesAdded.find((item) => item.id === roleId) if (level === 'organisation') { if (user) { - deleteRolePermissionUser({ - organisation_id: id, - role_id: roleId, - user_id: roleSelected.user_role_id, - }) + if (isEditUserPermission) { + deleteUserWithRoles({ + org_id: id, + role_id: roleId, + user_id: user?.id, + }) + } else { + deleteRolePermissionUser({ + organisation_id: id, + role_id: roleId, + user_id: roleSelected.user_role_id, + }) + } } if (group) { - deleteRolePermissionGroup({ - group_id: roleSelected.group_role_id, - organisation_id: id, - role_id: roleId, - }) + if (isEditGroupPermission) { + deleteGroupWithRoles({ + group_id: group?.id, + org_id: id, + role_id: roleId, + }) + } else { + deleteRolePermissionGroup({ + group_id: roleSelected.group_role_id, + organisation_id: id, + role_id: roleId, + }) + } } } setRolesSelected((rolesSelected || []).filter((v) => v.role !== roleId)) - toast('User role was removed') } useEffect(() => { @@ -679,19 +757,21 @@ const _EditPermissionsModal: FC = forwardRef( /> )} - {Utils.getFlagsmithHasFeature('show_role_management') && ( -
- v.role)} - onAdd={addRole} - onRemove={removeOwner} - isOpen={showRoles} - onToggle={() => setShowRoles(!showRoles)} - /> -
- )} + {Utils.getFlagsmithHasFeature('show_role_management') && + level !== 'environment' && + level !== 'project' && ( +
+ v.role)} + onAdd={addRole} + onRemove={removeOwner} + isOpen={showRoles} + onToggle={() => setShowRoles(!showRoles)} + /> +
+ )}
{!role && (