Skip to content

feat: add asset existence check before replacing static URLs #1708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
18 changes: 15 additions & 3 deletions src/editors/sharedComponents/TinyMceWidget/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ 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';
Expand Down Expand Up @@ -83,14 +84,15 @@ export const replaceStaticWithAsset = ({
learningContextId,
editorType,
lmsEndpointUrl,
validateAssetUrl = true,
}) => {
let content = initialContent;
let hasChanges = false;
const srcs = content.split(/(src="|src="|href="|href=&quot)/g).filter(
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/');
Expand Down Expand Up @@ -121,8 +123,18 @@ 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 {
if (validateAssetUrl) {
await getAuthenticatedHttpClient()
.get(staticFullUrl);
}
content = currentContent.replace(currentSrc, staticFullUrl);
hasChanges = true;
} catch (e) {
content = currentContent;
}
}
});
if (hasChanges) { return content; }
Expand Down
7 changes: 6 additions & 1 deletion src/editors/sharedComponents/TinyMceWidget/hooks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,11 @@ describe('TinyMceEditor hooks', () => {
const lmsEndpointUrl = getConfig().LMS_BASE_URL;
it('returns updated src for text editor to update content', () => {
const expected = `<img src="/${baseAssetUrl}@soMEImagEURl1.jpeg"/><a href="/${baseAssetUrl}@test.pdf">test</a><img src="/${baseAssetUrl}@correct.png" /><img src="/${baseAssetUrl}@correct.png" />`;
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', () => {
Expand All @@ -205,6 +209,7 @@ describe('TinyMceEditor hooks', () => {
editorType,
lmsEndpointUrl,
learningContextId,
validateAssetUrl: false,
});
expect(actual).toEqual(expected);
});
Expand Down
32 changes: 32 additions & 0 deletions src/schedule-and-details/data/apiHooks.ts
Original file line number Diff line number Diff line change
@@ -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,
};
};
2 changes: 0 additions & 2 deletions src/schedule-and-details/data/selectors.js
Original file line number Diff line number Diff line change
@@ -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;
14 changes: 0 additions & 14 deletions src/schedule-and-details/data/slice.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,17 @@ 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);
},
Expand All @@ -36,10 +25,7 @@ const slice = createSlice({

export const {
updateSavingStatus,
updateLoadingDetailsStatus,
updateLoadingSettingsStatus,
updateCourseDetailsSuccess,
fetchCourseDetailsSuccess,
fetchCourseSettingsSuccess,
} = slice.actions;

Expand Down
25 changes: 1 addition & 24 deletions src/schedule-and-details/data/thunks.js
Original file line number Diff line number Diff line change
@@ -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 (error) {
dispatch(updateSavingStatus({ status: RequestStatus.FAILED }));
Expand Down
14 changes: 7 additions & 7 deletions src/schedule-and-details/hooks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -54,7 +52,7 @@ const useSaveValuesPrompt = (
if (!isQueryPending && !isEditableState) {
setEditedValues(initialEditedData);
}
}, [initialEditedData]);
}, [initialEditedData.isLoading]);

useEffect(() => {
const errors = validateScheduleAndDetails(editedValues, canShowCertificateAvailableDateField, intl);
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 5 additions & 8 deletions src/schedule-and-details/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { useIntl } from '@edx/frontend-platform/i18n';

import Placeholder from '../editors/Placeholder';
import { RequestStatus } from '../data/constants';
import { useGetCourseDetails } from './data/apiHooks';
import { useModel } from '../generic/model-store';
import AlertMessage from '../generic/alert-message';
import InternetConnectionAlert from '../generic/internet-connection-alert';
Expand All @@ -21,13 +22,10 @@ import getPageHeadTitle from '../generic/utils';
import { useScrollToHashElement } from '../hooks';
import {
fetchCourseSettingsQuery,
fetchCourseDetailsQuery,
updateCourseDetailsQuery,
} from './data/thunks';
import {
getCourseSettings,
getCourseDetails,
getLoadingDetailsStatus,
getLoadingSettingsStatus,
} from './data/selectors';
import BasicSection from './basic-section';
Expand All @@ -46,11 +44,10 @@ import { useLoadValuesPrompt, useSaveValuesPrompt } from './hooks';

const ScheduleAndDetails = ({ courseId }) => {
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 course = useModel('courseDetails', courseId);
Expand Down Expand Up @@ -82,8 +79,8 @@ const ScheduleAndDetails = ({ courseId }) => {
showLoadFailedAlert,
} = useLoadValuesPrompt(
courseId,
fetchCourseDetailsQuery,
fetchCourseSettingsQuery,
courseDetails.isError,
);

const {
Expand Down Expand Up @@ -146,7 +143,7 @@ const ScheduleAndDetails = ({ courseId }) => {
return <></>;
}

if (loadingDetailsStatus === RequestStatus.DENIED || loadingSettingsStatus === RequestStatus.DENIED) {
if (loadingSettingsStatus === RequestStatus.DENIED) {
return (
<div className="row justify-content-center m-6">
<Placeholder />
Expand Down
Loading