From f1bd35eb0cd820d405946801ea8dc068ab487514 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Tue, 4 Mar 2025 16:16:21 -0500 Subject: [PATCH 01/11] feat: add asset existence check before replacing static URLs in TinyMCE content * fix: update image replacement in the initial value so the change is not taken as a user action --- .../sharedComponents/TinyMceWidget/hooks.ts | 16 +++++++++++++--- .../sharedComponents/TinyMceWidget/index.tsx | 4 ++++ src/schedule-and-details/data/slice.js | 4 ++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.ts b/src/editors/sharedComponents/TinyMceWidget/hooks.ts index 7b906de2d1..31ad904db0 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.ts +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.ts @@ -5,9 +5,11 @@ import { useEffect, } from 'react'; import { getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { getLocale, isRtl } from '@edx/frontend-platform/i18n'; import { a11ycheckerCss } from 'frontend-components-tinymce-advanced-plugins'; import { isEmpty } from 'lodash'; +import { updateCourseDetailsOverview } from '../../../schedule-and-details/data/slice'; import tinyMCEStyles from '../../data/constants/tinyMCEStyles'; import { StrictDict } from '../../utils'; import pluginConfig from './pluginConfig'; @@ -116,7 +118,7 @@ export const replaceStaticWithAsset = ({ src => src.startsWith('/static') || src.startsWith('/asset'), ); if (!isEmpty(srcs)) { - srcs.forEach(src => { + srcs.forEach(async src => { const currentContent = content; let staticFullUrl; const isStatic = src.startsWith('/static/'); @@ -154,8 +156,16 @@ export const replaceStaticWithAsset = ({ } if (staticFullUrl) { const currentSrc = src.substring(0, src.indexOf('"')); - content = currentContent.replace(currentSrc, staticFullUrl); - hasChanges = true; + + // check if the asset exists on the server before replacing + try { + await getAuthenticatedHttpClient() + .get(staticFullUrl); + content = currentContent.replace(currentSrc, staticFullUrl); + hasChanges = true; + } catch (e) { + content = currentContent; + } } }); if (hasChanges) { return content; } diff --git a/src/editors/sharedComponents/TinyMceWidget/index.tsx b/src/editors/sharedComponents/TinyMceWidget/index.tsx index 66bef9832e..62b87f0107 100644 --- a/src/editors/sharedComponents/TinyMceWidget/index.tsx +++ b/src/editors/sharedComponents/TinyMceWidget/index.tsx @@ -2,6 +2,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Editor } from '@tinymce/tinymce-react'; import { getConfig } from '@edx/frontend-platform'; +import { useDispatch } from 'react-redux'; import 'tinymce'; import 'tinymce/themes/silver'; @@ -54,6 +55,8 @@ const TinyMceWidget = ({ const { imagesRef } = hooks.useImages({ images, editorContentHtml }); const imageSelection = hooks.selectedImage(null); + const dispatch = useDispatch(); + return ( <> {enableImageUpload && ( @@ -92,6 +95,7 @@ const TinyMceWidget = ({ images: imagesRef, editorContentHtml, staticRootUrl, + dispatch, ...imageSelection, ...editorConfig, }) diff --git a/src/schedule-and-details/data/slice.js b/src/schedule-and-details/data/slice.js index ba65e02a18..9a9903bfdb 100644 --- a/src/schedule-and-details/data/slice.js +++ b/src/schedule-and-details/data/slice.js @@ -31,6 +31,9 @@ const slice = createSlice({ fetchCourseSettingsSuccess: (state, { payload }) => { Object.assign(state.courseSettings, payload); }, + updateCourseDetailsOverview: (state, { payload }) => { + state.courseDetails.overview = payload; + }, }, }); @@ -41,6 +44,7 @@ export const { updateCourseDetailsSuccess, fetchCourseDetailsSuccess, fetchCourseSettingsSuccess, + updateCourseDetailsOverview, } = slice.actions; export const { From c4628758784d0c4f328c8187c6716507ab943f88 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Tue, 22 Apr 2025 16:13:25 -0500 Subject: [PATCH 02/11] feat: add option to validate asset URLs before replacement in TinyMCE content --- src/editors/sharedComponents/TinyMceWidget/hooks.test.js | 8 +++++++- src/editors/sharedComponents/TinyMceWidget/hooks.ts | 7 +++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js index 61af2dfca7..539db74012 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js @@ -194,7 +194,12 @@ describe('TinyMceEditor hooks', () => { const lmsEndpointUrl = getConfig().LMS_BASE_URL; it('returns updated src for text editor to update content', () => { const expected = `test`; - const actual = module.replaceStaticWithAsset({ initialContent, learningContextId }); + const actual = module.replaceStaticWithAsset({ + initialContent, + learningContextId, + validateAssetUrl: false, + + }); expect(actual).toEqual(expected); }); it('returns updated src with absolute url for expandable editor to update content', () => { @@ -205,6 +210,7 @@ describe('TinyMceEditor hooks', () => { editorType, lmsEndpointUrl, learningContextId, + validateAssetUrl: false, }); expect(actual).toEqual(expected); }); diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.ts b/src/editors/sharedComponents/TinyMceWidget/hooks.ts index 31ad904db0..8d191f3975 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.ts +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.ts @@ -111,6 +111,7 @@ export const replaceStaticWithAsset = ({ learningContextId, editorType, lmsEndpointUrl, + validateAssetUrl = true, }) => { let content = initialContent; let hasChanges = false; @@ -159,8 +160,10 @@ export const replaceStaticWithAsset = ({ // check if the asset exists on the server before replacing try { - await getAuthenticatedHttpClient() - .get(staticFullUrl); + if (validateAssetUrl) { + await getAuthenticatedHttpClient() + .get(staticFullUrl); + } content = currentContent.replace(currentSrc, staticFullUrl); hasChanges = true; } catch (e) { From 0828e3b5699e42ef6d792af06f417e4ae1557db2 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Wed, 23 Apr 2025 15:39:08 -0500 Subject: [PATCH 03/11] test: add unit test for updateCourseDetailsOverview in scheduleAndDetails slice --- src/schedule-and-details/data/slice.test.js | 23 +++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/schedule-and-details/data/slice.test.js diff --git a/src/schedule-and-details/data/slice.test.js b/src/schedule-and-details/data/slice.test.js new file mode 100644 index 0000000000..3fab2d05c3 --- /dev/null +++ b/src/schedule-and-details/data/slice.test.js @@ -0,0 +1,23 @@ +import { reducer, updateCourseDetailsOverview } from './slice'; + +describe('scheduleAndDetails slice', () => { + it('should update courseDetails.overview when updateCourseDetailsOverview is dispatched', () => { + const prevState = { + loadingDetailsStatus: 'IN_PROGRESS', + loadingSettingsStatus: 'IN_PROGRESS', + savingStatus: '', + courseDetails: { + title: 'Intro to Testing', + overview: 'Old overview', + }, + courseSettings: {}, + }; + + const newOverview = '

New overview HTML content

'; + + const nextState = reducer(prevState, updateCourseDetailsOverview(newOverview)); + + expect(nextState.courseDetails.overview).toEqual(newOverview); + expect(nextState.courseDetails.title).toEqual('Intro to Testing'); + }); +}); From 3f5a3dd418b1c9d9bba69c7a31011e48bc98d0ee Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Mon, 16 Jun 2025 15:40:32 -0500 Subject: [PATCH 04/11] feat: refactor course details management by removing deprecated actions and integrating new API hooks --- .../sharedComponents/TinyMceWidget/hooks.ts | 1 - .../data/{api.js => api.ts} | 0 src/schedule-and-details/data/apiHooks.ts | 32 +++++++++++++++++++ src/schedule-and-details/data/selectors.js | 2 -- src/schedule-and-details/data/slice.js | 18 ----------- src/schedule-and-details/data/thunks.js | 25 +-------------- src/schedule-and-details/hooks.jsx | 14 ++++---- src/schedule-and-details/index.jsx | 28 +++++++--------- 8 files changed, 52 insertions(+), 68 deletions(-) rename src/schedule-and-details/data/{api.js => api.ts} (100%) create mode 100644 src/schedule-and-details/data/apiHooks.ts diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.ts b/src/editors/sharedComponents/TinyMceWidget/hooks.ts index 8d191f3975..8397de1527 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.ts +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.ts @@ -9,7 +9,6 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { getLocale, isRtl } from '@edx/frontend-platform/i18n'; import { a11ycheckerCss } from 'frontend-components-tinymce-advanced-plugins'; import { isEmpty } from 'lodash'; -import { updateCourseDetailsOverview } from '../../../schedule-and-details/data/slice'; import tinyMCEStyles from '../../data/constants/tinyMCEStyles'; import { StrictDict } from '../../utils'; import pluginConfig from './pluginConfig'; diff --git a/src/schedule-and-details/data/api.js b/src/schedule-and-details/data/api.ts similarity index 100% rename from src/schedule-and-details/data/api.js rename to src/schedule-and-details/data/api.ts diff --git a/src/schedule-and-details/data/apiHooks.ts b/src/schedule-and-details/data/apiHooks.ts new file mode 100644 index 0000000000..8f0147574b --- /dev/null +++ b/src/schedule-and-details/data/apiHooks.ts @@ -0,0 +1,32 @@ +import { useQuery, useQueryClient } from '@tanstack/react-query'; + +import { getCourseDetails } from './api'; + +/** + * Get the details of a course. + */ +export const useGetCourseDetails = (courseId?: string) => { + const queryClient = useQueryClient(); + + const { + data, isLoading, isError, refetch, + } = useQuery({ + queryKey: ['courseDetails', courseId], + queryFn: () => getCourseDetails(courseId), + refetchOnWindowFocus: false, + }); + let globalDefaults: { [key: string]: any } | 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(['courseDetails', undefined]); + } + return { + ...globalDefaults, + ...data, + id: courseId, + isLoading, + isError, + refetch, + }; +}; diff --git a/src/schedule-and-details/data/selectors.js b/src/schedule-and-details/data/selectors.js index cff5c69ba3..e388de4eee 100644 --- a/src/schedule-and-details/data/selectors.js +++ b/src/schedule-and-details/data/selectors.js @@ -1,5 +1,3 @@ -export const getLoadingDetailsStatus = (state) => state.scheduleAndDetails.loadingDetailsStatus; export const getLoadingSettingsStatus = (state) => state.scheduleAndDetails.loadingSettingsStatus; export const getSavingStatus = (state) => state.scheduleAndDetails.savingStatus; -export const getCourseDetails = state => state.scheduleAndDetails.courseDetails; export const getCourseSettings = (state) => state.scheduleAndDetails.courseSettings; diff --git a/src/schedule-and-details/data/slice.js b/src/schedule-and-details/data/slice.js index 9a9903bfdb..a73b3a1d19 100644 --- a/src/schedule-and-details/data/slice.js +++ b/src/schedule-and-details/data/slice.js @@ -6,45 +6,27 @@ import { RequestStatus } from '../../data/constants'; const slice = createSlice({ name: 'scheduleAndDetails', initialState: { - loadingDetailsStatus: RequestStatus.IN_PROGRESS, loadingSettingsStatus: RequestStatus.IN_PROGRESS, savingStatus: '', - courseDetails: {}, courseSettings: {}, }, reducers: { - updateLoadingDetailsStatus: (state, { payload }) => { - state.loadingDetailsStatus = payload.status; - }, updateLoadingSettingsStatus: (state, { payload }) => { state.loadingSettingsStatus = payload.status; }, updateSavingStatus: (state, { payload }) => { state.savingStatus = payload.status; }, - updateCourseDetailsSuccess: (state, { payload }) => { - Object.assign(state.courseDetails, payload); - }, - fetchCourseDetailsSuccess: (state, { payload }) => { - Object.assign(state.courseDetails, payload); - }, fetchCourseSettingsSuccess: (state, { payload }) => { Object.assign(state.courseSettings, payload); }, - updateCourseDetailsOverview: (state, { payload }) => { - state.courseDetails.overview = payload; - }, }, }); export const { updateSavingStatus, - updateLoadingDetailsStatus, updateLoadingSettingsStatus, - updateCourseDetailsSuccess, - fetchCourseDetailsSuccess, fetchCourseSettingsSuccess, - updateCourseDetailsOverview, } = slice.actions; export const { diff --git a/src/schedule-and-details/data/thunks.js b/src/schedule-and-details/data/thunks.js index bc2be6dc41..45773b91df 100644 --- a/src/schedule-and-details/data/thunks.js +++ b/src/schedule-and-details/data/thunks.js @@ -1,44 +1,21 @@ import { RequestStatus } from '../../data/constants'; import { - getCourseDetails, updateCourseDetails, getCourseSettings, } from './api'; import { updateSavingStatus, - updateLoadingDetailsStatus, updateLoadingSettingsStatus, - fetchCourseDetailsSuccess, - updateCourseDetailsSuccess, fetchCourseSettingsSuccess, } from './slice'; -export function fetchCourseDetailsQuery(courseId) { - return async (dispatch) => { - dispatch(updateLoadingDetailsStatus({ status: RequestStatus.IN_PROGRESS })); - - try { - const detailsValues = await getCourseDetails(courseId); - dispatch(fetchCourseDetailsSuccess(detailsValues)); - dispatch(updateLoadingDetailsStatus({ status: RequestStatus.SUCCESSFUL })); - } catch (error) { - if (error.response && error.response.status === 403) { - dispatch(updateLoadingDetailsStatus({ courseId, status: RequestStatus.DENIED })); - } else { - dispatch(updateLoadingDetailsStatus({ status: RequestStatus.FAILED })); - } - } - }; -} - export function updateCourseDetailsQuery(courseId, details) { return async (dispatch) => { dispatch(updateSavingStatus({ status: RequestStatus.IN_PROGRESS })); try { - const detailsValues = await updateCourseDetails(courseId, details); + await updateCourseDetails(courseId, details); dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - dispatch(updateCourseDetailsSuccess(detailsValues)); return true; } catch { dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); diff --git a/src/schedule-and-details/hooks.jsx b/src/schedule-and-details/hooks.jsx index 218b92dc1c..e5d8b8b3f0 100644 --- a/src/schedule-and-details/hooks.jsx +++ b/src/schedule-and-details/hooks.jsx @@ -3,30 +3,28 @@ import { useDispatch, useSelector } from 'react-redux'; import { useIntl } from '@edx/frontend-platform/i18n'; import { RequestStatus } from '../data/constants'; -import { getLoadingDetailsStatus, getLoadingSettingsStatus, getSavingStatus } from './data/selectors'; +import { getLoadingSettingsStatus, getSavingStatus } from './data/selectors'; import { validateScheduleAndDetails, updateWithDefaultValues } from './utils'; const useLoadValuesPrompt = ( courseId, - fetchCourseDetailsQuery, fetchCourseSettingsQuery, + courseDetailsError, ) => { const dispatch = useDispatch(); - const loadingDetailsStatus = useSelector(getLoadingDetailsStatus); const loadingSettingsStatus = useSelector(getLoadingSettingsStatus); const [showLoadFailedAlert, setShowLoadFailedAlert] = useState(false); useEffect(() => { - dispatch(fetchCourseDetailsQuery(courseId)); dispatch(fetchCourseSettingsQuery(courseId)); }, [courseId]); useEffect(() => { - if (loadingDetailsStatus === RequestStatus.FAILED || loadingSettingsStatus === RequestStatus.FAILED) { + if (courseDetailsError || loadingSettingsStatus === RequestStatus.FAILED) { setShowLoadFailedAlert(true); window.scrollTo({ top: 0, behavior: 'smooth' }); } - }, [loadingDetailsStatus, loadingSettingsStatus]); + }, [courseDetailsError, loadingSettingsStatus]); return { showLoadFailedAlert, @@ -54,7 +52,7 @@ const useSaveValuesPrompt = ( if (!isQueryPending && !isEditableState) { setEditedValues(initialEditedData); } - }, [initialEditedData]); + }, [initialEditedData.isLoading]); useEffect(() => { const errors = validateScheduleAndDetails(editedValues, canShowCertificateAvailableDateField, intl); @@ -115,6 +113,8 @@ const useSaveValuesPrompt = ( if (!isEditableState) { setShowModifiedAlert(false); } + // Refresh course data after successful save + initialEditedData.refetch(); } else if (savingStatus === RequestStatus.FAILED) { setIsQueryPending(false); setShowSuccessfulAlert(false); diff --git a/src/schedule-and-details/index.jsx b/src/schedule-and-details/index.jsx index 9b6aad15e3..15aa0db56f 100644 --- a/src/schedule-and-details/index.jsx +++ b/src/schedule-and-details/index.jsx @@ -9,24 +9,21 @@ import { } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; -import Placeholder from '@src/editors/Placeholder'; -import { RequestStatus } from '@src/data/constants'; -import AlertMessage from '@src/generic/alert-message'; -import InternetConnectionAlert from '@src/generic/internet-connection-alert'; -import { STATEFUL_BUTTON_STATES } from '@src/constants'; -import getPageHeadTitle from '@src/generic/utils'; -import { useScrollToHashElement } from '@src/hooks'; +import Placeholder from '../editors/Placeholder'; +import { RequestStatus } from '../data/constants'; +import { useGetCourseDetails } from './data/apiHooks'; +import AlertMessage from '../generic/alert-message'; +import InternetConnectionAlert from '../generic/internet-connection-alert'; +import { STATEFUL_BUTTON_STATES } from '../constants'; +import getPageHeadTitle from '../generic/utils'; +import { useScrollToHashElement } from '../hooks'; import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; - import { fetchCourseSettingsQuery, - fetchCourseDetailsQuery, updateCourseDetailsQuery, } from './data/thunks'; import { getCourseSettings, - getCourseDetails, - getLoadingDetailsStatus, getLoadingSettingsStatus, } from './data/selectors'; import BasicSection from './basic-section'; @@ -45,11 +42,10 @@ import { useLoadValuesPrompt, useSaveValuesPrompt } from './hooks'; const ScheduleAndDetails = () => { const intl = useIntl(); + const courseDetails = useGetCourseDetails(courseId); const courseSettings = useSelector(getCourseSettings); - const courseDetails = useSelector(getCourseDetails); - const loadingDetailsStatus = useSelector(getLoadingDetailsStatus); const loadingSettingsStatus = useSelector(getLoadingSettingsStatus); - const isLoading = loadingDetailsStatus === RequestStatus.IN_PROGRESS + const isLoading = courseDetails.isLoading || loadingSettingsStatus === RequestStatus.IN_PROGRESS; const { courseId, courseDetails: course } = useCourseAuthoringContext(); @@ -81,8 +77,8 @@ const ScheduleAndDetails = () => { showLoadFailedAlert, } = useLoadValuesPrompt( courseId, - fetchCourseDetailsQuery, fetchCourseSettingsQuery, + courseDetails.isError, ); const { @@ -149,7 +145,7 @@ const ScheduleAndDetails = () => { return <>; } - if (loadingDetailsStatus === RequestStatus.DENIED || loadingSettingsStatus === RequestStatus.DENIED) { + if (courseDetails.isError || loadingSettingsStatus === RequestStatus.DENIED) { return (
From 8bb8cdbccd4221de6ee0f59c4a5f440bec22069a Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Mon, 16 Jun 2025 16:03:48 -0500 Subject: [PATCH 05/11] fix: remove unnecessary error check for courseDetails in ScheduleAndDetails component --- src/schedule-and-details/data/slice.test.js | 23 --------------------- src/schedule-and-details/index.jsx | 2 +- 2 files changed, 1 insertion(+), 24 deletions(-) delete mode 100644 src/schedule-and-details/data/slice.test.js diff --git a/src/schedule-and-details/data/slice.test.js b/src/schedule-and-details/data/slice.test.js deleted file mode 100644 index 3fab2d05c3..0000000000 --- a/src/schedule-and-details/data/slice.test.js +++ /dev/null @@ -1,23 +0,0 @@ -import { reducer, updateCourseDetailsOverview } from './slice'; - -describe('scheduleAndDetails slice', () => { - it('should update courseDetails.overview when updateCourseDetailsOverview is dispatched', () => { - const prevState = { - loadingDetailsStatus: 'IN_PROGRESS', - loadingSettingsStatus: 'IN_PROGRESS', - savingStatus: '', - courseDetails: { - title: 'Intro to Testing', - overview: 'Old overview', - }, - courseSettings: {}, - }; - - const newOverview = '

New overview HTML content

'; - - const nextState = reducer(prevState, updateCourseDetailsOverview(newOverview)); - - expect(nextState.courseDetails.overview).toEqual(newOverview); - expect(nextState.courseDetails.title).toEqual('Intro to Testing'); - }); -}); diff --git a/src/schedule-and-details/index.jsx b/src/schedule-and-details/index.jsx index 15aa0db56f..7e7f25d1e9 100644 --- a/src/schedule-and-details/index.jsx +++ b/src/schedule-and-details/index.jsx @@ -145,7 +145,7 @@ const ScheduleAndDetails = () => { return <>; } - if (courseDetails.isError || loadingSettingsStatus === RequestStatus.DENIED) { + if (loadingSettingsStatus === RequestStatus.DENIED) { return (
From 0a3f35e386b566edcdf79c1a1e9ec210f460cf33 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Mon, 16 Jun 2025 16:17:00 -0500 Subject: [PATCH 06/11] fix: remove unnecessary blank line in TinyMceEditor hooks test --- src/editors/sharedComponents/TinyMceWidget/hooks.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js index 539db74012..0da6f4832b 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js @@ -198,7 +198,6 @@ describe('TinyMceEditor hooks', () => { initialContent, learningContextId, validateAssetUrl: false, - }); expect(actual).toEqual(expected); }); From f2b6861d737290ab2b90c89a954454b9fc98c3e7 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Mon, 16 Jun 2025 16:18:28 -0500 Subject: [PATCH 07/11] fix: remove unused dispatch from TinyMceWidget component --- src/editors/sharedComponents/TinyMceWidget/index.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/editors/sharedComponents/TinyMceWidget/index.tsx b/src/editors/sharedComponents/TinyMceWidget/index.tsx index 62b87f0107..66bef9832e 100644 --- a/src/editors/sharedComponents/TinyMceWidget/index.tsx +++ b/src/editors/sharedComponents/TinyMceWidget/index.tsx @@ -2,7 +2,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Editor } from '@tinymce/tinymce-react'; import { getConfig } from '@edx/frontend-platform'; -import { useDispatch } from 'react-redux'; import 'tinymce'; import 'tinymce/themes/silver'; @@ -55,8 +54,6 @@ const TinyMceWidget = ({ const { imagesRef } = hooks.useImages({ images, editorContentHtml }); const imageSelection = hooks.selectedImage(null); - const dispatch = useDispatch(); - return ( <> {enableImageUpload && ( @@ -95,7 +92,6 @@ const TinyMceWidget = ({ images: imagesRef, editorContentHtml, staticRootUrl, - dispatch, ...imageSelection, ...editorConfig, }) From 91daea4644649ac6b76ed325e4754a66c388fc40 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Tue, 11 Nov 2025 16:33:48 -0500 Subject: [PATCH 08/11] feat: add validateAssetUrl prop to TextEditor component --- src/editors/containers/TextEditor/index.jsx | 4 ++++ src/editors/containers/TextEditor/index.test.tsx | 1 + 2 files changed, 5 insertions(+) diff --git a/src/editors/containers/TextEditor/index.jsx b/src/editors/containers/TextEditor/index.jsx index 03fb498e9f..19e6d518a4 100644 --- a/src/editors/containers/TextEditor/index.jsx +++ b/src/editors/containers/TextEditor/index.jsx @@ -32,6 +32,7 @@ const TextEditor = ({ learningContextId, images, isLibrary, + validateAssetUrl, }) => { const intl = useIntl(); const { editorRef, refReady, setEditorRef } = prepareEditorRef(); @@ -39,6 +40,7 @@ const TextEditor = ({ const newContent = replaceStaticWithAsset({ initialContent, learningContextId, + validateAssetUrl, }); const editorContent = newContent || initialContent; let staticRootUrl; @@ -106,6 +108,7 @@ TextEditor.defaultProps = { blockValue: null, blockFinished: null, returnFunction: null, + validateAssetUrl: null, }; TextEditor.propTypes = { onClose: PropTypes.func.isRequired, @@ -122,6 +125,7 @@ TextEditor.propTypes = { learningContextId: PropTypes.string, // This should be required but is NULL when the store is in initial state :/ images: PropTypes.shape({}).isRequired, isLibrary: PropTypes.bool.isRequired, + validateAssetUrl: PropTypes.bool, }; export const mapStateToProps = (state) => ({ diff --git a/src/editors/containers/TextEditor/index.test.tsx b/src/editors/containers/TextEditor/index.test.tsx index 0844ffb932..12de26a2b1 100644 --- a/src/editors/containers/TextEditor/index.test.tsx +++ b/src/editors/containers/TextEditor/index.test.tsx @@ -70,6 +70,7 @@ describe('TextEditor', () => { test('renders static images with relative paths', () => { const updatedProps = { ...props, + validateAssetUrl: false, blockValue: { data: { data: 'eDiTablE Text with ' } }, }; const { container } = render(); From 0c86a39f7a193e5316f7d58cd1a358f051c12364 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 6 Mar 2026 14:43:01 -0500 Subject: [PATCH 09/11] refactor: address feedback --- .../TinyMceWidget/hooks.test.js | 99 +++++++++++++++++-- .../sharedComponents/TinyMceWidget/hooks.ts | 20 ++-- src/schedule-and-details/data/apiHooks.ts | 16 +-- src/schedule-and-details/hooks.jsx | 2 +- 4 files changed, 109 insertions(+), 28 deletions(-) diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js index 0da6f4832b..3a93b8bbd8 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js @@ -1,5 +1,7 @@ import 'CourseAuthoring/editors/setupEditorTest'; import { getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import * as keyUtils from '../../../generic/key-utils'; import { MockUseState } from '../../testUtils'; import * as tinyMCE from '../../data/constants/tinyMCE'; @@ -19,6 +21,10 @@ jest.mock('react', () => ({ useCallback: (cb, prereqs) => ({ cb, prereqs }), })); +jest.mock('@edx/frontend-platform/auth', () => ({ + getAuthenticatedHttpClient: jest.fn(), +})); + const state = new MockUseState(module); const moduleKeys = keyStore(module); @@ -192,30 +198,34 @@ describe('TinyMceEditor hooks', () => { const initialContent = `test`; const learningContextId = 'course-v1:org+test+run'; const lmsEndpointUrl = getConfig().LMS_BASE_URL; - it('returns updated src for text editor to update content', () => { + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns updated src for text editor to update content', async () => { const expected = `test`; - const actual = module.replaceStaticWithAsset({ + const actual = await module.replaceStaticWithAsset({ initialContent, learningContextId, validateAssetUrl: false, }); expect(actual).toEqual(expected); }); - it('returns updated src with absolute url for expandable editor to update content', () => { - const editorType = 'expandable'; + it('returns updated src with absolute url for expandable editor to update content', async () => { const expected = `test`; - const actual = module.replaceStaticWithAsset({ + const actual = await module.replaceStaticWithAsset({ initialContent, - editorType, + editorType: 'expandable', lmsEndpointUrl, learningContextId, validateAssetUrl: false, }); expect(actual).toEqual(expected); }); - it('returns false when there are no srcs to update', () => { + it('returns false when there are no srcs to update', async () => { const content = '
Hello world!
'; - const actual = module.replaceStaticWithAsset({ initialContent: content, learningContextId }); + const actual = await module.replaceStaticWithAsset({ initialContent: content, learningContextId }); expect(actual).toBeFalsy(); }); it('does not convert static URLs with subdirectories but converts direct static files', () => { @@ -227,6 +237,79 @@ describe('TinyMceEditor hooks', () => { }); expect(actual).toEqual(expected); }); + + it('replaces multiple static assets in one content string', async () => { + const content = ` + + + `; + + const result = await module.replaceStaticWithAsset({ + initialContent: content, + learningContextId, + validateAssetUrl: false, + }); + + expect(result).toBeTruthy(); + }); + + it('validateAssetUrl success path replaces url', async () => { + getAuthenticatedHttpClient.mockReturnValue({ + get: jest.fn(() => Promise.resolve({})), + }); + + const content = ''; + + const result = await module.replaceStaticWithAsset({ + initialContent: content, + learningContextId, + validateAssetUrl: true, + }); + + expect(result).toBeTruthy(); + }); + + it('validateAssetUrl failure path keeps original content', async () => { + getAuthenticatedHttpClient.mockReturnValue({ + get: jest.fn(() => Promise.reject(new Error('404'))), + }); + + const content = ''; + + const result = await module.replaceStaticWithAsset({ + initialContent: content, + learningContextId, + validateAssetUrl: true, + }); + + expect(result).toBeFalsy(); + }); + + it('handles library keys correctly', async () => { + jest.spyOn(keyUtils, 'isLibraryKey').mockReturnValue(true); + + const content = ''; + + const result = await module.replaceStaticWithAsset({ + initialContent: content, + learningContextId: 'lib:test', + validateAssetUrl: false, + }); + + expect(result).toContain('static/test.png'); + }); + + it('returns false when asset already valid and no replacement needed', async () => { + const content = ''; + + const result = await module.replaceStaticWithAsset({ + initialContent: content, + learningContextId, + validateAssetUrl: false, + }); + + expect(result).toBe(false); + }); }); describe('setAssetToStaticUrl', () => { it('returns content with updated img links', () => { diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.ts b/src/editors/sharedComponents/TinyMceWidget/hooks.ts index 8397de1527..f0d36cbaff 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.ts +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.ts @@ -105,7 +105,7 @@ export const parseContentForLabels = ({ editor, updateContent }) => { } }; -export const replaceStaticWithAsset = ({ +export const replaceStaticWithAsset = async ({ initialContent, learningContextId, editorType, @@ -118,7 +118,7 @@ export const replaceStaticWithAsset = ({ src => src.startsWith('/static') || src.startsWith('/asset'), ); if (!isEmpty(srcs)) { - srcs.forEach(async src => { + for (const src of srcs) { const currentContent = content; let staticFullUrl; const isStatic = src.startsWith('/static/'); @@ -160,6 +160,11 @@ export const replaceStaticWithAsset = ({ // check if the asset exists on the server before replacing try { if (validateAssetUrl) { + // We intentionally await inside this loop because each replacement + // depends on the progressively updated `content` value. + // Executing the requests in parallel could introduce race conditions + // and produce inconsistent string replacements. + // eslint-disable-next-line no-await-in-loop await getAuthenticatedHttpClient() .get(staticFullUrl); } @@ -169,7 +174,7 @@ export const replaceStaticWithAsset = ({ content = currentContent; } } - }); + } if (hasChanges) { return content; } } return false; @@ -353,9 +358,9 @@ export const setupCustomBehavior = ({ onAction: toggleLabelFormatting, }); if (editorType === 'expandable') { - editor.on('init', () => { + editor.on('init', async () => { const initialContent = editor.getContent(); - const newContent = replaceStaticWithAsset({ + const newContent = await replaceStaticWithAsset({ initialContent, editorType, lmsEndpointUrl, @@ -381,10 +386,11 @@ export const setupCustomBehavior = ({ } }); - editor.on('ExecCommand', /* istanbul ignore next */ (e) => { + editor.on('ExecCommand', /* istanbul ignore next */ async (e) => { if (editorType === 'text' && e.command === 'mceFocus') { const initialContent = editor.getContent(); - const newContent = replaceStaticWithAsset({ + // @ts-ignore Some parameters like 'lmsEndpointUrl' were missing here. Fix me? + const newContent = await replaceStaticWithAsset({ initialContent, editorType, lmsEndpointUrl, diff --git a/src/schedule-and-details/data/apiHooks.ts b/src/schedule-and-details/data/apiHooks.ts index 8f0147574b..9b65a2f5c0 100644 --- a/src/schedule-and-details/data/apiHooks.ts +++ b/src/schedule-and-details/data/apiHooks.ts @@ -1,32 +1,24 @@ -import { useQuery, useQueryClient } from '@tanstack/react-query'; - +import { useQuery } from '@tanstack/react-query'; import { getCourseDetails } from './api'; /** * Get the details of a course. */ export const useGetCourseDetails = (courseId?: string) => { - const queryClient = useQueryClient(); - const { - data, isLoading, isError, refetch, + data, isLoading, isError, refetch, dataUpdatedAt } = useQuery({ queryKey: ['courseDetails', courseId], queryFn: () => getCourseDetails(courseId), + enabled: !!courseId, refetchOnWindowFocus: false, }); - let globalDefaults: { [key: string]: any } | 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(['courseDetails', undefined]); - } return { - ...globalDefaults, ...data, id: courseId, isLoading, isError, refetch, + dataUpdatedAt, }; }; diff --git a/src/schedule-and-details/hooks.jsx b/src/schedule-and-details/hooks.jsx index e5d8b8b3f0..25e208a9ef 100644 --- a/src/schedule-and-details/hooks.jsx +++ b/src/schedule-and-details/hooks.jsx @@ -52,7 +52,7 @@ const useSaveValuesPrompt = ( if (!isQueryPending && !isEditableState) { setEditedValues(initialEditedData); } - }, [initialEditedData.isLoading]); + }, [initialEditedData.dataUpdatedAt]); useEffect(() => { const errors = validateScheduleAndDetails(editedValues, canShowCertificateAvailableDateField, intl); From 3b0be94b575c6d462be277606257c739697cc06e Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 6 Mar 2026 15:28:17 -0500 Subject: [PATCH 10/11] fix: solve rebase issues --- .../sharedComponents/TinyMceWidget/hooks.test.js | 5 +++-- src/editors/sharedComponents/TinyMceWidget/hooks.ts | 1 - src/schedule-and-details/index.jsx | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js index 3a93b8bbd8..211768d069 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js @@ -228,12 +228,13 @@ describe('TinyMceEditor hooks', () => { const actual = await module.replaceStaticWithAsset({ initialContent: content, learningContextId }); expect(actual).toBeFalsy(); }); - it('does not convert static URLs with subdirectories but converts direct static files', () => { + it('does not convert static URLs with subdirectories but converts direct static files', async () => { const contentWithSubdirectory = ''; const expected = ``; - const actual = module.replaceStaticWithAsset({ + const actual = await module.replaceStaticWithAsset({ initialContent: contentWithSubdirectory, learningContextId, + validateAssetUrl: false, }); expect(actual).toEqual(expected); }); diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.ts b/src/editors/sharedComponents/TinyMceWidget/hooks.ts index f0d36cbaff..bc1c9547b5 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.ts +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.ts @@ -389,7 +389,6 @@ export const setupCustomBehavior = ({ editor.on('ExecCommand', /* istanbul ignore next */ async (e) => { if (editorType === 'text' && e.command === 'mceFocus') { const initialContent = editor.getContent(); - // @ts-ignore Some parameters like 'lmsEndpointUrl' were missing here. Fix me? const newContent = await replaceStaticWithAsset({ initialContent, editorType, diff --git a/src/schedule-and-details/index.jsx b/src/schedule-and-details/index.jsx index 7e7f25d1e9..5ded3520c6 100644 --- a/src/schedule-and-details/index.jsx +++ b/src/schedule-and-details/index.jsx @@ -9,6 +9,7 @@ import { } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; +import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; import Placeholder from '../editors/Placeholder'; import { RequestStatus } from '../data/constants'; import { useGetCourseDetails } from './data/apiHooks'; @@ -17,7 +18,6 @@ import InternetConnectionAlert from '../generic/internet-connection-alert'; import { STATEFUL_BUTTON_STATES } from '../constants'; import getPageHeadTitle from '../generic/utils'; import { useScrollToHashElement } from '../hooks'; -import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; import { fetchCourseSettingsQuery, updateCourseDetailsQuery, @@ -42,15 +42,15 @@ import { useLoadValuesPrompt, useSaveValuesPrompt } from './hooks'; const ScheduleAndDetails = () => { const intl = useIntl(); - const courseDetails = useGetCourseDetails(courseId); const courseSettings = useSelector(getCourseSettings); const loadingSettingsStatus = useSelector(getLoadingSettingsStatus); - const isLoading = courseDetails.isLoading - || loadingSettingsStatus === RequestStatus.IN_PROGRESS; - const { courseId, courseDetails: course } = useCourseAuthoringContext(); document.title = getPageHeadTitle(course?.name || '', intl.formatMessage(messages.headingTitle)); + const courseDetails = useGetCourseDetails(courseId); + const isLoading = courseDetails.isLoading + || loadingSettingsStatus === RequestStatus.IN_PROGRESS; + const { platformName, isCreditCourse, From fdbadd3abdf9f65200716283523b8ee77ff51387 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Fri, 6 Mar 2026 17:02:17 -0500 Subject: [PATCH 11/11] refactor: add custom hook to manage static replacement --- .../ExplanationWidget/index.jsx | 9 +++-- .../EditProblemView/QuestionWidget/index.jsx | 9 +++-- src/editors/containers/TextEditor/index.jsx | 9 +++-- .../containers/TextEditor/index.test.tsx | 12 +++++-- .../sharedComponents/TinyMceWidget/hooks.ts | 36 +++++++++++++++++++ 5 files changed, 57 insertions(+), 18 deletions(-) diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx index cbe42ee20c..2f734991c6 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx @@ -7,7 +7,7 @@ import { getConfig } from '@edx/frontend-platform'; import { selectors } from '../../../../../data/redux'; import messages from './messages'; import TinyMceWidget from '../../../../../sharedComponents/TinyMceWidget'; -import { prepareEditorRef, replaceStaticWithAsset } from '../../../../../sharedComponents/TinyMceWidget/hooks'; +import { prepareEditorRef, useProcessedEditorContent } from '../../../../../sharedComponents/TinyMceWidget/hooks'; const ExplanationWidget = ({ // redux @@ -19,12 +19,11 @@ const ExplanationWidget = ({ }) => { const intl = useIntl(); const { editorRef, refReady, setEditorRef } = prepareEditorRef(); - const initialContent = settings?.solutionExplanation || ''; - const newContent = replaceStaticWithAsset({ - initialContent, + + const solutionContent = useProcessedEditorContent({ + initialContent: settings?.solutionExplanation || '', learningContextId, }); - const solutionContent = newContent || initialContent; let staticRootUrl; if (isLibrary) { staticRootUrl = `${getConfig().STUDIO_BASE_URL }/library_assets/blocks/${ blockId }/`; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx index f8020721a1..44b8d877b5 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx @@ -7,7 +7,7 @@ import { getConfig } from '@edx/frontend-platform'; import { selectors } from '../../../../../data/redux'; import messages from './messages'; import TinyMceWidget from '../../../../../sharedComponents/TinyMceWidget'; -import { prepareEditorRef, replaceStaticWithAsset } from '../../../../../sharedComponents/TinyMceWidget/hooks'; +import { prepareEditorRef, useProcessedEditorContent } from '../../../../../sharedComponents/TinyMceWidget/hooks'; const QuestionWidget = ({ // redux @@ -19,12 +19,11 @@ const QuestionWidget = ({ }) => { const intl = useIntl(); const { editorRef, refReady, setEditorRef } = prepareEditorRef(); - const initialContent = question; - const newContent = replaceStaticWithAsset({ - initialContent, + + const questionContent = useProcessedEditorContent({ + initialContent: question, learningContextId, }); - const questionContent = newContent || initialContent; let staticRootUrl; if (isLibrary) { staticRootUrl = `${getConfig().STUDIO_BASE_URL }/library_assets/blocks/${ blockId }/`; diff --git a/src/editors/containers/TextEditor/index.jsx b/src/editors/containers/TextEditor/index.jsx index 19e6d518a4..4f1b5f9083 100644 --- a/src/editors/containers/TextEditor/index.jsx +++ b/src/editors/containers/TextEditor/index.jsx @@ -17,7 +17,7 @@ import RawEditor from '../../sharedComponents/RawEditor'; import * as hooks from './hooks'; import messages from './messages'; import TinyMceWidget from '../../sharedComponents/TinyMceWidget'; -import { prepareEditorRef, replaceStaticWithAsset } from '../../sharedComponents/TinyMceWidget/hooks'; +import { prepareEditorRef, useProcessedEditorContent } from '../../sharedComponents/TinyMceWidget/hooks'; const TextEditor = ({ onClose, @@ -36,13 +36,12 @@ const TextEditor = ({ }) => { const intl = useIntl(); const { editorRef, refReady, setEditorRef } = prepareEditorRef(); - const initialContent = blockValue ? blockValue.data.data : ''; - const newContent = replaceStaticWithAsset({ - initialContent, + + const editorContent = useProcessedEditorContent({ + initialContent: blockValue ? blockValue.data.data : '', learningContextId, validateAssetUrl, }); - const editorContent = newContent || initialContent; let staticRootUrl; if (isLibrary) { staticRootUrl = `${getConfig().STUDIO_BASE_URL }/library_assets/blocks/${ blockId }/`; diff --git a/src/editors/containers/TextEditor/index.test.tsx b/src/editors/containers/TextEditor/index.test.tsx index 12de26a2b1..bbf11b0739 100644 --- a/src/editors/containers/TextEditor/index.test.tsx +++ b/src/editors/containers/TextEditor/index.test.tsx @@ -1,5 +1,7 @@ import React from 'react'; -import { render, screen, initializeMocks } from '@src/testUtils'; +import { + render, screen, initializeMocks, waitFor, +} from '@src/testUtils'; import { actions, selectors } from '../../data/redux'; import { RequestKeys } from '../../data/constants/requests'; import { TextEditorInternal as TextEditor, mapStateToProps, mapDispatchToProps } from '.'; @@ -67,7 +69,7 @@ describe('TextEditor', () => { expect(element?.getAttribute('editorcontenthtml')).toBe('eDiTablE Text'); }); - test('renders static images with relative paths', () => { + test('renders static images with relative paths', async () => { const updatedProps = { ...props, validateAssetUrl: false, @@ -76,7 +78,11 @@ describe('TextEditor', () => { const { container } = render(); const element = container.querySelector('tinymcewidget'); expect(element).toBeInTheDocument(); - expect(element?.getAttribute('editorcontenthtml')).toBe('eDiTablE Text with '); + await waitFor(() => { + expect(element?.getAttribute('editorcontenthtml')).toBe( + 'eDiTablE Text with ', + ); + }); }); test('not yet loaded, Spinner appears', () => { const { container } = render(); diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.ts b/src/editors/sharedComponents/TinyMceWidget/hooks.ts index bc1c9547b5..83afbe81cf 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.ts +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.ts @@ -577,3 +577,39 @@ export const selectedImage = (val) => { setSelection, }; }; + +export const useProcessedEditorContent = ({ + initialContent, + learningContextId, + editorType, + validateAssetUrl = false, +}) => { + const [content, setContent] = useState(initialContent); + + useEffect(() => { + let mounted = true; + + const process = async () => { + const newContent = await replaceStaticWithAsset({ + initialContent, + learningContextId, + editorType, + lmsEndpointUrl: getConfig().LMS_BASE_URL, + validateAssetUrl, + }); + + if (mounted) { + setContent(newContent || initialContent); + } + }; + + // eslint-disable-next-line no-void + void process(); + + return () => { + mounted = false; + }; + }, [initialContent, learningContextId, editorType, validateAssetUrl]); + + return content; +};