From 2e6070a788ca85a2fcc1ac3bb0169b776502b390 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Thu, 25 Jun 2026 11:58:01 +1000 Subject: [PATCH] refactor: remove unused team-members/library/permissions-by-role API and hooks --- src/authz-module/data/api.test.tsx | 101 -------------- src/authz-module/data/api.ts | 56 +------- src/authz-module/data/hooks.test.tsx | 198 --------------------------- src/authz-module/data/hooks.ts | 78 +---------- src/data/utils.ts | 1 - src/types.ts | 20 --- 6 files changed, 9 insertions(+), 445 deletions(-) diff --git a/src/authz-module/data/api.test.tsx b/src/authz-module/data/api.test.tsx index f8225c97..c9c7fb8c 100644 --- a/src/authz-module/data/api.test.tsx +++ b/src/authz-module/data/api.test.tsx @@ -1,12 +1,9 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { mockHttpClient } from '@src/setupTest'; import { - getTeamMembers, getUserAssignedRoles, assignTeamMembersRole, revokeUserRoles, - getPermissionsByRole, - getLibrary, getOrgs, getScopes, getCourseAuthoringFlagStates, @@ -32,57 +29,6 @@ describe('API functions', () => { jest.clearAllMocks(); }); - describe('getTeamMembers', () => { - it('should fetch team members successfully', async () => { - const mockResponse = { - data: { - results: [ - { username: 'user1', email: 'user1@example.com' }, - ], - count: 1, - }, - }; - - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValue(mockResponse), - }); - - const result = await getTeamMembers('lib:123', mockQuerySettings); - - expect(result.results).toHaveLength(1); - expect(result.count).toBe(1); - expect(getAuthenticatedHttpClient).toHaveBeenCalled(); - }); - - it('should handle all query parameters', async () => { - const mockResponse = { data: { results: [], count: 0 } }; - const mockGet = jest.fn().mockResolvedValue(mockResponse); - mockHttpClient().mockReturnValue({ - get: mockGet, - }); - - const queryWithAllParams = { - roles: 'admin,editor', - search: 'test user', - sortBy: 'username', - order: 'desc' as const, - scopes: null, - organizations: null, - pageSize: 20, - pageIndex: 2, - }; - - await getTeamMembers('lib:123', queryWithAllParams); - - expect(mockGet).toHaveBeenCalled(); - const calledUrl = mockGet.mock.calls[0][0]; - expect(calledUrl.toString()).toContain('roles=admin%2Ceditor'); - expect(calledUrl.toString()).toContain('search=test+user'); - expect(calledUrl.toString()).toContain('sort_by=username'); - expect(calledUrl.toString()).toContain('order=desc'); - }); - }); - describe('getUserAssignedRoles', () => { it('should fetch user assignments successfully', async () => { const mockResponse = { @@ -189,53 +135,6 @@ describe('API functions', () => { }); }); - describe('getPermissionsByRole', () => { - it('should fetch permissions by role successfully', async () => { - const mockResponse = { - data: { - results: [ - { role: 'admin', permissions: ['read', 'write'], userCount: 5 }, - ], - }, - }; - - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValue(mockResponse), - }); - - const result = await getPermissionsByRole('lib:123'); - - expect(result).toHaveLength(1); - expect(result[0].role).toBe('admin'); - expect(getAuthenticatedHttpClient).toHaveBeenCalled(); - }); - }); - - describe('getLibrary', () => { - it('should fetch library successfully', async () => { - const mockResponse = { - data: { - id: 'lib:123', - org: 'test-org', - title: 'Test Library', - slug: 'test-library', - allow_public_read: false, - }, - }; - - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValue(mockResponse), - }); - - const result = await getLibrary('lib:123'); - - expect(result.id).toBe('lib:123'); - expect(result.title).toBe('Test Library'); - expect(result.allowPublicRead).toBe(false); - expect(getAuthenticatedHttpClient).toHaveBeenCalled(); - }); - }); - describe('getOrgs', () => { it('should fetch organizations successfully', async () => { const mockResponse = { diff --git a/src/authz-module/data/api.ts b/src/authz-module/data/api.ts index affc5dae..af337a07 100644 --- a/src/authz-module/data/api.ts +++ b/src/authz-module/data/api.ts @@ -1,10 +1,7 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { - LibraryMetadata, Org, Scope, TeamMember, - UserRole, -} from '@src/types'; +import { Org, Scope, UserRole } from '@src/types'; import { camelCaseObject } from '@edx/frontend-platform'; -import { getApiUrl, getStudioApiUrl } from '@src/data/utils'; +import { getApiUrl } from '@src/data/utils'; export interface QuerySettings { roles: string | null; @@ -17,11 +14,6 @@ export interface QuerySettings { pageIndex: number; } -export interface GetTeamMembersResponse { - results: TeamMember[]; - count: number; -} - export interface GetUserAssignmentsResponse { results: UserRole[]; count: number; @@ -47,11 +39,6 @@ export interface DeleteRevokeUserRolesResponse { }[], } -export type PermissionsByRole = { - role: string; - permissions: string[]; - userCount: number; -}; export interface PutAssignTeamMembersRoleResponse { completed: { userIdentifier: string; status: string }[]; errors: { userIdentifier: string; scope: string; error: string }[]; @@ -122,26 +109,6 @@ export interface CourseAuthoringFlagStates { courseOverrides: WaffleFlagOverrides; } -export const getTeamMembers = async (object: string, querySettings: QuerySettings): Promise => { - const url = new URL(getApiUrl(`/api/authz/v1/roles/users/?scope=${object}`)); - - if (querySettings.roles) { - url.searchParams.set('roles', querySettings.roles); - } - if (querySettings.search) { - url.searchParams.set('search', querySettings.search); - } - if (querySettings.sortBy && querySettings.order) { - url.searchParams.set('sort_by', querySettings.sortBy); - url.searchParams.set('order', querySettings.order); - } - url.searchParams.set('page_size', querySettings.pageSize.toString()); - url.searchParams.set('page', (querySettings.pageIndex + 1).toString()); - - const { data } = await getAuthenticatedHttpClient().get(url); - return camelCaseObject(data); -}; - export const assignTeamMembersRole = async ( data: AssignTeamMembersRoleRequest, ): Promise => { @@ -159,25 +126,6 @@ export const validateUsers = async ( return camelCaseObject(res.data); }; -// TODO: this should be replaced in the future with Console API -export const getLibrary = async (libraryId: string): Promise => { - const { data } = await getAuthenticatedHttpClient().get(getStudioApiUrl(`/api/libraries/v2/${libraryId}/`)); - return { - id: data.id, - org: data.org, - title: data.title, - slug: data.slug, - allowPublicRead: data.allow_public_read, - }; -}; - -export const getPermissionsByRole = async (scope: string): Promise => { - const url = new URL(getApiUrl('/api/authz/v1/roles/')); - url.searchParams.append('scope', scope); - const { data } = await getAuthenticatedHttpClient().get(url); - return camelCaseObject(data.results); -}; - export const revokeUserRoles = async ( data: RevokeUserRolesRequest, ): Promise => { diff --git a/src/authz-module/data/hooks.test.tsx b/src/authz-module/data/hooks.test.tsx index 4b511bd2..9c96629f 100644 --- a/src/authz-module/data/hooks.test.tsx +++ b/src/authz-module/data/hooks.test.tsx @@ -5,9 +5,6 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { mockHttpClient } from '@src/setupTest'; import type { QuerySettings } from './api'; import { - useLibrary, - usePermissionsByRole, - useTeamMembers, useAssignTeamMembersRole, useRevokeUserRoles, useAllRoleAssignments, @@ -24,7 +21,6 @@ jest.mock('@edx/frontend-platform/auth', () => ({ jest.mock('@src/data/utils', () => ({ getApiUrl: (path: string) => `http://localhost:8000${path}`, - getStudioApiUrl: (path: string) => `http://localhost:8010${path}`, })); jest.mock('@edx/frontend-platform', () => ({ @@ -35,31 +31,6 @@ jest.mock('@src/constants', () => ({ appId: 'test-app', })); -const mockMembers = { - count: 2, - results: [ - { - fullName: 'Alice', - username: 'user1', - email: 'alice@example.com', - roles: ['admin', 'author'], - }, - { - fullName: 'Bob', - username: 'user2', - email: 'bob@example.com', - roles: ['collaborator'], - }, - ], -}; - -const mockLibrary = { - id: 'lib:123', - org: 'demo-org', - title: 'Test Library', - slug: 'test-library', -}; - const mockAssignments = { results: [ { @@ -165,175 +136,6 @@ const createWrapper = () => { return wrapper; }; -describe('useTeamMembers', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('returns data when API call succeeds', async () => { - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValue({ data: mockMembers }), - }); - - const { result } = renderHook(() => useTeamMembers('lib:123', mockQuerySettings), { - wrapper: createWrapper(), - }); - - await waitFor(() => expect(result.current.isSuccess).toBe(true)); - - expect(getAuthenticatedHttpClient).toHaveBeenCalled(); - expect(result.current.data).toEqual(mockMembers); - }); - - it('appends roles and search params when provided', async () => { - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValue({ data: { count: 0, results: [] } }), - }); - - const { result } = renderHook( - () => useTeamMembers('lib:123', { ...mockQuerySettings, roles: 'admin', search: 'alice' }), - { wrapper: createWrapper() }, - ); - - await waitFor(() => expect(result.current.isSuccess).toBe(true)); - - const mockGetFn = getAuthenticatedHttpClient().get as jest.Mock; - const calledUrl = new URL(mockGetFn.mock.calls[0][0]); - expect(calledUrl.searchParams.get('roles')).toBe('admin'); - expect(calledUrl.searchParams.get('search')).toBe('alice'); - }); - - it('appends sort params when sortBy and order are provided', async () => { - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValue({ data: { count: 0, results: [] } }), - }); - - const { result } = renderHook( - () => useTeamMembers('lib:123', { ...mockQuerySettings, sortBy: 'username', order: 'asc' }), - { wrapper: createWrapper() }, - ); - - await waitFor(() => expect(result.current.isSuccess).toBe(true)); - - const mockGetFn = getAuthenticatedHttpClient().get as jest.Mock; - const calledUrl = new URL(mockGetFn.mock.calls[0][0]); - expect(calledUrl.searchParams.get('sort_by')).toBe('username'); - expect(calledUrl.searchParams.get('order')).toBe('asc'); - }); - - it('handles error when API call fails', async () => { - mockHttpClient().mockReturnValue({ - get: jest.fn().mockRejectedValue(new Error('API failure')), - }); - - const { result } = renderHook(() => useTeamMembers('lib:123', mockQuerySettings), { - wrapper: createWrapper(), - }); - - await waitFor(() => expect(result.current.isError).toBe(true)); - - expect(getAuthenticatedHttpClient).toHaveBeenCalled(); - expect(result.current.error).toBeDefined(); - expect(result.current.data).toBeUndefined(); - }); -}); - -describe('useLibrary', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('returns metadata on success', async () => { - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValueOnce({ data: mockLibrary }), - }); - - const { result } = renderHook( - () => useLibrary('lib123'), - { wrapper: createWrapper() }, - ); - await waitFor(() => { - expect(result.current.data).toEqual(mockLibrary); - expect(getAuthenticatedHttpClient).toHaveBeenCalled(); - }); - }); - - it('maps allow_public_read to allowPublicRead', async () => { - const rawLibrary = { - id: 'lib:org/test', - org: 'org', - title: 'Test Library', - slug: 'test-library', - allow_public_read: true, - }; - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValueOnce({ data: rawLibrary }), - }); - - const { result } = renderHook(() => useLibrary('lib:org/test'), { wrapper: createWrapper() }); - - await waitFor(() => expect(result.current.data).toBeDefined()); - - expect(result.current.data).toEqual({ - id: 'lib:org/test', - org: 'org', - title: 'Test Library', - slug: 'test-library', - allowPublicRead: true, - }); - }); - - it('throws on error', () => { - mockHttpClient().mockReturnValue({ - get: jest.fn().mockRejectedValue(new Error('Not found')), - }); - - const wrapper = createWrapper(); - try { - act(() => { - renderHook(() => useLibrary('lib123'), { wrapper }); - }); - } catch (e) { - expect(e).toEqual(new Error('Not found')); - } - - expect(getAuthenticatedHttpClient).toHaveBeenCalled(); - }); -}); - -describe('usePermissionsByRole', () => { - it('fetches roles for a given scope', async () => { - const mockRoles = [ - { role: 'admin', permissions: ['perm1'], userCount: 1 }, - { role: 'user', permissions: ['perm2'], userCount: 2 }, - ]; - - mockHttpClient().mockReturnValue({ - get: jest.fn().mockResolvedValue({ data: { results: mockRoles } }), - }); - - const wrapper = createWrapper(); - const { result } = renderHook(() => usePermissionsByRole('lib'), { wrapper }); - await waitFor(() => result.current.data !== undefined); - expect(result.current.data).toEqual(mockRoles); - expect(getAuthenticatedHttpClient).toHaveBeenCalled(); - }); - - it('returns error if getRoles fails', async () => { - mockHttpClient().mockReturnValue({ - get: jest.fn().mockRejectedValue(new Error('Not found')), - }); - const wrapper = createWrapper(); - try { - act(() => { - renderHook(() => usePermissionsByRole('lib'), { wrapper }); - }); - } catch (e) { - expect(e).toEqual(new Error('Not found')); - } - }); -}); - describe('useAssignTeamMembersRole', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/src/authz-module/data/hooks.ts b/src/authz-module/data/hooks.ts index 3505705e..83a1504e 100644 --- a/src/authz-module/data/hooks.ts +++ b/src/authz-module/data/hooks.ts @@ -1,13 +1,11 @@ import { - useInfiniteQuery, useMutation, useQuery, useQueryClient, useSuspenseQuery, + useInfiniteQuery, useMutation, useQuery, useQueryClient, } from '@tanstack/react-query'; import { appId } from '@src/constants'; -import { LibraryMetadata } from '@src/types'; import { assignTeamMembersRole, AssignTeamMembersRoleRequest, getAllRoleAssignments, - GetAllRoleAssignmentsResponse, getLibrary, getOrgs, GetOrgsResponse, - getPermissionsByRole, getScopes, GetScopesResponse, getTeamMembers, - GetTeamMembersResponse, PermissionsByRole, QuerySettings, revokeUserRoles, + GetAllRoleAssignmentsResponse, getOrgs, GetOrgsResponse, + getScopes, GetScopesResponse, QuerySettings, revokeUserRoles, RevokeUserRolesRequest, getUserAssignedRoles, GetUserAssignmentsResponse, validateUsers, ValidateUsersRequest, GetScopesParams, getCourseAuthoringFlagStates, CourseAuthoringFlagStates, @@ -15,69 +13,13 @@ import { const authzQueryKeys = { all: [appId, 'authz'] as const, - teamMembersAll: (scope: string) => [...authzQueryKeys.all, 'teamMembers', scope] as const, - teamMembers: (scope: string, querySettings?: QuerySettings) => [ - ...authzQueryKeys.teamMembersAll(scope), querySettings] as const, - permissionsByRole: (scope: string) => [...authzQueryKeys.all, 'permissionsByRole', scope] as const, - library: (libraryId: string) => [...authzQueryKeys.all, 'library', libraryId] as const, allRoleAssignments: (querySettings?: QuerySettings) => [...authzQueryKeys.all, 'allRoleAssignments', querySettings] as const, orgs: (search?: string, page?: number, pageSize?: number) => [...authzQueryKeys.all, 'organizations', search, page, pageSize] as const, - scopes: (search?: string, page?: number, pageSize?: number) => [...authzQueryKeys.all, 'scopes', search, page, pageSize] as const, + scopes: (params?: Omit) => [...authzQueryKeys.all, 'scopes', params] as const, userRoles: (username?: string, querySettings?: QuerySettings) => [...authzQueryKeys.all, 'userRoles', username, querySettings] as const, courseAuthoringFlagStates: () => [...authzQueryKeys.all, 'courseAuthoringFlagStates'] as const, }; -/** - * React Query hook to fetch all team members for a specific object/scope. - * It retrieves the full list of members who have access to the given scope. - * - * @param scope - The unique identifier of the object/scope - * @param querySettings - Optional query parameters for filtering, sorting, and pagination - * - * @example - * ```tsx - * const { data: teamMembers, isLoading, isError } = useTeamMembers('lib:123', querySettings); - * ``` - */ -export const useTeamMembers = (scope: string, querySettings: QuerySettings) => useQuery({ - queryKey: authzQueryKeys.teamMembers(scope, querySettings), - queryFn: () => getTeamMembers(scope, querySettings), - staleTime: 1000 * 60 * 30, // refetch after 30 minutes - refetchOnWindowFocus: false, -}); - -/** - * React Query hook to fetch all the roles for the specific object/scope. - * It retrieves the full list of roles with the corresponding permissions. - * - * @param scope - The unique identifier of the object/scope - * - * @example - * ```tsx - * const { data: roles } = usePermissionsByRole('lib:123'); - * ``` - */ -export const usePermissionsByRole = (scope: string) => useSuspenseQuery({ - queryKey: authzQueryKeys.permissionsByRole(scope), - queryFn: () => getPermissionsByRole(scope), - retry: false, -}); - -/** - * React Query hook to retrieve the information of the current library. - * - * @param libraryId - The unique ID of the library. - * - * @example - * const { data } = useLibrary('lib:123',); - * - */ -export const useLibrary = (libraryId: string) => useSuspenseQuery({ - queryKey: authzQueryKeys.library(libraryId), - queryFn: () => getLibrary(libraryId), - retry: false, -}); - /** * React Query hook to add new team members to a specific scope or manage the corresponding roles. * It provides a mutation function to add users with specified roles to the team or assign new roles. @@ -92,12 +34,8 @@ export const useAssignTeamMembersRole = () => { mutationFn: async ({ data }: { data: AssignTeamMembersRoleRequest }) => assignTeamMembersRole(data), - onSettled: (_data, error, { data: { scopes } }) => { + onSettled: (_data, error) => { if (!error) { - scopes.forEach((scope) => { - queryClient.invalidateQueries({ queryKey: authzQueryKeys.teamMembersAll(scope) }); - queryClient.invalidateQueries({ queryKey: authzQueryKeys.permissionsByRole(scope) }); - }); queryClient.invalidateQueries({ queryKey: [...authzQueryKeys.all, 'userRoles'] }); queryClient.invalidateQueries({ predicate: (query) => query.queryKey.includes('allRoleAssignments'), @@ -134,9 +72,7 @@ export const useRevokeUserRoles = () => { mutationFn: async ({ data }: { data: RevokeUserRolesRequest }) => revokeUserRoles(data), - onSettled: (_data, _error, { data: { scope } }) => { - queryClient.invalidateQueries({ queryKey: authzQueryKeys.teamMembersAll(scope) }); - queryClient.invalidateQueries({ queryKey: authzQueryKeys.permissionsByRole(scope) }); + onSettled: () => { queryClient.invalidateQueries({ predicate: (query) => query.queryKey.includes('userRoles'), }); @@ -211,7 +147,7 @@ export const useOrgs = (search?: string, page?: number, pageSize?: number) => us * ``` */ export const useScopes = (params: Omit = {}) => useInfiniteQuery({ - queryKey: [...authzQueryKeys.all, 'scopes', params], + queryKey: authzQueryKeys.scopes(params), queryFn: ({ pageParam }) => getScopes({ ...params, page: pageParam as number }), getNextPageParam: (lastPage) => { if (!lastPage.next) { return undefined; } diff --git a/src/data/utils.ts b/src/data/utils.ts index e1536723..1c58c71f 100644 --- a/src/data/utils.ts +++ b/src/data/utils.ts @@ -1,7 +1,6 @@ import { getConfig } from '@edx/frontend-platform'; export const getApiUrl = (path: string) => `${getConfig().LMS_BASE_URL}${path || ''}`; -export const getStudioApiUrl = (path: string) => `${getConfig().STUDIO_BASE_URL}${path || ''}`; /** * Safely reads the HTTP status that @edx/frontend-platform's HTTP client attaches diff --git a/src/types.ts b/src/types.ts index e473331b..b50ba782 100644 --- a/src/types.ts +++ b/src/types.ts @@ -7,26 +7,6 @@ export interface PermissionValidationResponse extends PermissionValidationReques allowed: boolean; } -// Libraries AuthZ types -export interface TeamMember { - username: string; - fullName: string; - email: string; - roles: string[]; - createdAt: string; - scope: { resource: string, type: 'COURSE' | 'LIBRARY' | 'GLOBAL' }; - organization: string; - role: string; -} - -export interface LibraryMetadata { - id: string; - org: string; - title: string; - slug: string; - allowPublicRead: boolean; -} - export interface RoleMetadata { role: string; name: string;