From 9accbcfa8be4a04f30f96e47368b7dff6e4365f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Wed, 7 Dec 2022 10:22:42 +0000 Subject: [PATCH] Fix pro project role descriptions (#2612) https://linear.app/unleash/issue/2-485/bug-pro-access-page-the-tooltip-on-the-roles-is-not-working Fixes an issue where we would not show anything for project role permissions since we're not serving that specific endpoint below Enterprise level. This way we fallback to the access role descriptions, which are also nice and informative: ![image](https://user-images.githubusercontent.com/14320932/205987327-def46ef2-4010-47dd-ba7d-5a264f0b547d.png) PR also adds support for SWR options in ConditionalSWR and EnterpriseSWR. --- .../ProjectAccessAssign.tsx | 7 +- .../ProjectRoleDescription.tsx | 119 ++++++++++-------- ...tRoleDescriptionEnvironmentPermissions.tsx | 19 +++ ...ojectRoleDescriptionProjectPermissions.tsx | 17 +++ .../ProjectAccessRoleCell.tsx | 8 +- .../ProjectAccessTable/ProjectAccessTable.tsx | 1 + .../useChangeRequestConfig.ts | 4 +- .../useConditionalSWR/useConditionalSWR.ts | 23 ++++ .../useEnterpriseSWR/useEnterpriseSWR.ts | 28 ++--- .../usePendingChangeRequests.ts | 4 +- .../usePendingChangeRequestsForFeature.ts | 4 +- .../getters/useProjectRole/useProjectRole.ts | 10 +- 12 files changed, 162 insertions(+), 82 deletions(-) create mode 100644 frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescriptionEnvironmentPermissions/ProjectRoleDescriptionEnvironmentPermissions.tsx create mode 100644 frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescriptionProjectPermissions/ProjectRoleDescriptionProjectPermissions.tsx create mode 100644 frontend/src/hooks/api/getters/useConditionalSWR/useConditionalSWR.ts diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx index 26e750e103..1c7dea600b 100644 --- a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx @@ -392,7 +392,12 @@ export const ProjectAccessAssign = ({ } + show={ + + } /> diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescription.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescription.tsx index d9d68fa447..19b62b40a6 100644 --- a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescription.tsx +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescription.tsx @@ -2,6 +2,9 @@ import { styled, SxProps, Theme } from '@mui/material'; import { ForwardedRef, forwardRef, useMemo, VFC } from 'react'; import useProjectRole from 'hooks/api/getters/useProjectRole/useProjectRole'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; +import useProjectAccess from 'hooks/api/getters/useProjectAccess/useProjectAccess'; +import { ProjectRoleDescriptionProjectPermissions } from './ProjectRoleDescriptionProjectPermissions/ProjectRoleDescriptionProjectPermissions'; +import { ProjectRoleDescriptionEnvironmentPermissions } from './ProjectRoleDescriptionEnvironmentPermissions/ProjectRoleDescriptionEnvironmentPermissions'; const StyledDescription = styled('div', { shouldForwardProp: prop => @@ -46,15 +49,24 @@ interface IProjectRoleDescriptionStyleProps { interface IProjectRoleDescriptionProps extends IProjectRoleDescriptionStyleProps { roleId: number; + projectId: string; } export const ProjectRoleDescription: VFC = forwardRef( ( - { roleId, className, sx, ...props }: IProjectRoleDescriptionProps, + { + roleId, + projectId, + className, + sx, + ...props + }: IProjectRoleDescriptionProps, ref: ForwardedRef ) => { const { role } = useProjectRole(roleId.toString()); + const { access } = useProjectAccess(projectId); + const accessRole = access?.roles.find(role => role.id === roleId); const environments = useMemo(() => { const environments = new Set(); @@ -80,62 +92,65 @@ export const ProjectRoleDescription: VFC = ref={ref} > 0} show={ <> - - Project permissions - - - {role.permissions - ?.filter( - (permission: any) => - !permission.environment - ) - .map( - (permission: any) => - permission.displayName - ) - .sort() - .map((permission: any) => ( -

{permission}

- ))} -
+ + + Project permissions + + + + + + } + /> + + + Environment permissions + + {environments.map(environment => ( +
+ + {environment} + + + + +
+ ))} + + } + /> } - /> - - - Environment permissions - - {environments.map((environment: any) => ( -
- - {environment} - - - {role.permissions - .filter( - (permission: any) => - permission.environment === - environment - ) - .map( - (permission: any) => - permission.displayName - ) - .sort() - .map((permission: any) => ( -

- {permission} -

- ))} -
-
- ))} + + {accessRole?.name} + + + {accessRole?.description} + } /> diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescriptionEnvironmentPermissions/ProjectRoleDescriptionEnvironmentPermissions.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescriptionEnvironmentPermissions/ProjectRoleDescriptionEnvironmentPermissions.tsx new file mode 100644 index 0000000000..1f871a3371 --- /dev/null +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescriptionEnvironmentPermissions/ProjectRoleDescriptionEnvironmentPermissions.tsx @@ -0,0 +1,19 @@ +interface IProjectRoleDescriptionEnvironmentPermissionsProps { + environment: string; + permissions: any[]; +} + +export const ProjectRoleDescriptionEnvironmentPermissions = ({ + environment, + permissions, +}: IProjectRoleDescriptionEnvironmentPermissionsProps) => ( + <> + {permissions + .filter((permission: any) => permission.environment === environment) + .map((permission: any) => permission.displayName) + .sort() + .map((permission: any) => ( +

{permission}

+ ))} + +); diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescriptionProjectPermissions/ProjectRoleDescriptionProjectPermissions.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescriptionProjectPermissions/ProjectRoleDescriptionProjectPermissions.tsx new file mode 100644 index 0000000000..cb2cc73aed --- /dev/null +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectRoleDescription/ProjectRoleDescriptionProjectPermissions/ProjectRoleDescriptionProjectPermissions.tsx @@ -0,0 +1,17 @@ +interface IProjectRoleDescriptionProjectPermissionsProps { + permissions: any[]; +} + +export const ProjectRoleDescriptionProjectPermissions = ({ + permissions, +}: IProjectRoleDescriptionProjectPermissionsProps) => ( + <> + {permissions + ?.filter((permission: any) => !permission.environment) + .map((permission: any) => permission.displayName) + .sort() + .map((permission: any) => ( +

{permission}

+ ))} + +); diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessRoleCell/ProjectAccessRoleCell.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessRoleCell/ProjectAccessRoleCell.tsx index 81cd03220f..e3ad78e73e 100644 --- a/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessRoleCell/ProjectAccessRoleCell.tsx +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessRoleCell/ProjectAccessRoleCell.tsx @@ -17,12 +17,14 @@ const StyledPopover = styled(Popover)(() => ({ interface IProjectAccessRoleCellProps { roleId: number; + projectId: string; value?: string; emptyText?: string; } export const ProjectAccessRoleCell: VFC = ({ roleId, + projectId, value, emptyText, }) => { @@ -63,7 +65,11 @@ export const ProjectAccessRoleCell: VFC = ({ horizontal: 'left', }} > - + ); diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx index 18f9c8de99..72ad4b047b 100644 --- a/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx @@ -170,6 +170,7 @@ export const ProjectAccessTable: VFC = () => { Cell: ({ value, row: { original: row } }: any) => ( ), diff --git a/frontend/src/hooks/api/getters/useChangeRequestConfig/useChangeRequestConfig.ts b/frontend/src/hooks/api/getters/useChangeRequestConfig/useChangeRequestConfig.ts index 6b351aa1db..f3b93accf7 100644 --- a/frontend/src/hooks/api/getters/useChangeRequestConfig/useChangeRequestConfig.ts +++ b/frontend/src/hooks/api/getters/useChangeRequestConfig/useChangeRequestConfig.ts @@ -7,9 +7,9 @@ export const useChangeRequestConfig = (projectId: string) => { const { data, error, mutate } = useEnterpriseSWR< IChangeRequestEnvironmentConfig[] >( + [], formatApiPath(`api/admin/projects/${projectId}/change-requests/config`), - fetcher, - [] + fetcher ); return { diff --git a/frontend/src/hooks/api/getters/useConditionalSWR/useConditionalSWR.ts b/frontend/src/hooks/api/getters/useConditionalSWR/useConditionalSWR.ts new file mode 100644 index 0000000000..0162d0eaa4 --- /dev/null +++ b/frontend/src/hooks/api/getters/useConditionalSWR/useConditionalSWR.ts @@ -0,0 +1,23 @@ +import useSWR, { BareFetcher, Key, SWRConfiguration, SWRResponse } from 'swr'; +import { useEffect } from 'react'; + +export const useConditionalSWR = ( + condition: T, + fallback: Data, + key: Key, + fetcher: BareFetcher, + options: SWRConfiguration = {} +): SWRResponse => { + const result = useSWR( + key, + (path: string) => + condition ? fetcher(path) : Promise.resolve(fallback), + options + ); + + useEffect(() => { + result.mutate(); + }, [condition]); + + return result; +}; diff --git a/frontend/src/hooks/api/getters/useEnterpriseSWR/useEnterpriseSWR.ts b/frontend/src/hooks/api/getters/useEnterpriseSWR/useEnterpriseSWR.ts index c391bab7f5..1ccc1f53fe 100644 --- a/frontend/src/hooks/api/getters/useEnterpriseSWR/useEnterpriseSWR.ts +++ b/frontend/src/hooks/api/getters/useEnterpriseSWR/useEnterpriseSWR.ts @@ -1,33 +1,21 @@ -import useSWR, { BareFetcher, Key, SWRResponse } from 'swr'; -import { useEffect } from 'react'; +import { BareFetcher, Key, SWRConfiguration } from 'swr'; +import { useConditionalSWR } from '../useConditionalSWR/useConditionalSWR'; import useUiConfig from '../useUiConfig/useUiConfig'; -export const useConditionalSWR = ( - key: Key, - fetcher: BareFetcher, - condition: T -): SWRResponse => { - const result = useSWR(key, fetcher); - - useEffect(() => { - result.mutate(); - }, [condition]); - - return result; -}; - export const useEnterpriseSWR = ( + fallback: Data, key: Key, fetcher: BareFetcher, - fallback: Data + options: SWRConfiguration = {} ) => { const { isEnterprise } = useUiConfig(); const result = useConditionalSWR( + isEnterprise(), + fallback, key, - (path: string) => - isEnterprise() ? fetcher(path) : Promise.resolve(fallback), - isEnterprise() + fetcher, + options ); return result; diff --git a/frontend/src/hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests.ts b/frontend/src/hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests.ts index 9f8e939062..8e57a1997e 100644 --- a/frontend/src/hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests.ts +++ b/frontend/src/hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests.ts @@ -11,9 +11,9 @@ const fetcher = (path: string) => { export const usePendingChangeRequests = (project: string) => { const { data, error, mutate } = useEnterpriseSWR( + [], formatApiPath(`api/admin/projects/${project}/change-requests/pending`), - fetcher, - [] + fetcher ); return { diff --git a/frontend/src/hooks/api/getters/usePendingChangeRequestsForFeature/usePendingChangeRequestsForFeature.ts b/frontend/src/hooks/api/getters/usePendingChangeRequestsForFeature/usePendingChangeRequestsForFeature.ts index 5a196f1f70..aefb9f481d 100644 --- a/frontend/src/hooks/api/getters/usePendingChangeRequestsForFeature/usePendingChangeRequestsForFeature.ts +++ b/frontend/src/hooks/api/getters/usePendingChangeRequestsForFeature/usePendingChangeRequestsForFeature.ts @@ -14,11 +14,11 @@ export const usePendingChangeRequestsForFeature = ( featureName: string ) => { const { data, error, mutate } = useEnterpriseSWR( + [], formatApiPath( `api/admin/projects/${project}/change-requests/pending/${featureName}` ), - fetcher, - [] + fetcher ); return { diff --git a/frontend/src/hooks/api/getters/useProjectRole/useProjectRole.ts b/frontend/src/hooks/api/getters/useProjectRole/useProjectRole.ts index 2c476a7320..5763ae6ebf 100644 --- a/frontend/src/hooks/api/getters/useProjectRole/useProjectRole.ts +++ b/frontend/src/hooks/api/getters/useProjectRole/useProjectRole.ts @@ -1,7 +1,8 @@ -import useSWR, { mutate, SWRConfiguration } from 'swr'; +import { mutate, SWRConfiguration } from 'swr'; import { useState, useEffect } from 'react'; import { formatApiPath } from 'utils/formatPath'; import handleErrorResponses from '../httpErrorResponseHandler'; +import { useEnterpriseSWR } from '../useEnterpriseSWR/useEnterpriseSWR'; const useProjectRole = (id: string, options: SWRConfiguration = {}) => { const fetcher = () => { @@ -13,7 +14,12 @@ const useProjectRole = (id: string, options: SWRConfiguration = {}) => { .then(res => res.json()); }; - const { data, error } = useSWR(`api/admin/roles/${id}`, fetcher, options); + const { data, error } = useEnterpriseSWR( + {}, + `api/admin/roles/${id}`, + fetcher, + options + ); const [loading, setLoading] = useState(!error && !data); const refetch = () => {