diff --git a/.env.test b/.env.test index 7ee23dd9f..0be671946 100644 --- a/.env.test +++ b/.env.test @@ -39,4 +39,5 @@ INVITE_STUDENTS_EMAIL_TO="someone@domain.com" ENABLE_HOME_PAGE_COURSE_API_V2=true ENABLE_CHECKLIST_QUALITY=true ENABLE_GRADING_METHOD_IN_PROBLEMS=false -LIBRARY_SUPPORTED_BLOCKS="problem,video,html,drag-and-drop-v2" +# "other" is used to test the workflow for creating blocks that aren't supported by the built-in editors +LIBRARY_SUPPORTED_BLOCKS="problem,video,html,drag-and-drop-v2,other" diff --git a/src/editors/containers/EditorContainer/hooks.test.jsx b/src/editors/containers/EditorContainer/hooks.test.jsx index 815a4cc12..843468594 100644 --- a/src/editors/containers/EditorContainer/hooks.test.jsx +++ b/src/editors/containers/EditorContainer/hooks.test.jsx @@ -16,6 +16,7 @@ jest.mock('../../data/redux', () => ({ app: { isInitialized: (state) => ({ isInitialized: state }), images: (state) => ({ images: state }), + shouldCreateBlock: (state) => ({ shouldCreateBlock: state }), }, requests: { isFailed: (...args) => ({ requestFailed: args }), @@ -26,6 +27,7 @@ jest.mock('../../hooks', () => ({ ...jest.requireActual('../../hooks'), navigateCallback: jest.fn((args) => ({ navigateCallback: args })), saveBlock: jest.fn((args) => ({ saveBlock: args })), + createBlock: jest.fn((args) => ({ createBlock: args })), })); const dispatch = jest.fn(); @@ -53,6 +55,8 @@ describe('EditorContainer hooks', () => { const getContent = () => 'myTestContentValue'; const setAssetToStaticUrl = () => 'myTestContentValue'; const validateEntry = () => 'vaLIdAteENTry'; + reactRedux.useSelector.mockReturnValue(false); + const output = hooks.handleSaveClicked({ getContent, images: { @@ -73,6 +77,31 @@ describe('EditorContainer hooks', () => { validateEntry, }); }); + it('returns callback to createBlock with dispatch and content if shouldCreateBlock is true', () => { + const getContent = () => 'myTestContentValue'; + const setAssetToStaticUrl = () => 'myTestContentValue'; + const validateEntry = () => 'vaLIdAteENTry'; + reactRedux.useSelector.mockReturnValue(true); + + const output = hooks.handleSaveClicked({ + getContent, + images: { + portableUrl: '/static/sOmEuiMAge.jpeg', + displayName: 'sOmEuiMAge', + }, + destination: 'testDEsTURL', + analytics: 'soMEanALytics', + dispatch, + validateEntry, + }); + output(); + expect(appHooks.createBlock).toHaveBeenCalledWith({ + content: setAssetToStaticUrl(reactRedux.useSelector(selectors.app.images), getContent), + destination: reactRedux.useSelector(selectors.app.returnUrl), + analytics: reactRedux.useSelector(selectors.app.analytics), + dispatch, + }); + }); }); describe('cancelConfirmModalToggle', () => { diff --git a/src/editors/containers/EditorContainer/hooks.ts b/src/editors/containers/EditorContainer/hooks.ts index 935e3ad89..bb0b6d53e 100644 --- a/src/editors/containers/EditorContainer/hooks.ts +++ b/src/editors/containers/EditorContainer/hooks.ts @@ -9,9 +9,11 @@ import * as appHooks from '../../hooks'; export const { clearSaveError, + clearCreateError, navigateCallback, nullMethod, saveBlock, + createBlock, } = appHooks; export const state = StrictDict({ @@ -27,10 +29,20 @@ export const handleSaveClicked = ({ }) => { // eslint-disable-next-line react-hooks/rules-of-hooks const returnUrl = useSelector(selectors.app.returnUrl); + // eslint-disable-next-line react-hooks/rules-of-hooks + const createBlockOnSave = useSelector(selectors.app.shouldCreateBlock); const destination = returnFunction ? '' : returnUrl; // eslint-disable-next-line react-hooks/rules-of-hooks const analytics = useSelector(selectors.app.analytics); - + if (createBlockOnSave) { + return () => createBlock({ + analytics, + content: getContent({ dispatch }), + destination, + dispatch, + returnFunction, + }); + } return () => saveBlock({ analytics, content: getContent({ dispatch }), @@ -79,3 +91,14 @@ export const isInitialized = () => useSelector(selectors.app.isInitialized); export const saveFailed = () => useSelector((rootState) => ( selectors.requests.isFailed(rootState, { requestKey: RequestKeys.saveBlock }) )); + +export const createFailed = () => ({ + // eslint-disable-next-line react-hooks/rules-of-hooks + createFailed: useSelector((rootState) => ( + selectors.requests.isFailed(rootState, { requestKey: RequestKeys.createBlock }) + )), + // eslint-disable-next-line react-hooks/rules-of-hooks + createFailedError: useSelector((rootState) => ( + selectors.requests.error(rootState, { requestKey: RequestKeys.createBlock }) + )), +}); diff --git a/src/editors/containers/EditorContainer/index.test.tsx b/src/editors/containers/EditorContainer/index.test.tsx index a35e4d74b..f61730626 100644 --- a/src/editors/containers/EditorContainer/index.test.tsx +++ b/src/editors/containers/EditorContainer/index.test.tsx @@ -9,6 +9,7 @@ import { import editorCmsApi from '../../data/services/cms/api'; import EditorPage from '../../EditorPage'; +import * as hooks from './hooks'; // Mock this plugins component: jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' })); @@ -17,17 +18,6 @@ jest.spyOn(editorCmsApi, 'fetchCourseImages').mockImplementation(async () => ( / { data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } } )); // Mock out the 'get ancestors' API: -jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({ - status: 200, - data: { - ancestors: [{ - id: 'block-v1:Org+TS100+24+type@vertical+block@parent', - display_name: 'You-Knit? The Test Unit', - category: 'vertical', - has_children: true, - }], - }, -})); const isDirtyMock = jest.fn(); jest.mock('../TextEditor/hooks', () => ({ @@ -60,6 +50,17 @@ describe('EditorContainer', () => { jest.spyOn(window, 'removeEventListener'); jest.spyOn(mockEvent, 'preventDefault'); Object.defineProperty(mockEvent, 'returnValue', { writable: true }); + jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({ + status: 200, + data: { + ancestors: [{ + id: 'block-v1:Org+TS100+24+type@vertical+block@parent', + display_name: 'You-Knit? The Test Unit', + category: 'vertical', + has_children: true, + }], + }, + })); }); afterEach(() => { @@ -165,4 +166,14 @@ describe('EditorContainer', () => { expect(mockEvent.preventDefault).toHaveBeenCalled(); expect(mockEvent.returnValue).toBe(true); }); + test('should display an alert when is an error creating a new block', async () => { + jest.spyOn(hooks, 'createFailed').mockImplementation(() => ({ createFailed: true, createFailedError: 'error' })); + render(); + expect(await screen.findByText(/There was an error creating the content/i)).toBeInTheDocument(); + }); + test('should display an alert when is an error saving the changes', async () => { + jest.spyOn(hooks, 'saveFailed').mockImplementation(() => true); + render(); + expect(await screen.findByText(/Error: Content save failed. Please check recent changes and try again later./i)).toBeInTheDocument(); + }); }); diff --git a/src/editors/containers/EditorContainer/index.tsx b/src/editors/containers/EditorContainer/index.tsx index bc22c360d..6fa3cfc9b 100644 --- a/src/editors/containers/EditorContainer/index.tsx +++ b/src/editors/containers/EditorContainer/index.tsx @@ -18,6 +18,9 @@ import { useEditorContext } from '../../EditorContext'; import TitleHeader from './components/TitleHeader'; import * as hooks from './hooks'; import messages from './messages'; +import { parseErrorMsg } from '../../../library-authoring/add-content/AddContentContainer'; +import libraryMessages from '../../../library-authoring/add-content/messages'; + import './index.scss'; import usePromptIfDirty from '../../../generic/promptIfDirty/usePromptIfDirty'; import CancelConfirmModal from './components/CancelConfirmModal'; @@ -81,9 +84,12 @@ const EditorContainer: React.FC = ({ const isInitialized = hooks.isInitialized(); const { isCancelConfirmOpen, openCancelConfirmModal, closeCancelConfirmModal } = hooks.cancelConfirmModalToggle(); const handleCancel = hooks.handleCancel({ onClose, returnFunction }); + const { createFailed, createFailedError } = hooks.createFailed(); const disableSave = !isInitialized; const saveFailed = hooks.saveFailed(); const clearSaveFailed = hooks.clearSaveError({ dispatch }); + const clearCreateFailed = hooks.clearCreateError({ dispatch }); + const handleSave = hooks.handleSaveClicked({ dispatch, getContent, @@ -113,6 +119,16 @@ const EditorContainer: React.FC = ({ }; return ( + {createFailed && ( + + {parseErrorMsg( + intl, + createFailedError, + libraryMessages.errorCreateMessageWithDetail, + libraryMessages.errorCreateMessage, + )} + + )} {saveFailed && ( @@ -126,6 +142,7 @@ const EditorContainer: React.FC = ({ if (returnFunction) { closeCancelConfirmModal(); } + dispatch({ type: 'resetEditor' }); }} /> diff --git a/src/editors/containers/ProblemEditor/index.test.tsx b/src/editors/containers/ProblemEditor/index.test.tsx index ab3d37095..7f58db69c 100644 --- a/src/editors/containers/ProblemEditor/index.test.tsx +++ b/src/editors/containers/ProblemEditor/index.test.tsx @@ -27,6 +27,7 @@ jest.mock('../../data/redux', () => ({ selectors: { app: { blockValue: jest.fn(state => ({ blockValue: state })), + shouldCreateBlock: jest.fn(state => ({ shouldCreateBlock: state })), }, problem: { problemType: jest.fn(state => ({ problemType: state })), @@ -104,12 +105,18 @@ describe('ProblemEditor', () => { test('blockFinished from requests.isFinished', () => { expect( mapStateToProps(testState).blockFinished, - ).toEqual(selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchBlock })); + ).toEqual( + selectors.app.shouldCreateBlock(testState) + || selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchBlock }), + ); }); test('advancedSettingsFinished from requests.isFinished', () => { expect( mapStateToProps(testState).advancedSettingsFinished, - ).toEqual(selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchAdvancedSettings })); + ).toEqual( + selectors.app.shouldCreateBlock(testState) + || selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchAdvancedSettings }), + ); }); }); describe('mapDispatchToProps', () => { diff --git a/src/editors/containers/ProblemEditor/index.tsx b/src/editors/containers/ProblemEditor/index.tsx index d8154c7e8..8f30ff949 100644 --- a/src/editors/containers/ProblemEditor/index.tsx +++ b/src/editors/containers/ProblemEditor/index.tsx @@ -65,11 +65,13 @@ const ProblemEditor: React.FC = ({ }; export const mapStateToProps = (state) => ({ - blockFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }), + blockFinished: selectors.app.shouldCreateBlock(state) + || selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }), blockFailed: selectors.requests.isFailed(state, { requestKey: RequestKeys.fetchBlock }), problemType: selectors.problem.problemType(state), blockValue: selectors.app.blockValue(state), - advancedSettingsFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchAdvancedSettings }), + advancedSettingsFinished: selectors.app.shouldCreateBlock(state) + || selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchAdvancedSettings }), }); export const mapDispatchToProps = { diff --git a/src/editors/containers/TextEditor/index.jsx b/src/editors/containers/TextEditor/index.jsx index 0546e65b0..10bab16db 100644 --- a/src/editors/containers/TextEditor/index.jsx +++ b/src/editors/containers/TextEditor/index.jsx @@ -132,7 +132,8 @@ export const mapStateToProps = (state) => ({ blockFailed: selectors.requests.isFailed(state, { requestKey: RequestKeys.fetchBlock }), blockId: selectors.app.blockId(state), showRawEditor: selectors.app.showRawEditor(state), - blockFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }), + blockFinished: selectors.app.shouldCreateBlock(state) + || selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }), learningContextId: selectors.app.learningContextId(state), images: selectors.app.images(state), isLibrary: selectors.app.isLibrary(state), diff --git a/src/editors/containers/TextEditor/index.test.jsx b/src/editors/containers/TextEditor/index.test.jsx index ea3bffc94..2b386b3bb 100644 --- a/src/editors/containers/TextEditor/index.test.jsx +++ b/src/editors/containers/TextEditor/index.test.jsx @@ -55,6 +55,7 @@ jest.mock('../../data/redux', () => ({ selectors: { app: { blockValue: jest.fn(state => ({ blockValue: state })), + shouldCreateBlock: jest.fn(state => ({ shouldCreateBlock: state })), lmsEndpointUrl: jest.fn(state => ({ lmsEndpointUrl: state })), studioEndpointUrl: jest.fn(state => ({ studioEndpointUrl: state })), showRawEditor: jest.fn(state => ({ showRawEditor: state })), @@ -126,7 +127,8 @@ describe('TextEditor', () => { test('blockFinished from requests.isFinished', () => { expect( mapStateToProps(testState).blockFinished, - ).toEqual(selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchBlock })); + ).toEqual(selectors.app.shouldCreateBlock(testState) + || selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchBlock })); }); test('learningContextId from app.learningContextId', () => { expect( diff --git a/src/editors/containers/VideoEditor/index.tsx b/src/editors/containers/VideoEditor/index.tsx index f2ad7c670..0b3018812 100644 --- a/src/editors/containers/VideoEditor/index.tsx +++ b/src/editors/containers/VideoEditor/index.tsx @@ -23,6 +23,7 @@ const VideoEditor: React.FC = ({ (state) => selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchStudioView }), ); const isLibrary = useSelector(selectors.app.isLibrary) as boolean; + const isCreateWorkflow = useSelector(selectors.app.shouldCreateBlock) as boolean; const { error, validateEntry, @@ -36,7 +37,7 @@ const VideoEditor: React.FC = ({ returnFunction={returnFunction} validateEntry={validateEntry} > - {studioViewFinished ? ( + {(isCreateWorkflow || studioViewFinished) ? (
diff --git a/src/editors/data/constants/problem.ts b/src/editors/data/constants/problem.ts index 0c26a45f0..71989d3d8 100644 --- a/src/editors/data/constants/problem.ts +++ b/src/editors/data/constants/problem.ts @@ -239,3 +239,7 @@ export const ignoredOlxAttributes = [ '@_url_name', '@_x-is-pointer-node', ] as const; + +// Useful for the block creation workflow. +export const problemTitles = new Set([...Object.values(ProblemTypes).map((problem) => problem.title), + ...Object.values(AdvanceProblems).map((problem) => problem.title)]); diff --git a/src/editors/data/constants/requests.ts b/src/editors/data/constants/requests.ts index 561965af8..f9f907bed 100644 --- a/src/editors/data/constants/requests.ts +++ b/src/editors/data/constants/requests.ts @@ -13,6 +13,7 @@ export const RequestKeys = StrictDict({ fetchImages: 'fetchImages', fetchUnit: 'fetchUnit', fetchStudioView: 'fetchStudioView', + createBlock: 'createBlock', saveBlock: 'saveBlock', uploadVideo: 'uploadVideo', allowThumbnailUpload: 'allowThumbnailUpload', @@ -25,6 +26,7 @@ export const RequestKeys = StrictDict({ checkTranscriptsForImport: 'checkTranscriptsForImport', importTranscript: 'importTranscript', uploadAsset: 'uploadAsset', + batchUploadAssets: 'batchUploadAssets', fetchAdvancedSettings: 'fetchAdvancedSettings', fetchVideoFeatures: 'fetchVideoFeatures', getHandlerUrl: 'getHandlerUrl', diff --git a/src/editors/data/redux/app/reducer.ts b/src/editors/data/redux/app/reducer.ts index d53579250..91ce05256 100644 --- a/src/editors/data/redux/app/reducer.ts +++ b/src/editors/data/redux/app/reducer.ts @@ -54,7 +54,6 @@ const app = createSlice({ images: { ...state.images, ...payload.images }, imageCount: payload.imageCount, }), - resetImages: (state) => ({ ...state, images: {}, imageCount: 0 }), setVideos: (state, { payload }) => ({ ...state, videos: payload }), setCourseDetails: (state, { payload }) => ({ ...state, courseDetails: payload }), setShowRawEditor: (state, { payload }) => ({ diff --git a/src/editors/data/redux/app/selectors.test.ts b/src/editors/data/redux/app/selectors.test.ts index b380deca4..aab64b080 100644 --- a/src/editors/data/redux/app/selectors.test.ts +++ b/src/editors/data/redux/app/selectors.test.ts @@ -88,6 +88,7 @@ describe('app selectors unit tests', () => { simpleSelectors.unitUrl, simpleSelectors.blockValue, selectors.isLibrary, + selectors.shouldCreateBlock, ]); }); describe('for library blocks', () => { @@ -98,8 +99,8 @@ describe('app selectors unit tests', () => { }; [ - [[null, truthy.blockValue, true] as [any, any, any], true] as const, - [[null, null, true] as [any, any, any], false] as const, + [[null, truthy.blockValue, true, false] as [any, any, any, any], true] as const, + [[null, null, true, false] as [any, any, any, any], false] as const, ].map(([args, expected]) => expect(cb(...args)).toEqual(expected)); }); }); @@ -112,9 +113,19 @@ describe('app selectors unit tests', () => { }; [ - [[null, truthy.blockValue, false] as [any, any, any], false] as const, - [[truthy.unitUrl, null, false] as [any, any, any], false] as const, - [[truthy.unitUrl, truthy.blockValue, false] as [any, any, any], true] as const, + [[null, truthy.blockValue, false, false] as [any, any, any, any], false] as const, + [[truthy.unitUrl, null, false, false] as [any, any, any, any], false] as const, + [[truthy.unitUrl, truthy.blockValue, false, false] as [any, any, any, any], true] as const, + ].map(([args, expected]) => expect(cb(...args)).toEqual(expected)); + }); + }); + describe('component creation workflow', () => { + it('returns true if is shouldCreateBlock is truthy', () => { + const { resultFunc: cb } = selectors.isInitialized; + + [ + [[null, null, true, true] as [any, any, any, any], true] as const, + [[null, null, true, true] as [any, any, any, any], true] as const, ].map(([args, expected]) => expect(cb(...args)).toEqual(expected)); }); }); @@ -184,4 +195,9 @@ describe('app selectors unit tests', () => { }); }); }); + describe('shouldCreateBlock', () => { + it('should return false if the editor is initialized with a blockId', () => { + expect(selectors.shouldCreateBlock.resultFunc('block-v1:', 'text')).toEqual(false); + }); + }); }); diff --git a/src/editors/data/redux/app/selectors.ts b/src/editors/data/redux/app/selectors.ts index 3b7d15102..b2fd6d6ae 100644 --- a/src/editors/data/redux/app/selectors.ts +++ b/src/editors/data/redux/app/selectors.ts @@ -1,7 +1,7 @@ import { createSelector } from 'reselect'; import type { EditorState } from '..'; import { blockTypes } from '../../constants/app'; -import { isLibraryV1Key } from '../../../../generic/key-utils'; +import { isLibraryKey, isLibraryV1Key } from '../../../../generic/key-utils'; import * as urls from '../../services/cms/urls'; export const appSelector = (state: EditorState) => state.app; @@ -47,7 +47,19 @@ export const isLibrary = createSelector( if (isLibraryV1Key(learningContextId)) { return true; } - if (blockId && blockId.startsWith('lb:')) { + if ((blockId && blockId.startsWith('lb:')) || isLibraryKey(learningContextId)) { + return true; + } + return false; + }, +); + +export const shouldCreateBlock = createSelector( + [simpleSelectors.blockId, + simpleSelectors.blockType, + ], + (blockId, blockType) => { + if (blockId === '' && blockType) { return true; } return false; @@ -59,8 +71,13 @@ export const isInitialized = createSelector( simpleSelectors.unitUrl, simpleSelectors.blockValue, isLibrary, + shouldCreateBlock, ], - (unitUrl, blockValue, isLibraryBlock) => { + (unitUrl, blockValue, isLibraryBlock, initCreateWorkflow) => { + if (initCreateWorkflow) { + return true; + } + if (isLibraryBlock) { return !!blockValue; } @@ -105,4 +122,5 @@ export default { displayTitle, analytics, isLibrary, + shouldCreateBlock, }; diff --git a/src/editors/data/redux/index.ts b/src/editors/data/redux/index.ts index 775866296..3606f99aa 100644 --- a/src/editors/data/redux/index.ts +++ b/src/editors/data/redux/index.ts @@ -12,7 +12,7 @@ import { AdvancedProblemType, ProblemType } from '../constants/problem'; export { default as thunkActions } from './thunkActions'; -const rootReducer = combineReducers({ +const editorReducer = combineReducers({ app: app.reducer, requests: requests.reducer, video: video.reducer, @@ -20,6 +20,14 @@ const rootReducer = combineReducers({ game: game.reducer, }); +const rootReducer = (state: any, action: any) => { + if (action.type === 'resetEditor') { + return editorReducer(undefined, action); + } + + return editorReducer(state, action); +}; + const actions = StrictDict({ app: app.actions, requests: requests.actions, diff --git a/src/editors/data/redux/requests/reducer.js b/src/editors/data/redux/requests/reducer.js index 583c39064..4383be7a6 100644 --- a/src/editors/data/redux/requests/reducer.js +++ b/src/editors/data/redux/requests/reducer.js @@ -8,6 +8,7 @@ const initialState = { [RequestKeys.fetchUnit]: { status: RequestStates.inactive }, [RequestKeys.fetchBlock]: { status: RequestStates.inactive }, [RequestKeys.fetchStudioView]: { status: RequestStates.inactive }, + [RequestKeys.createBlock]: { status: RequestStates.inactive }, [RequestKeys.saveBlock]: { status: RequestStates.inactive }, [RequestKeys.uploadAsset]: { status: RequestStates.inactive }, [RequestKeys.allowThumbnailUpload]: { status: RequestStates.inactive }, diff --git a/src/editors/data/redux/thunkActions/app.js b/src/editors/data/redux/thunkActions/app.js index 09ab9eb61..8da9f24b7 100644 --- a/src/editors/data/redux/thunkActions/app.js +++ b/src/editors/data/redux/thunkActions/app.js @@ -1,12 +1,11 @@ import { StrictDict, camelizeKeys } from '../../../utils'; -import { isLibraryKey } from '../../../../generic/key-utils'; import * as requests from './requests'; // This 'module' self-import hack enables mocking during tests. // See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested // should be re-thought and cleaned up to avoid this pattern. // eslint-disable-next-line import/no-self-import import * as module from './app'; -import { actions as appActions } from '../app'; +import { actions as appActions, selectors } from '../app'; import { actions as requestsActions } from '../requests'; import { RequestKeys } from '../../constants/requests'; @@ -88,7 +87,14 @@ export const fetchCourseDetails = () => (dispatch) => { */ export const initialize = (data) => (dispatch) => { const editorType = data.blockType; + dispatch({ type: 'resetEditor' }); dispatch(actions.app.initialize(data)); + + if (data.blockId === '' && editorType) { + dispatch(actions.app.initializeEditor()); + return; + } + dispatch(module.fetchBlock()); if (data.blockId?.startsWith('block-v1:')) { dispatch(module.fetchUnit()); @@ -103,7 +109,6 @@ export const initialize = (data) => (dispatch) => { dispatch(module.fetchCourseDetails()); break; case 'html': - if (isLibraryKey(data.learningContextId)) { dispatch(actions.app.resetImages()); } dispatch(module.fetchImages({ pageNumber: 0 })); break; default: @@ -125,7 +130,54 @@ export const saveBlock = (content, returnToUnit) => (dispatch) => { })); }; -export const uploadAsset = ({ file, setSelection }) => (dispatch) => { +/** + * @param {func} onSuccess + */ +export const createBlock = (content, returnToUnit) => (dispatch, getState) => { + dispatch(requests.createBlock({ + onSuccess: (response) => { + dispatch(actions.app.setBlockId(response.id)); + const newImages = Object.values(selectors.images(getState())).map((image) => image.file); + + if (newImages.length === 0) { + dispatch(saveBlock(content, returnToUnit)); + return; + } + dispatch(requests.batchUploadAssets({ + assets: newImages, + content, + onSuccess: (updatedContent) => dispatch(saveBlock(updatedContent, returnToUnit)), + onFailure: (error) => dispatch(actions.requests.failRequest({ + requestKey: RequestKeys.batchUploadAssets, + error, + })), + })); + }, + onFailure: (error) => dispatch(actions.requests.failRequest({ + requestKey: RequestKeys.createBlock, + error, + })), + })); +}; + +export const uploadAsset = ({ file, setSelection }) => (dispatch, getState) => { + if (selectors.shouldCreateBlock(getState())) { + const tempFileURL = URL.createObjectURL(file); + const tempImage = { + displayName: file.name, + url: tempFileURL, + externalUrl: tempFileURL, + portableUrl: tempFileURL, + thumbnail: tempFileURL, + id: file.name, + locked: false, + file, + }; + setSelection(tempImage); + dispatch(appActions.setImages({ images: { [file.name]: tempImage }, imageCount: 1 })); + return; + } + dispatch(requests.uploadAsset({ asset: file, onSuccess: (response) => setSelection(camelizeKeys(response.data.asset)), @@ -142,4 +194,5 @@ export default StrictDict({ saveBlock, fetchImages, uploadAsset, + createBlock, }); diff --git a/src/editors/data/redux/thunkActions/app.test.js b/src/editors/data/redux/thunkActions/app.test.js index 5cf52f794..35debc7f3 100644 --- a/src/editors/data/redux/thunkActions/app.test.js +++ b/src/editors/data/redux/thunkActions/app.test.js @@ -1,14 +1,17 @@ /* eslint-disable no-import-assign */ import { actions } from '..'; import { camelizeKeys } from '../../../utils'; +import { mockImageData } from '../../constants/mockData'; import { RequestKeys } from '../../constants/requests'; import * as thunkActions from './app'; jest.mock('./requests', () => ({ fetchBlock: (args) => ({ fetchBlock: args }), fetchUnit: (args) => ({ fetchUnit: args }), + createBlock: (args) => ({ createBlock: args }), saveBlock: (args) => ({ saveBlock: args }), uploadAsset: (args) => ({ uploadAsset: args }), + batchUploadAssets: (args) => ({ batchUploadAssets: args }), fetchStudioView: (args) => ({ fetchStudioView: args }), fetchImages: (args) => ({ fetchImages: args }), fetchVideos: (args) => ({ fetchVideos: args }), @@ -30,8 +33,10 @@ const testValue = { describe('app thunkActions', () => { let dispatch; let dispatchedAction; + let getState; beforeEach(() => { dispatch = jest.fn((action) => ({ dispatch: action })); + getState = jest.fn().mockImplementation(() => ({ app: { blockId: 'blockId', images: {} } })); }); describe('fetchBlock', () => { beforeEach(() => { @@ -185,6 +190,7 @@ describe('app thunkActions', () => { thunkActions.fetchCourseDetails = () => 'fetchCourseDetails'; thunkActions.initialize(testValue)(dispatch); expect(dispatch.mock.calls).toEqual([ + [{ type: 'resetEditor' }], [actions.app.initialize(testValue)], [thunkActions.fetchBlock()], ]); @@ -196,6 +202,22 @@ describe('app thunkActions', () => { thunkActions.fetchCourseDetails = fetchCourseDetails; }); }); + describe('initialize without block id but block type defined', () => { + it('dispatches actions.app.initialize, and then fetches both block and unit', () => { + const data = { + ...testValue, + blockType: 'html', + blockId: '', + learningContextId: 'course-v1:UniversityX+PHYS+1', + }; + thunkActions.initialize(data)(dispatch); + expect(dispatch.mock.calls).toEqual([ + [{ type: 'resetEditor' }], + [actions.app.initialize(data)], + [actions.app.initializeEditor()], + ]); + }); + }); describe('initialize with block type html', () => { it('dispatches actions.app.initialize, and then fetches both block and unit', () => { const { @@ -220,6 +242,7 @@ describe('app thunkActions', () => { }; thunkActions.initialize(data)(dispatch); expect(dispatch.mock.calls).toEqual([ + [{ type: 'resetEditor' }], [actions.app.initialize(data)], [thunkActions.fetchBlock()], [thunkActions.fetchUnit()], @@ -257,6 +280,7 @@ describe('app thunkActions', () => { }; thunkActions.initialize(data)(dispatch); expect(dispatch.mock.calls).toEqual([ + [{ type: 'resetEditor' }], [actions.app.initialize(data)], [thunkActions.fetchBlock()], [thunkActions.fetchUnit()], @@ -294,6 +318,7 @@ describe('app thunkActions', () => { }; thunkActions.initialize(data)(dispatch); expect(dispatch.mock.calls).toEqual([ + [{ type: 'resetEditor' }], [actions.app.initialize(data)], [thunkActions.fetchBlock()], [thunkActions.fetchUnit()], @@ -333,21 +358,73 @@ describe('app thunkActions', () => { expect(returnToUnit).toHaveBeenCalled(); }); }); - describe('uploadAsset', () => { - const setSelection = jest.fn(); + describe('createBlock', () => { + let returnToUnit; beforeEach(() => { - thunkActions.uploadAsset({ file: testValue, setSelection })(dispatch); + returnToUnit = jest.fn(); + thunkActions.createBlock(testValue, returnToUnit)(dispatch, getState); [[dispatchedAction]] = dispatch.mock.calls; }); + it('dispatches createBlock', () => { + expect(dispatchedAction.createBlock).not.toBe(undefined); + }); + test('onSuccess: calls setBlockId and dispatches saveBlock', () => { + const data = { id: 'block-id' }; + dispatchedAction.createBlock.onSuccess(data); + expect(dispatch).toHaveBeenCalledWith(actions.app.setBlockId(data.id)); + expect(dispatch.mock.calls.length).toBe(3); + }); + test('onFailure: dispatches failRequest', () => { + const error = new Error('fail create a new component'); + dispatchedAction.createBlock.onFailure(error); + expect(dispatch).toHaveBeenCalledWith(actions.requests.failRequest({ + requestKey: RequestKeys.createBlock, + error, + })); + }); + test('should call batchUploadAssets if the block has images', () => { + mockImageData.map(image => ({ ...image, file: 'file' })); + getState.mockReturnValueOnce({ app: { blockId: '', images: mockImageData } }); + const data = { id: 'block-id' }; + dispatchedAction.createBlock.onSuccess(data); + expect(dispatch).toHaveBeenCalledWith(actions.app.setBlockId(data.id)); + expect(dispatch.mock.calls[2][0]).toEqual({ + [RequestKeys.batchUploadAssets]: { + assets: mockImageData.map(image => image.file), + content: { data: expect.any(Object) }, + onSuccess: expect.any(Function), + onFailure: expect.any(Function), + }, + }); + }); + }); + describe('uploadAsset', () => { + const setSelection = jest.fn(); + it('dispatches uploadAsset action', () => { + thunkActions.uploadAsset({ file: testValue, setSelection })(dispatch, getState); + [[dispatchedAction]] = dispatch.mock.calls; expect(dispatchedAction.uploadAsset).not.toBe(undefined); }); test('passes file as image prop', () => { + thunkActions.uploadAsset({ file: testValue, setSelection })(dispatch, getState); + [[dispatchedAction]] = dispatch.mock.calls; expect(dispatchedAction.uploadAsset.asset).toEqual(testValue); }); test('onSuccess: calls setSelection with camelized response.data.asset', () => { + thunkActions.uploadAsset({ file: testValue, setSelection })(dispatch, getState); + [[dispatchedAction]] = dispatch.mock.calls; dispatchedAction.uploadAsset.onSuccess({ data: { asset: testValue } }); expect(setSelection).toHaveBeenCalledWith(camelizeKeys(testValue)); }); + test('should save a new image temporary when is create workflow', () => { + getState.mockImplementation(() => ({ app: { blockId: '', blockType: 'html' } })); + global.URL.createObjectURL = jest.fn(); + thunkActions.uploadAsset({ file: testValue, setSelection })(dispatch, getState); + [[dispatchedAction]] = dispatch.mock.calls; + expect(URL.createObjectURL).toHaveBeenCalledWith(testValue); + expect(setSelection).toHaveBeenCalled(); + expect(dispatch).toHaveBeenCalledWith(actions.app.setImages({ images: expect.any(Object), imageCount: 1 })); + }); }); }); diff --git a/src/editors/data/redux/thunkActions/problem.ts b/src/editors/data/redux/thunkActions/problem.ts index 79916195e..cd9567d40 100644 --- a/src/editors/data/redux/thunkActions/problem.ts +++ b/src/editors/data/redux/thunkActions/problem.ts @@ -84,7 +84,7 @@ export const fetchAdvancedSettings = ({ rawOLX, rawSettings }) => (dispatch) => }; export const initializeProblem = (blockValue) => (dispatch, getState) => { - const rawOLX = _.get(blockValue, 'data.data', {}); + const rawOLX = _.get(blockValue, 'data.data', ''); const rawSettings = _.get(blockValue, 'data.metadata', {}); const learningContextId = selectors.app.learningContextId(getState()); if (isLibraryKey(learningContextId)) { diff --git a/src/editors/data/redux/thunkActions/requests.js b/src/editors/data/redux/thunkActions/requests.js index 3dd466347..335e98c60 100644 --- a/src/editors/data/redux/thunkActions/requests.js +++ b/src/editors/data/redux/thunkActions/requests.js @@ -1,3 +1,5 @@ +import { v4 as uuid4 } from 'uuid'; + import { StrictDict, parseLibraryImageData, getLibraryImageAssets } from '../../../utils'; import { RequestKeys } from '../../constants/requests'; @@ -12,7 +14,10 @@ import { selectors as videoSelectors } from '../video'; // eslint-disable-next-line import/no-self-import import * as module from './requests'; import { isLibraryKey } from '../../../../generic/key-utils'; +import { createLibraryBlock } from '../../../../library-authoring/data/api'; import { acceptedImgKeys } from '../../../sharedComponents/ImageUploadModal/SelectImageModal/utils'; +import { blockTypes } from '../../constants/app'; +import { problemTitles } from '../../constants/problem'; // Similar to `import { actions, selectors } from '..';` but avoid circular imports: const actions = { requests: requestsActions }; @@ -123,9 +128,69 @@ export const saveBlock = ({ content, ...rest }) => (dispatch, getState) => { ...rest, })); }; + +/** + * Tracked createBlock api method. Tracked to the `createBlock` request key. + * @param {[func]} onSuccess - onSuccess method ((response) => { ... }) + * @param {[func]} onFailure - onFailure method ((error) => { ... }) + */ +export const createBlock = ({ ...rest }) => (dispatch, getState) => { + const blockTitle = selectors.app.blockTitle(getState()); + const blockType = selectors.app.blockType(getState()); + // Remove any special character, a slug should be created with unicode letters, numbers, underscores or hyphens. + const cleanTitle = blockTitle?.toLowerCase().replace(/[^a-zA-Z0-9_\s-]/g, '').trim(); + let definitionId; + // Validates if the title has been assigned by the user, if not a UUID is returned as the key. + if (!cleanTitle || (blockType === blockTypes.problem && problemTitles.has(blockTitle))) { + definitionId = `${uuid4()}`; + } else { + // add a short random suffix to prevent conflicting IDs. + const suffix = uuid4().split('-')[4]; + definitionId = `${cleanTitle.replaceAll(/\s+/g, '-')}-${suffix}`; + } + dispatch(module.networkRequest({ + requestKey: RequestKeys.createBlock, + promise: createLibraryBlock({ + libraryId: selectors.app.learningContextId(getState()), + blockType, + definitionId, + }), + ...rest, + })); +}; + +// exportend only for test +export const removeTemporalLink = (response, asset, content, resolve) => { + const imagePath = `/${response.data.asset.portableUrl}`; + const reader = new FileReader(); + reader.addEventListener('load', () => { + const imageBS64 = reader.result.toString(); + const parsedContent = typeof content === 'string' ? content.replace(imageBS64, imagePath) : { ...content, olx: content.olx.replace(imageBS64, imagePath) }; + URL.revokeObjectURL(asset); + resolve(parsedContent); + }); + reader.readAsDataURL(asset); +}; + +export const batchUploadAssets = ({ assets, content, ...rest }) => (dispatch) => { + const promises = assets.reduce((promiseChain, asset) => promiseChain + .then((parsedContent) => new Promise((resolve) => { + dispatch(module.uploadAsset({ + asset, + onSuccess: (response) => removeTemporalLink(response, asset, parsedContent, resolve), + })); + })), Promise.resolve(content)); + + dispatch(module.networkRequest({ + requestKey: RequestKeys.batchUploadAssets, + promise: promises, + ...rest, + })); +}; + export const uploadAsset = ({ asset, ...rest }) => (dispatch, getState) => { const learningContextId = selectors.app.learningContextId(getState()); - dispatch(module.networkRequest({ + return dispatch(module.networkRequest({ requestKey: RequestKeys.uploadAsset, promise: api.uploadAsset({ learningContextId, @@ -424,6 +489,7 @@ export default StrictDict({ fetchBlock, fetchStudioView, fetchUnit, + createBlock, saveBlock, fetchImages, fetchVideos, diff --git a/src/editors/data/redux/thunkActions/requests.test.js b/src/editors/data/redux/thunkActions/requests.test.js index 27f288685..82bf74cfc 100644 --- a/src/editors/data/redux/thunkActions/requests.test.js +++ b/src/editors/data/redux/thunkActions/requests.test.js @@ -3,6 +3,7 @@ import { RequestKeys } from '../../constants/requests'; import api from '../../services/cms/api'; import * as requests from './requests'; import { actions, selectors } from '../index'; +import { createLibraryBlock } from '../../../../library-authoring/data/api'; const testState = { some: 'data', @@ -18,7 +19,7 @@ jest.mock('../app/selectors', () => ({ blockId: (state) => ({ blockId: state }), blockType: (state) => ({ blockType: state }), learningContextId: (state) => ({ learningContextId: state }), - blockTitle: (state) => ({ title: state }), + blockTitle: (state) => state.some, isLibrary: (state) => (state.isLibrary), })); @@ -51,6 +52,10 @@ jest.mock('../../services/cms/api', () => ({ uploadVideo: (args) => args, })); +jest.mock('../../../../library-authoring/data/api', () => ({ + createLibraryBlock: ({ id, url }) => ({ id, url }), +})); + jest.mock('../../../utils', () => ({ ...jest.requireActual('../../../utils'), parseLibraryImageData: jest.fn(), @@ -360,6 +365,22 @@ describe('requests thunkActions module', () => { }, }); }); + describe('createBlock', () => { + testNetworkRequestAction({ + action: requests.createBlock, + args: { ...fetchParams }, + expectedString: 'with create promise', + expectedData: { + ...fetchParams, + requestKey: RequestKeys.createBlock, + promise: createLibraryBlock({ + libraryId: selectors.app.learningContextId(testState), + blockType: selectors.app.blockType(testState), + }), + }, + }); + }); + describe('uploadAsset', () => { const asset = 'SoME iMage CoNtent As String'; let uploadAsset; @@ -418,6 +439,79 @@ describe('requests thunkActions module', () => { }); }); + describe('batchUploadAssets', () => { + const assets = [new Blob(['file1']), new Blob(['file2'])]; + testNetworkRequestAction({ + action: requests.batchUploadAssets, + args: { ...fetchParams, assets }, + expectedString: 'with upload batch assets promise', + expectedData: { + ...fetchParams, + requestKey: RequestKeys.batchUploadAssets, + promise: assets.reduce((acc, asset) => acc.then(() => requests.uploadAsset({ + asset, + learningContextId: selectors.app.learningContextId(testState), + studioEndpointUrl: selectors.app.studioEndpointUrl(testState), + })), Promise.resolve()), + }, + }); + describe('removeTemporalLink', () => { + let response; + let asset; + let content; + + beforeEach(() => { + response = { + data: { + asset: { + portableUrl: 'image/test.jpg', + }, + }, + }; + + asset = new Blob(['image data'], { type: 'image/jpeg' }); + content = 'Content with an image: data:image/jpeg;base64,TESTBASE64'; + }); + + it('should replace the base64 image with the image path in the content', (done) => { + /* const onLoad = jest.fn(); + const readAsDataURLMock = jest.fn(() => { + this.result = 'data:image/jpeg;base64,TESTBASE64'; + onLoad(); + }); */ + class FileReaderMock { + constructor() { + this.result = ''; + this.onload = null; + } + + addEventListener(event, callback) { + if (event === 'load') { + this.onLoad = callback; + } + } + + readAsDataURL() { + this.result = 'data:image/jpeg;base64,TESTBASE64'; + this.onLoad(); + } + } + global.FileReader = FileReaderMock; + global.URL.revokeObjectURL = jest.fn(); + const resolveMock = jest.fn((parsedContent) => { + // Assert that the content has been replaced correctly + expect(parsedContent).toEqual('Content with an image: /image/test.jpg'); + done(); + }); + + requests.removeTemporalLink(response, asset, content, resolveMock); + + // expect(global.FileReader.readAsDataURLMock).toHaveBeenCalledWith(asset); + expect(global.URL.revokeObjectURL).toHaveBeenCalledWith(asset); + }); + }); + }); + describe('uploadThumbnail', () => { const thumbnail = 'SoME tHumbNAil CoNtent As String'; const videoId = 'SoME VidEOid CoNtent As String'; diff --git a/src/editors/data/redux/thunkActions/video.js b/src/editors/data/redux/thunkActions/video.js index 0ea4d84ce..c84a3913c 100644 --- a/src/editors/data/redux/thunkActions/video.js +++ b/src/editors/data/redux/thunkActions/video.js @@ -17,8 +17,8 @@ const selectors = { app: appSelectors, video: videoSelectors }; export const loadVideoData = (selectedVideoId, selectedVideoUrl) => (dispatch, getState) => { const state = getState(); - const blockValueData = state.app.blockValue.data; - let rawVideoData = blockValueData.metadata ? blockValueData.metadata : {}; + const blockValueData = state.app?.blockValue?.data; + let rawVideoData = blockValueData?.metadata ? blockValueData.metadata : {}; const rawVideos = Object.values(selectors.app.videos(state)); if (selectedVideoId !== undefined && selectedVideoId !== null) { const selectedVideo = _.find(rawVideos, video => { diff --git a/src/editors/hooks.test.jsx b/src/editors/hooks.test.jsx index 103577a98..d1a2f1120 100644 --- a/src/editors/hooks.test.jsx +++ b/src/editors/hooks.test.jsx @@ -25,6 +25,7 @@ jest.mock('./data/redux', () => ({ app: { initialize: (args) => ({ initializeApp: args }), saveBlock: (args) => ({ saveBlock: args }), + createBlock: (args) => ({ createBlock: args }), }, }, })); @@ -165,6 +166,45 @@ describe('hooks', () => { }); }); + describe('createBlock', () => { + const navigateCallback = (args) => ({ navigateCallback: args }); + const dispatch = jest.fn(); + const destination = 'uRLwhENsAved'; + const analytics = 'dATAonEveNT'; + + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(hooks, hookKeys.navigateCallback).mockImplementationOnce(navigateCallback); + }); + it('returns null when content is null', () => { + const content = null; + const expected = hooks.createBlock({ + content, + destination, + analytics, + dispatch, + }); + expect(expected).toEqual(undefined); + }); + it('dispatches thunkActions.app.createBlock with navigateCallback, and passed content', () => { + const content = 'myContent'; + hooks.createBlock({ + content, + destination, + analytics, + dispatch, + }); + expect(dispatch).toHaveBeenCalledWith(thunkActions.app.createBlock( + content, + navigateCallback({ + destination, + analyticsEvent: analyticsEvt.editorSaveClick, + analytics, + }), + )); + }); + }); + describe('clearSaveError', () => { it('dispatches actions.requests.clearRequest with saveBlock requestKey', () => { const dispatch = jest.fn(); @@ -174,4 +214,14 @@ describe('hooks', () => { })); }); }); + + describe('clearCreateError', () => { + it('dispatches actions.requests.clearRequest with createBlock requestKey', () => { + const dispatch = jest.fn(); + hooks.clearCreateError({ dispatch })(); + expect(dispatch).toHaveBeenCalledWith(actions.requests.clearRequest({ + requestKey: RequestKeys.createBlock, + })); + }); + }); }); diff --git a/src/editors/hooks.ts b/src/editors/hooks.ts index c32d60800..887467534 100644 --- a/src/editors/hooks.ts +++ b/src/editors/hooks.ts @@ -65,7 +65,30 @@ export const saveBlock = ({ )); } }; - +export const createBlock = ({ + analytics, + content, + destination, + dispatch, + returnFunction, +}) => { + if (!content) { + return; + } + dispatch(thunkActions.app.createBlock( + content, + navigateCallback({ + destination, + analyticsEvent: analyticsEvt.editorSaveClick, + analytics, + returnFunction, + }), + )); +}; export const clearSaveError = ({ dispatch, }) => () => dispatch(actions.requests.clearRequest({ requestKey: RequestKeys.saveBlock })); + +export const clearCreateError = ({ + dispatch, +}) => () => dispatch(actions.requests.clearRequest({ requestKey: RequestKeys.createBlock })); diff --git a/src/editors/sharedComponents/ImageUploadModal/index.jsx b/src/editors/sharedComponents/ImageUploadModal/index.jsx index fea13225c..e72f4a56d 100644 --- a/src/editors/sharedComponents/ImageUploadModal/index.jsx +++ b/src/editors/sharedComponents/ImageUploadModal/index.jsx @@ -200,7 +200,7 @@ ImageUploadModal.propTypes = { images: PropTypes.shape({}).isRequired, lmsEndpointUrl: PropTypes.string.isRequired, editorType: PropTypes.string, - isLibrary: PropTypes.string, + isLibrary: PropTypes.bool, }; export const ImageUploadModalInternal = ImageUploadModal; // For testing only diff --git a/src/library-authoring/add-content/AddContentContainer.test.tsx b/src/library-authoring/add-content/AddContentContainer.test.tsx index 1f8a785f7..63b42895b 100644 --- a/src/library-authoring/add-content/AddContentContainer.test.tsx +++ b/src/library-authoring/add-content/AddContentContainer.test.tsx @@ -1,5 +1,4 @@ import MockAdapter from 'axios-mock-adapter/types'; -import { snakeCaseObject } from '@edx/frontend-platform'; import { fireEvent, render as baseRender, @@ -7,9 +6,10 @@ import { waitFor, initializeMocks, } from '../../testUtils'; -import { mockContentLibrary } from '../data/api.mocks'; +import { mockContentLibrary, mockXBlockFields } from '../data/api.mocks'; import { getContentLibraryApiUrl, getCreateLibraryBlockUrl, getLibraryCollectionComponentApiUrl, getLibraryPasteClipboardUrl, + getXBlockFieldsApiUrl, } from '../data/api'; import { mockBroadcastChannel, mockClipboardEmpty, mockClipboardHtml } from '../../generic/data/api.mock'; import { LibraryProvider } from '../common/context/LibraryContext'; @@ -17,8 +17,10 @@ import AddContentContainer from './AddContentContainer'; import { ComponentEditorModal } from '../components/ComponentEditorModal'; import editorCmsApi from '../../editors/data/services/cms/api'; import { ToastActionData } from '../../generic/toast-context'; +import * as textEditorHooks from '../../editors/containers/TextEditor/hooks'; mockBroadcastChannel(); +// mockCreateLibraryBlock.applyMock(); // Mocks for ComponentEditorModal to work in tests. jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' })); @@ -48,6 +50,7 @@ describe('', () => { axiosMock = mocks.axiosMock; mockShowToast = mocks.mockShowToast; axiosMock.onGet(getContentLibraryApiUrl(libraryId)).reply(200, {}); + jest.spyOn(textEditorHooks, 'getContent').mockImplementation(() => () => '

Edited HTML content

'); }); afterEach(() => { jest.restoreAllMocks(); @@ -61,11 +64,11 @@ describe('', () => { expect(screen.queryByRole('button', { name: /open reponse/i })).not.toBeInTheDocument(); // Excluded from MVP expect(screen.queryByRole('button', { name: /drag drop/i })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: /video/i })).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /advanced \/ other/i })).not.toBeInTheDocument(); // Excluded from MVP + expect(screen.queryByRole('button', { name: /advanced \/ other/i })).toBeInTheDocument(); // To test not editor supported blocks expect(screen.queryByRole('button', { name: /copy from clipboard/i })).not.toBeInTheDocument(); }); - it('should create a content', async () => { + it('should open the editor modal to create a content when the block is supported', async () => { mockClipboardEmpty.applyMock(); const url = getCreateLibraryBlockUrl(libraryId); axiosMock.onPost(url).reply(200); @@ -74,7 +77,16 @@ describe('', () => { const textButton = screen.getByRole('button', { name: /text/i }); fireEvent.click(textButton); + expect(await screen.findByRole('heading', { name: /Text/ })).toBeInTheDocument(); + }); + it('should create a component when the block is not supported by the editor', async () => { + mockClipboardEmpty.applyMock(); + const url = getCreateLibraryBlockUrl(libraryId); + axiosMock.onPost(url).reply(200); + render(); + const textButton = screen.getByRole('button', { name: /other/i }); + fireEvent.click(textButton); await waitFor(() => expect(axiosMock.history.post[0].url).toEqual(url)); await waitFor(() => expect(axiosMock.history.patch.length).toEqual(0)); }); @@ -87,13 +99,14 @@ describe('', () => { libraryId, collectionId, ); - // having id of block which is not video, html or problem will not trigger editor. + axiosMock.onPost(url).reply(200, { id: 'some-component-id' }); axiosMock.onPatch(collectionComponentUrl).reply(200); render(collectionId); - const textButton = screen.getByRole('button', { name: /text/i }); + // Select a block that is not supported by the editor should create the component + const textButton = screen.getByRole('button', { name: /other/i }); fireEvent.click(textButton); await waitFor(() => expect(axiosMock.history.post[0].url).toEqual(url)); @@ -105,6 +118,8 @@ describe('', () => { mockClipboardEmpty.applyMock(); const collectionId = 'some-collection-id'; const url = getCreateLibraryBlockUrl(libraryId); + const usageKey = mockXBlockFields.usageKeyNewHtml; + const updateBlockUrl = getXBlockFieldsApiUrl(usageKey); const collectionComponentUrl = getLibraryCollectionComponentApiUrl( libraryId, collectionId, @@ -113,39 +128,20 @@ describe('', () => { jest.spyOn(editorCmsApi, 'fetchCourseImages').mockImplementation(async () => ( // eslint-disable-next-line { data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } } )); - jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({ - status: 200, - data: { - ancestors: [{ - id: 'block-v1:Org+TS100+24+type@vertical+block@parent', - display_name: 'You-Knit? The Test Unit', - category: 'vertical', - has_children: true, - }], - }, - })); - axiosMock.onPost(url).reply(200, { - id: 'lb:OpenedX:CSPROB2:html:1a5efd56-4ee5-4df0-b466-44f08fbbf567', + id: usageKey, }); - const fieldsHtml = { - displayName: 'Introduction to Testing', - data: '

This is a text component which uses HTML.

', - metadata: { displayName: 'Introduction to Testing' }, - }; - jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( - { status: 200, data: snakeCaseObject(fieldsHtml) } - )); - axiosMock.onPatch(collectionComponentUrl).reply(200); + axiosMock.onPost(updateBlockUrl).reply(200, mockXBlockFields.dataHtml); + axiosMock.onPatch(collectionComponentUrl).reply(200); render(collectionId); const textButton = screen.getByRole('button', { name: /text/i }); fireEvent.click(textButton); - // Component should be linked to Collection on closing editor. - const closeButton = await screen.findByRole('button', { name: 'Exit the editor' }); - fireEvent.click(closeButton); + // Component should be linked to Collection on saving the changes in the editor. + const saveButton = screen.getByLabelText('Save changes and return to learning context'); + fireEvent.click(saveButton); await waitFor(() => expect(axiosMock.history.post[0].url).toEqual(url)); await waitFor(() => expect(axiosMock.history.patch.length).toEqual(1)); await waitFor(() => expect(axiosMock.history.patch[0].url).toEqual(collectionComponentUrl)); @@ -259,20 +255,6 @@ describe('', () => { expectedError: 'There was an error pasting the content: library cannot have more than 100000 components', buttonName: /paste from clipboard/i, }, - { - label: 'should handle failure to create content', - mockUrl: getCreateLibraryBlockUrl(libraryId), - mockResponse: undefined, - expectedError: 'There was an error creating the content.', - buttonName: /text/i, - }, - { - label: 'should show detailed error in toast on create failure', - mockUrl: getCreateLibraryBlockUrl(libraryId), - mockResponse: 'library cannot have more than 100000 components', - expectedError: 'There was an error creating the content: library cannot have more than 100000 components', - buttonName: /text/i, - }, ])('$label', async ({ mockUrl, mockResponse, buttonName, expectedError, }) => { diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx index 1a2e28b1a..d47f33e7e 100644 --- a/src/library-authoring/add-content/AddContentContainer.tsx +++ b/src/library-authoring/add-content/AddContentContainer.tsx @@ -26,8 +26,8 @@ import { useCopyToClipboard } from '../../generic/clipboard'; import { getCanEdit } from '../../course-unit/data/selectors'; import { useCreateLibraryBlock, useLibraryPasteClipboard, useAddComponentsToCollection } from '../data/apiHooks'; import { useLibraryContext } from '../common/context/LibraryContext'; -import { canEditComponent } from '../components/ComponentEditorModal'; import { PickLibraryContentModal } from './PickLibraryContentModal'; +import { blockTypes } from '../../editors/data/constants/app'; import messages from './messages'; @@ -62,7 +62,23 @@ const AddContentButton = ({ contentType, onCreateContent } : AddContentButtonPro ); }; - +export const parseErrorMsg = ( + intl, + error: any, + detailedMessage: MessageDescriptor, + defaultMessage: MessageDescriptor, +) => { + try { + const { response: { data } } = error; + const detail = data && (Array.isArray(data) ? data.join() : String(data)); + if (detail) { + return intl.formatMessage(detailedMessage, { detail }); + } + } catch (_err) { + // ignore + } + return intl.formatMessage(defaultMessage); +}; const AddContentContainer = () => { const intl = useIntl(); const { @@ -72,8 +88,8 @@ const AddContentContainer = () => { openComponentEditor, componentPicker, } = useLibraryContext(); - const createBlockMutation = useCreateLibraryBlock(); const updateComponentsMutation = useAddComponentsToCollection(libraryId, collectionId); + const createBlockMutation = useCreateLibraryBlock(); const pasteClipboardMutation = useLibraryPasteClipboard(); const { showToast } = useContext(ToastContext); const canEdit = useSelector(getCanEdit); @@ -81,23 +97,6 @@ const AddContentContainer = () => { const [isAddLibraryContentModalOpen, showAddLibraryContentModal, closeAddLibraryContentModal] = useToggle(); - const parseErrorMsg = ( - error: any, - detailedMessage: MessageDescriptor, - defaultMessage: MessageDescriptor, - ) => { - try { - const { response: { data } } = error; - const detail = data && (Array.isArray(data) ? data.join() : String(data)); - if (detail) { - return intl.formatMessage(detailedMessage, { detail }); - } - } catch (_err) { - // ignore - } - return intl.formatMessage(defaultMessage); - }; - const isBlockTypeEnabled = (blockType: string) => getConfig().LIBRARY_SUPPORTED_BLOCKS.includes(blockType); const collectionButtonData = { @@ -184,35 +183,36 @@ const AddContentContainer = () => { showToast(intl.formatMessage(messages.successPasteClipboardMessage)); }).catch((error) => { showToast(parseErrorMsg( + intl, error, messages.errorPasteClipboardMessageWithDetail, messages.errorPasteClipboardMessage, )); }); }; - const onCreateBlock = (blockType: string) => { - createBlockMutation.mutateAsync({ - libraryId, - blockType, - definitionId: `${uuid4()}`, - }).then((data) => { - const hasEditor = canEditComponent(data.id); - if (hasEditor) { - // linkComponent on editor close. - openComponentEditor(data.id, () => linkComponent(data.id)); - } else { + const suportedEditorTypes = Object.values(blockTypes); + if (suportedEditorTypes.includes(blockType)) { + // linkComponent on editor close. + openComponentEditor('', (data) => data && linkComponent(data.id), blockType); + } else { + createBlockMutation.mutateAsync({ + libraryId, + blockType, + definitionId: `${uuid4()}`, + }).then((data) => { // We can't start editing this right away so just show a toast message: showToast(intl.formatMessage(messages.successCreateMessage)); linkComponent(data.id); - } - }).catch((error) => { - showToast(parseErrorMsg( - error, - messages.errorCreateMessageWithDetail, - messages.errorCreateMessage, - )); - }); + }).catch((error) => { + showToast(parseErrorMsg( + intl, + error, + messages.errorCreateMessageWithDetail, + messages.errorCreateMessage, + )); + }); + } }; const onCreateContent = (blockType: string) => { @@ -237,7 +237,10 @@ const AddContentContainer = () => { {collectionId ? ( componentPicker && ( <> - + { ) ) : ( - + )}
{/* Note: for MVP we are hiding the unuspported types, not just disabling them. */} - {contentTypes.filter(ct => !ct.disabled).map((contentType) => ( - - ))} + {contentTypes + .filter((ct) => !ct.disabled) + .map((contentType) => ( + + ))} ); }; diff --git a/src/library-authoring/add-content/AddContentWorkflow.test.tsx b/src/library-authoring/add-content/AddContentWorkflow.test.tsx index b019700a5..f12dc0d91 100644 --- a/src/library-authoring/add-content/AddContentWorkflow.test.tsx +++ b/src/library-authoring/add-content/AddContentWorkflow.test.tsx @@ -60,20 +60,17 @@ describe('AddContentWorkflow test', () => { const newComponentButton = await screen.findByRole('button', { name: /New/ }); fireEvent.click(newComponentButton); - // Click "Text" to create a text component + // Click "Text" to open the editor in creation mode fireEvent.click(await screen.findByRole('button', { name: /Text/ })); - - // Then the editor should open - expect(await screen.findByRole('heading', { name: /New Text Component/ })).toBeInTheDocument(); + expect(await screen.findByRole('heading', { name: /Text/ })).toBeInTheDocument(); // Edit the title fireEvent.click(screen.getByRole('button', { name: /Edit Title/ })); const titleInput = screen.getByPlaceholderText('Title'); fireEvent.change(titleInput, { target: { value: 'A customized title' } }); fireEvent.blur(titleInput); - await waitFor(() => expect(screen.queryByRole('heading', { name: /New Text Component/ })).not.toBeInTheDocument()); - expect(screen.getByRole('heading', { name: /A customized title/ })); - + await waitFor(() => expect(screen.queryByRole('heading', { name: /Text/ })).not.toBeInTheDocument()); + expect(screen.getByRole('heading', { name: /A customized title/ })).toBeInTheDocument(); // Note that TinyMCE doesn't really load properly in our test environment // so we can't really edit the text, but we have getContent() mocked to simulate // using TinyMCE to enter some new HTML. @@ -83,10 +80,12 @@ describe('AddContentWorkflow test', () => { status: 200, data: { id: mockXBlockFields.usageKeyNewHtml }, })); - // Click Save + // Click Save should create the component and then save the content const saveButton = screen.getByLabelText('Save changes and return to learning context'); fireEvent.click(saveButton); - expect(saveSpy).toHaveBeenCalledTimes(1); + await waitFor(() => { + expect(saveSpy).toHaveBeenCalledTimes(1); + }); }); it('can create a Problem component', async () => { @@ -96,10 +95,8 @@ describe('AddContentWorkflow test', () => { const newComponentButton = await screen.findByRole('button', { name: /New/ }); fireEvent.click(newComponentButton); - // Click "Problem" to create a capa problem component + // Click "Problem" to create a capa problem component in the editor fireEvent.click(await screen.findByRole('button', { name: /Problem/ })); - - // Then the editor should open expect(await screen.findByRole('heading', { name: /Select problem type/ })).toBeInTheDocument(); // Select the type: Numerical Input @@ -117,10 +114,12 @@ describe('AddContentWorkflow test', () => { status: 200, data: { id: mockXBlockFields.usageKeyNewProblem }, })); - // Click Save + // Click Save should create the component and then save the content const saveButton = screen.getByLabelText('Save changes and return to learning context'); fireEvent.click(saveButton); - expect(saveSpy).toHaveBeenCalledTimes(1); + await waitFor(() => { + expect(saveSpy).toHaveBeenCalledTimes(1); + }); }); it('can create a Video component', async () => { @@ -133,8 +132,8 @@ describe('AddContentWorkflow test', () => { // Click "Video" to create a video component fireEvent.click(await screen.findByRole('button', { name: /Video/ })); - // Then the editor should open - this is the default title of a blank video in our mock - expect(await screen.findByRole('heading', { name: /New Video/ })).toBeInTheDocument(); + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Video/ })).toBeInTheDocument(); // Enter the video URL const urlInput = await screen.findByRole('textbox', { name: 'Video URL' }); @@ -146,9 +145,11 @@ describe('AddContentWorkflow test', () => { status: 200, data: { id: mockXBlockFields.usageKeyNewVideo }, })); - // Click Save + // Click Save should create the component and then save the content const saveButton = screen.getByLabelText('Save changes and return to learning context'); fireEvent.click(saveButton); - expect(saveSpy).toHaveBeenCalledTimes(1); + await waitFor(() => { + expect(saveSpy).toHaveBeenCalledTimes(1); + }); }); }); diff --git a/src/library-authoring/common/context/LibraryContext.tsx b/src/library-authoring/common/context/LibraryContext.tsx index d1abb4078..2e5e9526e 100644 --- a/src/library-authoring/common/context/LibraryContext.tsx +++ b/src/library-authoring/common/context/LibraryContext.tsx @@ -15,7 +15,8 @@ import { useComponentPickerContext } from './ComponentPickerContext'; export interface ComponentEditorInfo { usageKey: string; - onClose?: () => void; + blockType?:string + onClose?: (data?:any) => void; } export type LibraryContextData = { @@ -38,8 +39,8 @@ export type LibraryContextData = { /** If the editor is open and the user is editing some component, this is the component being edited. */ componentBeingEdited: ComponentEditorInfo | undefined; /** If an onClose callback is provided, it will be called when the editor is closed. */ - openComponentEditor: (usageKey: string, onClose?: () => void) => void; - closeComponentEditor: () => void; + openComponentEditor: (usageKey: string, onClose?: (data?:any) => void, blockType?:string) => void; + closeComponentEditor: (data?:any) => void; componentPicker?: typeof ComponentPicker; }; @@ -79,14 +80,14 @@ export const LibraryProvider = ({ }: LibraryProviderProps) => { const [isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal] = useToggle(false); const [componentBeingEdited, setComponentBeingEdited] = useState(); - const closeComponentEditor = useCallback(() => { + const closeComponentEditor = useCallback((data) => { setComponentBeingEdited((prev) => { - prev?.onClose?.(); + prev?.onClose?.(data); return undefined; }); }, []); - const openComponentEditor = useCallback((usageKey: string, onClose?: () => void) => { - setComponentBeingEdited({ usageKey, onClose }); + const openComponentEditor = useCallback((usageKey: string, onClose?: () => void, blockType?:string) => { + setComponentBeingEdited({ usageKey, onClose, blockType }); }, []); const { data: libraryData, isLoading: isLoadingLibraryData } = useContentLibrary(libraryId); diff --git a/src/library-authoring/components/ComponentEditorModal.tsx b/src/library-authoring/components/ComponentEditorModal.tsx index e139a8c40..0938fa7cc 100644 --- a/src/library-authoring/components/ComponentEditorModal.tsx +++ b/src/library-authoring/components/ComponentEditorModal.tsx @@ -25,10 +25,10 @@ export const ComponentEditorModal: React.FC> = () => { if (componentBeingEdited === undefined) { return null; } - const blockType = getBlockType(componentBeingEdited.usageKey); + const blockType = componentBeingEdited.blockType || getBlockType(componentBeingEdited.usageKey); - const onClose = () => { - closeComponentEditor(); + const onClose = (data?:any) => { + closeComponentEditor(data); invalidateComponentData(queryClient, libraryId, componentBeingEdited.usageKey); }; @@ -40,7 +40,7 @@ export const ComponentEditorModal: React.FC> = () => { studioEndpointUrl={getConfig().STUDIO_BASE_URL} lmsEndpointUrl={getConfig().LMS_BASE_URL} onClose={onClose} - returnFunction={() => { onClose(); return () => {}; }} + returnFunction={() => onClose} fullScreen={false} /> );