fix: show error information when taxonomy import fails (#1730)

Adds the error information when we have a failure while importing a Taxonomy
This commit is contained in:
Rômulo Penido
2025-03-17 12:00:33 -03:00
committed by GitHub
parent e66da2cb49
commit a26e3f9e92
13 changed files with 113 additions and 102 deletions

View File

@@ -35,6 +35,6 @@ describe('<AlertMessage />', () => {
const { getByText } = render(<RootWrapper error={error} />);
screen.logTestingPlaygroundURL();
expect(getByText(/this is an error message/i)).toBeInTheDocument();
expect(getByText(/\{"message":"this is a response body"\}/i)).toBeInTheDocument();
expect(getByText(/\{ "message": "this is a response body" \}/i)).toBeInTheDocument();
});
});

View File

@@ -1,14 +1,45 @@
import React from 'react';
import { useIntl } from '@edx/frontend-platform/i18n';
import {
Alert,
} from '@openedx/paragon';
import messages from './messages';
const AlertError: React.FC<{ error: unknown }> = ({ error }) => (
<Alert variant="danger" className="mt-3">
{error instanceof Object && 'message' in error ? error.message : String(error)}
<br />
{error instanceof Object && (error as any).response?.data && JSON.stringify((error as any).response?.data)}
</Alert>
);
export interface AlertErrorProps {
error: unknown;
title?: string;
onDismiss?: () => void;
}
/* eslint-disable react/prop-types */
const AlertError: React.FC<AlertErrorProps> = ({ error, title, onDismiss }) => {
const intl = useIntl();
let errorDetails: string | undefined;
if (error instanceof Object && (error as any).response?.data) {
if (typeof (error as any).response?.data === 'string') {
errorDetails = (error as any).response?.data;
} else {
errorDetails = JSON.stringify((error as any).response?.data, null, 2);
}
}
return (
<Alert
variant="danger"
className="mt-3"
dismissible={!!onDismiss}
closeLabel={intl.formatMessage(messages.dismissLabel)}
onClose={onDismiss}
>
{title && <Alert.Heading>{title}</Alert.Heading>}
{error instanceof Object && 'message' in error ? error.message : String(error)}
<br />
{errorDetails && (
<pre>
{errorDetails}
</pre>
)}
</Alert>
);
};
export default AlertError;

View File

@@ -0,0 +1,11 @@
import { defineMessages } from '@edx/frontend-platform/i18n';
const messages = defineMessages({
dismissLabel: {
id: 'authoring.alert-error-alert.dismiss',
defaultMessage: 'Dismiss',
description: 'The label for the dismiss button on the alert error component.',
},
});
export default messages;

View File

@@ -110,7 +110,8 @@ describe('<LibraryCollectionPage />', () => {
it('shows an error component if no collection returned', async () => {
// This mock will simulate incorrect collection id
await renderLibraryCollectionPage(mockCollection.collectionEmpty);
expect(await screen.findByText(/Mocked request failed with status code 404./)).toBeInTheDocument();
const errorMessage = 'Mocked request failed with status code 404{ "detail": "Not found." }';
expect(await screen.findByRole('alert')).toHaveTextContent(errorMessage);
});
it('shows collection data', async () => {

View File

@@ -176,8 +176,8 @@ describe('<CreateLibrary />', () => {
'{"description":"","title":"Test Library Name","org":"org1","slug":"test_library_slug"}',
);
expect(mockNavigate).not.toHaveBeenCalled();
expect(await screen.findByRole('alert')).toHaveTextContent('Request failed with status code 400');
expect(await screen.findByRole('alert')).toHaveTextContent('{"field":"Error message"}');
const errorMessage = 'Request failed with status code 400{ "field": "Error message" }';
expect(await screen.findByRole('alert')).toHaveTextContent(errorMessage);
});
});

View File

@@ -1,6 +1,6 @@
import React, { useContext } from 'react';
import { initializeMocks, render } from '../testUtils';
import { initializeMocks, render, screen } from '../testUtils';
import { TaxonomyContext } from './common/context';
import { TaxonomyLayout } from './TaxonomyLayout';
@@ -9,7 +9,7 @@ const alertErrorTitle = 'Error title';
const alertErrorDescription = 'Error description';
const MockChildComponent = () => {
const { setToastMessage, setAlertProps } = useContext(TaxonomyContext);
const { setToastMessage, setAlertError } = useContext(TaxonomyContext);
return (
<div data-testid="mock-content">
@@ -22,7 +22,7 @@ const MockChildComponent = () => {
</button>
<button
type="button"
onClick={() => setAlertProps!({ title: alertErrorTitle, description: alertErrorDescription })}
onClick={() => setAlertError!({ title: alertErrorTitle, error: new Error(alertErrorDescription) })}
data-testid="taxonomy-show-alert"
>
Show Alert
@@ -47,36 +47,31 @@ describe('<TaxonomyLayout />', () => {
});
it('should render page correctly', () => {
const { getByTestId } = render(<TaxonomyLayout />);
expect(getByTestId('mock-header')).toBeInTheDocument();
expect(getByTestId('mock-content')).toBeInTheDocument();
expect(getByTestId('mock-footer')).toBeInTheDocument();
render(<TaxonomyLayout />);
expect(screen.getByTestId('mock-header')).toBeInTheDocument();
expect(screen.getByTestId('mock-content')).toBeInTheDocument();
expect(screen.getByTestId('mock-footer')).toBeInTheDocument();
});
it('should show toast', () => {
const { getByTestId, getByText } = render(<TaxonomyLayout />);
const button = getByTestId('taxonomy-show-toast');
render(<TaxonomyLayout />);
const button = screen.getByTestId('taxonomy-show-toast');
button.click();
expect(getByTestId('taxonomy-toast')).toBeInTheDocument();
expect(getByText(toastMessage)).toBeInTheDocument();
expect(screen.getByTestId('taxonomy-toast')).toBeInTheDocument();
expect(screen.getByText(toastMessage)).toBeInTheDocument();
});
it('should show alert', () => {
const {
getByTestId,
getByText,
getByRole,
queryByTestId,
} = render(<TaxonomyLayout />);
render(<TaxonomyLayout />);
const button = getByTestId('taxonomy-show-alert');
const button = screen.getByTestId('taxonomy-show-alert');
button.click();
expect(getByTestId('taxonomy-alert')).toBeInTheDocument();
expect(getByText(alertErrorTitle)).toBeInTheDocument();
expect(getByText(alertErrorDescription)).toBeInTheDocument();
expect(screen.getByText(alertErrorTitle)).toBeInTheDocument();
expect(screen.getByText(alertErrorDescription)).toBeInTheDocument();
const closeAlertButton = getByRole('button', { name: 'Dismiss' });
const closeAlertButton = screen.getByRole('button', { name: 'Dismiss' });
closeAlertButton.click();
expect(queryByTestId('taxonomy-alert')).not.toBeInTheDocument();
expect(screen.queryByText(alertErrorTitle)).not.toBeInTheDocument();
expect(screen.queryByText(alertErrorDescription)).not.toBeInTheDocument();
});
});

View File

@@ -1,37 +1,30 @@
import React, { useMemo, useState } from 'react';
import { useMemo, useState } from 'react';
import { StudioFooter } from '@edx/frontend-component-footer';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Outlet, ScrollRestoration } from 'react-router-dom';
import { Toast } from '@openedx/paragon';
import AlertMessage from '../generic/alert-message';
import AlertError, { type AlertErrorProps } from '../generic/alert-error';
import Header from '../header';
import { type AlertProps, TaxonomyContext } from './common/context';
import messages from './messages';
import { TaxonomyContext } from './common/context';
export const TaxonomyLayout = () => {
const intl = useIntl();
// Use `setToastMessage` to show the toast.
const [toastMessage, setToastMessage] = useState<string | null>(null);
// Use `setToastMessage` to show the alert.
const [alertProps, setAlertProps] = useState<AlertProps | null>(null);
const [alertError, setAlertError] = useState<AlertErrorProps | null>(null);
const context = useMemo(() => ({
toastMessage, setToastMessage, alertProps, setAlertProps,
toastMessage, setToastMessage, alertError, setAlertError,
}), []);
return (
<TaxonomyContext.Provider value={context}>
<div className="bg-light-400">
<Header isHiddenMainMenu />
{ alertProps && (
<AlertMessage
data-testid="taxonomy-alert"
className="mb-0"
dismissible
closeLabel={intl.formatMessage(messages.taxonomyDismissLabel)}
onClose={() => setAlertProps(null)}
{...alertProps}
{ alertError && (
<AlertError
{...alertError}
onDismiss={() => setAlertError(null)}
/>
)}
<Outlet />

View File

@@ -29,8 +29,8 @@ const organizations = ['Org 1', 'Org 2'];
const context = {
toastMessage: null,
setToastMessage: jest.fn(),
alertProps: null,
setAlertProps: jest.fn(),
alertError: null,
setAlertError: jest.fn(),
};
const render = (ui: React.ReactElement) => baseRender(ui, {

View File

@@ -1,22 +1,17 @@
import React from 'react';
import type { AlertErrorProps } from '../../generic/alert-error';
export interface AlertProps {
/** title of the alert */
title: React.ReactNode;
/** description of the alert */
description: React.ReactNode;
}
// TODO: We shoud change the `toastMessage` and the `setToastMessage` to use the ToastContext
export interface TaxonomyContextData {
toastMessage: null | string;
setToastMessage: null | React.Dispatch<React.SetStateAction<null | string>>;
alertProps: null | AlertProps;
setAlertProps: null | React.Dispatch<React.SetStateAction<null | AlertProps>>;
alertError: null | AlertErrorProps;
setAlertError: null | React.Dispatch<React.SetStateAction<null | AlertErrorProps>>;
}
export const TaxonomyContext = React.createContext<TaxonomyContextData>({
toastMessage: null,
setToastMessage: null,
alertProps: null,
setAlertProps: null,
alertError: null,
setAlertError: null,
});

View File

@@ -16,7 +16,6 @@ import {
import {
DeleteOutline,
Download,
Error as ErrorIcon,
InsertDriveFile,
Warning,
} from '@openedx/paragon/icons';
@@ -286,7 +285,7 @@ const ImportTagsWizard = ({
reimport,
}) => {
const intl = useIntl();
const { setToastMessage, setAlertProps } = useContext(TaxonomyContext);
const { setToastMessage, setAlertError } = useContext(TaxonomyContext);
const [currentStep, setCurrentStep] = useState(reimport ? 'export' : 'upload');
@@ -315,16 +314,12 @@ const ImportTagsWizard = ({
if (setToastMessage) {
setToastMessage(intl.formatMessage(messages.importNewTaxonomyToast, { name: taxonomyName }));
}
} catch (/** @type {any} */ error) {
const alertProps = {
variant: 'danger',
icon: ErrorIcon,
title: intl.formatMessage(messages.importTaxonomyErrorAlert),
description: error.message,
};
if (setAlertProps) {
setAlertProps(alertProps);
} catch (/** @type {unknown} */ error) {
if (setAlertError) {
setAlertError({
title: intl.formatMessage(messages.importTaxonomyErrorAlert),
error,
});
}
} finally {
enableDialog();
@@ -369,16 +364,12 @@ const ImportTagsWizard = ({
if (setToastMessage) {
setToastMessage(intl.formatMessage(messages.importTaxonomyToast, { name: taxonomy?.name }));
}
} catch (/** @type {any} */ error) {
const alertProps = {
variant: 'danger',
icon: ErrorIcon,
title: intl.formatMessage(messages.importTaxonomyErrorAlert),
description: error.message,
};
if (setAlertProps) {
setAlertProps(alertProps);
} catch (/** @type {unknown} */ error) {
if (setAlertError) {
setAlertError({
title: intl.formatMessage(messages.importTaxonomyErrorAlert),
error,
});
}
} finally {
enableDialog();

View File

@@ -1,5 +1,4 @@
import MockAdapter from 'axios-mock-adapter';
import React from 'react';
import { IntlProvider } from '@edx/frontend-platform/i18n';
import { initializeMockApp } from '@edx/frontend-platform';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
@@ -30,12 +29,12 @@ jest.mock('../data/api', () => ({
}));
const mockSetToastMessage = jest.fn();
const mockSetAlertProps = jest.fn();
const mockSetAlertError = jest.fn();
const context = {
toastMessage: null,
setToastMessage: mockSetToastMessage,
alertProps: null,
setAlertProps: mockSetAlertProps,
setAlertError: mockSetAlertError,
};
const planImportUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/import/plan/';
@@ -230,16 +229,15 @@ describe('<ImportTagsWizard />', () => {
if (expectedResult === 'success') {
// Toast message shown
await waitFor(() => {
expect(mockSetToastMessage).toBeCalledWith(`"${sampleTaxonomy.name}" updated`);
expect(mockSetToastMessage).toHaveBeenCalledWith(`"${sampleTaxonomy.name}" updated`);
});
} else {
// Alert message shown
await waitFor(() => {
expect(mockSetAlertProps).toBeCalledWith(
expect(mockSetAlertError).toHaveBeenCalledWith(
expect.objectContaining({
variant: 'danger',
title: 'Import error',
description: 'Test error',
error: new Error('Test error'),
}),
);
});
@@ -340,15 +338,15 @@ describe('<ImportTagsWizard />', () => {
if (expectedResult === 'success') {
// Toast message shown
await waitFor(() => {
expect(mockSetToastMessage).toBeCalledWith(`"${newTaxonomyName}" imported`);
expect(mockSetToastMessage).toHaveBeenCalledWith(`"${newTaxonomyName}" imported`);
});
} else {
// Alert message shown
await waitFor(() => {
expect(mockSetAlertProps).toBeCalledWith(
expect(mockSetAlertError).toHaveBeenCalledWith(
expect.objectContaining({
variant: 'danger',
title: 'Import error',
error: new Error('Request failed with status code 400'),
}),
);
});

View File

@@ -45,10 +45,6 @@ const messages = defineMessages({
id: 'course-authoring.taxonomy-list.toast.delete',
defaultMessage: '"{name}" deleted',
},
taxonomyDismissLabel: {
id: 'course-authoring.taxonomy-list.alert.dismiss',
defaultMessage: 'Dismiss',
},
importInProgressAlertDescription: {
id: 'course-authoring.import-tags.prompt.in-progress',
defaultMessage: 'Please keep this window open. We\'ll let you know when it\'s done.',

View File

@@ -39,8 +39,8 @@ const TaxonomyMenuComponent: React.FC<{
const context = useMemo(() => ({
toastMessage: null,
setToastMessage: mockSetToastMessage,
alertProps: null,
setAlertProps: null,
alertError: null,
setAlertError: null,
}), []);
return (