From 433a3be715d56af9d1ebbe5347993c185ac0dccc Mon Sep 17 00:00:00 2001 From: Yi Cai Date: Tue, 26 Mar 2024 08:12:36 -0400 Subject: [PATCH] fix(rbac): alert display issue after role creating/updating Signed-off-by: Yi Cai --- packages/backend/package.json | 2 +- .../src/components/CreateRole/RoleForm.tsx | 18 +++++----- .../RoleOverview/RoleOverviewPage.tsx | 3 ++ .../src/components/RolesList/RolesList.tsx | 3 +- .../rbac/src/hooks/useLocationToast.test.ts | 36 +++++++++++++++++++ plugins/rbac/src/hooks/useLocationToast.ts | 15 ++++++++ plugins/rbac/tests/rbac.spec.ts | 10 +++++- 7 files changed, 74 insertions(+), 13 deletions(-) create mode 100644 plugins/rbac/src/hooks/useLocationToast.test.ts create mode 100644 plugins/rbac/src/hooks/useLocationToast.ts diff --git a/packages/backend/package.json b/packages/backend/package.json index fcb87a3155..6ae795d0b0 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -38,7 +38,7 @@ "@backstage/plugin-search-backend-module-pg": "^0.5.22", "@backstage/plugin-search-backend-node": "^1.2.17", "@backstage/plugin-techdocs-backend": "^1.9.6", - "@janus-idp/backstage-plugin-rbac-backend": "^2.2.4", + "@janus-idp/backstage-plugin-rbac-backend": "^2.4.1", "@backstage/plugin-catalog-backend-module-scaffolder-entity-model": "^0.1.10", "@backstage/plugin-search-backend-module-catalog": "^0.1.17", "@backstage/plugin-search-backend-module-techdocs": "^0.1.17", diff --git a/plugins/rbac/src/components/CreateRole/RoleForm.tsx b/plugins/rbac/src/components/CreateRole/RoleForm.tsx index 6c7c9db325..c469e4c198 100644 --- a/plugins/rbac/src/components/CreateRole/RoleForm.tsx +++ b/plugins/rbac/src/components/CreateRole/RoleForm.tsx @@ -28,7 +28,6 @@ import { isSamePermissionPolicy, onlyInLeft, } from '../../utils/rbac-utils'; -import { useToast } from '../ToastContext'; import { AddedMembersTable } from './AddedMembersTable'; import { AddMembersForm } from './AddMembersForm'; import { PermissionPoliciesForm } from './PermissionPoliciesForm'; @@ -58,18 +57,19 @@ export const RoleForm = ({ submitLabel, initialValues, }: RoleFormProps) => { - const { setToastMessage } = useToast(); const [activeStep, setActiveStep] = React.useState(step || 0); const navigate = useNavigate(); const rbacApi = useApi(rbacApiRef); - const navigateTo = () => { + const navigateTo = (action?: string) => { + const stateProp = action + ? { state: { toastMessage: `Role ${action} successfully` } } + : { state: { toastMessage: '' } }; if (step && roleName) { const { kind, namespace, name } = getKindNamespaceName(roleName); - - navigate(`../roles/${kind}/${namespace}/${name}`); + navigate(`../roles/${kind}/${namespace}/${name}`, stateProp); } else { - navigate('..'); + navigate('..', stateProp); } }; @@ -125,8 +125,7 @@ export const RoleForm = ({ ); } } - setToastMessage(`Role ${name} updated successfully`); - navigateTo(); + navigateTo(`${name} updated`); } } catch (e) { formikHelpers.setStatus({ submitError: e }); @@ -156,8 +155,7 @@ export const RoleForm = ({ }`, ); } - setToastMessage(`Role ${newData.name} created successfully`); - navigateTo(); + navigateTo(`${newData.name} created`); } catch (e) { formikHelpers.setStatus({ submitError: e }); } diff --git a/plugins/rbac/src/components/RoleOverview/RoleOverviewPage.tsx b/plugins/rbac/src/components/RoleOverview/RoleOverviewPage.tsx index fa81e6e1bb..e6cf501c88 100644 --- a/plugins/rbac/src/components/RoleOverview/RoleOverviewPage.tsx +++ b/plugins/rbac/src/components/RoleOverview/RoleOverviewPage.tsx @@ -5,6 +5,7 @@ import { Header, Page, TabbedLayout } from '@backstage/core-components'; import { Grid } from '@material-ui/core'; +import { useLocationToast } from '../../hooks/useLocationToast'; import { SnackbarAlert } from '../SnackbarAlert'; import { useToast } from '../ToastContext'; import { AboutCard } from './AboutCard'; @@ -15,6 +16,8 @@ export const RoleOverviewPage = () => { const { roleName, roleNamespace, roleKind } = useParams(); const { toastMessage, setToastMessage } = useToast(); + useLocationToast(setToastMessage); + const onAlertClose = () => { setToastMessage(''); }; diff --git a/plugins/rbac/src/components/RolesList/RolesList.tsx b/plugins/rbac/src/components/RolesList/RolesList.tsx index 8ce1bfc719..f8fe9e13e1 100644 --- a/plugins/rbac/src/components/RolesList/RolesList.tsx +++ b/plugins/rbac/src/components/RolesList/RolesList.tsx @@ -4,6 +4,7 @@ import { Table, WarningPanel } from '@backstage/core-components'; import { makeStyles } from '@material-ui/core'; +import { useLocationToast } from '../../hooks/useLocationToast'; import { useRoles } from '../../hooks/useRoles'; import { RolesData } from '../../types'; import { SnackbarAlert } from '../SnackbarAlert'; @@ -24,7 +25,7 @@ const useStyles = makeStyles(theme => ({ export const RolesList = () => { const { toastMessage, setToastMessage } = useToast(); const { openDialog, setOpenDialog, deleteRoleName } = useDeleteDialog(); - + useLocationToast(setToastMessage); const [roles, setRoles] = React.useState(); const classes = useStyles(); const { loading, data, retry, createRoleAllowed, createRoleLoading, error } = diff --git a/plugins/rbac/src/hooks/useLocationToast.test.ts b/plugins/rbac/src/hooks/useLocationToast.test.ts new file mode 100644 index 0000000000..d8fbff68a9 --- /dev/null +++ b/plugins/rbac/src/hooks/useLocationToast.test.ts @@ -0,0 +1,36 @@ +import { useLocation } from 'react-router-dom'; + +import { renderHook } from '@testing-library/react-hooks'; + +import { useLocationToast } from './useLocationToast'; + +jest.mock('react-router-dom', () => ({ + useLocation: jest.fn(), +})); + +describe('useLocationToast', () => { + it('sets toast message based on location state', () => { + const mockSetToastMessage = jest.fn(); + + (useLocation as jest.Mock).mockReturnValue({ + state: { toastMessage: 'Success Message' }, + }); + + renderHook(() => useLocationToast(mockSetToastMessage)); + + expect(mockSetToastMessage).toHaveBeenCalledWith('Success Message'); + }); + + it('cleans up by setting toast message to an empty string', () => { + const mockSetToastMessage = jest.fn(); + + (useLocation as jest.Mock).mockReturnValue({ + state: { toastMessage: 'Success Message' }, + }); + + const { unmount } = renderHook(() => useLocationToast(mockSetToastMessage)); + unmount(); + + expect(mockSetToastMessage).toHaveBeenCalledWith(''); + }); +}); diff --git a/plugins/rbac/src/hooks/useLocationToast.ts b/plugins/rbac/src/hooks/useLocationToast.ts new file mode 100644 index 0000000000..efce011bba --- /dev/null +++ b/plugins/rbac/src/hooks/useLocationToast.ts @@ -0,0 +1,15 @@ +import { useEffect } from 'react'; +import { useLocation } from 'react-router-dom'; + +export const useLocationToast = ( + setToastMessage: (message: string) => void, +) => { + const location = useLocation(); + + useEffect(() => { + if (location?.state?.toastMessage) { + setToastMessage(location.state.toastMessage); + } + return () => setToastMessage(''); + }, [location, setToastMessage]); +}; diff --git a/plugins/rbac/tests/rbac.spec.ts b/plugins/rbac/tests/rbac.spec.ts index 1987591637..0e7ad8ff71 100644 --- a/plugins/rbac/tests/rbac.spec.ts +++ b/plugins/rbac/tests/rbac.spec.ts @@ -108,7 +108,7 @@ test.describe('RBAC plugin', () => { }); await page .getByPlaceholder('Search by user name or group name') - .fill('Guest Use'); + .fill('Guest User'); await page.getByText('Guest User').click(); await expect( page.getByRole('heading', { @@ -122,6 +122,14 @@ test.describe('RBAC plugin', () => { await clickButton('Save', page); await verifyText('Role role:default/rbac_admin updated successfully', page); + // alert doesn't show up after Cancel button is clicked + await page.locator(RoleOverviewPO.updateMembers).click(); + await expect(page.getByRole('heading', { name: 'Edit Role' })).toBeVisible({ + timeout: 20000, + }); + await clickButton('Cancel', page); + await expect(page.getByRole('alert')).toHaveCount(0); + // edit/update policies await page.locator(RoleOverviewPO.updatePolicies).click(); await expect(page.getByRole('heading', { name: 'Edit Role' })).toBeVisible({