From a8deab2c2d3fdab188623fd38c7c1a41bf132bf2 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 9 Oct 2025 15:35:27 -0500 Subject: [PATCH 01/12] feat: add revokeUserRoles API and corresponding hook --- src/authz-module/data/api.ts | 30 ++++++++++++++++++++++++++++++ src/authz-module/data/hooks.ts | 22 +++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/authz-module/data/api.ts b/src/authz-module/data/api.ts index 0c59453..6fe08a8 100644 --- a/src/authz-module/data/api.ts +++ b/src/authz-module/data/api.ts @@ -17,6 +17,23 @@ export interface GetTeamMembersResponse { count: number; } +export type RevokeUserRolesRequest = { + users: string; + role: string; + scope: string; +}; + +export interface DeleteRevokeUserRolesResponse { + completed: { + userIdentifiers: string; + status: string; + }[], + errors: { + userIdentifiers: string; + error: string; + }[], +} + export type PermissionsByRole = { role: string; permissions: string[]; @@ -77,3 +94,16 @@ export const getPermissionsByRole = async (scope: string): Promise => { + const url = new URL(getApiUrl('/api/authz/v1/roles/users/')); + url.searchParams.append('users', data.users); + url.searchParams.append('role', data.role); + url.searchParams.append('scope', data.scope); + + // If this is not transformed to string, it shows a 404 with the token CSRF acquisition request + const res = await getAuthenticatedHttpClient().delete(url.toString()); + return camelCaseObject(res.data); +}; diff --git a/src/authz-module/data/hooks.ts b/src/authz-module/data/hooks.ts index e4ced66..d4860d5 100644 --- a/src/authz-module/data/hooks.ts +++ b/src/authz-module/data/hooks.ts @@ -5,7 +5,7 @@ import { appId } from '@src/constants'; import { LibraryMetadata } from '@src/types'; import { assignTeamMembersRole, AssignTeamMembersRoleRequest, getLibrary, getPermissionsByRole, getTeamMembers, - GetTeamMembersResponse, PermissionsByRole, QuerySettings, + GetTeamMembersResponse, PermissionsByRole, QuerySettings, revokeUserRoles, RevokeUserRolesRequest, } from './api'; const authzQueryKeys = { @@ -87,3 +87,23 @@ export const useAssignTeamMembersRole = () => { }, }); }; + + +/** + * React Query hook to remove roles for a specific team member within a scope. + * + * @example + * const { mutate: revokeUserRoles } = useRevokeUserRoles(); + * revokeUserRoles({ data: { libraryId: 'lib:123', users: ['jdoe'], role: 'editor' } }); + */ +export const useRevokeUserRoles = () => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async ({ data }: { + data: RevokeUserRolesRequest + }) => revokeUserRoles(data), + onSettled: (_data, _error, { data: { scope } }) => { + queryClient.invalidateQueries({ queryKey: authzQueryKeys.teamMembers(scope) }); + }, + }); +}; From 5ec5d6f9f3af94e490f60422b4a8a678a1b99b33 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 9 Oct 2025 15:40:40 -0500 Subject: [PATCH 02/12] feat: update RoleCard to use handleDelete prop and integrate revokeUserRoles functionality --- .../components/RoleCard/index.tsx | 8 +++-- .../LibrariesUserManager.tsx | 33 ++++++++++++++++--- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/authz-module/components/RoleCard/index.tsx b/src/authz-module/components/RoleCard/index.tsx index 350aed6..9bb5add 100644 --- a/src/authz-module/components/RoleCard/index.tsx +++ b/src/authz-module/components/RoleCard/index.tsx @@ -14,7 +14,7 @@ interface CardTitleProps { interface RoleCardProps extends CardTitleProps { objectName?: string | null; description: string; - showDelete?: boolean; + handleDelete?: () => void; permissionsByResource: any[]; } @@ -31,7 +31,7 @@ const CardTitle = ({ title, userCounter = null }: CardTitleProps) => ( ); const RoleCard = ({ - title, objectName, description, showDelete, permissionsByResource, userCounter, + title, objectName, description, handleDelete, permissionsByResource, userCounter, }: RoleCardProps) => { const intl = useIntl(); @@ -41,7 +41,9 @@ const RoleCard = ({ title={} subtitle={(objectName && {objectName}) || ''} actions={ - showDelete && + handleDelete && ( + + ) } /> diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.tsx index e916f8e..d22a483 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.tsx @@ -1,5 +1,5 @@ -import { useMemo } from 'react'; -import { useParams } from 'react-router-dom'; +import { useEffect, useMemo } from 'react'; +import { useNavigate, useParams } from 'react-router-dom'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Container, Skeleton } from '@openedx/paragon'; import { ROUTES } from '@src/authz-module/constants'; @@ -7,18 +7,20 @@ import AuthZLayout from '../components/AuthZLayout'; import { useLibraryAuthZ } from './context'; import RoleCard from '../components/RoleCard'; import { AssignNewRoleTrigger } from './components/AssignNewRoleModal'; -import { useLibrary, useTeamMembers } from '../data/hooks'; +import { useLibrary, useRevokeUserRoles, useTeamMembers } from '../data/hooks'; import { buildPermissionMatrixByRole } from './utils'; import messages from './messages'; const LibrariesUserManager = () => { const intl = useIntl(); + const navigate = useNavigate(); const { username } = useParams(); const { libraryId, permissions, roles, resources, canManageTeam, } = useLibraryAuthZ(); const { data: library } = useLibrary(libraryId); + const { mutate: revokeUserRoles } = useRevokeUserRoles(); const rootBreadcrumb = intl.formatMessage(messages['library.authz.breadcrumb.root']) || ''; const pageManageTitle = intl.formatMessage(messages['library.authz.manage.page.title']); const querySettings = { @@ -33,6 +35,8 @@ const LibrariesUserManager = () => { const { data: teamMember, isLoading: isLoadingTeamMember } = useTeamMembers(libraryId, querySettings); const user = teamMember?.results?.find(member => member.username === username); + const teamMembersPath = `/authz/${ROUTES.LIBRARIES_TEAM_PATH.replace(':libraryId', libraryId)}`; + const userRoles = useMemo(() => { const assignedRoles = roles.filter(role => user?.roles.includes(role.role)); return buildPermissionMatrixByRole({ @@ -40,11 +44,30 @@ const LibrariesUserManager = () => { }); }, [roles, user?.roles, permissions, resources, intl]); + const handleRevokeUserRole = (role: string) => { + if (user) { + const data = { + users: user.username, + role, + scope: libraryId, + }; + + revokeUserRoles({ data }); + } + }; + + useEffect(() => { + if (!isLoadingTeamMember && !userRoles.length) { + navigate(teamMembersPath); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [userRoles, libraryId]); + return (
{user?.email}

} @@ -64,7 +87,7 @@ const LibrariesUserManager = () => { title={role.name} objectName={library.title} description={role.description} - showDelete + handleDelete={() => handleRevokeUserRole(role.role)} permissionsByResource={role.resources as any[]} /> ))} From b0051229c8b2519badde0b67c54e21b2b1d70c2b Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 9 Oct 2025 16:48:32 -0500 Subject: [PATCH 03/12] feat: add ConfirmDeletionModal for role removal confirmation in LibrariesUserManager --- .../LibrariesUserManager.tsx | 42 ++++++++-- .../components/ConfirmDeletionModal.tsx | 84 +++++++++++++++++++ .../libraries-manager/components/messages.ts | 19 +++++ 3 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 src/authz-module/libraries-manager/components/ConfirmDeletionModal.tsx diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.tsx index d22a483..759a3b7 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.tsx @@ -1,8 +1,9 @@ -import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { useNavigate, useParams } from 'react-router-dom'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Container, Skeleton } from '@openedx/paragon'; import { ROUTES } from '@src/authz-module/constants'; +import { Role } from 'types'; import AuthZLayout from '../components/AuthZLayout'; import { useLibraryAuthZ } from './context'; import RoleCard from '../components/RoleCard'; @@ -11,6 +12,7 @@ import { useLibrary, useRevokeUserRoles, useTeamMembers } from '../data/hooks'; import { buildPermissionMatrixByRole } from './utils'; import messages from './messages'; +import ConfirmDeletionModal from './components/ConfirmDeletionModal'; const LibrariesUserManager = () => { const intl = useIntl(); @@ -20,7 +22,7 @@ const LibrariesUserManager = () => { libraryId, permissions, roles, resources, canManageTeam, } = useLibraryAuthZ(); const { data: library } = useLibrary(libraryId); - const { mutate: revokeUserRoles } = useRevokeUserRoles(); + const { mutate: revokeUserRoles, isPending: isRevokingUserRole } = useRevokeUserRoles(); const rootBreadcrumb = intl.formatMessage(messages['library.authz.breadcrumb.root']) || ''; const pageManageTitle = intl.formatMessage(messages['library.authz.manage.page.title']); const querySettings = { @@ -32,6 +34,9 @@ const LibrariesUserManager = () => { sortBy: null, }; + const [roleToDelete, setRoleToDelete] = useState(''); + const [showConfirmDeletionModal, setShowConfirmDeletionModal] = useState(false); + const { data: teamMember, isLoading: isLoadingTeamMember } = useTeamMembers(libraryId, querySettings); const user = teamMember?.results?.find(member => member.username === username); @@ -44,15 +49,29 @@ const LibrariesUserManager = () => { }); }, [roles, user?.roles, permissions, resources, intl]); + const handleCloseConfirmDeletionModal = () => { + setShowConfirmDeletionModal(false); + setRoleToDelete(''); + }; + + const handleShowConfirmDeletionModal = (role: Pick) => { + setRoleToDelete(role.name); + setShowConfirmDeletionModal(true); + }; + const handleRevokeUserRole = (role: string) => { if (user) { const data = { users: user.username, - role, + role: roles.find(r => r.name === role)!.role, scope: libraryId, }; - revokeUserRoles({ data }); + revokeUserRoles({ data }, { + onSuccess: () => { + handleCloseConfirmDeletionModal(); + }, + }); } }; @@ -65,6 +84,19 @@ const LibrariesUserManager = () => { return (
+ handleRevokeUserRole(roleToDelete)} + isDeleting={isRevokingUserRole} + context={{ + userName: user?.username || '', + scope: library.title, + role: roleToDelete, + rolesCount: userRoles.length, + }} + /> + { title={role.name} objectName={library.title} description={role.description} - handleDelete={() => handleRevokeUserRole(role.role)} + handleDelete={() => handleShowConfirmDeletionModal(role)} permissionsByResource={role.resources as any[]} /> ))} diff --git a/src/authz-module/libraries-manager/components/ConfirmDeletionModal.tsx b/src/authz-module/libraries-manager/components/ConfirmDeletionModal.tsx new file mode 100644 index 0000000..b49ee35 --- /dev/null +++ b/src/authz-module/libraries-manager/components/ConfirmDeletionModal.tsx @@ -0,0 +1,84 @@ +import { FC } from 'react'; +import { + ActionRow, Icon, ModalDialog, Stack, + StatefulButton, +} from '@openedx/paragon'; +import { useIntl } from '@edx/frontend-platform/i18n'; + +import { SpinnerSimple } from '@openedx/paragon/icons'; +import messages from './messages'; + +interface ConfirmDeletionModalProps { + isOpen: boolean; + close: () => void; + onSave: () => void; + isDeleting?: boolean; + context: { + userName: string; + scope: string; + role: string; + rolesCount: number; + } +} + +const ConfirmDeletionModal: FC = ({ + isOpen, close, onSave, isDeleting, context, +}) => { + const intl = useIntl(); + return ( + + + + {intl.formatMessage(messages['library.authz.team.remove.user.modal.title'])} + + + + + +

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.1'], { + userName: context.userName, + scope: context.scope, + role: context.role, + })} +

+ {context.rolesCount === 1 && ( +

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.2'])}

+ )} +

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.3'])}

+
+
+ + + + + {intl.formatMessage(messages['libraries.authz.manage.cancel.button'])} + + , + }} + state={isDeleting ? 'pending' : 'default'} + onClick={() => onSave()} + disabledStates={['pending']} + /> + + +
+ ); +}; + +export default ConfirmDeletionModal; diff --git a/src/authz-module/libraries-manager/components/messages.ts b/src/authz-module/libraries-manager/components/messages.ts index d73643a..7ff1e84 100644 --- a/src/authz-module/libraries-manager/components/messages.ts +++ b/src/authz-module/libraries-manager/components/messages.ts @@ -5,6 +5,25 @@ const messages = defineMessages({ id: 'libraries.authz.manage.assign.new.role.title', defaultMessage: 'Add New Role', description: 'Libraries AuthZ assign a new role to a user button title', + 'library.authz.team.remove.user.modal.title': { + id: 'library.authz.team.remove.user.modal.title', + defaultMessage: 'Remove role?', + description: 'Libraries team management remove user modal title', + }, + 'library.authz.team.remove.user.modal.body.1': { + id: 'library.authz.team.remove.user.modal.body', + defaultMessage: 'Are you sure you want to remove the {role} role from the user “{userName}” in the library {scope}?', + description: 'Libraries team management remove user modal body', + }, + 'library.authz.team.remove.user.modal.body.2': { + id: 'library.authz.team.remove.user.modal.body', + defaultMessage: "This is the user's only role in this library. Removing it will revoke their access completely, and they will no longer appear in the library's member List.", + description: 'Libraries team management remove user modal body', + }, + 'library.authz.team.remove.user.modal.body.3': { + id: 'library.authz.team.remove.user.modal.body', + defaultMessage: 'Are you sure you want to proceed?', + description: 'Libraries team management remove user modal body', }, 'library.authz.manage.role.select.label': { id: 'library.authz.role.select.label', From 47f1170feb344f3ea4508c28932d30b15450cad7 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 10 Oct 2025 14:56:20 -0500 Subject: [PATCH 04/12] feat: implement ToastManager for user role removal notifications --- src/authz-module/index.scss | 3 +- src/authz-module/index.tsx | 20 ++++--- .../LibrariesUserManager.tsx | 9 ++- .../libraries-manager/ToastManagerContext.tsx | 59 +++++++++++++++++++ .../libraries-manager/messages.ts | 5 ++ 5 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 src/authz-module/libraries-manager/ToastManagerContext.tsx diff --git a/src/authz-module/index.scss b/src/authz-module/index.scss index 907cd6c..96eb1a6 100644 --- a/src/authz-module/index.scss +++ b/src/authz-module/index.scss @@ -42,11 +42,10 @@ } } - .toast-container { // Ensure toast appears above modal z-index: 1000; // Move toast to the right left: auto; right: var(--pgn-spacing-toast-container-gutter-lg); -} \ No newline at end of file +} diff --git a/src/authz-module/index.tsx b/src/authz-module/index.tsx index d6e4218..0a4e130 100644 --- a/src/authz-module/index.tsx +++ b/src/authz-module/index.tsx @@ -4,6 +4,7 @@ import { ErrorBoundary } from 'react-error-boundary'; import { QueryErrorResetBoundary } from '@tanstack/react-query'; import LoadingPage from '@src/components/LoadingPage'; import LibrariesErrorFallback from '@src/authz-module/libraries-manager/ErrorPage'; +import { ToastManagerProvider } from './libraries-manager/ToastManagerContext'; import { LibrariesTeamManager, LibrariesUserManager, LibrariesLayout } from './libraries-manager'; import { ROUTES } from './constants'; @@ -13,14 +14,17 @@ const AuthZModule = () => ( {({ reset }) => ( - }> - - }> - } /> - } /> - - - + + }> + + }> + } /> + } /> + + + + + )} diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.tsx index 759a3b7..2bd205f 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.tsx @@ -4,6 +4,7 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { Container, Skeleton } from '@openedx/paragon'; import { ROUTES } from '@src/authz-module/constants'; import { Role } from 'types'; +import { useToastManager } from 'authz-module/libraries-manager/ToastManagerContext'; import AuthZLayout from '../components/AuthZLayout'; import { useLibraryAuthZ } from './context'; import RoleCard from '../components/RoleCard'; @@ -36,6 +37,7 @@ const LibrariesUserManager = () => { const [roleToDelete, setRoleToDelete] = useState(''); const [showConfirmDeletionModal, setShowConfirmDeletionModal] = useState(false); + const { handleShowToast, handleDiscardToast } = useToastManager(); const { data: teamMember, isLoading: isLoadingTeamMember } = useTeamMembers(libraryId, querySettings); const user = teamMember?.results?.find(member => member.username === username); @@ -50,11 +52,12 @@ const LibrariesUserManager = () => { }, [roles, user?.roles, permissions, resources, intl]); const handleCloseConfirmDeletionModal = () => { - setShowConfirmDeletionModal(false); setRoleToDelete(''); + setShowConfirmDeletionModal(false); }; const handleShowConfirmDeletionModal = (role: Pick) => { + handleDiscardToast(); setRoleToDelete(role.name); setShowConfirmDeletionModal(true); }; @@ -69,6 +72,10 @@ const LibrariesUserManager = () => { revokeUserRoles({ data }, { onSuccess: () => { + handleShowToast(intl.formatMessage(messages['library.authz.team.remove.user.toast.success.description'], { + role, + rolesCount: userRoles.length - 1, + })); handleCloseConfirmDeletionModal(); }, }); diff --git a/src/authz-module/libraries-manager/ToastManagerContext.tsx b/src/authz-module/libraries-manager/ToastManagerContext.tsx new file mode 100644 index 0000000..8f4319b --- /dev/null +++ b/src/authz-module/libraries-manager/ToastManagerContext.tsx @@ -0,0 +1,59 @@ +import { + createContext, useContext, useMemo, useState, +} from 'react'; +import { Toast } from '@openedx/paragon'; + +type ToastManagerContextType = { + handleShowToast: (message: string) => void; + handleDiscardToast: () => void; +}; + +const ToastManagerContext = createContext(undefined); + +interface ToastManagerProviderProps { + handleClose?: () => void + children: React.ReactNode | React.ReactNode[]; +} + +export const ToastManagerProvider = ({ handleClose, children }: ToastManagerProviderProps) => { + const [toastMessage, setToastMessage] = useState(null); + + const handleShowToast = (message: string) => { + setToastMessage(message); + }; + + const handleDiscardToast = () => { + setToastMessage(null); + }; + + const value = useMemo((): ToastManagerContextType => ({ + handleShowToast, + handleDiscardToast, + }), []); + + return ( + + {children} + + {toastMessage && ( + { + if (handleClose) { handleClose(); } + setToastMessage(null); + }} + show={!!toastMessage} + > + {toastMessage} + + )} + + ); +}; + +export const useToastManager = (): ToastManagerContextType => { + const context = useContext(ToastManagerContext); + if (context === undefined) { + throw new Error('useToastManager must be used within an ToastManagerProvider'); + } + return context; +}; diff --git a/src/authz-module/libraries-manager/messages.ts b/src/authz-module/libraries-manager/messages.ts index acba544..fcab7e5 100644 --- a/src/authz-module/libraries-manager/messages.ts +++ b/src/authz-module/libraries-manager/messages.ts @@ -26,6 +26,11 @@ const messages = defineMessages({ defaultMessage: 'Permissions', description: 'Libreries AuthZ title for the permissions tab', }, + 'library.authz.team.remove.user.toast.success.description': { + id: 'library.authz.team.remove.user.toast.success.description', + defaultMessage: 'The {role} role has been successfully removed.{rolesCount, plural, =0 { The user no longer has access to this library and has been removed from the member list.} other {}}', + description: 'Libraries team management remove user toast success', + }, }); export default messages; From e0eb2b6f29586281384570e293db43cd6274f8ab Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 10 Oct 2025 15:11:36 -0500 Subject: [PATCH 05/12] fix: update roleToDelete state type & improve role revocation handling --- .../LibrariesUserManager.tsx | 62 +++++++++++-------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.tsx index 2bd205f..4f39d89 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.tsx @@ -35,7 +35,7 @@ const LibrariesUserManager = () => { sortBy: null, }; - const [roleToDelete, setRoleToDelete] = useState(''); + const [roleToDelete, setRoleToDelete] = useState(null); const [showConfirmDeletionModal, setShowConfirmDeletionModal] = useState(false); const { handleShowToast, handleDiscardToast } = useToastManager(); @@ -52,34 +52,46 @@ const LibrariesUserManager = () => { }, [roles, user?.roles, permissions, resources, intl]); const handleCloseConfirmDeletionModal = () => { - setRoleToDelete(''); + setRoleToDelete(null); setShowConfirmDeletionModal(false); }; - const handleShowConfirmDeletionModal = (role: Pick) => { + const handleShowConfirmDeletionModal = (role: Role) => { + if (isRevokingUserRole) { return; } + handleDiscardToast(); - setRoleToDelete(role.name); + setRoleToDelete(role); setShowConfirmDeletionModal(true); }; - const handleRevokeUserRole = (role: string) => { - if (user) { - const data = { - users: user.username, - role: roles.find(r => r.name === role)!.role, - scope: libraryId, - }; - - revokeUserRoles({ data }, { - onSuccess: () => { - handleShowToast(intl.formatMessage(messages['library.authz.team.remove.user.toast.success.description'], { - role, - rolesCount: userRoles.length - 1, - })); - handleCloseConfirmDeletionModal(); - }, - }); - } + const handleRevokeUserRole = () => { + if (!user || !roleToDelete) { return; } + + const data = { + users: user.username, + role: roleToDelete.role, + scope: libraryId, + }; + + revokeUserRoles({ data }, { + onSuccess: () => { + const remainingRolesCount = userRoles.length - 1; + handleShowToast(intl.formatMessage( + messages['library.authz.team.remove.user.toast.success.description'], + { + role: roleToDelete.name, + rolesCount: remainingRolesCount, + }, + )); + handleCloseConfirmDeletionModal(); + }, + onError: (error) => { + // eslint-disable-next-line no-console + console.error('Failed to revoke user role:', error); + handleCloseConfirmDeletionModal(); + // Could add error toast here if needed + }, + }); }; useEffect(() => { @@ -87,19 +99,19 @@ const LibrariesUserManager = () => { navigate(teamMembersPath); } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [userRoles, libraryId]); + }, [userRoles.length]); return (
handleRevokeUserRole(roleToDelete)} + onSave={handleRevokeUserRole} isDeleting={isRevokingUserRole} context={{ userName: user?.username || '', scope: library.title, - role: roleToDelete, + role: roleToDelete?.name || '', rolesCount: userRoles.length, }} /> From ff9ce8ce62e2a10e0cde9f2a70fc36f9fdb6bee4 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Wed, 22 Oct 2025 15:23:26 +1100 Subject: [PATCH 06/12] feat: use AlertModal and update button labels for role removal --- .../components/ConfirmDeletionModal.tsx | 54 ++++++++----------- .../libraries-manager/components/messages.ts | 17 +++--- 2 files changed, 31 insertions(+), 40 deletions(-) diff --git a/src/authz-module/libraries-manager/components/ConfirmDeletionModal.tsx b/src/authz-module/libraries-manager/components/ConfirmDeletionModal.tsx index b49ee35..2b9066c 100644 --- a/src/authz-module/libraries-manager/components/ConfirmDeletionModal.tsx +++ b/src/authz-module/libraries-manager/components/ConfirmDeletionModal.tsx @@ -1,6 +1,6 @@ import { FC } from 'react'; import { - ActionRow, Icon, ModalDialog, Stack, + ActionRow, AlertModal, Icon, ModalDialog, Stack, StatefulButton, } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; @@ -26,37 +26,12 @@ const ConfirmDeletionModal: FC = ({ }) => { const intl = useIntl(); return ( - - - - {intl.formatMessage(messages['library.authz.team.remove.user.modal.title'])} - - - - - -

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.1'], { - userName: context.userName, - scope: context.scope, - role: context.role, - })} -

- {context.rolesCount === 1 && ( -

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.2'])}

- )} -

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.3'])}

-
-
- - + footerNode={( {intl.formatMessage(messages['libraries.authz.manage.cancel.button'])} @@ -65,8 +40,8 @@ const ConfirmDeletionModal: FC = ({ className="px-4" variant="danger" labels={{ - default: intl.formatMessage(messages['libraries.authz.manage.save.button']), - pending: intl.formatMessage(messages['libraries.authz.manage.saving.button']), + default: intl.formatMessage(messages['libraries.authz.manage.remove.button']), + pending: intl.formatMessage(messages['libraries.authz.manage.removing.button']), }} icons={{ pending: , @@ -76,8 +51,23 @@ const ConfirmDeletionModal: FC = ({ disabledStates={['pending']} /> - -
+ )} + isOverflowVisible={false} + > + +

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.1'], { + userName: context.userName, + scope: context.scope, + role: context.role, + })} +

+ {context.rolesCount === 1 && ( +

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.2'])}

+ )} +

{intl.formatMessage(messages['library.authz.team.remove.user.modal.body.3'])}

+
+ + ); }; diff --git a/src/authz-module/libraries-manager/components/messages.ts b/src/authz-module/libraries-manager/components/messages.ts index 7ff1e84..c9cd927 100644 --- a/src/authz-module/libraries-manager/components/messages.ts +++ b/src/authz-module/libraries-manager/components/messages.ts @@ -5,6 +5,7 @@ const messages = defineMessages({ id: 'libraries.authz.manage.assign.new.role.title', defaultMessage: 'Add New Role', description: 'Libraries AuthZ assign a new role to a user button title', + }, 'library.authz.team.remove.user.modal.title': { id: 'library.authz.team.remove.user.modal.title', defaultMessage: 'Remove role?', @@ -35,15 +36,15 @@ const messages = defineMessages({ defaultMessage: 'Cancel', description: 'Libraries AuthZ cancel button title', }, - 'libraries.authz.manage.saving.button': { - id: 'libraries.authz.manage.saving.button', - defaultMessage: 'Saving...', - description: 'Libraries AuthZ saving button title', + 'libraries.authz.manage.removing.button': { + id: 'libraries.authz.manage.removing.button', + defaultMessage: 'Removing...', + description: 'Libraries AuthZ removing button title', }, - 'libraries.authz.manage.save.button': { - id: 'libraries.authz.manage.save.button', - defaultMessage: 'Save', - description: 'Libraries AuthZ save button title', + 'libraries.authz.manage.remove.button': { + id: 'libraries.authz.manage.remove.button', + defaultMessage: 'Remove', + description: 'Libraries AuthZ remove button title', }, 'libraries.authz.manage.assign.role.success': { id: 'libraries.authz.manage.assign.role.success', From 144999ea4acba4952b84e30d72d0aad74106e576 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Wed, 22 Oct 2025 18:54:44 +1100 Subject: [PATCH 07/12] feat: add tost for error feedback --- .../components/RoleCard/index.test.tsx | 6 ++-- src/authz-module/data/hooks.ts | 1 - .../LibrariesUserManager.test.tsx | 8 ++++- .../LibrariesUserManager.tsx | 33 ++++++++++++++----- .../libraries-manager/components/messages.ts | 30 +++++++++++------ .../libraries-manager/messages.ts | 5 +++ 6 files changed, 60 insertions(+), 23 deletions(-) diff --git a/src/authz-module/components/RoleCard/index.test.tsx b/src/authz-module/components/RoleCard/index.test.tsx index 6afc397..4f8ceda 100644 --- a/src/authz-module/components/RoleCard/index.test.tsx +++ b/src/authz-module/components/RoleCard/index.test.tsx @@ -20,7 +20,7 @@ describe('RoleCard', () => { title: 'Admin', objectName: 'Test Library', description: 'Can manage everything', - showDelete: true, + handleDelete: jest.fn(), userCounter: 2, permissionsByResource: [ { @@ -75,8 +75,8 @@ describe('RoleCard', () => { expect(screen.getByTestId('manage-icon')).toBeInTheDocument(); }); - it('does not show delete button when showDelete is false', () => { - renderWrapper(); + it('does not show delete button when handleDelete is not passed', () => { + renderWrapper(); expect(screen.queryByRole('button', { name: /delete role action/i })).not.toBeInTheDocument(); }); diff --git a/src/authz-module/data/hooks.ts b/src/authz-module/data/hooks.ts index d4860d5..7a86e8f 100644 --- a/src/authz-module/data/hooks.ts +++ b/src/authz-module/data/hooks.ts @@ -88,7 +88,6 @@ export const useAssignTeamMembersRole = () => { }); }; - /** * React Query hook to remove roles for a specific team member within a scope. * diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx index 80c679e..0e52b6d 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx @@ -4,6 +4,7 @@ import { renderWrapper } from '@src/setupTest'; import LibrariesUserManager from './LibrariesUserManager'; import { useLibraryAuthZ } from './context'; import { useLibrary, useTeamMembers } from '../data/hooks'; +import { ToastManagerProvider } from './ToastManagerContext'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -17,6 +18,11 @@ jest.mock('./context', () => ({ jest.mock('../data/hooks', () => ({ useLibrary: jest.fn(), useTeamMembers: jest.fn(), + useRevokeUserRoles: jest.fn(() => ({ + isPending: false, + mutation: jest.fn(), + + })), })); jest.mock('../components/RoleCard', () => ({ __esModule: true, @@ -75,7 +81,7 @@ describe('LibrariesUserManager', () => { }); it('renders the user roles correctly', () => { - renderWrapper(); + renderWrapper(); // Breadcrumb check expect(screen.getByText('Manage Access')).toBeInTheDocument(); diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.tsx index 4f39d89..903bdfd 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.tsx @@ -1,19 +1,20 @@ import { useEffect, useMemo, useState } from 'react'; import { useNavigate, useParams } from 'react-router-dom'; import { useIntl } from '@edx/frontend-platform/i18n'; +import { logError } from '@edx/frontend-platform/logging'; import { Container, Skeleton } from '@openedx/paragon'; import { ROUTES } from '@src/authz-module/constants'; import { Role } from 'types'; -import { useToastManager } from 'authz-module/libraries-manager/ToastManagerContext'; +import { useToastManager } from '@src/authz-module/libraries-manager/ToastManagerContext'; import AuthZLayout from '../components/AuthZLayout'; import { useLibraryAuthZ } from './context'; import RoleCard from '../components/RoleCard'; import { AssignNewRoleTrigger } from './components/AssignNewRoleModal'; +import ConfirmDeletionModal from './components/ConfirmDeletionModal'; import { useLibrary, useRevokeUserRoles, useTeamMembers } from '../data/hooks'; import { buildPermissionMatrixByRole } from './utils'; import messages from './messages'; -import ConfirmDeletionModal from './components/ConfirmDeletionModal'; const LibrariesUserManager = () => { const intl = useIntl(); @@ -22,6 +23,15 @@ const LibrariesUserManager = () => { const { libraryId, permissions, roles, resources, canManageTeam, } = useLibraryAuthZ(); + const teamMembersPath = `/authz/${ROUTES.LIBRARIES_TEAM_PATH.replace(':libraryId', libraryId)}`; + + useEffect(() => { + if (!canManageTeam) { + navigate(teamMembersPath); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [canManageTeam]); + const { data: library } = useLibrary(libraryId); const { mutate: revokeUserRoles, isPending: isRevokingUserRole } = useRevokeUserRoles(); const rootBreadcrumb = intl.formatMessage(messages['library.authz.breadcrumb.root']) || ''; @@ -39,11 +49,9 @@ const LibrariesUserManager = () => { const [showConfirmDeletionModal, setShowConfirmDeletionModal] = useState(false); const { handleShowToast, handleDiscardToast } = useToastManager(); - const { data: teamMember, isLoading: isLoadingTeamMember } = useTeamMembers(libraryId, querySettings); + const { data: teamMember, isLoading: isLoadingTeamMember, isFetching: isFetchingMember } = useTeamMembers(libraryId, querySettings); const user = teamMember?.results?.find(member => member.username === username); - const teamMembersPath = `/authz/${ROUTES.LIBRARIES_TEAM_PATH.replace(':libraryId', libraryId)}`; - const userRoles = useMemo(() => { const assignedRoles = roles.filter(role => user?.roles.includes(role.role)); return buildPermissionMatrixByRole({ @@ -51,6 +59,15 @@ const LibrariesUserManager = () => { }); }, [roles, user?.roles, permissions, resources, intl]); + useEffect(() => { + if (!isFetchingMember) { + if (!isLoadingTeamMember && !user?.username) { + navigate(teamMembersPath); + } + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isFetchingMember, isLoadingTeamMember, user?.username]); + const handleCloseConfirmDeletionModal = () => { setRoleToDelete(null); setShowConfirmDeletionModal(false); @@ -86,10 +103,10 @@ const LibrariesUserManager = () => { handleCloseConfirmDeletionModal(); }, onError: (error) => { - // eslint-disable-next-line no-console - console.error('Failed to revoke user role:', error); + logError(error); + // eslint-disable-next-line react/no-unstable-nested-components + handleShowToast(intl.formatMessage(messages['library.authz.team.default.error.toast.message'], { b: chunk => {chunk}, br: () =>
})); handleCloseConfirmDeletionModal(); - // Could add error toast here if needed }, }); }; diff --git a/src/authz-module/libraries-manager/components/messages.ts b/src/authz-module/libraries-manager/components/messages.ts index c9cd927..b69444f 100644 --- a/src/authz-module/libraries-manager/components/messages.ts +++ b/src/authz-module/libraries-manager/components/messages.ts @@ -6,6 +6,26 @@ const messages = defineMessages({ defaultMessage: 'Add New Role', description: 'Libraries AuthZ assign a new role to a user button title', }, + 'libraries.authz.manage.cancel.button': { + id: 'libraries.authz.manage.cancel.button', + defaultMessage: 'Cancel', + description: 'Libraries AuthZ cancel button title', + }, + 'libraries.authz.manage.saving.button': { + id: 'libraries.authz.manage.saving.button', + defaultMessage: 'Saving...', + description: 'Libraries AuthZ saving button title', + }, + 'libraries.authz.manage.save.button': { + id: 'libraries.authz.manage.save.button', + defaultMessage: 'Save', + description: 'Libraries AuthZ save button title', + }, + 'libraries.authz.manage.assign.role.success': { + id: 'libraries.authz.manage.assign.role.success', + defaultMessage: 'Role added successfully.', + description: 'Libraries AuthZ assign role success message', + }, 'library.authz.team.remove.user.modal.title': { id: 'library.authz.team.remove.user.modal.title', defaultMessage: 'Remove role?', @@ -31,11 +51,6 @@ const messages = defineMessages({ defaultMessage: 'Roles', description: 'Libraries team management label for roles select', }, - 'libraries.authz.manage.cancel.button': { - id: 'libraries.authz.manage.cancel.button', - defaultMessage: 'Cancel', - description: 'Libraries AuthZ cancel button title', - }, 'libraries.authz.manage.removing.button': { id: 'libraries.authz.manage.removing.button', defaultMessage: 'Removing...', @@ -46,11 +61,6 @@ const messages = defineMessages({ defaultMessage: 'Remove', description: 'Libraries AuthZ remove button title', }, - 'libraries.authz.manage.assign.role.success': { - id: 'libraries.authz.manage.assign.role.success', - defaultMessage: 'Role added successfully.', - description: 'Libraries AuthZ assign role success message', - }, }); export default messages; diff --git a/src/authz-module/libraries-manager/messages.ts b/src/authz-module/libraries-manager/messages.ts index fcab7e5..4e17426 100644 --- a/src/authz-module/libraries-manager/messages.ts +++ b/src/authz-module/libraries-manager/messages.ts @@ -31,6 +31,11 @@ const messages = defineMessages({ defaultMessage: 'The {role} role has been successfully removed.{rolesCount, plural, =0 { The user no longer has access to this library and has been removed from the member list.} other {}}', description: 'Libraries team management remove user toast success', }, + 'library.authz.team.default.error.toast.message': { + id: 'library.authz.team.default.error.toast.message', + defaultMessage: 'Something went wrong on our end

Please try again later.', + description: 'Libraries team management remove user toast success', + }, }); export default messages; From 1597dc4ab661f31354b0dc518a94ad70c39704a5 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Fri, 24 Oct 2025 04:18:32 +1100 Subject: [PATCH 08/12] fix: update invalidate revoke query --- src/authz-module/data/hooks.ts | 2 +- src/authz-module/index.tsx | 1 - .../libraries-manager/LibrariesUserManager.tsx | 15 +++++---------- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/authz-module/data/hooks.ts b/src/authz-module/data/hooks.ts index 7a86e8f..c47ec60 100644 --- a/src/authz-module/data/hooks.ts +++ b/src/authz-module/data/hooks.ts @@ -102,7 +102,7 @@ export const useRevokeUserRoles = () => { data: RevokeUserRolesRequest }) => revokeUserRoles(data), onSettled: (_data, _error, { data: { scope } }) => { - queryClient.invalidateQueries({ queryKey: authzQueryKeys.teamMembers(scope) }); + queryClient.invalidateQueries({ queryKey: authzQueryKeys.teamMembersAll(scope) }); }, }); }; diff --git a/src/authz-module/index.tsx b/src/authz-module/index.tsx index 0a4e130..42fd884 100644 --- a/src/authz-module/index.tsx +++ b/src/authz-module/index.tsx @@ -24,7 +24,6 @@ const AuthZModule = () => ( - )} diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.tsx index 903bdfd..1433a41 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.tsx @@ -29,7 +29,7 @@ const LibrariesUserManager = () => { if (!canManageTeam) { navigate(teamMembersPath); } - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps }, [canManageTeam]); const { data: library } = useLibrary(libraryId); @@ -49,7 +49,9 @@ const LibrariesUserManager = () => { const [showConfirmDeletionModal, setShowConfirmDeletionModal] = useState(false); const { handleShowToast, handleDiscardToast } = useToastManager(); - const { data: teamMember, isLoading: isLoadingTeamMember, isFetching: isFetchingMember } = useTeamMembers(libraryId, querySettings); + const { + data: teamMember, isLoading: isLoadingTeamMember, isFetching: isFetchingMember, + } = useTeamMembers(libraryId, querySettings); const user = teamMember?.results?.find(member => member.username === username); const userRoles = useMemo(() => { @@ -65,7 +67,7 @@ const LibrariesUserManager = () => { navigate(teamMembersPath); } } - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps }, [isFetchingMember, isLoadingTeamMember, user?.username]); const handleCloseConfirmDeletionModal = () => { @@ -111,13 +113,6 @@ const LibrariesUserManager = () => { }); }; - useEffect(() => { - if (!isLoadingTeamMember && !userRoles.length) { - navigate(teamMembersPath); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [userRoles.length]); - return (
Date: Wed, 22 Oct 2025 16:02:55 -0500 Subject: [PATCH 09/12] test: add tests for useRevokeUserRoles --- src/authz-module/data/hooks.test.tsx | 102 +++++- .../LibrariesUserManager.test.tsx | 341 ++++++++++++++++-- .../ToastManagerContext.test.tsx | 176 +++++++++ .../libraries-manager/ToastManagerContext.tsx | 20 +- 4 files changed, 591 insertions(+), 48 deletions(-) create mode 100644 src/authz-module/libraries-manager/ToastManagerContext.test.tsx diff --git a/src/authz-module/data/hooks.test.tsx b/src/authz-module/data/hooks.test.tsx index ee2643f..d55c7e0 100644 --- a/src/authz-module/data/hooks.test.tsx +++ b/src/authz-module/data/hooks.test.tsx @@ -3,7 +3,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { - useLibrary, usePermissionsByRole, useTeamMembers, useAssignTeamMembersRole, + useLibrary, usePermissionsByRole, useTeamMembers, useAssignTeamMembersRole, useRevokeUserRoles, } from './hooks'; jest.mock('@edx/frontend-platform/auth', () => ({ @@ -240,3 +240,103 @@ describe('usePermissionsByRole', () => { }); }); }); + +describe('useRevokeUserRoles', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('successfully revokes user roles', async () => { + const mockResponse = { + completed: [ + { + userIdentifiers: 'jdoe', + status: 'role_removed', + }, + ], + errors: [], + }; + + getAuthenticatedHttpClient.mockReturnValue({ + delete: jest.fn().mockResolvedValue({ data: mockResponse }), + }); + + const { result } = renderHook(() => useRevokeUserRoles(), { + wrapper: createWrapper(), + }); + + const revokeRoleData = { + scope: 'lib:123', + users: 'jdoe', + role: 'author', + }; + + await act(async () => { + result.current.mutate({ data: revokeRoleData }); + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(getAuthenticatedHttpClient).toHaveBeenCalled(); + expect(result.current.data).toEqual(mockResponse); + }); + + it('handles error when revoking roles fails', async () => { + getAuthenticatedHttpClient.mockReturnValue({ + delete: jest.fn().mockRejectedValue(new Error('Failed to revoke roles')), + }); + + const { result } = renderHook(() => useRevokeUserRoles(), { + wrapper: createWrapper(), + }); + + const revokeRoleData = { + scope: 'lib:123', + users: 'jdoe', + role: 'author', + }; + + await act(async () => { + result.current.mutate({ data: revokeRoleData }); + }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + + expect(getAuthenticatedHttpClient).toHaveBeenCalled(); + expect(result.current.error).toEqual(new Error('Failed to revoke roles')); + }); + + it('constructs URL with correct query parameters', async () => { + const mockDelete = jest.fn().mockResolvedValue({ + data: { completed: [], errors: [] }, + }); + + getAuthenticatedHttpClient.mockReturnValue({ + delete: mockDelete, + }); + + const { result } = renderHook(() => useRevokeUserRoles(), { + wrapper: createWrapper(), + }); + + const revokeRoleData = { + scope: 'lib:org/test-lib', + users: 'user1@example.com', + role: 'instructor', + }; + + await act(async () => { + result.current.mutate({ data: revokeRoleData }); + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(mockDelete).toHaveBeenCalled(); + const calledUrl = new URL(mockDelete.mock.calls[0][0]); + + // Verify the URL contains the correct query parameters + expect(calledUrl.searchParams.get('users')).toBe(revokeRoleData.users); + expect(calledUrl.searchParams.get('role')).toBe(revokeRoleData.role); + expect(calledUrl.searchParams.get('scope')).toBe(revokeRoleData.scope); + }); +}); diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx index 0e52b6d..9ac63d3 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx @@ -1,11 +1,16 @@ import { useParams } from 'react-router-dom'; -import { screen } from '@testing-library/react'; +import { screen, waitFor, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { renderWrapper } from '@src/setupTest'; import LibrariesUserManager from './LibrariesUserManager'; import { useLibraryAuthZ } from './context'; -import { useLibrary, useTeamMembers } from '../data/hooks'; +import { useLibrary, useTeamMembers, useRevokeUserRoles } from '../data/hooks'; import { ToastManagerProvider } from './ToastManagerContext'; +jest.mock('@edx/frontend-platform/logging', () => ({ + logError: jest.fn(), +})); + jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), useParams: jest.fn(), @@ -18,23 +23,55 @@ jest.mock('./context', () => ({ jest.mock('../data/hooks', () => ({ useLibrary: jest.fn(), useTeamMembers: jest.fn(), - useRevokeUserRoles: jest.fn(() => ({ - isPending: false, - mutation: jest.fn(), - - })), + useRevokeUserRoles: jest.fn(), })); + jest.mock('../components/RoleCard', () => ({ __esModule: true, - default: ({ title, description }: { title: string, description: string }) => ( + default: ({ title, description, handleDelete }: { title: string; description: string; handleDelete: () => void }) => (
{title}
{description}
+
), })); +jest.mock('./components/AssignNewRoleModal', () => ({ + AssignNewRoleTrigger: () => ( + + ), +})); + describe('LibrariesUserManager', () => { + const mockMutate = jest.fn(); + const defaultMockData = { + libraryId: 'lib:123', + permissions: [{ key: 'view' }, { key: 'reuse' }], + roles: [ + { + role: 'admin', + name: 'Admin', + description: 'Administrator Role', + permissions: ['view', 'reuse'], + userCount: 5, + }, + { + role: 'instructor', + name: 'Instructor', + description: 'Instructor Role', + permissions: ['view'], + userCount: 10, + }, + ], + resources: [{ key: 'library', label: 'Library', description: '' }], + canManageTeam: true, + }; + beforeEach(() => { jest.clearAllMocks(); @@ -42,21 +79,7 @@ describe('LibrariesUserManager', () => { (useParams as jest.Mock).mockReturnValue({ username: 'testuser' }); // Mock library authz context - (useLibraryAuthZ as jest.Mock).mockReturnValue({ - libraryId: 'lib:123', - permissions: [{ key: 'view' }, { key: 'reuse' }], - roles: [ - { - role: 'admin', - name: 'Admin', - description: 'Administrator Role', - permissions: ['view', 'reuse'], - }, - ], - resources: [ - { key: 'library', label: 'Library', description: '' }, - ], - }); + (useLibraryAuthZ as jest.Mock).mockReturnValue(defaultMockData); // Mock library data (useLibrary as jest.Mock).mockReturnValue({ @@ -68,20 +91,34 @@ describe('LibrariesUserManager', () => { // Mock team members (useTeamMembers as jest.Mock).mockReturnValue({ - data: { - results: [ - { - username: 'testuser', - email: 'testuser@example.com', - roles: ['admin'], - }, - ], - }, + data: [ + { + username: 'testuser', + email: 'testuser@example.com', + roles: ['admin', 'instructor'], + }, + ], + isLoading: false, + isFetching: false, + }); + + // Mock revoke user roles + (useRevokeUserRoles as jest.Mock).mockReturnValue({ + mutate: mockMutate, + isPending: false, }); }); + const renderComponent = () => { + renderWrapper( + + + , + ); + }; + it('renders the user roles correctly', () => { - renderWrapper(); + renderComponent(); // Breadcrumb check expect(screen.getByText('Manage Access')).toBeInTheDocument(); @@ -91,8 +128,240 @@ describe('LibrariesUserManager', () => { expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('testuser'); expect(screen.getByRole('paragraph')).toHaveTextContent('testuser@example.com'); - // RoleCard rendering - expect(screen.getByTestId('role-card')).toHaveTextContent('Admin'); - expect(screen.getByTestId('role-card')).toHaveTextContent('Administrator Role'); + const roleCards = screen.getAllByTestId('role-card'); + expect(roleCards).toHaveLength(2); + + defaultMockData.roles.forEach((role) => { + expect(screen.getByText(role.name)).toBeInTheDocument(); + expect(screen.getByText(role.description)).toBeInTheDocument(); + }); + }); + + it('renders assign role trigger when user has canManageTeam permission', () => { + renderComponent(); + + expect(screen.getByTestId('assign-role-trigger')).toBeInTheDocument(); + }); + + it('does not render assign role trigger when user does not have canManageTeam permission', () => { + (useLibraryAuthZ as jest.Mock).mockReturnValue({ + ...defaultMockData, + canManageTeam: false, + }); + + renderComponent(); + + expect(screen.queryByTestId('assign-role-trigger')).not.toBeInTheDocument(); + }); + + describe('Revoking User Role Flow', () => { + it('opens confirmation modal when delete role button is clicked', async () => { + const user = userEvent.setup(); + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + }); + }); + + it('displays correct confirmation modal content', async () => { + const user = userEvent.setup(); + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + expect(screen.getByText(/Are you sure you want to remove the Admin role from/)).toBeInTheDocument(); + expect(screen.getByText(/Test Library/)).toBeInTheDocument(); + }); + }); + + it('closes confirmation modal when cancel button is clicked', async () => { + const user = userEvent.setup(); + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + }); + + const cancelButton = screen.getByText('Cancel'); + await user.click(cancelButton); + + await waitFor(() => { + expect(screen.queryByText('Remove role?')).not.toBeInTheDocument(); + }); + }); + + it('calls revokeUserRoles mutation when Remove button is clicked', async () => { + const user = userEvent.setup(); + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + }); + + const removeButton = screen.getByText('Remove'); + await user.click(removeButton); + + expect(mockMutate).toHaveBeenCalledWith( + { + data: { + users: 'testuser', + role: 'admin', + scope: 'lib:123', + }, + }, + expect.objectContaining({ + onSuccess: expect.any(Function), + onError: expect.any(Function), + }), + ); + }); + + it('shows success toast when role is revoked successfully with multiple roles remaining', async () => { + const user = userEvent.setup(); + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + }); + + const removeButton = screen.getByText('Remove'); + await user.click(removeButton); + + const onSuccessCallback = mockMutate.mock.calls[0][1].onSuccess; + onSuccessCallback(); + + await waitFor(() => { + expect(screen.getByText(/The Admin role has been successfully removed/)).toBeInTheDocument(); + }); + }); + + it('shows success toast with user removal message when last role is revoked', async () => { + const user = userEvent.setup(); + + (useTeamMembers as jest.Mock).mockReturnValue({ + data: [ + { + username: 'testuser', + email: 'testuser@example.com', + roles: ['admin'], + }, + ], + isLoading: false, + isFetching: false, + }); + + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + }); + + const removeButton = screen.getByText('Remove'); + await user.click(removeButton); + + const onSuccessCallback = mockMutate.mock.calls[0][1].onSuccess; + onSuccessCallback(); + + await waitFor(() => { + expect(screen.getByText(/The user no longer has access to this library/)).toBeInTheDocument(); + }); + }); + + it('shows error toast when role revocation fails', async () => { + const user = userEvent.setup(); + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + }); + + const removeButton = screen.getByText('Remove'); + await user.click(removeButton); + + const onErrorCallback = mockMutate.mock.calls[0][1].onError; + onErrorCallback(new Error('Network error')); + + await waitFor(() => { + expect(screen.getByText(/Something went wrong on our end/)).toBeInTheDocument(); + }); + }); + + it('closes confirmation modal after successful role revocation', async () => { + const user = userEvent.setup(); + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + }); + + const removeButton = screen.getByText('Remove'); + await user.click(removeButton); + + const onSuccessCallback = mockMutate.mock.calls[0][1].onSuccess; + onSuccessCallback(); + + await waitFor(() => { + expect(screen.queryByText('Remove role?')).not.toBeInTheDocument(); + }); + }); + + it('disables delete action when revocation is in progress', async () => { + const user = userEvent.setup(); + + (useRevokeUserRoles as jest.Mock).mockReturnValue({ + mutate: mockMutate, + isPending: true, + }); + + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Admin'); + await user.click(deleteButton); + + expect(screen.queryByText('Remove role?')).not.toBeInTheDocument(); + expect(mockMutate).not.toHaveBeenCalled(); + }); + + it('passes correct context to confirmation modal', async () => { + const user = userEvent.setup(); + renderComponent(); + + const deleteButton = screen.getByTestId('delete-role-Instructor'); + await user.click(deleteButton); + + await waitFor(() => { + expect(screen.getByText('Remove role?')).toBeInTheDocument(); + }); + + const modal = screen.getByRole('dialog'); + expect(within(modal).getByText(/Instructor role/)).toBeInTheDocument(); + expect(within(modal).getByText(/testuser/)).toBeInTheDocument(); + expect(within(modal).getByText(/Test Library/)).toBeInTheDocument(); + }); }); }); diff --git a/src/authz-module/libraries-manager/ToastManagerContext.test.tsx b/src/authz-module/libraries-manager/ToastManagerContext.test.tsx new file mode 100644 index 0000000..28a7547 --- /dev/null +++ b/src/authz-module/libraries-manager/ToastManagerContext.test.tsx @@ -0,0 +1,176 @@ +import { screen, waitFor, render as rtlRender } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { ToastManagerProvider, useToastManager } from './ToastManagerContext'; + +const render = (ui: React.ReactElement) => rtlRender( + + {ui} + , +); + +const TestComponent = () => { + const { handleShowToast, handleDiscardToast } = useToastManager(); + + return ( +
+ + + +
+ ); +}; + +describe('ToastManagerContext', () => { + describe('ToastManagerProvider', () => { + it('does not show toast initially', () => { + render( + + + , + ); + + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('shows toast when handleShowToast is called', async () => { + const user = userEvent.setup(); + render( + + + , + ); + + // handleShowToast is called on button click + const showButton = screen.getByText('Show Toast'); + await user.click(showButton); + + await waitFor(() => { + expect(screen.getByRole('alert')).toBeInTheDocument(); + expect(screen.getByText('Test toast message')).toBeInTheDocument(); + }); + }); + + it('updates toast message when handleShowToast is called with different message', async () => { + const user = userEvent.setup(); + render( + + + , + ); + + // Show first toast + const showButton = screen.getByText('Show Toast'); + await user.click(showButton); + + await waitFor(() => { + expect(screen.getByText('Test toast message')).toBeInTheDocument(); + }); + + // Show another toast + const showAnotherButton = screen.getByText('Show Another Toast'); + await user.click(showAnotherButton); + + await waitFor(() => { + expect(screen.getByText('Another message')).toBeInTheDocument(); + expect(screen.queryByText('Test toast message')).not.toBeInTheDocument(); + }); + }); + + it('hides toast when handleDiscardToast is called', async () => { + const user = userEvent.setup(); + render( + + + , + ); + + const showButton = screen.getByText('Show Toast'); + await user.click(showButton); + + await waitFor(() => { + expect(screen.getByText('Test toast message')).toBeInTheDocument(); + }); + + // handleDiscardToast is called on button click + const discardButton = screen.getByText('Discard Toast'); + await user.click(discardButton); + + await waitFor(() => { + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + }); + + it('hides toast when close button is clicked', async () => { + const user = userEvent.setup(); + render( + + + , + ); + + const showButton = screen.getByText('Show Toast'); + await user.click(showButton); + + await waitFor(() => { + expect(screen.getByText('Test toast message')).toBeInTheDocument(); + }); + + const closeButton = screen.getByLabelText('Close'); + await user.click(closeButton); + + await waitFor(() => { + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + }); + + it('calls handleClose callback when toast is closed', async () => { + const user = userEvent.setup(); + const mockHandleClose = jest.fn(); + + render( + + + , + ); + + const showButton = screen.getByText('Show Toast'); + await user.click(showButton); + + await waitFor(() => { + expect(screen.getByText('Test toast message')).toBeInTheDocument(); + }); + + const closeButton = screen.getByLabelText('Close'); + await user.click(closeButton); + + await waitFor(() => { + expect(mockHandleClose).toHaveBeenCalledTimes(1); + }); + }); + }); + + describe('useToastManager hook', () => { + it('throws error when used outside ToastManagerProvider', () => { + // Suppress console.error for this test + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + const TestComponentWithoutProvider = () => { + useToastManager(); + return
Test
; + }; + + expect(() => { + render(); + }).toThrow('useToastManager must be used within an ToastManagerProvider'); + + consoleSpy.mockRestore(); + }); + }); +}); diff --git a/src/authz-module/libraries-manager/ToastManagerContext.tsx b/src/authz-module/libraries-manager/ToastManagerContext.tsx index 8f4319b..7862140 100644 --- a/src/authz-module/libraries-manager/ToastManagerContext.tsx +++ b/src/authz-module/libraries-manager/ToastManagerContext.tsx @@ -35,17 +35,15 @@ export const ToastManagerProvider = ({ handleClose, children }: ToastManagerProv {children} - {toastMessage && ( - { - if (handleClose) { handleClose(); } - setToastMessage(null); - }} - show={!!toastMessage} - > - {toastMessage} - - )} + { + if (handleClose) { handleClose(); } + setToastMessage(null); + }} + show={!!toastMessage} + > + {toastMessage ?? ''} + ); }; From 6a40fcd379740f5507e1dc3c300d8ac684c79f50 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Wed, 22 Oct 2025 16:12:05 -0500 Subject: [PATCH 10/12] feat: wrap AuthZModule tests in IntlProvider for localization support --- src/authz-module/index.test.tsx | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/authz-module/index.test.tsx b/src/authz-module/index.test.tsx index 8963adc..15aac37 100644 --- a/src/authz-module/index.test.tsx +++ b/src/authz-module/index.test.tsx @@ -3,6 +3,7 @@ import { render, screen, waitFor } from '@testing-library/react'; import { MemoryRouter, Outlet } from 'react-router-dom'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { initializeMockApp } from '@edx/frontend-platform/testing'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; import AuthZModule from './index'; jest.mock('./libraries-manager', () => ({ @@ -32,11 +33,13 @@ describe('AuthZModule', () => { const path = '/libraries/lib:123'; render( - - - - - , + + + + + + + , ); expect(screen.getByTestId('loading-page')).toBeInTheDocument(); @@ -51,11 +54,13 @@ describe('AuthZModule', () => { const path = '/libraries/lib:123/testuser'; render( - - - - - , + + + + + + + , ); await waitFor(() => { expect(screen.getByTestId('libraries-user-manager')).toBeInTheDocument(); From 5177e9d772f07205c49c3aa2cfd8531bb1507c4d Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 23 Oct 2025 13:44:17 -0500 Subject: [PATCH 11/12] fix: update RoleCard component to conditionally render delete button and adjust team members data structure --- .../LibrariesUserManager.test.tsx | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx index 9ac63d3..4e04591 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx @@ -28,13 +28,23 @@ jest.mock('../data/hooks', () => ({ jest.mock('../components/RoleCard', () => ({ __esModule: true, - default: ({ title, description, handleDelete }: { title: string; description: string; handleDelete: () => void }) => ( + default: ({ + title, + description, + handleDelete, + }: { + title: string; + description: string; + handleDelete?: () => void; + }) => (
{title}
{description}
- + {handleDelete && ( + + )}
), })); @@ -91,13 +101,15 @@ describe('LibrariesUserManager', () => { // Mock team members (useTeamMembers as jest.Mock).mockReturnValue({ - data: [ - { - username: 'testuser', - email: 'testuser@example.com', - roles: ['admin', 'instructor'], - }, - ], + data: { + results: [ + { + username: 'testuser', + email: 'testuser@example.com', + roles: ['admin', 'instructor'], + }, + ], + }, isLoading: false, isFetching: false, }); @@ -255,13 +267,15 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); (useTeamMembers as jest.Mock).mockReturnValue({ - data: [ - { - username: 'testuser', - email: 'testuser@example.com', - roles: ['admin'], - }, - ], + data: { + results: [ + { + username: 'testuser', + email: 'testuser@example.com', + roles: ['admin'], + }, + ], + }, isLoading: false, isFetching: false, }); From de476125f8913a17c267da01164e236f66c1323e Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Fri, 24 Oct 2025 09:08:48 +1100 Subject: [PATCH 12/12] refactor: use i18n for alt and remove data-testid --- .../components/RoleCard/index.test.tsx | 2 +- .../components/RoleCard/index.tsx | 2 +- .../components/RoleCard/messages.ts | 5 ++ .../LibrariesUserManager.test.tsx | 49 +++++++------------ 4 files changed, 24 insertions(+), 34 deletions(-) diff --git a/src/authz-module/components/RoleCard/index.test.tsx b/src/authz-module/components/RoleCard/index.test.tsx index 4f8ceda..edfe900 100644 --- a/src/authz-module/components/RoleCard/index.test.tsx +++ b/src/authz-module/components/RoleCard/index.test.tsx @@ -56,7 +56,7 @@ describe('RoleCard', () => { expect(screen.getByText('Can manage everything')).toBeInTheDocument(); // Delete button - expect(screen.getByRole('button', { name: /delete role action/i })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Delete role action/i })).toBeInTheDocument(); // Collapsible title expect(screen.getByText('Permissions')).toBeInTheDocument(); diff --git a/src/authz-module/components/RoleCard/index.tsx b/src/authz-module/components/RoleCard/index.tsx index 9bb5add..2da8c3d 100644 --- a/src/authz-module/components/RoleCard/index.tsx +++ b/src/authz-module/components/RoleCard/index.tsx @@ -42,7 +42,7 @@ const RoleCard = ({ subtitle={(objectName && {objectName}) || ''} actions={ handleDelete && ( - + ) } /> diff --git a/src/authz-module/components/RoleCard/messages.ts b/src/authz-module/components/RoleCard/messages.ts index e61fb1f..950a83e 100644 --- a/src/authz-module/components/RoleCard/messages.ts +++ b/src/authz-module/components/RoleCard/messages.ts @@ -46,6 +46,11 @@ const messages = defineMessages({ defaultMessage: 'Reuse {resource}', description: 'Default label for the reuse action', }, + 'authz.role.card.delete.action.alt': { + id: 'authz.role.card.delete.action.alt', + defaultMessage: 'Delete role action', + description: 'Alt description for delete button', + }, }); export default messages; diff --git a/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx b/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx index 4e04591..76dfa1c 100644 --- a/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx +++ b/src/authz-module/libraries-manager/LibrariesUserManager.test.tsx @@ -37,12 +37,12 @@ jest.mock('../components/RoleCard', () => ({ description: string; handleDelete?: () => void; }) => ( -
+
{title}
{description}
{handleDelete && ( - )}
@@ -50,11 +50,7 @@ jest.mock('../components/RoleCard', () => ({ })); jest.mock('./components/AssignNewRoleModal', () => ({ - AssignNewRoleTrigger: () => ( - - ), + AssignNewRoleTrigger: () => , })); describe('LibrariesUserManager', () => { @@ -140,8 +136,8 @@ describe('LibrariesUserManager', () => { expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('testuser'); expect(screen.getByRole('paragraph')).toHaveTextContent('testuser@example.com'); - const roleCards = screen.getAllByTestId('role-card'); - expect(roleCards).toHaveLength(2); + expect(screen.getByText('Admin')).toBeInTheDocument(); + expect(screen.getByText('Instructor')).toBeInTheDocument(); defaultMockData.roles.forEach((role) => { expect(screen.getByText(role.name)).toBeInTheDocument(); @@ -152,18 +148,7 @@ describe('LibrariesUserManager', () => { it('renders assign role trigger when user has canManageTeam permission', () => { renderComponent(); - expect(screen.getByTestId('assign-role-trigger')).toBeInTheDocument(); - }); - - it('does not render assign role trigger when user does not have canManageTeam permission', () => { - (useLibraryAuthZ as jest.Mock).mockReturnValue({ - ...defaultMockData, - canManageTeam: false, - }); - - renderComponent(); - - expect(screen.queryByTestId('assign-role-trigger')).not.toBeInTheDocument(); + expect(screen.getByText('Assign Role')).toBeInTheDocument(); }); describe('Revoking User Role Flow', () => { @@ -171,7 +156,7 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); await waitFor(() => { @@ -183,7 +168,7 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); await waitFor(() => { @@ -197,7 +182,7 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); await waitFor(() => { @@ -216,7 +201,7 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); await waitFor(() => { @@ -245,7 +230,7 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); await waitFor(() => { @@ -282,7 +267,7 @@ describe('LibrariesUserManager', () => { renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); await waitFor(() => { @@ -304,7 +289,7 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); await waitFor(() => { @@ -326,7 +311,7 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); await waitFor(() => { @@ -354,7 +339,7 @@ describe('LibrariesUserManager', () => { renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Admin'); + const deleteButton = screen.getByText('delete-role-Admin'); await user.click(deleteButton); expect(screen.queryByText('Remove role?')).not.toBeInTheDocument(); @@ -365,7 +350,7 @@ describe('LibrariesUserManager', () => { const user = userEvent.setup(); renderComponent(); - const deleteButton = screen.getByTestId('delete-role-Instructor'); + const deleteButton = screen.getByText('delete-role-Instructor'); await user.click(deleteButton); await waitFor(() => {