fix: address feedback

Author: bra-i-am

src/authz/hooks.test.ts

@@ -1,7 +1,7 @@
 import { renderHook } from '@testing-library/react';
 import { useUserPermissions } from '@src/authz/data/apiHooks';
 import { mockWaffleFlags } from '@src/data/apiHooks.mock';
-import { useUserPermissionsWithAuthzCourse } from './hooks';
+import { useCourseUserPermissions } from './hooks';
 import { COURSE_PERMISSIONS } from './constants';
 
 jest.mock('@src/authz/data/apiHooks', () => ({
@@ -14,7 +14,7 @@ const permissions = {
   canEdit: { action: COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS, scope: courseId },
 };
 
-describe('useUserPermissionsWithAuthzCourse', () => {
+describe('useCourseUserPermissions', () => {
   beforeEach(() => {
     jest.clearAllMocks();
     jest.mocked(useUserPermissions).mockReturnValue({
@@ -26,12 +26,12 @@ describe('useUserPermissionsWithAuthzCourse', () => {
   it('defaults all permissions to true when authz is disabled', () => {
     mockWaffleFlags({ enableAuthzCourseAuthoring: false });
 
-    const { result } = renderHook(() => useUserPermissionsWithAuthzCourse(courseId, permissions));
+    const { result } = renderHook(() => useCourseUserPermissions(courseId, permissions));
 
     expect(result.current.isLoading).toBe(false);
     expect(result.current.isAuthzEnabled).toBe(false);
-    expect(result.current.permissions.canView).toBe(true);
-    expect(result.current.permissions.canEdit).toBe(true);
+    expect(result.current.canView).toBe(true);
+    expect(result.current.canEdit).toBe(true);
   });
 
   it('returns actual permission values when authz is enabled and permissions are loaded', () => {
@@ -41,26 +41,27 @@ describe('useUserPermissionsWithAuthzCourse', () => {
       data: { canView: true, canEdit: false },
     } as unknown as ReturnType<typeof useUserPermissions>);
 
-    const { result } = renderHook(() => useUserPermissionsWithAuthzCourse(courseId, permissions));
+    const { result } = renderHook(() => useCourseUserPermissions(courseId, permissions));
 
     expect(result.current.isLoading).toBe(false);
     expect(result.current.isAuthzEnabled).toBe(true);
-    expect(result.current.permissions.canView).toBe(true);
-    expect(result.current.permissions.canEdit).toBe(false);
+    expect(result.current.canView).toBe(true);
+    expect(result.current.canEdit).toBe(false);
   });
 
-  it('returns isLoading=true and empty permissions while authz permissions are loading', () => {
+  it('returns isLoading=true and no permission keys while authz permissions are loading', () => {
     mockWaffleFlags({ enableAuthzCourseAuthoring: true });
     jest.mocked(useUserPermissions).mockReturnValue({
       isLoading: true,
       data: undefined,
     } as unknown as ReturnType<typeof useUserPermissions>);
 
-    const { result } = renderHook(() => useUserPermissionsWithAuthzCourse(courseId, permissions));
+    const { result } = renderHook(() => useCourseUserPermissions(courseId, permissions));
 
     expect(result.current.isLoading).toBe(true);
     expect(result.current.isAuthzEnabled).toBe(true);
-    expect(result.current.permissions).toEqual({});
+    expect(result.current.canView).toBeUndefined();
+    expect(result.current.canEdit).toBeUndefined();
   });
 
   it('falls back to false for permissions absent from server response when authz is enabled', () => {
@@ -70,9 +71,9 @@ describe('useUserPermissionsWithAuthzCourse', () => {
       data: {},
     } as unknown as ReturnType<typeof useUserPermissions>);
 
-    const { result } = renderHook(() => useUserPermissionsWithAuthzCourse(courseId, permissions));
+    const { result } = renderHook(() => useCourseUserPermissions(courseId, permissions));
 
-    expect(result.current.permissions.canView).toBe(false);
-    expect(result.current.permissions.canEdit).toBe(false);
+    expect(result.current.canView).toBe(false);
+    expect(result.current.canEdit).toBe(false);
   });
 });

src/authz/hooks.ts

@@ -1,29 +1,28 @@
 import { useWaffleFlags } from '@src/data/apiHooks';
 import { useUserPermissions } from '@src/authz/data/apiHooks';
-import { PermissionValidationQuery, PermissionValidationAnswer } from '@src/authz/types';
+import { PermissionValidationQuery } from '@src/authz/types';
 
-interface UseUserPermissionsWithAuthzCourseReturn {
+type UseCourseUserPermissionsReturn<Query extends PermissionValidationQuery> = {
   isLoading: boolean;
-  permissions: PermissionValidationAnswer;
   isAuthzEnabled: boolean;
-}
+} & { [K in keyof Query]: boolean; };
 
 /**
- * Custom hook to handle user permissions with course authorization waffle flag
+ * Custom hook to retrieve and evaluate user permissions for the current course using the openedx-authz service.
  *
- * This hook abstracts the common pattern of:
- * 1. Checking if authz is enabled via waffle flag
- * 2. Fetching user permissions when authz is enabled
- * 3. Defaulting all permissions to true when authz is disabled
- * 4. Providing fallback values for undefined permissions
+ * The hook:
+ * 1. Validate if authz is enabled via waffle flag
+ * 2. Fetch user permissions when authz is enabled
+ * 3. Fallback all permissions to 'true' when authz is disabled
+ * 4. Provide fallback values for undefined permissions
  *
  * @param courseId - The course ID to check permissions for
  * @param permissions - Object mapping permission names to their action/scope definitions
  * @returns Object containing loading state, permissions results, and authz status
  *
  * @example
  * ```tsx
- * const { isLoading, permissions, isAuthzEnabled } = useUserPermissionsWithAuthzCourse(
+ * const { isLoading, canViewGradingSettings, canEditGradingSettings, isAuthzEnabled } = useCourseUserPermissions(
  *   courseId,
  *   {
  *     canViewGradingSettings: {
@@ -36,14 +35,12 @@ interface UseUserPermissionsWithAuthzCourseReturn {
  *     },
  *   }
  * );
- *
- * const { canViewGradingSettings, canEditGradingSettings } = permissions;
  * ```
  */
-export const useUserPermissionsWithAuthzCourse = (
+export const useCourseUserPermissions = <Query extends PermissionValidationQuery>(
   courseId: string,
-  permissions: PermissionValidationQuery,
-): UseUserPermissionsWithAuthzCourseReturn => {
+  permissions: Query,
+): UseCourseUserPermissionsReturn<Query> => {
   const waffleFlags = useWaffleFlags(courseId);
   const isAuthzEnabled: boolean = waffleFlags?.enableAuthzCourseAuthoring ?? false;
 
@@ -52,21 +49,23 @@ export const useUserPermissionsWithAuthzCourse = (
     data: userPermissions,
   } = useUserPermissions(permissions, isAuthzEnabled);
 
-  const permissionResults: PermissionValidationAnswer = {};
+  const resolvePermission = (key: string): boolean => {
+    if (!isAuthzEnabled) {
+      return true;
+    }
+    return userPermissions?.[key] ?? false;
+  };
 
-  if (isAuthzEnabled && !isLoadingUserPermissions) {
-    Object.keys(permissions).forEach((permissionKey: string) => {
-      permissionResults[permissionKey] = userPermissions?.[permissionKey] ?? false;
-    });
-  } else if (!isLoadingUserPermissions) {
-    Object.keys(permissions).forEach((permissionKey: string) => {
-      permissionResults[permissionKey] = true;
-    });
-  }
+  const permissionResults: Record<string, boolean> = isLoadingUserPermissions
+    ? {}
+    : Object.keys(permissions).reduce<Record<string, boolean>>((acc, key) => {
+      acc[key] = resolvePermission(key);
+      return acc;
+    }, {});
 
   return {
     isLoading: isAuthzEnabled ? isLoadingUserPermissions : false,
-    permissions: permissionResults,
     isAuthzEnabled,
+    ...permissionResults as { [K in keyof Query]: boolean; },
   };
 };

src/grading-settings/GradingSettings.jsx

@@ -12,7 +12,7 @@ import { Helmet } from 'react-helmet';
 import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
 import { STATEFUL_BUTTON_STATES } from '@src/constants';
 import { useCourseSettings } from '@src/data/apiHooks';
-import { useUserPermissionsWithAuthzCourse } from '@src/authz/hooks';
+import { useCourseUserPermissions } from '@src/authz/hooks';
 import { getGradingPermissions } from '@src/authz/permissionHelpers';
 import ConnectionErrorAlert from '@src/generic/ConnectionErrorAlert';
 import PermissionDeniedAlert from '@src/generic/PermissionDeniedAlert';
@@ -40,8 +40,9 @@ const GradingSettings = () => {
 
   const {
     isLoading: isLoadingUserPermissions,
-    permissions: userPermissions,
-  } = useUserPermissionsWithAuthzCourse(courseId, getGradingPermissions(courseId));
+    canViewGradingSettings,
+    canEditGradingSettings,
+  } = useCourseUserPermissions(courseId, getGradingPermissions(courseId));
 
   const {
     data: gradingSettings,
@@ -102,7 +103,7 @@ const GradingSettings = () => {
     }
   }, [savePending]);
 
-  if (!isLoadingUserPermissions && !userPermissions.canViewGradingSettings) {
+  if (!isLoadingUserPermissions && !canViewGradingSettings) {
     return <PermissionDeniedAlert />;
   }
 
@@ -118,7 +119,7 @@ const GradingSettings = () => {
     return null;
   }
 
-  const isEditable = !isLoadingUserPermissions && userPermissions.canEditGradingSettings;
+  const isEditable = !isLoadingUserPermissions && canEditGradingSettings;
 
   const handleQueryProcessing = () => {
     setShowSuccessAlert(false);

src/grading-settings/GradingSettings.test.jsx

@@ -8,7 +8,7 @@ import {
 import { CourseAuthoringProvider } from '@src/CourseAuthoringContext';
 import { getCourseSettingsApiUrl } from '@src/data/api';
 import { mockWaffleFlags } from '@src/data/apiHooks.mock';
-import { useUserPermissionsWithAuthzCourse } from '@src/authz/hooks';
+import { useCourseUserPermissions } from '@src/authz/hooks';
 
 import gradingSettings from './__mocks__/gradingSettings';
 import { getGradingSettingsApiUrl } from './data/api';
@@ -17,12 +17,10 @@ import GradingSettings from './GradingSettings';
 import messages from './messages';
 
 jest.mock('@src/authz/hooks', () => ({
-  useUserPermissionsWithAuthzCourse: jest.fn().mockReturnValue({
+  useCourseUserPermissions: jest.fn().mockReturnValue({
     isLoading: false,
-    permissions: {
-      canViewGradingSettings: true,
-      canEditGradingSettings: true,
-    },
+    canViewGradingSettings: true,
+    canEditGradingSettings: true,
   }),
 }));
 
@@ -151,9 +149,10 @@ describe('<GradingSettings /> permissions', () => {
     mock.onGet(getGradingSettingsApiUrl(courseId)).reply(200, gradingSettings);
     mock.onPost(getGradingSettingsApiUrl(courseId)).reply(200, {});
     mock.onGet(getCourseSettingsApiUrl(courseId)).reply(200, {});
-    jest.mocked(useUserPermissionsWithAuthzCourse).mockReturnValue({
+    jest.mocked(useCourseUserPermissions).mockReturnValue({
       isLoading: false,
-      permissions: { canViewGradingSettings: true, canEditGradingSettings: true },
+      canViewGradingSettings: true,
+      canEditGradingSettings: true,
     });
   });
 
@@ -171,19 +170,21 @@ describe('<GradingSettings /> permissions', () => {
 
   it('should show permission denied alert when user lacks view permission', async () => {
     mockWaffleFlags({ enableAuthzCourseAuthoring: true });
-    jest.mocked(useUserPermissionsWithAuthzCourse).mockReturnValue({
+    jest.mocked(useCourseUserPermissions).mockReturnValue({
       isLoading: false,
-      permissions: { canViewGradingSettings: false, canEditGradingSettings: false },
+      canViewGradingSettings: false,
+      canEditGradingSettings: false,
     });
     render(<RootWrapper />);
     expect(await screen.findByTestId('permissionDeniedAlert')).toBeInTheDocument();
   });
 
   it('should disable inputs when user has view but not edit permission', async () => {
     mockWaffleFlags({ enableAuthzCourseAuthoring: true });
-    jest.mocked(useUserPermissionsWithAuthzCourse).mockReturnValue({
+    jest.mocked(useCourseUserPermissions).mockReturnValue({
       isLoading: false,
-      permissions: { canViewGradingSettings: true, canEditGradingSettings: false },
+      canViewGradingSettings: true,
+      canEditGradingSettings: false,
     });
     render(<RootWrapper />);
     const segmentInputs = await screen.findAllByTestId('grading-scale-segment-input');
@@ -192,9 +193,10 @@ describe('<GradingSettings /> permissions', () => {
 
   it('should disable save button when user lacks edit permission', async () => {
     mockWaffleFlags({ enableAuthzCourseAuthoring: true });
-    jest.mocked(useUserPermissionsWithAuthzCourse).mockReturnValue({
+    jest.mocked(useCourseUserPermissions).mockReturnValue({
       isLoading: false,
-      permissions: { canViewGradingSettings: true, canEditGradingSettings: false },
+      canViewGradingSettings: true,
+      canEditGradingSettings: false,
     });
     render(<RootWrapper />);
     const segmentInputs = await screen.findAllByTestId('grading-scale-segment-input');

src/grading-settings/grading-scale/components/GradingScaleSegment.tsx

@@ -18,7 +18,7 @@ interface GradingScaleSegmentProps {
   letters: [string];
   gradingSegments: RangeSegment[];
   removeGradingSegment: (idx: number) => void;
-  isEditable?: boolean,
+  isEditable?: boolean;
 }
 
 const GradingScaleSegment = ({