From bf1f32a4df9243415460409aa8c255f1fd656790 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Mon, 27 Apr 2026 18:41:15 -0500 Subject: [PATCH 01/10] feat(pages-and-resources): add read-only access for course_auditor role - Add VIEW_ADVANCED_SETTINGS and PAGE_AND_RESOURCES permissions - Add getPagesAndResourcesPermissions helper - Calculate isEditable and isReadOnly from user permissions - Propagate isEditable via PagesAndResourcesContext - Add disabled={!isEditable} to all forms, toggles, and buttons - Update AppCard and AppListNextButton with isEditable logic - Change default isEditable to false (fail closed) - Add unified permission gate showing PermissionDeniedAlert --- plugins/course-apps/progress/Settings.jsx | 5 ++- src/CourseAuthoringPage.tsx | 8 +--- src/advanced-settings/AdvancedSettings.tsx | 21 ++++++++- .../setting-card/SettingCard.tsx | 7 +++ src/authz/constants.ts | 4 ++ src/authz/permissionHelpers.ts | 11 +++++ src/pages-and-resources/PagesAndResources.tsx | 43 ++++++++++++++++--- .../PagesAndResourcesProvider.tsx | 11 +++-- .../app-settings-modal/AppSettingsModal.jsx | 4 +- .../app-config-form/AppConfigForm.jsx | 5 ++- .../AppConfigFormSaveButton.jsx | 4 +- .../apps/lti/LtiConfigForm.jsx | 9 +++- .../apps/openedx/OpenedXConfigForm.jsx | 24 ++++++++--- .../apps/shared/InContextDiscussionFields.jsx | 10 ++++- .../discussions/app-list/AppCard.jsx | 10 +++-- .../discussions/app-list/AppList.jsx | 4 +- .../app-list/AppListNextButton.jsx | 3 ++ src/pages-and-resources/pages/PageCard.jsx | 5 ++- src/pages-and-resources/pages/PageGrid.jsx | 6 ++- .../pages/PageSettingButton.jsx | 21 ++++++++- 20 files changed, 177 insertions(+), 38 deletions(-) diff --git a/plugins/course-apps/progress/Settings.jsx b/plugins/course-apps/progress/Settings.jsx index 1f01c56c79..7e3aecf1ed 100644 --- a/plugins/course-apps/progress/Settings.jsx +++ b/plugins/course-apps/progress/Settings.jsx @@ -1,17 +1,19 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import PropTypes from 'prop-types'; -import React from 'react'; +import React, { useContext } from 'react'; import * as Yup from 'yup'; import { getConfig } from '@edx/frontend-platform'; import FormSwitchGroup from 'CourseAuthoring/generic/FormSwitchGroup'; import { useAppSetting } from 'CourseAuthoring/utils'; import AppSettingsModal from 'CourseAuthoring/pages-and-resources/app-settings-modal/AppSettingsModal'; +import { PagesAndResourcesContext } from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; import messages from './messages'; const ProgressSettings = ({ onClose }) => { const intl = useIntl(); const [disableProgressGraph, saveSetting] = useAppSetting('disableProgressGraph'); const showProgressGraphSetting = getConfig().ENABLE_PROGRESS_GRAPH_SETTINGS.toString().toLowerCase() === 'true'; + const { isEditable = false } = useContext(PagesAndResourcesContext); const handleSettingsSave = async (values) => { if (showProgressGraphSetting) { await saveSetting(!values.enableProgressGraph); } @@ -39,6 +41,7 @@ const ProgressSettings = ({ onClose }) => { onChange={handleChange} onBlur={handleBlur} checked={values.enableProgressGraph} + disabled={!isEditable} /> ) )} diff --git a/src/CourseAuthoringPage.tsx b/src/CourseAuthoringPage.tsx index 7e6d4055bd..4d6da9c65b 100644 --- a/src/CourseAuthoringPage.tsx +++ b/src/CourseAuthoringPage.tsx @@ -1,5 +1,5 @@ import React, { useEffect } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; +import { useDispatch } from 'react-redux'; import { useLocation, @@ -7,9 +7,7 @@ import { import { StudioFooterSlot } from '@edx/frontend-component-footer'; import Header from './header'; import NotFoundAlert from './generic/NotFoundAlert'; -import PermissionDeniedAlert from './generic/PermissionDeniedAlert'; import { fetchOnlyStudioHomeData } from './studio-home/data/thunks'; -import { getCourseAppsApiStatus } from './pages-and-resources/data/selectors'; import { RequestStatus } from './data/constants'; import Loading from './generic/Loading'; import { useCourseAuthoringContext } from './CourseAuthoringContext'; @@ -30,16 +28,12 @@ const CourseAuthoringPage = ({ children }: Props) => { const courseOrg = courseDetails?.org; const courseTitle = courseDetails?.name; const inProgress = courseDetailStatus === RequestStatus.IN_PROGRESS || courseDetailStatus === RequestStatus.PENDING; - const courseAppsApiStatus = useSelector(getCourseAppsApiStatus); const { pathname } = useLocation(); const isEditor = pathname.includes('/editor'); if (courseDetailStatus === RequestStatus.NOT_FOUND && !isEditor) { return ; } - if (courseAppsApiStatus === RequestStatus.DENIED) { - return ; - } return (
{ diff --git a/src/advanced-settings/AdvancedSettings.tsx b/src/advanced-settings/AdvancedSettings.tsx index ef5d37d921..0d56bbb386 100644 --- a/src/advanced-settings/AdvancedSettings.tsx +++ b/src/advanced-settings/AdvancedSettings.tsx @@ -51,6 +51,10 @@ const AdvancedSettings = () => { action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS, scope: courseId, }, + canViewAdvancedSettings: { + action: COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS, + scope: courseId, + }, }, isAuthzEnabled); const { @@ -148,15 +152,29 @@ const AdvancedSettings = () => { showSaveSettingsPrompt(true); }; - // Show permission denied alert when authz is enabled and user doesn't have permission + // Show permission denied alert when authz is enabled and user doesn't have VIEW or MANAGE const authzIsEnabledAndNoPermission = isAuthzEnabled && !isLoadingUserPermissions + && !userPermissions?.canViewAdvancedSettings && !userPermissions?.canManageAdvancedSettings; if (authzIsEnabledAndNoPermission) { return ; } + // Determine if UI should be disabled (has VIEW but not MANAGE) - auditor sees read-only view + const isReadOnly = isAuthzEnabled + && !isLoadingUserPermissions + && userPermissions?.canViewAdvancedSettings + && !userPermissions?.canManageAdvancedSettings; + + // Apply read-only mode for auditors + if (isReadOnly) { + setIsEditableState(false); + } + + // Show the page content (read-only or editable) + return ( <> @@ -255,6 +273,7 @@ const AdvancedSettings = () => { handleBlur={handleSettingBlur} isEditableState={isEditableState} setIsEditableState={setIsEditableState} + readOnly={isReadOnly} /> ); })} diff --git a/src/advanced-settings/setting-card/SettingCard.tsx b/src/advanced-settings/setting-card/SettingCard.tsx index e7dedd7df4..74322610c2 100644 --- a/src/advanced-settings/setting-card/SettingCard.tsx +++ b/src/advanced-settings/setting-card/SettingCard.tsx @@ -25,6 +25,7 @@ const SettingCard = ({ saveSettingsPrompt, isEditableState, setIsEditableState, + readOnly = false, }) => { const intl = useIntl(); const { deprecated, help, displayName } = settingData; @@ -99,6 +100,7 @@ const SettingCard = ({ onChange={handleSettingChange} aria-label={displayName} onBlur={handleCardBlur} + disabled={readOnly} /> @@ -133,6 +135,11 @@ SettingCard.propTypes = { saveSettingsPrompt: PropTypes.bool.isRequired, isEditableState: PropTypes.bool.isRequired, setIsEditableState: PropTypes.func.isRequired, + readOnly: PropTypes.bool, +}; + +SettingCard.defaultProps = { + readOnly: false, }; export default SettingCard; diff --git a/src/authz/constants.ts b/src/authz/constants.ts index aa148ea08b..dc08a3fb7a 100644 --- a/src/authz/constants.ts +++ b/src/authz/constants.ts @@ -17,7 +17,11 @@ export const CONTENT_LIBRARY_PERMISSIONS = { export const COURSE_PERMISSIONS = { MANAGE_ADVANCED_SETTINGS: 'courses.manage_advanced_settings', + VIEW_ADVANCED_SETTINGS: 'courses.view_advanced_settings', VIEW_GRADING_SETTINGS: 'courses.view_grading_settings', EDIT_GRADING_SETTINGS: 'courses.edit_grading_settings', + + VIEW_PAGES_AND_RESOURCES: 'courses.view_pages_and_resources', + EDIT_PAGES_AND_RESOURCES: 'courses.manage_pages_and_resources', }; diff --git a/src/authz/permissionHelpers.ts b/src/authz/permissionHelpers.ts index 76585ea0bf..3daee2efd7 100644 --- a/src/authz/permissionHelpers.ts +++ b/src/authz/permissionHelpers.ts @@ -10,3 +10,14 @@ export const getGradingPermissions = (courseId: string) => ({ scope: courseId, }, }); + +export const getPagesAndResourcesPermissions = (courseId: string) => ({ + canViewPagesAndResources: { + action: COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES, + scope: courseId, + }, + canEditPagesAndResources: { + action: COURSE_PERMISSIONS.EDIT_PAGES_AND_RESOURCES, + scope: courseId, + }, +}); diff --git a/src/pages-and-resources/PagesAndResources.tsx b/src/pages-and-resources/PagesAndResources.tsx index dcac151c20..69accf0c62 100644 --- a/src/pages-and-resources/PagesAndResources.tsx +++ b/src/pages-and-resources/PagesAndResources.tsx @@ -14,6 +14,8 @@ import { AdditionalCoursePluginSlot } from '@src/plugin-slots/AdditionalCoursePl import { AdditionalCourseContentPluginSlot } from '@src/plugin-slots/AdditionalCourseContentPluginSlot'; import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; import { DeprecatedReduxState } from '@src/store'; +import { useCourseUserPermissions } from '@src/authz/hooks'; +import { getPagesAndResourcesPermissions } from '@src/authz/permissionHelpers'; import messages from './messages'; import DiscussionsSettings from './discussions'; @@ -28,6 +30,13 @@ const PagesAndResources = () => { const { courseId, courseDetails } = useCourseAuthoringContext(); document.title = getPageHeadTitle(courseDetails?.name || '', intl.formatMessage(messages.heading)); + const { + isLoading: isLoadingUserPermissions, + isAuthzEnabled, + canViewPagesAndResources, + canEditPagesAndResources, + } = useCourseUserPermissions(courseId, getPagesAndResourcesPermissions(courseId)); + const dispatch = useDispatch(); useEffect(() => { dispatch(fetchCourseApps(courseId)); @@ -56,19 +65,33 @@ const PagesAndResources = () => { } }); - if (loadingStatus === RequestStatus.IN_PROGRESS) { + if (loadingStatus === RequestStatus.IN_PROGRESS || isLoadingUserPermissions) { // eslint-disable-next-line react/jsx-no-useless-fragment return <>; } - if (courseAppsApiStatus === RequestStatus.DENIED) { + // Gate: if user has neither VIEW nor MANAGE permission, show permission denied + const hasNoAccess = (!isAuthzEnabled && courseAppsApiStatus === RequestStatus.DENIED) + || (isAuthzEnabled && !isLoadingUserPermissions && !canViewPagesAndResources && !canEditPagesAndResources); + + if (hasNoAccess) { return ; } + const isEditable = !isLoadingUserPermissions && canEditPagesAndResources; const hasAdditionalCoursePlugin = getConfig()?.pluginSlots?.additional_course_plugin != null; + // Read-only mode: has VIEW permission but not MANAGE (auditor) + const isReadOnly = isAuthzEnabled + && !isLoadingUserPermissions + && canViewPagesAndResources + && !canEditPagesAndResources; + + // For the modal: if readOnly, isEditable should be false (show disabled fields) + const isEditableForModal = isReadOnly ? false : isEditable; + return ( - +

{intl.formatMessage(messages.heading)}

@@ -81,7 +104,6 @@ const PagesAndResources = () => {
- { /> - } courseId={courseId} /> + } + courseId={courseId} + readOnly={isReadOnly} + /> {(contentPermissionsPages.length > 0 || hasAdditionalCoursePlugin) && ( <>

{intl.formatMessage(messages.contentPermissions)}

- } /> + } + readOnly={isReadOnly} + /> )}
diff --git a/src/pages-and-resources/PagesAndResourcesProvider.tsx b/src/pages-and-resources/PagesAndResourcesProvider.tsx index 9184fce86d..ebafa2cfb9 100644 --- a/src/pages-and-resources/PagesAndResourcesProvider.tsx +++ b/src/pages-and-resources/PagesAndResourcesProvider.tsx @@ -3,19 +3,24 @@ import React, { useMemo } from 'react'; interface PagesAndResourcesContextData { courseId?: string; path?: string; + isEditable?: boolean; } -export const PagesAndResourcesContext = React.createContext({}); +export const PagesAndResourcesContext = React.createContext({ + isEditable: false, +}); interface PagesAndResourcesProviderProps { courseId: string; + isEditable?: boolean; children: React.ReactNode; } -const PagesAndResourcesProvider = ({ courseId, children }: PagesAndResourcesProviderProps) => { +const PagesAndResourcesProvider = ({ courseId, isEditable = true, children }: PagesAndResourcesProviderProps) => { const contextValue = useMemo(() => ({ courseId, path: `/course/${courseId}/pages-and-resources`, - }), []); + isEditable, + }), [courseId, isEditable]); return ( { const { formatMessage } = useIntl(); - const { courseId } = useContext(PagesAndResourcesContext); + const { courseId, isEditable = false } = useContext(PagesAndResourcesContext); const loadingStatus = useSelector(getLoadingStatus); const updateSettingsRequestStatus = useSelector(getSavingStatus); const alertRef = useRef(null); @@ -139,6 +139,7 @@ const AppSettingsModal = ({ }} state={submitButtonState} onClick={handleFormikSubmit(formikProps)} + disabled={!isEditable} /> } > @@ -157,6 +158,7 @@ const AppSettingsModal = ({ onChange={(event) => formikProps.handleChange(event)} onBlur={formikProps.handleBlur} checked={formikProps.values.enabled} + disabled={!isEditable} label={
{enableAppLabel} diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx index b498eef689..5add9f0e77 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx @@ -42,7 +42,7 @@ const AppConfigForm = ({ const navigate = useNavigate(); const { formRef } = useContext(AppConfigFormContext); - const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); + const { path: pagesAndResourcesPath, isEditable: contextIsEditable = false } = useContext(PagesAndResourcesContext); const { appId: routeAppId } = useParams(); const [isLoading, setLoading] = useState(true); const { @@ -102,6 +102,7 @@ const AppConfigForm = ({ formRef={formRef} onSubmit={handleSubmit} legacy + isEditable={contextIsEditable} /> ); } else if (selectedAppId === 'openedx') { @@ -110,6 +111,7 @@ const AppConfigForm = ({ formRef={formRef} onSubmit={handleSubmit} legacy={false} + isEditable={contextIsEditable} /> ); } else { @@ -117,6 +119,7 @@ const AppConfigForm = ({ ); } diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx index ced3101670..42903037da 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx @@ -9,14 +9,16 @@ import messages from './messages'; import { SAVING } from '../data/slice'; import { AppConfigFormContext } from './AppConfigFormProvider'; import { useModel } from '../../../generic/model-store'; +import { PagesAndResourcesContext } from '../../PagesAndResourcesProvider'; const AppConfigFormSaveButton = ({ labelText }) => { const intl = useIntl(); const saveStatus = useSelector(state => state.discussions.saveStatus); const { selectedAppId } = useSelector((state) => state.discussions); + const { isEditable = false } = useContext(PagesAndResourcesContext); const app = useModel('apps', selectedAppId); - const canSubmit = getAuthenticatedUser().administrator || !app?.adminOnlyConfig; + const canSubmit = (getAuthenticatedUser().administrator || !app?.adminOnlyConfig) && isEditable; const { formRef } = useContext(AppConfigFormContext); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index b0e2c37b84..f9de1412de 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -18,7 +18,7 @@ import { useModel } from '../../../../../generic/model-store'; ensureConfig(['SITE_NAME', 'SUPPORT_EMAIL'], 'LTI Config Form'); -const LtiConfigForm = ({ onSubmit, formRef }) => { +const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { const intl = useIntl(); const dispatch = useDispatch(); @@ -71,7 +71,7 @@ const LtiConfigForm = ({ onSubmit, formRef }) => { return ( -
+

{providerName}

{ const intl = useIntl(); const { @@ -135,13 +136,18 @@ const OpenedXConfigForm = ({ return ( - +

{intl.formatMessage(messages[`appName-${selectedAppId}`])}

{!legacy && ( <> - + )} @@ -149,14 +155,15 @@ const OpenedXConfigForm = ({ onBlur={handleBlur} onChange={handleChange} values={values} + disabled={!isEditable} /> - + - + - - + +
@@ -171,6 +178,11 @@ OpenedXConfigForm.propTypes = { onSubmit: PropTypes.func.isRequired, // eslint-disable-next-line react/forbid-prop-types formRef: PropTypes.object.isRequired, + isEditable: PropTypes.bool, +}; + +OpenedXConfigForm.defaultProps = { + isEditable: true, }; export default OpenedXConfigForm; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx index 38efa6fd60..2eb936d845 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx @@ -11,6 +11,7 @@ const InContextDiscussionFields = ({ onBlur, onChange, values, + disabled = false, }) => { const intl = useIntl(); const { @@ -44,12 +45,13 @@ const InContextDiscussionFields = ({ ) : ( setShowPopup(true)} + onChange={() => !disabled && setShowPopup(true)} onBlur={onBlur} id="enableGradedUnits" checked={values.enableGradedUnits} label={intl.formatMessage(messages.gradedUnitPagesLabel)} helpText={intl.formatMessage(messages.gradedUnitPagesHelp)} + disabled={disabled} /> )} @@ -60,6 +62,7 @@ const InContextDiscussionFields = ({ checked={values.groupAtSubsection} label={intl.formatMessage(messages.groupInContextSubsectionLabel)} helpText={intl.formatMessage(messages.groupInContextSubsectionHelp)} + disabled={disabled} /> ); @@ -72,6 +75,11 @@ InContextDiscussionFields.propTypes = { enableGradedUnits: PropTypes.bool, groupAtSubsection: PropTypes.bool, }).isRequired, + disabled: PropTypes.bool, +}; + +InContextDiscussionFields.defaultProps = { + disabled: false, }; export default InContextDiscussionFields; diff --git a/src/pages-and-resources/discussions/app-list/AppCard.jsx b/src/pages-and-resources/discussions/app-list/AppCard.jsx index 5e5f1fe1e2..f5c33ee697 100644 --- a/src/pages-and-resources/discussions/app-list/AppCard.jsx +++ b/src/pages-and-resources/discussions/app-list/AppCard.jsx @@ -8,6 +8,8 @@ import { breakpoints, } from '@openedx/paragon'; import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; +import { useContext } from 'react'; +import { PagesAndResourcesContext } from '../../PagesAndResourcesProvider'; import messages from './messages'; import appMessages from '../app-config-form/messages'; import FeaturesList from './FeaturesList'; @@ -20,6 +22,8 @@ const AppCard = ({ }) => { const intl = useIntl(); const { canChangeProviders } = useCourseAuthoringContext(); + const { isEditable = false } = useContext(PagesAndResourcesContext); + const canInteract = canChangeProviders && isEditable; const supportText = app.hasFullSupport ? intl.formatMessage(messages.appFullSupport) : intl.formatMessage(messages.appBasicSupport); @@ -27,8 +31,8 @@ const AppCard = ({ return ( canChangeProviders && onClick(app.id)} - onKeyPress={() => canChangeProviders && onClick(app.id)} + onClick={() => canInteract && onClick(app.id)} + onKeyPress={() => canInteract && onClick(app.id)} role="radio" aria-checked={selected} className={classNames({ @@ -42,7 +46,7 @@ const AppCard = ({
{ const intl = useIntl(); const dispatch = useDispatch(); - const { courseId } = useContext(PagesAndResourcesContext); + const { courseId, isEditable = false } = useContext(PagesAndResourcesContext); const { appIds, featureIds, @@ -155,6 +155,7 @@ const AppList = () => { onChange={handleChange} checked={!enabled} data-testid="hide-discussion" + disabled={!isEditable} > {intl.formatMessage(messages.hideDiscussionTab)} @@ -200,6 +201,7 @@ const AppList = () => { className="ml-2" variant="primary" onClick={handleOk} + disabled={!isEditable} /> } diff --git a/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx b/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx index 4119241a83..4ab6dcb7b7 100644 --- a/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx +++ b/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx @@ -5,6 +5,7 @@ import { useSelector } from 'react-redux'; import { useNavigate } from 'react-router-dom'; import { DiscussionsContext } from '../DiscussionsProvider'; +import { PagesAndResourcesContext } from '../../PagesAndResourcesProvider'; import messages from './messages'; @@ -12,6 +13,7 @@ const AppListNextButton = () => { const intl = useIntl(); const { selectedAppId } = useSelector(state => state.discussions); const { path: discussionsPath } = useContext(DiscussionsContext); + const { isEditable = false } = useContext(PagesAndResourcesContext); const navigate = useNavigate(); const handleStartConfig = useCallback(() => { @@ -22,6 +24,7 @@ const AppListNextButton = () => { diff --git a/src/pages-and-resources/pages/PageCard.jsx b/src/pages-and-resources/pages/PageCard.jsx index c6f428bc07..2c061b1c05 100644 --- a/src/pages-and-resources/pages/PageCard.jsx +++ b/src/pages-and-resources/pages/PageCard.jsx @@ -26,11 +26,12 @@ const PageCard = ({ page, settingButton, courseId, + readOnly = false, }) => { const { formatMessage } = useIntl(); const isDesktop = useIsDesktop(); - const SettingButton = settingButton || ; + const SettingButton = settingButton || ; return ( ( +const PageGrid = ({ pages, pluginSlotComponent, courseId, readOnly = false }) => ( ( xl: 6, }} > - {pages.map((page) => )} + {pages.map((page) => )} {pluginSlotComponent} ); @@ -20,12 +20,14 @@ const PageGrid = ({ pages, pluginSlotComponent, courseId }) => ( PageGrid.defaultProps = { pluginSlotComponent: null, courseId: null, + readOnly: false, }; PageGrid.propTypes = { pages: PropTypes.arrayOf(CoursePageShape.isRequired).isRequired, pluginSlotComponent: PropTypes.element, courseId: PropTypes.string, + readOnly: PropTypes.bool, }; export default PageGrid; diff --git a/src/pages-and-resources/pages/PageSettingButton.jsx b/src/pages-and-resources/pages/PageSettingButton.jsx index 4759032ae8..a6b5b5b59b 100644 --- a/src/pages-and-resources/pages/PageSettingButton.jsx +++ b/src/pages-and-resources/pages/PageSettingButton.jsx @@ -15,6 +15,7 @@ const PageSettingButton = ({ courseId, legacyLink, allowedOperations, + readOnly = false, }) => { const { formatMessage } = useIntl(); const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); @@ -41,6 +42,21 @@ const PageSettingButton = ({ const canConfigureOrEnable = allowedOperations?.configure || allowedOperations?.enable; + // If read-only (auditor), disable the arrow/link navigation but allow opening modal for settings + if (determineLinkDestination && readOnly) { + return ( + + + + ); + } + if (determineLinkDestination) { return ( @@ -58,13 +74,14 @@ const PageSettingButton = ({ return null; } + // Settings button - allow opening modal, but pass readOnly state via navigation return ( navigate(`${pagesAndResourcesPath}/${id}/settings`)} + onClick={() => navigate(`${pagesAndResourcesPath}/${id}/settings${readOnly ? '?readOnly=true' : ''}`)} /> ); }; @@ -73,6 +90,7 @@ PageSettingButton.defaultProps = { legacyLink: null, allowedOperations: null, courseId: null, + readOnly: false, }; PageSettingButton.propTypes = { @@ -83,6 +101,7 @@ PageSettingButton.propTypes = { configure: PropTypes.bool, enable: PropTypes.bool, }), + readOnly: PropTypes.bool, }; export default PageSettingButton; From 699af15b912c374e5a30be2b2bc3fd4583b8b6f6 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Wed, 29 Apr 2026 09:55:07 -0500 Subject: [PATCH 02/10] feat(pages-and-resources): add permission tests for grading and pages/resources access --- src/CourseAuthoringPage.test.tsx | 10 +- .../AdvancedSettings.test.tsx | 21 +++ src/advanced-settings/AdvancedSettings.tsx | 23 ++-- .../setting-card/SettingCard.test.jsx | 35 ++++- .../setting-card/SettingCard.tsx | 4 - src/authz/permissionHelpers.test.ts | 120 ++++++++++++++++++ .../PagesAndResources.test.tsx | 86 +++++++++++++ .../apps/openedx/OpenedXConfigForm.jsx | 4 - .../apps/openedx/OpenedXConfigForm.test.jsx | 52 +++++++- .../apps/shared/InContextDiscussionFields.jsx | 4 - .../shared/InContextDiscussionFields.test.jsx | 117 +++++++++++++++++ .../discussions/app-list/AppCard.test.jsx | 112 +++++++++++++++- src/pages-and-resources/pages/PageCard.jsx | 1 - .../pages/PageCard.test.jsx | 63 +++++++++ .../pages/PageSettingButton.jsx | 1 - .../pages/PageSettingButton.test.jsx | 43 ++++++- 16 files changed, 658 insertions(+), 38 deletions(-) create mode 100644 src/authz/permissionHelpers.test.ts create mode 100644 src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx diff --git a/src/CourseAuthoringPage.test.tsx b/src/CourseAuthoringPage.test.tsx index d7339a283f..8a221f1a8c 100644 --- a/src/CourseAuthoringPage.test.tsx +++ b/src/CourseAuthoringPage.test.tsx @@ -108,14 +108,20 @@ describe('Course authoring page', () => { axiosMock.onGet( `${courseAppsApiUrl}/${courseId}`, - ).reply(403); + ).reply(403, { response: { status: 403 } }); await executeThunk(fetchCourseApps(courseId), store.dispatch); }; test('renders PermissionDeniedAlert when courseAppsApiStatus is DENIED', async () => { mockPathname = '/editor/'; await mockStoreDenied(); - const wrapper = renderComponent(); + // Test PagesAndResources (which has the PermissionDeniedAlert logic), + // not CourseAuthoringPage which is just the layout wrapper + const wrapper = renderComponent( + + + , + ); expect(await wrapper.findByTestId('permissionDeniedAlert')).toBeInTheDocument(); }); }); diff --git a/src/advanced-settings/AdvancedSettings.test.tsx b/src/advanced-settings/AdvancedSettings.test.tsx index 8b9d73aa70..5981fa43ea 100644 --- a/src/advanced-settings/AdvancedSettings.test.tsx +++ b/src/advanced-settings/AdvancedSettings.test.tsx @@ -191,4 +191,25 @@ describe('', () => { render(); expect(await screen.findByTestId('permissionDeniedAlert')).toBeInTheDocument(); }); + + it('should render settings in read-only mode when user has VIEW but not MANAGE permissions (auditor)', async () => { + mockWaffleFlags({ enableAuthzCourseAuthoring: true }); + jest.mocked(useUserPermissions).mockReturnValue({ + isLoading: false, + data: { canViewAdvancedSettings: true, canManageAdvancedSettings: false }, + } as unknown as ReturnType); + render(); + const textarea = await screen.findByLabelText(/Advanced Module List/i); + expect(textarea).toBeDisabled(); + }); + + it('should show permission denied when user has NO permissions (null data)', async () => { + mockWaffleFlags({ enableAuthzCourseAuthoring: true }); + jest.mocked(useUserPermissions).mockReturnValue({ + isLoading: false, + data: null, + } as unknown as ReturnType); + render(); + expect(await screen.findByTestId('permissionDeniedAlert')).toBeInTheDocument(); + }); }); diff --git a/src/advanced-settings/AdvancedSettings.tsx b/src/advanced-settings/AdvancedSettings.tsx index 0d56bbb386..36bff15fc2 100644 --- a/src/advanced-settings/AdvancedSettings.tsx +++ b/src/advanced-settings/AdvancedSettings.tsx @@ -76,6 +76,18 @@ const AdvancedSettings = () => { } = updateMutation; const isLoading = isPendingSettingsStatus || (isAuthzEnabled && isLoadingUserPermissions); + + // Determine if UI should be read-only (has VIEW but not MANAGE) — auditor + const isReadOnly = isAuthzEnabled + && !isLoadingUserPermissions + && !!userPermissions?.canViewAdvancedSettings + && !userPermissions?.canManageAdvancedSettings; + + useEffect(() => { + if (isReadOnly) { + setIsEditableState(false); + } + }, [isReadOnly]); const updateSettingsButtonState = { labels: { default: intl.formatMessage(messages.buttonSaveText), @@ -162,17 +174,6 @@ const AdvancedSettings = () => { return ; } - // Determine if UI should be disabled (has VIEW but not MANAGE) - auditor sees read-only view - const isReadOnly = isAuthzEnabled - && !isLoadingUserPermissions - && userPermissions?.canViewAdvancedSettings - && !userPermissions?.canManageAdvancedSettings; - - // Apply read-only mode for auditors - if (isReadOnly) { - setIsEditableState(false); - } - // Show the page content (read-only or editable) return ( diff --git a/src/advanced-settings/setting-card/SettingCard.test.jsx b/src/advanced-settings/setting-card/SettingCard.test.jsx index cb2fd25aba..35218422dc 100644 --- a/src/advanced-settings/setting-card/SettingCard.test.jsx +++ b/src/advanced-settings/setting-card/SettingCard.test.jsx @@ -1,4 +1,4 @@ -import { fireEvent, render, waitFor } from '@testing-library/react'; +import { fireEvent, render, waitFor, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { IntlProvider } from '@edx/frontend-platform/i18n'; @@ -25,7 +25,7 @@ jest.mock('react-textarea-autosize', () => /> ))); -const RootWrapper = () => ( +const RootWrapper = (props = {}) => ( ( handleBlur={handleBlur} isEditableState saveSettingsPrompt={false} + {...props} /> ); @@ -91,4 +92,34 @@ describe('', () => { expect(handleBlur).toHaveBeenCalled(); }); }); + it('renders in readOnly mode with disabled input', () => { + render(); + const input = screen.getByLabelText(/Setting Name/i); + expect(input).toBeDisabled(); + }); + + it('renders enabled by default when readOnly is not specified (default false)', () => { + // readOnly defaults to false - input should be enabled + render(); + const input = screen.getByLabelText(/Setting Name/i); + expect(input).not.toBeDisabled(); + }); + + it('calls setIsEditableState when value changes and isEditableState is false', async () => { + // Test for line 45: setIsEditableState(true) called when value changes + const { getByLabelText } = render(); + const input = getByLabelText(/Setting Name/i); + fireEvent.change(input, { target: { value: 'new-different-value' } }); + await waitFor(() => { + expect(setIsEditableState).toHaveBeenCalledWith(true); + }); + }); + + it('shows help popup when clicking info button', () => { + render(); + const helpButton = screen.getByRole('button', { name: /show help text/i }); + fireEvent.click(helpButton); + // The help text should be visible in the popup - verify component renders help + expect(screen.queryByText(/This is a help message/i)).toBeInTheDocument(); + }); }); diff --git a/src/advanced-settings/setting-card/SettingCard.tsx b/src/advanced-settings/setting-card/SettingCard.tsx index 74322610c2..78da537195 100644 --- a/src/advanced-settings/setting-card/SettingCard.tsx +++ b/src/advanced-settings/setting-card/SettingCard.tsx @@ -138,8 +138,4 @@ SettingCard.propTypes = { readOnly: PropTypes.bool, }; -SettingCard.defaultProps = { - readOnly: false, -}; - export default SettingCard; diff --git a/src/authz/permissionHelpers.test.ts b/src/authz/permissionHelpers.test.ts new file mode 100644 index 0000000000..48e0205b15 --- /dev/null +++ b/src/authz/permissionHelpers.test.ts @@ -0,0 +1,120 @@ +import { getGradingPermissions, getPagesAndResourcesPermissions } from './permissionHelpers'; +import { COURSE_PERMISSIONS } from './constants'; + +describe('permissionHelpers', () => { + const courseId = 'course-v1:org+course+run'; + + describe('getGradingPermissions', () => { + it('returns correct permission structure with VIEW action', () => { + const result = getGradingPermissions(courseId); + + expect(result.canViewGradingSettings).toBeDefined(); + expect(result.canViewGradingSettings.action).toBe(COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS); + expect(result.canViewGradingSettings.scope).toBe(courseId); + }); + + it('returns correct permission structure with EDIT action', () => { + const result = getGradingPermissions(courseId); + + expect(result.canEditGradingSettings).toBeDefined(); + expect(result.canEditGradingSettings.action).toBe(COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS); + expect(result.canEditGradingSettings.scope).toBe(courseId); + }); + + it('returns both VIEW and EDIT permissions in single call', () => { + const result = getGradingPermissions(courseId); + + const permissions = Object.keys(result); + expect(permissions).toContain('canViewGradingSettings'); + expect(permissions).toContain('canEditGradingSettings'); + }); + + it('uses correct courseId as scope for all permissions', () => { + const customCourseId = 'course-v1:custom+org+custom_run'; + const result = getGradingPermissions(customCourseId); + + expect(result.canViewGradingSettings.scope).toBe(customCourseId); + expect(result.canEditGradingSettings.scope).toBe(customCourseId); + }); + }); + + describe('getPagesAndResourcesPermissions', () => { + it('returns correct permission structure with VIEW action', () => { + const result = getPagesAndResourcesPermissions(courseId); + + expect(result.canViewPagesAndResources).toBeDefined(); + expect(result.canViewPagesAndResources.action).toBe(COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES); + expect(result.canViewPagesAndResources.scope).toBe(courseId); + }); + + it('returns correct permission structure with EDIT action', () => { + const result = getPagesAndResourcesPermissions(courseId); + + expect(result.canEditPagesAndResources).toBeDefined(); + expect(result.canEditPagesAndResources.action).toBe(COURSE_PERMISSIONS.EDIT_PAGES_AND_RESOURCES); + expect(result.canEditPagesAndResources.scope).toBe(courseId); + }); + + it('returns both VIEW and EDIT permissions in single call', () => { + const result = getPagesAndResourcesPermissions(courseId); + + const permissions = Object.keys(result); + expect(permissions).toContain('canViewPagesAndResources'); + expect(permissions).toContain('canEditPagesAndResources'); + }); + + it('uses correct courseId as scope for all permissions', () => { + const customCourseId = 'course-v1:another+test+course'; + const result = getPagesAndResourcesPermissions(customCourseId); + + expect(result.canViewPagesAndResources.scope).toBe(customCourseId); + expect(result.canEditPagesAndResources.scope).toBe(customCourseId); + }); + }); + + describe('permission constants verification', () => { + it('uses correct VIEW_GRADING_SETTINGS constant', () => { + const result = getGradingPermissions(courseId); + expect(result.canViewGradingSettings.action).toBe('courses.view_grading_settings'); + }); + + it('uses correct EDIT_GRADING_SETTINGS constant', () => { + const result = getGradingPermissions(courseId); + expect(result.canEditGradingSettings.action).toBe('courses.edit_grading_settings'); + }); + + it('uses correct VIEW_PAGES_AND_RESOURCES constant', () => { + const result = getPagesAndResourcesPermissions(courseId); + expect(result.canViewPagesAndResources.action).toBe('courses.view_pages_and_resources'); + }); + + it('uses correct EDIT_PAGES_AND_RESOURCES constant', () => { + const result = getPagesAndResourcesPermissions(courseId); + expect(result.canEditPagesAndResources.action).toBe('courses.manage_pages_and_resources'); + }); + }); + + describe('edge cases', () => { + it('handles empty courseId', () => { + const result = getGradingPermissions(''); + + expect(result.canViewGradingSettings.scope).toBe(''); + expect(result.canEditGradingSettings.scope).toBe(''); + }); + + it('handles special characters in courseId', () => { + const specialCourseId = 'course-v1:test+special:id'; + const result = getPagesAndResourcesPermissions(specialCourseId); + + expect(result.canViewPagesAndResources.scope).toBe(specialCourseId); + }); + + it('returns consistent structure across multiple calls', () => { + const result1 = getGradingPermissions(courseId); + const result2 = getGradingPermissions(courseId); + + expect(Object.keys(result1)).toEqual(Object.keys(result2)); + expect(result1.canViewGradingSettings.action).toBe(result2.canViewGradingSettings.action); + }); + }); +}); diff --git a/src/pages-and-resources/PagesAndResources.test.tsx b/src/pages-and-resources/PagesAndResources.test.tsx index 7196f19db2..dc6307ecde 100644 --- a/src/pages-and-resources/PagesAndResources.test.tsx +++ b/src/pages-and-resources/PagesAndResources.test.tsx @@ -8,8 +8,16 @@ import { import { getConfig, setConfig } from '@edx/frontend-platform'; import { PLUGIN_OPERATIONS, DIRECT_PLUGIN } from '@openedx/frontend-plugin-framework'; import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; +import { mockWaffleFlags } from '@src/data/apiHooks.mock'; +import { useCourseUserPermissions } from '@src/authz/hooks'; import { PagesAndResources } from '.'; +// Mock authz hooks +jest.mock('@src/authz/hooks', () => ({ + ...jest.requireActual('@src/authz/hooks'), + useCourseUserPermissions: jest.fn(), +})); + const mockPlugin = (identifier) => ({ plugins: [ { @@ -45,8 +53,30 @@ describe('PagesAndResources', () => { ), }, }); + + // Set up waffle flags to disable authz by default + mockWaffleFlags({ enableAuthzCourseAuthoring: false }); + + // Default: authz disabled allows everything + jest.mocked(useCourseUserPermissions).mockReturnValue({ + isLoading: false, + isAuthzEnabled: false, + canViewPagesAndResources: true, + canEditPagesAndResources: true, + } as ReturnType); }); + // Helper to set up permission mocks + const mockPermissions = (canView: boolean, canEdit: boolean) => { + mockWaffleFlags({ enableAuthzCourseAuthoring: true }); + jest.mocked(useCourseUserPermissions).mockReturnValue({ + isLoading: false, + isAuthzEnabled: true, + canViewPagesAndResources: canView, + canEditPagesAndResources: canEdit, + } as ReturnType); + }; + it('doesn\'t show content permissions section if relevant apps are not enabled', async () => { const initialState = { models: { @@ -128,4 +158,60 @@ describe('PagesAndResources', () => { await waitFor(() => expect(screen.queryByTestId('additional_course_plugin')).toBeInTheDocument()); await waitFor(() => expect(screen.queryByTestId('additional_course_content_plugin')).toBeInTheDocument()); }); + + describe('permission integration', () => { + it('shows PermissionDeniedAlert when user has no VIEW or EDIT permissions', async () => { + mockPermissions(false, false); + + const initialState = { + models: { + courseApps: {}, + }, + pagesAndResources: { + courseAppIds: [], + }, + }; + + initializeMocks({ initialState }); + renderComponent(); + + await waitFor(() => expect(screen.getByTestId('permissionDeniedAlert')).toBeInTheDocument()); + }); + + it('does NOT show PermissionDeniedAlert when user has VIEW permission', async () => { + mockPermissions(true, false); + + const initialState = { + models: { + courseApps: {}, + }, + pagesAndResources: { + courseAppIds: [], + }, + }; + + initializeMocks({ initialState }); + renderComponent(); + + await waitFor(() => expect(screen.queryByTestId('permissionDeniedAlert')).not.toBeInTheDocument()); + }); + + it('does NOT show PermissionDeniedAlert when user has EDIT permission', async () => { + mockPermissions(true, true); + + const initialState = { + models: { + courseApps: {}, + }, + pagesAndResources: { + courseAppIds: [], + }, + }; + + initializeMocks({ initialState }); + renderComponent(); + + await waitFor(() => expect(screen.queryByTestId('permissionDeniedAlert')).not.toBeInTheDocument()); + }); + }); }); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx index 3ef0e76310..6f8d4d9240 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx @@ -181,8 +181,4 @@ OpenedXConfigForm.propTypes = { isEditable: PropTypes.bool, }; -OpenedXConfigForm.defaultProps = { - isEditable: true, -}; - export default OpenedXConfigForm; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx index 945952d262..b96388ce5b 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx @@ -77,7 +77,7 @@ describe('OpenedXConfigForm', () => { axiosMock.reset(); }); - const createComponent = (onSubmit = jest.fn(), formRef = createRef(), legacy = true) => { + const createComponent = (onSubmit = jest.fn(), formRef = createRef(), legacy = true, isEditable = false) => { const wrapper = render( @@ -85,6 +85,7 @@ describe('OpenedXConfigForm', () => { onSubmit={onSubmit} formRef={formRef} legacy={legacy} + isEditable={isEditable} /> , @@ -115,6 +116,36 @@ describe('OpenedXConfigForm', () => { expect(queryByText(container, messages.groupInContextSubsectionLabel.defaultMessage)).toBeInTheDocument(); }); + test('renders editable form when isEditable=true', async () => { + await mockStore(legacyApiResponse); + createComponent(jest.fn(), createRef(), true, true); + const form = container.querySelector('form'); + expect(form).toBeInTheDocument(); + expect(form).not.toBeDisabled(); + }); + + test('renders form using default isEditable=false when prop is omitted', async () => { + await mockStore(legacyApiResponse); + const formRef = createRef(); + // Render directly without isEditable to trigger the default param = false + const wrapper = render( + + + + + , + ); + const form = wrapper.container.querySelector('form'); + expect(form).toBeInTheDocument(); + // isEditable defaults to false — child controls should be disabled + const divideByCohorts = wrapper.container.querySelector('#divideByCohorts'); + expect(divideByCohorts).toBeDisabled(); + }); + test('calls onSubmit when the formRef is submitted', async () => { const formRef = createRef(); const handleSubmit = jest.fn(); @@ -348,4 +379,23 @@ describe('OpenedXConfigForm', () => { assertHasErrorValidation(false); }); }); + + describe('isEditable prop', () => { + test('renders with isEditable=false (read-only mode)', async () => { + await mockStore(legacyApiResponse); + const wrapper = createComponent(jest.fn(), createRef(), true, false); + + // Form renders with disabled when isEditable=false (lines 139, 149, etc) + const form = wrapper.querySelector('form'); + expect(form).toBeInTheDocument(); + }); + + test('renders with isEditable=true (edit mode)', async () => { + await mockStore(legacyApiResponse); + const wrapper = createComponent(jest.fn(), createRef(), true, true); + + const form = wrapper.querySelector('form'); + expect(form).toBeInTheDocument(); + }); + }); }); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx index 2eb936d845..6be2ed75ca 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx @@ -78,8 +78,4 @@ InContextDiscussionFields.propTypes = { disabled: PropTypes.bool, }; -InContextDiscussionFields.defaultProps = { - disabled: false, -}; - export default InContextDiscussionFields; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx new file mode 100644 index 0000000000..f29384c78b --- /dev/null +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx @@ -0,0 +1,117 @@ +// @ts-check +import { render, screen, fireEvent } from '@testing-library/react'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { Formik, Form } from 'formik'; +import InContextDiscussionFields from './InContextDiscussionFields'; + +const defaultProps = { + onBlur: jest.fn(), + onChange: jest.fn(), + setFieldValue: jest.fn(), + values: { + enableGradedUnits: false, + groupAtSubsection: false, + }, +}; + +const renderComponent = (props = {}) => + render( + + + {() => ( +
+ + + )} +
+
, + ); + +describe('InContextDiscussionFields', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders without crashing', () => { + renderComponent(); + // Component renders - check for presence of expected text + expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); + }); + + it('renders with disabled prop', () => { + renderComponent({ disabled: true }); + expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); + }); + + it('renders with default disabled (false)', () => { + // Test default param - no disabled prop passed + renderComponent(); + expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); + }); + + it('renders with enableGradedUnits true (shows cancel labels)', () => { + renderComponent({ + values: { enableGradedUnits: true, groupAtSubsection: false }, + }); + expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); + }); + + it('shows confirmation popup when toggling enableGradedUnits when not disabled', () => { + renderComponent({ disabled: false }); + // When not disabled, clicking the switch should trigger the onChange which sets showPopup + // The component should still render - we test via the callback behavior + const switchControl = screen.getByLabelText(/enable discussions/i); + expect(switchControl).not.toBeDisabled(); + }); + + it('does NOT show popup when disabled', () => { + renderComponent({ disabled: true }); + // When disabled, clicking should not trigger popup logic + // Verify the switch is disabled + const switchControl = screen.getByLabelText(/enable discussions/i); + expect(switchControl).toBeDisabled(); + }); + + it('shows confirmation popup and handles confirm', () => { + renderComponent({ disabled: false, setFieldValue: jest.fn() }); + // Click the switch to show popup + const switchControl = screen.getByLabelText(/enable discussions/i); + fireEvent.click(switchControl); + // Check popup appears with confirm button + expect(screen.getByText(/Confirm/i)).toBeInTheDocument(); + // Click confirm + fireEvent.click(screen.getByText(/Confirm/i)); + // Popup should close (no longer visible) + }); + + it('shows popup with cancel labels when enableGradedUnits is already true', () => { + renderComponent({ + disabled: false, + values: { enableGradedUnits: true, groupAtSubsection: false }, + }); + const switchControl = screen.getByLabelText(/enable discussions/i); + fireEvent.click(switchControl); + // Popup shows cancel-related labels (enableGradedUnits=true branch) + expect(screen.getByText(/Confirm/i)).toBeInTheDocument(); + }); + + it('shows confirmation popup and handles cancel', () => { + renderComponent({ disabled: false, setFieldValue: jest.fn() }); + // Click the switch to show popup + const switchControl = screen.getByLabelText(/enable discussions/i); + fireEvent.click(switchControl); + // Check popup appears with cancel button + expect(screen.getByText(/Cancel/i)).toBeInTheDocument(); + // Click cancel + fireEvent.click(screen.getByText(/Cancel/i)); + // Popup should close (no longer visible) + }); + + it('does not show popup when onChange fires with disabled=true', () => { + renderComponent({ disabled: true }); + const switchControl = screen.getByLabelText(/enable discussions/i); + // Fire change event even though switch is disabled — onChange guard (!disabled) prevents popup + fireEvent.click(switchControl); + expect(screen.queryByText(/Confirm/i)).not.toBeInTheDocument(); + }); +}); diff --git a/src/pages-and-resources/discussions/app-list/AppCard.test.jsx b/src/pages-and-resources/discussions/app-list/AppCard.test.jsx index f2c5279599..cec39cf316 100644 --- a/src/pages-and-resources/discussions/app-list/AppCard.test.jsx +++ b/src/pages-and-resources/discussions/app-list/AppCard.test.jsx @@ -3,10 +3,12 @@ import { queryByLabelText, queryByTestId, initializeMocks, + fireEvent, } from '@src/testUtils'; import { executeThunk } from '@src/utils'; import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; +import PagesAndResourcesProvider from '../../PagesAndResourcesProvider'; import AppCard from './AppCard'; import messages from './messages'; import appMessages from '../app-config-form/messages'; @@ -38,15 +40,17 @@ describe('AppCard', () => { await executeThunk(fetchProviders(courseId), store.dispatch); }; - const createComponent = (data) => { + const createComponent = (data, { isEditable = false } = {}) => { const wrapper = render( - jest.fn()} - selected={selected} - features={[]} - /> + + jest.fn()} + selected={selected} + features={[]} + /> + , ); container = wrapper.container; @@ -96,4 +100,98 @@ describe('AppCard', () => { expect(queryByTestId(container, 'card-subtitle')).toHaveTextContent(subtitle); }); + + describe('isEditable integration', () => { + test('card responds to click when isEditable=true', async () => { + const handleClick = jest.fn(); + await mockStore(legacyApiResponse); + + const wrapper = render( + + + + + , + ); + const card = wrapper.container.querySelector('[role="radio"]'); + + fireEvent.click(card); + + expect(handleClick).toHaveBeenCalledWith(app.id); + }); + + test('card does NOT respond to click when isEditable=false', async () => { + const handleClick = jest.fn(); + await mockStore(legacyApiResponse); + + const wrapper = render( + + + + + , + ); + const card = wrapper.container.querySelector('[role="radio"]'); + + fireEvent.click(card); + + expect(handleClick).not.toHaveBeenCalled(); + }); + + test('card responds to keyPress when isEditable=true', async () => { + const handleClick = jest.fn(); + await mockStore(legacyApiResponse); + + const wrapper = render( + + + + + , + ); + const card = wrapper.container.querySelector('[role="radio"]'); + + fireEvent.keyPress(card, { key: 'Enter', charCode: 13 }); + + expect(handleClick).toHaveBeenCalledWith(app.id); + }); + + test('card does NOT respond to keyPress when isEditable=false', async () => { + const handleClick = jest.fn(); + await mockStore(legacyApiResponse); + + const wrapper = render( + + + + + , + ); + const card = wrapper.container.querySelector('[role="radio"]'); + + fireEvent.keyPress(card, { key: 'Enter', charCode: 13 }); + + expect(handleClick).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/pages-and-resources/pages/PageCard.jsx b/src/pages-and-resources/pages/PageCard.jsx index 2c061b1c05..0b01a3d793 100644 --- a/src/pages-and-resources/pages/PageCard.jsx +++ b/src/pages-and-resources/pages/PageCard.jsx @@ -62,7 +62,6 @@ const PageCard = ({ PageCard.defaultProps = { settingButton: null, courseId: null, - readOnly: false, }; PageCard.propTypes = { diff --git a/src/pages-and-resources/pages/PageCard.test.jsx b/src/pages-and-resources/pages/PageCard.test.jsx index 19c6247f58..d4b9fcc6af 100644 --- a/src/pages-and-resources/pages/PageCard.test.jsx +++ b/src/pages-and-resources/pages/PageCard.test.jsx @@ -9,6 +9,7 @@ import { } from '@src/testUtils'; import PageGrid from './PageGrid'; +import PageCard from './PageCard'; import PagesAndResourcesProvider from '../PagesAndResourcesProvider'; @@ -70,4 +71,66 @@ describe('LiveSettings', () => { expect(textbookSettingsButton).toHaveAttribute('href', textbookPagePath); }); }); + + it('renders readOnly mode correctly', async () => { + render( + + + , + ); + // When isEditable=false, settings button should be disabled + await waitFor(() => { + const buttons = screen.queryAllByRole('button'); + expect(buttons.length).toBeGreaterThan(0); + }); + }); + + it('renders enabled by default when readOnly is not specified (default false)', async () => { + // readOnly defaults to false - page card should render with enabled settings + render( + + + , + ); + // Should render buttons (enabled by default) + await waitFor(() => { + const buttons = screen.queryAllByRole('button'); + expect(buttons.length).toBeGreaterThan(0); + }); + }); + + it('renders PageCard directly without readOnly prop (uses default false)', () => { + render( + + + , + ); + expect(screen.getByText('Test Page')).toBeInTheDocument(); + }); + + it('renders PageGrid with readOnly=true passing readOnly to PageCards', async () => { + render( + + + , + ); + await waitFor(() => { + // With readOnly=true and legacyLink-based pages, arrow buttons become disabled + const buttons = screen.queryAllByRole('button'); + expect(buttons.length).toBeGreaterThan(0); + // At least one button is disabled (the arrow buttons for pages with legacyLinks) + const disabledButtons = buttons.filter((btn) => btn.disabled); + expect(disabledButtons.length).toBeGreaterThan(0); + }); + }); }); diff --git a/src/pages-and-resources/pages/PageSettingButton.jsx b/src/pages-and-resources/pages/PageSettingButton.jsx index a6b5b5b59b..7cb7e787c9 100644 --- a/src/pages-and-resources/pages/PageSettingButton.jsx +++ b/src/pages-and-resources/pages/PageSettingButton.jsx @@ -90,7 +90,6 @@ PageSettingButton.defaultProps = { legacyLink: null, allowedOperations: null, courseId: null, - readOnly: false, }; PageSettingButton.propTypes = { diff --git a/src/pages-and-resources/pages/PageSettingButton.test.jsx b/src/pages-and-resources/pages/PageSettingButton.test.jsx index c44eeeaf6b..88873d7b16 100644 --- a/src/pages-and-resources/pages/PageSettingButton.test.jsx +++ b/src/pages-and-resources/pages/PageSettingButton.test.jsx @@ -1,5 +1,5 @@ // @ts-check -import { screen, render, initializeMocks } from '../../testUtils'; +import { screen, render, initializeMocks, fireEvent } from '../../testUtils'; import PageSettingButton from './PageSettingButton'; import { mockWaffleFlags } from '../../data/apiHooks.mock'; @@ -56,4 +56,45 @@ describe('PageSettingButton', () => { const linkElement = screen.getByRole('link'); expect(linkElement).toHaveAttribute('href', defaultProps.legacyLink); }); + + it('renders disabled icon button in read-only mode with legacy link', () => { + renderComponent({ readOnly: true, legacyLink: 'http://legacylink.com/textbooks' }); + + const button = screen.getByRole('button'); + expect(button).toBeDisabled(); + }); + + it('renders enabled button by default when readOnly is not specified (default false)', () => { + // readOnly defaults to false - button should be enabled + renderComponent({ legacyLink: 'http://legacylink.com/textbooks' }); + + const linkElement = screen.getByRole('link'); + expect(linkElement).toBeInTheDocument(); + }); + + it('does not render when no legacyLink and cannot configure', () => { + renderComponent({ allowedOperations: null, legacyLink: null }); + + expect(screen.queryByRole('button')).toBeNull(); + }); + + it('navigates to settings page when settings button clicked (readOnly=false)', () => { + renderComponent({ legacyLink: 'http://legacylink.com/some-value', readOnly: false }); + + const button = screen.getByRole('button'); + expect(button).toBeInTheDocument(); + expect(button).not.toBeDisabled(); + fireEvent.click(button); + // The button click invokes navigate — component stays mounted without errors + }); + + it('navigates to settings page with readOnly param when settings button clicked (readOnly=true)', () => { + renderComponent({ legacyLink: 'http://legacylink.com/some-value', readOnly: true }); + + const button = screen.getByRole('button'); + expect(button).toBeInTheDocument(); + expect(button).not.toBeDisabled(); + fireEvent.click(button); + // Clicking triggers navigate with ?readOnly=true + }); }); From 39f0298f8b5a15fe6169d505cbe199e2421665be Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Wed, 29 Apr 2026 14:43:17 -0500 Subject: [PATCH 03/10] feat(pages-and-resources): implement advanced settings permissions and refactor related components --- src/advanced-settings/AdvancedSettings.tsx | 40 +++---- src/authz/permissionHelpers.test.ts | 107 ++++-------------- src/authz/permissionHelpers.ts | 11 ++ src/pages-and-resources/PagesAndResources.tsx | 13 +-- .../PagesAndResourcesProvider.tsx | 2 +- .../app-config-form/AppConfigForm.jsx | 8 +- .../AppConfigFormSaveButton.jsx | 3 +- .../apps/lti/LtiConfigForm.jsx | 7 +- .../apps/openedx/OpenedXConfigForm.jsx | 2 +- .../shared/InContextDiscussionFields.test.jsx | 2 +- src/pages-and-resources/pages/PageCard.jsx | 4 +- .../pages/PageCard.test.jsx | 12 +- src/pages-and-resources/pages/PageGrid.jsx | 6 +- .../pages/PageSettingButton.jsx | 10 +- .../pages/PageSettingButton.test.jsx | 30 ++--- 15 files changed, 82 insertions(+), 175 deletions(-) diff --git a/src/advanced-settings/AdvancedSettings.tsx b/src/advanced-settings/AdvancedSettings.tsx index 36bff15fc2..0a054448de 100644 --- a/src/advanced-settings/AdvancedSettings.tsx +++ b/src/advanced-settings/AdvancedSettings.tsx @@ -10,9 +10,6 @@ import { import { CheckCircle, Info, Warning } from '@openedx/paragon/icons'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; -import { useWaffleFlags } from '@src/data/apiHooks'; -import { useUserPermissions } from '@src/authz/data/apiHooks'; -import { COURSE_PERMISSIONS } from '@src/authz/constants'; import PermissionDeniedAlert from 'CourseAuthoring/generic/PermissionDeniedAlert'; import AlertProctoringError from '@src/generic/AlertProctoringError'; import { LoadingSpinner } from '@src/generic/Loading'; @@ -30,6 +27,8 @@ import validateAdvancedSettingsData from './utils'; import messages from './messages'; import ModalError from './modal-error/ModalError'; import { useCourseAdvancedSettings, useProctoringExamErrors, useUpdateCourseAdvancedSettings } from './data/apiHooks'; +import { useCourseUserPermissions } from '@src/authz/hooks'; +import { getAdvancedSettingsPermissions } from '@src/authz/permissionHelpers'; const AdvancedSettings = () => { const intl = useIntl(); @@ -44,18 +43,12 @@ const AdvancedSettings = () => { const { courseId, courseDetails } = useCourseAuthoringContext(); - const waffleFlags = useWaffleFlags(courseId); - const isAuthzEnabled = waffleFlags.enableAuthzCourseAuthoring; - const { isLoading: isLoadingUserPermissions, data: userPermissions } = useUserPermissions({ - canManageAdvancedSettings: { - action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS, - scope: courseId, - }, - canViewAdvancedSettings: { - action: COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS, - scope: courseId, - }, - }, isAuthzEnabled); + const { + isLoading: isLoadingUserPermissions, + isAuthzEnabled, + canViewAdvancedSettings, + canManageAdvancedSettings, + } = useCourseUserPermissions(courseId, getAdvancedSettingsPermissions(courseId)); const { data: advancedSettingsData = {}, @@ -77,17 +70,12 @@ const AdvancedSettings = () => { const isLoading = isPendingSettingsStatus || (isAuthzEnabled && isLoadingUserPermissions); - // Determine if UI should be read-only (has VIEW but not MANAGE) — auditor + // Determine if UI should be read-only (has VIEW but not MANAGE) const isReadOnly = isAuthzEnabled && !isLoadingUserPermissions - && !!userPermissions?.canViewAdvancedSettings - && !userPermissions?.canManageAdvancedSettings; + && !!canViewAdvancedSettings + && !canManageAdvancedSettings; - useEffect(() => { - if (isReadOnly) { - setIsEditableState(false); - } - }, [isReadOnly]); const updateSettingsButtonState = { labels: { default: intl.formatMessage(messages.buttonSaveText), @@ -167,15 +155,13 @@ const AdvancedSettings = () => { // Show permission denied alert when authz is enabled and user doesn't have VIEW or MANAGE const authzIsEnabledAndNoPermission = isAuthzEnabled && !isLoadingUserPermissions - && !userPermissions?.canViewAdvancedSettings - && !userPermissions?.canManageAdvancedSettings; + && !canViewAdvancedSettings + && !canManageAdvancedSettings; if (authzIsEnabledAndNoPermission) { return ; } - // Show the page content (read-only or editable) - return ( <> diff --git a/src/authz/permissionHelpers.test.ts b/src/authz/permissionHelpers.test.ts index 48e0205b15..cf2ad4c158 100644 --- a/src/authz/permissionHelpers.test.ts +++ b/src/authz/permissionHelpers.test.ts @@ -1,120 +1,51 @@ -import { getGradingPermissions, getPagesAndResourcesPermissions } from './permissionHelpers'; +import { + getGradingPermissions, + getPagesAndResourcesPermissions, + getAdvancedSettingsPermissions, +} from './permissionHelpers'; import { COURSE_PERMISSIONS } from './constants'; describe('permissionHelpers', () => { const courseId = 'course-v1:org+course+run'; describe('getGradingPermissions', () => { - it('returns correct permission structure with VIEW action', () => { + it('returns VIEW and EDIT permissions with the correct actions and scope', () => { const result = getGradingPermissions(courseId); - expect(result.canViewGradingSettings).toBeDefined(); expect(result.canViewGradingSettings.action).toBe(COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS); expect(result.canViewGradingSettings.scope).toBe(courseId); - }); - - it('returns correct permission structure with EDIT action', () => { - const result = getGradingPermissions(courseId); - - expect(result.canEditGradingSettings).toBeDefined(); expect(result.canEditGradingSettings.action).toBe(COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS); expect(result.canEditGradingSettings.scope).toBe(courseId); }); - - it('returns both VIEW and EDIT permissions in single call', () => { - const result = getGradingPermissions(courseId); - - const permissions = Object.keys(result); - expect(permissions).toContain('canViewGradingSettings'); - expect(permissions).toContain('canEditGradingSettings'); - }); - - it('uses correct courseId as scope for all permissions', () => { - const customCourseId = 'course-v1:custom+org+custom_run'; - const result = getGradingPermissions(customCourseId); - - expect(result.canViewGradingSettings.scope).toBe(customCourseId); - expect(result.canEditGradingSettings.scope).toBe(customCourseId); - }); }); describe('getPagesAndResourcesPermissions', () => { - it('returns correct permission structure with VIEW action', () => { + it('returns VIEW and EDIT permissions with the correct actions and scope', () => { const result = getPagesAndResourcesPermissions(courseId); - expect(result.canViewPagesAndResources).toBeDefined(); expect(result.canViewPagesAndResources.action).toBe(COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES); expect(result.canViewPagesAndResources.scope).toBe(courseId); - }); - - it('returns correct permission structure with EDIT action', () => { - const result = getPagesAndResourcesPermissions(courseId); - - expect(result.canEditPagesAndResources).toBeDefined(); expect(result.canEditPagesAndResources.action).toBe(COURSE_PERMISSIONS.EDIT_PAGES_AND_RESOURCES); expect(result.canEditPagesAndResources.scope).toBe(courseId); }); - - it('returns both VIEW and EDIT permissions in single call', () => { - const result = getPagesAndResourcesPermissions(courseId); - - const permissions = Object.keys(result); - expect(permissions).toContain('canViewPagesAndResources'); - expect(permissions).toContain('canEditPagesAndResources'); - }); - - it('uses correct courseId as scope for all permissions', () => { - const customCourseId = 'course-v1:another+test+course'; - const result = getPagesAndResourcesPermissions(customCourseId); - - expect(result.canViewPagesAndResources.scope).toBe(customCourseId); - expect(result.canEditPagesAndResources.scope).toBe(customCourseId); - }); }); - describe('permission constants verification', () => { - it('uses correct VIEW_GRADING_SETTINGS constant', () => { - const result = getGradingPermissions(courseId); - expect(result.canViewGradingSettings.action).toBe('courses.view_grading_settings'); - }); - - it('uses correct EDIT_GRADING_SETTINGS constant', () => { - const result = getGradingPermissions(courseId); - expect(result.canEditGradingSettings.action).toBe('courses.edit_grading_settings'); - }); - - it('uses correct VIEW_PAGES_AND_RESOURCES constant', () => { - const result = getPagesAndResourcesPermissions(courseId); - expect(result.canViewPagesAndResources.action).toBe('courses.view_pages_and_resources'); - }); - - it('uses correct EDIT_PAGES_AND_RESOURCES constant', () => { - const result = getPagesAndResourcesPermissions(courseId); - expect(result.canEditPagesAndResources.action).toBe('courses.manage_pages_and_resources'); - }); - }); - - describe('edge cases', () => { - it('handles empty courseId', () => { - const result = getGradingPermissions(''); - - expect(result.canViewGradingSettings.scope).toBe(''); - expect(result.canEditGradingSettings.scope).toBe(''); - }); - - it('handles special characters in courseId', () => { - const specialCourseId = 'course-v1:test+special:id'; - const result = getPagesAndResourcesPermissions(specialCourseId); + describe('getAdvancedSettingsPermissions', () => { + it('returns VIEW and MANAGE permissions with the correct actions and scope', () => { + const result = getAdvancedSettingsPermissions(courseId); - expect(result.canViewPagesAndResources.scope).toBe(specialCourseId); + expect(result.canViewAdvancedSettings.action).toBe(COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS); + expect(result.canViewAdvancedSettings.scope).toBe(courseId); + expect(result.canManageAdvancedSettings.action).toBe(COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS); + expect(result.canManageAdvancedSettings.scope).toBe(courseId); }); - it('returns consistent structure across multiple calls', () => { - const result1 = getGradingPermissions(courseId); - const result2 = getGradingPermissions(courseId); + it('uses the provided courseId as scope', () => { + const otherId = 'course-v1:another+test+run'; + const result = getAdvancedSettingsPermissions(otherId); - expect(Object.keys(result1)).toEqual(Object.keys(result2)); - expect(result1.canViewGradingSettings.action).toBe(result2.canViewGradingSettings.action); + expect(result.canViewAdvancedSettings.scope).toBe(otherId); + expect(result.canManageAdvancedSettings.scope).toBe(otherId); }); }); }); diff --git a/src/authz/permissionHelpers.ts b/src/authz/permissionHelpers.ts index 3daee2efd7..7ecfa4693a 100644 --- a/src/authz/permissionHelpers.ts +++ b/src/authz/permissionHelpers.ts @@ -21,3 +21,14 @@ export const getPagesAndResourcesPermissions = (courseId: string) => ({ scope: courseId, }, }); + +export const getAdvancedSettingsPermissions = (courseId: string) => ({ + canViewAdvancedSettings: { + action: COURSE_PERMISSIONS.VIEW_ADVANCED_SETTINGS, + scope: courseId, + }, + canManageAdvancedSettings: { + action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS, + scope: courseId, + }, +}); diff --git a/src/pages-and-resources/PagesAndResources.tsx b/src/pages-and-resources/PagesAndResources.tsx index 69accf0c62..a8a62366ef 100644 --- a/src/pages-and-resources/PagesAndResources.tsx +++ b/src/pages-and-resources/PagesAndResources.tsx @@ -81,17 +81,8 @@ const PagesAndResources = () => { const isEditable = !isLoadingUserPermissions && canEditPagesAndResources; const hasAdditionalCoursePlugin = getConfig()?.pluginSlots?.additional_course_plugin != null; - // Read-only mode: has VIEW permission but not MANAGE (auditor) - const isReadOnly = isAuthzEnabled - && !isLoadingUserPermissions - && canViewPagesAndResources - && !canEditPagesAndResources; - - // For the modal: if readOnly, isEditable should be false (show disabled fields) - const isEditableForModal = isReadOnly ? false : isEditable; - return ( - +

{intl.formatMessage(messages.heading)}

@@ -143,7 +134,6 @@ const PagesAndResources = () => { pages={pages} pluginSlotComponent={} courseId={courseId} - readOnly={isReadOnly} /> {(contentPermissionsPages.length > 0 || hasAdditionalCoursePlugin) && ( @@ -154,7 +144,6 @@ const PagesAndResources = () => { } - readOnly={isReadOnly} /> )} diff --git a/src/pages-and-resources/PagesAndResourcesProvider.tsx b/src/pages-and-resources/PagesAndResourcesProvider.tsx index ebafa2cfb9..07f1a8c709 100644 --- a/src/pages-and-resources/PagesAndResourcesProvider.tsx +++ b/src/pages-and-resources/PagesAndResourcesProvider.tsx @@ -15,7 +15,7 @@ interface PagesAndResourcesProviderProps { children: React.ReactNode; } -const PagesAndResourcesProvider = ({ courseId, isEditable = true, children }: PagesAndResourcesProviderProps) => { +const PagesAndResourcesProvider = ({ courseId, isEditable = false, children }: PagesAndResourcesProviderProps) => { const contextValue = useMemo(() => ({ courseId, path: `/course/${courseId}/pages-and-resources`, diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx index 5add9f0e77..d760771eed 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx @@ -42,7 +42,7 @@ const AppConfigForm = ({ const navigate = useNavigate(); const { formRef } = useContext(AppConfigFormContext); - const { path: pagesAndResourcesPath, isEditable: contextIsEditable = false } = useContext(PagesAndResourcesContext); + const { path: pagesAndResourcesPath, isEditable = false } = useContext(PagesAndResourcesContext); const { appId: routeAppId } = useParams(); const [isLoading, setLoading] = useState(true); const { @@ -102,7 +102,7 @@ const AppConfigForm = ({ formRef={formRef} onSubmit={handleSubmit} legacy - isEditable={contextIsEditable} + isEditable={isEditable} /> ); } else if (selectedAppId === 'openedx') { @@ -111,7 +111,7 @@ const AppConfigForm = ({ formRef={formRef} onSubmit={handleSubmit} legacy={false} - isEditable={contextIsEditable} + isEditable={isEditable} /> ); } else { @@ -119,7 +119,7 @@ const AppConfigForm = ({ ); } diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx index 42903037da..f0fee27ae4 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx @@ -18,7 +18,7 @@ const AppConfigFormSaveButton = ({ labelText }) => { const { isEditable = false } = useContext(PagesAndResourcesContext); const app = useModel('apps', selectedAppId); - const canSubmit = (getAuthenticatedUser().administrator || !app?.adminOnlyConfig) && isEditable; + const canSubmit = getAuthenticatedUser().administrator || !app?.adminOnlyConfig; const { formRef } = useContext(AppConfigFormContext); @@ -47,6 +47,7 @@ const AppConfigFormSaveButton = ({ labelText }) => { }} state={submitButtonState} onClick={handleSave} + disabled={!isEditable} style={{ minWidth: '88px' }} /> ) diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index f9de1412de..f26d94ed6e 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -71,7 +71,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { return ( -
+

{providerName}

{ onChange={handleChange} onBlur={handleBlur} value={values.consumerKey} + disabled={!isEditable} /> {isInvalidConsumerKey && ( @@ -130,6 +131,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { onChange={handleChange} onBlur={handleBlur} value={values.consumerSecret} + disabled={!isEditable} /> {isInvalidConsumerSecret && ( @@ -143,6 +145,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { onChange={handleChange} onBlur={handleBlur} value={values.launchUrl} + disabled={!isEditable} /> {isInvalidLaunchUrl && ( @@ -163,6 +166,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { onBlur={handleBlur} checked={values.piiShareUsername} label={intl.formatMessage(messages.piiShareUsername)} + disabled={!isEditable} /> { onBlur={handleBlur} checked={values.piiShareEmail} label={intl.formatMessage(messages.piiShareEmail)} + disabled={!isEditable} />

diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx index 6f8d4d9240..353847fda4 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx @@ -136,7 +136,7 @@ const OpenedXConfigForm = ({ return ( - +

{intl.formatMessage(messages[`appName-${selectedAppId}`])}

{!legacy diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx index f29384c78b..9a7e593cea 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx @@ -110,7 +110,7 @@ describe('InContextDiscussionFields', () => { it('does not show popup when onChange fires with disabled=true', () => { renderComponent({ disabled: true }); const switchControl = screen.getByLabelText(/enable discussions/i); - // Fire change event even though switch is disabled — onChange guard (!disabled) prevents popup + // fireEvent bypasses browser disabled-input behavior, but the !disabled guard in onChange prevents the popup fireEvent.click(switchControl); expect(screen.queryByText(/Confirm/i)).not.toBeInTheDocument(); }); diff --git a/src/pages-and-resources/pages/PageCard.jsx b/src/pages-and-resources/pages/PageCard.jsx index 0b01a3d793..c6f428bc07 100644 --- a/src/pages-and-resources/pages/PageCard.jsx +++ b/src/pages-and-resources/pages/PageCard.jsx @@ -26,12 +26,11 @@ const PageCard = ({ page, settingButton, courseId, - readOnly = false, }) => { const { formatMessage } = useIntl(); const isDesktop = useIsDesktop(); - const SettingButton = settingButton || ; + const SettingButton = settingButton || ; return ( { render( - + , ); @@ -99,7 +99,7 @@ describe('LiveSettings', () => { }); }); - it('renders PageCard directly without readOnly prop (uses default false)', () => { + it('renders PageCard without isEditable in context (defaults to false, no settings button)', () => { render( { expect(screen.getByText('Test Page')).toBeInTheDocument(); }); - it('renders PageGrid with readOnly=true passing readOnly to PageCards', async () => { + it('disables arrow buttons for legacy-link pages when isEditable=false in context', async () => { render( - - + + , ); await waitFor(() => { - // With readOnly=true and legacyLink-based pages, arrow buttons become disabled const buttons = screen.queryAllByRole('button'); expect(buttons.length).toBeGreaterThan(0); - // At least one button is disabled (the arrow buttons for pages with legacyLinks) const disabledButtons = buttons.filter((btn) => btn.disabled); expect(disabledButtons.length).toBeGreaterThan(0); }); diff --git a/src/pages-and-resources/pages/PageGrid.jsx b/src/pages-and-resources/pages/PageGrid.jsx index 4c11ec8779..fda21275be 100644 --- a/src/pages-and-resources/pages/PageGrid.jsx +++ b/src/pages-and-resources/pages/PageGrid.jsx @@ -3,7 +3,7 @@ import React from 'react'; import { CardGrid } from '@openedx/paragon'; import PageCard, { CoursePageShape } from './PageCard'; -const PageGrid = ({ pages, pluginSlotComponent, courseId, readOnly = false }) => ( +const PageGrid = ({ pages, pluginSlotComponent, courseId }) => ( xl: 6, }} > - {pages.map((page) => )} + {pages.map((page) => )} {pluginSlotComponent} ); @@ -20,14 +20,12 @@ const PageGrid = ({ pages, pluginSlotComponent, courseId, readOnly = false }) => PageGrid.defaultProps = { pluginSlotComponent: null, courseId: null, - readOnly: false, }; PageGrid.propTypes = { pages: PropTypes.arrayOf(CoursePageShape.isRequired).isRequired, pluginSlotComponent: PropTypes.element, courseId: PropTypes.string, - readOnly: PropTypes.bool, }; export default PageGrid; diff --git a/src/pages-and-resources/pages/PageSettingButton.jsx b/src/pages-and-resources/pages/PageSettingButton.jsx index 7cb7e787c9..917973609c 100644 --- a/src/pages-and-resources/pages/PageSettingButton.jsx +++ b/src/pages-and-resources/pages/PageSettingButton.jsx @@ -15,10 +15,9 @@ const PageSettingButton = ({ courseId, legacyLink, allowedOperations, - readOnly = false, }) => { const { formatMessage } = useIntl(); - const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); + const { path: pagesAndResourcesPath, isEditable = false } = useContext(PagesAndResourcesContext); const navigate = useNavigate(); const waffleFlags = useWaffleFlags(courseId); @@ -42,8 +41,7 @@ const PageSettingButton = ({ const canConfigureOrEnable = allowedOperations?.configure || allowedOperations?.enable; - // If read-only (auditor), disable the arrow/link navigation but allow opening modal for settings - if (determineLinkDestination && readOnly) { + if (determineLinkDestination && !isEditable) { return ( navigate(`${pagesAndResourcesPath}/${id}/settings${readOnly ? '?readOnly=true' : ''}`)} + onClick={() => navigate(`${pagesAndResourcesPath}/${id}/settings`)} /> ); }; @@ -100,7 +97,6 @@ PageSettingButton.propTypes = { configure: PropTypes.bool, enable: PropTypes.bool, }), - readOnly: PropTypes.bool, }; export default PageSettingButton; diff --git a/src/pages-and-resources/pages/PageSettingButton.test.jsx b/src/pages-and-resources/pages/PageSettingButton.test.jsx index 88873d7b16..9d9ddf80c4 100644 --- a/src/pages-and-resources/pages/PageSettingButton.test.jsx +++ b/src/pages-and-resources/pages/PageSettingButton.test.jsx @@ -2,6 +2,7 @@ import { screen, render, initializeMocks, fireEvent } from '../../testUtils'; import PageSettingButton from './PageSettingButton'; import { mockWaffleFlags } from '../../data/apiHooks.mock'; +import PagesAndResourcesProvider from '../PagesAndResourcesProvider'; const defaultProps = { id: 'page_id', @@ -10,7 +11,12 @@ const defaultProps = { allowedOperations: { configure: true, enable: true }, }; -const renderComponent = (props = {}) => render(); +const renderComponent = (props = {}, { isEditable = true } = {}) => + render( + + + , + ); mockWaffleFlags(); @@ -58,15 +64,14 @@ describe('PageSettingButton', () => { }); it('renders disabled icon button in read-only mode with legacy link', () => { - renderComponent({ readOnly: true, legacyLink: 'http://legacylink.com/textbooks' }); + renderComponent({ legacyLink: 'http://legacylink.com/textbooks' }, { isEditable: false }); const button = screen.getByRole('button'); expect(button).toBeDisabled(); }); - it('renders enabled button by default when readOnly is not specified (default false)', () => { - // readOnly defaults to false - button should be enabled - renderComponent({ legacyLink: 'http://legacylink.com/textbooks' }); + it('renders arrow link when user is editable', () => { + renderComponent({ legacyLink: 'http://legacylink.com/textbooks' }, { isEditable: true }); const linkElement = screen.getByRole('link'); expect(linkElement).toBeInTheDocument(); @@ -78,23 +83,12 @@ describe('PageSettingButton', () => { expect(screen.queryByRole('button')).toBeNull(); }); - it('navigates to settings page when settings button clicked (readOnly=false)', () => { - renderComponent({ legacyLink: 'http://legacylink.com/some-value', readOnly: false }); - - const button = screen.getByRole('button'); - expect(button).toBeInTheDocument(); - expect(button).not.toBeDisabled(); - fireEvent.click(button); - // The button click invokes navigate — component stays mounted without errors - }); - - it('navigates to settings page with readOnly param when settings button clicked (readOnly=true)', () => { - renderComponent({ legacyLink: 'http://legacylink.com/some-value', readOnly: true }); + it('navigates to settings page when settings gear button clicked', () => { + renderComponent({ legacyLink: 'http://legacylink.com/some-value' }); const button = screen.getByRole('button'); expect(button).toBeInTheDocument(); expect(button).not.toBeDisabled(); fireEvent.click(button); - // Clicking triggers navigate with ?readOnly=true }); }); From 4aa9e5917f200b460cd1b527963a8261b2b45209 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Wed, 29 Apr 2026 15:08:16 -0500 Subject: [PATCH 04/10] feat(pages-and-resources): update permissions terminology and enhance related components --- plugins/course-apps/progress/Settings.jsx | 2 +- src/authz/constants.ts | 2 +- src/authz/permissionHelpers.test.ts | 6 +++--- src/authz/permissionHelpers.ts | 4 ++-- .../PagesAndResources.test.tsx | 6 +++--- src/pages-and-resources/PagesAndResources.tsx | 7 ++++--- .../PagesAndResourcesProvider.tsx | 2 +- .../discussions/app-list/AppCard.jsx | 5 +++-- .../discussions/app-list/AppCard.test.jsx | 8 ++++---- src/pages-and-resources/index.ts | 1 + .../pages/PageCard.test.jsx | 18 +++++++++--------- 11 files changed, 32 insertions(+), 29 deletions(-) diff --git a/plugins/course-apps/progress/Settings.jsx b/plugins/course-apps/progress/Settings.jsx index 7e3aecf1ed..eed52cdfba 100644 --- a/plugins/course-apps/progress/Settings.jsx +++ b/plugins/course-apps/progress/Settings.jsx @@ -6,7 +6,7 @@ import { getConfig } from '@edx/frontend-platform'; import FormSwitchGroup from 'CourseAuthoring/generic/FormSwitchGroup'; import { useAppSetting } from 'CourseAuthoring/utils'; import AppSettingsModal from 'CourseAuthoring/pages-and-resources/app-settings-modal/AppSettingsModal'; -import { PagesAndResourcesContext } from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; +import { PagesAndResourcesContext } from 'CourseAuthoring/pages-and-resources'; import messages from './messages'; const ProgressSettings = ({ onClose }) => { diff --git a/src/authz/constants.ts b/src/authz/constants.ts index dc08a3fb7a..a9b8ae3c41 100644 --- a/src/authz/constants.ts +++ b/src/authz/constants.ts @@ -23,5 +23,5 @@ export const COURSE_PERMISSIONS = { EDIT_GRADING_SETTINGS: 'courses.edit_grading_settings', VIEW_PAGES_AND_RESOURCES: 'courses.view_pages_and_resources', - EDIT_PAGES_AND_RESOURCES: 'courses.manage_pages_and_resources', + MANAGE_PAGES_AND_RESOURCES: 'courses.manage_pages_and_resources', }; diff --git a/src/authz/permissionHelpers.test.ts b/src/authz/permissionHelpers.test.ts index cf2ad4c158..edc8b5f2ea 100644 --- a/src/authz/permissionHelpers.test.ts +++ b/src/authz/permissionHelpers.test.ts @@ -20,13 +20,13 @@ describe('permissionHelpers', () => { }); describe('getPagesAndResourcesPermissions', () => { - it('returns VIEW and EDIT permissions with the correct actions and scope', () => { + it('returns VIEW and MANAGE permissions with the correct actions and scope', () => { const result = getPagesAndResourcesPermissions(courseId); expect(result.canViewPagesAndResources.action).toBe(COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES); expect(result.canViewPagesAndResources.scope).toBe(courseId); - expect(result.canEditPagesAndResources.action).toBe(COURSE_PERMISSIONS.EDIT_PAGES_AND_RESOURCES); - expect(result.canEditPagesAndResources.scope).toBe(courseId); + expect(result.canManagePagesAndResources.action).toBe(COURSE_PERMISSIONS.MANAGE_PAGES_AND_RESOURCES); + expect(result.canManagePagesAndResources.scope).toBe(courseId); }); }); diff --git a/src/authz/permissionHelpers.ts b/src/authz/permissionHelpers.ts index 7ecfa4693a..be2a944d54 100644 --- a/src/authz/permissionHelpers.ts +++ b/src/authz/permissionHelpers.ts @@ -16,8 +16,8 @@ export const getPagesAndResourcesPermissions = (courseId: string) => ({ action: COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES, scope: courseId, }, - canEditPagesAndResources: { - action: COURSE_PERMISSIONS.EDIT_PAGES_AND_RESOURCES, + canManagePagesAndResources: { + action: COURSE_PERMISSIONS.MANAGE_PAGES_AND_RESOURCES, scope: courseId, }, }); diff --git a/src/pages-and-resources/PagesAndResources.test.tsx b/src/pages-and-resources/PagesAndResources.test.tsx index dc6307ecde..8c06c1a625 100644 --- a/src/pages-and-resources/PagesAndResources.test.tsx +++ b/src/pages-and-resources/PagesAndResources.test.tsx @@ -62,18 +62,18 @@ describe('PagesAndResources', () => { isLoading: false, isAuthzEnabled: false, canViewPagesAndResources: true, - canEditPagesAndResources: true, + canManagePagesAndResources: true, } as ReturnType); }); // Helper to set up permission mocks - const mockPermissions = (canView: boolean, canEdit: boolean) => { + const mockPermissions = (canView: boolean, canManage: boolean) => { mockWaffleFlags({ enableAuthzCourseAuthoring: true }); jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: true, canViewPagesAndResources: canView, - canEditPagesAndResources: canEdit, + canManagePagesAndResources: canManage, } as ReturnType); }; diff --git a/src/pages-and-resources/PagesAndResources.tsx b/src/pages-and-resources/PagesAndResources.tsx index a8a62366ef..972611a5ea 100644 --- a/src/pages-and-resources/PagesAndResources.tsx +++ b/src/pages-and-resources/PagesAndResources.tsx @@ -34,7 +34,7 @@ const PagesAndResources = () => { isLoading: isLoadingUserPermissions, isAuthzEnabled, canViewPagesAndResources, - canEditPagesAndResources, + canManagePagesAndResources, } = useCourseUserPermissions(courseId, getPagesAndResourcesPermissions(courseId)); const dispatch = useDispatch(); @@ -72,13 +72,14 @@ const PagesAndResources = () => { // Gate: if user has neither VIEW nor MANAGE permission, show permission denied const hasNoAccess = (!isAuthzEnabled && courseAppsApiStatus === RequestStatus.DENIED) - || (isAuthzEnabled && !isLoadingUserPermissions && !canViewPagesAndResources && !canEditPagesAndResources); + || (isAuthzEnabled && !canViewPagesAndResources && !canManagePagesAndResources); if (hasNoAccess) { return ; } - const isEditable = !isLoadingUserPermissions && canEditPagesAndResources; + // When authz is disabled every authenticated user has full edit access. + const isEditable = !isAuthzEnabled || !!canManagePagesAndResources; const hasAdditionalCoursePlugin = getConfig()?.pluginSlots?.additional_course_plugin != null; return ( diff --git a/src/pages-and-resources/PagesAndResourcesProvider.tsx b/src/pages-and-resources/PagesAndResourcesProvider.tsx index 07f1a8c709..ebafa2cfb9 100644 --- a/src/pages-and-resources/PagesAndResourcesProvider.tsx +++ b/src/pages-and-resources/PagesAndResourcesProvider.tsx @@ -15,7 +15,7 @@ interface PagesAndResourcesProviderProps { children: React.ReactNode; } -const PagesAndResourcesProvider = ({ courseId, isEditable = false, children }: PagesAndResourcesProviderProps) => { +const PagesAndResourcesProvider = ({ courseId, isEditable = true, children }: PagesAndResourcesProviderProps) => { const contextValue = useMemo(() => ({ courseId, path: `/course/${courseId}/pages-and-resources`, diff --git a/src/pages-and-resources/discussions/app-list/AppCard.jsx b/src/pages-and-resources/discussions/app-list/AppCard.jsx index f5c33ee697..164e142bf0 100644 --- a/src/pages-and-resources/discussions/app-list/AppCard.jsx +++ b/src/pages-and-resources/discussions/app-list/AppCard.jsx @@ -30,9 +30,10 @@ const AppCard = ({ return ( canInteract && onClick(app.id)} - onKeyPress={() => canInteract && onClick(app.id)} + onKeyDown={(e) => canInteract && (e.key === 'Enter' || e.key === ' ') && onClick(app.id)} role="radio" aria-checked={selected} className={classNames({ diff --git a/src/pages-and-resources/discussions/app-list/AppCard.test.jsx b/src/pages-and-resources/discussions/app-list/AppCard.test.jsx index cec39cf316..040d702413 100644 --- a/src/pages-and-resources/discussions/app-list/AppCard.test.jsx +++ b/src/pages-and-resources/discussions/app-list/AppCard.test.jsx @@ -148,7 +148,7 @@ describe('AppCard', () => { expect(handleClick).not.toHaveBeenCalled(); }); - test('card responds to keyPress when isEditable=true', async () => { + test('card responds to keyDown Enter when isEditable=true', async () => { const handleClick = jest.fn(); await mockStore(legacyApiResponse); @@ -166,12 +166,12 @@ describe('AppCard', () => { ); const card = wrapper.container.querySelector('[role="radio"]'); - fireEvent.keyPress(card, { key: 'Enter', charCode: 13 }); + fireEvent.keyDown(card, { key: 'Enter' }); expect(handleClick).toHaveBeenCalledWith(app.id); }); - test('card does NOT respond to keyPress when isEditable=false', async () => { + test('card does NOT respond to keyDown when isEditable=false', async () => { const handleClick = jest.fn(); await mockStore(legacyApiResponse); @@ -189,7 +189,7 @@ describe('AppCard', () => { ); const card = wrapper.container.querySelector('[role="radio"]'); - fireEvent.keyPress(card, { key: 'Enter', charCode: 13 }); + fireEvent.keyDown(card, { key: 'Enter' }); expect(handleClick).not.toHaveBeenCalled(); }); diff --git a/src/pages-and-resources/index.ts b/src/pages-and-resources/index.ts index 6cf33b0219..f35e8b67c0 100644 --- a/src/pages-and-resources/index.ts +++ b/src/pages-and-resources/index.ts @@ -1 +1,2 @@ export { default as PagesAndResources } from './PagesAndResources'; +export { PagesAndResourcesContext } from './PagesAndResourcesProvider'; diff --git a/src/pages-and-resources/pages/PageCard.test.jsx b/src/pages-and-resources/pages/PageCard.test.jsx index a135827509..da1600744b 100644 --- a/src/pages-and-resources/pages/PageCard.test.jsx +++ b/src/pages-and-resources/pages/PageCard.test.jsx @@ -72,34 +72,34 @@ describe('LiveSettings', () => { }); }); - it('renders readOnly mode correctly', async () => { + it('disables legacy-link arrow buttons in readOnly mode, but keeps settings gear accessible', async () => { render( , ); - // When isEditable=false, settings button should be disabled await waitFor(() => { - const buttons = screen.queryAllByRole('button'); - expect(buttons.length).toBeGreaterThan(0); + // Arrow buttons for legacy-link pages must be disabled so auditors + // can't navigate to external Studio pages that bypass isEditable. + const disabledButtons = screen.queryAllByRole('button').filter((btn) => btn.disabled); + expect(disabledButtons.length).toBeGreaterThan(0); }); }); - it('renders enabled by default when readOnly is not specified (default false)', async () => { - // readOnly defaults to false - page card should render with enabled settings + it('all buttons are enabled when isEditable=true', async () => { render( - + , ); - // Should render buttons (enabled by default) await waitFor(() => { const buttons = screen.queryAllByRole('button'); expect(buttons.length).toBeGreaterThan(0); + buttons.forEach((btn) => expect(btn).not.toBeDisabled()); }); }); - it('renders PageCard without isEditable in context (defaults to false, no settings button)', () => { + it('renders PageCard with default isEditable=true — settings button is present and enabled', () => { render( Date: Wed, 29 Apr 2026 16:02:33 -0500 Subject: [PATCH 05/10] feat(pages-and-resources): refactor isEditable context usage and improve accessibility attributes --- src/advanced-settings/AdvancedSettings.tsx | 2 +- .../PagesAndResourcesProvider.tsx | 3 +++ .../app-settings-modal/AppSettingsModal.jsx | 2 +- .../app-config-form/AppConfigForm.jsx | 2 +- .../AppConfigFormSaveButton.jsx | 2 +- .../apps/lti/LtiConfigForm.jsx | 4 ---- .../apps/openedx/OpenedXConfigForm.test.jsx | 19 ------------------- .../shared/InContextDiscussionFields.test.jsx | 15 +++++++++------ .../discussions/app-list/AppCard.jsx | 2 +- .../discussions/app-list/AppList.jsx | 2 +- .../app-list/AppListNextButton.jsx | 2 +- .../pages/PageCard.test.jsx | 13 ------------- .../pages/PageSettingButton.jsx | 19 +++++++++---------- 13 files changed, 28 insertions(+), 59 deletions(-) diff --git a/src/advanced-settings/AdvancedSettings.tsx b/src/advanced-settings/AdvancedSettings.tsx index 0a054448de..9a91dc0977 100644 --- a/src/advanced-settings/AdvancedSettings.tsx +++ b/src/advanced-settings/AdvancedSettings.tsx @@ -218,7 +218,7 @@ const AdvancedSettings = () => { />
-
+
{ const contextValue = useMemo(() => ({ courseId, diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx b/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx index de9dc60240..fa8eeae3e4 100644 --- a/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx +++ b/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx @@ -52,7 +52,7 @@ const AppSettingsModal = ({ hideAppToggle, }) => { const { formatMessage } = useIntl(); - const { courseId, isEditable = false } = useContext(PagesAndResourcesContext); + const { courseId, isEditable } = useContext(PagesAndResourcesContext); const loadingStatus = useSelector(getLoadingStatus); const updateSettingsRequestStatus = useSelector(getSavingStatus); const alertRef = useRef(null); diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx index d760771eed..23fc50f5e8 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx @@ -42,7 +42,7 @@ const AppConfigForm = ({ const navigate = useNavigate(); const { formRef } = useContext(AppConfigFormContext); - const { path: pagesAndResourcesPath, isEditable = false } = useContext(PagesAndResourcesContext); + const { path: pagesAndResourcesPath, isEditable } = useContext(PagesAndResourcesContext); const { appId: routeAppId } = useParams(); const [isLoading, setLoading] = useState(true); const { diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx index f0fee27ae4..47b93a1ad8 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx @@ -15,7 +15,7 @@ const AppConfigFormSaveButton = ({ labelText }) => { const intl = useIntl(); const saveStatus = useSelector(state => state.discussions.saveStatus); const { selectedAppId } = useSelector((state) => state.discussions); - const { isEditable = false } = useContext(PagesAndResourcesContext); + const { isEditable } = useContext(PagesAndResourcesContext); const app = useModel('apps', selectedAppId); const canSubmit = getAuthenticatedUser().administrator || !app?.adminOnlyConfig; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index f26d94ed6e..19ca91affc 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -197,8 +197,4 @@ LtiConfigForm.propTypes = { isEditable: PropTypes.bool, }; -LtiConfigForm.defaultProps = { - isEditable: true, -}; - export default LtiConfigForm; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx index b96388ce5b..9208654b99 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx @@ -121,7 +121,6 @@ describe('OpenedXConfigForm', () => { createComponent(jest.fn(), createRef(), true, true); const form = container.querySelector('form'); expect(form).toBeInTheDocument(); - expect(form).not.toBeDisabled(); }); test('renders form using default isEditable=false when prop is omitted', async () => { @@ -380,22 +379,4 @@ describe('OpenedXConfigForm', () => { }); }); - describe('isEditable prop', () => { - test('renders with isEditable=false (read-only mode)', async () => { - await mockStore(legacyApiResponse); - const wrapper = createComponent(jest.fn(), createRef(), true, false); - - // Form renders with disabled when isEditable=false (lines 139, 149, etc) - const form = wrapper.querySelector('form'); - expect(form).toBeInTheDocument(); - }); - - test('renders with isEditable=true (edit mode)', async () => { - await mockStore(legacyApiResponse); - const wrapper = createComponent(jest.fn(), createRef(), true, true); - - const form = wrapper.querySelector('form'); - expect(form).toBeInTheDocument(); - }); - }); }); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx index 9a7e593cea..731b81d140 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx @@ -3,6 +3,9 @@ import { render, screen, fireEvent } from '@testing-library/react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { Formik, Form } from 'formik'; import InContextDiscussionFields from './InContextDiscussionFields'; +import messages from '../../messages'; + +const gradedUnitLabel = messages.gradedUnitPagesLabel.defaultMessage; const defaultProps = { onBlur: jest.fn(), @@ -60,7 +63,7 @@ describe('InContextDiscussionFields', () => { renderComponent({ disabled: false }); // When not disabled, clicking the switch should trigger the onChange which sets showPopup // The component should still render - we test via the callback behavior - const switchControl = screen.getByLabelText(/enable discussions/i); + const switchControl = screen.getByLabelText(gradedUnitLabel); expect(switchControl).not.toBeDisabled(); }); @@ -68,14 +71,14 @@ describe('InContextDiscussionFields', () => { renderComponent({ disabled: true }); // When disabled, clicking should not trigger popup logic // Verify the switch is disabled - const switchControl = screen.getByLabelText(/enable discussions/i); + const switchControl = screen.getByLabelText(gradedUnitLabel); expect(switchControl).toBeDisabled(); }); it('shows confirmation popup and handles confirm', () => { renderComponent({ disabled: false, setFieldValue: jest.fn() }); // Click the switch to show popup - const switchControl = screen.getByLabelText(/enable discussions/i); + const switchControl = screen.getByLabelText(gradedUnitLabel); fireEvent.click(switchControl); // Check popup appears with confirm button expect(screen.getByText(/Confirm/i)).toBeInTheDocument(); @@ -89,7 +92,7 @@ describe('InContextDiscussionFields', () => { disabled: false, values: { enableGradedUnits: true, groupAtSubsection: false }, }); - const switchControl = screen.getByLabelText(/enable discussions/i); + const switchControl = screen.getByLabelText(gradedUnitLabel); fireEvent.click(switchControl); // Popup shows cancel-related labels (enableGradedUnits=true branch) expect(screen.getByText(/Confirm/i)).toBeInTheDocument(); @@ -98,7 +101,7 @@ describe('InContextDiscussionFields', () => { it('shows confirmation popup and handles cancel', () => { renderComponent({ disabled: false, setFieldValue: jest.fn() }); // Click the switch to show popup - const switchControl = screen.getByLabelText(/enable discussions/i); + const switchControl = screen.getByLabelText(gradedUnitLabel); fireEvent.click(switchControl); // Check popup appears with cancel button expect(screen.getByText(/Cancel/i)).toBeInTheDocument(); @@ -109,7 +112,7 @@ describe('InContextDiscussionFields', () => { it('does not show popup when onChange fires with disabled=true', () => { renderComponent({ disabled: true }); - const switchControl = screen.getByLabelText(/enable discussions/i); + const switchControl = screen.getByLabelText(gradedUnitLabel); // fireEvent bypasses browser disabled-input behavior, but the !disabled guard in onChange prevents the popup fireEvent.click(switchControl); expect(screen.queryByText(/Confirm/i)).not.toBeInTheDocument(); diff --git a/src/pages-and-resources/discussions/app-list/AppCard.jsx b/src/pages-and-resources/discussions/app-list/AppCard.jsx index 164e142bf0..c4ae0e3079 100644 --- a/src/pages-and-resources/discussions/app-list/AppCard.jsx +++ b/src/pages-and-resources/discussions/app-list/AppCard.jsx @@ -22,7 +22,7 @@ const AppCard = ({ }) => { const intl = useIntl(); const { canChangeProviders } = useCourseAuthoringContext(); - const { isEditable = false } = useContext(PagesAndResourcesContext); + const { isEditable } = useContext(PagesAndResourcesContext); const canInteract = canChangeProviders && isEditable; const supportText = app.hasFullSupport ? intl.formatMessage(messages.appFullSupport) diff --git a/src/pages-and-resources/discussions/app-list/AppList.jsx b/src/pages-and-resources/discussions/app-list/AppList.jsx index 722e9018d7..c49e092d36 100644 --- a/src/pages-and-resources/discussions/app-list/AppList.jsx +++ b/src/pages-and-resources/discussions/app-list/AppList.jsx @@ -40,7 +40,7 @@ import { discussionRestriction } from '../data/constants'; const AppList = () => { const intl = useIntl(); const dispatch = useDispatch(); - const { courseId, isEditable = false } = useContext(PagesAndResourcesContext); + const { courseId, isEditable } = useContext(PagesAndResourcesContext); const { appIds, featureIds, diff --git a/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx b/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx index 4ab6dcb7b7..82ed282ee0 100644 --- a/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx +++ b/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx @@ -13,7 +13,7 @@ const AppListNextButton = () => { const intl = useIntl(); const { selectedAppId } = useSelector(state => state.discussions); const { path: discussionsPath } = useContext(DiscussionsContext); - const { isEditable = false } = useContext(PagesAndResourcesContext); + const { isEditable } = useContext(PagesAndResourcesContext); const navigate = useNavigate(); const handleStartConfig = useCallback(() => { diff --git a/src/pages-and-resources/pages/PageCard.test.jsx b/src/pages-and-resources/pages/PageCard.test.jsx index da1600744b..ca5327bdb1 100644 --- a/src/pages-and-resources/pages/PageCard.test.jsx +++ b/src/pages-and-resources/pages/PageCard.test.jsx @@ -118,17 +118,4 @@ describe('LiveSettings', () => { expect(screen.getByText('Test Page')).toBeInTheDocument(); }); - it('disables arrow buttons for legacy-link pages when isEditable=false in context', async () => { - render( - - - , - ); - await waitFor(() => { - const buttons = screen.queryAllByRole('button'); - expect(buttons.length).toBeGreaterThan(0); - const disabledButtons = buttons.filter((btn) => btn.disabled); - expect(disabledButtons.length).toBeGreaterThan(0); - }); - }); }); diff --git a/src/pages-and-resources/pages/PageSettingButton.jsx b/src/pages-and-resources/pages/PageSettingButton.jsx index 917973609c..ab2b3ce1b4 100644 --- a/src/pages-and-resources/pages/PageSettingButton.jsx +++ b/src/pages-and-resources/pages/PageSettingButton.jsx @@ -17,7 +17,7 @@ const PageSettingButton = ({ allowedOperations, }) => { const { formatMessage } = useIntl(); - const { path: pagesAndResourcesPath, isEditable = false } = useContext(PagesAndResourcesContext); + const { path: pagesAndResourcesPath, isEditable } = useContext(PagesAndResourcesContext); const navigate = useNavigate(); const waffleFlags = useWaffleFlags(courseId); @@ -43,15 +43,14 @@ const PageSettingButton = ({ if (determineLinkDestination && !isEditable) { return ( - - - + ); } From 39ffd8509b7045e2f382658f8bda94b2e997c93d Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Wed, 29 Apr 2026 16:22:29 -0500 Subject: [PATCH 06/10] test(pages-and-resources): remove unnecessary blank lines in OpenedXConfigForm and PageCard tests --- .../app-config-form/apps/openedx/OpenedXConfigForm.test.jsx | 1 - src/pages-and-resources/pages/PageCard.test.jsx | 1 - 2 files changed, 2 deletions(-) diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx index 9208654b99..88d0de9d15 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx @@ -378,5 +378,4 @@ describe('OpenedXConfigForm', () => { assertHasErrorValidation(false); }); }); - }); diff --git a/src/pages-and-resources/pages/PageCard.test.jsx b/src/pages-and-resources/pages/PageCard.test.jsx index ca5327bdb1..c1bf2b756c 100644 --- a/src/pages-and-resources/pages/PageCard.test.jsx +++ b/src/pages-and-resources/pages/PageCard.test.jsx @@ -117,5 +117,4 @@ describe('LiveSettings', () => { ); expect(screen.getByText('Test Page')).toBeInTheDocument(); }); - }); From 8474790259b7e49cbb460f2919ea45ae209a222d Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 30 Apr 2026 08:33:06 -0500 Subject: [PATCH 07/10] refactor: change readOnly to isEditable to keep consistency --- src/advanced-settings/AdvancedSettings.tsx | 12 +++++------- .../setting-card/SettingCard.test.jsx | 7 +++---- src/advanced-settings/setting-card/SettingCard.tsx | 6 +++--- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/advanced-settings/AdvancedSettings.tsx b/src/advanced-settings/AdvancedSettings.tsx index 9a91dc0977..78c404e604 100644 --- a/src/advanced-settings/AdvancedSettings.tsx +++ b/src/advanced-settings/AdvancedSettings.tsx @@ -70,11 +70,9 @@ const AdvancedSettings = () => { const isLoading = isPendingSettingsStatus || (isAuthzEnabled && isLoadingUserPermissions); - // Determine if UI should be read-only (has VIEW but not MANAGE) - const isReadOnly = isAuthzEnabled - && !isLoadingUserPermissions - && !!canViewAdvancedSettings - && !canManageAdvancedSettings; + const isEditable = !isAuthzEnabled + || isLoadingUserPermissions + || !!canManageAdvancedSettings; const updateSettingsButtonState = { labels: { @@ -218,7 +216,7 @@ const AdvancedSettings = () => { />
-
+
{ handleBlur={handleSettingBlur} isEditableState={isEditableState} setIsEditableState={setIsEditableState} - readOnly={isReadOnly} + isEditable={isEditable} /> ); })} diff --git a/src/advanced-settings/setting-card/SettingCard.test.jsx b/src/advanced-settings/setting-card/SettingCard.test.jsx index 35218422dc..f8c3831d1e 100644 --- a/src/advanced-settings/setting-card/SettingCard.test.jsx +++ b/src/advanced-settings/setting-card/SettingCard.test.jsx @@ -92,14 +92,13 @@ describe('', () => { expect(handleBlur).toHaveBeenCalled(); }); }); - it('renders in readOnly mode with disabled input', () => { - render(); + it('renders in read-only mode with disabled input', () => { + render(); const input = screen.getByLabelText(/Setting Name/i); expect(input).toBeDisabled(); }); - it('renders enabled by default when readOnly is not specified (default false)', () => { - // readOnly defaults to false - input should be enabled + it('renders enabled by default when isEditable is not specified (default true)', () => { render(); const input = screen.getByLabelText(/Setting Name/i); expect(input).not.toBeDisabled(); diff --git a/src/advanced-settings/setting-card/SettingCard.tsx b/src/advanced-settings/setting-card/SettingCard.tsx index 78da537195..12a8226327 100644 --- a/src/advanced-settings/setting-card/SettingCard.tsx +++ b/src/advanced-settings/setting-card/SettingCard.tsx @@ -25,7 +25,7 @@ const SettingCard = ({ saveSettingsPrompt, isEditableState, setIsEditableState, - readOnly = false, + isEditable = true, }) => { const intl = useIntl(); const { deprecated, help, displayName } = settingData; @@ -100,7 +100,7 @@ const SettingCard = ({ onChange={handleSettingChange} aria-label={displayName} onBlur={handleCardBlur} - disabled={readOnly} + disabled={!isEditable} /> @@ -135,7 +135,7 @@ SettingCard.propTypes = { saveSettingsPrompt: PropTypes.bool.isRequired, isEditableState: PropTypes.bool.isRequired, setIsEditableState: PropTypes.func.isRequired, - readOnly: PropTypes.bool, + isEditable: PropTypes.bool, }; export default SettingCard; From 9edeba97637d894ac8129862da8f1d704039188c Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 30 Apr 2026 08:52:05 -0500 Subject: [PATCH 08/10] feat(pages-and-resources): remove isEditable prop from forms and related components --- src/advanced-settings/AdvancedSettings.tsx | 2 +- .../app-config-form/AppConfigForm.jsx | 5 +- .../apps/lti/LtiConfigForm.jsx | 13 +++-- .../apps/openedx/OpenedXConfigForm.jsx | 12 ++--- .../apps/openedx/OpenedXConfigForm.test.jsx | 32 +----------- .../apps/shared/InContextDiscussionFields.jsx | 6 +-- .../shared/InContextDiscussionFields.test.jsx | 51 +------------------ 7 files changed, 16 insertions(+), 105 deletions(-) diff --git a/src/advanced-settings/AdvancedSettings.tsx b/src/advanced-settings/AdvancedSettings.tsx index 78c404e604..bb64afa6f5 100644 --- a/src/advanced-settings/AdvancedSettings.tsx +++ b/src/advanced-settings/AdvancedSettings.tsx @@ -72,7 +72,7 @@ const AdvancedSettings = () => { const isEditable = !isAuthzEnabled || isLoadingUserPermissions - || !!canManageAdvancedSettings; + || canManageAdvancedSettings; const updateSettingsButtonState = { labels: { diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx index 23fc50f5e8..b498eef689 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx @@ -42,7 +42,7 @@ const AppConfigForm = ({ const navigate = useNavigate(); const { formRef } = useContext(AppConfigFormContext); - const { path: pagesAndResourcesPath, isEditable } = useContext(PagesAndResourcesContext); + const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); const { appId: routeAppId } = useParams(); const [isLoading, setLoading] = useState(true); const { @@ -102,7 +102,6 @@ const AppConfigForm = ({ formRef={formRef} onSubmit={handleSubmit} legacy - isEditable={isEditable} /> ); } else if (selectedAppId === 'openedx') { @@ -111,7 +110,6 @@ const AppConfigForm = ({ formRef={formRef} onSubmit={handleSubmit} legacy={false} - isEditable={isEditable} /> ); } else { @@ -119,7 +117,6 @@ const AppConfigForm = ({ ); } diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index 19ca91affc..f8881a55d9 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -18,7 +18,7 @@ import { useModel } from '../../../../../generic/model-store'; ensureConfig(['SITE_NAME', 'SUPPORT_EMAIL'], 'LTI Config Form'); -const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { +const LtiConfigForm = ({ onSubmit, formRef }) => { const intl = useIntl(); const dispatch = useDispatch(); @@ -113,7 +113,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { onChange={handleChange} onBlur={handleBlur} value={values.consumerKey} - disabled={!isEditable} + /> {isInvalidConsumerKey && ( @@ -131,7 +131,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { onChange={handleChange} onBlur={handleBlur} value={values.consumerSecret} - disabled={!isEditable} + /> {isInvalidConsumerSecret && ( @@ -145,7 +145,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { onChange={handleChange} onBlur={handleBlur} value={values.launchUrl} - disabled={!isEditable} + /> {isInvalidLaunchUrl && ( @@ -166,7 +166,7 @@ const LtiConfigForm = ({ onSubmit, formRef, isEditable = false }) => { onBlur={handleBlur} checked={values.piiShareUsername} label={intl.formatMessage(messages.piiShareUsername)} - disabled={!isEditable} + /> { onBlur={handleBlur} checked={values.piiShareEmail} label={intl.formatMessage(messages.piiShareEmail)} - disabled={!isEditable} + />
@@ -194,7 +194,6 @@ LtiConfigForm.propTypes = { onSubmit: PropTypes.func.isRequired, // eslint-disable-next-line react/forbid-prop-types formRef: PropTypes.object.isRequired, - isEditable: PropTypes.bool, }; export default LtiConfigForm; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx index 353847fda4..05df6e5ef4 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx @@ -25,7 +25,6 @@ const OpenedXConfigForm = ({ onSubmit, formRef, legacy, - isEditable = false, }) => { const intl = useIntl(); const { @@ -146,7 +145,6 @@ const OpenedXConfigForm = ({ onBlur={handleBlur} onChange={handleChange} values={values} - disabled={!isEditable} /> @@ -155,15 +153,14 @@ const OpenedXConfigForm = ({ onBlur={handleBlur} onChange={handleChange} values={values} - disabled={!isEditable} /> - + - + - - + + @@ -178,7 +175,6 @@ OpenedXConfigForm.propTypes = { onSubmit: PropTypes.func.isRequired, // eslint-disable-next-line react/forbid-prop-types formRef: PropTypes.object.isRequired, - isEditable: PropTypes.bool, }; export default OpenedXConfigForm; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx index 88d0de9d15..945952d262 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx @@ -77,7 +77,7 @@ describe('OpenedXConfigForm', () => { axiosMock.reset(); }); - const createComponent = (onSubmit = jest.fn(), formRef = createRef(), legacy = true, isEditable = false) => { + const createComponent = (onSubmit = jest.fn(), formRef = createRef(), legacy = true) => { const wrapper = render( @@ -85,7 +85,6 @@ describe('OpenedXConfigForm', () => { onSubmit={onSubmit} formRef={formRef} legacy={legacy} - isEditable={isEditable} /> , @@ -116,35 +115,6 @@ describe('OpenedXConfigForm', () => { expect(queryByText(container, messages.groupInContextSubsectionLabel.defaultMessage)).toBeInTheDocument(); }); - test('renders editable form when isEditable=true', async () => { - await mockStore(legacyApiResponse); - createComponent(jest.fn(), createRef(), true, true); - const form = container.querySelector('form'); - expect(form).toBeInTheDocument(); - }); - - test('renders form using default isEditable=false when prop is omitted', async () => { - await mockStore(legacyApiResponse); - const formRef = createRef(); - // Render directly without isEditable to trigger the default param = false - const wrapper = render( - - - - - , - ); - const form = wrapper.container.querySelector('form'); - expect(form).toBeInTheDocument(); - // isEditable defaults to false — child controls should be disabled - const divideByCohorts = wrapper.container.querySelector('#divideByCohorts'); - expect(divideByCohorts).toBeDisabled(); - }); - test('calls onSubmit when the formRef is submitted', async () => { const formRef = createRef(); const handleSubmit = jest.fn(); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx index 6be2ed75ca..38efa6fd60 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx @@ -11,7 +11,6 @@ const InContextDiscussionFields = ({ onBlur, onChange, values, - disabled = false, }) => { const intl = useIntl(); const { @@ -45,13 +44,12 @@ const InContextDiscussionFields = ({ ) : ( !disabled && setShowPopup(true)} + onChange={() => setShowPopup(true)} onBlur={onBlur} id="enableGradedUnits" checked={values.enableGradedUnits} label={intl.formatMessage(messages.gradedUnitPagesLabel)} helpText={intl.formatMessage(messages.gradedUnitPagesHelp)} - disabled={disabled} /> )} @@ -62,7 +60,6 @@ const InContextDiscussionFields = ({ checked={values.groupAtSubsection} label={intl.formatMessage(messages.groupInContextSubsectionLabel)} helpText={intl.formatMessage(messages.groupInContextSubsectionHelp)} - disabled={disabled} /> ); @@ -75,7 +72,6 @@ InContextDiscussionFields.propTypes = { enableGradedUnits: PropTypes.bool, groupAtSubsection: PropTypes.bool, }).isRequired, - disabled: PropTypes.bool, }; export default InContextDiscussionFields; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx index 731b81d140..03b9d82c8c 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx @@ -10,7 +10,6 @@ const gradedUnitLabel = messages.gradedUnitPagesLabel.defaultMessage; const defaultProps = { onBlur: jest.fn(), onChange: jest.fn(), - setFieldValue: jest.fn(), values: { enableGradedUnits: false, groupAtSubsection: false, @@ -36,18 +35,6 @@ describe('InContextDiscussionFields', () => { }); it('renders without crashing', () => { - renderComponent(); - // Component renders - check for presence of expected text - expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); - }); - - it('renders with disabled prop', () => { - renderComponent({ disabled: true }); - expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); - }); - - it('renders with default disabled (false)', () => { - // Test default param - no disabled prop passed renderComponent(); expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); }); @@ -59,62 +46,28 @@ describe('InContextDiscussionFields', () => { expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); }); - it('shows confirmation popup when toggling enableGradedUnits when not disabled', () => { - renderComponent({ disabled: false }); - // When not disabled, clicking the switch should trigger the onChange which sets showPopup - // The component should still render - we test via the callback behavior - const switchControl = screen.getByLabelText(gradedUnitLabel); - expect(switchControl).not.toBeDisabled(); - }); - - it('does NOT show popup when disabled', () => { - renderComponent({ disabled: true }); - // When disabled, clicking should not trigger popup logic - // Verify the switch is disabled - const switchControl = screen.getByLabelText(gradedUnitLabel); - expect(switchControl).toBeDisabled(); - }); - it('shows confirmation popup and handles confirm', () => { - renderComponent({ disabled: false, setFieldValue: jest.fn() }); - // Click the switch to show popup + renderComponent({ setFieldValue: jest.fn() }); const switchControl = screen.getByLabelText(gradedUnitLabel); fireEvent.click(switchControl); - // Check popup appears with confirm button expect(screen.getByText(/Confirm/i)).toBeInTheDocument(); - // Click confirm fireEvent.click(screen.getByText(/Confirm/i)); - // Popup should close (no longer visible) }); it('shows popup with cancel labels when enableGradedUnits is already true', () => { renderComponent({ - disabled: false, values: { enableGradedUnits: true, groupAtSubsection: false }, }); const switchControl = screen.getByLabelText(gradedUnitLabel); fireEvent.click(switchControl); - // Popup shows cancel-related labels (enableGradedUnits=true branch) expect(screen.getByText(/Confirm/i)).toBeInTheDocument(); }); it('shows confirmation popup and handles cancel', () => { - renderComponent({ disabled: false, setFieldValue: jest.fn() }); - // Click the switch to show popup + renderComponent({ setFieldValue: jest.fn() }); const switchControl = screen.getByLabelText(gradedUnitLabel); fireEvent.click(switchControl); - // Check popup appears with cancel button expect(screen.getByText(/Cancel/i)).toBeInTheDocument(); - // Click cancel fireEvent.click(screen.getByText(/Cancel/i)); - // Popup should close (no longer visible) - }); - - it('does not show popup when onChange fires with disabled=true', () => { - renderComponent({ disabled: true }); - const switchControl = screen.getByLabelText(gradedUnitLabel); - // fireEvent bypasses browser disabled-input behavior, but the !disabled guard in onChange prevents the popup - fireEvent.click(switchControl); - expect(screen.queryByText(/Confirm/i)).not.toBeInTheDocument(); }); }); From 427d7930376ca53623abbef771d06fd5b36aa943 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 30 Apr 2026 08:57:29 -0500 Subject: [PATCH 09/10] feat(pages-and-resources): remove isEditable prop and clean up related components --- .../AppConfigFormSaveButton.jsx | 3 - .../apps/lti/LtiConfigForm.jsx | 4 - .../apps/openedx/OpenedXConfigForm.jsx | 6 +- .../shared/InContextDiscussionFields.test.jsx | 73 ------------------- 4 files changed, 1 insertion(+), 85 deletions(-) delete mode 100644 src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx index 47b93a1ad8..ced3101670 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigFormSaveButton.jsx @@ -9,13 +9,11 @@ import messages from './messages'; import { SAVING } from '../data/slice'; import { AppConfigFormContext } from './AppConfigFormProvider'; import { useModel } from '../../../generic/model-store'; -import { PagesAndResourcesContext } from '../../PagesAndResourcesProvider'; const AppConfigFormSaveButton = ({ labelText }) => { const intl = useIntl(); const saveStatus = useSelector(state => state.discussions.saveStatus); const { selectedAppId } = useSelector((state) => state.discussions); - const { isEditable } = useContext(PagesAndResourcesContext); const app = useModel('apps', selectedAppId); const canSubmit = getAuthenticatedUser().administrator || !app?.adminOnlyConfig; @@ -47,7 +45,6 @@ const AppConfigFormSaveButton = ({ labelText }) => { }} state={submitButtonState} onClick={handleSave} - disabled={!isEditable} style={{ minWidth: '88px' }} /> ) diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index f8881a55d9..20171f57b2 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -113,7 +113,6 @@ const LtiConfigForm = ({ onSubmit, formRef }) => { onChange={handleChange} onBlur={handleBlur} value={values.consumerKey} - /> {isInvalidConsumerKey && ( @@ -145,7 +144,6 @@ const LtiConfigForm = ({ onSubmit, formRef }) => { onChange={handleChange} onBlur={handleBlur} value={values.launchUrl} - /> {isInvalidLaunchUrl && ( @@ -166,7 +164,6 @@ const LtiConfigForm = ({ onSubmit, formRef }) => { onBlur={handleBlur} checked={values.piiShareUsername} label={intl.formatMessage(messages.piiShareUsername)} - /> { onBlur={handleBlur} checked={values.piiShareEmail} label={intl.formatMessage(messages.piiShareEmail)} - />
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx index 05df6e5ef4..ccc1cbe6a8 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx @@ -141,11 +141,7 @@ const OpenedXConfigForm = ({ {!legacy && ( <> - + )} diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx deleted file mode 100644 index 03b9d82c8c..0000000000 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.test.jsx +++ /dev/null @@ -1,73 +0,0 @@ -// @ts-check -import { render, screen, fireEvent } from '@testing-library/react'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { Formik, Form } from 'formik'; -import InContextDiscussionFields from './InContextDiscussionFields'; -import messages from '../../messages'; - -const gradedUnitLabel = messages.gradedUnitPagesLabel.defaultMessage; - -const defaultProps = { - onBlur: jest.fn(), - onChange: jest.fn(), - values: { - enableGradedUnits: false, - groupAtSubsection: false, - }, -}; - -const renderComponent = (props = {}) => - render( - - - {() => ( -
- - - )} -
-
, - ); - -describe('InContextDiscussionFields', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('renders without crashing', () => { - renderComponent(); - expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); - }); - - it('renders with enableGradedUnits true (shows cancel labels)', () => { - renderComponent({ - values: { enableGradedUnits: true, groupAtSubsection: false }, - }); - expect(screen.getByText(/Visibility of in-context discussions/)).toBeInTheDocument(); - }); - - it('shows confirmation popup and handles confirm', () => { - renderComponent({ setFieldValue: jest.fn() }); - const switchControl = screen.getByLabelText(gradedUnitLabel); - fireEvent.click(switchControl); - expect(screen.getByText(/Confirm/i)).toBeInTheDocument(); - fireEvent.click(screen.getByText(/Confirm/i)); - }); - - it('shows popup with cancel labels when enableGradedUnits is already true', () => { - renderComponent({ - values: { enableGradedUnits: true, groupAtSubsection: false }, - }); - const switchControl = screen.getByLabelText(gradedUnitLabel); - fireEvent.click(switchControl); - expect(screen.getByText(/Confirm/i)).toBeInTheDocument(); - }); - - it('shows confirmation popup and handles cancel', () => { - renderComponent({ setFieldValue: jest.fn() }); - const switchControl = screen.getByLabelText(gradedUnitLabel); - fireEvent.click(switchControl); - expect(screen.getByText(/Cancel/i)).toBeInTheDocument(); - fireEvent.click(screen.getByText(/Cancel/i)); - }); -}); From fa1867f308f43eb3d18cf04debb9a306ef96470b Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 30 Apr 2026 09:02:48 -0500 Subject: [PATCH 10/10] fix(lti-config-form): remove unnecessary blank line in consumerSecret input --- .../discussions/app-config-form/apps/lti/LtiConfigForm.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index 20171f57b2..b0e2c37b84 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -130,7 +130,6 @@ const LtiConfigForm = ({ onSubmit, formRef }) => { onChange={handleChange} onBlur={handleBlur} value={values.consumerSecret} - /> {isInvalidConsumerSecret && (