feat: cancellable block creation/edit workflow in libraries v2 (#1574)

Before, clicking "new problem" (etc) would create a new block, then launch the editor. Now it launches the editor and then creates the new block only on save. This makes the "Cancel" button work as expected. Only affects libraries so far, not courses.

.env.test

@@ -39,4 +39,5 @@ INVITE_STUDENTS_EMAIL_TO="[email protected]"
 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"

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', () => {

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 })
+  )),
+});

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(<EditorPage {...{ ...defaultPropsHtml, blockId: '' }} />);
+    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(<EditorPage {...defaultPropsHtml} />);
+    expect(await screen.findByText(/Error: Content save failed. Please check recent changes and try again later./i)).toBeInTheDocument();
+  });
 });

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<Props> = ({
   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<Props> = ({
   };
   return (
     <EditorModalWrapper onClose={confirmCancelIfDirty}>
+      {createFailed && (
+        <Toast show onClose={clearCreateFailed}>
+          {parseErrorMsg(
+            intl,
+            createFailedError,
+            libraryMessages.errorCreateMessageWithDetail,
+            libraryMessages.errorCreateMessage,
+          )}
+        </Toast>
+      )}
       {saveFailed && (
         <Toast show onClose={clearSaveFailed}>
           <FormattedMessage {...messages.contentSaveFailed} />
@@ -126,6 +142,7 @@ const EditorContainer: React.FC<Props> = ({
           if (returnFunction) {
             closeCancelConfirmModal();
           }
+          dispatch({ type: 'resetEditor' });
         }}
       />
       <ModalDialog.Header className="shadow-sm zindex-10">

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', () => {

src/editors/containers/ProblemEditor/index.tsx

@@ -65,11 +65,13 @@ const ProblemEditor: React.FC<Props> = ({
 };
 
 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 = {

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),

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(

src/editors/containers/VideoEditor/index.tsx

@@ -23,6 +23,7 @@ const VideoEditor: React.FC<EditorComponent> = ({
     (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<EditorComponent> = ({
         returnFunction={returnFunction}
         validateEntry={validateEntry}
       >
-        {studioViewFinished ? (
+        {(isCreateWorkflow || studioViewFinished) ? (
           <div className="video-editor">
             <VideoEditorModal {...{ isLibrary }} />
           </div>

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)]);

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',

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 }) => ({

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);
+    });
+  });
 });