From fdd8aef6dc65f45391ab4f1461287fdc8badb6b4 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 3 Jul 2026 13:43:08 -0500 Subject: [PATCH 01/10] fix: hide course authoring content when enable_course_authoring is off --- src/authz-module/audit-user/index.tsx | 16 +++++- .../TableControlBar/RolesFilter.tsx | 9 ++- .../TableControlBar/ScopesFilter.test.tsx | 56 ++++++++++++++++--- .../TableControlBar/ScopesFilter.tsx | 14 ++++- src/authz-module/roles-permissions/index.ts | 5 ++ .../team-members/TeamMembersTable.tsx | 19 ++++++- 6 files changed, 102 insertions(+), 17 deletions(-) diff --git a/src/authz-module/audit-user/index.tsx b/src/authz-module/audit-user/index.tsx index 0619402a..8ff233ad 100644 --- a/src/authz-module/audit-user/index.tsx +++ b/src/authz-module/audit-user/index.tsx @@ -14,6 +14,7 @@ import { import AuthZLayout from '@src/authz-module/components/AuthZLayout'; import { useNavigate, useParams } from 'react-router-dom'; import { useUserAccount, useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; +import { CONTENT_COURSE_PERMISSIONS, VIEW_TEAM_PERMISSIONS, libraryRolesMetadata } from '@src/authz-module/roles-permissions'; import baseMessages from '@src/authz-module/messages'; import AddRoleButton from '@src/authz-module/components/AddRoleButton'; import { @@ -32,6 +33,8 @@ import messages from './messages'; import ConfirmDeletionModal from '../components/ConfirmDeletionModal'; import { getCellHeader, getScopeManageActionPermission } from '../utils'; +const LIBRARY_ROLE_KEYS = libraryRolesMetadata.map((r) => r.role).join(','); + const AuditUserPage = () => { const { formatMessage } = useIntl(); const [columnsWithFiltersApplied, setColumnsWithFiltersApplied] = useState([]); @@ -42,9 +45,20 @@ const AuditUserPage = () => { isLoading: isLoadingUser, data: user, isError: isErrorUser, error: errorUser, } = useUserAccount(username); const { querySettings, handleTableFetch } = useQuerySettings(); + + const { data: permissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); + const isCourseViewAllowed = permissions + ? permissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) + : true; + + const effectiveQuerySettings = useMemo(() => { + if (isCourseViewAllowed || querySettings.roles) { return querySettings; } + return { ...querySettings, roles: LIBRARY_ROLE_KEYS }; + }, [isCourseViewAllowed, querySettings]); + const { isLoading: isLoadingUserAssignments, data: { results: userAssignments, count } = { results: [], count: 0 }, - } = useUserAssignedRoles(username, querySettings); + } = useUserAssignedRoles(username, effectiveQuerySettings); const [roleToDelete, setRoleToDelete] = useState(null); const [showConfirmDeletionModal, setShowConfirmDeletionModal] = useState(false); const { diff --git a/src/authz-module/components/TableControlBar/RolesFilter.tsx b/src/authz-module/components/TableControlBar/RolesFilter.tsx index db66a8c2..da746461 100644 --- a/src/authz-module/components/TableControlBar/RolesFilter.tsx +++ b/src/authz-module/components/TableControlBar/RolesFilter.tsx @@ -2,7 +2,9 @@ import { useMemo } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Person } from '@openedx/paragon/icons'; import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; -import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS } from '@src/authz-module/roles-permissions'; +import { + CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS, VIEW_TEAM_PERMISSIONS, +} from '@src/authz-module/roles-permissions'; import { CONTEXT_TYPES } from '@src/authz-module/constants'; import MultipleChoiceFilter from './MultipleChoiceFilter'; import { MultipleChoiceFilterProps } from './types'; @@ -14,10 +16,7 @@ const RolesFilter = ({ filterButtonText, filterValue, setFilter, disabled, }: RolesFilterProps) => { const intl = useIntl(); - const { data: permissions } = useValidateUserPermissionsNonSuspense([ - { action: CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM }, - { action: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM }, - ]); + const { data: permissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); // Only show role groups for the domains the user can view. Global roles stay // hidden until a platform-wide permission is available to gate them on. diff --git a/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx b/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx index e2d0cbf0..a2ec6f3d 100644 --- a/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx +++ b/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx @@ -1,31 +1,47 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderWrapper } from '@src/setupTest'; +import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; +import { useScopes } from '@src/authz-module/data/hooks'; +import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS } from '@src/authz-module/roles-permissions'; import ScopesFilter from './ScopesFilter'; +jest.mock('@src/data/hooks', () => ({ + useValidateUserPermissionsNonSuspense: jest.fn(), +})); + +const mockUsePermissions = useValidateUserPermissionsNonSuspense as jest.Mock; + jest.mock('@src/authz-module/data/hooks', () => ({ - useScopes: () => ({ + useScopes: jest.fn(() => ({ data: { pages: [ { results: [ { - externalKey: 'course:123', - name: 'Test Course', - organization: { name: 'Test Org' }, + externalKey: 'course-v1:org+course+run', + displayName: 'Test Course', + org: { shortName: 'TestOrg' }, }, { - externalKey: 'library:456', - name: 'Test Library', - organization: { name: 'Another Org' }, + externalKey: 'lib:org:library', + displayName: 'Test Library', + org: { shortName: 'TestOrg' }, }, ], }, ], }, - }), + })), })); +const mockUseScopes = useScopes as jest.Mock; + +const permissionsData = ({ library, course }: { library?: boolean; course?: boolean }) => [ + { action: CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM, allowed: !!library }, + { action: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM, allowed: !!course }, +]; + describe('ScopesFilter', () => { const defaultProps = { filterButtonText: 'Scopes', @@ -36,6 +52,7 @@ describe('ScopesFilter', () => { beforeEach(() => { jest.clearAllMocks(); + mockUsePermissions.mockReturnValue({ data: permissionsData({ library: true, course: true }) }); }); it('renders without crashing', () => { @@ -68,4 +85,27 @@ describe('ScopesFilter', () => { renderWrapper(); expect(screen.getByText('Scopes')).toBeInTheDocument(); }); + + it('fetches all scope types when the user can view courses', () => { + renderWrapper(); + expect(mockUseScopes).toHaveBeenCalledWith( + expect.not.objectContaining({ scopeType: 'library' }), + ); + }); + + it('fetches only library scopes when the user cannot view courses', () => { + mockUsePermissions.mockReturnValue({ data: permissionsData({ library: true, course: false }) }); + renderWrapper(); + expect(mockUseScopes).toHaveBeenCalledWith( + expect.objectContaining({ scopeType: 'library' }), + ); + }); + + it('defaults to showing all scopes while permissions are loading', () => { + mockUsePermissions.mockReturnValue({ data: undefined }); + renderWrapper(); + expect(mockUseScopes).toHaveBeenCalledWith( + expect.not.objectContaining({ scopeType: 'library' }), + ); + }); }); diff --git a/src/authz-module/components/TableControlBar/ScopesFilter.tsx b/src/authz-module/components/TableControlBar/ScopesFilter.tsx index a93854a6..4de700a9 100644 --- a/src/authz-module/components/TableControlBar/ScopesFilter.tsx +++ b/src/authz-module/components/TableControlBar/ScopesFilter.tsx @@ -1,6 +1,8 @@ import { useMemo, useState } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { LocationOn } from '@openedx/paragon/icons'; +import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; +import { CONTENT_COURSE_PERMISSIONS, VIEW_TEAM_PERMISSIONS } from '@src/authz-module/roles-permissions'; import { useScopes } from '@src/authz-module/data/hooks'; import { DEFAULT_FILTER_PAGE_SIZE } from '@src/authz-module/constants'; import { MultipleChoiceFilterProps } from './types'; @@ -15,7 +17,17 @@ const ScopesFilter = ({ }: ScopesFilterProps) => { const { formatMessage } = useIntl(); const [searchValue, setSearchValue] = useState(undefined); - const { data: scopesData } = useScopes({ search: searchValue, pageSize: DEFAULT_FILTER_PAGE_SIZE }); + + const { data: permissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); + const isCourseViewAllowed = permissions + ? permissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) + : true; + + const { data: scopesData } = useScopes({ + search: searchValue, + pageSize: DEFAULT_FILTER_PAGE_SIZE, + ...(isCourseViewAllowed ? {} : { scopeType: 'library' }), + }); const filterChoices = useMemo(() => (scopesData?.pages?.flatMap((p) => p.results) ?? []).map((scope) => { const scopeIcon = scope.externalKey?.startsWith('lib') ? RESOURCE_ICONS.LIBRARY : RESOURCE_ICONS.COURSE; diff --git a/src/authz-module/roles-permissions/index.ts b/src/authz-module/roles-permissions/index.ts index 54ef22cc..c4723f54 100644 --- a/src/authz-module/roles-permissions/index.ts +++ b/src/authz-module/roles-permissions/index.ts @@ -21,3 +21,8 @@ export const MANAGE_TEAM_PERMISSIONS: { action: string }[] = [ { action: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM }, { action: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM }, ]; + +export const VIEW_TEAM_PERMISSIONS: { action: string }[] = [ + { action: CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM }, + { action: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM }, +]; diff --git a/src/authz-module/team-members/TeamMembersTable.tsx b/src/authz-module/team-members/TeamMembersTable.tsx index dfdbce56..c705db57 100644 --- a/src/authz-module/team-members/TeamMembersTable.tsx +++ b/src/authz-module/team-members/TeamMembersTable.tsx @@ -7,6 +7,8 @@ import { } from '@openedx/paragon'; import { useToastManager } from '@src/components/ToastManager/ToastManagerContext'; +import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; +import { CONTENT_COURSE_PERMISSIONS, VIEW_TEAM_PERMISSIONS, libraryRolesMetadata } from '@src/authz-module/roles-permissions'; import { useQuerySettings } from '@src/authz-module/hooks/useQuerySettings'; import OrgFilter from '@src/authz-module/components/TableControlBar/OrgFilter'; import RolesFilter from '@src/authz-module/components/TableControlBar/RolesFilter'; @@ -18,9 +20,12 @@ import { } from '@src/authz-module/components/TableCells'; import { useAllRoleAssignments } from '@src/authz-module/data/hooks'; import { TABLE_DEFAULT_PAGE_SIZE } from '@src/authz-module/constants'; +import { UserRole } from '@src/types'; import messages from './messages'; import TableFooter from '../components/TableFooter/TableFooter'; +const LIBRARY_ROLE_KEYS = libraryRolesMetadata.map((r) => r.role).join(','); + interface TeamMembersTableProps { presetScope?: string; } @@ -43,12 +48,22 @@ const TeamMembersTable = ({ presetScope }: TeamMembersTableProps) => { const { querySettings, handleTableFetch } = useQuerySettings(initialQuerySettings); + const { data: permissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); + const isCourseViewAllowed = permissions + ? permissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) + : true; + + const effectiveQuerySettings = useMemo(() => { + if (isCourseViewAllowed || querySettings.roles) { return querySettings; } + return { ...querySettings, roles: LIBRARY_ROLE_KEYS }; + }, [isCourseViewAllowed, querySettings]); + const { - data: { results: roleAssignments, count } = { results: [], count: 0 }, + data: { results: roleAssignments, count } = { results: [] as UserRole[], count: 0 }, isLoading: isLoadingAllRoleAssignments, error, refetch, - } = useAllRoleAssignments(querySettings); + } = useAllRoleAssignments(effectiveQuerySettings); const initialFilters = presetScope ? [{ id: 'scope', value: [presetScope] }] : []; From 537f02619f7c1b7f513851c6beb9f21a2a5915aa Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 3 Jul 2026 13:43:35 -0500 Subject: [PATCH 02/10] fix: enhance role assignment logic to conditionally hide course roles based on view permissions --- .../AssignRoleWizardPage.test.tsx | 48 ++++++++++++++++--- .../AssignRoleWizardPage.tsx | 15 ++++-- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx index 4197518d..63a953a8 100644 --- a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx +++ b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx @@ -49,6 +49,26 @@ const allowAllPermissions = { isLoading: false, }; +const mockPermissions = ( + manageData: typeof allowAllPermissions, + { courseViewAllowed = true } = {}, +) => (permissions: { action: string }[]) => { + const isViewCall = permissions.some( + (p) => p.action === CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM + || p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM, + ); + if (isViewCall) { + return { + data: [ + { action: CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM, allowed: true }, + { action: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM, allowed: courseViewAllowed }, + ], + isLoading: false, + }; + } + return manageData; +}; + const setupMocks = ({ users = '', from = '' } = {}) => { const { useSearchParams, useNavigate } = jest.requireMock('react-router-dom'); const params = new URLSearchParams(); @@ -73,7 +93,7 @@ describe('AssignRoleWizardPage', () => { mutateAsync: jest.fn(), isPending: false, }); - mockUseValidatePermissions.mockReturnValue(allowAllPermissions); + mockUseValidatePermissions.mockImplementation(mockPermissions(allowAllPermissions)); }); it('renders the page with the wizard and title', () => { @@ -147,10 +167,10 @@ describe('AssignRoleWizardPage', () => { }); it.each(scopeRoles)('shows only the roles for the allowed scope %#', ({ action, roles }) => { - mockUseValidatePermissions.mockReturnValue({ + mockUseValidatePermissions.mockImplementation(mockPermissions({ data: scopeRoles.map((scope) => ({ action: scope.action, allowed: scope.action === action })), isLoading: false, - }); + })); setupMocks(); renderPage(); @@ -165,10 +185,10 @@ describe('AssignRoleWizardPage', () => { }); it('shows no roles when no scope is allowed', () => { - mockUseValidatePermissions.mockReturnValue({ + mockUseValidatePermissions.mockImplementation(mockPermissions({ data: scopeRoles.map(({ action }) => ({ action, allowed: false })), isLoading: false, - }); + })); setupMocks(); renderPage(); allRoles.forEach((role) => { @@ -177,15 +197,29 @@ describe('AssignRoleWizardPage', () => { }); it('ignores allowed permissions whose action is not a known role scope', () => { - mockUseValidatePermissions.mockReturnValue({ + mockUseValidatePermissions.mockImplementation(mockPermissions({ data: [{ action: 'some.unrelated.permission', allowed: true }], isLoading: false, - }); + })); setupMocks(); renderPage(); allRoles.forEach((role) => { expect(screen.queryByText(role.name)).not.toBeInTheDocument(); }); }); + + it('hides course roles when VIEW_COURSE_TEAM is not allowed even if MANAGE_COURSE_TEAM is allowed', () => { + mockUseValidatePermissions.mockImplementation( + mockPermissions(allowAllPermissions, { courseViewAllowed: false }), + ); + setupMocks(); + renderPage(); + courseRolesMetadata.forEach((role) => { + expect(screen.queryByText(role.name)).not.toBeInTheDocument(); + }); + libraryRolesMetadata.forEach((role) => { + expect(screen.getByText(role.name)).toBeInTheDocument(); + }); + }); }); }); diff --git a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx index 58cc7f20..5685774d 100644 --- a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx +++ b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx @@ -7,7 +7,7 @@ import { ROUTES } from '../constants'; import messages from './messages'; import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS, courseRolesMetadata, libraryRolesMetadata, - MANAGE_TEAM_PERMISSIONS, + MANAGE_TEAM_PERMISSIONS, VIEW_TEAM_PERMISSIONS, } from '../roles-permissions'; const AssignRoleWizardPage = () => { @@ -23,12 +23,19 @@ const AssignRoleWizardPage = () => { ? `${ROUTES.HOME_PATH}/user/${presetUser}` : returnTo; - const { data: permissionValidationResponse } = useValidateUserPermissionsNonSuspense(MANAGE_TEAM_PERMISSIONS); + const { data: managePermissions } = useValidateUserPermissionsNonSuspense(MANAGE_TEAM_PERMISSIONS); + const { data: viewPermissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); - const rolesAssignable = permissionValidationResponse?.flatMap((p) => { + const isCourseViewAllowed = viewPermissions + ? viewPermissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) + : true; + + const rolesAssignable = managePermissions?.flatMap((p) => { if (!p.allowed) { return []; } if (p.action === CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM) { return libraryRolesMetadata; } - if (p.action === CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM) { return courseRolesMetadata; } + if (p.action === CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM) { + return isCourseViewAllowed ? courseRolesMetadata : []; + } return []; }); From b049a3795792e7233821c5958d78ee1eabd7e91c Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 3 Jul 2026 13:44:02 -0500 Subject: [PATCH 03/10] fix: refactor permission handling to use custom hook for team view permissions --- src/authz-module/audit-user/index.tsx | 10 ++---- .../TableControlBar/RolesFilter.test.tsx | 2 +- .../TableControlBar/RolesFilter.tsx | 29 +++++---------- .../TableControlBar/ScopesFilter.tsx | 8 ++--- .../hooks/useViewTeamPermissions.ts | 18 ++++++++++ .../AssignRoleWizardPage.tsx | 9 ++--- src/authz-module/roles-permissions/index.ts | 4 ++- .../team-members/TeamMembersTable.test.tsx | 35 +++++++++++++++++++ .../team-members/TeamMembersTable.tsx | 11 ++---- 9 files changed, 77 insertions(+), 49 deletions(-) create mode 100644 src/authz-module/hooks/useViewTeamPermissions.ts diff --git a/src/authz-module/audit-user/index.tsx b/src/authz-module/audit-user/index.tsx index 8ff233ad..b5aaa221 100644 --- a/src/authz-module/audit-user/index.tsx +++ b/src/authz-module/audit-user/index.tsx @@ -14,7 +14,8 @@ import { import AuthZLayout from '@src/authz-module/components/AuthZLayout'; import { useNavigate, useParams } from 'react-router-dom'; import { useUserAccount, useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; -import { CONTENT_COURSE_PERMISSIONS, VIEW_TEAM_PERMISSIONS, libraryRolesMetadata } from '@src/authz-module/roles-permissions'; +import { LIBRARY_ROLE_KEYS } from '@src/authz-module/roles-permissions'; +import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; import baseMessages from '@src/authz-module/messages'; import AddRoleButton from '@src/authz-module/components/AddRoleButton'; import { @@ -33,8 +34,6 @@ import messages from './messages'; import ConfirmDeletionModal from '../components/ConfirmDeletionModal'; import { getCellHeader, getScopeManageActionPermission } from '../utils'; -const LIBRARY_ROLE_KEYS = libraryRolesMetadata.map((r) => r.role).join(','); - const AuditUserPage = () => { const { formatMessage } = useIntl(); const [columnsWithFiltersApplied, setColumnsWithFiltersApplied] = useState([]); @@ -46,10 +45,7 @@ const AuditUserPage = () => { } = useUserAccount(username); const { querySettings, handleTableFetch } = useQuerySettings(); - const { data: permissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); - const isCourseViewAllowed = permissions - ? permissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) - : true; + const { isCourseViewAllowed } = useViewTeamPermissions(); const effectiveQuerySettings = useMemo(() => { if (isCourseViewAllowed || querySettings.roles) { return querySettings; } diff --git a/src/authz-module/components/TableControlBar/RolesFilter.test.tsx b/src/authz-module/components/TableControlBar/RolesFilter.test.tsx index a4be6eb1..d0f9be3b 100644 --- a/src/authz-module/components/TableControlBar/RolesFilter.test.tsx +++ b/src/authz-module/components/TableControlBar/RolesFilter.test.tsx @@ -91,7 +91,7 @@ describe('RolesFilter', () => { it('shows no role options while permissions are still loading', async () => { const user = userEvent.setup(); - mockUsePermissions.mockReturnValue({ data: undefined }); + mockUsePermissions.mockReturnValue({ data: undefined, isLoading: true }); renderWrapper(); const menu = await openDropdown(user); expect(menu.queryByText('Courses')).not.toBeInTheDocument(); diff --git a/src/authz-module/components/TableControlBar/RolesFilter.tsx b/src/authz-module/components/TableControlBar/RolesFilter.tsx index da746461..95c6bd03 100644 --- a/src/authz-module/components/TableControlBar/RolesFilter.tsx +++ b/src/authz-module/components/TableControlBar/RolesFilter.tsx @@ -1,10 +1,7 @@ import { useMemo } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Person } from '@openedx/paragon/icons'; -import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; -import { - CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS, VIEW_TEAM_PERMISSIONS, -} from '@src/authz-module/roles-permissions'; +import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; import { CONTEXT_TYPES } from '@src/authz-module/constants'; import MultipleChoiceFilter from './MultipleChoiceFilter'; import { MultipleChoiceFilterProps } from './types'; @@ -16,24 +13,16 @@ const RolesFilter = ({ filterButtonText, filterValue, setFilter, disabled, }: RolesFilterProps) => { const intl = useIntl(); - const { data: permissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); + const { isCourseViewAllowed, isLibraryViewAllowed, isLoading } = useViewTeamPermissions(); - // Only show role groups for the domains the user can view. Global roles stay - // hidden until a platform-wide permission is available to gate them on. - const allowedContexts = useMemo(() => { - const contexts = new Set(); - permissions?.forEach((p) => { - if (!p.allowed) { return; } - if (p.action === CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM) { contexts.add(CONTEXT_TYPES.LIBRARY); } - if (p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM) { contexts.add(CONTEXT_TYPES.COURSE); } + const rolesOptions = useMemo(() => { + if (isLoading) { return []; } + return getRolesFiltersOptions(intl).filter((option) => { + if (option.contextType === CONTEXT_TYPES.COURSE) { return isCourseViewAllowed; } + if (option.contextType === CONTEXT_TYPES.LIBRARY) { return isLibraryViewAllowed; } + return false; }); - return contexts; - }, [permissions]); - - const rolesOptions = useMemo( - () => getRolesFiltersOptions(intl).filter((option) => allowedContexts.has(option.contextType)), - [intl, allowedContexts], - ); + }, [intl, isCourseViewAllowed, isLibraryViewAllowed, isLoading]); return ( (undefined); - const { data: permissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); - const isCourseViewAllowed = permissions - ? permissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) - : true; + const { isCourseViewAllowed } = useViewTeamPermissions(); const { data: scopesData } = useScopes({ search: searchValue, diff --git a/src/authz-module/hooks/useViewTeamPermissions.ts b/src/authz-module/hooks/useViewTeamPermissions.ts new file mode 100644 index 00000000..00d94674 --- /dev/null +++ b/src/authz-module/hooks/useViewTeamPermissions.ts @@ -0,0 +1,18 @@ +import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; +import { + CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS, VIEW_TEAM_PERMISSIONS, +} from '@src/authz-module/roles-permissions'; + +export const useViewTeamPermissions = () => { + const { data: permissions, isLoading } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); + + const isCourseViewAllowed = permissions + ? permissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) + : true; + + const isLibraryViewAllowed = permissions + ? permissions.some((p) => p.action === CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM && p.allowed) + : true; + + return { isCourseViewAllowed, isLibraryViewAllowed, isLoading }; +}; diff --git a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx index 5685774d..a5bd050e 100644 --- a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx +++ b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx @@ -7,8 +7,9 @@ import { ROUTES } from '../constants'; import messages from './messages'; import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS, courseRolesMetadata, libraryRolesMetadata, - MANAGE_TEAM_PERMISSIONS, VIEW_TEAM_PERMISSIONS, + MANAGE_TEAM_PERMISSIONS, } from '../roles-permissions'; +import { useViewTeamPermissions } from '../hooks/useViewTeamPermissions'; const AssignRoleWizardPage = () => { const intl = useIntl(); @@ -24,11 +25,7 @@ const AssignRoleWizardPage = () => { : returnTo; const { data: managePermissions } = useValidateUserPermissionsNonSuspense(MANAGE_TEAM_PERMISSIONS); - const { data: viewPermissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); - - const isCourseViewAllowed = viewPermissions - ? viewPermissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) - : true; + const { isCourseViewAllowed } = useViewTeamPermissions(); const rolesAssignable = managePermissions?.flatMap((p) => { if (!p.allowed) { return []; } diff --git a/src/authz-module/roles-permissions/index.ts b/src/authz-module/roles-permissions/index.ts index c4723f54..982a59e3 100644 --- a/src/authz-module/roles-permissions/index.ts +++ b/src/authz-module/roles-permissions/index.ts @@ -1,5 +1,5 @@ import { CONTENT_COURSE_PERMISSIONS } from './course/constants'; -import { CONTENT_LIBRARY_PERMISSIONS } from './library/constants'; +import { CONTENT_LIBRARY_PERMISSIONS, libraryRolesMetadata as _libraryRolesMetadata } from './library/constants'; export { CONTENT_LIBRARY_PERMISSIONS, @@ -9,6 +9,8 @@ export { rolesLibraryObject, } from './library/constants'; +export const LIBRARY_ROLE_KEYS = _libraryRolesMetadata.map((r) => r.role).join(','); + export { CONTENT_COURSE_PERMISSIONS, courseResourceTypes, diff --git a/src/authz-module/team-members/TeamMembersTable.test.tsx b/src/authz-module/team-members/TeamMembersTable.test.tsx index ba3aecfa..9f9cd834 100644 --- a/src/authz-module/team-members/TeamMembersTable.test.tsx +++ b/src/authz-module/team-members/TeamMembersTable.test.tsx @@ -2,9 +2,17 @@ import { screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderWithAllProviders } from '@src/setupTest'; import { useAllRoleAssignments, useOrgs, useScopes } from '@src/authz-module/data/hooks'; +import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; +import { LIBRARY_ROLE_KEYS } from '@src/authz-module/roles-permissions'; import { ToastManagerProvider } from '@src/components/ToastManager/ToastManagerContext'; import TeamMembersTable from './TeamMembersTable'; +jest.mock('@src/authz-module/hooks/useViewTeamPermissions', () => ({ + useViewTeamPermissions: jest.fn(), +})); + +const mockUseViewTeamPermissions = useViewTeamPermissions as jest.Mock; + const mockedAllRoleAssignments = { data: { results: [ @@ -127,6 +135,11 @@ const mockApiResponses = ( describe('TeamMembersTable', () => { beforeEach(() => { mockNavigate.mockClear(); + mockUseViewTeamPermissions.mockReturnValue({ + isCourseViewAllowed: true, + isLibraryViewAllowed: true, + isLoading: false, + }); }); it('renders table with role assignments data', async () => { @@ -194,6 +207,28 @@ describe('TeamMembersTable', () => { expect(mockNavigate).toHaveBeenCalledWith('/authz/user/johndoe'); }); + it('renders safely when role assignments data is undefined', () => { + // @ts-ignore + mockApiResponses({ ...mockedAllRoleAssignments, data: undefined }); + renderWithAllProviders(); + expect(screen.queryByText('John Doe')).not.toBeInTheDocument(); + }); + + it('filters to library roles only when course view is not allowed', async () => { + mockUseViewTeamPermissions.mockReturnValue({ + isCourseViewAllowed: false, + isLibraryViewAllowed: true, + isLoading: false, + }); + mockApiResponses(); + renderWithAllProviders(); + await waitFor(() => { + expect(useAllRoleAssignments).toHaveBeenCalledWith( + expect.objectContaining({ roles: LIBRARY_ROLE_KEYS }), + ); + }); + }); + it('handles empty data gracefully', async () => { const allAsignmentsResponse = { data: { diff --git a/src/authz-module/team-members/TeamMembersTable.tsx b/src/authz-module/team-members/TeamMembersTable.tsx index c705db57..b03fcd3b 100644 --- a/src/authz-module/team-members/TeamMembersTable.tsx +++ b/src/authz-module/team-members/TeamMembersTable.tsx @@ -7,8 +7,8 @@ import { } from '@openedx/paragon'; import { useToastManager } from '@src/components/ToastManager/ToastManagerContext'; -import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; -import { CONTENT_COURSE_PERMISSIONS, VIEW_TEAM_PERMISSIONS, libraryRolesMetadata } from '@src/authz-module/roles-permissions'; +import { LIBRARY_ROLE_KEYS } from '@src/authz-module/roles-permissions'; +import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; import { useQuerySettings } from '@src/authz-module/hooks/useQuerySettings'; import OrgFilter from '@src/authz-module/components/TableControlBar/OrgFilter'; import RolesFilter from '@src/authz-module/components/TableControlBar/RolesFilter'; @@ -24,8 +24,6 @@ import { UserRole } from '@src/types'; import messages from './messages'; import TableFooter from '../components/TableFooter/TableFooter'; -const LIBRARY_ROLE_KEYS = libraryRolesMetadata.map((r) => r.role).join(','); - interface TeamMembersTableProps { presetScope?: string; } @@ -48,10 +46,7 @@ const TeamMembersTable = ({ presetScope }: TeamMembersTableProps) => { const { querySettings, handleTableFetch } = useQuerySettings(initialQuerySettings); - const { data: permissions } = useValidateUserPermissionsNonSuspense(VIEW_TEAM_PERMISSIONS); - const isCourseViewAllowed = permissions - ? permissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) - : true; + const { isCourseViewAllowed } = useViewTeamPermissions(); const effectiveQuerySettings = useMemo(() => { if (isCourseViewAllowed || querySettings.roles) { return querySettings; } From 92351b07e3b32267d0d07a16fa730feace5150dc Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 3 Jul 2026 13:54:51 -0500 Subject: [PATCH 04/10] refactor: address feedback --- .../components/TableControlBar/ScopesFilter.test.tsx | 6 +++--- src/authz-module/hooks/useViewTeamPermissions.ts | 4 ++-- src/authz-module/team-members/TeamMembersTable.test.tsx | 9 +++++++-- src/authz-module/team-members/TeamMembersTable.tsx | 3 +-- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx b/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx index a2ec6f3d..95eaac5e 100644 --- a/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx +++ b/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx @@ -101,11 +101,11 @@ describe('ScopesFilter', () => { ); }); - it('defaults to showing all scopes while permissions are loading', () => { - mockUsePermissions.mockReturnValue({ data: undefined }); + it('defaults to showing only library scopes while permissions are loading', () => { + mockUsePermissions.mockReturnValue({ data: undefined, isLoading: true }); renderWrapper(); expect(mockUseScopes).toHaveBeenCalledWith( - expect.not.objectContaining({ scopeType: 'library' }), + expect.objectContaining({ scopeType: 'library' }), ); }); }); diff --git a/src/authz-module/hooks/useViewTeamPermissions.ts b/src/authz-module/hooks/useViewTeamPermissions.ts index 00d94674..84b73f4c 100644 --- a/src/authz-module/hooks/useViewTeamPermissions.ts +++ b/src/authz-module/hooks/useViewTeamPermissions.ts @@ -8,11 +8,11 @@ export const useViewTeamPermissions = () => { const isCourseViewAllowed = permissions ? permissions.some((p) => p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM && p.allowed) - : true; + : false; const isLibraryViewAllowed = permissions ? permissions.some((p) => p.action === CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM && p.allowed) - : true; + : false; return { isCourseViewAllowed, isLibraryViewAllowed, isLoading }; }; diff --git a/src/authz-module/team-members/TeamMembersTable.test.tsx b/src/authz-module/team-members/TeamMembersTable.test.tsx index 9f9cd834..7b873d12 100644 --- a/src/authz-module/team-members/TeamMembersTable.test.tsx +++ b/src/authz-module/team-members/TeamMembersTable.test.tsx @@ -2,6 +2,7 @@ import { screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderWithAllProviders } from '@src/setupTest'; import { useAllRoleAssignments, useOrgs, useScopes } from '@src/authz-module/data/hooks'; +import type { GetAllRoleAssignmentsResponse } from '@src/authz-module/data/api'; import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; import { LIBRARY_ROLE_KEYS } from '@src/authz-module/roles-permissions'; import { ToastManagerProvider } from '@src/components/ToastManager/ToastManagerContext'; @@ -13,7 +14,12 @@ jest.mock('@src/authz-module/hooks/useViewTeamPermissions', () => ({ const mockUseViewTeamPermissions = useViewTeamPermissions as jest.Mock; -const mockedAllRoleAssignments = { +const mockedAllRoleAssignments: { + data: GetAllRoleAssignmentsResponse | undefined; + error: Error | null; + isLoading: boolean; + refetch: jest.Mock; +} = { data: { results: [ { @@ -208,7 +214,6 @@ describe('TeamMembersTable', () => { }); it('renders safely when role assignments data is undefined', () => { - // @ts-ignore mockApiResponses({ ...mockedAllRoleAssignments, data: undefined }); renderWithAllProviders(); expect(screen.queryByText('John Doe')).not.toBeInTheDocument(); diff --git a/src/authz-module/team-members/TeamMembersTable.tsx b/src/authz-module/team-members/TeamMembersTable.tsx index b03fcd3b..a683e0ff 100644 --- a/src/authz-module/team-members/TeamMembersTable.tsx +++ b/src/authz-module/team-members/TeamMembersTable.tsx @@ -20,7 +20,6 @@ import { } from '@src/authz-module/components/TableCells'; import { useAllRoleAssignments } from '@src/authz-module/data/hooks'; import { TABLE_DEFAULT_PAGE_SIZE } from '@src/authz-module/constants'; -import { UserRole } from '@src/types'; import messages from './messages'; import TableFooter from '../components/TableFooter/TableFooter'; @@ -54,7 +53,7 @@ const TeamMembersTable = ({ presetScope }: TeamMembersTableProps) => { }, [isCourseViewAllowed, querySettings]); const { - data: { results: roleAssignments, count } = { results: [] as UserRole[], count: 0 }, + data: { results: roleAssignments, count } = { results: [], count: 0 }, isLoading: isLoadingAllRoleAssignments, error, refetch, From dc6d53eac6c11345e12cbeb96d3e861ea54b1f35 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Wed, 15 Jul 2026 17:50:18 +1000 Subject: [PATCH 05/10] refactor: extend course with waffle validation --- ...0003-course-authoring-flag-enforcement.rst | 206 ++++++++++++++++++ src/authz-module/audit-user/index.test.tsx | 17 ++ src/authz-module/authz-home/index.test.tsx | 8 + .../TableControlBar/OrgFilter.test.tsx | 84 ++++++- .../components/TableControlBar/OrgFilter.tsx | 20 +- .../TableControlBar/RolesFilter.test.tsx | 38 ++++ .../TableControlBar/RolesFilter.tsx | 9 +- .../TableControlBar/ScopesFilter.test.tsx | 32 +++ .../TableControlBar/ScopesFilter.tsx | 33 +-- .../TableControlBar/TableControlBar.test.tsx | 8 + src/authz-module/data/api.test.tsx | 26 +++ src/authz-module/data/api.ts | 21 ++ src/authz-module/data/hooks.ts | 19 ++ .../hooks/useCourseAuthoringFlag.test.tsx | 137 ++++++++++++ .../hooks/useCourseAuthoringFlag.ts | 85 ++++++++ .../AssignRoleWizard.test.tsx | 17 ++ .../AssignRoleWizardPage.test.tsx | 39 ++-- .../AssignRoleWizardPage.tsx | 7 +- .../DefineApplicationScopeStep.test.tsx | 17 ++ .../components/ScopeList.tsx | 4 +- .../hooks/useScopeListData.test.ts | 98 +++++++++ .../hooks/useScopeListData.ts | 27 ++- .../team-members/TeamMembersTable.test.tsx | 8 + 23 files changed, 906 insertions(+), 54 deletions(-) create mode 100644 docs/decisions/0003-course-authoring-flag-enforcement.rst create mode 100644 src/authz-module/hooks/useCourseAuthoringFlag.test.tsx create mode 100644 src/authz-module/hooks/useCourseAuthoringFlag.ts diff --git a/docs/decisions/0003-course-authoring-flag-enforcement.rst b/docs/decisions/0003-course-authoring-flag-enforcement.rst new file mode 100644 index 00000000..0874cabb --- /dev/null +++ b/docs/decisions/0003-course-authoring-flag-enforcement.rst @@ -0,0 +1,206 @@ +3. Course Authoring Flag Enforcement in the Admin Console +--------------------------------------------------------- + +Status +###### + +Proposed + +.. note:: + + This is a **temporary** measure for the flag-gated rollout period. It is + expected to be removed and re-evaluated once the authz system is enabled by + default and `authz.enable_course_authoring` is deprecated (see `ADR 0010`_). + See *Temporary nature and future work* below. + +Context +####### + +The authorization backend (`openedx-authz`) is rolling out the course +authoring domain behind the Waffle flag +`authz.enable_course_authoring`. The flag can be enabled globally +(instance-wide), per organization, or per course. + +The rollout is defined by the following backend decisions: + +* `ADR 0010 - Course Authoring Flag`_ introduces the multi-level Waffle flag. +* `ADR 0013 - Course Authoring Automatic Migration`_ makes the flag the source + of truth. When the flag changes for a scope, role assignments migrate between + the legacy `CourseAccessRole` model and the new authz (Casbin) system. +* `ADR 0007 - Enforcement Mechanisms (MFEs)`_ establishes that frontends enforce + authorization by querying the backend rather than deriving policy from tokens. + +The Admin Console manages role assignments for two authorization domains: + +* **Content libraries**, which are always enabled. +* **Course authoring**, which is gated by the feature flag. + +When course authoring is disabled for a scope, the UI must not expose that +domain. Specifically: + +* The **Scope** filter must not list courses where authoring is disabled. +* The **Role** filter must not list course-authoring roles when the domain is + disabled. +* The **Organization** filter must not hide organizations that are reachable + through libraries, but it must exclude organizations that expose only disabled + course authoring. +* The **assignment lists** must not display users or assignments that belong + only to course authoring. Users with both library and authoring roles should + display only their library roles. + +Because the flag is evaluated per scope, a single instance-wide boolean is +insufficient. For example, the global flag may be disabled while a specific +organization or course is enabled, and vice versa. The frontend must therefore +resolve enablement for the specific scope it is rendering. + +Enablement must also remain independent of permission validation. + +A natural implementation would infer whether course authoring is enabled from +`/permissions/validate/me`. However, permission validation reflects Casbin role +assignments, while enablement reflects the current Waffle flag. These sources can +temporarily diverge. + +As described in `ADR 0013`_, Casbin assignments exist only after migration has +been executed. Automatic migration is disabled by default +(`ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION = False`). Even when +enabled, only organization- and course-level changes migrate automatically; +global flag changes still require manual migration commands. + +Consequently, a runtime change to the global flag may not be reflected in the +permission data returned by the authorization APIs. During rollout, the frontend +must therefore determine whether the course authoring domain should be displayed +from the flag state itself rather than from permission validation. + +To support this, the backend exposes +`GET /api/authz/v1/waffle-flag-states/`:: + + + { + "global": false, + "org_overrides": { "on": ["Demo"], "off": [] }, + "course_overrides": { + "on": [], + "off": ["course-v1:testing+CT01+CT01-2024"] + } + } + + +The endpoint returns the global flag value together with only those +organization and course overrides whose value differs from the global flag. +Because overrides are separated into `on` and `off` lists, each scope's +effective value can be resolved without ambiguity while preserving override +precedence. + +Decision +######## + +**1. Consume flag state through a single cached hook.** + +The frontend exposes `getCourseAuthoringFlagStates` and the React Query hook +`useCourseAuthoringFlagStates` to fetch the endpoint. The hook uses the same +React Query configuration as the permission-validation hooks to keep caching +behavior consistent. + +**2. Centralize enablement resolution.** + +A derived hook, `useCourseAuthoringFlag`, encapsulates all resolution logic. +Components consume only its public API: + +* `isCourseAuthoringEnabled` — returns whether the authoring domain is enabled + anywhere (globally or through any `on` override). This is used for coarse + domain-level gating. +* `isCourseEnabled(courseId)` — resolves enablement using precedence: + **course override -> organization override -> global**. +* `isOrgAuthoringEnabled(org)` — returns `true` when the organization is + explicitly enabled, contains at least one enabled course, or inherits a global + `on` value without an explicit organization `off` override. + +While flag states are loading, all resolvers return `false` so course +authoring remains hidden until enablement is known. + +**3. Apply gating only to client-owned UI elements.** + +* **Role filter:** course-authoring roles require both the corresponding view + permission and `isCourseAuthoringEnabled`. +* **Scope filter:** course options are filtered with `isCourseEnabled`; + library scopes are always included. +* **Organization filter:** organizations are filtered with + `isOrgAuthoringEnabled` only for users without library-view permissions. + Users with library access—and all users while permissions are loading—see all + organizations. +* **Assignment wizard:** assignable course roles require both the appropriate + management permission and `isCourseAuthoringEnabled`. + +**4. Leave paginated assignment data to the backend.** + +The role, scope, and organization filters operate on small client-owned data +sets, making client-side filtering appropriate. + +The assignment lists (`/assignments/` and the per-user assignment view) are +server-driven and paginated. Client-side filtering would invalidate pagination, +counts, and sorting. The backend therefore remains responsible for hiding +authoring-only assignments. As described in `ADR 0013`_, only authoring +assignments for enabled scopes are expected to exist in Casbin, so the frontend +renders the backend response without additional filtering. + +Implications and Assumptions +############################ + +Behavior depends on +`ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION` (`ADR 0013`_). + +**When the setting is enabled (`True`)** + +* Organization- and course-level flag changes synchronously migrate role + assignments between the legacy model and Casbin, keeping authorization data + largely aligned with the flag. +* Global flag changes are **not** migrated automatically. The flag-state + endpoint immediately reflects the new value, but Casbin data does not until + migration commands are executed. +* Migration may fail or complete only partially (`ADR 0013`_), creating + temporary inconsistencies between flag state and authorization data. + +**When the setting is disabled (default, `False`)** + +* Flag changes never migrate authorization data automatically. +* The flag-state endpoint reflects runtime configuration, while Casbin continues + to reflect the most recent manual migration. +* The frontend uses the flag only to determine which UI controls are available. + Paginated assignment lists continue to reflect Casbin and may therefore appear + stale. This inconsistency cannot be resolved by the frontend. + +Assumptions +*********** + +* `waffle-flag-states` is the authoritative runtime source of enablement. +* Permission validation and assignment lists reflect Casbin state, which may lag + behind the current flag configuration. +* Content libraries are never gated by this flag. +* The frontend mirrors the backend's course-key parsing and override precedence + (course -> organization -> global). + +Temporary nature and future work +################################ + +This enforcement exists only for the feature-flag rollout period. It is required +because enablement can temporarily diverge from migrated authorization data. + +Once the authz system is enabled by default and +`authz.enable_course_authoring` is deprecated (`ADR 0010`_), enablement will +be unconditional. At that point, the +`waffle-flag-states` endpoint, `useCourseAuthoringFlag`, and all +feature-flag-based UI gating should be removed, allowing the console to rely +solely on permission validation. + + +## References + +* `ADR 0010 - Course Authoring Flag`_ +* `ADR 0013 - Course Authoring Automatic Migration`_ +* `ADR 0007 - Enforcement Mechanisms (MFEs)`_ + +.. _ADR 0010 - Course Authoring Flag: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0010-course-authoring-flag.rst +.. _ADR 0010: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0010-course-authoring-flag.rst +.. _ADR 0013 - Course Authoring Automatic Migration: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0013-course-authoring-automatic-migration.rst +.. _ADR 0013: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0013-course-authoring-automatic-migration.rst +.. _ADR 0007 - Enforcement Mechanisms (MFEs): https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0007-enforcement-mechanisms-mfe.rst diff --git a/src/authz-module/audit-user/index.test.tsx b/src/authz-module/audit-user/index.test.tsx index f535ece4..7fad65d7 100644 --- a/src/authz-module/audit-user/index.test.tsx +++ b/src/authz-module/audit-user/index.test.tsx @@ -38,6 +38,23 @@ jest.mock('@src/data/hooks', () => ({ // Mock the useRevokeUserRoles hook const mockRevokeUserRoles = jest.fn(); +jest.mock('@src/authz-module/hooks/useViewTeamPermissions', () => ({ + useViewTeamPermissions: () => ({ + isCourseViewAllowed: true, + isLibraryViewAllowed: true, + isLoading: false, + }), +})); + +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: () => ({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isOrgAuthoringEnabled: () => true, + isLoading: false, + }), +})); + jest.mock('@src/authz-module/data/hooks', () => ({ ...jest.requireActual('@src/authz-module/data/hooks'), useRevokeUserRoles: () => ({ diff --git a/src/authz-module/authz-home/index.test.tsx b/src/authz-module/authz-home/index.test.tsx index 8ddb952b..b22789b6 100644 --- a/src/authz-module/authz-home/index.test.tsx +++ b/src/authz-module/authz-home/index.test.tsx @@ -6,6 +6,14 @@ import { ToastManagerProvider } from '@src/components/ToastManager/ToastManagerC import AuthzHome from './index'; import messages from './messages'; +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: () => ({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isLoading: false, + }), +})); + jest.mock('@src/authz-module/data/hooks', () => ({ useAllRoleAssignments: jest.fn(), useOrgs: jest.fn(), diff --git a/src/authz-module/components/TableControlBar/OrgFilter.test.tsx b/src/authz-module/components/TableControlBar/OrgFilter.test.tsx index 228b9d69..6ef477ce 100644 --- a/src/authz-module/components/TableControlBar/OrgFilter.test.tsx +++ b/src/authz-module/components/TableControlBar/OrgFilter.test.tsx @@ -1,6 +1,8 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderWrapper } from '@src/setupTest'; +import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import OrgFilter from './OrgFilter'; jest.mock('@src/authz-module/data/hooks', () => ({ @@ -10,13 +12,24 @@ jest.mock('@src/authz-module/data/hooks', () => ({ next: null, previous: null, results: [ - { id: 'org1', name: 'Organization 1' }, - { id: 'org2', name: 'Organization 2' }, + { id: 'org1', name: 'Organization 1', shortName: 'Org1' }, + { id: 'org2', name: 'Organization 2', shortName: 'Org2' }, ], }, }), })); +jest.mock('@src/authz-module/hooks/useViewTeamPermissions', () => ({ + useViewTeamPermissions: jest.fn(), +})); + +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: jest.fn(), +})); + +const mockUseViewTeamPermissions = useViewTeamPermissions as jest.Mock; +const mockUseCourseAuthoringFlag = useCourseAuthoringFlag as jest.Mock; + describe('OrgFilter', () => { const defaultProps = { filterButtonText: 'Organizations', @@ -25,8 +38,23 @@ describe('OrgFilter', () => { disabled: false, }; + const openDropdown = async (user: ReturnType) => { + await user.click(screen.getByRole('button', { name: /Organizations/i })); + }; + beforeEach(() => { jest.clearAllMocks(); + mockUseViewTeamPermissions.mockReturnValue({ + isCourseViewAllowed: true, + isLibraryViewAllowed: true, + isLoading: false, + }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isOrgAuthoringEnabled: () => true, + isLoading: false, + }); }); it('renders without crashing', () => { @@ -60,4 +88,56 @@ describe('OrgFilter', () => { renderWrapper(); expect(screen.getByText('Organizations')).toBeInTheDocument(); }); + + it('keeps all orgs for users with library access even when authoring is disabled', async () => { + const user = userEvent.setup(); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: false, + isCourseEnabled: () => false, + isOrgAuthoringEnabled: () => false, + isLoading: false, + }); + renderWrapper(); + await openDropdown(user); + expect(await screen.findByText('Organization 1')).toBeInTheDocument(); + expect(screen.getByText('Organization 2')).toBeInTheDocument(); + }); + + it('hides authoring-disabled orgs for course-only users', async () => { + const user = userEvent.setup(); + mockUseViewTeamPermissions.mockReturnValue({ + isCourseViewAllowed: true, + isLibraryViewAllowed: false, + isLoading: false, + }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isOrgAuthoringEnabled: (org: string) => org === 'Org1', + isLoading: false, + }); + renderWrapper(); + await openDropdown(user); + expect(await screen.findByText('Organization 1')).toBeInTheDocument(); + expect(screen.queryByText('Organization 2')).not.toBeInTheDocument(); + }); + + it('keeps all orgs while permissions are still loading', async () => { + const user = userEvent.setup(); + mockUseViewTeamPermissions.mockReturnValue({ + isCourseViewAllowed: false, + isLibraryViewAllowed: false, + isLoading: true, + }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: false, + isCourseEnabled: () => false, + isOrgAuthoringEnabled: () => false, + isLoading: false, + }); + renderWrapper(); + await openDropdown(user); + expect(await screen.findByText('Organization 1')).toBeInTheDocument(); + expect(screen.getByText('Organization 2')).toBeInTheDocument(); + }); }); diff --git a/src/authz-module/components/TableControlBar/OrgFilter.tsx b/src/authz-module/components/TableControlBar/OrgFilter.tsx index cc0cd52e..f6e115b0 100644 --- a/src/authz-module/components/TableControlBar/OrgFilter.tsx +++ b/src/authz-module/components/TableControlBar/OrgFilter.tsx @@ -1,6 +1,8 @@ import React, { useMemo } from 'react'; import { Business } from '@openedx/paragon/icons'; import { useOrgs } from '@src/authz-module/data/hooks'; +import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import { DEFAULT_FILTER_PAGE_SIZE } from '@src/authz-module/constants'; import { MultipleChoiceFilterProps } from './types'; import MultipleChoiceFilter from './MultipleChoiceFilter'; @@ -11,15 +13,25 @@ const OrgFilter = ({ filterButtonText, filterValue, setFilter, disabled, }: OrgFilterProps) => { const [searchValue, setSearchValue] = React.useState(undefined); + const { isLibraryViewAllowed, isLoading } = useViewTeamPermissions(); + const { isOrgAuthoringEnabled, isLoading: isFlagLoading } = useCourseAuthoringFlag(); const { data: orgsData = { count: 0, next: null, previous: null, results: [], }, } = useOrgs(searchValue, 1, DEFAULT_FILTER_PAGE_SIZE); - const filterChoices = useMemo(() => orgsData?.results?.map((org) => ({ - displayName: org.name, - value: org.shortName, - })) || [], [orgsData]); + + // Libraries span orgs and are always enabled, so they must keep their behavior: only + // filter orgs by the course-authoring flag for course-only users, and never while + // permissions or flag states are still loading (default to showing every org). + const filterByAuthoringFlag = !isLoading && !isFlagLoading && !isLibraryViewAllowed; + + const filterChoices = useMemo(() => (orgsData?.results ?? []) + .filter((org) => !filterByAuthoringFlag || isOrgAuthoringEnabled(org.shortName)) + .map((org) => ({ + displayName: org.name, + value: org.shortName, + })), [orgsData, filterByAuthoringFlag, isOrgAuthoringEnabled]); const handleSearchChange = (value: string) => { setSearchValue(value); diff --git a/src/authz-module/components/TableControlBar/RolesFilter.test.tsx b/src/authz-module/components/TableControlBar/RolesFilter.test.tsx index d0f9be3b..98d34c92 100644 --- a/src/authz-module/components/TableControlBar/RolesFilter.test.tsx +++ b/src/authz-module/components/TableControlBar/RolesFilter.test.tsx @@ -2,6 +2,7 @@ import { screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderWrapper } from '@src/setupTest'; import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS } from '@src/authz-module/roles-permissions'; import RolesFilter from './RolesFilter'; @@ -9,7 +10,12 @@ jest.mock('@src/data/hooks', () => ({ useValidateUserPermissionsNonSuspense: jest.fn(), })); +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: jest.fn(), +})); + const mockUsePermissions = useValidateUserPermissionsNonSuspense as jest.Mock; +const mockUseCourseAuthoringFlag = useCourseAuthoringFlag as jest.Mock; const permissionsData = ({ library, course }: { library?: boolean; course?: boolean }) => [ { action: CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM, allowed: !!library }, @@ -32,6 +38,11 @@ describe('RolesFilter', () => { beforeEach(() => { jest.clearAllMocks(); mockUsePermissions.mockReturnValue({ data: permissionsData({ library: true, course: true }) }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isLoading: false, + }); }); it('renders the filter toggle', () => { @@ -97,4 +108,31 @@ describe('RolesFilter', () => { expect(menu.queryByText('Courses')).not.toBeInTheDocument(); expect(menu.queryByText('Libraries')).not.toBeInTheDocument(); }); + + it('hides course roles when the course-authoring flag is disabled', async () => { + const user = userEvent.setup(); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: false, + isCourseEnabled: () => false, + isLoading: false, + }); + renderWrapper(); + const menu = await openDropdown(user); + expect(menu.getByText('Libraries')).toBeInTheDocument(); + expect(menu.queryByText('Courses')).not.toBeInTheDocument(); + expect(menu.queryByLabelText('Course Admin')).not.toBeInTheDocument(); + }); + + it('shows no role options while the course-authoring flag is still loading', async () => { + const user = userEvent.setup(); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: false, + isCourseEnabled: () => false, + isLoading: true, + }); + renderWrapper(); + const menu = await openDropdown(user); + expect(menu.queryByText('Courses')).not.toBeInTheDocument(); + expect(menu.queryByText('Libraries')).not.toBeInTheDocument(); + }); }); diff --git a/src/authz-module/components/TableControlBar/RolesFilter.tsx b/src/authz-module/components/TableControlBar/RolesFilter.tsx index 95c6bd03..638f6358 100644 --- a/src/authz-module/components/TableControlBar/RolesFilter.tsx +++ b/src/authz-module/components/TableControlBar/RolesFilter.tsx @@ -2,6 +2,7 @@ import { useMemo } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Person } from '@openedx/paragon/icons'; import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import { CONTEXT_TYPES } from '@src/authz-module/constants'; import MultipleChoiceFilter from './MultipleChoiceFilter'; import { MultipleChoiceFilterProps } from './types'; @@ -14,15 +15,17 @@ const RolesFilter = ({ }: RolesFilterProps) => { const intl = useIntl(); const { isCourseViewAllowed, isLibraryViewAllowed, isLoading } = useViewTeamPermissions(); + const { isCourseAuthoringEnabled, isLoading: isFlagLoading } = useCourseAuthoringFlag(); const rolesOptions = useMemo(() => { - if (isLoading) { return []; } + if (isLoading || isFlagLoading) { return []; } return getRolesFiltersOptions(intl).filter((option) => { - if (option.contextType === CONTEXT_TYPES.COURSE) { return isCourseViewAllowed; } + // Authoring (course) roles require both view permission and the course-authoring flag. + if (option.contextType === CONTEXT_TYPES.COURSE) { return isCourseViewAllowed && isCourseAuthoringEnabled; } if (option.contextType === CONTEXT_TYPES.LIBRARY) { return isLibraryViewAllowed; } return false; }); - }, [intl, isCourseViewAllowed, isLibraryViewAllowed, isLoading]); + }, [intl, isCourseViewAllowed, isLibraryViewAllowed, isCourseAuthoringEnabled, isLoading, isFlagLoading]); return ( ({ useValidateUserPermissionsNonSuspense: jest.fn(), })); +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: jest.fn(), +})); + const mockUsePermissions = useValidateUserPermissionsNonSuspense as jest.Mock; +const mockUseCourseAuthoringFlag = useCourseAuthoringFlag as jest.Mock; jest.mock('@src/authz-module/data/hooks', () => ({ useScopes: jest.fn(() => ({ @@ -53,6 +59,11 @@ describe('ScopesFilter', () => { beforeEach(() => { jest.clearAllMocks(); mockUsePermissions.mockReturnValue({ data: permissionsData({ library: true, course: true }) }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isLoading: false, + }); }); it('renders without crashing', () => { @@ -108,4 +119,25 @@ describe('ScopesFilter', () => { expect.objectContaining({ scopeType: 'library' }), ); }); + + it('lists course scopes whose course-authoring flag is enabled', async () => { + const user = userEvent.setup(); + renderWrapper(); + await user.click(screen.getByRole('button', { name: /Scopes/i })); + expect(await screen.findByText('Test Library')).toBeInTheDocument(); + expect(screen.getByText('Test Course')).toBeInTheDocument(); + }); + + it('hides course scopes whose course-authoring flag is disabled but keeps libraries', async () => { + const user = userEvent.setup(); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: false, + isCourseEnabled: () => false, + isLoading: false, + }); + renderWrapper(); + await user.click(screen.getByRole('button', { name: /Scopes/i })); + expect(await screen.findByText('Test Library')).toBeInTheDocument(); + expect(screen.queryByText('Test Course')).not.toBeInTheDocument(); + }); }); diff --git a/src/authz-module/components/TableControlBar/ScopesFilter.tsx b/src/authz-module/components/TableControlBar/ScopesFilter.tsx index 1457a8cd..2e77c620 100644 --- a/src/authz-module/components/TableControlBar/ScopesFilter.tsx +++ b/src/authz-module/components/TableControlBar/ScopesFilter.tsx @@ -2,6 +2,7 @@ import { useMemo, useState } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { LocationOn } from '@openedx/paragon/icons'; import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import { useScopes } from '@src/authz-module/data/hooks'; import { DEFAULT_FILTER_PAGE_SIZE } from '@src/authz-module/constants'; import { MultipleChoiceFilterProps } from './types'; @@ -18,6 +19,7 @@ const ScopesFilter = ({ const [searchValue, setSearchValue] = useState(undefined); const { isCourseViewAllowed } = useViewTeamPermissions(); + const { isCourseEnabled } = useCourseAuthoringFlag(); const { data: scopesData } = useScopes({ search: searchValue, @@ -25,20 +27,23 @@ const ScopesFilter = ({ ...(isCourseViewAllowed ? {} : { scopeType: 'library' }), }); - const filterChoices = useMemo(() => (scopesData?.pages?.flatMap((p) => p.results) ?? []).map((scope) => { - const scopeIcon = scope.externalKey?.startsWith('lib') ? RESOURCE_ICONS.LIBRARY : RESOURCE_ICONS.COURSE; - let groupName = formatMessage(messages['authz.team.members.table.group.courses']); - if (scope.externalKey?.startsWith('lib')) { - groupName = formatMessage(messages['authz.team.members.table.group.libraries']); - } - return { - displayName: scope.displayName, - value: scope.externalKey, - description: scope.org?.shortName, - groupName, - groupIcon: scopeIcon, - }; - }), [scopesData?.pages, formatMessage]); + const filterChoices = useMemo(() => (scopesData?.pages?.flatMap((p) => p.results) ?? []) + // Libraries are always available; courses only when the authoring flag is enabled for them. + .filter((scope) => scope.externalKey?.startsWith('lib') || isCourseEnabled(scope.externalKey)) + .map((scope) => { + const scopeIcon = scope.externalKey?.startsWith('lib') ? RESOURCE_ICONS.LIBRARY : RESOURCE_ICONS.COURSE; + let groupName = formatMessage(messages['authz.team.members.table.group.courses']); + if (scope.externalKey?.startsWith('lib')) { + groupName = formatMessage(messages['authz.team.members.table.group.libraries']); + } + return { + displayName: scope.displayName, + value: scope.externalKey, + description: scope.org?.shortName, + groupName, + groupIcon: scopeIcon, + }; + }), [scopesData?.pages, formatMessage, isCourseEnabled]); const handleSearchChange = (value: string) => { setSearchValue(value); diff --git a/src/authz-module/components/TableControlBar/TableControlBar.test.tsx b/src/authz-module/components/TableControlBar/TableControlBar.test.tsx index def9e931..dd4ac52e 100644 --- a/src/authz-module/components/TableControlBar/TableControlBar.test.tsx +++ b/src/authz-module/components/TableControlBar/TableControlBar.test.tsx @@ -34,6 +34,14 @@ const mockState = { ], }; +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: () => ({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isLoading: false, + }), +})); + jest.mock('@src/authz-module/data/hooks', () => ({ useOrgs: () => ({ data: { diff --git a/src/authz-module/data/api.test.tsx b/src/authz-module/data/api.test.tsx index 35d7ed99..f8225c97 100644 --- a/src/authz-module/data/api.test.tsx +++ b/src/authz-module/data/api.test.tsx @@ -9,6 +9,7 @@ import { getLibrary, getOrgs, getScopes, + getCourseAuthoringFlagStates, } from './api'; jest.mock('@edx/frontend-platform/auth', () => ({ @@ -317,4 +318,29 @@ describe('API functions', () => { expect(calledUrl.toString()).toContain('page_size=50'); }); }); + + describe('getCourseAuthoringFlagStates', () => { + it('should fetch the flag states and camel-case the response', async () => { + const mockResponse = { + data: { + global: false, + org_overrides: { on: ['Demo'], off: [] }, + course_overrides: { on: [], off: ['course-v1:testing+CT01+CT01-2024'] }, + }, + }; + const mockGet = jest.fn().mockResolvedValue(mockResponse); + mockHttpClient().mockReturnValue({ get: mockGet }); + + const result = await getCourseAuthoringFlagStates(); + + expect(getAuthenticatedHttpClient).toHaveBeenCalled(); + const calledUrl = mockGet.mock.calls[0][0]; + expect(calledUrl.toString()).toContain('/api/authz/v1/waffle-flag-states/'); + expect(result).toEqual({ + global: false, + orgOverrides: { on: ['Demo'], off: [] }, + courseOverrides: { on: [], off: ['course-v1:testing+CT01+CT01-2024'] }, + }); + }); + }); }); diff --git a/src/authz-module/data/api.ts b/src/authz-module/data/api.ts index 078687f6..affc5dae 100644 --- a/src/authz-module/data/api.ts +++ b/src/authz-module/data/api.ts @@ -106,6 +106,22 @@ export interface GetScopesParams { managementPermissionOnly?: boolean; } +export interface WaffleFlagOverrides { + on: string[]; + off: string[]; +} + +/** + * Enablement state of the course-authoring waffle flag across scopes. + * `orgOverrides`/`courseOverrides` list the org short names / course ids whose + * override differs from `global` (explicitly forced `on` or `off`). + */ +export interface CourseAuthoringFlagStates { + global: boolean; + orgOverrides: WaffleFlagOverrides; + courseOverrides: WaffleFlagOverrides; +} + export const getTeamMembers = async (object: string, querySettings: QuerySettings): Promise => { const url = new URL(getApiUrl(`/api/authz/v1/roles/users/?scope=${object}`)); @@ -229,6 +245,11 @@ export const getScopes = async (params: GetScopesParams): Promise => { + const { data } = await getAuthenticatedHttpClient().get(getApiUrl('/api/authz/v1/waffle-flag-states/')); + return camelCaseObject(data); +}; + export const getUserAssignedRoles = async (username?: string, querySettings?: QuerySettings) : Promise => { const url = new URL(getApiUrl(`/api/authz/v1/users/${username}/assignments/`)); diff --git a/src/authz-module/data/hooks.ts b/src/authz-module/data/hooks.ts index 62ec4122..cd5d948c 100644 --- a/src/authz-module/data/hooks.ts +++ b/src/authz-module/data/hooks.ts @@ -10,6 +10,7 @@ import { GetTeamMembersResponse, PermissionsByRole, QuerySettings, revokeUserRoles, RevokeUserRolesRequest, getUserAssignedRoles, GetUserAssignmentsResponse, validateUsers, ValidateUsersRequest, GetScopesParams, + getCourseAuthoringFlagStates, CourseAuthoringFlagStates, } from './api'; const authzQueryKeys = { @@ -23,6 +24,7 @@ const authzQueryKeys = { 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, userRoles: (username?: string, querySettings?: QuerySettings) => [...authzQueryKeys.all, 'userRoles', username, querySettings] as const, + courseAuthoringFlagStates: () => [...authzQueryKeys.all, 'courseAuthoringFlagStates'] as const, }; /** @@ -247,3 +249,20 @@ export const useUserAssignedRoles = ( enabled: !!username, refetchOnWindowFocus: false, }); + +/** + * React Query hook to fetch the enablement state of the course-authoring waffle flag + * across scopes (global, org overrides, course overrides). + * + * Follows the same caching rules as the permission-validation hooks. + * + * @example + * ```tsx + * const { data } = useCourseAuthoringFlagStates(); + * ``` + */ +export const useCourseAuthoringFlagStates = () => useQuery({ + queryKey: authzQueryKeys.courseAuthoringFlagStates(), + queryFn: () => getCourseAuthoringFlagStates(), + retry: false, +}); diff --git a/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx b/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx new file mode 100644 index 00000000..b5d3e761 --- /dev/null +++ b/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx @@ -0,0 +1,137 @@ +import { renderHook, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { ReactNode } from 'react'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { logError } from '@edx/frontend-platform/logging'; +import { ToastManagerProvider } from '@src/components/ToastManager/ToastManagerContext'; +import { useCourseAuthoringFlagStates } from '@src/authz-module/data/hooks'; +import { useCourseAuthoringFlag } from './useCourseAuthoringFlag'; + +jest.mock('@edx/frontend-platform/logging', () => ({ + logError: jest.fn(), +})); + +jest.mock('@src/authz-module/data/hooks', () => ({ + useCourseAuthoringFlagStates: jest.fn(), +})); + +const mockUseCourseAuthoringFlagStates = useCourseAuthoringFlagStates as jest.Mock; + +const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + +); + +const flagStates = (overrides = {}) => ({ + global: false, + orgOverrides: { on: [], off: [] }, + courseOverrides: { on: [], off: [] }, + ...overrides, +}); + +const statesHook = (overrides = {}) => ({ + data: flagStates(), + isLoading: false, + error: null, + refetch: jest.fn(), + ...overrides, +}); + +describe('useCourseAuthoringFlag', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook()); + }); + + describe('flag resolution', () => { + it('enables the domain when the global flag is on', () => { + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ data: flagStates({ global: true }) })); + const { result } = renderHook(() => useCourseAuthoringFlag(), { wrapper }); + expect(result.current.isCourseAuthoringEnabled).toBe(true); + expect(result.current.isCourseEnabled('course-v1:org1+A+2024')).toBe(true); + expect(result.current.isOrgAuthoringEnabled('org1')).toBe(true); + }); + + it('applies course override precedence over org override and global', () => { + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ + data: flagStates({ + global: true, + orgOverrides: { on: [], off: ['org1'] }, + courseOverrides: { on: ['course-v1:org1+A+2024'], off: [] }, + }), + })); + const { result } = renderHook(() => useCourseAuthoringFlag(), { wrapper }); + expect(result.current.isCourseEnabled('course-v1:org1+A+2024')).toBe(true); + expect(result.current.isCourseEnabled('course-v1:org1+B+2024')).toBe(false); + expect(result.current.isOrgAuthoringEnabled('org1')).toBe(true); + }); + + it('resolves everything to false while loading', () => { + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ data: undefined, isLoading: true })); + const { result } = renderHook(() => useCourseAuthoringFlag(), { wrapper }); + expect(result.current.isLoading).toBe(true); + expect(result.current.isCourseAuthoringEnabled).toBe(false); + expect(result.current.isCourseEnabled('course-v1:org1+A+2024')).toBe(false); + expect(result.current.isOrgAuthoringEnabled('org1')).toBe(false); + }); + }); + + describe('error handling', () => { + it('shows no toast when the flag states load successfully', () => { + renderHook(() => useCourseAuthoringFlag(), { wrapper }); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('surfaces a generic error toast and keeps resolving to false when the fetch fails', async () => { + const error = new Error('Request failed'); + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ data: undefined, error })); + + const { result } = renderHook(() => useCourseAuthoringFlag(), { wrapper }); + + expect(await screen.findByRole('alert')).toBeInTheDocument(); + expect(screen.getByText('Something went wrong on our end.')).toBeInTheDocument(); + expect(logError).toHaveBeenCalledWith(error); + expect(result.current.isCourseAuthoringEnabled).toBe(false); + expect(result.current.isCourseEnabled('course-v1:org1+A+2024')).toBe(false); + expect(result.current.isOrgAuthoringEnabled('org1')).toBe(false); + }); + + it('retries the fetch from the toast action', async () => { + const user = userEvent.setup(); + const error = new Error('Request failed'); + const refetch = jest.fn(); + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ data: undefined, error, refetch })); + + renderHook(() => useCourseAuthoringFlag(), { wrapper }); + + await user.click(await screen.findByRole('button', { name: 'Retry' })); + expect(refetch).toHaveBeenCalled(); + }); + + it('shows a single toast per failure when several components consume the hook', async () => { + const error = new Error('Request failed'); + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ data: undefined, error })); + + const Consumer = () => { + useCourseAuthoringFlag(); + return null; + }; + renderHook(() => useCourseAuthoringFlag(), { + wrapper: ({ children }: { children: ReactNode }) => wrapper({ + children: ( + <> + + + {children} + + ), + }), + }); + + await waitFor(() => { + expect(screen.getAllByRole('alert')).toHaveLength(1); + }); + }); + }); +}); diff --git a/src/authz-module/hooks/useCourseAuthoringFlag.ts b/src/authz-module/hooks/useCourseAuthoringFlag.ts new file mode 100644 index 00000000..ef5dbcab --- /dev/null +++ b/src/authz-module/hooks/useCourseAuthoringFlag.ts @@ -0,0 +1,85 @@ +import { useEffect, useMemo } from 'react'; +import { useCourseAuthoringFlagStates } from '@src/authz-module/data/hooks'; +import { useToastManager } from '@src/components/ToastManager/ToastManagerContext'; + +/** + * Extract the org short name from a course external key. + * e.g. `course-v1:testing+CT01+CT01-2024` -> `testing` + */ +const orgOf = (courseId: string): string | undefined => courseId.split(':')[1]?.split('+')[0]; + +// Several components on the same page consume this hook and share the underlying query, +// so they all observe the same error instance; track toasted errors to show one toast +// per failure instead of one per consumer. +const toastedErrors = new WeakSet(); + +/** + * Resolve the course-authoring waffle flag state, both at the coarse domain level and + * per individual course scope. + * + * `isCourseAuthoringEnabled` is the domain-level gate (enabled anywhere) used to decide + * whether the authoring category should appear in controls like the roles filter. + * + * `isCourseEnabled(courseId)` resolves a single course against the overrides, applying + * the precedence course override -> org override -> global. Overrides only appear when + * they differ from `global`, so membership in the `on`/`off` lists is authoritative. + * + * `isOrgAuthoringEnabled(org)` resolves whether authoring is enabled for at least one + * course within an org: the org override forces it on, some course in the org is forced + * on, or (absent an off override) the global flag is on. + * + * While the flag states are loading, everything resolves to `false` so authoring scopes + * and roles stay hidden until enablement is known. If the fetch fails, a generic error + * toast (with retry) is surfaced and everything keeps resolving to `false`. + */ +export const useCourseAuthoringFlag = () => { + const { + data: flagStates, isLoading, error, refetch, + } = useCourseAuthoringFlagStates(); + const { showErrorToast } = useToastManager(); + + useEffect(() => { + if (error && !toastedErrors.has(error)) { + toastedErrors.add(error); + showErrorToast(error, refetch); + } + }, [error, refetch, showErrorToast]); + + const isCourseAuthoringEnabled = flagStates + ? flagStates.global + || flagStates.orgOverrides.on.length > 0 + || flagStates.courseOverrides.on.length > 0 + : false; + + const { isCourseEnabled, isOrgAuthoringEnabled } = useMemo(() => { + const courseOn = new Set(flagStates?.courseOverrides.on ?? []); + const courseOff = new Set(flagStates?.courseOverrides.off ?? []); + const orgOn = new Set(flagStates?.orgOverrides.on ?? []); + const orgOff = new Set(flagStates?.orgOverrides.off ?? []); + const global = flagStates?.global ?? false; + + const courseEnabled = (courseId: string): boolean => { + if (courseOn.has(courseId)) { return true; } + if (courseOff.has(courseId)) { return false; } + const org = orgOf(courseId); + if (org && orgOn.has(org)) { return true; } + if (org && orgOff.has(org)) { return false; } + return global; + }; + + const orgsWithForcedOnCourse = new Set([...courseOn].map(orgOf)); + + const orgAuthoringEnabled = (org: string): boolean => { + if (orgOn.has(org)) { return true; } + if (orgsWithForcedOnCourse.has(org)) { return true; } + if (orgOff.has(org)) { return false; } + return global; + }; + + return { isCourseEnabled: courseEnabled, isOrgAuthoringEnabled: orgAuthoringEnabled }; + }, [flagStates]); + + return { + isCourseAuthoringEnabled, isCourseEnabled, isOrgAuthoringEnabled, isLoading, + }; +}; diff --git a/src/authz-module/role-assignation-wizard/AssignRoleWizard.test.tsx b/src/authz-module/role-assignation-wizard/AssignRoleWizard.test.tsx index f1c3becc..6d787b5b 100644 --- a/src/authz-module/role-assignation-wizard/AssignRoleWizard.test.tsx +++ b/src/authz-module/role-assignation-wizard/AssignRoleWizard.test.tsx @@ -27,6 +27,23 @@ jest.mock('@edx/frontend-platform/logging', () => ({ logError: jest.fn(), })); +jest.mock('@src/authz-module/hooks/useViewTeamPermissions', () => ({ + useViewTeamPermissions: () => ({ + isCourseViewAllowed: true, + isLibraryViewAllowed: true, + isLoading: false, + }), +})); + +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: () => ({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isOrgAuthoringEnabled: () => true, + isLoading: false, + }), +})); + jest.mock('../data/hooks', () => ({ useValidateUsers: jest.fn(), useAssignTeamMembersRole: jest.fn(), diff --git a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx index 63a953a8..cab7e77b 100644 --- a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx +++ b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx @@ -4,6 +4,7 @@ import { renderWithAllProviders } from '@src/setupTest'; import { ToastManagerProvider } from '@src/components/ToastManager/ToastManagerContext'; import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; import { useValidateUsers } from '../data/hooks'; +import { useCourseAuthoringFlag } from '../hooks/useCourseAuthoringFlag'; import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS, @@ -39,8 +40,13 @@ jest.mock('@src/data/hooks', () => ({ useValidateUserPermissionsNonSuspense: jest.fn(), })); +jest.mock('../hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: jest.fn(), +})); + const mockUseValidateUsers = useValidateUsers as jest.Mock; const mockUseValidatePermissions = useValidateUserPermissionsNonSuspense as jest.Mock; +const mockUseCourseAuthoringFlag = useCourseAuthoringFlag as jest.Mock; const allowAllPermissions = { data: [ { action: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM, allowed: true }, @@ -51,23 +57,7 @@ const allowAllPermissions = { const mockPermissions = ( manageData: typeof allowAllPermissions, - { courseViewAllowed = true } = {}, -) => (permissions: { action: string }[]) => { - const isViewCall = permissions.some( - (p) => p.action === CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM - || p.action === CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM, - ); - if (isViewCall) { - return { - data: [ - { action: CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM, allowed: true }, - { action: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM, allowed: courseViewAllowed }, - ], - isLoading: false, - }; - } - return manageData; -}; +) => () => manageData; const setupMocks = ({ users = '', from = '' } = {}) => { const { useSearchParams, useNavigate } = jest.requireMock('react-router-dom'); @@ -94,6 +84,11 @@ describe('AssignRoleWizardPage', () => { isPending: false, }); mockUseValidatePermissions.mockImplementation(mockPermissions(allowAllPermissions)); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isLoading: false, + }); }); it('renders the page with the wizard and title', () => { @@ -208,10 +203,12 @@ describe('AssignRoleWizardPage', () => { }); }); - it('hides course roles when VIEW_COURSE_TEAM is not allowed even if MANAGE_COURSE_TEAM is allowed', () => { - mockUseValidatePermissions.mockImplementation( - mockPermissions(allowAllPermissions, { courseViewAllowed: false }), - ); + it('hides course roles when the course-authoring flag is disabled even if MANAGE_COURSE_TEAM is allowed', () => { + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: false, + isCourseEnabled: () => false, + isLoading: false, + }); setupMocks(); renderPage(); courseRolesMetadata.forEach((role) => { diff --git a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx index a5bd050e..a146542d 100644 --- a/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx +++ b/src/authz-module/role-assignation-wizard/AssignRoleWizardPage.tsx @@ -9,7 +9,7 @@ import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS, courseRolesMetadata, libraryRolesMetadata, MANAGE_TEAM_PERMISSIONS, } from '../roles-permissions'; -import { useViewTeamPermissions } from '../hooks/useViewTeamPermissions'; +import { useCourseAuthoringFlag } from '../hooks/useCourseAuthoringFlag'; const AssignRoleWizardPage = () => { const intl = useIntl(); @@ -25,13 +25,14 @@ const AssignRoleWizardPage = () => { : returnTo; const { data: managePermissions } = useValidateUserPermissionsNonSuspense(MANAGE_TEAM_PERMISSIONS); - const { isCourseViewAllowed } = useViewTeamPermissions(); + const { isCourseAuthoringEnabled } = useCourseAuthoringFlag(); const rolesAssignable = managePermissions?.flatMap((p) => { if (!p.allowed) { return []; } if (p.action === CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM) { return libraryRolesMetadata; } if (p.action === CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM) { - return isCourseViewAllowed ? courseRolesMetadata : []; + // Course (authoring) roles are only assignable when the course-authoring flag is enabled. + return isCourseAuthoringEnabled ? courseRolesMetadata : []; } return []; }); diff --git a/src/authz-module/role-assignation-wizard/components/DefineApplicationScopeStep.test.tsx b/src/authz-module/role-assignation-wizard/components/DefineApplicationScopeStep.test.tsx index c08bc34f..8684e31e 100644 --- a/src/authz-module/role-assignation-wizard/components/DefineApplicationScopeStep.test.tsx +++ b/src/authz-module/role-assignation-wizard/components/DefineApplicationScopeStep.test.tsx @@ -6,6 +6,23 @@ import DefineApplicationScopeStep from './DefineApplicationScopeStep'; import { useScopes, useOrgs } from '../../data/hooks'; import useScopePermissions from '../hooks/useScopePermissions'; +jest.mock('@src/authz-module/hooks/useViewTeamPermissions', () => ({ + useViewTeamPermissions: () => ({ + isCourseViewAllowed: true, + isLibraryViewAllowed: true, + isLoading: false, + }), +})); + +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: () => ({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isOrgAuthoringEnabled: () => true, + isLoading: false, + }), +})); + jest.mock('../../data/hooks', () => ({ useScopes: jest.fn(), useOrgs: jest.fn(), diff --git a/src/authz-module/role-assignation-wizard/components/ScopeList.tsx b/src/authz-module/role-assignation-wizard/components/ScopeList.tsx index e6744b42..842fb51e 100644 --- a/src/authz-module/role-assignation-wizard/components/ScopeList.tsx +++ b/src/authz-module/role-assignation-wizard/components/ScopeList.tsx @@ -83,7 +83,9 @@ const ScopeList = ({ /> ))} - {orderedOrgs.length === 0 && ( + {/* Loaded pages can be entirely filtered out client-side while later pages + still hold visible scopes, so only report empty once every page is in. */} + {orderedOrgs.length === 0 && !hasNextPage && !isFetchingNextPage && (

{intl.formatMessage(messages['wizard.step2.scopeList.empty'])}

)} diff --git a/src/authz-module/role-assignation-wizard/hooks/useScopeListData.test.ts b/src/authz-module/role-assignation-wizard/hooks/useScopeListData.test.ts index 0a9d6483..3ec889f0 100644 --- a/src/authz-module/role-assignation-wizard/hooks/useScopeListData.test.ts +++ b/src/authz-module/role-assignation-wizard/hooks/useScopeListData.test.ts @@ -1,5 +1,6 @@ import { renderHook } from '@testing-library/react'; import { intlWrapper as wrapper } from '@src/setupTest'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import useScopeListData from './useScopeListData'; import { useScopes, useOrgs } from '../../data/hooks'; import useScopePermissions from './useScopePermissions'; @@ -9,11 +10,16 @@ jest.mock('../../data/hooks', () => ({ useOrgs: jest.fn(), })); +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: jest.fn(), +})); + jest.mock('./useScopePermissions'); const mockUseScopes = useScopes as jest.Mock; const mockUseOrganizations = useOrgs as jest.Mock; const mockUseScopePermissions = useScopePermissions as jest.Mock; +const mockUseCourseAuthoringFlag = useCourseAuthoringFlag as jest.Mock; const makeScopesHook = (overrides = {}) => ({ data: { @@ -45,6 +51,12 @@ describe('useScopeListData', () => { hasPlatformPermission: false, orgHasPermission: { org1: true, org2: true }, }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isOrgAuthoringEnabled: () => true, + isLoading: false, + }); }); describe('Return value structure', () => { @@ -500,4 +512,90 @@ describe('useScopeListData', () => { expect(result.current.scopesByOrg).toEqual({}); }); }); + + describe('Course-authoring flag filtering', () => { + const courseScopes = makeScopesHook({ + data: { + pages: [{ + results: [ + { externalKey: 'course-v1:org1+A+2024', displayName: 'Course A', org: { id: 1, name: 'Org 1', shortName: 'org1' } }, + { externalKey: 'course-v1:org1+B+2024', displayName: 'Course B', org: { id: 1, name: 'Org 1', shortName: 'org1' } }, + ], + count: 2, + next: null, + previous: null, + }], + }, + }); + + it('filters out course scopes whose authoring flag is disabled', () => { + mockUseScopes.mockReturnValue(courseScopes); + mockUseOrganizations.mockReturnValue({ data: { results: defaultOrgs } }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: (id: string) => id === 'course-v1:org1+A+2024', + isOrgAuthoringEnabled: () => true, + isLoading: false, + }); + + const { result } = renderHook(() => useScopeListData({ + contextType: 'course', + search: '', + orgs: [], + }), { wrapper }); + + expect(result.current.allScopes.map((s) => s.externalKey)).toEqual(['course-v1:org1+A+2024']); + expect(result.current.totalCount).toBe(1); + }); + + it('does not filter library scopes by the authoring flag', () => { + const libScopes = makeScopesHook({ + data: { + pages: [{ + results: [ + { externalKey: 'lib:org1:lib1', displayName: 'Library 1', org: { id: 1, name: 'Org 1', shortName: 'org1' } }, + ], + count: 1, + next: null, + previous: null, + }], + }, + }); + mockUseScopes.mockReturnValue(libScopes); + mockUseOrganizations.mockReturnValue({ data: { results: defaultOrgs } }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: false, + isCourseEnabled: () => false, + isOrgAuthoringEnabled: () => false, + isLoading: false, + }); + + const { result } = renderHook(() => useScopeListData({ + contextType: 'library', + search: '', + orgs: [], + }), { wrapper }); + + expect(result.current.allScopes).toHaveLength(1); + }); + + it('omits org-wide aggregates for authoring-disabled orgs', () => { + mockUseScopes.mockReturnValue(courseScopes); + mockUseOrganizations.mockReturnValue({ data: { results: defaultOrgs } }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isOrgAuthoringEnabled: () => false, + isLoading: false, + }); + + const { result } = renderHook(() => useScopeListData({ + contextType: 'course', + search: '', + orgs: [], + }), { wrapper }); + + expect(result.current.orgAggregateScopeItems).toEqual({}); + }); + }); }); diff --git a/src/authz-module/role-assignation-wizard/hooks/useScopeListData.ts b/src/authz-module/role-assignation-wizard/hooks/useScopeListData.ts index 205a6976..7e9bcb6b 100644 --- a/src/authz-module/role-assignation-wizard/hooks/useScopeListData.ts +++ b/src/authz-module/role-assignation-wizard/hooks/useScopeListData.ts @@ -2,6 +2,7 @@ import { useMemo } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Scope } from '@src/types'; import { useOrgs, useScopes } from '@src/authz-module/data/hooks'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import { getOrgAggregateScopeKey } from '@src/authz-module/constants'; import messages from '../messages'; import useScopePermissions from './useScopePermissions'; @@ -37,13 +38,25 @@ const useScopeListData = ({ contextType, search, orgs }: UseScopeListDataParams) const { data: orgsData } = useOrgs(); const organizations = orgsData?.results; - const allScopes = useMemo( - () => scopesData?.pages.flatMap((page) => page.results) ?? [], - [scopesData], + const { isCourseEnabled, isOrgAuthoringEnabled } = useCourseAuthoringFlag(); + + const { allScopes, totalCount } = useMemo( + () => { + const scopes = scopesData?.pages.flatMap((page) => page.results) ?? []; + const serverCount = scopesData?.pages[0]?.count ?? 0; + // Course scopes are gated by the course-authoring flag; libraries always pass. + if (contextType !== 'course') { return { allScopes: scopes, totalCount: serverCount }; } + const enabledScopes = scopes.filter((scope) => isCourseEnabled(scope.externalKey)); + // The server count includes authoring-disabled courses this filter hides; subtract + // the ones already loaded so the reported total converges as pages arrive. + return { + allScopes: enabledScopes, + totalCount: serverCount - (scopes.length - enabledScopes.length), + }; + }, + [scopesData, contextType, isCourseEnabled], ); - const totalCount = scopesData?.pages[0]?.count ?? 0; - const scopesByOrg = useMemo( () => allScopes .filter((s: Scope) => !!s.org) @@ -90,6 +103,8 @@ const useScopeListData = ({ contextType, search, orgs }: UseScopeListDataParams) return Object.fromEntries( orderedOrgs .filter((orgSlug) => orgHasPermission[orgSlug]) + // The org-wide aggregate is only offered when authoring is enabled for that org. + .filter((orgSlug) => contextType !== 'course' || isOrgAuthoringEnabled(orgSlug)) .map((orgSlug) => [ orgSlug, { @@ -100,7 +115,7 @@ const useScopeListData = ({ contextType, search, orgs }: UseScopeListDataParams) } satisfies Scope, ]), ); - }, [orderedOrgs, contextType, orgHasPermission, orgAggregateLabel, aggregateDescription]); + }, [orderedOrgs, contextType, orgHasPermission, orgAggregateLabel, aggregateDescription, isOrgAuthoringEnabled]); return { organizations, diff --git a/src/authz-module/team-members/TeamMembersTable.test.tsx b/src/authz-module/team-members/TeamMembersTable.test.tsx index 7b873d12..fda3a6fe 100644 --- a/src/authz-module/team-members/TeamMembersTable.test.tsx +++ b/src/authz-module/team-members/TeamMembersTable.test.tsx @@ -122,6 +122,14 @@ jest.mock('@edx/frontend-platform/logging', () => ({ logError: jest.fn(), })); +jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ + useCourseAuthoringFlag: () => ({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isLoading: false, + }), +})); + jest.mock('@src/authz-module/data/hooks', () => ({ useAllRoleAssignments: jest.fn(), useOrgs: jest.fn(), From ee184393bc2cff6e542f623480650edbc0a94f6a Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Wed, 15 Jul 2026 20:07:55 -0500 Subject: [PATCH 06/10] docs(adr): expand ADR 0003 and improve useCourseAuthoringFlag --- ...0003-course-authoring-flag-enforcement.rst | 174 +++++++----------- src/authz-module/data/hooks.test.tsx | 39 ++++ src/authz-module/data/hooks.ts | 2 + .../hooks/useCourseAuthoringFlag.test.tsx | 4 +- .../hooks/useCourseAuthoringFlag.ts | 4 +- .../hooks/useViewTeamPermissions.test.ts | 50 +++++ 6 files changed, 160 insertions(+), 113 deletions(-) create mode 100644 src/authz-module/hooks/useViewTeamPermissions.test.ts diff --git a/docs/decisions/0003-course-authoring-flag-enforcement.rst b/docs/decisions/0003-course-authoring-flag-enforcement.rst index 0874cabb..e7d2c25d 100644 --- a/docs/decisions/0003-course-authoring-flag-enforcement.rst +++ b/docs/decisions/0003-course-authoring-flag-enforcement.rst @@ -8,71 +8,44 @@ Proposed .. note:: - This is a **temporary** measure for the flag-gated rollout period. It is - expected to be removed and re-evaluated once the authz system is enabled by - default and `authz.enable_course_authoring` is deprecated (see `ADR 0010`_). - See *Temporary nature and future work* below. + This is a **temporary** measure for the flag-gated rollout period. It is expected to be removed and re-evaluated once the authz system is enabled by default and `authz.enable_course_authoring` is deprecated (see `ADR 0010`_). See *Temporary nature and future work* below. Context ####### -The authorization backend (`openedx-authz`) is rolling out the course -authoring domain behind the Waffle flag -`authz.enable_course_authoring`. The flag can be enabled globally -(instance-wide), per organization, or per course. +The authorization backend (`openedx-authz`) is rolling out the course authoring domain behind the Waffle flag `authz.enable_course_authoring`. The flag can be enabled globally (instance-wide), per organization, or per course. The rollout is defined by the following backend decisions: * `ADR 0010 - Course Authoring Flag`_ introduces the multi-level Waffle flag. -* `ADR 0013 - Course Authoring Automatic Migration`_ makes the flag the source - of truth. When the flag changes for a scope, role assignments migrate between - the legacy `CourseAccessRole` model and the new authz (Casbin) system. -* `ADR 0007 - Enforcement Mechanisms (MFEs)`_ establishes that frontends enforce - authorization by querying the backend rather than deriving policy from tokens. +* `ADR 0013 - Course Authoring Automatic Migration`_ makes the flag the source of truth. When the flag changes for a scope, role assignments migrate between the legacy `CourseAccessRole` model and the new authz (Casbin) system. +* `ADR 0007 - Enforcement Mechanisms (MFEs)`_ establishes that frontends enforce authorization by querying the backend rather than deriving policy from tokens. The Admin Console manages role assignments for two authorization domains: * **Content libraries**, which are always enabled. * **Course authoring**, which is gated by the feature flag. -When course authoring is disabled for a scope, the UI must not expose that -domain. Specifically: +When course authoring is disabled for a scope, the UI must not expose that domain. Specifically: * The **Scope** filter must not list courses where authoring is disabled. -* The **Role** filter must not list course-authoring roles when the domain is - disabled. -* The **Organization** filter must not hide organizations that are reachable - through libraries, but it must exclude organizations that expose only disabled - course authoring. -* The **assignment lists** must not display users or assignments that belong - only to course authoring. Users with both library and authoring roles should - display only their library roles. - -Because the flag is evaluated per scope, a single instance-wide boolean is -insufficient. For example, the global flag may be disabled while a specific -organization or course is enabled, and vice versa. The frontend must therefore -resolve enablement for the specific scope it is rendering. +* The **Role** filter must not list course-authoring roles when the domain is disabled. +* The **Organization** filter must not hide organizations that are reachable through libraries, but it must exclude organizations that expose only disabled course authoring. +* The **assignment lists** must not display users or assignments that belong only to course authoring. Users with both library and authoring roles should display only their library roles. + +Because the flag is evaluated per scope, a single instance-wide boolean is insufficient. For example, the global flag may be disabled while a specific organization or course is enabled, and vice versa. The frontend must therefore resolve enablement for the specific scope it is rendering. Enablement must also remain independent of permission validation. -A natural implementation would infer whether course authoring is enabled from -`/permissions/validate/me`. However, permission validation reflects Casbin role -assignments, while enablement reflects the current Waffle flag. These sources can -temporarily diverge. +A natural implementation would infer whether course authoring is enabled from `/permissions/validate/me`. However, permission validation reflects Casbin role assignments, while enablement reflects the current Waffle flag. These sources can temporarily diverge. + +As described in `ADR 0013`_, Casbin assignments exist only after migration has been executed. Automatic migration is disabled by default (`ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION = False`). Even when enabled, only organization-level and course-level changes migrate automatically; global flag changes still require manual migration commands. -As described in `ADR 0013`_, Casbin assignments exist only after migration has -been executed. Automatic migration is disabled by default -(`ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION = False`). Even when -enabled, only organization- and course-level changes migrate automatically; -global flag changes still require manual migration commands. +Consequently, a runtime change to the global flag may not be reflected in the permission data returned by the authorization APIs. During rollout, the frontend must therefore determine whether the course authoring domain should be displayed from the flag state itself rather than from permission validation. -Consequently, a runtime change to the global flag may not be reflected in the -permission data returned by the authorization APIs. During rollout, the frontend -must therefore determine whether the course authoring domain should be displayed -from the flag state itself rather than from permission validation. +`Issue #340`_ and `Issue #341`_ reported that the Admin Console showed course-authoring roles and scopes even when the flag was disabled at every level. `PR #361`_ attempted to enforce the full per-scope truth table directly inside ``PermissionValidationMeView`` and other release-blocking permission endpoints, but correctness and performance across the framework could not be validated in time for the Verawood release, so the approach was reverted. The dedicated ``waffle-flag-states`` endpoint (see `ADR 0015`_) was introduced instead as an additive, isolated alternative that leaves those endpoints untouched. -To support this, the backend exposes -`GET /api/authz/v1/waffle-flag-states/`:: +To support this, the backend exposes `GET /api/authz/v1/waffle-flag-states/`:: { @@ -85,122 +58,103 @@ To support this, the backend exposes } -The endpoint returns the global flag value together with only those -organization and course overrides whose value differs from the global flag. -Because overrides are separated into `on` and `off` lists, each scope's -effective value can be resolved without ambiguity while preserving override -precedence. +The endpoint returns the global flag value together with only those organization and course overrides whose value differs from the global flag. Because overrides are separated into `on` and `off` lists, each scope's effective value can be resolved without ambiguity while preserving override precedence. Decision ######## **1. Consume flag state through a single cached hook.** -The frontend exposes `getCourseAuthoringFlagStates` and the React Query hook -`useCourseAuthoringFlagStates` to fetch the endpoint. The hook uses the same -React Query configuration as the permission-validation hooks to keep caching -behavior consistent. +The frontend exposes `getCourseAuthoringFlagStates` and the React Query hook `useCourseAuthoringFlagStates` to fetch the endpoint. Because waffle flag states change rarely (set by operators, not by end users), the hook is configured with a 30-minute `staleTime` and `refetchOnWindowFocus: false` to avoid unnecessary requests, and `retry: false` to surface failures immediately. **2. Centralize enablement resolution.** -A derived hook, `useCourseAuthoringFlag`, encapsulates all resolution logic. -Components consume only its public API: +A derived hook, `useCourseAuthoringFlag`, encapsulates all resolution logic. Components consume only its public API: + +* `isCourseAuthoringEnabled` — returns whether the authoring domain is enabled anywhere (globally or through any `on` override). This is the "any tier on" rule from `Issue #340`_ and `Issue #341`_ and is used for coarse domain-level gating. +* `isCourseEnabled(courseId)` — resolves enablement using precedence: **course override -> organization override -> global**. +* `isOrgAuthoringEnabled(org)` — returns `true` when the organization is explicitly enabled, has at least one course with an explicit `on` override, or inherits a global `on` value without an explicit organization `off` override. -* `isCourseAuthoringEnabled` — returns whether the authoring domain is enabled - anywhere (globally or through any `on` override). This is used for coarse - domain-level gating. -* `isCourseEnabled(courseId)` — resolves enablement using precedence: - **course override -> organization override -> global**. -* `isOrgAuthoringEnabled(org)` — returns `true` when the organization is - explicitly enabled, contains at least one enabled course, or inherits a global - `on` value without an explicit organization `off` override. +`isCourseEnabled` and `isOrgAuthoringEnabled` implement the fuller per-scope cascade (course override → organization override → global). Applying this logic client-side is safe because it does not touch the release-blocking permission endpoints that motivated the revert of `PR #361`_. -While flag states are loading, all resolvers return `false` so course -authoring remains hidden until enablement is known. +While flag states are loading, all resolvers return `false` so course authoring remains hidden until enablement is known. If the fetch fails, all resolvers also return `false` and a single error toast with a retry action is surfaced regardless of how many components consume the hook concurrently. **3. Apply gating only to client-owned UI elements.** -* **Role filter:** course-authoring roles require both the corresponding view - permission and `isCourseAuthoringEnabled`. -* **Scope filter:** course options are filtered with `isCourseEnabled`; - library scopes are always included. -* **Organization filter:** organizations are filtered with - `isOrgAuthoringEnabled` only for users without library-view permissions. - Users with library access—and all users while permissions are loading—see all - organizations. -* **Assignment wizard:** assignable course roles require both the appropriate - management permission and `isCourseAuthoringEnabled`. +* **Role filter:** course-authoring roles require both the corresponding view permission and `isCourseAuthoringEnabled`. +* **Scope filter:** course options are filtered with `isCourseEnabled`; library scopes are always included. +* **Organization filter:** organizations are filtered with `isOrgAuthoringEnabled` only for users without library-view permissions. Users with library access—and all users while permissions or flag states are loading—see all organizations. +* **Assignment wizard:** assignable course roles require both the appropriate management permission and `isCourseAuthoringEnabled`. **4. Leave paginated assignment data to the backend.** -The role, scope, and organization filters operate on small client-owned data -sets, making client-side filtering appropriate. +The role, scope, and organization filters operate on small client-owned data sets, making client-side filtering appropriate. -The assignment lists (`/assignments/` and the per-user assignment view) are -server-driven and paginated. Client-side filtering would invalidate pagination, -counts, and sorting. The backend therefore remains responsible for hiding -authoring-only assignments. As described in `ADR 0013`_, only authoring -assignments for enabled scopes are expected to exist in Casbin, so the frontend -renders the backend response without additional filtering. +The assignment lists (`/assignments/` and the per-user assignment view) are server-driven and paginated. Client-side filtering would invalidate pagination, counts, and sorting. The backend therefore remains responsible for hiding authoring-only assignments. As described in `ADR 0013`_, only authoring assignments for enabled scopes are expected to exist in Casbin, so the frontend renders the backend response without additional filtering. Implications and Assumptions ############################ -Behavior depends on -`ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION` (`ADR 0013`_). +Behavior depends on `ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION` (`ADR 0013`_). **When the setting is enabled (`True`)** -* Organization- and course-level flag changes synchronously migrate role - assignments between the legacy model and Casbin, keeping authorization data - largely aligned with the flag. -* Global flag changes are **not** migrated automatically. The flag-state - endpoint immediately reflects the new value, but Casbin data does not until - migration commands are executed. -* Migration may fail or complete only partially (`ADR 0013`_), creating - temporary inconsistencies between flag state and authorization data. +* Organization- and course-level flag changes synchronously migrate role assignments between the legacy model and Casbin, keeping authorization data largely aligned with the flag. +* Global flag changes are **not** migrated automatically. The flag-state endpoint immediately reflects the new value, but Casbin data does not until migration commands are executed. +* Migration may fail or complete only partially (`ADR 0013`_), creating temporary inconsistencies between flag state and authorization data. **When the setting is disabled (default, `False`)** * Flag changes never migrate authorization data automatically. -* The flag-state endpoint reflects runtime configuration, while Casbin continues - to reflect the most recent manual migration. -* The frontend uses the flag only to determine which UI controls are available. - Paginated assignment lists continue to reflect Casbin and may therefore appear - stale. This inconsistency cannot be resolved by the frontend. +* The flag-state endpoint reflects runtime configuration, while Casbin continues to reflect the most recent manual migration. +* The frontend uses the flag only to determine which UI controls are available. Paginated assignment lists continue to reflect Casbin and may therefore appear stale. This inconsistency cannot be resolved by the frontend. Assumptions *********** * `waffle-flag-states` is the authoritative runtime source of enablement. -* Permission validation and assignment lists reflect Casbin state, which may lag - behind the current flag configuration. +* Permission validation and assignment lists reflect Casbin state, which may lag behind the current flag configuration. * Content libraries are never gated by this flag. -* The frontend mirrors the backend's course-key parsing and override precedence - (course -> organization -> global). +* The frontend mirrors the backend's course-key parsing and override precedence (course -> organization -> global). + +Rejected Alternatives +##################### + +**Enforcing per-scope flag logic in release-blocking permission API endpoints (PR #361)** + This would have been the authoritative approach: the permission endpoints themselves would check the flag per scope and suppress disabled scopes from their responses. However, correctness and performance across the whole framework could not be validated in time for the Verawood release, per `PR #361's own comment thread`_. The per-scope logic (``is_scope_visible`` / ``has_visible_scope``) remains documented on the `PR #361`_ branch for a future cycle. + +**Inferring flag state from** ``/permissions/validate/me`` + Permission validation reflects Casbin role assignments, not the current Waffle flag value. As described in the Context section, these sources can temporarily diverge: Casbin data is populated by migration commands that may not run immediately (or at all when `ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION = False`). Relying on permission data to infer enablement would produce stale or incorrect UI state during and after flag changes. Temporary nature and future work ################################ -This enforcement exists only for the feature-flag rollout period. It is required -because enablement can temporarily diverge from migrated authorization data. +This enforcement exists only for the feature-flag rollout period. It is required because enablement can temporarily diverge from migrated authorization data. -Once the authz system is enabled by default and -`authz.enable_course_authoring` is deprecated (`ADR 0010`_), enablement will -be unconditional. At that point, the -`waffle-flag-states` endpoint, `useCourseAuthoringFlag`, and all -feature-flag-based UI gating should be removed, allowing the console to rely -solely on permission validation. +Once the authz system is enabled by default and `authz.enable_course_authoring` is deprecated (`ADR 0010`_), enablement will be unconditional. At that point, the `waffle-flag-states` endpoint, `useCourseAuthoringFlag`, and all feature-flag-based UI gating should be removed, allowing the console to rely solely on permission validation. +Once the release-blocking permission endpoints can safely enforce per-scope flag logic (`PR #361`_), the client-side filtering in `isCourseEnabled` / `isOrgAuthoringEnabled` can also be removed in favor of authoritative server-side enforcement. -## References +References +########## * `ADR 0010 - Course Authoring Flag`_ * `ADR 0013 - Course Authoring Automatic Migration`_ * `ADR 0007 - Enforcement Mechanisms (MFEs)`_ +* `ADR 0015 - Expose Course-Authoring Waffle Flag State via REST API`_ +* `Issue #340`_ and `Issue #341`_ — admin console shows authoring roles/scopes when flag is off +* `PR #358`_ — implements the ``waffle-flag-states`` endpoint +* `PR #361`_ — reverted attempt to enforce per-scope logic in permission endpoints .. _ADR 0010 - Course Authoring Flag: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0010-course-authoring-flag.rst .. _ADR 0010: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0010-course-authoring-flag.rst .. _ADR 0013 - Course Authoring Automatic Migration: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0013-course-authoring-automatic-migration.rst .. _ADR 0013: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0013-course-authoring-automatic-migration.rst .. _ADR 0007 - Enforcement Mechanisms (MFEs): https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0007-enforcement-mechanisms-mfe.rst +.. _ADR 0015 - Expose Course-Authoring Waffle Flag State via REST API: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0015-expose-course-authoring-waffle-flag-state-via-rest-api.rst +.. _ADR 0015: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0015-expose-course-authoring-waffle-flag-state-via-rest-api.rst +.. _Issue #340: https://github.com/openedx/openedx-authz/issues/340 +.. _Issue #341: https://github.com/openedx/openedx-authz/issues/341 +.. _PR #358: https://github.com/openedx/openedx-authz/pull/358 +.. _PR #361: https://github.com/openedx/openedx-authz/pull/361 +.. _PR #361's own comment thread: https://github.com/openedx/openedx-authz/pull/361#issuecomment-4967053225 diff --git a/src/authz-module/data/hooks.test.tsx b/src/authz-module/data/hooks.test.tsx index c2db5a8e..4b511bd2 100644 --- a/src/authz-module/data/hooks.test.tsx +++ b/src/authz-module/data/hooks.test.tsx @@ -15,6 +15,7 @@ import { useScopes, useUserAssignedRoles, useValidateUsers, + useCourseAuthoringFlagStates, } from './hooks'; jest.mock('@edx/frontend-platform/auth', () => ({ @@ -915,3 +916,41 @@ describe('useUserAssignedRoles', () => { expect(mockGet).toHaveBeenCalledTimes(2); }); }); + +describe('useCourseAuthoringFlagStates', () => { + const mockFlagStates = { + global: true, + org_overrides: { on: ['OrgA'], off: [] }, + course_overrides: { on: [], off: ['course-v1:OrgA+COURSE1+2024'] }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns flag states when API call succeeds', async () => { + mockHttpClient().mockReturnValue({ + get: jest.fn().mockResolvedValue({ data: mockFlagStates }), + }); + + const { result } = renderHook(() => useCourseAuthoringFlagStates(), { wrapper: createWrapper() }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data).toEqual(mockFlagStates); + const mockGetFn = getAuthenticatedHttpClient().get as jest.Mock; + expect(mockGetFn.mock.calls[0][0]).toContain('/api/authz/v1/waffle-flag-states/'); + }); + + it('returns error state when API call fails', async () => { + mockHttpClient().mockReturnValue({ + get: jest.fn().mockRejectedValue(new Error('Server error')), + }); + + const { result } = renderHook(() => useCourseAuthoringFlagStates(), { wrapper: createWrapper() }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + + expect(result.current.data).toBeUndefined(); + }); +}); diff --git a/src/authz-module/data/hooks.ts b/src/authz-module/data/hooks.ts index cd5d948c..3505705e 100644 --- a/src/authz-module/data/hooks.ts +++ b/src/authz-module/data/hooks.ts @@ -264,5 +264,7 @@ export const useUserAssignedRoles = ( export const useCourseAuthoringFlagStates = () => useQuery({ queryKey: authzQueryKeys.courseAuthoringFlagStates(), queryFn: () => getCourseAuthoringFlagStates(), + staleTime: 1000 * 60 * 30, + refetchOnWindowFocus: false, retry: false, }); diff --git a/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx b/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx index b5d3e761..2700bdc7 100644 --- a/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx +++ b/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx @@ -33,6 +33,7 @@ const flagStates = (overrides = {}) => ({ const statesHook = (overrides = {}) => ({ data: flagStates(), isLoading: false, + isError: false, error: null, refetch: jest.fn(), ...overrides, @@ -85,13 +86,14 @@ describe('useCourseAuthoringFlag', () => { it('surfaces a generic error toast and keeps resolving to false when the fetch fails', async () => { const error = new Error('Request failed'); - mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ data: undefined, error })); + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ data: undefined, isError: true, error })); const { result } = renderHook(() => useCourseAuthoringFlag(), { wrapper }); expect(await screen.findByRole('alert')).toBeInTheDocument(); expect(screen.getByText('Something went wrong on our end.')).toBeInTheDocument(); expect(logError).toHaveBeenCalledWith(error); + expect(result.current.isError).toBe(true); expect(result.current.isCourseAuthoringEnabled).toBe(false); expect(result.current.isCourseEnabled('course-v1:org1+A+2024')).toBe(false); expect(result.current.isOrgAuthoringEnabled('org1')).toBe(false); diff --git a/src/authz-module/hooks/useCourseAuthoringFlag.ts b/src/authz-module/hooks/useCourseAuthoringFlag.ts index ef5dbcab..7ff60753 100644 --- a/src/authz-module/hooks/useCourseAuthoringFlag.ts +++ b/src/authz-module/hooks/useCourseAuthoringFlag.ts @@ -34,7 +34,7 @@ const toastedErrors = new WeakSet(); */ export const useCourseAuthoringFlag = () => { const { - data: flagStates, isLoading, error, refetch, + data: flagStates, isLoading, isError, error, refetch, } = useCourseAuthoringFlagStates(); const { showErrorToast } = useToastManager(); @@ -80,6 +80,6 @@ export const useCourseAuthoringFlag = () => { }, [flagStates]); return { - isCourseAuthoringEnabled, isCourseEnabled, isOrgAuthoringEnabled, isLoading, + isCourseAuthoringEnabled, isCourseEnabled, isOrgAuthoringEnabled, isLoading, isError, }; }; diff --git a/src/authz-module/hooks/useViewTeamPermissions.test.ts b/src/authz-module/hooks/useViewTeamPermissions.test.ts new file mode 100644 index 00000000..3bd5c3d8 --- /dev/null +++ b/src/authz-module/hooks/useViewTeamPermissions.test.ts @@ -0,0 +1,50 @@ +import { renderHook } from '@testing-library/react'; +import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; +import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS } from '@src/authz-module/roles-permissions'; +import { useViewTeamPermissions } from './useViewTeamPermissions'; + +jest.mock('@src/data/hooks', () => ({ + useValidateUserPermissionsNonSuspense: jest.fn(), +})); + +const mockUsePermissions = useValidateUserPermissionsNonSuspense as jest.Mock; + +const permissionsData = ({ course, library }: { course: boolean; library: boolean }) => [ + { action: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_TEAM, allowed: course }, + { action: CONTENT_LIBRARY_PERMISSIONS.VIEW_LIBRARY_TEAM, allowed: library }, +]; + +describe('useViewTeamPermissions', () => { + beforeEach(() => { + mockUsePermissions.mockReturnValue({ data: permissionsData({ course: true, library: true }), isLoading: false }); + }); + + it('returns both flags allowed when permissions are granted', () => { + const { result } = renderHook(() => useViewTeamPermissions()); + expect(result.current.isCourseViewAllowed).toBe(true); + expect(result.current.isLibraryViewAllowed).toBe(true); + expect(result.current.isLoading).toBe(false); + }); + + it('returns isCourseViewAllowed false when VIEW_COURSE_TEAM is denied', () => { + mockUsePermissions.mockReturnValue({ data: permissionsData({ course: false, library: true }), isLoading: false }); + const { result } = renderHook(() => useViewTeamPermissions()); + expect(result.current.isCourseViewAllowed).toBe(false); + expect(result.current.isLibraryViewAllowed).toBe(true); + }); + + it('returns isLibraryViewAllowed false when VIEW_LIBRARY_TEAM is denied', () => { + mockUsePermissions.mockReturnValue({ data: permissionsData({ course: true, library: false }), isLoading: false }); + const { result } = renderHook(() => useViewTeamPermissions()); + expect(result.current.isCourseViewAllowed).toBe(true); + expect(result.current.isLibraryViewAllowed).toBe(false); + }); + + it('defaults both to false while permissions are loading', () => { + mockUsePermissions.mockReturnValue({ data: undefined, isLoading: true }); + const { result } = renderHook(() => useViewTeamPermissions()); + expect(result.current.isLoading).toBe(true); + expect(result.current.isCourseViewAllowed).toBe(false); + expect(result.current.isLibraryViewAllowed).toBe(false); + }); +}); From 843486df53c8d2c11b03b9a6d4f45a69518026c3 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 16 Jul 2026 17:50:07 -0500 Subject: [PATCH 07/10] fix: address feedback --- src/authz-module/components/TableCells.tsx | 14 ++++++++- .../components/TableControlBar/OrgFilter.tsx | 7 +++-- .../hooks/useCourseAuthoringFlag.ts | 15 ++++++---- .../team-members/TeamMembersTable.test.tsx | 29 +++++++++++++++---- .../team-members/TeamMembersTable.tsx | 8 +++-- 5 files changed, 57 insertions(+), 16 deletions(-) diff --git a/src/authz-module/components/TableCells.tsx b/src/authz-module/components/TableCells.tsx index 1daf4ad6..2d596463 100644 --- a/src/authz-module/components/TableCells.tsx +++ b/src/authz-module/components/TableCells.tsx @@ -19,6 +19,10 @@ import { RESOURCE_ICONS } from './constants'; import messages from './messages'; import ViewMoreLink from './ViewMoreLink'; +type ViewActionCellExtraProps = { + isCourseEnabled: (scope: string) => boolean; +}; + interface DataTableInstance { state?: { expanded?: Record; @@ -59,20 +63,27 @@ const NameCell = ({ row }: CellProps) => { return {row.original.fullName || row.original.username || ''}; }; -const ViewActionCell = ({ row }: CellProps) => { +const ViewActionCell = ({ row, isCourseEnabled }: CellProps & Partial) => { const { formatMessage } = useIntl(); const navigate = useNavigate(); const viewPath = `/authz/user/${row.original.username}`; + const isCourseScope = !row.original.role?.startsWith('lib') && !DJANGO_MANAGED_ROLES.includes(row.original.role); + const isDisabled = isCourseEnabled !== undefined && isCourseScope && !isCourseEnabled(row.original.scope); return ( navigate(viewPath)} + disabled={isDisabled} /> ); }; +const createViewActionCell = (extraProps: ViewActionCellExtraProps) => function customViewActionCell(cellProps) { + return ; +}; + const OrgCell = ({ value, row }: CellPropsWithValue) => { const { formatMessage } = useIntl(); return ( @@ -239,4 +250,5 @@ export { PermissionsCell, ViewAllPermissionsCell, createActionsCell, + createViewActionCell, }; diff --git a/src/authz-module/components/TableControlBar/OrgFilter.tsx b/src/authz-module/components/TableControlBar/OrgFilter.tsx index f6e115b0..7d4f1a3c 100644 --- a/src/authz-module/components/TableControlBar/OrgFilter.tsx +++ b/src/authz-module/components/TableControlBar/OrgFilter.tsx @@ -21,9 +21,10 @@ const OrgFilter = ({ }, } = useOrgs(searchValue, 1, DEFAULT_FILTER_PAGE_SIZE); - // Libraries span orgs and are always enabled, so they must keep their behavior: only - // filter orgs by the course-authoring flag for course-only users, and never while - // permissions or flag states are still loading (default to showing every org). + // Users with library access (including those with both library and course roles) must + // always see all orgs: library access is not gated by the course-authoring flag, + // so the flag must never be used to filter orgs for them. Only filter orgs by the + // flag for course-only users, and never while permissions or flag states are loading. const filterByAuthoringFlag = !isLoading && !isFlagLoading && !isLibraryViewAllowed; const filterChoices = useMemo(() => (orgsData?.results ?? []) diff --git a/src/authz-module/hooks/useCourseAuthoringFlag.ts b/src/authz-module/hooks/useCourseAuthoringFlag.ts index 7ff60753..e084d605 100644 --- a/src/authz-module/hooks/useCourseAuthoringFlag.ts +++ b/src/authz-module/hooks/useCourseAuthoringFlag.ts @@ -59,18 +59,23 @@ export const useCourseAuthoringFlag = () => { const global = flagStates?.global ?? false; const courseEnabled = (courseId: string): boolean => { - if (courseOn.has(courseId)) { return true; } - if (courseOff.has(courseId)) { return false; } + const courseFlagOverrideEnabled = courseOn.has(courseId); + const courseFlagOverrideDisabled = courseOff.has(courseId); + if (courseFlagOverrideEnabled) { return true; } + if (courseFlagOverrideDisabled) { return false; } const org = orgOf(courseId); - if (org && orgOn.has(org)) { return true; } - if (org && orgOff.has(org)) { return false; } + const orgFlagOverrideEnabled = !!org && orgOn.has(org); + const orgFlagOverrideDisabled = !!org && orgOff.has(org); + if (orgFlagOverrideEnabled) { return true; } + if (orgFlagOverrideDisabled) { return false; } return global; }; const orgsWithForcedOnCourse = new Set([...courseOn].map(orgOf)); const orgAuthoringEnabled = (org: string): boolean => { - if (orgOn.has(org)) { return true; } + const orgFlagOverrideEnabled = orgOn.has(org); + if (orgFlagOverrideEnabled) { return true; } if (orgsWithForcedOnCourse.has(org)) { return true; } if (orgOff.has(org)) { return false; } return global; diff --git a/src/authz-module/team-members/TeamMembersTable.test.tsx b/src/authz-module/team-members/TeamMembersTable.test.tsx index fda3a6fe..afcd062e 100644 --- a/src/authz-module/team-members/TeamMembersTable.test.tsx +++ b/src/authz-module/team-members/TeamMembersTable.test.tsx @@ -4,6 +4,7 @@ import { renderWithAllProviders } from '@src/setupTest'; import { useAllRoleAssignments, useOrgs, useScopes } from '@src/authz-module/data/hooks'; import type { GetAllRoleAssignmentsResponse } from '@src/authz-module/data/api'; import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import { LIBRARY_ROLE_KEYS } from '@src/authz-module/roles-permissions'; import { ToastManagerProvider } from '@src/components/ToastManager/ToastManagerContext'; import TeamMembersTable from './TeamMembersTable'; @@ -123,13 +124,11 @@ jest.mock('@edx/frontend-platform/logging', () => ({ })); jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ - useCourseAuthoringFlag: () => ({ - isCourseAuthoringEnabled: true, - isCourseEnabled: () => true, - isLoading: false, - }), + useCourseAuthoringFlag: jest.fn(), })); +const mockUseCourseAuthoringFlag = useCourseAuthoringFlag as jest.Mock; + jest.mock('@src/authz-module/data/hooks', () => ({ useAllRoleAssignments: jest.fn(), useOrgs: jest.fn(), @@ -154,6 +153,11 @@ describe('TeamMembersTable', () => { isLibraryViewAllowed: true, isLoading: false, }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: () => true, + isLoading: false, + }); }); it('renders table with role assignments data', async () => { @@ -242,6 +246,21 @@ describe('TeamMembersTable', () => { }); }); + it('disables the view action for course assignments in disabled scopes', async () => { + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: (scope: string) => scope !== 'course-v1:OpenedX+DemoX+DemoCourse', + isLoading: false, + }); + mockApiResponses(); + renderWithAllProviders(); + await waitFor(() => { + const viewButtons = screen.getAllByRole('button', { name: /view/i }); + expect(viewButtons[0]).toBeDisabled(); + expect(viewButtons[1]).not.toBeDisabled(); + }); + }); + it('handles empty data gracefully', async () => { const allAsignmentsResponse = { data: { diff --git a/src/authz-module/team-members/TeamMembersTable.tsx b/src/authz-module/team-members/TeamMembersTable.tsx index a683e0ff..d14cd988 100644 --- a/src/authz-module/team-members/TeamMembersTable.tsx +++ b/src/authz-module/team-members/TeamMembersTable.tsx @@ -9,6 +9,7 @@ import { import { useToastManager } from '@src/components/ToastManager/ToastManagerContext'; import { LIBRARY_ROLE_KEYS } from '@src/authz-module/roles-permissions'; import { useViewTeamPermissions } from '@src/authz-module/hooks/useViewTeamPermissions'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import { useQuerySettings } from '@src/authz-module/hooks/useQuerySettings'; import OrgFilter from '@src/authz-module/components/TableControlBar/OrgFilter'; import RolesFilter from '@src/authz-module/components/TableControlBar/RolesFilter'; @@ -16,7 +17,7 @@ import ScopesFilter from '@src/authz-module/components/TableControlBar/ScopesFil import TableControlBar from '@src/authz-module/components/TableControlBar/TableControlBar'; import { getCellHeader } from '@src/authz-module/utils'; import { - ViewActionCell, NameCell, OrgCell, RoleCell, ScopeCell, + createViewActionCell, NameCell, OrgCell, RoleCell, ScopeCell, } from '@src/authz-module/components/TableCells'; import { useAllRoleAssignments } from '@src/authz-module/data/hooks'; import { TABLE_DEFAULT_PAGE_SIZE } from '@src/authz-module/constants'; @@ -46,6 +47,7 @@ const TeamMembersTable = ({ presetScope }: TeamMembersTableProps) => { const { querySettings, handleTableFetch } = useQuerySettings(initialQuerySettings); const { isCourseViewAllowed } = useViewTeamPermissions(); + const { isCourseEnabled } = useCourseAuthoringFlag(); const effectiveQuerySettings = useMemo(() => { if (isCourseViewAllowed || querySettings.roles) { return querySettings; } @@ -59,6 +61,8 @@ const TeamMembersTable = ({ presetScope }: TeamMembersTableProps) => { refetch, } = useAllRoleAssignments(effectiveQuerySettings); + const viewActionCell = useMemo(() => createViewActionCell({ isCourseEnabled }), [isCourseEnabled]); + const initialFilters = presetScope ? [{ id: 'scope', value: [presetScope] }] : []; useEffect(() => { @@ -93,7 +97,7 @@ const TeamMembersTable = ({ presetScope }: TeamMembersTableProps) => { { id: 'action', Header: intl.formatMessage(messages['authz.team.members.table.column.actions.title']), - Cell: ViewActionCell, + Cell: viewActionCell, }, ]} columns={ From ddfbb53a17398b0f8f654146900b4c406ad714da Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 17 Jul 2026 13:02:19 -0500 Subject: [PATCH 08/10] test: add tests for course authoring flag behavior --- .../TableControlBar/ScopesFilter.test.tsx | 37 ++++ .../hooks/useCourseAuthoringFlag.test.tsx | 175 ++++++++++++++++++ 2 files changed, 212 insertions(+) diff --git a/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx b/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx index aaeafdf4..3a0bf436 100644 --- a/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx +++ b/src/authz-module/components/TableControlBar/ScopesFilter.test.tsx @@ -140,4 +140,41 @@ describe('ScopesFilter', () => { expect(await screen.findByText('Test Library')).toBeInTheDocument(); expect(screen.queryByText('Test Course')).not.toBeInTheDocument(); }); + + it('hides only the flag-disabled course while keeping enabled courses and libraries (mixed per-course)', async () => { + const user = userEvent.setup(); + mockUseScopes.mockReturnValue({ + data: { + pages: [{ + results: [ + { + externalKey: 'course-v1:org+enabled+run', + displayName: 'Enabled Course', + org: { shortName: 'TestOrg' }, + }, + { + externalKey: 'course-v1:org+disabled+run', + displayName: 'Disabled Course', + org: { shortName: 'TestOrg' }, + }, + { + externalKey: 'lib:org:library', + displayName: 'Test Library', + org: { shortName: 'TestOrg' }, + }, + ], + }], + }, + }); + mockUseCourseAuthoringFlag.mockReturnValue({ + isCourseAuthoringEnabled: true, + isCourseEnabled: (id: string) => id === 'course-v1:org+enabled+run', + isLoading: false, + }); + renderWrapper(); + await user.click(screen.getByRole('button', { name: /Scopes/i })); + expect(await screen.findByText('Enabled Course')).toBeInTheDocument(); + expect(screen.queryByText('Disabled Course')).not.toBeInTheDocument(); + expect(screen.getByText('Test Library')).toBeInTheDocument(); + }); }); diff --git a/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx b/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx index 2700bdc7..08d537b3 100644 --- a/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx +++ b/src/authz-module/hooks/useCourseAuthoringFlag.test.tsx @@ -39,6 +39,30 @@ const statesHook = (overrides = {}) => ({ ...overrides, }); +// Cascade-test fixtures +const ORG = 'TestOrg'; +const OTHER_ORG = 'OtherOrg'; +const COURSE = `course-v1:${ORG}+C1+2024`; +const OTHER_COURSE = `course-v1:${ORG}+C2+2024`; +const OTHER_ORG_COURSE = `course-v1:${OTHER_ORG}+C1+2024`; + +const setup = ( + global: boolean, + orgOn: string[] = [], + orgOff: string[] = [], + courseOn: string[] = [], + courseOff: string[] = [], +) => { + mockUseCourseAuthoringFlagStates.mockReturnValue(statesHook({ + data: flagStates({ + global, + orgOverrides: { on: orgOn, off: orgOff }, + courseOverrides: { on: courseOn, off: courseOff }, + }), + })); + return renderHook(() => useCourseAuthoringFlag(), { wrapper }); +}; + describe('useCourseAuthoringFlag', () => { beforeEach(() => { jest.clearAllMocks(); @@ -136,4 +160,155 @@ describe('useCourseAuthoringFlag', () => { }); }); }); + + // Parametric truth-table tests for the cascade resolution logic. + // Each test corresponds to one or more rows from the manual test table documented in + // https://github.com/openedx/frontend-app-admin-console/pull/176#issuecomment-4995133343 + // Each test covers two manual rows at once — the "permission=No" and "permission=Yes" + // variants of the same flag configuration — because the flag resolvers produce identical + // output regardless of Casbin state. The "User Has Permission?" dimension is tested + // separately via useViewTeamPermissions and the role/scope filter component tests. + describe('cascade resolution — truth table', () => { + // Mirrors the "Platform flag" section of the manual test table + describe('platform flag only (no overrides)', () => { + it('global off → authoring disabled for all courses and orgs', () => { // [Platform: off / no overrides] + const { result } = setup(false); + expect(result.current.isCourseAuthoringEnabled).toBe(false); + expect(result.current.isCourseEnabled(COURSE)).toBe(false); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(false); + }); + + it('global on → authoring enabled for all courses and orgs', () => { // [Platform: on / no overrides] + const { result } = setup(true); + expect(result.current.isCourseAuthoringEnabled).toBe(true); + expect(result.current.isCourseEnabled(COURSE)).toBe(true); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); + }); + }); + + // Mirrors the "ORG Override — Force On" rows of the manual test table + describe('org Force On override', () => { + it('global off + org Force On → overridden org enabled, other orgs remain off', () => { // [Org: platform off + org Force On] + const { result } = setup(false, [ORG]); + expect(result.current.isCourseAuthoringEnabled).toBe(true); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); + expect(result.current.isOrgAuthoringEnabled(OTHER_ORG)).toBe(false); + expect(result.current.isCourseEnabled(COURSE)).toBe(true); + expect(result.current.isCourseEnabled(OTHER_ORG_COURSE)).toBe(false); + }); + + it('global on + org Force On → all orgs and courses enabled', () => { // [Org: platform on + org Force On] + const { result } = setup(true, [ORG]); + expect(result.current.isCourseAuthoringEnabled).toBe(true); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); + expect(result.current.isOrgAuthoringEnabled(OTHER_ORG)).toBe(true); + expect(result.current.isCourseEnabled(COURSE)).toBe(true); + expect(result.current.isCourseEnabled(OTHER_ORG_COURSE)).toBe(true); + }); + }); + + // Mirrors the "ORG Override — Force Off" rows of the manual test table + describe('org Force Off override', () => { + it('global off + org Force Off → authoring disabled everywhere', () => { // [Org: platform off + org Force Off] + const { result } = setup(false, [], [ORG]); + expect(result.current.isCourseAuthoringEnabled).toBe(false); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(false); + expect(result.current.isCourseEnabled(COURSE)).toBe(false); + }); + + it('global on + org Force Off → overridden org disabled, other orgs remain on', () => { // [Org: platform on + org Force Off] + const { result } = setup(true, [], [ORG]); + expect(result.current.isCourseAuthoringEnabled).toBe(true); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(false); + expect(result.current.isOrgAuthoringEnabled(OTHER_ORG)).toBe(true); + expect(result.current.isCourseEnabled(COURSE)).toBe(false); + expect(result.current.isCourseEnabled(OTHER_ORG_COURSE)).toBe(true); + }); + }); + + // Mirrors the "Course Override — Force On" rows of the manual test table + describe('course Force On override', () => { + it('global off + course Force On → overridden course enabled, other courses in the org remain off', () => { // [Course: platform off + course Force On] + const { result } = setup(false, [], [], [COURSE]); + expect(result.current.isCourseAuthoringEnabled).toBe(true); + expect(result.current.isCourseEnabled(COURSE)).toBe(true); + expect(result.current.isCourseEnabled(OTHER_COURSE)).toBe(false); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); // has a forced-on course + expect(result.current.isOrgAuthoringEnabled(OTHER_ORG)).toBe(false); + }); + + it('global on + course Force On → all courses enabled', () => { // [Course: platform on + course Force On] + const { result } = setup(true, [], [], [COURSE]); + expect(result.current.isCourseAuthoringEnabled).toBe(true); + expect(result.current.isCourseEnabled(COURSE)).toBe(true); + expect(result.current.isCourseEnabled(OTHER_COURSE)).toBe(true); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); + }); + }); + + // Mirrors the "Course Override — Force Off" rows of the manual test table + describe('course Force Off override', () => { + it('global off + course Force Off → authoring disabled everywhere', () => { // [Course: platform off + course Force Off] + const { result } = setup(false, [], [], [], [COURSE]); + expect(result.current.isCourseAuthoringEnabled).toBe(false); + expect(result.current.isCourseEnabled(COURSE)).toBe(false); + expect(result.current.isCourseEnabled(OTHER_COURSE)).toBe(false); + }); + + it('global on + course Force Off → overridden course disabled, other courses remain on', () => { // [Course: platform on + course Force Off] + const { result } = setup(true, [], [], [], [COURSE]); + expect(result.current.isCourseAuthoringEnabled).toBe(true); + expect(result.current.isCourseEnabled(COURSE)).toBe(false); + expect(result.current.isCourseEnabled(OTHER_COURSE)).toBe(true); + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); + }); + }); + + // Mirrors the "Override Priority (Cascade)" section of the manual test table + describe('cascade priority — course override beats org override beats platform', () => { + it('platform on + org Force Off + course Force On → course override wins, course enabled', () => { // [Cascade: platform on + org Force Off + course Force On → course wins] + const { result } = setup(true, [], [ORG], [COURSE]); + expect(result.current.isCourseEnabled(COURSE)).toBe(true); // course override wins + expect(result.current.isCourseEnabled(OTHER_COURSE)).toBe(false); // org off, no course override + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); // has a forced-on course + }); + + it('platform on + org Force On + course Force Off → course override wins, course disabled', () => { // [Cascade: platform on + org Force On + course Force Off → course wins] + const { result } = setup(true, [ORG], [], [], [COURSE]); + expect(result.current.isCourseEnabled(COURSE)).toBe(false); // course override wins + expect(result.current.isCourseEnabled(OTHER_COURSE)).toBe(true); // inherits org on + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); // org override on + }); + + it('platform off + org Force On + course Force Off → course override wins, course disabled', () => { // [Cascade: platform off + org Force On + course Force Off → course wins] + const { result } = setup(false, [ORG], [], [], [COURSE]); + expect(result.current.isCourseAuthoringEnabled).toBe(true); // org override is on + expect(result.current.isCourseEnabled(COURSE)).toBe(false); // course override wins + expect(result.current.isCourseEnabled(OTHER_COURSE)).toBe(true); // inherits org on + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); // org override on + }); + + it('platform off + org Force Off + course Force On → course override wins, course enabled', () => { // [Cascade: platform off + org Force Off + course Force On → course wins] + const { result } = setup(false, [], [ORG], [COURSE]); + expect(result.current.isCourseAuthoringEnabled).toBe(true); // course override is on + expect(result.current.isCourseEnabled(COURSE)).toBe(true); // course override wins + expect(result.current.isCourseEnabled(OTHER_COURSE)).toBe(false); // org off, no course override + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); // has a forced-on course + }); + + it('platform off + org Force On → org override wins over platform, org courses enabled', () => { // [Cascade: platform off + org Force On → org wins over platform] + const { result } = setup(false, [ORG]); + expect(result.current.isCourseEnabled(COURSE)).toBe(true); // inherits org on + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(true); + expect(result.current.isOrgAuthoringEnabled(OTHER_ORG)).toBe(false); // global off, no override + }); + + it('platform on + org Force Off → org override wins over platform, org courses disabled', () => { // [Cascade: platform on + org Force Off → org wins over platform] + const { result } = setup(true, [], [ORG]); + expect(result.current.isCourseEnabled(COURSE)).toBe(false); // inherits org off + expect(result.current.isOrgAuthoringEnabled(ORG)).toBe(false); + expect(result.current.isOrgAuthoringEnabled(OTHER_ORG)).toBe(true); // global on, no override + }); + }); + }); }); From 697bc02057b86a586273522c98bc4b1414388780 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Fri, 17 Jul 2026 15:23:26 +1000 Subject: [PATCH 09/10] fix: disabled actions from user table --- src/authz-module/audit-user/index.test.tsx | 60 +++++++++++++++++++++- src/authz-module/audit-user/index.tsx | 7 ++- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/authz-module/audit-user/index.test.tsx b/src/authz-module/audit-user/index.test.tsx index 7fad65d7..4d2368cf 100644 --- a/src/authz-module/audit-user/index.test.tsx +++ b/src/authz-module/audit-user/index.test.tsx @@ -8,7 +8,7 @@ import { mockHttpClient, mockAppContext } from '@src/setupTest'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { ToastManagerProvider } from '@src/components/ToastManager/ToastManagerContext'; -import { useUserAccount } from '@src/data/hooks'; +import { useUserAccount, useValidateUserPermissionsNonSuspense } from '@src/data/hooks'; import { useUserAssignedRoles } from '@src/authz-module/data/hooks'; import AuditUserPage from './index'; @@ -46,10 +46,11 @@ jest.mock('@src/authz-module/hooks/useViewTeamPermissions', () => ({ }), })); +const mockIsCourseEnabled = jest.fn(() => true); jest.mock('@src/authz-module/hooks/useCourseAuthoringFlag', () => ({ useCourseAuthoringFlag: () => ({ isCourseAuthoringEnabled: true, - isCourseEnabled: () => true, + isCourseEnabled: mockIsCourseEnabled, isOrgAuthoringEnabled: () => true, isLoading: false, }), @@ -114,6 +115,7 @@ const renderWithRouter = (route = '/audit/johndoe') => { describe('AuditUserPage', () => { beforeEach(() => { jest.clearAllMocks(); + mockIsCourseEnabled.mockReturnValue(true); // Set up default mock behavior for useRevokeUserRoles mockRevokeUserRoles.mockImplementation((_variables, { onSuccess }) => { // Simulate successful deletion by default @@ -570,4 +572,58 @@ describe('AuditUserPage', () => { expect(screen.getByText('Home Page')).toBeInTheDocument(); }); }); + + describe('course authoring flag', () => { + const courseScope = 'course-v1:TestOrg+C101+2026'; + const mockCourseAssignments = { + count: 1, + results: [ + { + id: '1', + role: 'course_staff', + org: 'Test Org', + scope: courseScope, + permissionCount: 5, + }, + ], + next: null, + previous: null, + }; + + beforeEach(() => { + (useUserAccount as jest.Mock).mockReturnValue({ + data: mockUser, + isLoading: false, + isError: false, + error: null, + }); + (useUserAssignedRoles as jest.Mock).mockReturnValue({ + data: mockCourseAssignments, + isLoading: false, + }); + (useValidateUserPermissionsNonSuspense as jest.Mock).mockReturnValue({ + data: [{ scope: courseScope, allowed: true }], + isLoading: false, + }); + }); + + it('disables the delete action for a course assignment when authoring is disabled for the course', async () => { + mockIsCourseEnabled.mockReturnValue(false); + + renderWithRouter(); + + await waitFor(() => { + expect(screen.getByRole('button', { name: /delete role action/i })).toBeDisabled(); + }); + expect(mockIsCourseEnabled).toHaveBeenCalledWith(courseScope); + }); + + it('keeps the delete action enabled for a course assignment when authoring is enabled for the course', async () => { + renderWithRouter(); + + await waitFor(() => { + expect(screen.getByRole('button', { name: /delete role action/i })).toBeEnabled(); + }); + }); + }); }); diff --git a/src/authz-module/audit-user/index.tsx b/src/authz-module/audit-user/index.tsx index b5aaa221..93a2e7c0 100644 --- a/src/authz-module/audit-user/index.tsx +++ b/src/authz-module/audit-user/index.tsx @@ -23,6 +23,7 @@ import { createActionsCell, } from '@src/authz-module/components/TableCells'; import { useQuerySettings } from '@src/authz-module/hooks/useQuerySettings'; +import { useCourseAuthoringFlag } from '@src/authz-module/hooks/useCourseAuthoringFlag'; import { useRevokeUserRoles, useUserAssignedRoles } from '@src/authz-module/data/hooks'; import { RoleToDelete } from '@src/types'; import { useToastManager } from '@src/components/ToastManager/ToastManagerContext'; @@ -46,6 +47,7 @@ const AuditUserPage = () => { const { querySettings, handleTableFetch } = useQuerySettings(); const { isCourseViewAllowed } = useViewTeamPermissions(); + const { isCourseEnabled } = useCourseAuthoringFlag(); const effectiveQuerySettings = useMemo(() => { if (isCourseViewAllowed || querySettings.roles) { return querySettings; } @@ -75,15 +77,16 @@ const AuditUserPage = () => { if (!permissionsToManageScope) { return userAssignments; } return userAssignments.map(assignment => { + // Library roles are always manageable; course roles only when the authoring flag is enabled for the course. const canManageScope = permissionsToManageScope.some( permission => permission.scope === assignment.scope && permission.allowed, - ); + ) && (assignment.scope.startsWith('lib') || isCourseEnabled(assignment.scope)); return { ...assignment, canManageScope, }; }); - }, [userAssignments, permissionsToManageScope]); + }, [userAssignments, permissionsToManageScope, isCourseEnabled]); const fetchData = useMemo(() => handleTableFetch, [handleTableFetch]); From 3ea9c34bc52bfc0fe8ddcf783ff60f574420abe6 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Mon, 20 Jul 2026 15:33:07 +1000 Subject: [PATCH 10/10] feat: add a tooltip that explains why actions is disabled --- ...0003-course-authoring-flag-enforcement.rst | 3 +- src/authz-module/audit-user/index.tsx | 8 +-- .../components/TableCells.test.tsx | 48 +++++++++++++++ src/authz-module/components/TableCells.tsx | 61 ++++++++++++++++++- src/authz-module/components/messages.ts | 5 ++ 5 files changed, 117 insertions(+), 8 deletions(-) diff --git a/docs/decisions/0003-course-authoring-flag-enforcement.rst b/docs/decisions/0003-course-authoring-flag-enforcement.rst index e7d2c25d..63aee3a3 100644 --- a/docs/decisions/0003-course-authoring-flag-enforcement.rst +++ b/docs/decisions/0003-course-authoring-flag-enforcement.rst @@ -85,6 +85,7 @@ While flag states are loading, all resolvers return `false` so course authoring * **Scope filter:** course options are filtered with `isCourseEnabled`; library scopes are always included. * **Organization filter:** organizations are filtered with `isOrgAuthoringEnabled` only for users without library-view permissions. Users with library access—and all users while permissions or flag states are loading—see all organizations. * **Assignment wizard:** assignable course roles require both the appropriate management permission and `isCourseAuthoringEnabled`. +* **Assignment table row actions:** the per-row view and delete actions are disabled — with a tooltip pointing users to Studio — when the row's course scope resolves to disabled via `isCourseEnabled`. Library rows are unaffected. **4. Leave paginated assignment data to the backend.** @@ -107,7 +108,7 @@ Behavior depends on `ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION` (`ADR 00 * Flag changes never migrate authorization data automatically. * The flag-state endpoint reflects runtime configuration, while Casbin continues to reflect the most recent manual migration. -* The frontend uses the flag only to determine which UI controls are available. Paginated assignment lists continue to reflect Casbin and may therefore appear stale. This inconsistency cannot be resolved by the frontend. +* The frontend uses the flag only to determine which UI controls are available. Paginated assignment lists continue to reflect Casbin and may therefore appear stale. The frontend cannot hide such rows without breaking pagination, but it disables their row actions (see Decision 3) so stale course assignments cannot be managed from the console. Assumptions *********** diff --git a/src/authz-module/audit-user/index.tsx b/src/authz-module/audit-user/index.tsx index 93a2e7c0..aa7666a3 100644 --- a/src/authz-module/audit-user/index.tsx +++ b/src/authz-module/audit-user/index.tsx @@ -77,16 +77,15 @@ const AuditUserPage = () => { if (!permissionsToManageScope) { return userAssignments; } return userAssignments.map(assignment => { - // Library roles are always manageable; course roles only when the authoring flag is enabled for the course. const canManageScope = permissionsToManageScope.some( permission => permission.scope === assignment.scope && permission.allowed, - ) && (assignment.scope.startsWith('lib') || isCourseEnabled(assignment.scope)); + ); return { ...assignment, canManageScope, }; }); - }, [userAssignments, permissionsToManageScope, isCourseEnabled]); + }, [userAssignments, permissionsToManageScope]); const fetchData = useMemo(() => handleTableFetch, [handleTableFetch]); @@ -125,9 +124,10 @@ const AuditUserPage = () => { Cell: createActionsCell({ onClickDeleteButton: handleShowConfirmDeletionModal, isUserAuthenticatedPage: username === authenticatedUser?.username, + isCourseEnabled, }), }, - ], [authenticatedUser?.username, formatMessage, handleShowConfirmDeletionModal, username]); + ], [authenticatedUser?.username, formatMessage, handleShowConfirmDeletionModal, username, isCourseEnabled]); const columns = useMemo(() => [ { diff --git a/src/authz-module/components/TableCells.test.tsx b/src/authz-module/components/TableCells.test.tsx index af121e36..397e39a8 100644 --- a/src/authz-module/components/TableCells.test.tsx +++ b/src/authz-module/components/TableCells.test.tsx @@ -262,6 +262,17 @@ describe('TableCells Components', () => { expect(mockNavigate).toHaveBeenCalledWith('/authz/user/user+with@special.chars'); }); + + it('disables the view action and shows a tooltip when course authoring is disabled for the course', async () => { + const user = userEvent.setup(); + renderWrapper( false} />); + + const viewButton = screen.getByRole('button', { name: /view/i }); + expect(viewButton).toBeDisabled(); + + await user.hover(viewButton); + expect(screen.getByText(/manage its team in Studio instead/i)).toBeInTheDocument(); + }); }); describe('RoleCell', () => { @@ -608,6 +619,43 @@ describe('TableCells Components', () => { const deleteButton = screen.queryByRole('button', { name: /delete role action/i }); expect(deleteButton).toBeDisabled(); }); + + it('renders a disabled delete button with a tooltip when course authoring is disabled for the course', async () => { + const user = userEvent.setup(); + const CustomActionsCell = createActionsCell({ + onClickDeleteButton: mockOnClickDeleteButton, + isUserAuthenticatedPage: false, + isCourseEnabled: () => false, + }); + const courseRow = { + original: { + role: 'course_staff', + org: 'Test Org', + scope: 'course-v1:TestOrg+C101+2026', + permissionCount: 1, + canManageScope: true, + }, + }; + renderWrapper(); + + const deleteButton = screen.getByRole('button', { name: /delete role action/i }); + expect(deleteButton).toBeDisabled(); + + await user.hover(deleteButton); + expect(screen.getByText(/manage its team in Studio instead/i)).toBeInTheDocument(); + }); + + it('keeps the delete action enabled for library roles when course authoring is disabled', () => { + const CustomActionsCell = createActionsCell({ + onClickDeleteButton: mockOnClickDeleteButton, + isUserAuthenticatedPage: false, + isCourseEnabled: () => false, + }); + renderWrapper(); + + const deleteButton = screen.getByRole('button', { name: /delete role action/i }); + expect(deleteButton).toBeEnabled(); + }); }); describe('ViewAllPermissionsCell', () => { diff --git a/src/authz-module/components/TableCells.tsx b/src/authz-module/components/TableCells.tsx index 2d596463..0b7febba 100644 --- a/src/authz-module/components/TableCells.tsx +++ b/src/authz-module/components/TableCells.tsx @@ -7,7 +7,7 @@ import { } from '@openedx/paragon/icons'; import { UserRoleWithPermissions, RoleToDelete } from '@src/types'; import { useNavigate } from 'react-router-dom'; -import { useContext, useMemo } from 'react'; +import { useContext, useMemo, type ComponentProps } from 'react'; import { ADMIN_ROLES, DJANGO_MANAGED_ROLES, MAP_ROLE_KEY_TO_LABEL, } from '@src/authz-module/constants'; @@ -43,10 +43,41 @@ type ExtendedCellProps = CellPropsWithValue & { type ActionsCellExtraProps = { onClickDeleteButton: (role: RoleToDelete) => void; isUserAuthenticatedPage: boolean; + isCourseEnabled?: (scope: string) => boolean; }; type ActionsCellProps = CellProps & ActionsCellExtraProps; +type DisabledCourseActionButtonProps = Pick, 'src' | 'alt' | 'size' | 'variant'>; + +// A disabled button can't trigger its own tooltip (Paragon sets pointer-events: none on it), +// so the OverlayTrigger must live on a wrapper element that still receives hover events. +const DisabledCourseActionButton = ({ + src, alt, size, variant, +}: DisabledCourseActionButtonProps) => { + const { formatMessage } = useIntl(); + return ( + + {formatMessage(messages['authz.table.actions.course.disabled.tooltip'])} + + )} + > + + + + + ); +}; + const NameCell = ({ row }: CellProps) => { const intl = useIntl(); const { authenticatedUser } = useContext(AppContext); @@ -69,13 +100,23 @@ const ViewActionCell = ({ row, isCourseEnabled }: CellProps & Partial + ); + } + return ( navigate(viewPath)} - disabled={isDisabled} /> ); }; @@ -176,7 +217,7 @@ const ViewAllPermissionsCell = ({ row }: CellProps) => { }; const ActionsCell = ({ - row, onClickDeleteButton, isUserAuthenticatedPage, + row, onClickDeleteButton, isUserAuthenticatedPage, isCourseEnabled, }: ActionsCellProps) => { const { formatMessage } = useIntl(); const { role, canManageScope } = row.original; @@ -226,6 +267,20 @@ const ActionsCell = ({ ); } + const isCourseScope = !role?.startsWith('lib'); + const isCourseAuthoringDisabled = isCourseEnabled !== undefined + && isCourseScope && !isCourseEnabled(row.original.scope); + + if (isCourseAuthoringDisabled) { + return ( + + ); + } + return (