fix: remove unnecessary toast notification on adding component (#1490) (

#1528) (cherry picked from commit 033acc4)
openedx · Nov 22, 2024 · d08ef83 · d08ef83
1 parent 13bce7e
commit d08ef83
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 35 deletions.
diff --git a/src/library-authoring/add-content/AddContentContainer.test.tsx b/src/library-authoring/add-content/AddContentContainer.test.tsx
@@ -1,3 +1,5 @@
+import MockAdapter from 'axios-mock-adapter/types';
+import { snakeCaseObject } from '@edx/frontend-platform';
 import {
   fireEvent,
   render as baseRender,
@@ -6,13 +8,21 @@ import {
   initializeMocks,
 } from '../../testUtils';
 import { mockContentLibrary } from '../data/api.mocks';
-import { getCreateLibraryBlockUrl, getLibraryCollectionComponentApiUrl, getLibraryPasteClipboardUrl } from '../data/api';
+import {
+  getContentLibraryApiUrl, getCreateLibraryBlockUrl, getLibraryCollectionComponentApiUrl, getLibraryPasteClipboardUrl,
+} from '../data/api';
 import { mockBroadcastChannel, mockClipboardEmpty, mockClipboardHtml } from '../../generic/data/api.mock';
 import { LibraryProvider } from '../common/context';
 import AddContentContainer from './AddContentContainer';
+import { ComponentEditorModal } from '../components/ComponentEditorModal';
+import editorCmsApi from '../../editors/data/services/cms/api';
+import { ToastActionData } from '../../generic/toast-context';
 
 mockBroadcastChannel();
 
+// Mocks for ComponentEditorModal to work in tests.
+jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' }));
+
 const { libraryId } = mockContentLibrary;
 const render = (collectionId?: string) => {
   const params: { libraryId: string, collectionId?: string } = { libraryId };
@@ -26,15 +36,27 @@ const render = (collectionId?: string) => {
       <LibraryProvider
         libraryId={libraryId}
         collectionId={collectionId}
-      >{ children }
+      >
+        { children }
+        <ComponentEditorModal />
       </LibraryProvider>
     ),
   });
 };
+let axiosMock: MockAdapter;
+let mockShowToast: (message: string, action?: ToastActionData | undefined) => void;
 
 describe('<AddContentContainer />', () => {
+  beforeEach(() => {
+    const mocks = initializeMocks();
+    axiosMock = mocks.axiosMock;
+    mockShowToast = mocks.mockShowToast;
+    axiosMock.onGet(getContentLibraryApiUrl(libraryId)).reply(200, {});
+  });
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
   it('should render content buttons', () => {
-    initializeMocks();
     mockClipboardEmpty.applyMock();
     render();
     expect(screen.queryByRole('button', { name: /collection/i })).toBeInTheDocument();
@@ -48,7 +70,6 @@ describe('<AddContentContainer />', () => {
   });
 
   it('should create a content', async () => {
-    const { axiosMock } = initializeMocks();
     mockClipboardEmpty.applyMock();
     const url = getCreateLibraryBlockUrl(libraryId);
     axiosMock.onPost(url).reply(200);
@@ -62,15 +83,15 @@ describe('<AddContentContainer />', () => {
     await waitFor(() => expect(axiosMock.history.patch.length).toEqual(0));
   });
 
-  it('should create a content in a collection', async () => {
-    const { axiosMock } = initializeMocks();
+  it('should create a content in a collection for non-editable blocks', async () => {
     mockClipboardEmpty.applyMock();
     const collectionId = 'some-collection-id';
     const url = getCreateLibraryBlockUrl(libraryId);
     const collectionComponentUrl = getLibraryCollectionComponentApiUrl(
       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);
 
@@ -84,8 +105,57 @@ describe('<AddContentContainer />', () => {
     await waitFor(() => expect(axiosMock.history.patch[0].url).toEqual(collectionComponentUrl));
   });
 
+  it('should create a content in a collection for editable blocks', async () => {
+    mockClipboardEmpty.applyMock();
+    const collectionId = 'some-collection-id';
+    const url = getCreateLibraryBlockUrl(libraryId);
+    const collectionComponentUrl = getLibraryCollectionComponentApiUrl(
+      libraryId,
+      collectionId,
+    );
+    // Mocks for ComponentEditorModal to work in tests.
+    jest.spyOn(editorCmsApi, 'fetchImages').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',
+    });
+    const fieldsHtml = {
+      displayName: 'Introduction to Testing',
+      data: '<p>This is a text component which uses <strong>HTML</strong>.</p>',
+      metadata: { displayName: 'Introduction to Testing' },
+    };
+    jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => (
+      { status: 200, data: snakeCaseObject(fieldsHtml) }
+    ));
+    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);
+    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));
+  });
+
   it('should render paste button if clipboard contains pastable xblock', async () => {
-    initializeMocks();
     // Simulate having an HTML block in the clipboard:
     const getClipboardSpy = mockClipboardHtml.applyMock();
     render();
@@ -94,7 +164,6 @@ describe('<AddContentContainer />', () => {
   });
 
   it('should paste content', async () => {
-    const { axiosMock } = initializeMocks();
     // Simulate having an HTML block in the clipboard:
     const getClipboardSpy = mockClipboardHtml.applyMock();
 
@@ -112,7 +181,6 @@ describe('<AddContentContainer />', () => {
   });
 
   it('should paste content inside a collection', async () => {
-    const { axiosMock } = initializeMocks();
     // Simulate having an HTML block in the clipboard:
     const getClipboardSpy = mockClipboardHtml.applyMock();
 
@@ -138,7 +206,6 @@ describe('<AddContentContainer />', () => {
   });
 
   it('should show error toast on linking failure', async () => {
-    const { axiosMock, mockShowToast } = initializeMocks();
     // Simulate having an HTML block in the clipboard:
     const getClipboardSpy = mockClipboardHtml.applyMock();
 
@@ -165,7 +232,6 @@ describe('<AddContentContainer />', () => {
   });
 
   it('should stop user from pasting unsupported blocks and show toast', async () => {
-    const { axiosMock, mockShowToast } = initializeMocks();
     // Simulate having an HTML block in the clipboard:
     mockClipboardHtml.applyMock('openassessment');
 
@@ -214,7 +280,6 @@ describe('<AddContentContainer />', () => {
   ])('$label', async ({
     mockUrl, mockResponse, buttonName, expectedError,
   }) => {
-    const { axiosMock, mockShowToast } = initializeMocks();
     axiosMock.onPost(mockUrl).reply(400, mockResponse);
 
     // Simulate having an HTML block in the clipboard:

diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx
@@ -166,9 +166,7 @@ const AddContentContainer = () => {
   }
 
   const linkComponent = (usageKey: string) => {
-    updateComponentsMutation.mutateAsync([usageKey]).then(() => {
-      showToast(intl.formatMessage(messages.successAssociateComponentMessage));
-    }).catch(() => {
+    updateComponentsMutation.mutateAsync([usageKey]).catch(() => {
       showToast(intl.formatMessage(messages.errorAssociateComponentMessage));
     });
   };
@@ -199,13 +197,14 @@ const AddContentContainer = () => {
       blockType,
       definitionId: `${uuid4()}`,
     }).then((data) => {
-      linkComponent(data.id);
       const hasEditor = canEditComponent(data.id);
       if (hasEditor) {
-        openComponentEditor(data.id);
+        // linkComponent on editor close.
+        openComponentEditor(data.id, () => linkComponent(data.id));
       } else {
         // 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(
@@ -228,14 +227,11 @@ const AddContentContainer = () => {
     }
   };
 
+  /* istanbul ignore next */
   if (pasteClipboardMutation.isLoading) {
     showToast(intl.formatMessage(messages.pastingClipboardMessage));
   }
 
-  if (updateComponentsMutation.isLoading) {
-    showToast(intl.formatMessage(messages.linkingComponentMessage));
-  }
-
   return (
     <Stack direction="vertical">
       {collectionId ? (

diff --git a/src/library-authoring/add-content/messages.ts b/src/library-authoring/add-content/messages.ts
@@ -74,11 +74,6 @@ const messages = defineMessages({
       + ' The {detail} text provides more information about the error.'
     ),
   },
-  linkingComponentMessage: {
-    id: 'course-authoring.library-authoring.linking-collection-content.progress.text',
-    defaultMessage: 'Adding component to collection...',
-    description: 'Message when component is being linked to collection in library',
-  },
   successAssociateComponentMessage: {
     id: 'course-authoring.library-authoring.associate-collection-content.success.text',
     defaultMessage: 'Content linked successfully.',

diff --git a/src/library-authoring/common/context.tsx b/src/library-authoring/common/context.tsx
@@ -68,6 +68,11 @@ export interface SidebarComponentInfo {
   additionalAction?: SidebarAdditionalActions;
 }
 
+export interface ComponentEditorInfo {
+  usageKey: string;
+  onClose?: () => void;
+}
+
 export enum SidebarAdditionalActions {
   JumpToAddCollections = 'jump-to-add-collections',
 }
@@ -99,9 +104,10 @@ export type LibraryContextData = {
   // Current collection
   openCollectionInfoSidebar: (collectionId: string, additionalAction?: SidebarAdditionalActions) => void;
   // Editor modal - for editing some component
-  /** If the editor is open and the user is editing some component, this is its usageKey */
-  componentBeingEdited: string | undefined;
-  openComponentEditor: (usageKey: string) => void;
+  /** 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;
   resetSidebarAdditionalActions: () => void;
 } & ComponentPickerType;
@@ -174,8 +180,16 @@ export const LibraryProvider = ({
   );
   const [isLibraryTeamModalOpen, openLibraryTeamModal, closeLibraryTeamModal] = useToggle(false);
   const [isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal] = useToggle(false);
-  const [componentBeingEdited, openComponentEditor] = useState<string | undefined>();
-  const closeComponentEditor = useCallback(() => openComponentEditor(undefined), []);
+  const [componentBeingEdited, setComponentBeingEdited] = useState<ComponentEditorInfo | undefined>();
+  const closeComponentEditor = useCallback(() => {
+    setComponentBeingEdited((prev) => {
+      prev?.onClose?.();
+      return undefined;
+    });
+  }, []);
+  const openComponentEditor = useCallback((usageKey: string, onClose?: () => void) => {
+    setComponentBeingEdited({ usageKey, onClose });
+  }, []);
 
   const [selectedComponents, setSelectedComponents] = useState<SelectedComponent[]>([]);
 

diff --git a/src/library-authoring/components/ComponentEditorModal.tsx b/src/library-authoring/components/ComponentEditorModal.tsx
@@ -28,18 +28,18 @@ export const ComponentEditorModal: React.FC<Record<never, never>> = () => {
   if (componentBeingEdited === undefined) {
     return null;
   }
-  const blockType = getBlockType(componentBeingEdited);
+  const blockType = getBlockType(componentBeingEdited.usageKey);
 
   const onClose = () => {
     closeComponentEditor();
-    invalidateComponentData(queryClient, libraryId, componentBeingEdited);
+    invalidateComponentData(queryClient, libraryId, componentBeingEdited.usageKey);
   };
 
   return (
     <EditorPage
       courseId={libraryId}
       blockType={blockType}
-      blockId={componentBeingEdited}
+      blockId={componentBeingEdited.usageKey}
       studioEndpointUrl={getConfig().STUDIO_BASE_URL}
       lmsEndpointUrl={getConfig().LMS_BASE_URL}
       onClose={onClose}

diff --git a/src/testUtils.tsx b/src/testUtils.tsx
@@ -27,7 +27,7 @@ import initializeReduxStore from './store';
 
 /** @deprecated Use React Query and/or regular React Context instead of redux */
 let reduxStore: Store;
-let queryClient;
+let queryClient: QueryClient;
 let axiosMock: MockAdapter;
 
 /** To use this: `const { mockShowToast } = initializeMocks()` and `expect(mockShowToast).toHaveBeenCalled()` */