From f1bdc6200fe77dd1f50e5273c387a262be74b665 Mon Sep 17 00:00:00 2001 From: Jillian Date: Thu, 7 Nov 2024 13:23:24 +1030 Subject: [PATCH] fix: show a more detailed error on Bad Request (#1468) Show a detailed error when 400 Bad Request received while adding a component to a library, either a new or pasted component. The most likely error from the backend here is "library can only have {max} components", and since this error is translated already, we can just report it through. --- .../add-content/AddContentContainer.test.tsx | 95 ++++++++++--------- .../add-content/AddContentContainer.tsx | 33 +++++-- src/library-authoring/add-content/messages.ts | 18 +++- 3 files changed, 90 insertions(+), 56 deletions(-) diff --git a/src/library-authoring/add-content/AddContentContainer.test.tsx b/src/library-authoring/add-content/AddContentContainer.test.tsx index 95e9e4ccb..9587576a9 100644 --- a/src/library-authoring/add-content/AddContentContainer.test.tsx +++ b/src/library-authoring/add-content/AddContentContainer.test.tsx @@ -164,52 +164,6 @@ describe('', () => { expect(mockShowToast).toHaveBeenCalledWith('There was an error linking the content to this collection.'); }); - it('should handle failure to paste content', async () => { - const { axiosMock, mockShowToast } = initializeMocks(); - // Simulate having an HTML block in the clipboard: - mockClipboardHtml.applyMock(); - - const pasteUrl = getLibraryPasteClipboardUrl(libraryId); - axiosMock.onPost(pasteUrl).reply(400); - - render(); - - const pasteButton = await screen.findByRole('button', { name: /paste from clipboard/i }); - fireEvent.click(pasteButton); - - await waitFor(() => { - expect(axiosMock.history.post[0].url).toEqual(pasteUrl); - expect(mockShowToast).toHaveBeenCalledWith('There was an error pasting the content.'); - }); - }); - - it('should handle failure to paste content and show server error if available', async () => { - const { axiosMock, mockShowToast } = initializeMocks(); - // Simulate having an HTML block in the clipboard: - mockClipboardHtml.applyMock(); - - const errMsg = 'Libraries do not support this type of content yet.'; - const pasteUrl = getLibraryPasteClipboardUrl(libraryId); - - // eslint-disable-next-line prefer-promise-reject-errors - axiosMock.onPost(pasteUrl).reply(() => Promise.reject({ - customAttributes: { - httpErrorStatus: 400, - httpErrorResponseData: JSON.stringify({ block_type: errMsg }), - }, - })); - - render(); - - const pasteButton = await screen.findByRole('button', { name: /paste from clipboard/i }); - fireEvent.click(pasteButton); - - await waitFor(() => { - expect(axiosMock.history.post[0].url).toEqual(pasteUrl); - expect(mockShowToast).toHaveBeenCalledWith(errMsg); - }); - }); - it('should stop user from pasting unsupported blocks and show toast', async () => { const { axiosMock, mockShowToast } = initializeMocks(); // Simulate having an HTML block in the clipboard: @@ -227,4 +181,53 @@ describe('', () => { expect(mockShowToast).toHaveBeenCalledWith(errMsg); }); }); + + test.each([ + { + label: 'should handle failure to paste content', + mockUrl: getLibraryPasteClipboardUrl(libraryId), + mockResponse: undefined, + expectedError: 'There was an error pasting the content.', + buttonName: /paste from clipboard/i, + }, + { + label: 'should show detailed error in toast on paste failure', + mockUrl: getLibraryPasteClipboardUrl(libraryId), + mockResponse: ['library cannot have more than 100000 components'], + 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, + }) => { + const { axiosMock, mockShowToast } = initializeMocks(); + axiosMock.onPost(mockUrl).reply(400, mockResponse); + + // Simulate having an HTML block in the clipboard: + mockClipboardHtml.applyMock(); + + render(); + const button = await screen.findByRole('button', { name: buttonName }); + fireEvent.click(button); + + await waitFor(() => { + expect(axiosMock.history.post.length).toEqual(1); + expect(axiosMock.history.post[0].url).toEqual(mockUrl); + expect(mockShowToast).toHaveBeenCalledWith(expectedError); + }); + }); }); diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx index 3ae948b8a..0dbb2da4c 100644 --- a/src/library-authoring/add-content/AddContentContainer.tsx +++ b/src/library-authoring/add-content/AddContentContainer.tsx @@ -1,4 +1,5 @@ import React, { useContext } from 'react'; +import type { MessageDescriptor } from 'react-intl'; import { useSelector } from 'react-redux'; import { Stack, @@ -80,15 +81,21 @@ const AddContentContainer = () => { const [isAddLibraryContentModalOpen, showAddLibraryContentModal, closeAddLibraryContentModal] = useToggle(); - const parsePasteErrorMsg = (error: any) => { - let errMsg: string; + const parseErrorMsg = ( + error: any, + detailedMessage: MessageDescriptor, + defaultMessage: MessageDescriptor, + ) => { try { - const { customAttributes: { httpErrorResponseData } } = error; - errMsg = JSON.parse(httpErrorResponseData).block_type; + const { response: { data } } = error; + const detail = data && (Array.isArray(data) ? data.join() : String(data)); + if (detail) { + return intl.formatMessage(detailedMessage, { detail }); + } } catch (_err) { - errMsg = intl.formatMessage(messages.errorPasteClipboardMessage); + // ignore } - return errMsg; + return intl.formatMessage(defaultMessage); }; const isBlockTypeEnabled = (blockType: string) => getConfig().LIBRARY_SUPPORTED_BLOCKS.includes(blockType); @@ -178,7 +185,11 @@ const AddContentContainer = () => { linkComponent(data.id); showToast(intl.formatMessage(messages.successPasteClipboardMessage)); }).catch((error) => { - showToast(parsePasteErrorMsg(error)); + showToast(parseErrorMsg( + error, + messages.errorPasteClipboardMessageWithDetail, + messages.errorPasteClipboardMessage, + )); }); }; @@ -196,8 +207,12 @@ const AddContentContainer = () => { // We can't start editing this right away so just show a toast message: showToast(intl.formatMessage(messages.successCreateMessage)); } - }).catch(() => { - showToast(intl.formatMessage(messages.errorCreateMessage)); + }).catch((error) => { + showToast(parseErrorMsg( + error, + messages.errorCreateMessageWithDetail, + messages.errorCreateMessage, + )); }); }; diff --git a/src/library-authoring/add-content/messages.ts b/src/library-authoring/add-content/messages.ts index b1be04e89..a720c12ce 100644 --- a/src/library-authoring/add-content/messages.ts +++ b/src/library-authoring/add-content/messages.ts @@ -64,7 +64,15 @@ const messages = defineMessages({ errorCreateMessage: { id: 'course-authoring.library-authoring.add-content.error.text', defaultMessage: 'There was an error creating the content.', - description: 'Message when creation of content in library is on error', + description: 'Message when creation of content in library is on error.', + }, + errorCreateMessageWithDetail: { + id: 'course-authoring.library-authoring.add-content.error.text-detail', + defaultMessage: 'There was an error creating the content: {detail}', + description: ( + 'Message when creation of content in library is on error.' + + ' The {detail} text provides more information about the error.' + ), }, linkingComponentMessage: { id: 'course-authoring.library-authoring.linking-collection-content.progress.text', @@ -96,6 +104,14 @@ const messages = defineMessages({ defaultMessage: 'There was an error pasting the content.', description: 'Message when pasting clipboard in library errors', }, + errorPasteClipboardMessageWithDetail: { + id: 'course-authoring.library-authoring.paste-clipboard.error.text-detail', + defaultMessage: 'There was an error pasting the content: {detail}', + description: ( + 'Message when pasting clipboard in library errors.' + + ' The {detail} text provides more information about the error.' + ), + }, pastingClipboardMessage: { id: 'course-authoring.library-authoring.paste-clipboard.loading.text', defaultMessage: 'Pasting content from clipboard...',