From 5991fd39976ad336cde0f068e50af198d737bdad Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 5 Jun 2025 11:02:57 -0700 Subject: [PATCH] refactor: Load waffle flags using React Query (#2068) * refactor: use React Query to load waffle flags * test: add test case * fix: more clear handling of data loading and fallbacks * refactor: simplify handling of useReactMarkdownEditor * test: use new mockWaffleFlags() helper * test: simplify test mocks in hooks.test.js * refactor: avoid duplicating flag names, clarify how defaults work --- src/CourseAuthoringPage.jsx | 3 +- src/advanced-settings/AdvancedSettings.jsx | 8 +- .../settings-sidebar/SettingsSidebar.jsx | 22 ++-- .../settings-sidebar/SettingsSidebar.test.jsx | 32 +----- .../ChecklistSection/ChecklistItemBody.jsx | 5 +- .../ChecklistSection/ChecklistItemComment.jsx | 5 +- src/course-outline/hooks.jsx | 4 +- src/course-outline/status-bar/StatusBar.jsx | 5 +- src/course-unit/CourseUnit.test.jsx | 4 +- .../add-component/AddComponent.jsx | 5 +- .../breadcrumbs/Breadcrumbs.test.tsx | 29 ++--- src/course-unit/breadcrumbs/Breadcrumbs.tsx | 4 +- .../xblock-container-iframe/index.tsx | 7 +- src/custom-pages/CustomPages.jsx | 4 +- src/data/api.js | 32 ------ src/data/api.ts | 73 ++++++++++++ src/data/apiHooks.mock.ts | 20 ++++ src/data/apiHooks.test.tsx | 106 ++++++++++++++++++ src/data/apiHooks.ts | 32 ++++++ src/data/{constants.js => constants.ts} | 0 src/data/selectors.js | 1 - src/data/slice.js | 23 ---- src/data/thunks.js | 13 +-- src/editors/Editor.tsx | 6 +- src/editors/EditorContainer.test.jsx | 4 + src/editors/EditorContainer.tsx | 5 - src/editors/EditorPage.tsx | 3 - .../EditorContainer.test.jsx.snap | 1 - src/export-page/CourseExportPage.jsx | 10 +- .../export-sidebar/ExportSidebar.jsx | 12 +- src/generic/help-sidebar/HelpSidebar.jsx | 5 +- src/generic/key-utils.ts | 5 + src/header/Header.tsx | 5 +- src/header/hooks.jsx | 8 +- src/header/hooks.test.js | 22 ++-- src/import-page/CourseImportPage.jsx | 10 +- .../pages/PageSettingButton.jsx | 5 +- .../pages/PageSettingButton.test.jsx | 52 ++------- src/schedule-and-details/index.jsx | 8 +- src/studio-home/card-item/CardItem.test.tsx | 4 +- src/studio-home/card-item/index.tsx | 4 +- src/studio-home/hooks.jsx | 2 - src/testUtils.tsx | 11 +- src/textbooks/Textbooks.jsx | 4 +- 44 files changed, 355 insertions(+), 268 deletions(-) delete mode 100644 src/data/api.js create mode 100644 src/data/api.ts create mode 100644 src/data/apiHooks.mock.ts create mode 100644 src/data/apiHooks.test.tsx create mode 100644 src/data/apiHooks.ts rename src/data/{constants.js => constants.ts} (100%) delete mode 100644 src/data/selectors.js diff --git a/src/CourseAuthoringPage.jsx b/src/CourseAuthoringPage.jsx index d1c87cd56..1a170b8dd 100644 --- a/src/CourseAuthoringPage.jsx +++ b/src/CourseAuthoringPage.jsx @@ -7,7 +7,7 @@ import { } from 'react-router-dom'; import { StudioFooterSlot } from '@edx/frontend-component-footer'; import Header from './header'; -import { fetchCourseDetail, fetchWaffleFlags } from './data/thunks'; +import { fetchCourseDetail } from './data/thunks'; import { useModel } from './generic/model-store'; import NotFoundAlert from './generic/NotFoundAlert'; import PermissionDeniedAlert from './generic/PermissionDeniedAlert'; @@ -21,7 +21,6 @@ const CourseAuthoringPage = ({ courseId, children }) => { useEffect(() => { dispatch(fetchCourseDetail(courseId)); - dispatch(fetchWaffleFlags(courseId)); }, [courseId]); useEffect(() => { diff --git a/src/advanced-settings/AdvancedSettings.jsx b/src/advanced-settings/AdvancedSettings.jsx index 67bd62b55..74c1a2674 100644 --- a/src/advanced-settings/AdvancedSettings.jsx +++ b/src/advanced-settings/AdvancedSettings.jsx @@ -5,7 +5,7 @@ import { Container, Button, Layout, StatefulButton, TransitionReplace, } from '@openedx/paragon'; import { CheckCircle, Info, Warning } from '@openedx/paragon/icons'; -import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import Placeholder from '../editors/Placeholder'; import AlertProctoringError from '../generic/AlertProctoringError'; @@ -26,7 +26,8 @@ import messages from './messages'; import ModalError from './modal-error/ModalError'; import getPageHeadTitle from '../generic/utils'; -const AdvancedSettings = ({ intl, courseId }) => { +const AdvancedSettings = ({ courseId }) => { + const intl = useIntl(); const dispatch = useDispatch(); const [saveSettingsPrompt, showSaveSettingsPrompt] = useState(false); const [showDeprecated, setShowDeprecated] = useState(false); @@ -278,8 +279,7 @@ const AdvancedSettings = ({ intl, courseId }) => { }; AdvancedSettings.propTypes = { - intl: intlShape.isRequired, courseId: PropTypes.string.isRequired, }; -export default injectIntl(AdvancedSettings); +export default AdvancedSettings; diff --git a/src/advanced-settings/settings-sidebar/SettingsSidebar.jsx b/src/advanced-settings/settings-sidebar/SettingsSidebar.jsx index dd20e63c9..823ffbd5f 100644 --- a/src/advanced-settings/settings-sidebar/SettingsSidebar.jsx +++ b/src/advanced-settings/settings-sidebar/SettingsSidebar.jsx @@ -1,28 +1,25 @@ +// @ts-check import React from 'react'; -import { - FormattedMessage, - injectIntl, - intlShape, -} from '@edx/frontend-platform/i18n'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; import PropTypes from 'prop-types'; import { HelpSidebar } from '../../generic/help-sidebar'; import messages from './messages'; -const SettingsSidebar = ({ intl, courseId, proctoredExamSettingsUrl }) => ( +const SettingsSidebar = ({ courseId, proctoredExamSettingsUrl = '' }) => (

- {intl.formatMessage(messages.about)} +

- {intl.formatMessage(messages.aboutDescription1)} +

- {intl.formatMessage(messages.aboutDescription2)} +

( ); -SettingsSidebar.defaultProps = { - proctoredExamSettingsUrl: '', -}; - SettingsSidebar.propTypes = { - intl: intlShape.isRequired, courseId: PropTypes.string.isRequired, proctoredExamSettingsUrl: PropTypes.string, }; -export default injectIntl(SettingsSidebar); +export default SettingsSidebar; diff --git a/src/advanced-settings/settings-sidebar/SettingsSidebar.test.jsx b/src/advanced-settings/settings-sidebar/SettingsSidebar.test.jsx index 9a5801a8e..386d8eb5a 100644 --- a/src/advanced-settings/settings-sidebar/SettingsSidebar.test.jsx +++ b/src/advanced-settings/settings-sidebar/SettingsSidebar.test.jsx @@ -1,43 +1,21 @@ -import React from 'react'; -import { render } from '@testing-library/react'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { initializeMockApp } from '@edx/frontend-platform'; -import { AppProvider } from '@edx/frontend-platform/react'; - -import initializeStore from '../../store'; +// @ts-check +import { initializeMocks, render } from '../../testUtils'; import SettingsSidebar from './SettingsSidebar'; import messages from './messages'; const courseId = 'course-123'; -let store; - -const RootWrapper = () => ( - - - - - -); describe('', () => { beforeEach(() => { - initializeMockApp({ - authenticatedUser: { - userId: 3, - username: 'abc123', - administrator: true, - roles: [], - }, - }); - store = initializeStore(); + initializeMocks(); }); it('renders about and other sidebar titles correctly', () => { - const { getByText } = render(); + const { getByText } = render(); expect(getByText(messages.about.defaultMessage)).toBeInTheDocument(); expect(getByText(messages.other.defaultMessage)).toBeInTheDocument(); }); it('renders about descriptions correctly', () => { - const { getByText } = render(); + const { getByText } = render(); const aboutThirtyDescription = getByText('When you enter strings as policy values, ensure that you use double quotation marks (“) around the string. Do not use single quotation marks (‘).'); expect(getByText(messages.aboutDescription1.defaultMessage)).toBeInTheDocument(); expect(getByText(messages.aboutDescription2.defaultMessage)).toBeInTheDocument(); diff --git a/src/course-checklist/ChecklistSection/ChecklistItemBody.jsx b/src/course-checklist/ChecklistSection/ChecklistItemBody.jsx index 703dd8c41..d42a5cb0d 100644 --- a/src/course-checklist/ChecklistSection/ChecklistItemBody.jsx +++ b/src/course-checklist/ChecklistSection/ChecklistItemBody.jsx @@ -2,11 +2,10 @@ import PropTypes from 'prop-types'; import { Link } from 'react-router-dom'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { ActionRow, Button, Icon } from '@openedx/paragon'; -import { useSelector } from 'react-redux'; import { CheckCircle, RadioButtonUnchecked } from '@openedx/paragon/icons'; import { getConfig } from '@edx/frontend-platform'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import messages from './messages'; const getUpdateLinks = (courseId, waffleFlags) => { @@ -35,7 +34,7 @@ const ChecklistItemBody = ({ isCompleted, }) => { const intl = useIntl(); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(courseId); const updateLinks = getUpdateLinks(courseId, waffleFlags); return ( diff --git a/src/course-checklist/ChecklistSection/ChecklistItemComment.jsx b/src/course-checklist/ChecklistSection/ChecklistItemComment.jsx index deec84f2c..1fda16188 100644 --- a/src/course-checklist/ChecklistSection/ChecklistItemComment.jsx +++ b/src/course-checklist/ChecklistSection/ChecklistItemComment.jsx @@ -1,11 +1,10 @@ import PropTypes from 'prop-types'; -import { useSelector } from 'react-redux'; import { injectIntl, FormattedMessage, FormattedNumber } from '@edx/frontend-platform/i18n'; import { Icon } from '@openedx/paragon'; import { Link } from 'react-router-dom'; import { ModeComment } from '@openedx/paragon/icons'; import { getConfig } from '@edx/frontend-platform'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import messages from './messages'; const ChecklistItemComment = ({ @@ -13,7 +12,7 @@ const ChecklistItemComment = ({ checkId, data, }) => { - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(courseId); const getPathToCourseOutlinePage = (assignmentId) => (waffleFlags.useNewCourseOutlinePage ? `/course/${courseId}#${assignmentId}` : `${getConfig().STUDIO_BASE_URL}/course/${courseId}#${assignmentId}`); diff --git a/src/course-outline/hooks.jsx b/src/course-outline/hooks.jsx index ae1b5000f..88d202391 100644 --- a/src/course-outline/hooks.jsx +++ b/src/course-outline/hooks.jsx @@ -6,7 +6,7 @@ import { getConfig } from '@edx/frontend-platform'; import moment from 'moment'; import { getSavingStatus as getGenericSavingStatus } from '../generic/data/selectors'; -import { getWaffleFlags } from '../data/selectors'; +import { useWaffleFlags } from '../data/apiHooks'; import { RequestStatus } from '../data/constants'; import { COURSE_BLOCK_NAMES } from './constants'; import { @@ -61,7 +61,7 @@ import { const useCourseOutline = ({ courseId }) => { const dispatch = useDispatch(); const navigate = useNavigate(); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(courseId); const { reindexLink, diff --git a/src/course-outline/status-bar/StatusBar.jsx b/src/course-outline/status-bar/StatusBar.jsx index 9b8828da6..e71a2f925 100644 --- a/src/course-outline/status-bar/StatusBar.jsx +++ b/src/course-outline/status-bar/StatusBar.jsx @@ -8,12 +8,11 @@ import { } from '@openedx/paragon'; import { Link } from 'react-router-dom'; import { AppContext } from '@edx/frontend-platform/react'; -import { useSelector } from 'react-redux'; import { ContentTagsDrawerSheet } from '../../content-tags-drawer'; import TagCount from '../../generic/tag-count'; import { useHelpUrls } from '../../help-urls/hooks'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import { VIDEO_SHARING_OPTIONS } from '../constants'; import { useContentTagsCount } from '../../generic/data/apiHooks'; import messages from './messages'; @@ -46,7 +45,7 @@ const StatusBar = ({ }) => { const intl = useIntl(); const { config } = useContext(AppContext); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(courseId); const { courseReleaseDate, diff --git a/src/course-unit/CourseUnit.test.jsx b/src/course-unit/CourseUnit.test.jsx index 3585d144c..dabc4210e 100644 --- a/src/course-unit/CourseUnit.test.jsx +++ b/src/course-unit/CourseUnit.test.jsx @@ -62,7 +62,7 @@ import xblockContainerIframeMessages from './xblock-container-iframe/messages'; import headerNavigationsMessages from './header-navigations/messages'; import sidebarMessages from './sidebar/messages'; import messages from './messages'; -import * as selectors from '../data/selectors'; +import { mockWaffleFlags } from '../data/apiHooks.mock'; let axiosMock; let store; @@ -847,7 +847,7 @@ describe('', () => { }); it('handles creating Video xblock and showing editor modal using videogalleryflow', async () => { - const waffleSpy = jest.spyOn(selectors, 'getWaffleFlags').mockReturnValue({ useVideoGalleryFlow: true }); + const waffleSpy = mockWaffleFlags({ useVideoGalleryFlow: true }); axiosMock .onPost(postXBlockBaseApiUrl({ type: 'video', category: 'video', parentLocator: blockId })) diff --git a/src/course-unit/add-component/AddComponent.jsx b/src/course-unit/add-component/AddComponent.jsx index 8ac317af6..71a41b962 100644 --- a/src/course-unit/add-component/AddComponent.jsx +++ b/src/course-unit/add-component/AddComponent.jsx @@ -8,7 +8,7 @@ import { } from '@openedx/paragon'; import { getCourseSectionVertical } from '../data/selectors'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import { COMPONENT_TYPES } from '../../generic/block-type-utils/constants'; import ComponentModalView from './add-component-modals/ComponentModalView'; import AddComponentButton from './add-component-btn'; @@ -45,7 +45,7 @@ const AddComponent = ({ const [selectedComponents, setSelectedComponents] = useState([]); const [usageId, setUsageId] = useState(null); const { sendMessageToIframe } = useIframe(); - const { useVideoGalleryFlow, useReactMarkdownEditor } = useSelector(getWaffleFlags); + const { useVideoGalleryFlow } = useWaffleFlags(courseId ?? undefined); const receiveMessage = useCallback(({ data: { type, payload } }) => { if (type === messageTypes.showMultipleComponentPicker) { @@ -266,7 +266,6 @@ const AddComponent = ({ courseId={courseId} blockType={blockType} blockId={newBlockId} - isMarkdownEditorEnabledForCourse={useReactMarkdownEditor} studioEndpointUrl={getConfig().STUDIO_BASE_URL} lmsEndpointUrl={getConfig().LMS_BASE_URL} onClose={closeXBlockEditorModal} diff --git a/src/course-unit/breadcrumbs/Breadcrumbs.test.tsx b/src/course-unit/breadcrumbs/Breadcrumbs.test.tsx index fb727d803..d0017f5e8 100644 --- a/src/course-unit/breadcrumbs/Breadcrumbs.test.tsx +++ b/src/course-unit/breadcrumbs/Breadcrumbs.test.tsx @@ -1,13 +1,12 @@ import userEvent from '@testing-library/user-event'; import { getConfig } from '@edx/frontend-platform'; import { - initializeMocks, waitFor, act, render, + initializeMocks, waitFor, render, } from '../../testUtils'; import { executeThunk } from '../../utils'; import { getCourseSectionVerticalApiUrl } from '../data/api'; import { getApiWaffleFlagsUrl } from '../../data/api'; -import { fetchWaffleFlags } from '../../data/thunks'; import { fetchCourseSectionVerticalData } from '../data/thunk'; import { courseSectionVerticalMock } from '../__mocks__'; import Breadcrumbs from './Breadcrumbs'; @@ -53,12 +52,6 @@ describe('', () => { axiosMock .onGet(getApiWaffleFlagsUrl(courseId)) .reply(200, { useNewCourseOutlinePage: true }); - await executeThunk(fetchWaffleFlags(courseId), reduxStore.dispatch); - }); - - afterEach(() => { - jest.clearAllMocks(); - axiosMock.restore(); }); it('render Breadcrumbs component correctly', async () => { @@ -128,16 +121,12 @@ describe('', () => { const { ancestor_xblocks: [{ children: [{ display_name, url }] }] } = courseSectionVerticalMock; const { getByText, getByRole } = renderComponent(); - await act(async () => { - const dropdownBtn = getByText(breadcrumbsExpected.section.displayName); - userEvent.click(dropdownBtn); - }); + const dropdownBtn = getByText(breadcrumbsExpected.section.displayName); + userEvent.click(dropdownBtn); - await act(async () => { - const dropdownItem = getByRole('link', { name: display_name }); - userEvent.click(dropdownItem); - expect(dropdownItem).toHaveAttribute('href', url); - }); + const dropdownItem = getByRole('link', { name: display_name }); + userEvent.click(dropdownItem); + expect(dropdownItem).toHaveAttribute('href', url); }); it('falls back to window.location.href when the waffle flag is disabled', async () => { @@ -146,7 +135,6 @@ describe('', () => { axiosMock .onGet(getApiWaffleFlagsUrl(courseId)) .reply(200, { useNewCourseOutlinePage: false }); - await executeThunk(fetchWaffleFlags(courseId), reduxStore.dispatch); const { getByText, getByRole } = renderComponent(); @@ -154,6 +142,9 @@ describe('', () => { userEvent.click(dropdownBtn); const dropdownItem = getByRole('link', { name: display_name }); - expect(dropdownItem).toHaveAttribute('href', `${getConfig().STUDIO_BASE_URL}${url}`); + // We need waitFor here because the waffle flag defaults to true but asynchronously loads false from our axiosMock + await waitFor(() => { + expect(dropdownItem).toHaveAttribute('href', `${getConfig().STUDIO_BASE_URL}${url}`); + }); }); }); diff --git a/src/course-unit/breadcrumbs/Breadcrumbs.tsx b/src/course-unit/breadcrumbs/Breadcrumbs.tsx index a23a46e03..dec4ce6da 100644 --- a/src/course-unit/breadcrumbs/Breadcrumbs.tsx +++ b/src/course-unit/breadcrumbs/Breadcrumbs.tsx @@ -7,13 +7,13 @@ import { } from '@openedx/paragon/icons'; import { getConfig } from '@edx/frontend-platform'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import { getCourseSectionVertical } from '../data/selectors'; import { adoptCourseSectionUrl } from '../utils'; const Breadcrumbs = ({ courseId, parentUnitId }: { courseId: string, parentUnitId: string }) => { const { ancestorXblocks = [] } = useSelector(getCourseSectionVertical); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(courseId); const getPathToCourseOutlinePage = (url) => (waffleFlags.useNewCourseOutlinePage ? url : `${getConfig().STUDIO_BASE_URL}${url}`); diff --git a/src/course-unit/xblock-container-iframe/index.tsx b/src/course-unit/xblock-container-iframe/index.tsx index dac95c96a..65b9c3bd3 100644 --- a/src/course-unit/xblock-container-iframe/index.tsx +++ b/src/course-unit/xblock-container-iframe/index.tsx @@ -4,7 +4,7 @@ import { } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { useToggle, Sheet, StandardModal } from '@openedx/paragon'; -import { useDispatch, useSelector } from 'react-redux'; +import { useDispatch } from 'react-redux'; import { hideProcessingNotification, @@ -13,7 +13,7 @@ import { import DeleteModal from '../../generic/delete-modal/DeleteModal'; import ConfigureModal from '../../generic/configure-modal/ConfigureModal'; import ModalIframe from '../../generic/modal-iframe'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import { IFRAME_FEATURE_POLICY } from '../../constants'; import ContentTagsDrawer from '../../content-tags-drawer/ContentTagsDrawer'; import { useIframe } from '../../generic/hooks/context/hooks'; @@ -49,7 +49,7 @@ const XBlockContainerIframe: FC = ({ const [isVideoSelectorModalOpen, showVideoSelectorModal, closeVideoSelectorModal] = useToggle(); const [isXBlockEditorModalOpen, showXBlockEditorModal, closeXBlockEditorModal] = useToggle(); const [blockType, setBlockType] = useState(''); - const { useVideoGalleryFlow, useReactMarkdownEditor } = useSelector(getWaffleFlags); + const { useVideoGalleryFlow } = useWaffleFlags(courseId); const [newBlockId, setNewBlockId] = useState(''); const [accessManagedXBlockData, setAccessManagedXBlockData] = useState({}); const [iframeOffset, setIframeOffset] = useState(0); @@ -230,7 +230,6 @@ const XBlockContainerIframe: FC = ({ courseId={courseId} blockType={blockType} blockId={newBlockId} - isMarkdownEditorEnabledForCourse={useReactMarkdownEditor} studioEndpointUrl={getConfig().STUDIO_BASE_URL} lmsEndpointUrl={getConfig().LMS_BASE_URL} onClose={closeXBlockEditorModal} diff --git a/src/custom-pages/CustomPages.jsx b/src/custom-pages/CustomPages.jsx index d6abcc1d2..40f90cd0d 100644 --- a/src/custom-pages/CustomPages.jsx +++ b/src/custom-pages/CustomPages.jsx @@ -39,7 +39,7 @@ import CustomPageCard from './CustomPageCard'; import messages from './messages'; import CustomPagesProvider from './CustomPagesProvider'; import EditModal from './EditModal'; -import { getWaffleFlags } from '../data/selectors'; +import { useWaffleFlags } from '../data/apiHooks'; import getPageHeadTitle from '../generic/utils'; import { getPagePath } from '../utils'; @@ -68,7 +68,7 @@ const CustomPages = ({ const deletePageStatus = useSelector(state => state.customPages.deletingStatus); const savingStatus = useSelector(getSavingStatus); const loadingStatus = useSelector(getLoadingStatus); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(courseId); const pages = useModels('customPages', customPagesIds); diff --git a/src/data/api.js b/src/data/api.js deleted file mode 100644 index 61a0719fb..000000000 --- a/src/data/api.js +++ /dev/null @@ -1,32 +0,0 @@ -import { camelCaseObject, getConfig } from '@edx/frontend-platform'; -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; - -const getStudioBaseUrl = () => getConfig().STUDIO_BASE_URL; - -export const getApiWaffleFlagsUrl = (courseId) => { - const baseUrl = getStudioBaseUrl(); - const apiPath = '/api/contentstore/v1/course_waffle_flags'; - - return courseId ? `${baseUrl}${apiPath}/${courseId}` : `${baseUrl}${apiPath}`; -}; - -function normalizeCourseDetail(data) { - return { - id: data.course_id, - ...camelCaseObject(data), - }; -} - -export async function getCourseDetail(courseId, username) { - const { data } = await getAuthenticatedHttpClient() - .get(`${getConfig().LMS_BASE_URL}/api/courses/v1/courses/${courseId}?username=${username}`); - - return normalizeCourseDetail(data); -} - -export async function getWaffleFlags(courseId) { - const { data } = await getAuthenticatedHttpClient() - .get(getApiWaffleFlagsUrl(courseId)); - - return normalizeCourseDetail(data); -} diff --git a/src/data/api.ts b/src/data/api.ts new file mode 100644 index 000000000..cddb466a2 --- /dev/null +++ b/src/data/api.ts @@ -0,0 +1,73 @@ +import { camelCaseObject, getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; + +const getStudioBaseUrl = () => getConfig().STUDIO_BASE_URL as string; + +export const getApiWaffleFlagsUrl = (courseId?: string): string => { + const baseUrl = getStudioBaseUrl(); + const apiPath = '/api/contentstore/v1/course_waffle_flags'; + + return courseId ? `${baseUrl}${apiPath}/${courseId}` : `${baseUrl}${apiPath}`; +}; + +function normalizeCourseDetail(data) { + return { + id: data.course_id, + ...camelCaseObject(data), + }; +} + +export async function getCourseDetail(courseId: string, username: string) { + const { data } = await getAuthenticatedHttpClient() + .get(`${getConfig().LMS_BASE_URL}/api/courses/v1/courses/${courseId}?username=${username}`); + + return normalizeCourseDetail(data); +} + +/** + * The default values of waffle flags, used while we're loading the "real" + * values from Studio's REST API, and/or if we fail to load them. + * May drift from edx-platform's actual defaults! + * TODO: clarify our strategy here: https://github.com/openedx/frontend-app-authoring/issues/2094 + */ +export const waffleFlagDefaults = { + enableCourseOptimizer: false, + useNewHomePage: true, + useNewCustomPages: true, + useNewScheduleDetailsPage: true, + useNewAdvancedSettingsPage: true, + useNewGradingPage: true, + useNewUpdatesPage: true, + useNewImportPage: false, + useNewExportPage: true, + useNewFilesUploadsPage: true, + useNewVideoUploadsPage: true, + useNewCourseOutlinePage: true, + useNewUnitPage: false, + useNewCourseTeamPage: true, + useNewCertificatesPage: true, + useNewTextbooksPage: true, + useNewGroupConfigurationsPage: true, + useReactMarkdownEditor: true, + useVideoGalleryFlow: false, +} as const; + +export type WaffleFlagName = keyof typeof waffleFlagDefaults; + +export type WaffleFlagsStatus = { id: string | undefined } & Record; + +/** + * Get Waffle Flags from Studio's REST API. + * Don't use this directly; use the `useWaffleFlags()` hook. + * + * A `mockWaffleFlags()` method is available if you need to override this in + * tests. + * + * @param courseId Get the flags for a specific course, which may be different + * than the system-wide flags. + */ +export async function getWaffleFlags(courseId?: string): Promise { + const { data } = await getAuthenticatedHttpClient() + .get(getApiWaffleFlagsUrl(courseId)); + return normalizeCourseDetail(data); +} diff --git a/src/data/apiHooks.mock.ts b/src/data/apiHooks.mock.ts new file mode 100644 index 000000000..bf922f9cf --- /dev/null +++ b/src/data/apiHooks.mock.ts @@ -0,0 +1,20 @@ +import { waffleFlagDefaults, WaffleFlagName } from './api'; +import * as apiHooks from './apiHooks'; + +/** + * For testing purposes, override the waffle flags (which enable/disable + * specific features). This will completely bypass React Query's waffle flag + * loading; if you need more realistic handling, use: + * axiosMock + * .onGet(getApiWaffleFlagsUrl(courseId)) + * .reply(200, { useNewCourseOutlinePage: true }); // etc + */ +export function mockWaffleFlags(overrides: Partial> = {}) { + return jest.spyOn(apiHooks, 'useWaffleFlags').mockImplementation(() => ({ + id: undefined, + isLoading: false, + isError: false, + ...waffleFlagDefaults, + ...overrides, + })); +} diff --git a/src/data/apiHooks.test.tsx b/src/data/apiHooks.test.tsx new file mode 100644 index 000000000..b5857d8f6 --- /dev/null +++ b/src/data/apiHooks.test.tsx @@ -0,0 +1,106 @@ +import { + initializeMocks, + cleanup, + screen, + render, + waitFor, +} from '../testUtils'; +import { useWaffleFlags } from './apiHooks'; +import { getApiWaffleFlagsUrl } from './api'; + +// A little component for testing our waffle flag hooks. +const FlagComponent = ({ courseId }: { courseId?: string }) => { + const waffleFlags = useWaffleFlags(courseId); + return ( +

+ ); +}; + +describe('useWaffleFlags', () => { + it('uses the default values while the waffle flags are loaded from the server', async () => { + const { axiosMock } = initializeMocks(); + // Simulate an actual slow response from the Waffle Flags REST API: + let resolveResponse; + const promise = new Promise<[number, unknown]>(resolve => { resolveResponse = resolve; }); + axiosMock.onGet(getApiWaffleFlagsUrl()).reply(() => promise); + + render(); + expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading'); + expect(screen.getByLabelText('isError')).toHaveTextContent('false'); + // The default should be enabled, even before we hear back from the server: + expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); + + // Then, the server responds with a new value: + resolveResponse([200, { useNewCourseOutlinePage: false }]); + + // Now, we're no longer loading and we have the new value: + await waitFor(() => { + expect(screen.getByLabelText('isLoading')).toHaveTextContent('false'); + }); + expect(screen.getByLabelText('isError')).toHaveTextContent('false'); + expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); + }); + + it('uses the default values if there\'s an error', async () => { + const { axiosMock } = initializeMocks(); + // Simulate an actual slow response from the Waffle Flags REST API: + let resolveResponse; + const promise = new Promise<[number, unknown]>(resolve => { resolveResponse = resolve; }); + axiosMock.onGet(getApiWaffleFlagsUrl()).reply(() => promise); + + render(); + expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading'); + expect(screen.getByLabelText('isError')).toHaveTextContent('false'); + // The default should be enabled, even before we hear back from the server: + expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); + + // Then, the server responds with an error + resolveResponse([500, {}]); + + // Now, we're no longer loading, we have an error state, and we still have the default value: + await waitFor(() => { + expect(screen.getByLabelText('isLoading')).toHaveTextContent('false'); + }); + expect(screen.getByLabelText('isError')).toHaveTextContent('error'); + expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); + }); + + it('uses the global flag values while loading the course-specific flags', async () => { + const { axiosMock } = initializeMocks(); + const courseId = 'course-v1:A+b+C'; + // Set the global flag OFF: + axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, { useNewCourseOutlinePage: false }); + // Control when we respond with the course-specific flag value: + let resolveResponse; + const promise = new Promise<[number, unknown]>(resolve => { resolveResponse = resolve; }); + axiosMock.onGet(getApiWaffleFlagsUrl(courseId)).reply(() => promise); + + // Check the global flag: + render(); + await waitFor(() => { + // Once it loads the flags from the server, the global 'false' value will override the default 'true': + expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); + }); + + // Now check the course-specific flag: + cleanup(); + render(); + + // Now, the course-specific value is loading but in the meantime we use the global default: + expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading'); + expect(screen.getByLabelText('isError')).toHaveTextContent('false'); + expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); + + // Now the server responds: the course-specific flag is ON: + resolveResponse([200, { useNewCourseOutlinePage: true }]); + await waitFor(() => { + expect(screen.getByLabelText('isLoading')).toHaveTextContent('false'); + }); + expect(screen.getByLabelText('isError')).toHaveTextContent('false'); + expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); + }); +}); diff --git a/src/data/apiHooks.ts b/src/data/apiHooks.ts new file mode 100644 index 000000000..709f9d315 --- /dev/null +++ b/src/data/apiHooks.ts @@ -0,0 +1,32 @@ +import { useQuery, useQueryClient } from '@tanstack/react-query'; +import { getWaffleFlags, waffleFlagDefaults } from './api'; + +/** + * Get the waffle flags (which enable/disable specific features). They may + * depend on which course we're in. + */ +export const useWaffleFlags = (courseId?: string) => { + const queryClient = useQueryClient(); + + const { data, isLoading, isError } = useQuery({ + queryKey: ['waffleFlags', courseId], + queryFn: () => getWaffleFlags(courseId), + // Waffle flags change rarely, so never bother refetching them: + staleTime: Infinity, + refetchOnWindowFocus: false, + }); + let globalDefaults: typeof waffleFlagDefaults | undefined; + if (data === undefined && courseId) { + // If course-specific waffle flags were requested, first default to the + // global (studio-wide) flags until we've loaded the course-specific ones. + globalDefaults = queryClient.getQueryData(['waffleFlags', undefined]); + } + return { + ...waffleFlagDefaults, + ...globalDefaults, // Only used if we're requesting course-specific flags. + ...data, // the actual flag values loaded from the server + id: courseId, + isLoading, + isError, + }; +}; diff --git a/src/data/constants.js b/src/data/constants.ts similarity index 100% rename from src/data/constants.js rename to src/data/constants.ts diff --git a/src/data/selectors.js b/src/data/selectors.js deleted file mode 100644 index ad41d0852..000000000 --- a/src/data/selectors.js +++ /dev/null @@ -1 +0,0 @@ -export const getWaffleFlags = (state) => state.courseDetail?.waffleFlags; diff --git a/src/data/slice.js b/src/data/slice.js index 4f858916b..749e53f2b 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -9,25 +9,6 @@ const slice = createSlice({ courseId: null, status: null, canChangeProvider: null, - waffleFlags: { - useNewHomePage: true, - useNewCustomPages: true, - useNewScheduleDetailsPage: true, - useNewAdvancedSettingsPage: true, - useNewGradingPage: true, - useNewUpdatesPage: true, - useNewImportPage: false, - useNewExportPage: true, - useNewFilesUploadsPage: true, - useNewVideoUploadsPage: true, - useNewCourseOutlinePage: true, - useNewUnitPage: false, - useNewCourseTeamPage: true, - useNewCertificatesPage: true, - useNewTextbooksPage: true, - useNewGroupConfigurationsPage: true, - useVideoGalleryFlow: false, - }, }, reducers: { updateStatus: (state, { payload }) => { @@ -37,16 +18,12 @@ const slice = createSlice({ updateCanChangeProviders: (state, { payload }) => { state.canChangeProviders = payload.canChangeProviders; }, - fetchWaffleFlagsSuccess: (state, { payload }) => { - state.waffleFlags = payload.waffleFlags; - }, }, }); export const { updateStatus, updateCanChangeProviders, - fetchWaffleFlagsSuccess, } = slice.actions; export const { diff --git a/src/data/thunks.js b/src/data/thunks.js index 9c3069316..5e2e862bf 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -1,10 +1,9 @@ import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import { addModel } from '../generic/model-store'; -import { getCourseDetail, getWaffleFlags } from './api'; +import { getCourseDetail } from './api'; import { updateStatus, updateCanChangeProviders, - fetchWaffleFlagsSuccess, } from './slice'; import { RequestStatus } from './constants'; @@ -29,13 +28,3 @@ export function fetchCourseDetail(courseId) { } }; } - -export function fetchWaffleFlags(courseId) { - return async (dispatch) => { - dispatch(updateStatus({ courseId, status: RequestStatus.IN_PROGRESS })); - - const waffleFlags = await getWaffleFlags(courseId); - dispatch(updateStatus({ courseId, status: RequestStatus.SUCCESSFUL })); - dispatch(fetchWaffleFlagsSuccess({ waffleFlags })); - }; -} diff --git a/src/editors/Editor.tsx b/src/editors/Editor.tsx index 4c565451b..538d31d3b 100644 --- a/src/editors/Editor.tsx +++ b/src/editors/Editor.tsx @@ -5,6 +5,8 @@ import { useDispatch } from 'react-redux'; import * as hooks from './hooks'; +import { useWaffleFlags } from '../data/apiHooks'; +import { isCourseKey } from '../generic/key-utils'; import supportedEditors from './supportedEditors'; import type { EditorComponent } from './EditorComponent'; import AdvancedEditor from './AdvancedEditor'; @@ -12,7 +14,6 @@ import AdvancedEditor from './AdvancedEditor'; export interface Props extends EditorComponent { blockType: string; blockId: string | null; - isMarkdownEditorEnabledForCourse: boolean; learningContextId: string | null; lmsEndpointUrl: string | null; studioEndpointUrl: string | null; @@ -22,12 +23,13 @@ const Editor: React.FC = ({ learningContextId, blockType, blockId, - isMarkdownEditorEnabledForCourse, lmsEndpointUrl, studioEndpointUrl, onClose = null, returnFunction = null, }) => { + const courseIdIfCourse = isCourseKey(learningContextId) ? learningContextId : undefined; + const isMarkdownEditorEnabledForCourse = useWaffleFlags(courseIdIfCourse).useReactMarkdownEditor; const dispatch = useDispatch(); const loading = hooks.useInitializeApp({ dispatch, diff --git a/src/editors/EditorContainer.test.jsx b/src/editors/EditorContainer.test.jsx index b9076dbf5..a50718eb9 100644 --- a/src/editors/EditorContainer.test.jsx +++ b/src/editors/EditorContainer.test.jsx @@ -1,6 +1,10 @@ +// @ts-check import React from 'react'; import { shallow } from '@edx/react-unit-test-utils'; import EditorContainer from './EditorContainer'; +import { mockWaffleFlags } from '../data/apiHooks.mock'; + +mockWaffleFlags(); const mockPathname = '/editor/'; jest.mock('react-router-dom', () => ({ diff --git a/src/editors/EditorContainer.tsx b/src/editors/EditorContainer.tsx index 6b808aba1..c3962cf3b 100644 --- a/src/editors/EditorContainer.tsx +++ b/src/editors/EditorContainer.tsx @@ -5,13 +5,11 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { Button, Hyperlink } from '@openedx/paragon'; import { Warning as WarningIcon } from '@openedx/paragon/icons'; -import { useSelector } from 'react-redux'; import EditorPage from './EditorPage'; import AlertMessage from '../generic/alert-message'; import messages from './messages'; import { getLibraryId } from '../generic/key-utils'; import { createCorrectInternalRoute } from '../utils'; -import { getWaffleFlags } from '../data/selectors'; interface Props { /** Course ID or Library ID */ @@ -39,8 +37,6 @@ const EditorContainer: React.FC = ({ const location = useLocation(); const [searchParams] = useSearchParams(); const upstreamLibRef = searchParams.get('upstreamLibRef'); - const waffleFlags = useSelector(getWaffleFlags); - const isMarkdownEditorEnabledForCourse = waffleFlags?.useReactMarkdownEditor; if (blockType === undefined || blockId === undefined) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. @@ -80,7 +76,6 @@ const EditorContainer: React.FC = ({ courseId={learningContextId} blockType={blockType} blockId={blockId} - isMarkdownEditorEnabledForCourse={isMarkdownEditorEnabledForCourse} studioEndpointUrl={getConfig().STUDIO_BASE_URL} lmsEndpointUrl={getConfig().LMS_BASE_URL} onClose={onClose ? () => onClose(location.state?.from) : null} diff --git a/src/editors/EditorPage.tsx b/src/editors/EditorPage.tsx index d7c80ed4e..47c0ff479 100644 --- a/src/editors/EditorPage.tsx +++ b/src/editors/EditorPage.tsx @@ -11,7 +11,6 @@ interface Props extends EditorComponent { blockId?: string; blockType: string; courseId: string; - isMarkdownEditorEnabledForCourse?: boolean; lmsEndpointUrl?: string; studioEndpointUrl?: string; children?: never; @@ -25,7 +24,6 @@ const EditorPage: React.FC = ({ courseId, blockType, blockId = null, - isMarkdownEditorEnabledForCourse = false, lmsEndpointUrl = null, studioEndpointUrl = null, onClose = null, @@ -45,7 +43,6 @@ const EditorPage: React.FC = ({ learningContextId: courseId, blockType, blockId, - isMarkdownEditorEnabledForCourse, lmsEndpointUrl, studioEndpointUrl, returnFunction, diff --git a/src/editors/__snapshots__/EditorContainer.test.jsx.snap b/src/editors/__snapshots__/EditorContainer.test.jsx.snap index 71043325c..d73484841 100644 --- a/src/editors/__snapshots__/EditorContainer.test.jsx.snap +++ b/src/editors/__snapshots__/EditorContainer.test.jsx.snap @@ -60,7 +60,6 @@ exports[`Editor Container snapshots rendering correctly with expected Input 1`] blockId="company-id1" blockType="html" courseId="cOuRsEId" - isMarkdownEditorEnabledForCourse={true} lmsEndpointUrl="http://localhost:18000" onClose={null} returnFunction={null} diff --git a/src/export-page/CourseExportPage.jsx b/src/export-page/CourseExportPage.jsx index 36f96f251..c83123336 100644 --- a/src/export-page/CourseExportPage.jsx +++ b/src/export-page/CourseExportPage.jsx @@ -1,7 +1,7 @@ import React, { useEffect } from 'react'; import PropTypes from 'prop-types'; import { useDispatch, useSelector } from 'react-redux'; -import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { useIntl } from '@edx/frontend-platform/i18n'; import { Container, Layout, Button, Card, } from '@openedx/paragon'; @@ -27,7 +27,8 @@ import ExportModalError from './export-modal-error/ExportModalError'; import ExportFooter from './export-footer/ExportFooter'; import ExportStepper from './export-stepper/ExportStepper'; -const CourseExportPage = ({ intl, courseId }) => { +const CourseExportPage = ({ courseId }) => { + const intl = useIntl(); const dispatch = useDispatch(); const exportTriggered = useSelector(getExportTriggered); const courseDetails = useModel('courseDetails', courseId); @@ -128,10 +129,7 @@ const CourseExportPage = ({ intl, courseId }) => { }; CourseExportPage.propTypes = { - intl: intlShape.isRequired, courseId: PropTypes.string.isRequired, }; -CourseExportPage.defaultProps = {}; - -export default injectIntl(CourseExportPage); +export default CourseExportPage; diff --git a/src/export-page/export-sidebar/ExportSidebar.jsx b/src/export-page/export-sidebar/ExportSidebar.jsx index c8684a577..05deae297 100644 --- a/src/export-page/export-sidebar/ExportSidebar.jsx +++ b/src/export-page/export-sidebar/ExportSidebar.jsx @@ -1,8 +1,4 @@ -import React from 'react'; -import { - injectIntl, - intlShape, -} from '@edx/frontend-platform/i18n'; +import { useIntl } from '@edx/frontend-platform/i18n'; import PropTypes from 'prop-types'; import { Hyperlink } from '@openedx/paragon'; import { getConfig } from '@edx/frontend-platform'; @@ -11,7 +7,8 @@ import { HelpSidebar } from '../../generic/help-sidebar'; import { useHelpUrls } from '../../help-urls/hooks'; import messages from './messages'; -const ExportSidebar = ({ intl, courseId }) => { +const ExportSidebar = ({ courseId }) => { + const intl = useIntl(); const { exportCourse: exportLearnMoreUrl } = useHelpUrls(['exportCourse']); return ( @@ -42,8 +39,7 @@ const ExportSidebar = ({ intl, courseId }) => { }; ExportSidebar.propTypes = { - intl: intlShape.isRequired, courseId: PropTypes.string.isRequired, }; -export default injectIntl(ExportSidebar); +export default ExportSidebar; diff --git a/src/generic/help-sidebar/HelpSidebar.jsx b/src/generic/help-sidebar/HelpSidebar.jsx index a3ca03dc8..6053e835e 100644 --- a/src/generic/help-sidebar/HelpSidebar.jsx +++ b/src/generic/help-sidebar/HelpSidebar.jsx @@ -1,11 +1,10 @@ import PropTypes from 'prop-types'; -import { useSelector } from 'react-redux'; import { useLocation } from 'react-router-dom'; import classNames from 'classnames'; import { useIntl } from '@edx/frontend-platform/i18n'; import { getConfig } from '@edx/frontend-platform'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import { otherLinkURLParams } from './constants'; import messages from './messages'; import HelpSidebarLink from './HelpSidebarLink'; @@ -26,7 +25,7 @@ const HelpSidebar = ({ scheduleAndDetails, groupConfigurations, } = otherLinkURLParams; - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(); const showOtherLink = (params) => !pathname.includes(params); const generateLegacyURL = (urlParameter) => { diff --git a/src/generic/key-utils.ts b/src/generic/key-utils.ts index 03689a29d..3e219292c 100644 --- a/src/generic/key-utils.ts +++ b/src/generic/key-utils.ts @@ -27,6 +27,11 @@ export function getLibraryId(usageKey: string): string { throw new Error(`Invalid usageKey: ${usageKey}`); } +/** Check if this is a course key */ +export function isCourseKey(learningContextKey: string | undefined | null): learningContextKey is string { + return typeof learningContextKey === 'string' && learningContextKey.startsWith('course-v1:'); +} + /** Check if this is a V2 library key. */ export function isLibraryKey(learningContextKey: string | undefined | null): learningContextKey is string { return typeof learningContextKey === 'string' && learningContextKey.startsWith('lib:'); diff --git a/src/header/Header.tsx b/src/header/Header.tsx index d3a7d478d..4b0325291 100644 --- a/src/header/Header.tsx +++ b/src/header/Header.tsx @@ -1,10 +1,9 @@ -import { useSelector } from 'react-redux'; import { getConfig } from '@edx/frontend-platform'; import { useIntl } from '@edx/frontend-platform/i18n'; import { StudioHeader } from '@edx/frontend-component-header'; import { type Container, useToggle } from '@openedx/paragon'; -import { getWaffleFlags } from '../data/selectors'; +import { useWaffleFlags } from '../data/apiHooks'; import { SearchModal } from '../search-modal'; import { useContentMenuItems, useSettingMenuItems, useToolsMenuItems } from './hooks'; import messages from './messages'; @@ -31,7 +30,7 @@ const Header = ({ containerProps = {}, }: HeaderProps) => { const intl = useIntl(); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(); const [isShowSearchModalOpen, openSearchModal, closeSearchModal] = useToggle(false); diff --git a/src/header/hooks.jsx b/src/header/hooks.jsx index d070b8949..81f3a4dbf 100644 --- a/src/header/hooks.jsx +++ b/src/header/hooks.jsx @@ -4,7 +4,7 @@ import { useSelector } from 'react-redux'; import { Badge } from '@openedx/paragon'; import { getPagePath } from '../utils'; -import { getWaffleFlags } from '../data/selectors'; +import { useWaffleFlags } from '../data/apiHooks'; import { getStudioHomeData } from '../studio-home/data/selectors'; import messages from './messages'; import courseOptimizerMessages from '../optimizer-page/messages'; @@ -12,7 +12,7 @@ import courseOptimizerMessages from '../optimizer-page/messages'; export const useContentMenuItems = courseId => { const intl = useIntl(); const studioBaseUrl = getConfig().STUDIO_BASE_URL; - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(); const { librariesV2Enabled } = useSelector(getStudioHomeData); const items = [ @@ -54,7 +54,7 @@ export const useSettingMenuItems = courseId => { const intl = useIntl(); const studioBaseUrl = getConfig().STUDIO_BASE_URL; const { canAccessAdvancedSettings } = useSelector(getStudioHomeData); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(); const items = [ { @@ -92,7 +92,7 @@ export const useSettingMenuItems = courseId => { export const useToolsMenuItems = courseId => { const intl = useIntl(); const studioBaseUrl = getConfig().STUDIO_BASE_URL; - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(); const items = [ { diff --git a/src/header/hooks.test.js b/src/header/hooks.test.js index 95546293d..176c0905a 100644 --- a/src/header/hooks.test.js +++ b/src/header/hooks.test.js @@ -3,6 +3,7 @@ import { getConfig, setConfig } from '@edx/frontend-platform'; import { renderHook } from '@testing-library/react'; import messages from './messages'; import { useContentMenuItems, useToolsMenuItems, useSettingMenuItems } from './hooks'; +import { mockWaffleFlags } from '../data/apiHooks.mock'; jest.mock('@edx/frontend-platform/i18n', () => ({ ...jest.requireActual('@edx/frontend-platform/i18n'), @@ -11,6 +12,14 @@ jest.mock('@edx/frontend-platform/i18n', () => ({ }), })); +// Bypass React Query for waffle flags, and just return the default values. +mockWaffleFlags({ + // Some flags can be enabled with either a config value or a waffle flag. + // For test purposes, we'll configure the video upload page using the config, so leave the waffle flag off. + useNewVideoUploadsPage: false, + useNewCertificatesPage: false, +}); + jest.mock('react-redux', () => ({ ...jest.requireActual('react-redux'), useSelector: jest.fn(), @@ -84,11 +93,6 @@ describe('header utils', () => { }); describe('getToolsMenuItems', () => { - beforeEach(() => { - useSelector.mockReturnValue({ - waffleFlags: jest.fn(), - }); - }); it('when tags enabled should include export tags option', () => { setConfig({ ...getConfig(), @@ -116,7 +120,9 @@ describe('header utils', () => { }); it('when course optimizer enabled should include optimizer option', () => { - useSelector.mockReturnValue({ enableCourseOptimizer: true }); + mockWaffleFlags({ + enableCourseOptimizer: true, + }); const optimizerItem = renderHook(() => useToolsMenuItems('course-123')).result.current.find( item => item.href === '/course/course-123/optimizer', ); @@ -124,7 +130,9 @@ describe('header utils', () => { }); it('when course optimizer disabled should not include optimizer option', () => { - useSelector.mockReturnValue({ enableCourseOptimizer: false }); + mockWaffleFlags({ + enableCourseOptimizer: false, + }); const actualItemsTitle = renderHook(() => useToolsMenuItems('course-123')).result.current.map((item) => item.title); expect(actualItemsTitle).not.toContain(messages['header.links.optimizer'].defaultMessage); }); diff --git a/src/import-page/CourseImportPage.jsx b/src/import-page/CourseImportPage.jsx index 2daea3fdb..78cce332f 100644 --- a/src/import-page/CourseImportPage.jsx +++ b/src/import-page/CourseImportPage.jsx @@ -2,7 +2,7 @@ import React, { useEffect } from 'react'; import PropTypes from 'prop-types'; import { useDispatch, useSelector } from 'react-redux'; -import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { useIntl } from '@edx/frontend-platform/i18n'; import { Container, Layout, } from '@openedx/paragon'; @@ -24,7 +24,8 @@ import ImportSidebar from './import-sidebar/ImportSidebar'; import FileSection from './file-section/FileSection'; import messages from './messages'; -const CourseImportPage = ({ intl, courseId }) => { +const CourseImportPage = ({ courseId }) => { + const intl = useIntl(); const dispatch = useDispatch(); const cookies = new Cookies(); const courseDetails = useModel('courseDetails', courseId); @@ -104,10 +105,7 @@ const CourseImportPage = ({ intl, courseId }) => { }; CourseImportPage.propTypes = { - intl: intlShape.isRequired, courseId: PropTypes.string.isRequired, }; -CourseImportPage.defaultProps = {}; - -export default injectIntl(CourseImportPage); +export default CourseImportPage; diff --git a/src/pages-and-resources/pages/PageSettingButton.jsx b/src/pages-and-resources/pages/PageSettingButton.jsx index 039ac14e2..4759032ae 100644 --- a/src/pages-and-resources/pages/PageSettingButton.jsx +++ b/src/pages-and-resources/pages/PageSettingButton.jsx @@ -1,13 +1,12 @@ import { useContext, useMemo } from 'react'; import PropTypes from 'prop-types'; -import { useSelector } from 'react-redux'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Icon, IconButton } from '@openedx/paragon'; import { ArrowForward, Settings } from '@openedx/paragon/icons'; import { useNavigate, Link } from 'react-router-dom'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import messages from '../messages'; import { PagesAndResourcesContext } from '../PagesAndResourcesProvider'; @@ -20,7 +19,7 @@ const PageSettingButton = ({ const { formatMessage } = useIntl(); const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); const navigate = useNavigate(); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(courseId); const determineLinkDestination = useMemo(() => { if (!legacyLink) { return null; } diff --git a/src/pages-and-resources/pages/PageSettingButton.test.jsx b/src/pages-and-resources/pages/PageSettingButton.test.jsx index cf12b0794..c44eeeaf6 100644 --- a/src/pages-and-resources/pages/PageSettingButton.test.jsx +++ b/src/pages-and-resources/pages/PageSettingButton.test.jsx @@ -1,33 +1,7 @@ -import { screen, render } from '@testing-library/react'; -import { useSelector } from 'react-redux'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; +// @ts-check +import { screen, render, initializeMocks } from '../../testUtils'; import PageSettingButton from './PageSettingButton'; - -jest.mock('react-redux', () => ({ - useSelector: jest.fn(), -})); - -jest.mock('react-router-dom', () => { - // eslint-disable-next-line global-require - const PropTypes = require('prop-types'); - - const Link = ({ children, to }) => {children}; - - Link.propTypes = { - children: PropTypes.node.isRequired, - to: PropTypes.string.isRequired, - }; - - return { - useNavigate: jest.fn(), - Link, - }; -}); - -const mockWaffleFlags = { - useNewTextbooksPage: true, - useNewCustomPages: true, -}; +import { mockWaffleFlags } from '../../data/apiHooks.mock'; const defaultProps = { id: 'page_id', @@ -36,20 +10,16 @@ const defaultProps = { allowedOperations: { configure: true, enable: true }, }; -const renderComponent = (props = {}) => render( - - - , -); +const renderComponent = (props = {}) => render(); + +mockWaffleFlags(); describe('PageSettingButton', () => { beforeEach(() => { - useSelector.mockClear(); + initializeMocks(); }); it('renders the settings button with the new textbooks page link when useNewTextbooksPage is true', () => { - useSelector.mockReturnValue(mockWaffleFlags); - renderComponent({ legacyLink: 'http://legacylink.com/textbooks' }); const linkElement = screen.getByRole('link'); @@ -57,15 +27,13 @@ describe('PageSettingButton', () => { }); it('does not render link when legacyLink prop value incorrect', () => { - useSelector.mockReturnValue(mockWaffleFlags); - renderComponent({ legacyLink: 'http://legacylink.com/some-value' }); expect(screen.queryByRole('link')).toBeNull(); }); it('renders the settings button with the legacy link when useNewTextbooksPage is false', () => { - useSelector.mockReturnValue({ ...mockWaffleFlags, useNewTextbooksPage: false }); + mockWaffleFlags({ useNewTextbooksPage: false }); renderComponent({ legacyLink: 'http://legacylink.com/textbooks' }); @@ -74,8 +42,6 @@ describe('PageSettingButton', () => { }); it('renders the settings button with the new custom pages link when useNewCustomPages is true', () => { - useSelector.mockReturnValue(mockWaffleFlags); - renderComponent(); const linkElement = screen.getByRole('link'); @@ -83,7 +49,7 @@ describe('PageSettingButton', () => { }); it('renders the settings button with the legacy link when useNewCustomPages is false', () => { - useSelector.mockReturnValue({ ...mockWaffleFlags, useNewCustomPages: false }); + mockWaffleFlags({ useNewCustomPages: false }); renderComponent(); diff --git a/src/schedule-and-details/index.jsx b/src/schedule-and-details/index.jsx index f8d114f44..dcb7adc36 100644 --- a/src/schedule-and-details/index.jsx +++ b/src/schedule-and-details/index.jsx @@ -9,7 +9,7 @@ import { ErrorOutline as ErrorOutlineIcon, Warning as WarningIcon, } from '@openedx/paragon/icons'; -import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { useIntl } from '@edx/frontend-platform/i18n'; import Placeholder from '../editors/Placeholder'; import { RequestStatus } from '../data/constants'; @@ -44,7 +44,8 @@ import ScheduleSidebar from './schedule-sidebar'; import messages from './messages'; import { useLoadValuesPrompt, useSaveValuesPrompt } from './hooks'; -const ScheduleAndDetails = ({ intl, courseId }) => { +const ScheduleAndDetails = ({ courseId }) => { + const intl = useIntl(); const courseSettings = useSelector(getCourseSettings); const courseDetails = useSelector(getCourseDetails); const loadingDetailsStatus = useSelector(getLoadingDetailsStatus); @@ -391,8 +392,7 @@ const ScheduleAndDetails = ({ intl, courseId }) => { }; ScheduleAndDetails.propTypes = { - intl: intlShape.isRequired, courseId: PropTypes.string.isRequired, }; -export default injectIntl(ScheduleAndDetails); +export default ScheduleAndDetails; diff --git a/src/studio-home/card-item/CardItem.test.tsx b/src/studio-home/card-item/CardItem.test.tsx index b63102a59..189b112f1 100644 --- a/src/studio-home/card-item/CardItem.test.tsx +++ b/src/studio-home/card-item/CardItem.test.tsx @@ -28,7 +28,7 @@ describe('', () => { const props = studioHomeMock.archivedCourses[0]; render(); const courseTitleLink = screen.getByText(props.displayName); - expect(courseTitleLink).toHaveAttribute('href', `${getConfig().STUDIO_BASE_URL}${props.url}`); + expect(courseTitleLink).toHaveAttribute('href', `${props.url}`); const dropDownMenu = screen.getByRole('button', { name: /course actions/i }); fireEvent.click(dropDownMenu); const btnReRunCourse = screen.getByText(messages.btnReRunText.defaultMessage); @@ -41,7 +41,7 @@ describe('', () => { const props = studioHomeMock.archivedCourses[0]; render(); const courseTitleLink = screen.getByText(props.displayName); - expect(courseTitleLink).toHaveAttribute('href', `${getConfig().STUDIO_BASE_URL}${props.url}`); + expect(courseTitleLink).toHaveAttribute('href', `${props.url}`); const dropDownMenu = screen.getByRole('button', { name: /course actions/i }); fireEvent.click(dropDownMenu); const btnReRunCourse = screen.getByText(messages.btnReRunText.defaultMessage); diff --git a/src/studio-home/card-item/index.tsx b/src/studio-home/card-item/index.tsx index 22b5fc3a1..9c56c44b2 100644 --- a/src/studio-home/card-item/index.tsx +++ b/src/studio-home/card-item/index.tsx @@ -10,7 +10,7 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { getConfig } from '@edx/frontend-platform'; import { Link } from 'react-router-dom'; -import { getWaffleFlags } from '../../data/selectors'; +import { useWaffleFlags } from '../../data/apiHooks'; import { COURSE_CREATOR_STATES } from '../../constants'; import { getStudioHomeData } from '../data/selectors'; import messages from '../messages'; @@ -56,7 +56,7 @@ const CardItem: React.FC = ({ courseCreatorStatus, rerunCreatorStatus, } = useSelector(getStudioHomeData); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(); const destinationUrl: string = path ?? ( waffleFlags.useNewCourseOutlinePage && !isLibraries diff --git a/src/studio-home/hooks.jsx b/src/studio-home/hooks.jsx index 0134111f2..d1673aae9 100644 --- a/src/studio-home/hooks.jsx +++ b/src/studio-home/hooks.jsx @@ -6,7 +6,6 @@ import { RequestStatus } from '../data/constants'; import { COURSE_CREATOR_STATES } from '../constants'; import { getCourseData, getSavingStatus } from '../generic/data/selectors'; import { fetchStudioHomeData } from './data/thunks'; -import { fetchWaffleFlags } from '../data/thunks'; import { getLoadingStatuses, getSavingStatuses, @@ -35,7 +34,6 @@ const useStudioHome = () => { useEffect(() => { dispatch(fetchStudioHomeData(location.search ?? '')); setShowNewCourseContainer(false); - dispatch(fetchWaffleFlags()); }, [location.search]); useEffect(() => { diff --git a/src/testUtils.tsx b/src/testUtils.tsx index 2d9bf83b9..3cb64361a 100644 --- a/src/testUtils.tsx +++ b/src/testUtils.tsx @@ -172,9 +172,14 @@ export function initializeMocks({ user = defaultUser, initialState = undefined } }); axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - axiosMock - .onGet(getApiWaffleFlagsUrl()) - .reply(200, {}); + // Many tests use waffle flags, so don't bother trying to load them from the (non-existent) + // server during test runs. This avoids a lot of noisy 'Request failed with + // status code 404' warnings. (Note this won't mock out course-specific requests) + // + // To override waffle flags for specific tests, just re-create this onGet mock + // with new values within your test/beforeAll, or use mockWaffleFlags() + // from src/data/apiHooks.mock.ts + axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {}); // Reset `mockToastContext` for this current test mockToastContext = { diff --git a/src/textbooks/Textbooks.jsx b/src/textbooks/Textbooks.jsx index b2c060797..5658f60a1 100644 --- a/src/textbooks/Textbooks.jsx +++ b/src/textbooks/Textbooks.jsx @@ -12,6 +12,7 @@ import { useSelector } from 'react-redux'; import { Helmet } from 'react-helmet'; import { Link } from 'react-router-dom'; +import { useWaffleFlags } from '../data/apiHooks'; import { SavingErrorAlert } from '../generic/saving-error-alert'; import { getProcessingNotification } from '../generic/processing-notification/data/selectors'; import { useModel } from '../generic/model-store'; @@ -26,11 +27,10 @@ import TextbookForm from './textbook-form/TextbookForm'; import { useTextbooks } from './hooks'; import { getTextbookFormInitialValues } from './utils'; import messages from './messages'; -import { getWaffleFlags } from '../data/selectors'; const Textbooks = ({ courseId }) => { const intl = useIntl(); - const waffleFlags = useSelector(getWaffleFlags); + const waffleFlags = useWaffleFlags(courseId); const courseDetails = useModel('courseDetails', courseId);