Skip to content

Commit bea844b

Browse files
fix: adding missing tooltip to delete button and fixing race condition
1 parent df73f14 commit bea844b

6 files changed

Lines changed: 80 additions & 35 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ jest.mock('@src/data/hooks', () => ({
3131
...jest.requireActual('@src/data/hooks'),
3232
useUserAccount: jest.fn(),
3333
useValidateUserPermissions: jest.fn().mockReturnValue({
34-
data: [{ allowed: true }],
34+
data: [{ scope: 'lib:test', allowed: true }],
3535
isLoading: false,
3636
}),
3737
}));

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

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,23 @@ const AuditUserPage = () => {
5959
return uniqueScopes.map(scope => getScopeManageActionPermission(scope));
6060
}, [userAssignments]);
6161

62-
const { data: permissionsToManageScope } = useValidateUserPermissions(deletePermissions);
62+
const {
63+
data: permissionsToManageScope,
64+
} = useValidateUserPermissions(deletePermissions);
65+
66+
const rowsWithPermissions = useMemo(() => {
67+
if (!permissionsToManageScope) { return userAssignments; }
68+
69+
return userAssignments.map(assignment => {
70+
const canManageScope = permissionsToManageScope.some(
71+
permission => permission.scope === assignment.scope && permission.allowed,
72+
);
73+
return {
74+
...assignment,
75+
canManageScope,
76+
};
77+
});
78+
}, [userAssignments, permissionsToManageScope]);
6379

6480
const fetchData = useMemo(() => debounce(handleTableFetch, 500), [handleTableFetch]);
6581

@@ -81,11 +97,6 @@ const AuditUserPage = () => {
8197
setShowConfirmDeletionModal(true);
8298
}, [isRevokingUserRolePending]);
8399

84-
const hasPermissionToDeleteScope = useCallback((scope: string) => {
85-
const permissionIndex = deletePermissions.findIndex(permission => permission.scope === scope);
86-
return permissionsToManageScope?.[permissionIndex]?.allowed;
87-
}, [deletePermissions, permissionsToManageScope]);
88-
89100
const navLinks = useMemo(() => [
90101
{
91102
label: formatMessage(baseMessages['authz.management.home.nav.link']),
@@ -105,10 +116,9 @@ const AuditUserPage = () => {
105116
Cell: createActionsCell({
106117
onClickDeleteButton: handleShowConfirmDeletionModal,
107118
isUserAuthenticatedPage: username === authenticatedUser.username,
108-
hasPermissionToDeleteScope,
109119
}),
110120
},
111-
], [authenticatedUser.username, formatMessage, handleShowConfirmDeletionModal, hasPermissionToDeleteScope, username]);
121+
], [authenticatedUser.username, formatMessage, handleShowConfirmDeletionModal, username]);
112122

113123
const columns = useMemo(() => [
114124
{
@@ -242,7 +252,7 @@ const AuditUserPage = () => {
242252
isFilterable
243253
isSortable
244254
manualPagination
245-
data={userAssignments}
255+
data={rowsWithPermissions}
246256
manualFilters
247257
manualSortBy
248258
fetchData={fetchData}

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

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ describe('TableCells Components', () => {
486486
org: 'Test Org',
487487
scope: 'Test Scope',
488488
permissionCount: 1,
489+
canManageScope: true,
489490
},
490491
};
491492

@@ -497,7 +498,6 @@ describe('TableCells Components', () => {
497498
const user = userEvent.setup();
498499
const CustomActionsCell = createActionsCell({
499500
onClickDeleteButton: mockOnClickDeleteButton,
500-
hasPermissionToDeleteScope: () => true,
501501
isUserAuthenticatedPage: false,
502502
});
503503
renderWrapper(<CustomActionsCell row={baseRow} column={{ id: 'actions' }} />);
@@ -509,7 +509,7 @@ describe('TableCells Components', () => {
509509
expect(mockOnClickDeleteButton).toHaveBeenCalledWith({ name: 'Library Admin', role: 'library_admin', scope: 'Test Scope' });
510510
});
511511

512-
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', () => {
513513
const adminRow = {
514514
original: {
515515
role: 'course_admin',
@@ -521,12 +521,32 @@ describe('TableCells Components', () => {
521521
const CustomActionsCell = createActionsCell({
522522
onClickDeleteButton: mockOnClickDeleteButton,
523523
isUserAuthenticatedPage: true,
524-
hasPermissionToDeleteScope: () => true,
525524
});
526525
renderWrapper(<CustomActionsCell row={adminRow} column={{ id: 'actions' }} />);
527526

528-
const button = screen.getByRole('button', { name: /delete role action/i });
529-
expect(button).toBeDisabled();
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 () => {
532+
const adminRow = {
533+
original: {
534+
role: 'course_admin',
535+
org: 'Test Org',
536+
scope: 'Test Scope',
537+
permissionCount: 1,
538+
},
539+
};
540+
const user = userEvent.setup();
541+
const CustomActionsCell = createActionsCell({
542+
onClickDeleteButton: mockOnClickDeleteButton,
543+
isUserAuthenticatedPage: true,
544+
});
545+
renderWrapper(<CustomActionsCell row={adminRow} column={{ id: 'actions' }} />);
546+
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();
530550
});
531551

532552
it('renders info icon with tooltip for Django managed roles', async () => {
@@ -541,7 +561,6 @@ describe('TableCells Components', () => {
541561
const user = userEvent.setup();
542562
const CustomActionsCell = createActionsCell({
543563
onClickDeleteButton: mockOnClickDeleteButton,
544-
hasPermissionToDeleteScope: () => true,
545564
isUserAuthenticatedPage: true,
546565
});
547566
renderWrapper(<CustomActionsCell row={djangoRow} column={{ id: 'actions' }} />);
@@ -556,9 +575,14 @@ describe('TableCells Components', () => {
556575
const CustomActionsCell = createActionsCell({
557576
onClickDeleteButton: mockOnClickDeleteButton,
558577
isUserAuthenticatedPage: false,
559-
hasPermissionToDeleteScope: () => false,
560578
});
561-
renderWrapper(<CustomActionsCell row={baseRow} column={{ id: 'actions' }} />);
579+
const customRow = {
580+
original: {
581+
...baseRow,
582+
canManageScope: false,
583+
},
584+
};
585+
renderWrapper(<CustomActionsCell row={customRow} column={{ id: 'actions' }} />);
562586

563587
const deleteButton = screen.queryByRole('button', { name: /delete role action/i });
564588
expect(deleteButton).toBeDisabled();

src/authz-module/components/TableCells.tsx

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ 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';
@@ -27,7 +27,7 @@ interface DataTableInstance {
2727
toggleRowExpanded?: (rowId: string, expanded: boolean) => void;
2828
}
2929

30-
type CellProps = TableCellValue<UserRole>;
30+
type CellProps = TableCellValue<UserRoleWithPermissions>;
3131
type CellPropsWithValue = CellProps & {
3232
value: string;
3333
};
@@ -39,7 +39,6 @@ type ExtendedCellProps = CellPropsWithValue & {
3939

4040
type ActionsCellExtraProps = {
4141
onClickDeleteButton: (role: RoleToDelete) => void;
42-
hasPermissionToDeleteScope: (scope: string) => boolean;
4342
isUserAuthenticatedPage: boolean;
4443
};
4544

@@ -164,12 +163,10 @@ const ViewAllPermissionsCell = ({ row }: CellProps) => {
164163
};
165164

166165
const ActionsCell = ({
167-
row, onClickDeleteButton, isUserAuthenticatedPage, hasPermissionToDeleteScope,
166+
row, onClickDeleteButton, isUserAuthenticatedPage,
168167
}: ActionsCellProps) => {
169168
const { formatMessage } = useIntl();
170-
const { role, scope } = row.original;
171-
172-
const hasPermissionsToDelete = useMemo(() => hasPermissionToDeleteScope(scope), [hasPermissionToDeleteScope, scope]);
169+
const { role, canManageScope } = row.original;
173170

174171
const handleDelete = () => {
175172
const roleToDelete = {
@@ -200,21 +197,26 @@ const ActionsCell = ({
200197

201198
if (ADMIN_ROLES.includes(role) && isUserAuthenticatedPage) {
202199
return (
203-
<IconButton
204-
// @ts-ignore
205-
disabled
206-
isActive={false}
207-
variant="light"
208-
alt={formatMessage(messages['authz.user.table.delete.action.alt'])}
209-
src={Delete}
210-
/>
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>
211213
);
212214
}
213215

214216
return (
215217
<IconButton
216-
disabled={!hasPermissionsToDelete}
217-
variant={hasPermissionsToDelete ? 'danger' : 'light'}
218+
disabled={!canManageScope}
219+
variant={canManageScope ? 'danger' : 'light'}
218220
onClick={handleDelete}
219221
alt={formatMessage(messages['authz.user.table.delete.action.alt'])}
220222
src={Delete}

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/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,7 @@ export type RoleToDelete = {
128128
name?: string;
129129
scope: string;
130130
};
131+
132+
export type UserRoleWithPermissions = UserRole & {
133+
canManageScope?: boolean;
134+
};

0 commit comments

Comments
 (0)