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

Fix for: #1489
openedx · Nov 14, 2024 · 033acc4 · 033acc4
1 parent efd2b3d
commit 033acc4
Show file tree

Hide file tree

Showing 10 changed files with 115 additions and 70 deletions.
diff --git a/src/hooks.test.ts b/src/hooks.test.ts
@@ -85,6 +85,8 @@ describe('Custom Hooks', () => {
 
       fireEvent.scroll(window);
 
+      // Called on scroll once and then due to content being less than screen height
+      // and hasNextPage being true.
       expect(fetchNextPage).toHaveBeenCalledTimes(2);
     });
 

diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx
@@ -9,10 +9,6 @@ import {
   waitFor,
   within,
 } from '../testUtils';
-import { executeThunk } from '../utils';
-import initializeStore from '../store';
-import { getApiWaffleFlagsUrl } from '../data/api';
-import { fetchWaffleFlags } from '../data/thunks';
 import mockResult from './__mocks__/library-search.json';
 import mockEmptyResult from '../search-modal/__mocks__/empty-search-result.json';
 import {
@@ -27,6 +23,7 @@ import { getStudioHomeApiUrl } from '../studio-home/data/api';
 import { mockBroadcastChannel } from '../generic/data/api.mock';
 import { LibraryLayout } from '.';
 import { getLibraryCollectionsApiUrl } from './data/api';
+import { getApiWaffleFlagsUrl } from '../data/api';
 
 mockGetCollectionMetadata.applyMock();
 mockContentSearchConfig.applyMock();
@@ -54,17 +51,12 @@ const returnEmptyResult = (_url, req) => {
 
 const path = '/library/:libraryId/*';
 const libraryTitle = mockContentLibrary.libraryData.title;
-let store;
 
 describe('<LibraryAuthoringPage />', () => {
   beforeEach(async () => {
     const { axiosMock } = initializeMocks();
-    store = initializeStore();
     axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock);
-    axiosMock
-      .onGet(getApiWaffleFlagsUrl())
-      .reply(200, {});
-    await executeThunk(fetchWaffleFlags(), store.dispatch);
+    axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {});
 
     // The Meilisearch client-side API uses fetch, not Axios.
     fetchMock.mockReset();
@@ -689,17 +681,15 @@ describe('<LibraryAuthoringPage />', () => {
 
   it('Shows an error if libraries V2 is disabled', async () => {
     const { axiosMock } = initializeMocks();
+    axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {});
     axiosMock.onGet(getStudioHomeApiUrl()).reply(200, {
       ...studioHomeMock,
       libraries_v2_enabled: false,
     });
-    axiosMock
-      .onGet(getApiWaffleFlagsUrl())
-      .reply(200, {});
-    await executeThunk(fetchWaffleFlags(), store.dispatch);
 
     render(<LibraryLayout />, { path, params: { libraryId: mockContentLibrary.libraryId } });
-    await waitFor(() => { expect(axiosMock.history.get.length).toBe(4); });
-    expect(screen.getByRole('alert')).toHaveTextContent('This page cannot be shown: Libraries v2 are disabled.');
+    expect(await screen.findByRole('alert')).toHaveTextContent(
+      'This page cannot be shown: Libraries v2 are disabled.',
+    );
   });
 });
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/AddContentWorkflow.test.tsx b/src/library-authoring/add-content/AddContentWorkflow.test.tsx
@@ -17,14 +17,11 @@ import {
   mockCreateLibraryBlock,
   mockXBlockFields,
 } from '../data/api.mocks';
-import initializeStore from '../../store';
-import { executeThunk } from '../../utils';
 import { mockBroadcastChannel, mockClipboardEmpty } from '../../generic/data/api.mock';
 import { mockContentSearchConfig, mockSearchResult } from '../../search-manager/data/api.mock';
 import { studioHomeMock } from '../../studio-home/__mocks__';
 import { getStudioHomeApiUrl } from '../../studio-home/data/api';
 import { getApiWaffleFlagsUrl } from '../../data/api';
-import { fetchWaffleFlags } from '../../data/thunks';
 import LibraryLayout from '../LibraryLayout';
 
 mockContentSearchConfig.applyMock();
@@ -51,17 +48,11 @@ const renderOpts = {
   routerProps: { initialEntries: [`/library/${libraryId}/components`] },
 };
 
-let store;
-
 describe('AddContentWorkflow test', () => {
   beforeEach(async () => {
     const { axiosMock } = initializeMocks();
-    store = initializeStore();
     axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock);
-    axiosMock
-      .onGet(getApiWaffleFlagsUrl())
-      .reply(200, {});
-    await executeThunk(fetchWaffleFlags(), store.dispatch);
+    axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {});
   });
 
   it('can create an HTML component', async () => {

diff --git a/src/library-authoring/add-content/PickLibraryContentModal.test.tsx b/src/library-authoring/add-content/PickLibraryContentModal.test.tsx
@@ -6,8 +6,6 @@ import {
   screen,
   initializeMocks,
 } from '../../testUtils';
-import initializeStore from '../../store';
-import { executeThunk } from '../../utils';
 import { studioHomeMock } from '../../studio-home/__mocks__';
 import { getStudioHomeApiUrl } from '../../studio-home/data/api';
 import mockResult from '../__mocks__/library-search.json';
@@ -20,7 +18,6 @@ import {
 } from '../data/api.mocks';
 import { PickLibraryContentModal } from './PickLibraryContentModal';
 import { getApiWaffleFlagsUrl } from '../../data/api';
-import { fetchWaffleFlags } from '../../data/thunks';
 
 mockContentSearchConfig.applyMock();
 mockContentLibrary.applyMock();
@@ -31,7 +28,6 @@ const { libraryId } = mockContentLibrary;
 
 const onClose = jest.fn();
 let mockShowToast: (message: string) => void;
-let store;
 
 const render = () => baseRender(<PickLibraryContentModal isOpen onClose={onClose} />, {
   path: '/library/:libraryId/collection/:collectionId/*',
@@ -50,13 +46,9 @@ const render = () => baseRender(<PickLibraryContentModal isOpen onClose={onClose
 describe('<PickLibraryContentModal />', () => {
   beforeEach(async () => {
     const mocks = initializeMocks();
-    store = initializeStore();
     mockShowToast = mocks.mockShowToast;
     mocks.axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock);
-    mocks.axiosMock
-      .onGet(getApiWaffleFlagsUrl())
-      .reply(200, {});
-    await executeThunk(fetchWaffleFlags(), store.dispatch);
+    mocks.axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {});
   });
 
   it('can pick components from the modal', async () => {

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