Skip to content

fix(rbac): alert display issue after role creating/updating #1354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 8 additions & 10 deletions plugins/rbac/src/components/CreateRole/RoleForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -58,18 +57,19 @@ export const RoleForm = ({
submitLabel,
initialValues,
}: RoleFormProps) => {
const { setToastMessage } = useToast();
const [activeStep, setActiveStep] = React.useState<number>(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);
}
};

Expand Down Expand Up @@ -125,8 +125,7 @@ export const RoleForm = ({
);
}
}
setToastMessage(`Role ${name} updated successfully`);
navigateTo();
navigateTo(`${name} updated`);
}
} catch (e) {
formikHelpers.setStatus({ submitError: e });
Expand Down Expand Up @@ -156,8 +155,7 @@ export const RoleForm = ({
}`,
);
}
setToastMessage(`Role ${newData.name} created successfully`);
navigateTo();
navigateTo(`${newData.name} created`);
} catch (e) {
formikHelpers.setStatus({ submitError: e });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -15,6 +16,8 @@ export const RoleOverviewPage = () => {
const { roleName, roleNamespace, roleKind } = useParams();
const { toastMessage, setToastMessage } = useToast();

useLocationToast(setToastMessage);

const onAlertClose = () => {
setToastMessage('');
};
Expand Down
3 changes: 2 additions & 1 deletion plugins/rbac/src/components/RolesList/RolesList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<number | undefined>();
const classes = useStyles();
const { loading, data, retry, createRoleAllowed, createRoleLoading, error } =
Expand Down
36 changes: 36 additions & 0 deletions plugins/rbac/src/hooks/useLocationToast.test.ts
Original file line number Diff line number Diff line change
@@ -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('');
});
});
15 changes: 15 additions & 0 deletions plugins/rbac/src/hooks/useLocationToast.ts
Original file line number Diff line number Diff line change
@@ -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]);
};
10 changes: 9 additions & 1 deletion plugins/rbac/tests/rbac.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand All @@ -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({
Expand Down