From fe9cd6a911aa6c4e04a26e7b39d72da70426fdc1 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 26 Aug 2026 18:38:07 -0500 Subject: [PATCH 01/10] refactor: Migration of pages and resources to React Query - Migrates the main pages of pages and resources - Migrates the Settings modal of: ORA, Progress, Teams and Wiki --- .../ora_settings/Settings.test.jsx | 163 ----------------- .../ora_settings/Settings.test.tsx | 126 +++++++++++++ .../{Settings.jsx => Settings.tsx} | 85 ++++----- .../progress/{Settings.jsx => Settings.tsx} | 27 ++- .../teams/{Settings.jsx => Settings.tsx} | 33 ++-- .../wiki/{Settings.jsx => Settings.tsx} | 31 +++- src/CourseAuthoringContext.tsx | 51 +++++- src/CourseAuthoringPage.test.tsx | 12 +- src/data/api.ts | 72 ++++++++ src/data/apiHooks.ts | 53 ++++++ src/pages-and-resources/PagesAndResources.tsx | 38 ++-- ...SettingsModal.jsx => AppSettingsModal.tsx} | 171 +++++++++--------- .../AppSettingsModalBase.jsx | 68 ------- .../AppSettingsModalBase.tsx | 54 ++++++ src/pages-and-resources/data/api.js | 65 ------- src/utils.tsx | 30 +-- 16 files changed, 563 insertions(+), 516 deletions(-) delete mode 100644 plugins/course-apps/ora_settings/Settings.test.jsx create mode 100644 plugins/course-apps/ora_settings/Settings.test.tsx rename plugins/course-apps/ora_settings/{Settings.jsx => Settings.tsx} (65%) rename plugins/course-apps/progress/{Settings.jsx => Settings.tsx} (68%) rename plugins/course-apps/teams/{Settings.jsx => Settings.tsx} (88%) rename plugins/course-apps/wiki/{Settings.jsx => Settings.tsx} (60%) rename src/pages-and-resources/app-settings-modal/{AppSettingsModal.jsx => AppSettingsModal.tsx} (58%) delete mode 100644 src/pages-and-resources/app-settings-modal/AppSettingsModalBase.jsx create mode 100644 src/pages-and-resources/app-settings-modal/AppSettingsModalBase.tsx delete mode 100644 src/pages-and-resources/data/api.js diff --git a/plugins/course-apps/ora_settings/Settings.test.jsx b/plugins/course-apps/ora_settings/Settings.test.jsx deleted file mode 100644 index 82c7f7b497..0000000000 --- a/plugins/course-apps/ora_settings/Settings.test.jsx +++ /dev/null @@ -1,163 +0,0 @@ -import { - render, - screen, - waitFor, - within, -} from '@testing-library/react'; -import ReactDOM from 'react-dom'; -import { Routes, Route, MemoryRouter } from 'react-router-dom'; -import { initializeMockApp } from '@edx/frontend-platform'; -import MockAdapter from 'axios-mock-adapter'; -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { AppProvider, PageWrap } from '@edx/frontend-platform/react'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; - -import initializeStore from 'CourseAuthoring/store'; -import { executeThunk } from 'CourseAuthoring/utils'; -import PagesAndResourcesProvider from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; -import { getCourseAppsApiUrl, getCourseAdvancedSettingsApiUrl } from 'CourseAuthoring/pages-and-resources/data/api'; -import { fetchCourseApps, fetchCourseAppSettings } from 'CourseAuthoring/pages-and-resources/data/thunks'; -import ORASettings from './Settings'; -import messages from './messages'; -import { - courseId, - inititalState, -} from './factories/mockData'; - -let axiosMock; -let store; -const oraSettingsUrl = `/course/${courseId}/pages-and-resources/live/settings`; - -// Modal creates a portal. Overriding ReactDOM.createPortal allows portals to be tested in jest. -ReactDOM.createPortal = jest.fn(node => node); - -const renderComponent = () => ( - render( - - - - - - - - - } - /> - - - - - , - ) -); - -const mockStore = async ({ - apiStatus, - enabled, -}) => { - const settings = ['forceOnFlexiblePeerOpenassessments']; - const fetchCourseAppsUrl = `${getCourseAppsApiUrl()}/${courseId}`; - const fetchAdvancedSettingsUrl = `${getCourseAdvancedSettingsApiUrl()}/${courseId}`; - - axiosMock.onGet(fetchCourseAppsUrl).reply( - 200, - [{ - allowed_operations: { enable: false, configure: true }, - description: 'setting', - documentation_links: { learnMoreConfiguration: '' }, - enabled, - id: 'ora_settings', - name: 'Flexible Peer Grading for ORAs', - }], - ); - axiosMock.onGet(fetchAdvancedSettingsUrl).reply( - apiStatus, - { force_on_flexible_peer_openassessments: { value: enabled } }, - ); - - await executeThunk(fetchCourseApps(courseId), store.dispatch); - await executeThunk(fetchCourseAppSettings(courseId, settings), store.dispatch); -}; - -describe('ORASettings', () => { - beforeEach(async () => { - initializeMockApp({ - authenticatedUser: { - userId: 3, - username: 'abc123', - administrator: false, - roles: [], - }, - }); - store = initializeStore(inititalState); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - }); - - it('Flexible peer grading configuration modal is visible', async () => { - renderComponent(); - expect(screen.getByRole('dialog')).toBeVisible(); - }); - - it('Displays "Configure Flexible Peer Grading" heading', async () => { - renderComponent(); - const headingElement = screen.getByText(messages.heading.defaultMessage); - - expect(headingElement).toBeVisible(); - }); - - it('Displays loading component', () => { - renderComponent(); - const loadingElement = screen.getByRole('status'); - - expect(within(loadingElement).getByText('Loading...')).toBeInTheDocument(); - }); - - it('Displays Connection Error Alert', async () => { - await mockStore({ apiStatus: 404, enabled: true }); - renderComponent(); - const errorAlert = screen.getByRole('alert'); - - expect(within(errorAlert).getByText('We encountered a technical error when loading this page.', { exact: false })) - .toBeVisible(); - }); - - it('Displays Permissions Error Alert', async () => { - await mockStore({ apiStatus: 403, enabled: true }); - renderComponent(); - const errorAlert = screen.getByRole('alert'); - - expect(within(errorAlert).getByText('You are not authorized to view this page', { exact: false })).toBeVisible(); - }); - - it('Displays title, helper text and badge when flexible peer grading button is enabled', async () => { - await mockStore({ apiStatus: 200, enabled: true }); - renderComponent(); - - const checkbox = screen.getByRole('checkbox', { name: /Flex Peer Grading/ }); - expect(checkbox).toBeChecked(); - - await waitFor(() => { - const label = screen.getByText(messages.enableFlexPeerGradeLabel.defaultMessage); - const enableBadge = screen.getByTestId('enable-badge'); - - expect(label).toBeVisible(); - - expect(enableBadge).toHaveTextContent('Enabled'); - }); - }); - - it('Displays title, helper text and hides badge when flexible peer grading button is disabled', async () => { - renderComponent(); - await mockStore({ apiStatus: 200, enabled: false }); - - const label = await screen.findByText(messages.enableFlexPeerGradeLabel.defaultMessage); - const enableBadge = screen.queryByTestId('enable-badge'); - - expect(label).toBeVisible(); - - expect(enableBadge).toBeNull(); - }); -}); diff --git a/plugins/course-apps/ora_settings/Settings.test.tsx b/plugins/course-apps/ora_settings/Settings.test.tsx new file mode 100644 index 0000000000..76e73e9f19 --- /dev/null +++ b/plugins/course-apps/ora_settings/Settings.test.tsx @@ -0,0 +1,126 @@ +import { + screen, + waitFor, + within, +} from '@testing-library/react'; +import ReactDOM from 'react-dom'; + +import { getCourseAppsApiUrl, getCourseAdvancedSettingsApiUrl } from 'CourseAuthoring/data/api'; +import { CourseAuthoringProvider } from 'CourseAuthoring/CourseAuthoringContext'; +import { initializeMocks, render } from 'CourseAuthoring/testUtils'; +import ORASettings from './Settings'; +import messages from './messages'; + +const courseId = 'course-v1:org+num+run'; + +let axiosMock; + +// Modal creates a portal. Overriding ReactDOM.createPortal allows portals to be tested in jest. +// @ts-ignore +ReactDOM.createPortal = jest.fn(node => node); + +const renderComponent = () => render( + + + , +); + +const mockCourseApps = ({ apiStatus = 200, enabled = true } = {}) => { + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply( + apiStatus, + [{ + allowed_operations: { enable: false, configure: true }, + description: 'setting', + documentation_links: { learn_more_configuration: '' }, + enabled, + id: 'ora_settings', + name: 'Flexible Peer Grading for ORAs', + }], + ); +}; + +const mockAdvancedSettings = ({ enabled = true } = {}) => { + axiosMock.onGet(`${getCourseAdvancedSettingsApiUrl()}/${courseId}`).reply( + 200, + { force_on_flexible_peer_openassessments: { value: enabled } }, + ); +}; + +describe('ORASettings', () => { + beforeEach(() => { + const mocks = initializeMocks(); + axiosMock = mocks.axiosMock; + }); + + it('Flexible peer grading configuration modal is visible', async () => { + mockCourseApps(); + mockAdvancedSettings(); + renderComponent(); + + expect(await screen.findByRole('dialog')).toBeVisible(); + }); + + it('Displays "Configure Flexible Peer Grading" heading', async () => { + mockCourseApps(); + mockAdvancedSettings(); + renderComponent(); + + const headingElement = await screen.findByText(messages.heading.defaultMessage); + expect(headingElement).toBeVisible(); + }); + + it('Displays loading component', () => { + mockCourseApps(); + mockAdvancedSettings(); + renderComponent(); + const loadingElement = screen.getByRole('status'); + + expect(within(loadingElement).getByText('Loading...')).toBeInTheDocument(); + }); + + it('Displays Connection Error Alert', async () => { + mockCourseApps({ apiStatus: 404 }); + renderComponent(); + + const errorAlert = await screen.findByRole('alert'); + expect(within(errorAlert).getByText('We encountered a technical error when loading this page.', { exact: false })) + .toBeVisible(); + }); + + it('Displays Permissions Error Alert', async () => { + mockCourseApps({ apiStatus: 403 }); + renderComponent(); + + const errorAlert = await screen.findByRole('alert'); + expect(within(errorAlert).getByText('You are not authorized to view this page', { exact: false })).toBeVisible(); + }); + + it('Displays title, helper text and badge when flexible peer grading button is enabled', async () => { + mockCourseApps(); + mockAdvancedSettings({ enabled: true }); + renderComponent(); + + const checkbox = await screen.findByRole('checkbox', { name: /Flex Peer Grading/ }); + expect(checkbox).toBeChecked(); + + await waitFor(() => { + const label = screen.getByText(messages.enableFlexPeerGradeLabel.defaultMessage); + const enableBadge = screen.getByTestId('enable-badge'); + + expect(label).toBeVisible(); + expect(enableBadge).toHaveTextContent('Enabled'); + }); + }); + + it('Displays title, helper text and hides badge when flexible peer grading button is disabled', async () => { + mockCourseApps(); + mockAdvancedSettings({ enabled: false }); + renderComponent(); + + const label = await screen.findByText(messages.enableFlexPeerGradeLabel.defaultMessage); + const enableBadge = screen.queryByTestId('enable-badge'); + + expect(label).toBeVisible(); + expect(enableBadge).toBeNull(); + }); +}); diff --git a/plugins/course-apps/ora_settings/Settings.jsx b/plugins/course-apps/ora_settings/Settings.tsx similarity index 65% rename from plugins/course-apps/ora_settings/Settings.jsx rename to plugins/course-apps/ora_settings/Settings.tsx index 0ea271817d..50891d2612 100644 --- a/plugins/course-apps/ora_settings/Settings.jsx +++ b/plugins/course-apps/ora_settings/Settings.tsx @@ -1,8 +1,6 @@ import { useEffect, useState, useRef } from 'react'; -import PropTypes from 'prop-types'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { useDispatch, useSelector } from 'react-redux'; import { ActionRow, @@ -14,7 +12,6 @@ import { StatefulButton, } from '@openedx/paragon'; import { Info } from '@openedx/paragon/icons'; -import { updateModel, useModel } from 'CourseAuthoring/generic/model-store'; import { RequestStatus } from 'CourseAuthoring/data/constants'; import FormSwitchGroup from 'CourseAuthoring/generic/FormSwitchGroup'; @@ -22,50 +19,46 @@ import Loading from 'CourseAuthoring/generic/Loading'; import PermissionDeniedAlert from 'CourseAuthoring/generic/PermissionDeniedAlert'; import ConnectionErrorAlert from 'CourseAuthoring/generic/ConnectionErrorAlert'; import { useAppSetting, useIsMobile } from 'CourseAuthoring/utils'; -import { getLoadingStatus, getSavingStatus } from 'CourseAuthoring/pages-and-resources/data/selectors'; -import { updateSavingStatus } from 'CourseAuthoring/pages-and-resources/data/slice'; +import { useUpdateCourseAdvancedSettings } from 'CourseAuthoring/data/apiHooks'; +import { useCourseAuthoringContext } from 'CourseAuthoring/CourseAuthoringContext'; import messages from './messages'; -const ORASettings = ({ onClose }) => { - const dispatch = useDispatch(); +const ORASettings = ({ onClose }: { onClose : () => void}) => { const { formatMessage } = useIntl(); - const alertRef = useRef(null); - const updateSettingsRequestStatus = useSelector(getSavingStatus); - const loadingStatus = useSelector(getLoadingStatus); + const alertRef = useRef(null); + const { + courseId, + courseApps, + courseAppsStatus, + + } = useCourseAuthoringContext(); + const isMobile = useIsMobile(); const modalVariant = isMobile ? 'dark' : 'default'; const appId = 'ora_settings'; - const appInfo = useModel('courseApps', appId); + const appInfo = courseApps.find((app) => app.id === appId); - const [enableFlexiblePeerGrade, saveSetting] = useAppSetting( - 'forceOnFlexiblePeerOpenassessments', - ); - const initialFormValues = { enableFlexiblePeerGrade }; + const updateCourseAdvancedSettingsMutation = useUpdateCourseAdvancedSettings(courseId); + const settingName = 'forceOnFlexiblePeerOpenassessments'; - const [formValues, setFormValues] = useState(initialFormValues); - const [saveError, setSaveError] = useState(false); + const enableFlexiblePeerGrade = useAppSetting(settingName); - const submitButtonState = updateSettingsRequestStatus === RequestStatus.IN_PROGRESS ? 'pending' : 'default'; - const handleSettingsSave = (values) => saveSetting(values.enableFlexiblePeerGrade); + const [formValues, setFormValues] = useState({ enableFlexiblePeerGrade }); + + useEffect(() => { + setFormValues({ enableFlexiblePeerGrade }); + }, [enableFlexiblePeerGrade]); + + const submitButtonState = updateCourseAdvancedSettingsMutation.isPending ? 'pending' : 'default'; + const handleSettingsSave = (values) => updateCourseAdvancedSettingsMutation.mutate({ + setting: settingName, + value: values.enableFlexiblePeerGrade, + }); const handleSubmit = async (event) => { - let success = true; event.preventDefault(); - - success = success && await handleSettingsSave(formValues); - setSaveError(!success); - if ((initialFormValues.enableFlexiblePeerGrade !== formValues.enableFlexiblePeerGrade) && success) { - // oxlint-disable-next-line @typescript-eslint/await-thenable - this dispatch() IS returning a promise. - success = await dispatch(updateModel({ - modelType: 'courseApps', - model: { - id: appId, - enabled: formValues.enableFlexiblePeerGrade, - }, - })); - } - !success && alertRef?.current.scrollIntoView(); // eslint-disable-line @typescript-eslint/no-unused-expressions + await handleSettingsSave(formValues); }; const handleChange = (e) => { @@ -73,18 +66,23 @@ const ORASettings = ({ onClose }) => { }; useEffect(() => { - if (updateSettingsRequestStatus === RequestStatus.SUCCESSFUL) { - dispatch(updateSavingStatus({ status: '' })); + if (updateCourseAdvancedSettingsMutation.isSuccess) { onClose(); } - }, [updateSettingsRequestStatus]); + }, [updateCourseAdvancedSettingsMutation.isSuccess]); + + useEffect(() => { + if (updateCourseAdvancedSettingsMutation.isError) { + alertRef?.current?.scrollIntoView?.(); + } + }, [updateCourseAdvancedSettingsMutation.isError]); const renderBody = () => { - switch (loadingStatus) { + switch (courseAppsStatus) { case RequestStatus.SUCCESSFUL: return ( <> - {saveError && ( + {updateCourseAdvancedSettingsMutation.isError && ( {formatMessage(messages.errorSavingTitle)} @@ -111,7 +109,7 @@ const ORASettings = ({ onClose }) => { @@ -144,6 +142,7 @@ const ORASettings = ({ onClose }) => { hasCloseButton={isMobile} isFullscreenScroll isFullscreenOnMobile + isOverflowVisible >
@@ -166,7 +165,7 @@ const ORASettings = ({ onClose }) => { }} description="Form save button" data-testid="submissionButton" - disabled={submitButtonState === RequestStatus.IN_PROGRESS} + disabled={submitButtonState === 'pending'} state={submitButtonState} type="submit" /> @@ -177,8 +176,4 @@ const ORASettings = ({ onClose }) => { ); }; -ORASettings.propTypes = { - onClose: PropTypes.func.isRequired, -}; - export default ORASettings; diff --git a/plugins/course-apps/progress/Settings.jsx b/plugins/course-apps/progress/Settings.tsx similarity index 68% rename from plugins/course-apps/progress/Settings.jsx rename to plugins/course-apps/progress/Settings.tsx index 1f01c56c79..672e479701 100644 --- a/plugins/course-apps/progress/Settings.jsx +++ b/plugins/course-apps/progress/Settings.tsx @@ -1,20 +1,35 @@ import { useIntl } from '@edx/frontend-platform/i18n'; -import PropTypes from 'prop-types'; import React 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 { useUpdateCourseAdvancedSettings } from 'CourseAuthoring/data/apiHooks'; +import { useCourseAuthoringContext } from 'CourseAuthoring/CourseAuthoringContext'; import messages from './messages'; -const ProgressSettings = ({ onClose }) => { +const ProgressSettings = ({ onClose }: { onClose: () => void }) => { const intl = useIntl(); - const [disableProgressGraph, saveSetting] = useAppSetting('disableProgressGraph'); + const { courseId } = useCourseAuthoringContext(); + const settingsName = 'disableProgressGraph' + const disableProgressGraph = useAppSetting(settingsName); + const updateCourseAdvancedSettingsMutation = useUpdateCourseAdvancedSettings(courseId); const showProgressGraphSetting = getConfig().ENABLE_PROGRESS_GRAPH_SETTINGS.toString().toLowerCase() === 'true'; const handleSettingsSave = async (values) => { - if (showProgressGraphSetting) { await saveSetting(!values.enableProgressGraph); } + if (showProgressGraphSetting) { + try { + await updateCourseAdvancedSettingsMutation.mutateAsync({ + setting: settingsName, + value: !values.enableProgressGraph, + }); + return true; + } catch { + return false; + } + } + return true; }; return ( @@ -46,8 +61,4 @@ const ProgressSettings = ({ onClose }) => { ); }; -ProgressSettings.propTypes = { - onClose: PropTypes.func.isRequired, -}; - export default ProgressSettings; diff --git a/plugins/course-apps/teams/Settings.jsx b/plugins/course-apps/teams/Settings.tsx similarity index 88% rename from plugins/course-apps/teams/Settings.jsx rename to plugins/course-apps/teams/Settings.tsx index 038cf6fb7b..cdd50129fe 100644 --- a/plugins/course-apps/teams/Settings.jsx +++ b/plugins/course-apps/teams/Settings.tsx @@ -3,14 +3,15 @@ import { Button, Form } from '@openedx/paragon'; import { Add } from '@openedx/paragon/icons'; import { FieldArray } from 'formik'; -import PropTypes from 'prop-types'; import React from 'react'; import { v4 as uuid } from 'uuid'; import * as Yup from 'yup'; import { GroupTypes, TeamSizes } from 'CourseAuthoring/data/constants'; import FormikControl from 'CourseAuthoring/generic/FormikControl'; import { setupYupExtensions, useAppSetting } from 'CourseAuthoring/utils'; +import { useUpdateCourseAdvancedSettings } from 'CourseAuthoring/data/apiHooks'; import AppSettingsModal from 'CourseAuthoring/pages-and-resources/app-settings-modal/AppSettingsModal'; +import { useCourseAuthoringContext } from 'CourseAuthoring/CourseAuthoringContext'; import GroupEditor from './GroupEditor'; import messages from './messages'; @@ -18,9 +19,12 @@ setupYupExtensions(); const TeamSettings = ({ onClose, -}) => { +}: { onClose: () => void }) => { const intl = useIntl(); - const [teamsConfiguration, saveSettings] = useAppSetting('teamsConfiguration'); + const { courseId } = useCourseAuthoringContext(); + const settingName = 'teamsConfiguration'; + const teamsConfiguration = useAppSetting(settingName); + const updateCourseAdvancedSettingsMutation = useUpdateCourseAdvancedSettings(courseId); const blankNewGroup = { name: '', description: '', @@ -41,11 +45,19 @@ const TeamSettings = ({ max_team_size: group.maxTeamSize, user_partition_id: group.userPartitionId, })); - return saveSettings({ - team_sets: groups, - max_team_size: values.maxTeamSize, - enabled: values.enabled, - }); + try { + await updateCourseAdvancedSettingsMutation.mutateAsync({ + setting: settingName, + value: { + team_sets: groups, + max_team_size: values.maxTeamSize, + enabled: values.enabled, + }, + }); + return true; + } catch { + return false; + } }; const enableAppError = { title: intl.formatMessage(messages.noGroupsErrorTitle), @@ -60,7 +72,6 @@ const TeamSettings = ({ enableAppLabel={intl.formatMessage(messages.enableTeamsLabel)} learnMoreText={intl.formatMessage(messages.enableTeamsLink)} onClose={onClose} - bodyClassName="bg-light-200" // Topic is supported for backwards compatibility, the new field is team_sets: // ref: https://github.com/openedx/edx-platform/blob/15461d3b6e6c0a724a7b8ed09241d970f201e5e7/openedx/core/lib/teams_config.py#L104-L108 initialValues={{ @@ -167,8 +178,4 @@ const TeamSettings = ({ ); }; -TeamSettings.propTypes = { - onClose: PropTypes.func.isRequired, -}; - export default TeamSettings; diff --git a/plugins/course-apps/wiki/Settings.jsx b/plugins/course-apps/wiki/Settings.tsx similarity index 60% rename from plugins/course-apps/wiki/Settings.jsx rename to plugins/course-apps/wiki/Settings.tsx index ffd62cbf35..00b3c6d14c 100644 --- a/plugins/course-apps/wiki/Settings.jsx +++ b/plugins/course-apps/wiki/Settings.tsx @@ -1,17 +1,31 @@ import { useIntl } from '@edx/frontend-platform/i18n'; -import PropTypes from 'prop-types'; -import React from 'react'; import * as Yup from 'yup'; +import { useCourseAuthoringContext } from 'CourseAuthoring/CourseAuthoringContext'; import FormSwitchGroup from 'CourseAuthoring/generic/FormSwitchGroup'; import { useAppSetting } from 'CourseAuthoring/utils'; import AppSettingsModal from 'CourseAuthoring/pages-and-resources/app-settings-modal/AppSettingsModal'; +import { useUpdateCourseAdvancedSettings } from 'CourseAuthoring/data/apiHooks'; import messages from './messages'; -const WikiSettings = ({ onClose }) => { +const WikiSettings = ({ onClose }: { onClose: () => void }) => { const intl = useIntl(); - const [enablePublicWiki, saveSetting] = useAppSetting('allowPublicWikiAccess'); - const handleSettingsSave = (values) => saveSetting(values.enablePublicWiki); + const settingName = 'allowPublicWikiAccess'; + const enablePublicWiki = useAppSetting(settingName); + const { courseId } = useCourseAuthoringContext(); + + const updateCourseAdvancedSettingsMutation = useUpdateCourseAdvancedSettings(courseId); + const handleSettingsSave = async (values) => { + try { + await updateCourseAdvancedSettingsMutation.mutateAsync({ + setting: settingName, + value: values.enablePublicWiki, + }); + return true; + } catch { + return false; + } + }; return ( { enableAppLabel={intl.formatMessage(messages.enableWikiLabel)} learnMoreText={intl.formatMessage(messages.enableWikiLink)} onClose={onClose} - initialValues={{ enablePublicWiki }} + initialValues={{ enablePublicWiki: !!enablePublicWiki }} validationSchema={{ enablePublicWiki: Yup.boolean() }} onSettingsSave={handleSettingsSave} + enableReinitialize > {({ values, handleChange, handleBlur }) => ( { ); }; -WikiSettings.propTypes = { - onClose: PropTypes.func.isRequired, -}; - export default WikiSettings; diff --git a/src/CourseAuthoringContext.tsx b/src/CourseAuthoringContext.tsx index bc444bd7cb..a78fbed0d1 100644 --- a/src/CourseAuthoringContext.tsx +++ b/src/CourseAuthoringContext.tsx @@ -1,15 +1,16 @@ +import { useNavigate } from 'react-router'; import { createContext, useContext, useMemo, } from 'react'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; -import { useNavigate } from 'react-router'; + import { useToggleWithValue } from '@src/hooks'; import { type UnitXBlock, type XBlock } from '@src/data/types'; -import { CourseDetailsData } from './data/api'; -import { useCourseDetails } from './data/apiHooks'; -import { RequestStatusType } from './data/constants'; +import { CourseAppData, CourseDetailsData } from './data/api'; +import { useCourseApps, useCourseDetails } from './data/apiHooks'; +import { RequestStatus, RequestStatusType } from './data/constants'; export type ModalState = { value?: XBlock | UnitXBlock; @@ -17,6 +18,20 @@ export type ModalState = { sectionId?: string; }; +const COURSE_APPS_ORDER = [ + 'progress', + 'discussion', + 'teams', + 'edxnotes', + 'wiki', + 'calculator', + 'proctoring', + 'live', + 'textbooks', + 'custom_pages', + 'ora_settings', +]; + export type CourseAuthoringContextData = { /** The ID of the current course */ courseId: string; @@ -29,6 +44,8 @@ export type CourseAuthoringContextData = { currentUnlinkModalData?: ModalState; openUnlinkModal: (value: ModalState) => void; closeUnlinkModal: () => void; + courseApps: CourseAppData[]; + courseAppsStatus: RequestStatusType; }; /** @@ -60,6 +77,28 @@ export const CourseAuthoringProvider = ({ const getUnitUrl = (locator: string) => `/course/${courseId}/container/${locator}`; + const { + data: courseApps, + isPending: courseAppsIsPending, + failureReason: courseAppsError, + } = useCourseApps(courseId); + + let courseAppsStatus: RequestStatusType = RequestStatus.SUCCESSFUL; + + if (courseAppsIsPending) { + courseAppsStatus = RequestStatus.PENDING; + } else if (courseAppsError) { + if (courseAppsError?.response?.status === 403) { + courseAppsStatus = RequestStatus.DENIED; + } else { + courseAppsStatus = RequestStatus.FAILED; + } + } + + courseApps?.sort((firstEl, secondEl) => ( + COURSE_APPS_ORDER.indexOf(firstEl.id) - COURSE_APPS_ORDER.indexOf(secondEl.id) + )); + /** * Open the unit page for a given locator. */ @@ -78,6 +117,8 @@ export const CourseAuthoringProvider = ({ openUnlinkModal, closeUnlinkModal, currentUnlinkModalData, + courseApps: courseApps || [], + courseAppsStatus, }), [ courseId, courseDetails, @@ -89,6 +130,8 @@ export const CourseAuthoringProvider = ({ openUnlinkModal, closeUnlinkModal, currentUnlinkModalData, + courseApps, + courseAppsStatus, ]); return ( diff --git a/src/CourseAuthoringPage.test.tsx b/src/CourseAuthoringPage.test.tsx index 8a221f1a8c..8505d659b3 100644 --- a/src/CourseAuthoringPage.test.tsx +++ b/src/CourseAuthoringPage.test.tsx @@ -2,9 +2,7 @@ import { getConfig } from '@edx/frontend-platform'; import CourseAuthoringPage from './CourseAuthoringPage'; import PagesAndResources from './pages-and-resources/PagesAndResources'; -import { executeThunk } from './utils'; -import { fetchCourseApps } from './pages-and-resources/data/thunks'; -import { getApiWaffleFlagsUrl } from './data/api'; +import { getApiWaffleFlagsUrl, getCourseAppsApiUrl } from './data/api'; import { initializeMocks, render } from './testUtils'; import { CourseAuthoringProvider } from './CourseAuthoringContext'; @@ -17,7 +15,6 @@ jest.mock('react-router-dom', () => ({ }), })); let axiosMock; -let store; const renderComponent = children => render( @@ -28,7 +25,6 @@ const renderComponent = children => beforeEach(async () => { const mocks = initializeMocks(); - store = mocks.reduxStore; axiosMock = mocks.axiosMock; axiosMock .onGet(getApiWaffleFlagsUrl(courseId)) @@ -103,13 +99,9 @@ describe('Course authoring page', () => { expect(wrapper.queryByTestId('notFoundAlert')).not.toBeInTheDocument(); }); const mockStoreDenied = async () => { - const studioApiBaseUrl = getConfig().STUDIO_BASE_URL; - const courseAppsApiUrl = `${studioApiBaseUrl}/api/course_apps/v1/apps`; - axiosMock.onGet( - `${courseAppsApiUrl}/${courseId}`, + `${getCourseAppsApiUrl()}/${courseId}`, ).reply(403, { response: { status: 403 } }); - await executeThunk(fetchCourseApps(courseId), store.dispatch); }; test('renders PermissionDeniedAlert when courseAppsApiStatus is DENIED', async () => { mockPathname = '/editor/'; diff --git a/src/data/api.ts b/src/data/api.ts index a41644ab30..25412738d9 100644 --- a/src/data/api.ts +++ b/src/data/api.ts @@ -1,3 +1,4 @@ +import { snakeCase } from 'lodash/string'; import { camelCaseObject, getConfig, snakeCaseObject } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; @@ -264,3 +265,74 @@ export async function getCourseSettings(courseId: string): Promise `${getStudioBaseUrl()}/api/course_apps/v1/apps`; +export const getCourseAdvancedSettingsApiUrl = () => `${getStudioBaseUrl()}/api/contentstore/v0/advanced_settings`; + +export interface CourseAppData { + id: string; + name: string; + description: string; + enabled: boolean; + documentationLinks: Record; + allowedOperations: { + enable: boolean; + configure: boolean; + }; + legacyLink?: string; +} + +/** + * Fetches the course apps installed for provided course + */ +export async function getCourseApps(courseId: string): Promise { + const { data } = await getAuthenticatedHttpClient() + .get(`${getCourseAppsApiUrl()}/${courseId}`); + + return camelCaseObject(data); +} + +/** + * Updates the status of a course app. + */ +export async function updateCourseApp(courseId: string, appId: string, state: boolean) { + await getAuthenticatedHttpClient() + .patch( + `${getCourseAppsApiUrl()}/${courseId}`, + { + id: appId, + enabled: state, + }, + ); +} + +/** + * Get's advanced setting for a course. + */ +export async function getCourseAdvancedSettings( + courseId: string, + settings: string[], +): Promise { + const { data } = await getAuthenticatedHttpClient() + .get(`${getCourseAdvancedSettingsApiUrl()}/${courseId}`, { + params: { + filter_fields: settings.map(snakeCase).join(','), + }, + }); + + return camelCaseObject(data); +} + +/** + * Get's advanced setting for a course. + */ +export async function updateCourseAdvancedSettings( + courseId: string, + setting: string, + value: any, +): Promise> { + const { data } = await getAuthenticatedHttpClient() + .patch(`${getCourseAdvancedSettingsApiUrl()}/${courseId}`, { [snakeCase(setting)]: { value } }); + + return camelCaseObject(data); +} diff --git a/src/data/apiHooks.ts b/src/data/apiHooks.ts index ba8baee8ff..efee6e9a67 100644 --- a/src/data/apiHooks.ts +++ b/src/data/apiHooks.ts @@ -26,6 +26,11 @@ import { waffleFlagDefaults, getCourseSettings, CourseSettingsData, + CourseAppData, + getCourseApps, + updateCourseApp, + getCourseAdvancedSettings, + updateCourseAdvancedSettings, } from './api'; import { RequestStatus, RequestStatusType } from './constants'; @@ -237,3 +242,51 @@ export const useCourseSettings = (courseId: string) => ( queryFn: () => getCourseSettings(courseId), }) ); + +/** + * Fetch the course apps installed for a course. + */ +export const useCourseApps = (courseId: string) => ( + useQuery({ + queryKey: ['courseApps', courseId], + queryFn: () => getCourseApps(courseId), + }) +); + +/** + * Update the enabled status of a course app. + * Invalidates the course apps list on success. + */ +export const useUpdateCourseAppStatus = (courseId: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: ({ appId, state }: { appId: string; state: boolean; }) => updateCourseApp(courseId, appId, state), + onSuccess: () => queryClient.invalidateQueries({ queryKey: ['courseApps', courseId] }), + }); +}; + +/** + * Fetch advanced settings for a course, filtered to the given setting names. + */ +export const useCourseAdvancedSettings = (courseId: string, settings: string[]) => ( + useQuery({ + queryKey: ['courseSettings', courseId], + queryFn: () => getCourseAdvancedSettings(courseId, settings), + }) +); + +/** + * Update a single advanced setting for a course. + */ +export const useUpdateCourseAdvancedSettings = (courseId: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: ({ setting, value }: { setting: string; value: unknown; }) => ( + updateCourseAdvancedSettings(courseId, setting, value) + ), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['courseSettings', courseId] }); + queryClient.invalidateQueries({ queryKey: ['courseApps', courseId] }); + }, + }); +}; diff --git a/src/pages-and-resources/PagesAndResources.tsx b/src/pages-and-resources/PagesAndResources.tsx index 972611a5ea..fccd50c24e 100644 --- a/src/pages-and-resources/PagesAndResources.tsx +++ b/src/pages-and-resources/PagesAndResources.tsx @@ -1,33 +1,32 @@ -import { useEffect } from 'react'; import { Routes, Route } from 'react-router-dom'; -import { useDispatch, useSelector } from 'react-redux'; import { getConfig } from '@edx/frontend-platform'; import { useIntl } from '@edx/frontend-platform/i18n'; import { PageWrap } from '@edx/frontend-platform/react'; import { Button, Hyperlink } from '@openedx/paragon'; -import { useModels } from '@src/generic/model-store'; -import { RequestStatus } from '@src/data/constants'; import PermissionDeniedAlert from '@src/generic/PermissionDeniedAlert'; import getPageHeadTitle from '@src/generic/utils'; import { AdditionalCoursePluginSlot } from '@src/plugin-slots/AdditionalCoursePluginSlot'; 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'; import PageGrid from './pages/PageGrid'; -import { fetchCourseApps } from './data/thunks'; -import { getCourseAppsApiStatus, getLoadingStatus } from './data/selectors'; import PagesAndResourcesProvider from './PagesAndResourcesProvider'; import SettingsComponent from './SettingsComponent'; +import { RequestStatus } from '@src/data/constants'; const PagesAndResources = () => { const intl = useIntl(); - const { courseId, courseDetails } = useCourseAuthoringContext(); + const { + courseId, + courseDetails, + courseApps, + courseAppsStatus, + } = useCourseAuthoringContext(); document.title = getPageHeadTitle(courseDetails?.name || '', intl.formatMessage(messages.heading)); const { @@ -37,41 +36,28 @@ const PagesAndResources = () => { canManagePagesAndResources, } = useCourseUserPermissions(courseId, getPagesAndResourcesPermissions(courseId)); - const dispatch = useDispatch(); - useEffect(() => { - dispatch(fetchCourseApps(courseId)); - }, [courseId]); - - const courseAppIds = useSelector((state: DeprecatedReduxState) => state.pagesAndResources.courseAppIds); - const loadingStatus = useSelector(getLoadingStatus); - const courseAppsApiStatus = useSelector(getCourseAppsApiStatus); - const learningCourseURL = `${getConfig().LEARNING_BASE_URL}/course/${courseId}`; const redirectUrl = `/course/${courseId}/pages-and-resources`; - // The pages here are driven by course apps. The list of course app IDs comes from the LMS API. - // We display all enabled course apps regardless of whether or not the corresponding frontend plugin is available. - const pages = useModels('courseApps', courseAppIds); - // We want the Xpert learning assistant and unit summaries to appear in the "Content Permissions" section instead, // so we remove them from pages and add them to contentPermissionsPages. const contentPermissionsPages: any[] = []; ['xpert_unit_summary', 'learning_assistant'].forEach(separateAppId => { - const index = pages.findIndex(app => app.id === separateAppId); + const index = courseApps.findIndex(app => app.id === separateAppId); if (index !== -1) { - const [page] = pages.splice(index, 1); + const [page] = courseApps.splice(index, 1); contentPermissionsPages.push(page); } }); - if (loadingStatus === RequestStatus.IN_PROGRESS || isLoadingUserPermissions) { + if (courseAppsStatus === RequestStatus.PENDING || isLoadingUserPermissions) { // eslint-disable-next-line react/jsx-no-useless-fragment return <>; } // Gate: if user has neither VIEW nor MANAGE permission, show permission denied - const hasNoAccess = (!isAuthzEnabled && courseAppsApiStatus === RequestStatus.DENIED) + const hasNoAccess = (!isAuthzEnabled && courseAppsStatus === RequestStatus.DENIED) || (isAuthzEnabled && !canViewPagesAndResources && !canManagePagesAndResources); if (hasNoAccess) { @@ -132,7 +118,7 @@ const PagesAndResources = () => { } courseId={courseId} /> diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx b/src/pages-and-resources/app-settings-modal/AppSettingsModal.tsx similarity index 58% rename from src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx rename to src/pages-and-resources/app-settings-modal/AppSettingsModal.tsx index fa8eeae3e4..e60b1a861a 100644 --- a/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx +++ b/src/pages-and-resources/app-settings-modal/AppSettingsModal.tsx @@ -10,93 +10,127 @@ import { import { Info } from '@openedx/paragon/icons'; import { Formik } from 'formik'; -import PropTypes from 'prop-types'; import React, { + ReactNode, useContext, useEffect, useRef, useState, } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; import * as Yup from 'yup'; -import { RequestStatus } from '../../data/constants'; -import ConnectionErrorAlert from '../../generic/ConnectionErrorAlert'; -import FormSwitchGroup from '../../generic/FormSwitchGroup'; -import Loading from '../../generic/Loading'; -import { useModel } from '../../generic/model-store'; -import PermissionDeniedAlert from '../../generic/PermissionDeniedAlert'; -import { useIsMobile } from '../../utils'; -import { getLoadingStatus, getSavingStatus } from '../data/selectors'; -import { updateSavingStatus } from '../data/slice'; -import { updateAppStatus } from '../data/thunks'; +import ConnectionErrorAlert from '@src/generic/ConnectionErrorAlert'; +import FormSwitchGroup from '@src/generic/FormSwitchGroup'; +import Loading from '@src/generic/Loading'; +import PermissionDeniedAlert from '@src/generic/PermissionDeniedAlert'; +import { RequestStatus } from '@src/data/constants'; +import { useIsMobile } from '@src/utils'; +import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; +import { useUpdateCourseAppStatus } from '@src/data/apiHooks'; + import AppConfigFormDivider from '../discussions/app-config-form/apps/shared/AppConfigFormDivider'; import { PagesAndResourcesContext } from '../PagesAndResourcesProvider'; import AppSettingsModalBase from './AppSettingsModalBase'; import messages from './messages'; +export interface AppSettingsModalProps { + title: string; + appId: string; + children?: Function; + bodyChildren?: ReactNode; + onSettingsSave?: Function; + initialValues: Record; + validationSchema: Record; + onClose: () => void; + enableAppLabel: string; + enableAppHelp: string; + learnMoreText?: string; + configureBeforeEnable?: boolean; + enableReinitialize?: boolean; + hideAppToggle?: boolean; +} + const AppSettingsModal = ({ appId, title, children, bodyChildren, - configureBeforeEnable, - initialValues, - validationSchema, + configureBeforeEnable = false, + initialValues = {}, + validationSchema = {}, onClose, onSettingsSave, enableAppLabel, enableAppHelp, learnMoreText, - enableReinitialize, - hideAppToggle, -}) => { + enableReinitialize = false, + hideAppToggle = false, +}: AppSettingsModalProps) => { const { formatMessage } = useIntl(); - const { courseId, isEditable } = useContext(PagesAndResourcesContext); - const loadingStatus = useSelector(getLoadingStatus); - const updateSettingsRequestStatus = useSelector(getSavingStatus); - const alertRef = useRef(null); - const [saveError, setSaveError] = useState(false); - // FIXME: open the "Live" settings, then refresh the page. The courseApps model is not loaded, and an error occurs - // when trying to access 'appInfo.documentationLinks'. This happens even before the refactor to use plugins. - const appInfo = useModel('courseApps', appId); - const dispatch = useDispatch(); - const submitButtonState = updateSettingsRequestStatus === RequestStatus.IN_PROGRESS ? 'pending' : 'default'; + const { isEditable } = useContext(PagesAndResourcesContext); + const { + courseId, + courseApps, + courseAppsStatus, + } = useCourseAuthoringContext(); + const appInfo = courseApps.find((app) => app.id === appId); + const updateCourseAppStatusMutation = useUpdateCourseAppStatus(courseId); + + const alertRef = useRef(null); + const [formError, setFormError] = useState(false); + const [isSavingCallbackSuccess, setIsSavingCallbackSuccess] = useState(true); + const [isSubmitting, setIsSubmitting] = useState(false); + const inError = updateCourseAppStatusMutation.isError || !isSavingCallbackSuccess || formError; + + const submitButtonState = (isSubmitting || updateCourseAppStatusMutation.isPending) ? 'pending' : 'default'; const isMobile = useIsMobile(); const modalVariant = isMobile ? 'dark' : 'default'; - useEffect(() => { - if (updateSettingsRequestStatus === RequestStatus.SUCCESSFUL) { - dispatch(updateSavingStatus({ status: '' })); - onClose(); - } - }, [updateSettingsRequestStatus]); - const handleFormSubmit = async (values) => { - let success = true; - if (appInfo.enabled !== values.enabled) { - // oxlint-disable-next-line @typescript-eslint/await-thenable - this dispatch() IS returning a promise. - success = await dispatch(updateAppStatus(courseId, appInfo.id, values.enabled)); + if (!appInfo) { + return; } - // Call the submit handler for the settings component to save its settings - if (onSettingsSave) { - success = success && await onSettingsSave(values); + setIsSubmitting(true); + try { + if (appInfo.enabled !== values.enabled) { + await updateCourseAppStatusMutation.mutateAsync({ + appId: appInfo.id, + state: values.enabled, + }); + } + // Call the submit handler for the settings component to save its settings + const success = onSettingsSave ? await onSettingsSave(values) : true; + setIsSavingCallbackSuccess(success); + if (success) { + onClose(); + } + } catch { + setIsSavingCallbackSuccess(false); + } finally { + setIsSubmitting(false); } - setSaveError(!success); - !success && alertRef?.current.scrollIntoView(); // eslint-disable-line @typescript-eslint/no-unused-expressions }; + useEffect(() => { + if (inError) { + alertRef?.current?.scrollIntoView() + } + }, [inError]) + + const handleFormikSubmit = ({ handleSubmit, errors }) => async (event) => { + // Clear any error left over from a previous failed attempt so a successful + // resubmit isn't blocked by stale state. + setFormError(false); // If submitting the form with errors, show the alert and scroll to it. await handleSubmit(event); if (Object.keys(errors).length > 0) { /* istanbul ignore next: temp to unblock lint cleanup. We probably should test this. */ - setSaveError(true); - alertRef?.current.scrollIntoView?.(); // eslint-disable-line no-unused-expressions + setFormError(true); } }; - const learnMoreLink = appInfo.documentationLinks?.learnMoreConfiguration && ( + const learnMoreLink = appInfo?.documentationLinks?.learnMoreConfiguration && ( ); - if (loadingStatus === RequestStatus.SUCCESSFUL) { + if (courseAppsStatus === RequestStatus.SUCCESSFUL) { return ( } > - {saveError && ( + {inError && ( + {/* @ts-expect-error -- errors.enabled is typed as string|FormikErrors by Formik, but the API returns { title, message } here */} {formikProps.errors.enabled?.title || formatMessage(messages.errorSavingTitle)} + {/* @ts-expect-error -- see above */} {formikProps.errors.enabled?.message || formatMessage(messages.errorSavingMessage)} )} @@ -202,45 +238,14 @@ const AppSettingsModal = ({ title={title} isOpen onClose={onClose} - size="sm" variant={modalVariant} isMobile={isMobile} - isFullscreenOnMobile > - {loadingStatus === RequestStatus.IN_PROGRESS && } - {loadingStatus === RequestStatus.FAILED && } - {loadingStatus === RequestStatus.DENIED && } + {courseAppsStatus === RequestStatus.PENDING && } + {courseAppsStatus === RequestStatus.FAILED && } + {courseAppsStatus === RequestStatus.DENIED && } ); }; -AppSettingsModal.propTypes = { - title: PropTypes.string.isRequired, - appId: PropTypes.string.isRequired, - children: PropTypes.func, - bodyChildren: PropTypes.node, - onSettingsSave: PropTypes.func, - initialValues: PropTypes.shape({}), - validationSchema: PropTypes.shape({}), - onClose: PropTypes.func.isRequired, - enableAppLabel: PropTypes.string.isRequired, - enableAppHelp: PropTypes.string.isRequired, - learnMoreText: PropTypes.string, - configureBeforeEnable: PropTypes.bool, - enableReinitialize: PropTypes.bool, - hideAppToggle: PropTypes.bool, -}; - -AppSettingsModal.defaultProps = { - children: null, - bodyChildren: null, - onSettingsSave: null, - initialValues: {}, - validationSchema: {}, - learnMoreText: null, - configureBeforeEnable: false, - enableReinitialize: false, - hideAppToggle: false, -}; - export default AppSettingsModal; diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.jsx b/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.jsx deleted file mode 100644 index 19224a2d0a..0000000000 --- a/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.jsx +++ /dev/null @@ -1,68 +0,0 @@ -import { useIntl } from '@edx/frontend-platform/i18n'; -import { ActionRow, ModalDialog } from '@openedx/paragon'; - -import PropTypes from 'prop-types'; -import React from 'react'; - -import messages from './messages'; - -const AppSettingsModalBase = ({ - title, - onClose, - variant, - isMobile, - children, - footer, - disclaimer, - isOpen, -}) => { - const { formatMessage } = useIntl(); - return ( - - - {title} - - {children} - - {disclaimer} - - - {formatMessage(messages.cancel)} - - {footer} - - - - ); -}; - -AppSettingsModalBase.defaultProps = { - isOpen: true, -}; - -AppSettingsModalBase.propTypes = { - title: PropTypes.string.isRequired, - onClose: PropTypes.func.isRequired, - variant: PropTypes.oneOf(['default', 'dark']).isRequired, - isMobile: PropTypes.bool.isRequired, - children: PropTypes.node.isRequired, - footer: PropTypes.node, - disclaimer: PropTypes.node, - isOpen: PropTypes.bool, -}; - -AppSettingsModalBase.defaultProps = { - footer: null, - disclaimer: null, -}; - -export default AppSettingsModalBase; diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.tsx b/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.tsx new file mode 100644 index 0000000000..d8cf53b6e7 --- /dev/null +++ b/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.tsx @@ -0,0 +1,54 @@ +import { ReactNode } from 'react'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; +import { ActionRow, ModalDialog } from '@openedx/paragon'; + +import messages from './messages'; + +export interface AppSettingsModalBaseProps { + title: string; + onClose: () => void; + variant: 'default' | 'dark'; + isMobile: boolean; + children: ReactNode; + footer?: ReactNode; + disclaimer?: ReactNode; + isOpen: boolean; +} + +const AppSettingsModalBase = ({ + title, + onClose, + variant, + isMobile, + children, + footer, + disclaimer, + isOpen = true, +}: AppSettingsModalBaseProps) => ( + + + {title} + + {children} + + {disclaimer} + + + + + {footer} + + + +); + +export default AppSettingsModalBase; diff --git a/src/pages-and-resources/data/api.js b/src/pages-and-resources/data/api.js deleted file mode 100644 index 92ddc751b2..0000000000 --- a/src/pages-and-resources/data/api.js +++ /dev/null @@ -1,65 +0,0 @@ -import { snakeCase } from 'lodash/string'; - -import { camelCaseObject, ensureConfig, getConfig } from '@edx/frontend-platform'; -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; - -ensureConfig([ - 'STUDIO_BASE_URL', -], 'Course Apps API service'); - -const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL; -export const getCourseAppsApiUrl = () => `${getApiBaseUrl()}/api/course_apps/v1/apps`; -export const getCourseAdvancedSettingsApiUrl = () => `${getApiBaseUrl()}/api/contentstore/v0/advanced_settings`; - -/** - * Fetches the course apps installed for provided course - * @param {string} courseId - * @returns {Promise<[{}]>} - */ -export async function getCourseApps(courseId) { - const { data } = await getAuthenticatedHttpClient() - .get(`${getCourseAppsApiUrl()}/${courseId}`); - return camelCaseObject(data); -} - -/** - * Updates the status of a course app. - * @param {string} courseId Course ID for the course to operate on - * @param {string} appId ID for the application to operate on - * @param {boolean} state The new state - */ -export async function updateCourseApp(courseId, appId, state) { - await getAuthenticatedHttpClient() - .patch( - `${getCourseAppsApiUrl()}/${courseId}`, - { - id: appId, - enabled: state, - }, - ); -} - -/** - * Get's advanced setting for a course. - * @param {string} courseId - * @param {[string]} settings - * @returns {Promise} - */ -export async function getCourseAdvancedSettings(courseId, settings) { - const { data } = await getAuthenticatedHttpClient() - .get(`${getCourseAdvancedSettingsApiUrl()}/${courseId}`, { filter_fields: settings.map(snakeCase).join(',') }); - return camelCaseObject(data); -} - -/** - * Get's advanced setting for a course. - * @param {string} courseId - * @param {string} setting - * @param {*} value - * @returns {Promise} - */ -export async function updateCourseAdvancedSettings(courseId, setting, value) { - const { data } = await getAuthenticatedHttpClient() - .patch(`${getCourseAdvancedSettingsApiUrl()}/${courseId}`, { [snakeCase(setting)]: { value } }); - return camelCaseObject(data); -} diff --git a/src/utils.tsx b/src/utils.tsx index e42e31a3cc..047e0537da 100644 --- a/src/utils.tsx +++ b/src/utils.tsx @@ -1,5 +1,4 @@ -import React, { useState, useContext, useEffect } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; +import React, { useState, useEffect } from 'react'; import { useMediaQuery } from 'react-responsive'; import * as Yup from 'yup'; import { snakeCase } from 'lodash/string'; @@ -8,10 +7,6 @@ import { getConfig, getPath } from '@edx/frontend-platform'; import type { Dispatch, AnyAction } from 'redux'; import type { TypeOfShape } from 'yup/lib/object'; -import { RequestStatus } from './data/constants'; -import { getCourseAppSettingValue, getLoadingStatus } from './pages-and-resources/data/selectors'; -import { fetchCourseAppSettings, updateCourseAppSetting } from './pages-and-resources/data/thunks'; -import { PagesAndResourcesContext } from './pages-and-resources/PagesAndResourcesProvider'; import { hasValidDateFormat, hasValidTimeFormat, @@ -20,6 +15,8 @@ import { startOfDayTime, } from './pages-and-resources/discussions/app-config-form/utils'; import { DATE_TIME_FORMAT } from './constants'; +import { useCourseAdvancedSettings, useUpdateCourseAdvancedSettings } from './data/apiHooks'; +import { useCourseAuthoringContext } from './CourseAuthoringContext'; export const executeThunk = async ( thunk: (dispatch: any, state?: any) => Promise, @@ -132,22 +129,13 @@ export function getPagePath(courseId: string | undefined, isMfePageEnabled: stri return `${getConfig().STUDIO_BASE_URL}/${urlParameter}/${courseId}`; } -export function useAppSetting(settingName: string) { - const dispatch = useDispatch(); - const { courseId } = useContext(PagesAndResourcesContext); - const settingValue = useSelector(getCourseAppSettingValue(settingName)); - const loadingStatus = useSelector(getLoadingStatus); - useEffect(() => { - if ([RequestStatus.DENIED, RequestStatus.FAILED].includes(loadingStatus)) { - return; - } - if (settingValue === undefined || settingValue === null) { - dispatch(fetchCourseAppSettings(courseId, [settingName])); - } - }, [courseId]); +export function useAppSetting(settingName: string): any { + const { courseId } = useCourseAuthoringContext(); - const saveSetting = async (value: any) => dispatch(updateCourseAppSetting(courseId, settingName, value)); - return [settingValue, saveSetting]; + const { + data: settingValue, + } = useCourseAdvancedSettings(courseId, [settingName]); + return settingValue?.[settingName]?.value; } export const getLabelById = (options: any[], id: any) => { From 72ccf4a96a2d448879c46f1be493612b1e8d0f32 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Sat, 29 Aug 2026 17:31:53 -0500 Subject: [PATCH 02/10] feat: Migrate Xpert Unit Summary to React Query --- .../xpert_unit_summary/Settings.jsx | 11 +- .../xpert_unit_summary/Settings.test.jsx | 182 +++++------------- .../xpert_unit_summary/data/api.js | 41 ---- .../xpert_unit_summary/data/api.ts | 50 +++++ .../xpert_unit_summary/data/apiHooks.ts | 59 ++++++ .../xpert_unit_summary/data/thunks.js | 122 ------------ .../settings-modal/SettingsModal.jsx | 100 +++++----- src/CourseAuthoringContext.tsx | 11 +- src/pages-and-resources/PagesAndResources.tsx | 18 +- 9 files changed, 224 insertions(+), 370 deletions(-) delete mode 100644 plugins/course-apps/xpert_unit_summary/data/api.js create mode 100644 plugins/course-apps/xpert_unit_summary/data/api.ts create mode 100644 plugins/course-apps/xpert_unit_summary/data/apiHooks.ts delete mode 100644 plugins/course-apps/xpert_unit_summary/data/thunks.js diff --git a/plugins/course-apps/xpert_unit_summary/Settings.jsx b/plugins/course-apps/xpert_unit_summary/Settings.jsx index 69577444e6..d687eb47ef 100644 --- a/plugins/course-apps/xpert_unit_summary/Settings.jsx +++ b/plugins/course-apps/xpert_unit_summary/Settings.jsx @@ -1,5 +1,4 @@ -import React, { useCallback, useContext, useEffect } from 'react'; -import { useDispatch } from 'react-redux'; +import React, { useCallback, useContext } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { PagesAndResourcesContext } from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; import { useNavigate } from 'react-router-dom'; @@ -7,18 +6,12 @@ import { useNavigate } from 'react-router-dom'; import SettingsModal from './settings-modal/SettingsModal'; import messages from './messages'; -import { fetchXpertSettings } from './data/thunks'; const XpertUnitSummarySettings = () => { const intl = useIntl(); - const { path: pagesAndResourcesPath, courseId } = useContext(PagesAndResourcesContext); - const dispatch = useDispatch(); + const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); const navigate = useNavigate(); - useEffect(() => { - dispatch(fetchXpertSettings(courseId)); - }, [courseId]); - const handleClose = useCallback(() => { navigate(pagesAndResourcesPath); }, [pagesAndResourcesPath]); diff --git a/plugins/course-apps/xpert_unit_summary/Settings.test.jsx b/plugins/course-apps/xpert_unit_summary/Settings.test.jsx index d8ff859913..adf4041a8a 100644 --- a/plugins/course-apps/xpert_unit_summary/Settings.test.jsx +++ b/plugins/course-apps/xpert_unit_summary/Settings.test.jsx @@ -1,33 +1,18 @@ import ReactDOM from 'react-dom'; -import React from 'react'; -import { MemoryRouter, Routes, Route } from 'react-router-dom'; import { - getConfig, - initializeMockApp, - setConfig, -} from '@edx/frontend-platform'; -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { AppProvider, PageWrap } from '@edx/frontend-platform/react'; -import { - findByTestId, - queryByTestId, + initializeMocks, render, + screen, + userEvent, waitFor, - getByText, - fireEvent, -} from '@testing-library/react'; -import MockAdapter from 'axios-mock-adapter'; +} from 'CourseAuthoring/testUtils'; import PagesAndResourcesProvider from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; -import initializeStore from 'CourseAuthoring/store'; -import { executeThunk } from 'CourseAuthoring/utils'; import XpertUnitSummarySettings from './Settings'; import * as API from './data/api'; -import * as Thunks from './data/thunks'; const courseId = 'course-v1:edX+TestX+Test_Course'; let axiosMock; -let store; let container; // Modal creates a portal. Overriding ReactDOM.createPortal allows portals to be tested in jest. @@ -35,30 +20,9 @@ ReactDOM.createPortal = jest.fn(node => node); function renderComponent() { const wrapper = render( - - - - - - - - } - /> - -
- - } - /> - - - - , + + + , ); container = wrapper.container; } @@ -77,38 +41,7 @@ function generateCourseLevelAPIResponse({ describe('XpertUnitSummarySettings', () => { beforeEach(() => { - setConfig({ - ...getConfig(), - BASE_URL: 'http://test.edx.org', - LMS_BASE_URL: 'http://lmstest.edx.org', - CMS_BASE_URL: 'http://cmstest.edx.org', - LOGIN_URL: 'http://support.edx.org/login', - LOGOUT_URL: 'http://support.edx.org/logout', - REFRESH_ACCESS_TOKEN_ENDPOINT: 'http://support.edx.org/access_token', - ACCESS_TOKEN_COOKIE_NAME: 'cookie', - CSRF_TOKEN_API_PATH: '/', - SUPPORT_URL: 'http://support.edx.org', - }); - - initializeMockApp({ - authenticatedUser: { - userId: 3, - username: 'abc123', - administrator: true, - roles: [], - }, - }); - - store = initializeStore({ - models: { - courseDetails: { - [courseId]: { - start: Date(), - }, - }, - }, - }); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + ({ axiosMock } = initializeMocks()); }); describe('with successful network connections', () => { @@ -126,12 +59,19 @@ describe('XpertUnitSummarySettings', () => { }); test('Shows switch on if enabled from backend', async () => { - const enableBadge = await findByTestId(container, 'enable-badge'); + const enableBadge = await screen.findByTestId('enable-badge'); expect(container.querySelector('#enable-xpert-unit-summary-toggle').checked).toBeTruthy(); expect(enableBadge).toBeTruthy(); }); - test('Shows switch on if disabled from backend', async () => { + test('Shows enable radio selected if enabled from backend', async () => { + await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); + expect(screen.getByTestId('enable-radio').checked).toBeTruthy(); + }); + }); + + describe('course configured with units disabled by default', () => { + beforeEach(() => { axiosMock.onGet(API.getXpertSettingsUrl(courseId)) .reply( 200, @@ -142,42 +82,28 @@ describe('XpertUnitSummarySettings', () => { ); renderComponent(); - await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - expect(container.querySelector('#enable-xpert-unit-summary-toggle').checked).toBeTruthy(); - expect(queryByTestId(container, 'enable-badge')).toBeTruthy(); }); - test('Shows enable radio selected if enabled from backend', async () => { + // A course-level record existing with enabled: false still means the app itself is + // configured for this course, so the top switch and badge remain on -- only the + // "which units get summaries by default" radio reflects the false value. + test('Shows switch on (app is configured) even though units are disabled by default', async () => { await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - expect(queryByTestId(container, 'enable-radio').checked).toBeTruthy(); + expect(container.querySelector('#enable-xpert-unit-summary-toggle').checked).toBeTruthy(); + expect(screen.queryByTestId('enable-badge')).toBeTruthy(); }); - test('Shows disable radio selected if enabled from backend', async () => { - axiosMock.onGet(API.getXpertSettingsUrl(courseId)) - .reply( - 200, - generateCourseLevelAPIResponse({ - success: true, - enabled: false, - }), - ); - - renderComponent(); + test('Shows disable radio selected', async () => { await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - expect(queryByTestId(container, 'disable-radio').checked).toBeTruthy(); + expect(screen.getByTestId('disable-radio').checked).toBeTruthy(); }); }); describe('first time course configuration', () => { beforeEach(() => { + // A course that has never been configured gets a 404 from ai_aside, not a 400. axiosMock.onGet(API.getXpertSettingsUrl(courseId)) - .reply( - 400, - generateCourseLevelAPIResponse({ - success: false, - enabled: undefined, - }), - ); + .reply(404); renderComponent(); }); @@ -185,7 +111,7 @@ describe('XpertUnitSummarySettings', () => { test('Does not show as enabled if configuration does not exist', async () => { await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); expect(container.querySelector('#enable-xpert-unit-summary-toggle').checked).not.toBeTruthy(); - expect(queryByTestId(container, 'enable-badge')).not.toBeTruthy(); + expect(screen.queryByTestId('enable-badge')).not.toBeTruthy(); }); }); @@ -213,34 +139,14 @@ describe('XpertUnitSummarySettings', () => { }); test('Saving configuration changes', async () => { + const user = userEvent.setup(); jest.spyOn(API, 'postXpertSettings'); await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - expect(queryByTestId(container, 'disable-radio').checked).toBeTruthy(); - fireEvent.click(queryByTestId(container, 'enable-radio')); - fireEvent.click(getByText(container, 'Save')); - await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - expect(API.postXpertSettings).toBeCalled(); - }); - }); - - describe('testing configurable gating', () => { - beforeEach(async () => { - axiosMock.onGet(API.getXpertConfigurationStatusUrl(courseId)) - .reply( - 200, - generateCourseLevelAPIResponse({ - success: true, - enabled: true, - }), - ); - jest.spyOn(API, 'getXpertPluginConfigurable'); - await executeThunk(Thunks.fetchXpertPluginConfigurable(courseId), store.dispatch); - renderComponent(); - }); - - test('getting Xpert Plugin configurable status', () => { - expect(API.getXpertPluginConfigurable).toBeCalled(); + expect(screen.getByTestId('disable-radio').checked).toBeTruthy(); + await user.click(screen.getByTestId('enable-radio')); + await user.click(screen.getByText('Save')); + await waitFor(() => expect(API.postXpertSettings).toBeCalled()); }); }); @@ -268,18 +174,19 @@ describe('XpertUnitSummarySettings', () => { }); test('Deleting course configuration', async () => { + const user = userEvent.setup(); jest.spyOn(API, 'deleteXpertSettings'); await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - fireEvent.click(container.querySelector('#enable-xpert-unit-summary-toggle')); - fireEvent.click(getByText(container, 'Save')); - await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - expect(API.deleteXpertSettings).toBeCalled(); + await user.click(container.querySelector('#enable-xpert-unit-summary-toggle')); + await user.click(screen.getByText('Save')); + await waitFor(() => expect(API.deleteXpertSettings).toBeCalled()); }); }); describe('resetting course units', () => { test('reset all units to be enabled', async () => { + const user = userEvent.setup(); axiosMock.onGet(API.getXpertSettingsUrl(courseId)) .reply( 200, @@ -303,11 +210,14 @@ describe('XpertUnitSummarySettings', () => { jest.spyOn(API, 'postXpertSettings'); await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - fireEvent.click(queryByTestId(container, 'reset-units')); - expect(API.postXpertSettings).toBeCalledWith(courseId, { reset: true, enabled: true }); + await user.click(screen.getByTestId('reset-units')); + await waitFor(() => ( + expect(API.postXpertSettings).toBeCalledWith(courseId, { reset: true, enabled: true }) + )); }); test('reset all units to be disabled', async () => { + const user = userEvent.setup(); axiosMock.onGet(API.getXpertSettingsUrl(courseId)) .reply( 200, @@ -331,8 +241,10 @@ describe('XpertUnitSummarySettings', () => { jest.spyOn(API, 'postXpertSettings'); await waitFor(() => expect(container.querySelector('#enable-xpert-unit-summary-toggle')).toBeTruthy()); - fireEvent.click(queryByTestId(container, 'reset-units')); - expect(API.postXpertSettings).toBeCalledWith(courseId, { reset: true, enabled: false }); + await user.click(screen.getByTestId('reset-units')); + await waitFor(() => ( + expect(API.postXpertSettings).toBeCalledWith(courseId, { reset: true, enabled: false }) + )); }); }); }); diff --git a/plugins/course-apps/xpert_unit_summary/data/api.js b/plugins/course-apps/xpert_unit_summary/data/api.js deleted file mode 100644 index 32233ac2b8..0000000000 --- a/plugins/course-apps/xpert_unit_summary/data/api.js +++ /dev/null @@ -1,41 +0,0 @@ -import { getConfig } from '@edx/frontend-platform'; -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; - -export function getXpertSettingsUrl(courseId) { - return `${getConfig().STUDIO_BASE_URL}/ai_aside/v1/${courseId}`; -} - -export function getXpertConfigurationStatusUrl(courseId) { - return `${getConfig().STUDIO_BASE_URL}/ai_aside/v1/${courseId}/configurable`; -} - -export async function getXpertSettings(courseId) { - const { data } = await getAuthenticatedHttpClient() - .get(getXpertSettingsUrl(courseId)); - - return data; -} - -export async function postXpertSettings(courseId, state) { - const { data } = await getAuthenticatedHttpClient() - .post(getXpertSettingsUrl(courseId), { - enabled: state.enabled, - reset: state.reset, - }); - - return data; -} - -export async function getXpertPluginConfigurable(courseId) { - const { data } = await getAuthenticatedHttpClient() - .get(getXpertConfigurationStatusUrl(courseId)); - - return data; -} - -export async function deleteXpertSettings(courseId) { - const { data } = await getAuthenticatedHttpClient() - .delete(getXpertSettingsUrl(courseId)); - - return data; -} diff --git a/plugins/course-apps/xpert_unit_summary/data/api.ts b/plugins/course-apps/xpert_unit_summary/data/api.ts new file mode 100644 index 0000000000..948fa2e7ec --- /dev/null +++ b/plugins/course-apps/xpert_unit_summary/data/api.ts @@ -0,0 +1,50 @@ +import { getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; + +export function getXpertSettingsUrl(courseId: string) { + return `${getConfig().STUDIO_BASE_URL}/ai_aside/v1/${courseId}`; +} + +export function getXpertConfigurationStatusUrl(courseId: string) { + return `${getConfig().STUDIO_BASE_URL}/ai_aside/v1/${courseId}/configurable`; +} + +export interface XpertSettings { + enabled?: boolean; + success?: boolean; +} + +export interface XpertSettingsState { + enabled: boolean; + reset?: boolean; +} + +export interface XpertResponse { + response: { + success: boolean; + }; +} + +export async function getXpertSettings(courseId: string): Promise { + const { data } = await getAuthenticatedHttpClient() + .get(getXpertSettingsUrl(courseId)); + + return data.response; +} + +export async function postXpertSettings(courseId: string, state: XpertSettingsState): Promise { + const { data } = await getAuthenticatedHttpClient() + .post(getXpertSettingsUrl(courseId), { + enabled: state.enabled, + reset: state.reset || false, + }); + + return data; +} + +export async function deleteXpertSettings(courseId: string): Promise { + const { data } = await getAuthenticatedHttpClient() + .delete(getXpertSettingsUrl(courseId)); + + return data; +} diff --git a/plugins/course-apps/xpert_unit_summary/data/apiHooks.ts b/plugins/course-apps/xpert_unit_summary/data/apiHooks.ts new file mode 100644 index 0000000000..8ff614dfde --- /dev/null +++ b/plugins/course-apps/xpert_unit_summary/data/apiHooks.ts @@ -0,0 +1,59 @@ +import { + useMutation, + useQuery, + useQueryClient, +} from '@tanstack/react-query'; +import { AxiosError } from 'axios'; + +import { + deleteXpertSettings, + getXpertSettings, + postXpertSettings, + XpertSettings, +} from './api'; + +export const xpertUnitSummaryQueryKeys = { + all: (courseId: string) => ['xpertUnitSummary', courseId], + settings: (courseId: string) => [...xpertUnitSummaryQueryKeys.all(courseId), 'settings'], + configurable: (courseId: string) => [...xpertUnitSummaryQueryKeys.all(courseId), 'configurable'], +}; + +/** + * Fetch the current Xpert unit summary settings for this course. + */ +export const useXpertSettings = (courseId: string) => ( + useQuery({ + queryKey: xpertUnitSummaryQueryKeys.settings(courseId), + // A course that hasn't been configured yet returns a 404 here, which is expected + // (not a real failure), so it's treated the same as "no settings" rather than an error. + queryFn: async () => { + try { + return await getXpertSettings(courseId); + } catch { + return { enabled: undefined }; + } + }, + }) +); + +/** + * Update the Xpert unit summary settings for this course. + */ +export const useUpdateXpertSettings = (courseId: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (state: { enabled: boolean; reset?: boolean }) => postXpertSettings(courseId, state), + onSuccess: () => queryClient.invalidateQueries({ queryKey: xpertUnitSummaryQueryKeys.settings(courseId) }), + }); +}; + +/** + * Delete (disable) the Xpert unit summary settings for this course. + */ +export const useDeleteXpertSettings = (courseId: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: () => deleteXpertSettings(courseId), + onSuccess: () => queryClient.invalidateQueries({ queryKey: xpertUnitSummaryQueryKeys.settings(courseId) }), + }); +}; diff --git a/plugins/course-apps/xpert_unit_summary/data/thunks.js b/plugins/course-apps/xpert_unit_summary/data/thunks.js deleted file mode 100644 index ec0bd91ff5..0000000000 --- a/plugins/course-apps/xpert_unit_summary/data/thunks.js +++ /dev/null @@ -1,122 +0,0 @@ -import { - updateSavingStatus, - updateLoadingStatus, - updateResetStatus, -} from 'CourseAuthoring/pages-and-resources/data/slice'; -import { RequestStatus } from 'CourseAuthoring/data/constants'; -import { addModel, updateModel } from 'CourseAuthoring/generic/model-store'; - -import { - getXpertSettings, - postXpertSettings, - getXpertPluginConfigurable, - deleteXpertSettings, -} from './api'; - -export function updateXpertSettings(courseId, state) { - return async (dispatch) => { - dispatch(updateSavingStatus({ status: RequestStatus.IN_PROGRESS })); - try { - const { response } = await postXpertSettings(courseId, state); - const { success } = response; - if (success) { - dispatch( - updateModel({ modelType: 'XpertSettings', model: { id: 'xpert-unit-summary', enabled: state.enabled } }), - ); - dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - return true; - } - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - return false; - } catch { - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - return false; - } - }; -} - -export function fetchXpertPluginConfigurable(courseId) { - return async (dispatch) => { - let enabled; - dispatch(updateLoadingStatus({ status: RequestStatus.PENDING })); - try { - const { response } = await getXpertPluginConfigurable(courseId); - enabled = response?.enabled; - } catch { - enabled = undefined; - } - - dispatch(addModel({ - modelType: 'XpertSettings.enabled', - model: { - id: 'xpert-unit-summary', - enabled, - }, - })); - }; -} - -export function fetchXpertSettings(courseId) { - return async (dispatch) => { - let enabled; - dispatch(updateLoadingStatus({ status: RequestStatus.PENDING })); - - try { - const { response } = await getXpertSettings(courseId); - enabled = response?.enabled; - } catch { - enabled = undefined; - } - - dispatch(addModel({ - modelType: 'XpertSettings', - model: { - id: 'xpert-unit-summary', - enabled, - }, - })); - - dispatch(updateLoadingStatus({ status: RequestStatus.SUCCESSFUL })); - }; -} - -export function removeXpertSettings(courseId) { - return async (dispatch) => { - dispatch(updateSavingStatus({ status: RequestStatus.PENDING })); - - try { - const { response } = await deleteXpertSettings(courseId); - const { success } = response; - if (success) { - const model = { id: 'xpert-unit-summary', enabled: undefined }; - dispatch(updateModel({ modelType: 'XpertSettings', model })); - dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - return true; - } - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - return false; - } catch { - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - return false; - } - }; -} - -export function resetXpertSettings(courseId, state) { - return async (dispatch) => { - dispatch(updateResetStatus({ status: RequestStatus.PENDING })); - try { - const { response } = await postXpertSettings(courseId, state); - const { success } = response; - if (success) { - dispatch(updateResetStatus({ status: RequestStatus.SUCCESSFUL })); - return true; - } - dispatch(updateResetStatus({ status: RequestStatus.FAILED })); - return false; - } catch { - dispatch(updateResetStatus({ status: RequestStatus.FAILED })); - return false; - } - }; -} diff --git a/plugins/course-apps/xpert_unit_summary/settings-modal/SettingsModal.jsx b/plugins/course-apps/xpert_unit_summary/settings-modal/SettingsModal.jsx index e06846dd46..39dfaf7848 100644 --- a/plugins/course-apps/xpert_unit_summary/settings-modal/SettingsModal.jsx +++ b/plugins/course-apps/xpert_unit_summary/settings-modal/SettingsModal.jsx @@ -27,27 +27,22 @@ import React, { useRef, useState, } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; import * as Yup from 'yup'; -import { RequestStatus } from 'CourseAuthoring/data/constants'; import ConnectionErrorAlert from 'CourseAuthoring/generic/ConnectionErrorAlert'; import FormSwitchGroup from 'CourseAuthoring/generic/FormSwitchGroup'; import Loading from 'CourseAuthoring/generic/Loading'; -import { useModel } from 'CourseAuthoring/generic/model-store'; import PermissionDeniedAlert from 'CourseAuthoring/generic/PermissionDeniedAlert'; import { useIsMobile } from 'CourseAuthoring/utils'; -import { getLoadingStatus, getSavingStatus, getResetStatus } from 'CourseAuthoring/pages-and-resources/data/selectors'; -import { updateSavingStatus, updateResetStatus } from 'CourseAuthoring/pages-and-resources/data/slice'; import AppConfigFormDivider from 'CourseAuthoring/pages-and-resources/discussions/app-config-form/apps/shared/AppConfigFormDivider'; import { PagesAndResourcesContext } from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; -import { updateXpertSettings, resetXpertSettings, removeXpertSettings } from '../data/thunks'; import messages from './messages'; import appInfo from '../appInfo'; import ResetIcon from './ResetIcon'; import './SettingsModal.scss'; +import { useDeleteXpertSettings, useUpdateXpertSettings, useXpertSettings } from '../data/apiHooks'; const AppSettingsForm = ({ formikProps, @@ -136,30 +131,27 @@ const ResetUnitsButton = ({ visible, }) => { const intl = useIntl(); - const resetStatusRequestStatus = useSelector(getResetStatus); - const dispatch = useDispatch(); + const updateSettingsMutation = useUpdateXpertSettings(courseId); useEffect(() => { - if (resetStatusRequestStatus === RequestStatus.SUCCESSFUL) { + if (updateSettingsMutation.isSuccess) { setTimeout(() => { - dispatch(updateResetStatus({ status: '' })); + updateSettingsMutation.reset(); }, 2000); } - }, [resetStatusRequestStatus]); + }, [updateSettingsMutation]); const handleResetUnits = () => { - dispatch(resetXpertSettings(courseId, { enabled: checked === 'true', reset: true })); + updateSettingsMutation.mutate({ enabled: checked === 'true', reset: true }); }; const getResetButtonState = () => { - switch (resetStatusRequestStatus) { - case RequestStatus.PENDING: - return 'pending'; - case RequestStatus.SUCCESSFUL: - return 'finish'; - default: - return 'default'; + if (updateSettingsMutation.isPending) { + return 'pending'; + } else if (updateSettingsMutation.isSuccess) { + return 'finish'; } + return 'default'; }; if (!visible) { return null; } @@ -229,50 +221,62 @@ const SettingsModal = ({ }) => { const intl = useIntl(); const { courseId } = useContext(PagesAndResourcesContext); - const loadingStatus = useSelector(getLoadingStatus); - const updateSettingsRequestStatus = useSelector(getSavingStatus); const alertRef = useRef(null); - const [saveError, setSaveError] = useState(false); - const dispatch = useDispatch(); - const submitButtonState = updateSettingsRequestStatus === RequestStatus.IN_PROGRESS ? 'pending' : 'default'; + const [formIsError, setFormIsError] = useState(false); const isMobile = useIsMobile(); const modalVariant = isMobile ? 'dark' : 'default'; - const xpertSettings = useModel('XpertSettings', appId); + const { + data: xpertSettings, + isPending: xpertSettingsIsPending, + isError: xpertSettingsIsFailed, + isSuccess: xpertSettingsIsSuccess, + failureReason: xpertSettingsError, + } = useXpertSettings(courseId); + + const updateSettingsMutation = useUpdateXpertSettings(courseId); + const deleteSettingsMutation = useDeleteXpertSettings(courseId); + + const saveInProgress = updateSettingsMutation.isPending || deleteSettingsMutation.isPending; + const saveIsSuccess = updateSettingsMutation.isSuccess || deleteSettingsMutation.isSuccess; + const saveIsError = updateSettingsMutation.isError || deleteSettingsMutation.isError || formIsError; + + const submitButtonState = saveInProgress ? 'pending' : 'default'; useEffect(() => { - if (updateSettingsRequestStatus === RequestStatus.SUCCESSFUL) { - dispatch(updateSavingStatus({ status: '' })); + if (saveIsSuccess) { + updateSettingsMutation.reset(); + deleteSettingsMutation.reset(); onClose(); + } else if (saveIsError) { + alertRef?.current.scrollIntoView(); } - }, [updateSettingsRequestStatus]); + }, [ + saveIsSuccess, + saveIsError, + updateSettingsMutation, + deleteSettingsMutation + ]); const handleFormSubmit = async ({ enabled, checked, ...rest }) => { - let success; const values = { ...rest, enabled: enabled ? checked === 'true' : undefined }; - + let success = false; if (enabled) { - // oxlint-disable-next-line @typescript-eslint/await-thenable - this dispatch() IS returning a promise. - success = await dispatch(updateXpertSettings(courseId, values)); + await updateSettingsMutation.mutateAsync(values); + success = updateSettingsMutation.isSuccess; } else { - // oxlint-disable-next-line @typescript-eslint/await-thenable - this dispatch() IS returning a promise. - success = await dispatch(removeXpertSettings(courseId)); + await deleteSettingsMutation.mutateAsync(); + success = updateSettingsMutation.isSuccess; } - - if (onSettingsSave) { - success = success && await onSettingsSave(values); + if (success && onSettingsSave) { + await onSettingsSave(values); } - setSaveError(!success); - !success && alertRef?.current.scrollIntoView(); // eslint-disable-line @typescript-eslint/no-unused-expressions }; const handleFormikSubmit = ({ handleSubmit, errors }) => async (event) => { // If submitting the form with errors, show the alert and scroll to it. await handleSubmit(event); - if (Object.keys(errors).length > 0) { - setSaveError(true); - alertRef?.current.scrollIntoView?.(); // eslint-disable-line no-unused-expressions - } + setFormIsError(Object.keys(errors).length > 0); }; const learnMoreLink = appInfo.documentationLinks?.learnMoreConfiguration && ( @@ -301,7 +305,7 @@ const SettingsModal = ({
); - if (loadingStatus === RequestStatus.SUCCESSFUL) { + if (xpertSettingsIsSuccess) { return ( } > - {saveError && ( + {saveIsError && ( {formikProps.errors.enabled?.title || intl.formatMessage(messages.errorSavingTitle)} @@ -428,9 +432,9 @@ const SettingsModal = ({ isMobile={isMobile} isFullscreenOnMobile > - {loadingStatus === RequestStatus.IN_PROGRESS && } - {loadingStatus === RequestStatus.FAILED && } - {loadingStatus === RequestStatus.DENIED && } + {xpertSettingsIsPending && } + {xpertSettingsIsFailed && } + {xpertSettingsError?.response?.status === 403 && } ); }; diff --git a/src/CourseAuthoringContext.tsx b/src/CourseAuthoringContext.tsx index a78fbed0d1..9983c6b4ef 100644 --- a/src/CourseAuthoringContext.tsx +++ b/src/CourseAuthoringContext.tsx @@ -95,9 +95,12 @@ export const CourseAuthoringProvider = ({ } } - courseApps?.sort((firstEl, secondEl) => ( + // courseApps is the array reference held by the React Query cache; sort a copy + // so we don't mutate it during render (StrictMode double-renders can otherwise + // produce inconsistent results between the two passes). + const sortedCourseApps = courseApps ? [...courseApps].sort((firstEl, secondEl) => ( COURSE_APPS_ORDER.indexOf(firstEl.id) - COURSE_APPS_ORDER.indexOf(secondEl.id) - )); + )) : courseApps; /** * Open the unit page for a given locator. @@ -117,7 +120,7 @@ export const CourseAuthoringProvider = ({ openUnlinkModal, closeUnlinkModal, currentUnlinkModalData, - courseApps: courseApps || [], + courseApps: sortedCourseApps || [], courseAppsStatus, }), [ courseId, @@ -130,7 +133,7 @@ export const CourseAuthoringProvider = ({ openUnlinkModal, closeUnlinkModal, currentUnlinkModalData, - courseApps, + sortedCourseApps, courseAppsStatus, ]); diff --git a/src/pages-and-resources/PagesAndResources.tsx b/src/pages-and-resources/PagesAndResources.tsx index fccd50c24e..acb4158284 100644 --- a/src/pages-and-resources/PagesAndResources.tsx +++ b/src/pages-and-resources/PagesAndResources.tsx @@ -40,16 +40,12 @@ const PagesAndResources = () => { const redirectUrl = `/course/${courseId}/pages-and-resources`; // We want the Xpert learning assistant and unit summaries to appear in the "Content Permissions" section instead, - // so we remove them from pages and add them to contentPermissionsPages. - const contentPermissionsPages: any[] = []; - - ['xpert_unit_summary', 'learning_assistant'].forEach(separateAppId => { - const index = courseApps.findIndex(app => app.id === separateAppId); - if (index !== -1) { - const [page] = courseApps.splice(index, 1); - contentPermissionsPages.push(page); - } - }); + // so we split them out of the regular pages list into contentPermissionsPages. + // courseApps is the array reference held by the React Query cache, so it must not be mutated here + // (e.g. via splice/sort) or StrictMode's double-render will silently drop entries from the UI. + const separateAppIds = ['xpert_unit_summary', 'learning_assistant']; + const contentPermissionsPages = courseApps.filter(app => separateAppIds.includes(app.id)); + const regularPages = courseApps.filter(app => !separateAppIds.includes(app.id)); if (courseAppsStatus === RequestStatus.PENDING || isLoadingUserPermissions) { // eslint-disable-next-line react/jsx-no-useless-fragment @@ -118,7 +114,7 @@ const PagesAndResources = () => { } courseId={courseId} /> From 06c182198d6ca7524743ea93e6d52c0f41ab63d4 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Sun, 30 Aug 2026 16:17:15 -0500 Subject: [PATCH 03/10] feat: Migrating learning assistant and proctoring --- .../learning_assistant/Settings.jsx | 5 +- .../learning_assistant/Settings.test.jsx | 61 +++++----- .../ora_settings/Settings.test.tsx | 11 +- plugins/course-apps/ora_settings/Settings.tsx | 14 +-- plugins/course-apps/proctoring/Settings.jsx | 6 +- .../course-apps/proctoring/Settings.test.jsx | 33 ++++-- plugins/course-apps/progress/Settings.tsx | 4 +- plugins/course-apps/teams/Settings.tsx | 2 +- plugins/course-apps/wiki/Settings.tsx | 2 +- .../xpert_unit_summary/Settings.jsx | 1 - .../xpert_unit_summary/data/apiHooks.ts | 2 +- .../settings-modal/SettingsModal.jsx | 4 +- src/CourseAuthoringContext.tsx | 8 +- src/data/api.ts | 2 +- .../app-settings-modal/AppSettingsModal.tsx | 5 +- src/pages-and-resources/data/selectors.js | 7 -- src/pages-and-resources/data/slice.js | 53 --------- src/pages-and-resources/data/thunks.js | 106 ------------------ src/store.ts | 3 - src/utils.tsx | 2 +- 20 files changed, 92 insertions(+), 239 deletions(-) delete mode 100644 src/pages-and-resources/data/selectors.js delete mode 100644 src/pages-and-resources/data/slice.js delete mode 100644 src/pages-and-resources/data/thunks.js diff --git a/plugins/course-apps/learning_assistant/Settings.jsx b/plugins/course-apps/learning_assistant/Settings.jsx index 6aba6ab2da..35a078d2ce 100644 --- a/plugins/course-apps/learning_assistant/Settings.jsx +++ b/plugins/course-apps/learning_assistant/Settings.jsx @@ -5,13 +5,14 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { Hyperlink } from '@openedx/paragon'; import AppSettingsModal from 'CourseAuthoring/pages-and-resources/app-settings-modal/AppSettingsModal'; -import { useModel } from 'CourseAuthoring/generic/model-store'; +import { useCourseAuthoringContext } from 'CourseAuthoring/CourseAuthoringContext'; import messages from './messages'; const LearningAssistantSettings = ({ onClose }) => { + const { courseApps } = useCourseAuthoringContext(); const appId = 'learning_assistant'; - const appInfo = useModel('courseApps', appId); + const appInfo = courseApps.find((app) => app.id === appId); const intl = useIntl(); // We need to render more than one link, so we use the bodyChildren prop. diff --git a/plugins/course-apps/learning_assistant/Settings.test.jsx b/plugins/course-apps/learning_assistant/Settings.test.jsx index 87feaf1f1d..1c9a1b34dd 100644 --- a/plugins/course-apps/learning_assistant/Settings.test.jsx +++ b/plugins/course-apps/learning_assistant/Settings.test.jsx @@ -1,40 +1,51 @@ -import React from 'react'; import { screen, waitFor } from '@testing-library/react'; -import { RequestStatus } from 'CourseAuthoring/data/constants'; +import { CourseAuthoringProvider } from 'CourseAuthoring/CourseAuthoringContext'; +import PagesAndResourcesProvider from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; +import { getCourseAppsApiUrl, getCourseDetailsUrl } from 'CourseAuthoring/data/api'; import { initializeMocks, render } from 'CourseAuthoring/testUtils'; import LearningAssistantSettings from './Settings'; const onClose = () => {}; +const courseId = 'course-v1:edX+TestX+Test_Course'; + +const renderComponent = () => + render( + + + + + , + ); describe('Learning Assistant Settings', () => { it('renders', async () => { - const initialState = { - models: { - courseApps: { - learning_assistant: { - id: 'learning_assistant', - enabled: true, - name: 'Learning Assistant', - description: 'Learning Assistant description', - allowedOperations: { - configure: false, - enable: true, - }, - documentationLinks: { - learnMoreOpenaiDataPrivacy: 'www.example.com/learn-more-data-privacy', - learnMoreOpenai: 'www.example.com/learn-more', - }, - }, + const { axiosMock } = initializeMocks(); + + axiosMock.onGet(getCourseDetailsUrl(courseId, 'abc123')).reply(200, { + courseId, + name: 'Course Test', + start: Date(), + }); + + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ + { + id: 'learning_assistant', + name: 'Learning Assistant', + description: 'Learning Assistant description', + enabled: true, + documentation_links: { + learn_more_openai_data_privacy: 'www.example.com/learn-more-data-privacy', + learn_more_openai: 'www.example.com/learn-more', + }, + allowed_operations: { + configure: false, + enable: true, }, }, - pagesAndResources: { - loadingStatus: RequestStatus.SUCCESSFUL, - }, - }; + ]); - initializeMocks({ initialState }); - render(); + renderComponent(); const toggleDescription = 'Reinforce learning concepts by sharing text-based course content ' + 'with OpenAI (via API) to power an in-course Learning Assistant. Learners can leave feedback about the quality ' diff --git a/plugins/course-apps/ora_settings/Settings.test.tsx b/plugins/course-apps/ora_settings/Settings.test.tsx index 76e73e9f19..e150873cb9 100644 --- a/plugins/course-apps/ora_settings/Settings.test.tsx +++ b/plugins/course-apps/ora_settings/Settings.test.tsx @@ -19,11 +19,12 @@ let axiosMock; // @ts-ignore ReactDOM.createPortal = jest.fn(node => node); -const renderComponent = () => render( - - - , -); +const renderComponent = () => + render( + + + , + ); const mockCourseApps = ({ apiStatus = 200, enabled = true } = {}) => { axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply( diff --git a/plugins/course-apps/ora_settings/Settings.tsx b/plugins/course-apps/ora_settings/Settings.tsx index 50891d2612..ca644e7034 100644 --- a/plugins/course-apps/ora_settings/Settings.tsx +++ b/plugins/course-apps/ora_settings/Settings.tsx @@ -24,14 +24,13 @@ import { useCourseAuthoringContext } from 'CourseAuthoring/CourseAuthoringContex import messages from './messages'; -const ORASettings = ({ onClose }: { onClose : () => void}) => { +const ORASettings = ({ onClose }: { onClose: () => void; }) => { const { formatMessage } = useIntl(); const alertRef = useRef(null); const { courseId, courseApps, courseAppsStatus, - } = useCourseAuthoringContext(); const isMobile = useIsMobile(); @@ -51,14 +50,15 @@ const ORASettings = ({ onClose }: { onClose : () => void}) => { }, [enableFlexiblePeerGrade]); const submitButtonState = updateCourseAdvancedSettingsMutation.isPending ? 'pending' : 'default'; - const handleSettingsSave = (values) => updateCourseAdvancedSettingsMutation.mutate({ - setting: settingName, - value: values.enableFlexiblePeerGrade, - }); + const handleSettingsSave = (values) => + updateCourseAdvancedSettingsMutation.mutate({ + setting: settingName, + value: values.enableFlexiblePeerGrade, + }); const handleSubmit = async (event) => { event.preventDefault(); - await handleSettingsSave(formValues); + handleSettingsSave(formValues); }; const handleChange = (e) => { diff --git a/plugins/course-apps/proctoring/Settings.jsx b/plugins/course-apps/proctoring/Settings.jsx index 3a8bcda11c..f138d19f6c 100644 --- a/plugins/course-apps/proctoring/Settings.jsx +++ b/plugins/course-apps/proctoring/Settings.jsx @@ -27,7 +27,6 @@ import StudioApiService from 'CourseAuthoring/data/services/StudioApiService'; import Loading from 'CourseAuthoring/generic/Loading'; import ConnectionErrorAlert from 'CourseAuthoring/generic/ConnectionErrorAlert'; import FormSwitchGroup from 'CourseAuthoring/generic/FormSwitchGroup'; -import { useModel } from 'CourseAuthoring/generic/model-store'; import PermissionDeniedAlert from 'CourseAuthoring/generic/PermissionDeniedAlert'; import { useIsMobile } from 'CourseAuthoring/utils'; import { PagesAndResourcesContext } from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; @@ -75,9 +74,10 @@ const ProctoringSettings = ({ onClose }) => { } const { courseId } = useContext(PagesAndResourcesContext); - const { courseDetails } = useCourseAuthoringContext(); + const { courseDetails, courseApps } = useCourseAuthoringContext(); const org = courseDetails?.org; - const appInfo = useModel('courseApps', 'proctoring'); + const appId = 'proctoring'; + const appInfo = courseApps.find((app) => app.id === appId); const alertRef = React.createRef(); const saveStatusAlertRef = React.createRef(); const proctoringEscalationEmailInputRef = useRef(null); diff --git a/plugins/course-apps/proctoring/Settings.test.jsx b/plugins/course-apps/proctoring/Settings.test.jsx index 6a07c7143c..9d4507693b 100644 --- a/plugins/course-apps/proctoring/Settings.test.jsx +++ b/plugins/course-apps/proctoring/Settings.test.jsx @@ -14,7 +14,7 @@ import StudioApiService from 'CourseAuthoring/data/services/StudioApiService'; import ExamsApiService from 'CourseAuthoring/data/services/ExamsApiService'; import PagesAndResourcesProvider from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; import { CourseAuthoringProvider } from 'CourseAuthoring/CourseAuthoringContext'; -import { getCourseDetailsUrl } from 'CourseAuthoring/data/api'; +import { getCourseAppsApiUrl, getCourseDetailsUrl } from 'CourseAuthoring/data/api'; import ProctoredExamSettings from './Settings'; const courseId = 'course-v1%3AedX%2BDemoX%2BDemo_Course'; @@ -47,16 +47,7 @@ describe('ProctoredExamSettings', () => { administrator: isAdmin, roles: [], }; - const mocks = initializeMocks({ - user, - initialState: { - models: { - courseApps: { - proctoring: {}, - }, - }, - }, - }); + const mocks = initializeMocks({ user }); axiosMock = mocks.axiosMock; axiosMock @@ -67,6 +58,23 @@ describe('ProctoredExamSettings', () => { start: Date(), ...(org ? { org } : {}), }); + axiosMock + .onGet(`${getCourseAppsApiUrl()}/${courseId}`) + .reply(200, [ + { + id: 'proctoring', + name: 'Proctoring', + description: 'Maintain exam integrity by enabling a proctoring solution for your course', + enabled: true, + documentation_links: { + learn_more_configuration: 'http://example.com/learn-more-proctoring', + }, + allowed_operations: { + enable: false, + configure: true, + }, + }, + ]); axiosMock.onGet(`${ExamsApiService.getExamsBaseUrl()}/api/v1/providers`) .reply(200, [{ name: 'test_lti', verbose_name: 'LTI Provider' }]); if (org) { @@ -472,7 +480,8 @@ describe('ProctoredExamSettings', () => { }); // (1) for studio settings // (2) for course details - expect(axiosMock.history.get.length).toBe(2); + // (3) for the course apps list (used to read this app's documentation links) + expect(axiosMock.history.get.length).toBe(3); expect(axiosMock.history.get[0].url.includes('proctored_exam_settings')).toEqual(true); }); diff --git a/plugins/course-apps/progress/Settings.tsx b/plugins/course-apps/progress/Settings.tsx index 672e479701..629b147579 100644 --- a/plugins/course-apps/progress/Settings.tsx +++ b/plugins/course-apps/progress/Settings.tsx @@ -9,10 +9,10 @@ import { useUpdateCourseAdvancedSettings } from 'CourseAuthoring/data/apiHooks'; import { useCourseAuthoringContext } from 'CourseAuthoring/CourseAuthoringContext'; import messages from './messages'; -const ProgressSettings = ({ onClose }: { onClose: () => void }) => { +const ProgressSettings = ({ onClose }: { onClose: () => void; }) => { const intl = useIntl(); const { courseId } = useCourseAuthoringContext(); - const settingsName = 'disableProgressGraph' + const settingsName = 'disableProgressGraph'; const disableProgressGraph = useAppSetting(settingsName); const updateCourseAdvancedSettingsMutation = useUpdateCourseAdvancedSettings(courseId); const showProgressGraphSetting = getConfig().ENABLE_PROGRESS_GRAPH_SETTINGS.toString().toLowerCase() === 'true'; diff --git a/plugins/course-apps/teams/Settings.tsx b/plugins/course-apps/teams/Settings.tsx index cdd50129fe..ea51edad9c 100644 --- a/plugins/course-apps/teams/Settings.tsx +++ b/plugins/course-apps/teams/Settings.tsx @@ -19,7 +19,7 @@ setupYupExtensions(); const TeamSettings = ({ onClose, -}: { onClose: () => void }) => { +}: { onClose: () => void; }) => { const intl = useIntl(); const { courseId } = useCourseAuthoringContext(); const settingName = 'teamsConfiguration'; diff --git a/plugins/course-apps/wiki/Settings.tsx b/plugins/course-apps/wiki/Settings.tsx index 00b3c6d14c..830310be58 100644 --- a/plugins/course-apps/wiki/Settings.tsx +++ b/plugins/course-apps/wiki/Settings.tsx @@ -8,7 +8,7 @@ import AppSettingsModal from 'CourseAuthoring/pages-and-resources/app-settings-m import { useUpdateCourseAdvancedSettings } from 'CourseAuthoring/data/apiHooks'; import messages from './messages'; -const WikiSettings = ({ onClose }: { onClose: () => void }) => { +const WikiSettings = ({ onClose }: { onClose: () => void; }) => { const intl = useIntl(); const settingName = 'allowPublicWikiAccess'; const enablePublicWiki = useAppSetting(settingName); diff --git a/plugins/course-apps/xpert_unit_summary/Settings.jsx b/plugins/course-apps/xpert_unit_summary/Settings.jsx index d687eb47ef..d8ba868fdf 100644 --- a/plugins/course-apps/xpert_unit_summary/Settings.jsx +++ b/plugins/course-apps/xpert_unit_summary/Settings.jsx @@ -6,7 +6,6 @@ import { useNavigate } from 'react-router-dom'; import SettingsModal from './settings-modal/SettingsModal'; import messages from './messages'; - const XpertUnitSummarySettings = () => { const intl = useIntl(); const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); diff --git a/plugins/course-apps/xpert_unit_summary/data/apiHooks.ts b/plugins/course-apps/xpert_unit_summary/data/apiHooks.ts index 8ff614dfde..e0abc11d2e 100644 --- a/plugins/course-apps/xpert_unit_summary/data/apiHooks.ts +++ b/plugins/course-apps/xpert_unit_summary/data/apiHooks.ts @@ -42,7 +42,7 @@ export const useXpertSettings = (courseId: string) => ( export const useUpdateXpertSettings = (courseId: string) => { const queryClient = useQueryClient(); return useMutation({ - mutationFn: (state: { enabled: boolean; reset?: boolean }) => postXpertSettings(courseId, state), + mutationFn: (state: { enabled: boolean; reset?: boolean; }) => postXpertSettings(courseId, state), onSuccess: () => queryClient.invalidateQueries({ queryKey: xpertUnitSummaryQueryKeys.settings(courseId) }), }); }; diff --git a/plugins/course-apps/xpert_unit_summary/settings-modal/SettingsModal.jsx b/plugins/course-apps/xpert_unit_summary/settings-modal/SettingsModal.jsx index 39dfaf7848..b493d5134c 100644 --- a/plugins/course-apps/xpert_unit_summary/settings-modal/SettingsModal.jsx +++ b/plugins/course-apps/xpert_unit_summary/settings-modal/SettingsModal.jsx @@ -142,7 +142,7 @@ const ResetUnitsButton = ({ }, [updateSettingsMutation]); const handleResetUnits = () => { - updateSettingsMutation.mutate({ enabled: checked === 'true', reset: true }); + updateSettingsMutation.mutate({ enabled: checked === 'true', reset: true }); }; const getResetButtonState = () => { @@ -255,7 +255,7 @@ const SettingsModal = ({ saveIsSuccess, saveIsError, updateSettingsMutation, - deleteSettingsMutation + deleteSettingsMutation, ]); const handleFormSubmit = async ({ enabled, checked, ...rest }) => { diff --git a/src/CourseAuthoringContext.tsx b/src/CourseAuthoringContext.tsx index 9983c6b4ef..bb8d6a0749 100644 --- a/src/CourseAuthoringContext.tsx +++ b/src/CourseAuthoringContext.tsx @@ -98,9 +98,11 @@ export const CourseAuthoringProvider = ({ // courseApps is the array reference held by the React Query cache; sort a copy // so we don't mutate it during render (StrictMode double-renders can otherwise // produce inconsistent results between the two passes). - const sortedCourseApps = courseApps ? [...courseApps].sort((firstEl, secondEl) => ( - COURSE_APPS_ORDER.indexOf(firstEl.id) - COURSE_APPS_ORDER.indexOf(secondEl.id) - )) : courseApps; + const sortedCourseApps = courseApps ? + [...courseApps].sort((firstEl, secondEl) => ( + COURSE_APPS_ORDER.indexOf(firstEl.id) - COURSE_APPS_ORDER.indexOf(secondEl.id) + )) : + courseApps; /** * Open the unit page for a given locator. diff --git a/src/data/api.ts b/src/data/api.ts index 25412738d9..5c61439421 100644 --- a/src/data/api.ts +++ b/src/data/api.ts @@ -319,7 +319,7 @@ export async function getCourseAdvancedSettings( filter_fields: settings.map(snakeCase).join(','), }, }); - + return camelCaseObject(data); } diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModal.tsx b/src/pages-and-resources/app-settings-modal/AppSettingsModal.tsx index e60b1a861a..46b83d86f2 100644 --- a/src/pages-and-resources/app-settings-modal/AppSettingsModal.tsx +++ b/src/pages-and-resources/app-settings-modal/AppSettingsModal.tsx @@ -113,10 +113,9 @@ const AppSettingsModal = ({ useEffect(() => { if (inError) { - alertRef?.current?.scrollIntoView() + alertRef?.current?.scrollIntoView(); } - }, [inError]) - + }, [inError]); const handleFormikSubmit = ({ handleSubmit, errors }) => async (event) => { // Clear any error left over from a previous failed attempt so a successful diff --git a/src/pages-and-resources/data/selectors.js b/src/pages-and-resources/data/selectors.js deleted file mode 100644 index b630a698e8..0000000000 --- a/src/pages-and-resources/data/selectors.js +++ /dev/null @@ -1,7 +0,0 @@ -export const getLoadingStatus = (state) => state.pagesAndResources.loadingStatus; -export const getSavingStatus = (state) => state.pagesAndResources.savingStatus; -export const getCourseAppsApiStatus = (state) => state.pagesAndResources.courseAppsApiStatus; -export const getCourseAppSettingValue = (setting) => (state) => ( - state.pagesAndResources.courseAppSettings[setting]?.value -); -export const getResetStatus = (state) => state.pagesAndResources.resetStatus; diff --git a/src/pages-and-resources/data/slice.js b/src/pages-and-resources/data/slice.js deleted file mode 100644 index 95379cd34a..0000000000 --- a/src/pages-and-resources/data/slice.js +++ /dev/null @@ -1,53 +0,0 @@ -/* eslint-disable no-param-reassign */ -import { createSlice } from '@reduxjs/toolkit'; - -import { RequestStatus } from '../../data/constants'; - -const slice = createSlice({ - name: 'pagesAndResources', - initialState: { - courseAppIds: [], - loadingStatus: RequestStatus.IN_PROGRESS, - savingStatus: '', - resetStatus: '', - courseAppsApiStatus: {}, - courseAppSettings: {}, - }, - reducers: { - fetchCourseAppsSuccess: (state, { payload }) => { - state.courseAppIds = payload.courseAppIds; - }, - updateLoadingStatus: (state, { payload }) => { - state.loadingStatus = payload.status; - }, - updateSavingStatus: (state, { payload }) => { - state.savingStatus = payload.status; - }, - updateResetStatus: (state, { payload }) => { - state.resetStatus = payload.status; - }, - updateCourseAppsApiStatus: (state, { payload }) => { - state.courseAppsApiStatus = payload.status; - }, - fetchCourseAppsSettingsSuccess: (state, { payload }) => { - Object.assign(state.courseAppSettings, payload); - }, - updateCourseAppsSettingsSuccess: (state, { payload }) => { - Object.assign(state.courseAppSettings, payload); - }, - }, -}); - -export const { - fetchCourseAppsSuccess, - updateLoadingStatus, - updateSavingStatus, - updateResetStatus, - updateCourseAppsApiStatus, - fetchCourseAppsSettingsSuccess, - updateCourseAppsSettingsSuccess, -} = slice.actions; - -export const { - reducer, -} = slice; diff --git a/src/pages-and-resources/data/thunks.js b/src/pages-and-resources/data/thunks.js deleted file mode 100644 index 9b77eb0e22..0000000000 --- a/src/pages-and-resources/data/thunks.js +++ /dev/null @@ -1,106 +0,0 @@ -import { RequestStatus } from '../../data/constants'; -import { addModels, updateModel } from '../../generic/model-store'; -import { - getCourseAdvancedSettings, - getCourseApps, - updateCourseAdvancedSettings, - updateCourseApp, -} from './api'; -import { - fetchCourseAppsSettingsSuccess, - fetchCourseAppsSuccess, - updateCourseAppsApiStatus, - updateCourseAppsSettingsSuccess, - updateLoadingStatus, - updateSavingStatus, -} from './slice'; - -const COURSE_APPS_ORDER = [ - 'progress', - 'discussion', - 'teams', - 'edxnotes', - 'wiki', - 'calculator', - 'proctoring', - 'live', - 'textbooks', - 'custom_pages', - 'ora_settings', -]; - -export function fetchCourseApps(courseId) { - return async (dispatch) => { - dispatch(updateLoadingStatus({ courseId, status: RequestStatus.IN_PROGRESS })); - - try { - const courseApps = await getCourseApps(courseId); - - courseApps.sort((firstEl, secondEl) => ( - COURSE_APPS_ORDER.indexOf(firstEl.id) - COURSE_APPS_ORDER.indexOf(secondEl.id) - )); - - dispatch(addModels({ modelType: 'courseApps', models: courseApps })); - dispatch(fetchCourseAppsSuccess({ - courseAppIds: courseApps.map(courseApp => courseApp.id), - })); - dispatch(updateLoadingStatus({ courseId, status: RequestStatus.SUCCESSFUL })); - } catch (error) { - if (error.response && error.response.status === 403) { - dispatch(updateCourseAppsApiStatus({ status: RequestStatus.DENIED })); - } - - dispatch(updateLoadingStatus({ courseId, status: RequestStatus.FAILED })); - } - }; -} - -export function updateAppStatus(courseId, appId, state) { - return async (dispatch) => { - dispatch(updateSavingStatus({ status: RequestStatus.IN_PROGRESS })); - - try { - await updateCourseApp(courseId, appId, state); - dispatch(updateModel({ modelType: 'courseApps', model: { id: appId, enabled: state } })); - dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - return true; - } catch { - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - return false; - } - }; -} - -export function fetchCourseAppSettings(courseId, settings) { - return async (dispatch) => { - dispatch(updateLoadingStatus({ status: RequestStatus.IN_PROGRESS })); - - try { - const settingValues = await getCourseAdvancedSettings(courseId, settings); - dispatch(fetchCourseAppsSettingsSuccess(settingValues)); - dispatch(updateLoadingStatus({ status: RequestStatus.SUCCESSFUL })); - } catch (error) { - if (error.response && error.response.status === 403) { - dispatch(updateLoadingStatus({ status: RequestStatus.DENIED })); - } else { - dispatch(updateLoadingStatus({ status: RequestStatus.FAILED })); - } - } - }; -} - -export function updateCourseAppSetting(courseId, setting, value) { - return async (dispatch) => { - dispatch(updateSavingStatus({ status: RequestStatus.IN_PROGRESS })); - - try { - const settingValues = await updateCourseAdvancedSettings(courseId, setting, value); - dispatch(updateCourseAppsSettingsSuccess(settingValues)); - dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - return true; - } catch { - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - return false; - } - }; -} diff --git a/src/store.ts b/src/store.ts index 466405716a..dcf1d4edf0 100644 --- a/src/store.ts +++ b/src/store.ts @@ -6,7 +6,6 @@ import { reducer as liveReducer } from '@openedx-plugins/course-app-live/data/sl import { reducer as modelsReducer } from './generic/model-store'; import { reducer as discussionsReducer } from './pages-and-resources/discussions/data/slice'; -import { reducer as pagesAndResourcesReducer } from './pages-and-resources/data/slice'; import { reducer as studioHomeReducer } from './studio-home/data/slice'; import { reducer as filesReducer } from './files-and-videos/files-page/data/slice'; import { reducer as courseOptimizerReducer } from './optimizer-page/data/slice'; @@ -24,7 +23,6 @@ type InferState = ReducerType extends Reducer ? T : never; export interface DeprecatedReduxState { discussions: Record; assets: Record; - pagesAndResources: Record; studioHome: InferState; models: Record; live: Record; @@ -40,7 +38,6 @@ export default function initializeStore(preloadedState: Partial Date: Sun, 30 Aug 2026 19:25:54 -0500 Subject: [PATCH 04/10] test: Fixing Tests --- .../PagesAndResources.test.tsx | 111 +++++------------- .../SettingsComponent.test.jsx | 67 ++++++----- 2 files changed, 67 insertions(+), 111 deletions(-) diff --git a/src/pages-and-resources/PagesAndResources.test.tsx b/src/pages-and-resources/PagesAndResources.test.tsx index 8c06c1a625..9e30b95b52 100644 --- a/src/pages-and-resources/PagesAndResources.test.tsx +++ b/src/pages-and-resources/PagesAndResources.test.tsx @@ -8,6 +8,7 @@ import { import { getConfig, setConfig } from '@edx/frontend-platform'; import { PLUGIN_OPERATIONS, DIRECT_PLUGIN } from '@openedx/frontend-plugin-framework'; import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; +import { getCourseAppsApiUrl } from '@src/data/api'; import { mockWaffleFlags } from '@src/data/apiHooks.mock'; import { useCourseUserPermissions } from '@src/authz/hooks'; import { PagesAndResources } from '.'; @@ -42,6 +43,8 @@ const renderComponent = () => ); describe('PagesAndResources', () => { + let axiosMock; + beforeEach(() => { jest.clearAllMocks(); setConfig({ @@ -64,6 +67,10 @@ describe('PagesAndResources', () => { canViewPagesAndResources: true, canManagePagesAndResources: true, } as ReturnType); + + ({ axiosMock } = initializeMocks()); + // Default: no course apps installed. Override per-test with axiosMock as needed. + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, []); }); // Helper to set up permission mocks @@ -78,16 +85,6 @@ describe('PagesAndResources', () => { }; it('doesn\'t show content permissions section if relevant apps are not enabled', async () => { - const initialState = { - models: { - courseApps: {}, - }, - pagesAndResources: { - courseAppIds: [], - }, - }; - - initializeMocks({ initialState }); renderComponent(); await waitFor(() => expect(screen.queryByRole('heading', { name: 'Content permissions' })).not.toBeInTheDocument()); @@ -96,28 +93,20 @@ describe('PagesAndResources', () => { }); it('show content permissions section if Learning Assistant app is enabled', async () => { - const initialState = { - models: { - courseApps: { - learning_assistant: { - id: 'learning_assistant', - enabled: true, - name: 'Learning Assistant', - description: 'Learning Assistant description', - allowedOperations: { - configure: false, - enable: true, - }, - documentationLinks: {}, - }, + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ + { + id: 'learning_assistant', + enabled: true, + name: 'Learning Assistant', + description: 'Learning Assistant description', + allowed_operations: { + configure: false, + enable: true, }, + documentation_links: {}, }, - pagesAndResources: { - courseAppIds: ['learning_assistant'], - }, - }; + ]); - initializeMocks({ initialState }); renderComponent(); await waitFor(() => expect(screen.getByRole('heading', { name: 'Content permissions' })).toBeInTheDocument()); @@ -127,30 +116,22 @@ describe('PagesAndResources', () => { }); it('show content permissions section if Xpert learning summaries app is enabled', async () => { - const initialState = { - models: { - courseApps: { - xpert_unit_summary: { - id: 'xpert_unit_summary', - enabled: false, - name: 'Xpert unit summaries', - description: 'Use generative AI to summarize course content and reinforce learning.', - allowedOperations: { - enable: true, - configure: true, - }, - documentationLinks: { - learnMoreConfiguration: 'https://openai.com/', - }, - }, + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ + { + id: 'xpert_unit_summary', + enabled: false, + name: 'Xpert unit summaries', + description: 'Use generative AI to summarize course content and reinforce learning.', + allowed_operations: { + enable: true, + configure: true, + }, + documentation_links: { + learn_more_configuration: 'https://openai.com/', }, }, - pagesAndResources: { - courseAppIds: ['xpert_unit_summary'], - }, - }; + ]); - initializeMocks({ initialState }); renderComponent(); await waitFor(() => expect(screen.getByRole('heading', { name: 'Content permissions' })).toBeInTheDocument()); @@ -163,16 +144,6 @@ describe('PagesAndResources', () => { 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()); @@ -181,16 +152,6 @@ describe('PagesAndResources', () => { 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()); @@ -199,16 +160,6 @@ describe('PagesAndResources', () => { 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/SettingsComponent.test.jsx b/src/pages-and-resources/SettingsComponent.test.jsx index 7e7bc42340..95eab62c84 100644 --- a/src/pages-and-resources/SettingsComponent.test.jsx +++ b/src/pages-and-resources/SettingsComponent.test.jsx @@ -1,14 +1,10 @@ -import React from 'react'; import { useParams, useLocation } from 'react-router-dom'; -import { render, screen, waitFor } from '@testing-library/react'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { AppProvider } from '@edx/frontend-platform/react'; -import { initializeMockApp } from '@edx/frontend-platform/testing'; +import userEvent from '@testing-library/user-event'; +import { CourseAuthoringProvider } from 'CourseAuthoring/CourseAuthoringContext'; import PagesAndResourcesProvider from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; -import initializeStore from 'CourseAuthoring/store'; -import { RequestStatus } from 'CourseAuthoring/data/constants'; -import userEvent from '@testing-library/user-event'; +import { getCourseAppsApiUrl, getCourseDetailsUrl } from 'CourseAuthoring/data/api'; +import { initializeMocks, render, screen, waitFor } from 'CourseAuthoring/testUtils'; import SettingsComponent from './SettingsComponent'; jest.mock('react-router-dom', () => ({ @@ -17,36 +13,45 @@ jest.mock('react-router-dom', () => ({ })); jest.mock('CourseAuthoring/utils', () => ({ - useAppSetting: () => [false, () => undefined], + ...jest.requireActual('CourseAuthoring/utils'), + // Real useAppSetting() returns a single value (not a [value, setter] tuple). + useAppSetting: () => false, useIsMobile: () => false, })); -let store; +const courseId = 'course-v1:foo+bar+baz'; -// eslint-disable-next-line react/prop-types const RequiredProviders = ({ children }) => ( - - - - {children} - - - + + + {children} + + ); describe('SettingsComponent', () => { - beforeEach(async () => { - initializeMockApp(); - store = initializeStore({ - models: { - courseApps: { - wiki: {}, + beforeEach(() => { + const { axiosMock } = initializeMocks(); + + axiosMock.onGet(getCourseDetailsUrl(courseId, 'abc123')).reply(200, { + courseId, + name: 'Course Test', + start: Date(), + }); + + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ + { + id: 'wiki', + name: 'Wiki', + description: 'Wiki description', + enabled: false, + documentation_links: {}, + allowed_operations: { + enable: true, + configure: true, }, }, - pagesAndResources: { - loadingStatus: RequestStatus.SUCCESSFUL, - }, - }); + ]); }); test('renders LazyLoadedComponent when provided with props', async () => { @@ -54,7 +59,7 @@ describe('SettingsComponent', () => { render( , - { wrapper: RequiredProviders }, + { extraWrapper: RequiredProviders }, ); await screen.findByText('Configure wiki'); @@ -77,7 +82,7 @@ describe('SettingsComponent', () => { , - { wrapper: RequiredProviders }, + { extraWrapper: RequiredProviders }, ); await screen.findByText('Configure wiki'); @@ -98,7 +103,7 @@ describe('SettingsComponent', () => { const rendered = render( , - { wrapper: RequiredProviders }, + { extraWrapper: RequiredProviders }, ); const errorMessage = 'An error occurred when loading the configuration UI'; From d25ca021c9b5ee8265f8c8adbb3c4e17ac2890e7 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Sun, 30 Aug 2026 19:37:21 -0500 Subject: [PATCH 05/10] fix: Nits on test --- plugins/course-apps/ora_settings/Settings.test.tsx | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/plugins/course-apps/ora_settings/Settings.test.tsx b/plugins/course-apps/ora_settings/Settings.test.tsx index e150873cb9..dde63009ec 100644 --- a/plugins/course-apps/ora_settings/Settings.test.tsx +++ b/plugins/course-apps/ora_settings/Settings.test.tsx @@ -1,6 +1,5 @@ import { screen, - waitFor, within, } from '@testing-library/react'; import ReactDOM from 'react-dom'; @@ -103,14 +102,10 @@ describe('ORASettings', () => { const checkbox = await screen.findByRole('checkbox', { name: /Flex Peer Grading/ }); expect(checkbox).toBeChecked(); - - await waitFor(() => { - const label = screen.getByText(messages.enableFlexPeerGradeLabel.defaultMessage); - const enableBadge = screen.getByTestId('enable-badge'); - - expect(label).toBeVisible(); - expect(enableBadge).toHaveTextContent('Enabled'); - }); + const label = await screen.findByText(messages.enableFlexPeerGradeLabel.defaultMessage); + const enableBadge = await screen.findByTestId('enable-badge'); + expect(label).toBeVisible(); + expect(enableBadge).toHaveTextContent('Enabled'); }); it('Displays title, helper text and hides badge when flexible peer grading button is disabled', async () => { From 33c6bc50cbc7ab28d6011b57b947a133cdfdf0c8 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Sun, 30 Aug 2026 20:33:58 -0500 Subject: [PATCH 06/10] fix: Broken tests --- plugins/course-apps/live/BbbSettings.test.jsx | 23 +++++++++++- plugins/course-apps/live/Settings.test.jsx | 37 +++++++++++++++---- .../course-apps/live/ZoomSettings.test.jsx | 23 +++++++++++- 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/plugins/course-apps/live/BbbSettings.test.jsx b/plugins/course-apps/live/BbbSettings.test.jsx index 56b18731f6..b417b7b1a5 100644 --- a/plugins/course-apps/live/BbbSettings.test.jsx +++ b/plugins/course-apps/live/BbbSettings.test.jsx @@ -14,11 +14,11 @@ import { executeThunk } from 'CourseAuthoring/utils'; import PagesAndResourcesProvider from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; import { CourseAuthoringProvider } from 'CourseAuthoring/CourseAuthoringContext'; +import { getCourseAppsApiUrl, getCourseDetailsUrl } from 'CourseAuthoring/data/api'; import LiveSettings from './Settings'; import { generateLiveConfigurationApiResponse, courseId, - initialState, configurationProviders, } from './factories/mockApiResponses'; import { fetchLiveConfiguration, fetchLiveProviders } from './data/thunks'; @@ -78,9 +78,28 @@ const mockStore = async ({ describe('BBB Settings', () => { beforeEach(async () => { - const mocks = initializeMocks({ initialState }); + const mocks = initializeMocks(); store = mocks.reduxStore; axiosMock = mocks.axiosMock; + + axiosMock.onGet(getCourseDetailsUrl(courseId, 'abc123')).reply(200, { + courseId, + name: 'Course Test', + start: Date(), + }); + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ + { + id: 'live', + enabled: true, + name: 'Live', + description: 'Enable in-platform video conferencing by configuring live', + allowed_operations: { + enable: true, + configure: true, + }, + documentation_links: {}, + }, + ]); }); test('Plan dropdown to be visible and enabled in UI', async () => { diff --git a/plugins/course-apps/live/Settings.test.jsx b/plugins/course-apps/live/Settings.test.jsx index c4c33dc963..7a60c869d5 100644 --- a/plugins/course-apps/live/Settings.test.jsx +++ b/plugins/course-apps/live/Settings.test.jsx @@ -16,11 +16,11 @@ import { executeThunk } from 'CourseAuthoring/utils'; import PagesAndResourcesProvider from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; import { CourseAuthoringProvider } from 'CourseAuthoring/CourseAuthoringContext'; +import { getCourseAppsApiUrl, getCourseDetailsUrl } from 'CourseAuthoring/data/api'; import LiveSettings from './Settings'; import { generateLiveConfigurationApiResponse, courseId, - initialState, configurationProviders, } from './factories/mockApiResponses'; @@ -35,14 +35,16 @@ const liveSettingsUrl = `/course/${courseId}/pages-and-resources/live/settings`; // Modal creates a portal. Overriding ReactDOM.createPortal allows portals to be tested in jest. ReactDOM.createPortal = jest.fn(node => node); +// jsdom doesn't implement scrollIntoView; AppSettingsModal calls it when showing a save error. +window.HTMLElement.prototype.scrollIntoView = jest.fn(); const renderComponent = () => { const wrapper = render( - - + + {}} /> - - , + + , { path: liveSettingsUrl, routerProps: { @@ -74,11 +76,28 @@ const mockStore = async ({ describe('LiveSettings', () => { beforeEach(async () => { - const mocks = initializeMocks({ - initialState, - }); + const mocks = initializeMocks(); store = mocks.reduxStore; axiosMock = mocks.axiosMock; + + axiosMock.onGet(getCourseDetailsUrl(courseId, 'abc123')).reply(200, { + courseId, + name: 'Course Test', + start: Date(), + }); + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ + { + id: 'live', + enabled: true, + name: 'Live', + description: 'Enable in-platform video conferencing by configuring live', + allowed_operations: { + enable: true, + configure: true, + }, + documentation_links: {}, + }, + ]); }); test('Live Configuration modal is visible', async () => { @@ -96,6 +115,7 @@ describe('LiveSettings', () => { await mockStore({ enabled: true }); renderComponent(); + await waitFor(() => expect(container.querySelector('label[for="enable-live-toggle"]')).not.toBeNull()); const label = container.querySelector('label[for="enable-live-toggle"]'); const helperText = container.querySelector('#enable-live-toggleHelpText'); const enableBadge = queryByTestId(container, 'enable-badge'); @@ -109,6 +129,7 @@ describe('LiveSettings', () => { await mockStore({ enabled: false, piiSharingAllowed: false }); renderComponent(); + await waitFor(() => expect(container.querySelector('label[for="enable-live-toggle"]')).not.toBeNull()); const label = container.querySelector('label[for="enable-live-toggle"]'); const helperText = container.querySelector('#enable-live-toggleHelpText'); diff --git a/plugins/course-apps/live/ZoomSettings.test.jsx b/plugins/course-apps/live/ZoomSettings.test.jsx index ef772cbd79..4882eaeae1 100644 --- a/plugins/course-apps/live/ZoomSettings.test.jsx +++ b/plugins/course-apps/live/ZoomSettings.test.jsx @@ -11,11 +11,11 @@ import ReactDOM from 'react-dom'; import { executeThunk } from 'CourseAuthoring/utils'; import PagesAndResourcesProvider from 'CourseAuthoring/pages-and-resources/PagesAndResourcesProvider'; import { CourseAuthoringProvider } from 'CourseAuthoring/CourseAuthoringContext'; +import { getCourseAppsApiUrl, getCourseDetailsUrl } from 'CourseAuthoring/data/api'; import LiveSettings from './Settings'; import { generateLiveConfigurationApiResponse, courseId, - initialState, configurationProviders, } from './factories/mockApiResponses'; @@ -69,9 +69,28 @@ const mockStore = async ({ describe('Zoom Settings', () => { beforeEach(async () => { - const mocks = initializeMocks({ initialState }); + const mocks = initializeMocks(); store = mocks.reduxStore; axiosMock = mocks.axiosMock; + + axiosMock.onGet(getCourseDetailsUrl(courseId, 'abc123')).reply(200, { + courseId, + name: 'Course Test', + start: Date(), + }); + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ + { + id: 'live', + enabled: true, + name: 'Live', + description: 'Enable in-platform video conferencing by configuring live', + allowed_operations: { + enable: true, + configure: true, + }, + documentation_links: {}, + }, + ]); }); test('LTI fields are visible when pii sharing is enabled', async () => { From 89465596760f9e1020d7fd67dbf89ebb39ce5379 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 2 Sep 2026 19:14:01 -0500 Subject: [PATCH 07/10] test: Fix coverage --- src/CourseAuthoringContext.test.tsx | 38 ++++++++++++++++++++++ src/data/apiHooks.test.tsx | 49 +++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 src/CourseAuthoringContext.test.tsx diff --git a/src/CourseAuthoringContext.test.tsx b/src/CourseAuthoringContext.test.tsx new file mode 100644 index 0000000000..ab04c34d6a --- /dev/null +++ b/src/CourseAuthoringContext.test.tsx @@ -0,0 +1,38 @@ +import { CourseAuthoringProvider, useCourseAuthoringContext } from './CourseAuthoringContext'; +import { getApiWaffleFlagsUrl, getCourseAppsApiUrl } from './data/api'; +import { initializeMocks, render, screen } from './testUtils'; + +const courseId = 'course-v1:edX+DemoX+Demo_Course'; + +const CourseAppsList = () => { + const { courseApps } = useCourseAuthoringContext(); + return ( +
    + {courseApps.map(app =>
  • {app.id}
    • )} +
+ ); +}; + +describe('CourseAuthoringProvider', () => { + it('sorts course apps according to COURSE_APPS_ORDER', async () => { + const { axiosMock } = initializeMocks(); + axiosMock.onGet(getApiWaffleFlagsUrl(courseId)).reply(200, {}); + axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ + { + id: 'wiki', name: 'Wiki', description: '', enabled: true, allowed_operations: { enable: true, configure: true }, + }, + { + id: 'discussion', name: 'Discussion', description: '', enabled: true, allowed_operations: { enable: true, configure: true }, + }, + ]); + + render( + + + , + ); + + const items = await screen.findAllByRole('listitem'); + expect(items.map(item => item.textContent)).toEqual(['discussion', 'wiki']); + }); +}); diff --git a/src/data/apiHooks.test.tsx b/src/data/apiHooks.test.tsx index 996a62daf1..d878dace6b 100644 --- a/src/data/apiHooks.test.tsx +++ b/src/data/apiHooks.test.tsx @@ -1,12 +1,16 @@ +import { renderHook } from '@testing-library/react'; import { initializeMocks, cleanup, screen, render, waitFor, + makeQueryClientWrapper, } from '../testUtils'; -import { useWaffleFlags } from './apiHooks'; -import { getApiWaffleFlagsUrl } from './api'; +import { useWaffleFlags, useUpdateCourseAppStatus, useUpdateCourseAdvancedSettings } from './apiHooks'; +import { getApiWaffleFlagsUrl, getCourseAppsApiUrl, getCourseAdvancedSettingsApiUrl } from './api'; + +const courseId = 'course-v1:edX+DemoX+Demo_Course'; // A little component for testing our waffle flag hooks. const FlagComponent = ({ courseId }: { courseId?: string; }) => { @@ -110,3 +114,44 @@ describe('useWaffleFlags', () => { expect(await screen.findByLabelText('useReactMarkdownEditor')).toHaveTextContent('enabled'); }); }); + +describe('useUpdateCourseAppStatus', () => { + it('sends a PATCH request and invalidates the course apps query on success', async () => { + const { axiosMock, queryClient } = initializeMocks(); + axiosMock.onPatch(`${getCourseAppsApiUrl()}/${courseId}`).reply(200); + const invalidateQueriesSpy = jest.spyOn(queryClient, 'invalidateQueries'); + + const { result } = renderHook( + () => useUpdateCourseAppStatus(courseId), + { wrapper: makeQueryClientWrapper(queryClient) }, + ); + + result.current.mutate({ appId: 'discussion', state: true }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(axiosMock.history.patch[0].url).toBe(`${getCourseAppsApiUrl()}/${courseId}`); + expect(JSON.parse(axiosMock.history.patch[0].data)).toEqual({ id: 'discussion', enabled: true }); + expect(invalidateQueriesSpy).toHaveBeenCalledWith({ queryKey: ['courseApps', courseId] }); + }); +}); + +describe('useUpdateCourseAdvancedSettings', () => { + it('sends a PATCH request and invalidates the course settings and apps queries on success', async () => { + const { axiosMock, queryClient } = initializeMocks(); + axiosMock.onPatch(`${getCourseAdvancedSettingsApiUrl()}/${courseId}`).reply(200, {}); + const invalidateQueriesSpy = jest.spyOn(queryClient, 'invalidateQueries'); + + const { result } = renderHook( + () => useUpdateCourseAdvancedSettings(courseId), + { wrapper: makeQueryClientWrapper(queryClient) }, + ); + + result.current.mutate({ setting: 'courseDisplayName', value: 'New Name' }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(axiosMock.history.patch[0].url).toBe(`${getCourseAdvancedSettingsApiUrl()}/${courseId}`); + expect(JSON.parse(axiosMock.history.patch[0].data)).toEqual({ course_display_name: { value: 'New Name' } }); + expect(invalidateQueriesSpy).toHaveBeenCalledWith({ queryKey: ['courseSettings', courseId] }); + expect(invalidateQueriesSpy).toHaveBeenCalledWith({ queryKey: ['courseApps', courseId] }); + }); +}); From 71b165220af7393b792f87a694c249d694102490 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 2 Sep 2026 19:19:28 -0500 Subject: [PATCH 08/10] fix: Linter issues --- src/CourseAuthoringContext.test.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/CourseAuthoringContext.test.tsx b/src/CourseAuthoringContext.test.tsx index ab04c34d6a..5c1694006d 100644 --- a/src/CourseAuthoringContext.test.tsx +++ b/src/CourseAuthoringContext.test.tsx @@ -19,10 +19,18 @@ describe('CourseAuthoringProvider', () => { axiosMock.onGet(getApiWaffleFlagsUrl(courseId)).reply(200, {}); axiosMock.onGet(`${getCourseAppsApiUrl()}/${courseId}`).reply(200, [ { - id: 'wiki', name: 'Wiki', description: '', enabled: true, allowed_operations: { enable: true, configure: true }, + id: 'wiki', + name: 'Wiki', + description: '', + enabled: true, + allowed_operations: { enable: true, configure: true }, }, { - id: 'discussion', name: 'Discussion', description: '', enabled: true, allowed_operations: { enable: true, configure: true }, + id: 'discussion', + name: 'Discussion', + description: '', + enabled: true, + allowed_operations: { enable: true, configure: true }, }, ]); From 68b2dfe4a1ec3d2ebe31479659d7a0d1a3185806 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 2 Sep 2026 19:53:01 -0500 Subject: [PATCH 09/10] test: Adding tests for AppSettingsModalBase --- .../AppSettingsModalBase.test.tsx | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 src/pages-and-resources/app-settings-modal/AppSettingsModalBase.test.tsx diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.test.tsx b/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.test.tsx new file mode 100644 index 0000000000..fb9475972b --- /dev/null +++ b/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.test.tsx @@ -0,0 +1,68 @@ +import { userEvent } from '@testing-library/user-event'; +import { initializeMocks, render, screen } from '@src/testUtils'; +import AppSettingsModalBase, { AppSettingsModalBaseProps } from './AppSettingsModalBase'; + +const onClose = jest.fn(); + +const baseProps: AppSettingsModalBaseProps = { + title: 'App Settings', + onClose, + variant: 'default', + isMobile: false, + isOpen: true, + children:
content
, +}; + +const renderComponent = (props: Partial = {}) => + render( + , + ); + +describe('AppSettingsModalBase', () => { + beforeEach(() => { + initializeMocks(); + onClose.mockClear(); + }); + + it('renders the title, children, footer, and disclaimer', () => { + renderComponent({ + children:
Modal body content
, + footer: , + disclaimer:

Some disclaimer text

, + }); + + expect(screen.getByTestId('modal-title')).toHaveTextContent('App Settings'); + expect(screen.getByText('Modal body content')).toBeInTheDocument(); + expect(screen.getByText('Some disclaimer text')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument(); + }); + + it('does not render the close button when not on mobile', () => { + renderComponent({ isMobile: false }); + + expect(screen.queryByRole('button', { name: /close/i })).not.toBeInTheDocument(); + }); + + it('renders the close button and calls onClose when on mobile', async () => { + const user = userEvent.setup(); + renderComponent({ isMobile: true }); + + const closeButton = screen.getByRole('button', { name: /close/i }); + await user.click(closeButton); + expect(onClose).toHaveBeenCalled(); + }); + + it('calls onClose when the cancel button is clicked', async () => { + const user = userEvent.setup(); + renderComponent(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + expect(onClose).toHaveBeenCalled(); + }); + + it('does not render the modal when isOpen is false', () => { + renderComponent({ isOpen: false }); + + expect(screen.queryByTestId('modal-title')).not.toBeInTheDocument(); + }); +}); From 41ade6a832429eda0f842fc22a171e571d06ab80 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 2 Sep 2026 21:09:22 -0500 Subject: [PATCH 10/10] fix: Broken coverage --- .../AppSettingsModalBase.test.tsx | 68 ------------------- .../AppSettingsModalBase.tsx | 2 +- 2 files changed, 1 insertion(+), 69 deletions(-) delete mode 100644 src/pages-and-resources/app-settings-modal/AppSettingsModalBase.test.tsx diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.test.tsx b/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.test.tsx deleted file mode 100644 index fb9475972b..0000000000 --- a/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.test.tsx +++ /dev/null @@ -1,68 +0,0 @@ -import { userEvent } from '@testing-library/user-event'; -import { initializeMocks, render, screen } from '@src/testUtils'; -import AppSettingsModalBase, { AppSettingsModalBaseProps } from './AppSettingsModalBase'; - -const onClose = jest.fn(); - -const baseProps: AppSettingsModalBaseProps = { - title: 'App Settings', - onClose, - variant: 'default', - isMobile: false, - isOpen: true, - children:
content
, -}; - -const renderComponent = (props: Partial = {}) => - render( - , - ); - -describe('AppSettingsModalBase', () => { - beforeEach(() => { - initializeMocks(); - onClose.mockClear(); - }); - - it('renders the title, children, footer, and disclaimer', () => { - renderComponent({ - children:
Modal body content
, - footer: , - disclaimer:

Some disclaimer text

, - }); - - expect(screen.getByTestId('modal-title')).toHaveTextContent('App Settings'); - expect(screen.getByText('Modal body content')).toBeInTheDocument(); - expect(screen.getByText('Some disclaimer text')).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument(); - }); - - it('does not render the close button when not on mobile', () => { - renderComponent({ isMobile: false }); - - expect(screen.queryByRole('button', { name: /close/i })).not.toBeInTheDocument(); - }); - - it('renders the close button and calls onClose when on mobile', async () => { - const user = userEvent.setup(); - renderComponent({ isMobile: true }); - - const closeButton = screen.getByRole('button', { name: /close/i }); - await user.click(closeButton); - expect(onClose).toHaveBeenCalled(); - }); - - it('calls onClose when the cancel button is clicked', async () => { - const user = userEvent.setup(); - renderComponent(); - - await user.click(screen.getByRole('button', { name: 'Cancel' })); - expect(onClose).toHaveBeenCalled(); - }); - - it('does not render the modal when isOpen is false', () => { - renderComponent({ isOpen: false }); - - expect(screen.queryByTestId('modal-title')).not.toBeInTheDocument(); - }); -}); diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.tsx b/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.tsx index d8cf53b6e7..395c6c3395 100644 --- a/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.tsx +++ b/src/pages-and-resources/app-settings-modal/AppSettingsModalBase.tsx @@ -23,7 +23,7 @@ const AppSettingsModalBase = ({ children, footer, disclaimer, - isOpen = true, + isOpen, }: AppSettingsModalBaseProps) => (