Skip to content

Commit 16ed82c

Browse files
refactor: replace useUserPermissionsWithAuthzCourse with useCourseUserPermissions
Remove the deprecated useUserPermissionsWithAuthzCourse hook and migrate all consumers to use useCourseUserPermissions, which provides a flatter API by spreading permission booleans directly into the return object.
1 parent 5dedece commit 16ed82c

6 files changed

Lines changed: 71 additions & 98 deletions

File tree

src/authz/hooks.test.tsx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
55
import * as authzApi from '@src/authz/data/api';
66
import { PermissionValidationQuery } from '@src/authz/types';
77
import { mockWaffleFlags } from '@src/data/apiHooks.mock';
8-
import { useUserPermissionsWithAuthzCourse } from './hooks';
8+
import { useCourseUserPermissions } from './hooks';
99

1010
jest.mock('@src/data/api');
1111
jest.mock('@src/authz/data/api');
1212

1313
const mockedAuthzApi = jest.mocked(authzApi);
1414

15-
describe('useUserPermissionsWithAuthzCourse', () => {
15+
describe('useCourseUserPermissions', () => {
1616
let queryClient: QueryClient;
1717

1818
const createWrapper = () =>
@@ -50,7 +50,7 @@ describe('useUserPermissionsWithAuthzCourse', () => {
5050
});
5151

5252
const { result } = renderHook(
53-
() => useUserPermissionsWithAuthzCourse('course-v1:Test+101+2023', mockPermissions),
53+
() => useCourseUserPermissions('course-v1:Test+101+2023', mockPermissions),
5454
{ wrapper: createWrapper() },
5555
);
5656

@@ -59,8 +59,8 @@ describe('useUserPermissionsWithAuthzCourse', () => {
5959
});
6060

6161
expect(result.current.isAuthzEnabled).toBe(false);
62-
expect(result.current.permissions.canViewFiles).toBe(true);
63-
expect(result.current.permissions.canManageFiles).toBe(true);
62+
expect(result.current.canViewFiles).toBe(true);
63+
expect(result.current.canManageFiles).toBe(true);
6464
});
6565

6666
it('returns loading state when authz is enabled and permissions are loading', async () => {
@@ -73,7 +73,7 @@ describe('useUserPermissionsWithAuthzCourse', () => {
7373
);
7474

7575
const { result } = renderHook(
76-
() => useUserPermissionsWithAuthzCourse('course-v1:Test+101+2023', mockPermissions),
76+
() => useCourseUserPermissions('course-v1:Test+101+2023', mockPermissions),
7777
{ wrapper: createWrapper() },
7878
);
7979

@@ -95,7 +95,7 @@ describe('useUserPermissionsWithAuthzCourse', () => {
9595
});
9696

9797
const { result } = renderHook(
98-
() => useUserPermissionsWithAuthzCourse('course-v1:Test+101+2023', mockPermissions),
98+
() => useCourseUserPermissions('course-v1:Test+101+2023', mockPermissions),
9999
{ wrapper: createWrapper() },
100100
);
101101

@@ -104,8 +104,8 @@ describe('useUserPermissionsWithAuthzCourse', () => {
104104
});
105105

106106
expect(result.current.isAuthzEnabled).toBe(true);
107-
expect(result.current.permissions.canViewFiles).toBe(true);
108-
expect(result.current.permissions.canManageFiles).toBe(false);
107+
expect(result.current.canViewFiles).toBe(true);
108+
expect(result.current.canManageFiles).toBe(false);
109109
});
110110

111111
it('falls back to false for undefined permissions when authz is enabled', async () => {
@@ -118,7 +118,7 @@ describe('useUserPermissionsWithAuthzCourse', () => {
118118
});
119119

120120
const { result } = renderHook(
121-
() => useUserPermissionsWithAuthzCourse('course-v1:Test+101+2023', mockPermissions),
121+
() => useCourseUserPermissions('course-v1:Test+101+2023', mockPermissions),
122122
{ wrapper: createWrapper() },
123123
);
124124

@@ -127,7 +127,7 @@ describe('useUserPermissionsWithAuthzCourse', () => {
127127
});
128128

129129
expect(result.current.isAuthzEnabled).toBe(true);
130-
expect(result.current.permissions.canViewFiles).toBe(true);
131-
expect(result.current.permissions.canManageFiles).toBe(false);
130+
expect(result.current.canViewFiles).toBe(true);
131+
expect(result.current.canManageFiles).toBe(false);
132132
});
133133
});

src/authz/hooks.ts

Lines changed: 0 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,6 @@ type UseCourseUserPermissionsReturn<Query extends PermissionValidationQuery> = {
77
isAuthzEnabled: boolean;
88
} & PermissionValidationAnswer<Query>;
99

10-
/**
11-
* Return type for the useUserCoursePermissionsWithAuthz hook
12-
*/
13-
interface UseUserPermissionsWithAuthzCourseReturn {
14-
/** Whether permissions are currently loading */
15-
isLoading: boolean;
16-
/** Object containing permission results with boolean values */
17-
permissions: PermissionValidationAnswer;
18-
/** Whether authorization is enabled for the course */
19-
isAuthzEnabled: boolean;
20-
}
21-
2210
/**
2311
* Custom hook to retrieve and evaluate user permissions for the current course using the openedx-authz service.
2412
*
@@ -81,69 +69,3 @@ export const useCourseUserPermissions = <Query extends PermissionValidationQuery
8169
...permissionResults as PermissionValidationAnswer<Query>,
8270
};
8371
};
84-
85-
/**
86-
* Custom hook to handle user permissions with course authorization waffle flag
87-
*
88-
* This hook abstracts the common pattern of:
89-
* 1. Checking if authz is enabled via waffle flag
90-
* 2. Fetching user permissions when authz is enabled
91-
* 3. Defaulting all permissions to true when authz is disabled
92-
* 4. Providing fallback values for undefined permissions
93-
*
94-
* @param courseId - The course ID to check permissions for
95-
* @param permissions - Object mapping permission names to their action/scope definitions
96-
* @returns Object containing loading state, permissions results, and authz status
97-
*
98-
* @example
99-
* ```tsx
100-
* const { isLoading, permissions, isAuthzEnabled } = useUserPermissionsWithAuthzCourse(
101-
* courseId,
102-
* {
103-
* canViewFiles: {
104-
* action: COURSE_PERMISSIONS.VIEW_FILES,
105-
* scope: courseId,
106-
* },
107-
* canManageFiles: {
108-
* action: COURSE_PERMISSIONS.MANAGE_FILES,
109-
* scope: courseId,
110-
* },
111-
* }
112-
* );
113-
*
114-
* const { canViewFiles, canManageFiles } = permissions;
115-
* ```
116-
*/
117-
export const useUserPermissionsWithAuthzCourse = (
118-
courseId: string,
119-
permissions: PermissionValidationQuery,
120-
): UseUserPermissionsWithAuthzCourseReturn => {
121-
const waffleFlags = useWaffleFlags(courseId);
122-
const isAuthzEnabled: boolean = waffleFlags?.enableAuthzCourseAuthoring ?? false;
123-
124-
const {
125-
isLoading: isLoadingUserPermissions,
126-
data: userPermissions,
127-
} = useUserPermissions(permissions, isAuthzEnabled);
128-
129-
// Build permission results object
130-
const permissionResults: PermissionValidationAnswer = {};
131-
132-
if (!isAuthzEnabled) {
133-
// Authz is disabled, default all permissions to true immediately
134-
Object.keys(permissions).forEach((permissionKey: string) => {
135-
permissionResults[permissionKey] = true;
136-
});
137-
} else if (!isLoadingUserPermissions) {
138-
// Authz is enabled and permissions loaded, use actual permission values with fallback to false
139-
Object.keys(permissions).forEach((permissionKey: string) => {
140-
permissionResults[permissionKey] = userPermissions?.[permissionKey] ?? false;
141-
});
142-
}
143-
144-
return {
145-
isLoading: isAuthzEnabled ? isLoadingUserPermissions : false,
146-
permissions: permissionResults,
147-
isAuthzEnabled,
148-
};
149-
};

src/course-updates/CourseUpdates.test.tsx

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
fireEvent,
99
screen,
1010
} from '@src/testUtils';
11+
import userEvent from '@testing-library/user-event';
1112

1213
import { useUserPermissions } from '@src/authz/data/apiHooks';
1314

@@ -403,6 +404,15 @@ describe('<CourseUpdates />', () => {
403404
expect(await screen.findAllByRole('button', { name: /edit/i })).toHaveLength(4); // 3 for course updates and 1 for handouts
404405
expect(await screen.findAllByRole('button', { name: /delete/i })).toHaveLength(3);
405406
});
407+
408+
it('should open delete modal when clicking delete button on a course update', async () => {
409+
render(<RootWrapper />);
410+
411+
const deleteButtons = await screen.findAllByTestId('course-update-delete-button');
412+
await userEvent.click(deleteButtons[0]);
413+
414+
expect(screen.getByText('Are you sure you want to delete this update?')).toBeInTheDocument();
415+
});
406416
});
407417

408418
describe('when user does NOT have permission to manage course updates and enableAuthzCourseAuthoring is enabled', () => {
@@ -479,6 +489,34 @@ describe('<CourseUpdates />', () => {
479489
});
480490
});
481491

492+
describe('when permissions are still loading', () => {
493+
beforeEach(() => {
494+
const mocks = initializeMocks();
495+
store = mocks.reduxStore;
496+
axiosMock = mocks.axiosMock;
497+
498+
(apiHooks.useWaffleFlags as jest.Mock).mockReturnValue({ enableAuthzCourseAuthoring: true });
499+
(useUserPermissions as jest.Mock).mockReturnValue({
500+
isLoading: true,
501+
data: undefined,
502+
});
503+
504+
axiosMock
505+
.onGet(getCourseUpdatesApiUrl(courseId))
506+
.reply(200, courseUpdatesMock);
507+
axiosMock
508+
.onGet(getCourseHandoutApiUrl(courseId))
509+
.reply(200, courseHandoutsMock);
510+
});
511+
512+
it('should render nothing while permissions are loading', () => {
513+
render(<RootWrapper />);
514+
515+
expect(screen.queryByText(messages.headingTitle.defaultMessage)).not.toBeInTheDocument();
516+
expect(screen.queryByRole('button', { name: messages.newUpdateButton.defaultMessage })).not.toBeInTheDocument();
517+
});
518+
});
519+
482520
describe('when user does NOT have permission to view course updates', () => {
483521
beforeEach(() => {
484522
const mocks = initializeMocks();

src/course-updates/CourseUpdates.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { matchesAnyStatus } from './utils';
3131
import getPageHeadTitle from '../generic/utils';
3232
import AlertMessage from '../generic/alert-message';
3333
import { getCourseUpdatesPermissions } from '@src/authz/permissionHelpers';
34-
import { useUserPermissionsWithAuthzCourse } from '@src/authz/hooks';
34+
import { useCourseUserPermissions } from '@src/authz/hooks';
3535

3636
const CourseUpdates = () => {
3737
const intl = useIntl();
@@ -56,10 +56,9 @@ const CourseUpdates = () => {
5656

5757
const {
5858
isLoading: isLoadingPermissions,
59-
permissions,
60-
} = useUserPermissionsWithAuthzCourse(courseId, getCourseUpdatesPermissions(courseId));
61-
62-
const { canViewCourseUpdates, canManageCourseUpdates } = permissions;
59+
canViewCourseUpdates,
60+
canManageCourseUpdates,
61+
} = useCourseUserPermissions(courseId, getCourseUpdatesPermissions(courseId));
6362

6463
const loadingStatuses = useSelector(getLoadingStatuses);
6564
const savingStatuses = useSelector(getSavingStatuses);

src/header/hooks.test.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,20 @@ describe('header utils', () => {
135135
const actualItemsTitle = actualItems.map((item) => item.title);
136136
expect(actualItemsTitle).toContain(messages['header.links.updates'].defaultMessage);
137137
});
138+
it('when useNewUpdatesPage is false should use legacy studio URL for updates', () => {
139+
mockWaffleFlags({ enableAuthzCourseAuthoring: false, useNewUpdatesPage: false });
140+
jest.mocked(useUserPermissions).mockReturnValue({
141+
isLoading: false,
142+
data: { canViewCourseUpdates: true },
143+
} as any);
144+
jest.mocked(useSelector).mockReturnValue({
145+
librariesV2Enabled: false,
146+
});
147+
const actualItems =
148+
renderHook(() => useContentMenuItems('course-123'), { wrapper: createWrapper() }).result.current;
149+
const updatesItem = actualItems.find((item) => item.title === messages['header.links.updates'].defaultMessage);
150+
expect(updatesItem?.href).toContain('/course_info/course-123');
151+
});
138152
});
139153

140154
describe('getSettingsMenuitems', () => {

src/header/hooks.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { useUserPermissions } from '@src/authz/data/apiHooks';
1414
import { COURSE_PERMISSIONS } from '@src/authz/constants';
1515
import messages from './messages';
1616
import { getCourseUpdatesPermissions } from '@src/authz/permissionHelpers';
17-
import { useUserPermissionsWithAuthzCourse } from '@src/authz/hooks';
17+
import { useCourseUserPermissions } from '@src/authz/hooks';
1818

1919
export const useContentMenuItems = (courseId: string) => {
2020
const intl = useIntl();
@@ -23,8 +23,8 @@ export const useContentMenuItems = (courseId: string) => {
2323
const { librariesV2Enabled } = useSelector(getStudioHomeData);
2424

2525
const {
26-
permissions: { canViewCourseUpdates },
27-
} = useUserPermissionsWithAuthzCourse(courseId, getCourseUpdatesPermissions(courseId));
26+
canViewCourseUpdates,
27+
} = useCourseUserPermissions(courseId, getCourseUpdatesPermissions(courseId));
2828

2929
const items = [
3030
{

0 commit comments

Comments
 (0)