Skip to content

Commit

Permalink
fix: show a more detailed error on Bad Request (#1468) (#1478)
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit f1bdc62)
  • Loading branch information
pomegranited authored Nov 7, 2024
1 parent d4e9a6b commit e2adb45
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 50 deletions.
83 changes: 43 additions & 40 deletions src/library-authoring/add-content/AddContentContainer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,67 +164,70 @@ describe('<AddContentContainer />', () => {
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 () => {
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();
mockClipboardHtml.applyMock('openassessment');

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(axiosMock.history.post.length).toEqual(0);
expect(mockShowToast).toHaveBeenCalledWith(errMsg);
});
});

it('should stop user from pasting unsupported blocks and show toast', async () => {
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();
// Simulate having an HTML block in the clipboard:
mockClipboardHtml.applyMock('openassessment');
axiosMock.onPost(mockUrl).reply(400, mockResponse);

const errMsg = 'Libraries do not support this type of content yet.';
// Simulate having an HTML block in the clipboard:
mockClipboardHtml.applyMock();

render();

const pasteButton = await screen.findByRole('button', { name: /paste from clipboard/i });
fireEvent.click(pasteButton);
const button = await screen.findByRole('button', { name: buttonName });
fireEvent.click(button);

await waitFor(() => {
expect(axiosMock.history.post.length).toEqual(0);
expect(mockShowToast).toHaveBeenCalledWith(errMsg);
expect(axiosMock.history.post.length).toEqual(1);
expect(axiosMock.history.post[0].url).toEqual(mockUrl);
expect(mockShowToast).toHaveBeenCalledWith(expectedError);
});
});
});
33 changes: 24 additions & 9 deletions src/library-authoring/add-content/AddContentContainer.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useContext } from 'react';
import type { MessageDescriptor } from 'react-intl';
import { useSelector } from 'react-redux';
import {
Stack,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
));
});
};

Expand All @@ -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,
));
});
};

Expand Down
18 changes: 17 additions & 1 deletion src/library-authoring/add-content/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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...',
Expand Down

0 comments on commit e2adb45

Please sign in to comment.