Skip to content

Commit adf92ef

Browse files
fix: hide remove role button for users without permissions (#142)
* fix: hide remove role button for users without permissions * fix: adding missing tooltip to delete button and fixing race condition
1 parent 2b3b480 commit adf92ef

15 files changed

Lines changed: 568 additions & 134 deletions

File tree

src/authz-module/audit-user/index.test.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ jest.mock('@edx/frontend-component-header', () => ({
3030
jest.mock('@src/data/hooks', () => ({
3131
...jest.requireActual('@src/data/hooks'),
3232
useUserAccount: jest.fn(),
33+
useValidateUserPermissionsNonSuspense: jest.fn().mockReturnValue({
34+
data: [{ scope: 'lib:test', allowed: true }],
35+
isLoading: false,
36+
}),
3337
}));
3438

3539
// Mock the useRevokeUserRoles hook

src/authz-module/audit-user/index.tsx

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,16 @@ import {
55
import { useIntl } from '@edx/frontend-platform/i18n';
66
import { AppContext } from '@edx/frontend-platform/react';
77
import type { AppContextType } from '@edx/frontend-platform/react';
8-
import debounce from 'lodash.debounce';
98
import {
109
Container, DataTable,
1110
} from '@openedx/paragon';
1211
import TableFooter from '@src/authz-module/components/TableFooter/TableFooter';
13-
import { AUTHZ_HOME_PATH, TABLE_DEFAULT_PAGE_SIZE } from '@src/authz-module/constants';
12+
import {
13+
AUTHZ_HOME_PATH, TABLE_DEFAULT_PAGE_SIZE,
14+
} from '@src/authz-module/constants';
1415
import AuthZLayout from '@src/authz-module/components/AuthZLayout';
1516
import { useNavigate, useParams } from 'react-router-dom';
16-
import { useUserAccount } from '@src/data/hooks';
17+
import { useUserAccount, useValidateUserPermissionsNonSuspense } from '@src/data/hooks';
1718
import baseMessages from '@src/authz-module/messages';
1819
import AddRoleButton from '@src/authz-module/components/AddRoleButton';
1920
import {
@@ -30,7 +31,7 @@ import RolesFilter from '@src/authz-module/components/TableControlBar/RolesFilte
3031
import TableControlBar from '@src/authz-module/components/TableControlBar/TableControlBar';
3132
import messages from './messages';
3233
import ConfirmDeletionModal from '../components/ConfirmDeletionModal';
33-
import { getCellHeader } from '../components/utils';
34+
import { getCellHeader, getScopeManageActionPermission } from '../utils';
3435

3536
const AuditUserPage = () => {
3637
const { formatMessage } = useIntl();
@@ -52,7 +53,30 @@ const AuditUserPage = () => {
5253
} = useToastManager();
5354
const { mutate: revokeUserRoles, isPending: isRevokingUserRolePending } = useRevokeUserRoles();
5455

55-
const fetchData = useMemo(() => debounce(handleTableFetch, 500), [handleTableFetch]);
56+
const deletePermissions = useMemo(() => {
57+
const uniqueScopes = [...new Set(userAssignments.map(assignment => assignment.scope))];
58+
return uniqueScopes.map(scope => getScopeManageActionPermission(scope));
59+
}, [userAssignments]);
60+
61+
const {
62+
data: permissionsToManageScope,
63+
} = useValidateUserPermissionsNonSuspense(deletePermissions);
64+
65+
const rowsWithPermissions = useMemo(() => {
66+
if (!permissionsToManageScope) { return userAssignments; }
67+
68+
return userAssignments.map(assignment => {
69+
const canManageScope = permissionsToManageScope.some(
70+
permission => permission.scope === assignment.scope && permission.allowed,
71+
);
72+
return {
73+
...assignment,
74+
canManageScope,
75+
};
76+
});
77+
}, [userAssignments, permissionsToManageScope]);
78+
79+
const fetchData = useMemo(() => handleTableFetch, [handleTableFetch]);
5680

5781
useEffect(() => {
5882
if (!user && !isLoadingUser) {
@@ -63,8 +87,6 @@ const AuditUserPage = () => {
6387
}
6488
}, [user, isLoadingUser, navigate, isErrorUser, errorUser]);
6589

66-
useEffect(() => () => fetchData.cancel(), [fetchData]);
67-
6890
const handleShowConfirmDeletionModal = useCallback((role: RoleToDelete) => {
6991
if (isRevokingUserRolePending) { return; }
7092

@@ -227,7 +249,7 @@ const AuditUserPage = () => {
227249
isFilterable
228250
isSortable
229251
manualPagination
230-
data={userAssignments}
252+
data={rowsWithPermissions}
231253
manualFilters
232254
manualSortBy
233255
fetchData={fetchData}

src/authz-module/components/TableCells.test.tsx

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@ import {
1414
createActionsCell,
1515
} from './TableCells';
1616

17-
// TODO: remove console.log mocks and implement actual logic for these cells, then update tests accordingly
18-
// Mock console.log for TODO functions
19-
jest.spyOn(console, 'log').mockImplementation(() => {});
20-
2117
const mockNavigate = jest.fn();
2218

2319
jest.mock('react-router-dom', () => ({
@@ -490,6 +486,7 @@ describe('TableCells Components', () => {
490486
org: 'Test Org',
491487
scope: 'Test Scope',
492488
permissionCount: 1,
489+
canManageScope: true,
493490
},
494491
};
495492

@@ -512,7 +509,26 @@ describe('TableCells Components', () => {
512509
expect(mockOnClickDeleteButton).toHaveBeenCalledWith({ name: 'Library Admin', role: 'library_admin', scope: 'Test Scope' });
513510
});
514511

515-
it('renders a disabled button for admin roles when isUserAuthenticatedPage is true', () => {
512+
it('renders a disabled delete icon for admin roles when isUserAuthenticatedPage is true', () => {
513+
const adminRow = {
514+
original: {
515+
role: 'course_admin',
516+
org: 'Test Org',
517+
scope: 'Test Scope',
518+
permissionCount: 1,
519+
},
520+
};
521+
const CustomActionsCell = createActionsCell({
522+
onClickDeleteButton: mockOnClickDeleteButton,
523+
isUserAuthenticatedPage: true,
524+
});
525+
renderWrapper(<CustomActionsCell row={adminRow} column={{ id: 'actions' }} />);
526+
527+
const infoIcon = screen.getByRole('img', { hidden: true });
528+
expect(infoIcon).toBeInTheDocument();
529+
});
530+
531+
it('renders a tooltip when hovering over delete icon for admin roles when isUserAuthenticatedPage is true', async () => {
516532
const adminRow = {
517533
original: {
518534
role: 'course_admin',
@@ -521,14 +537,16 @@ describe('TableCells Components', () => {
521537
permissionCount: 1,
522538
},
523539
};
540+
const user = userEvent.setup();
524541
const CustomActionsCell = createActionsCell({
525542
onClickDeleteButton: mockOnClickDeleteButton,
526543
isUserAuthenticatedPage: true,
527544
});
528545
renderWrapper(<CustomActionsCell row={adminRow} column={{ id: 'actions' }} />);
529546

530-
const button = screen.getByRole('button', { name: /delete role action/i });
531-
expect(button).toBeDisabled();
547+
const infoIcon = screen.getByRole('img', { hidden: true });
548+
await user.hover(infoIcon);
549+
expect(screen.getByText(/You cant remove your own admin role/i)).toBeInTheDocument();
532550
});
533551

534552
it('renders info icon with tooltip for Django managed roles', async () => {
@@ -552,6 +570,23 @@ describe('TableCells Components', () => {
552570
await user.hover(infoIcon);
553571
expect(screen.getByText(/Please go to Django Admin to manage it/i)).toBeInTheDocument();
554572
});
573+
574+
it('renders a disabled button when user does not have permission', async () => {
575+
const CustomActionsCell = createActionsCell({
576+
onClickDeleteButton: mockOnClickDeleteButton,
577+
isUserAuthenticatedPage: false,
578+
});
579+
const customRow = {
580+
original: {
581+
...baseRow,
582+
canManageScope: false,
583+
},
584+
};
585+
renderWrapper(<CustomActionsCell row={customRow} column={{ id: 'actions' }} />);
586+
587+
const deleteButton = screen.queryByRole('button', { name: /delete role action/i });
588+
expect(deleteButton).toBeDisabled();
589+
});
555590
});
556591

557592
describe('ViewAllPermissionsCell', () => {

src/authz-module/components/TableCells.tsx

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ import {
66
Info,
77
} from '@openedx/paragon/icons';
88
import {
9-
TableCellValue, AppContextType, UserRole, RoleToDelete,
9+
TableCellValue, AppContextType, UserRoleWithPermissions, RoleToDelete,
1010
} from '@src/types';
1111
import { useNavigate } from 'react-router-dom';
1212
import { useContext, useMemo } from 'react';
13-
import { ADMIN_ROLES, DJANGO_MANAGED_ROLES, MAP_ROLE_KEY_TO_LABEL } from '@src/authz-module/constants';
13+
import {
14+
ADMIN_ROLES, DJANGO_MANAGED_ROLES, MAP_ROLE_KEY_TO_LABEL,
15+
} from '@src/authz-module/constants';
1416
import {
1517
Icon, IconButton, OverlayTrigger, Tooltip, DataTableContext,
1618
} from '@openedx/paragon';
@@ -25,7 +27,7 @@ interface DataTableInstance {
2527
toggleRowExpanded?: (rowId: string, expanded: boolean) => void;
2628
}
2729

28-
type CellProps = TableCellValue<UserRole>;
30+
type CellProps = TableCellValue<UserRoleWithPermissions>;
2931
type CellPropsWithValue = CellProps & {
3032
value: string;
3133
};
@@ -160,9 +162,11 @@ const ViewAllPermissionsCell = ({ row }: CellProps) => {
160162
);
161163
};
162164

163-
const ActionsCell = ({ row, onClickDeleteButton, isUserAuthenticatedPage }: ActionsCellProps) => {
165+
const ActionsCell = ({
166+
row, onClickDeleteButton, isUserAuthenticatedPage,
167+
}: ActionsCellProps) => {
164168
const { formatMessage } = useIntl();
165-
const { role } = row.original;
169+
const { role, canManageScope } = row.original;
166170

167171
const handleDelete = () => {
168172
const roleToDelete = {
@@ -193,19 +197,30 @@ const ActionsCell = ({ row, onClickDeleteButton, isUserAuthenticatedPage }: Acti
193197

194198
if (ADMIN_ROLES.includes(role) && isUserAuthenticatedPage) {
195199
return (
196-
<IconButton
197-
// @ts-ignore
198-
disabled
199-
isActive={false}
200-
variant="light"
201-
alt={formatMessage(messages['authz.user.table.delete.action.alt'])}
202-
src={Delete}
203-
/>
200+
<OverlayTrigger
201+
placement="left"
202+
overlay={(
203+
<Tooltip variant="light" id="tooltip-left">
204+
{formatMessage(messages['authz.user.table.delete.action.adminrole.tooltip'])}
205+
</Tooltip>
206+
)}
207+
>
208+
<Icon
209+
className="mx-2 pl-1 text-light-500"
210+
src={Delete}
211+
/>
212+
</OverlayTrigger>
204213
);
205214
}
206215

207216
return (
208-
<IconButton variant="danger" onClick={handleDelete} alt={formatMessage(messages['authz.user.table.delete.action.alt'])} src={Delete} />
217+
<IconButton
218+
disabled={!canManageScope}
219+
variant={canManageScope ? 'danger' : 'light'}
220+
onClick={handleDelete}
221+
alt={formatMessage(messages['authz.user.table.delete.action.alt'])}
222+
src={Delete}
223+
/>
209224
);
210225
};
211226

src/authz-module/components/messages.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,11 @@ const messages = defineMessages({
157157
defaultMessage: 'You can’t remove this role here. Please go to Django Admin to manage it.',
158158
description: 'Tooltip for delete button when hovering over Django roles',
159159
},
160+
'authz.user.table.delete.action.adminrole.tooltip': {
161+
id: 'authz.user.table.delete.action.adminrole.tooltip',
162+
defaultMessage: 'You can’t remove your own admin role. This prevents a resource from being left without an admin. Another user with the required permissions can revoke it.',
163+
description: 'Tooltip for delete button when hovering over Admin roles',
164+
},
160165
'authz.user.table.view_all_permissions.link.text.close': {
161166
id: 'authz.user.table.view_all_permissions.link.text.close',
162167
defaultMessage: 'Hide all permissions',

src/authz-module/components/utils.test.tsx

Lines changed: 0 additions & 84 deletions
This file was deleted.

src/authz-module/components/utils.tsx

Lines changed: 0 additions & 14 deletions
This file was deleted.

0 commit comments

Comments
 (0)