From 6dab0de66fecd0111cc0f7966cca8bfca02eb3e1 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 22 Nov 2024 10:25:46 +0530 Subject: [PATCH] fix: remove unnecessary toast notification on adding component (#1490) (cherry picked from commit 033acc45f19a695f1d98fd318caafa466bfb728b) --- .../add-content/AddContentContainer.test.tsx | 89 ++++++++++++++++--- .../add-content/AddContentContainer.tsx | 14 ++- src/library-authoring/add-content/messages.ts | 5 -- src/library-authoring/common/context.tsx | 24 +++-- .../components/ComponentEditorModal.tsx | 6 +- src/testUtils.tsx | 2 +- 6 files changed, 105 insertions(+), 35 deletions(-) diff --git a/src/library-authoring/add-content/AddContentContainer.test.tsx b/src/library-authoring/add-content/AddContentContainer.test.tsx index 9587576a98..4798d271ad 100644 --- 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) => { { children } + > + { children } + ), }); }; +let axiosMock: MockAdapter; +let mockShowToast: (message: string, action?: ToastActionData | undefined) => void; describe('', () => { + 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('', () => { }); it('should create a content', async () => { - const { axiosMock } = initializeMocks(); mockClipboardEmpty.applyMock(); const url = getCreateLibraryBlockUrl(libraryId); axiosMock.onPost(url).reply(200); @@ -62,8 +83,7 @@ describe('', () => { 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); @@ -71,6 +91,7 @@ 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); @@ -84,8 +105,57 @@ describe('', () => { 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: '

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); + + 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('', () => { }); 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('', () => { }); 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('', () => { }); 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('', () => { }); 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('', () => { ])('$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 index 0dbb2da4c0..0afcd43309 100644 --- 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 ( {collectionId ? ( diff --git a/src/library-authoring/add-content/messages.ts b/src/library-authoring/add-content/messages.ts index a720c12ce6..898073eb49 100644 --- 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 index be229065f5..ef1fb7ef1c 100644 --- 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(); - const closeComponentEditor = useCallback(() => openComponentEditor(undefined), []); + const [componentBeingEdited, setComponentBeingEdited] = useState(); + const closeComponentEditor = useCallback(() => { + setComponentBeingEdited((prev) => { + prev?.onClose?.(); + return undefined; + }); + }, []); + const openComponentEditor = useCallback((usageKey: string, onClose?: () => void) => { + setComponentBeingEdited({ usageKey, onClose }); + }, []); const [selectedComponents, setSelectedComponents] = useState([]); diff --git a/src/library-authoring/components/ComponentEditorModal.tsx b/src/library-authoring/components/ComponentEditorModal.tsx index 9dfcec1637..0883ab6dc5 100644 --- a/src/library-authoring/components/ComponentEditorModal.tsx +++ b/src/library-authoring/components/ComponentEditorModal.tsx @@ -28,18 +28,18 @@ export const ComponentEditorModal: React.FC> = () => { 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 (