🥅(frontend) improve add member form error handling

- translate known errors, including already existing
group error, and directly display the other ones
- add component tests
- update translations
- add parseAPIError, a reusable function to
catch errors on the whole frontend app

Closes issue #293

♻️(frontend) improve general error catching

- change parseAPIError to make it reusable on all
requests
- update components depending on it
This commit is contained in:
daproclaima
2024-09-10 15:01:36 +02:00
committed by Sebastien Nobour
parent 3d7dfa019c
commit ee5a785d43
11 changed files with 455 additions and 274 deletions

View File

@@ -13,8 +13,9 @@ and this project adheres to
- ✨(domains) add endpoint to list and retrieve domain accesses #404
- 🍱(dev) embark dimail-api as container #366
- ✨(dimail) allow la regie to request a token for another user #416
- ⚗️(frontend) show username on AccountDropDown #412
- ✨(dimail) allow la regie to request a token for another user #416
- (frontend) show username on AccountDropDown #412
- 🥅(frontend) improve add & update group forms error handling #387
### Changed

View File

@@ -0,0 +1,157 @@
import { APIError } from '@/api';
import {
parseAPIError,
parseAPIErrorCause,
parseServerAPIError,
} from '../parseAPIError';
describe('parseAPIError', () => {
const handleErrorMock = jest.fn();
const handleServerErrorMock = jest.fn();
beforeEach(() => {
jest.clearAllMocks();
});
it('should handle specific API error and return no unhandled causes', () => {
const error = new APIError('client error', {
cause: ['Mail domain with this name already exists.'],
status: 400,
});
const result = parseAPIError({
error,
errorParams: [
[
['Mail domain with this name already exists.'],
'This domain already exists.',
handleErrorMock,
],
],
serverErrorParams: ['Server error', handleServerErrorMock],
});
expect(handleErrorMock).toHaveBeenCalled();
expect(result).toEqual(['This domain already exists.']);
});
it('should return unhandled causes when no match is found', () => {
const error = new APIError('client error', {
cause: ['Unhandled error'],
status: 400,
});
const result = parseAPIError({
error,
errorParams: [
[
['Mail domain with this name already exists.'],
'This domain already exists.',
handleErrorMock,
],
],
serverErrorParams: ['Server error', handleServerErrorMock],
});
expect(handleErrorMock).not.toHaveBeenCalled();
expect(result).toEqual(['Unhandled error']);
});
it('should handle server errors correctly and prepend server error message', () => {
const error = new APIError('server error', { status: 500 });
const result = parseAPIError({
error,
errorParams: undefined,
serverErrorParams: ['Server error occurred', handleServerErrorMock],
});
expect(handleServerErrorMock).toHaveBeenCalled();
expect(result).toEqual(['Server error occurred']);
});
it('should handle absence of errors gracefully', () => {
const result = parseAPIError({
error: null,
errorParams: [
[
['Mail domain with this name already exists.'],
'This domain already exists.',
handleErrorMock,
],
],
serverErrorParams: ['Server error', handleServerErrorMock],
});
expect(result).toBeUndefined();
});
});
describe('parseAPIErrorCause', () => {
it('should handle specific errors and call handleError', () => {
const handleErrorMock = jest.fn();
const causes = ['Mail domain with this name already exists.'];
const errorParams: [string[], string, () => void][] = [
[
['Mail domain with this name already exists.'],
'This domain already exists.',
handleErrorMock,
],
];
const result = parseAPIErrorCause(
new APIError('client error', { cause: causes, status: 400 }),
errorParams,
);
expect(handleErrorMock).toHaveBeenCalled();
expect(result).toEqual(['This domain already exists.']);
});
it('should handle multiple causes and return unhandled causes', () => {
const handleErrorMock = jest.fn();
const causes = [
'Mail domain with this name already exists.',
'Unhandled error',
];
const errorParams: [string[], string, () => void][] = [
[
['Mail domain with this name already exists.'],
'This domain already exists.',
handleErrorMock,
],
];
const result = parseAPIErrorCause(
new APIError('client error', { cause: causes, status: 400 }),
errorParams,
);
expect(handleErrorMock).toHaveBeenCalled();
expect(result).toEqual(['This domain already exists.', 'Unhandled error']);
});
});
describe('parseServerAPIError', () => {
const handleServerErrorMock = jest.fn();
beforeEach(() => {
jest.clearAllMocks();
});
it('should return the server error message and handle callback', () => {
const result = parseServerAPIError(['Server error', handleServerErrorMock]);
expect(result).toEqual('Server error');
expect(handleServerErrorMock).toHaveBeenCalled();
});
it('should return only the server error message when no callback is provided', () => {
const result = parseServerAPIError(['Server error', undefined]);
expect(result).toEqual('Server error');
});
});

View File

@@ -1,174 +0,0 @@
import { APIError } from '@/api';
import {
parseAPIError,
parseAPIErrorCause,
parseServerAPIError,
} from '../parseAPIError';
describe('parseAPIError', () => {
const handleErrorMock = jest.fn();
const handleServerErrorMock = jest.fn();
beforeEach(() => {
jest.clearAllMocks();
});
it('should handle specific API error and return no unhandled causes', () => {
const error = new APIError('client error', {
cause: ['Mail domain with this name already exists.'],
status: 400,
});
const result = parseAPIError({
error,
errorParams: {
name: {
causes: ['Mail domain with this name already exists.'],
handleError: handleErrorMock,
},
},
serverErrorParams: {
defaultMessage: 'Server error',
handleError: handleServerErrorMock,
},
});
expect(handleErrorMock).toHaveBeenCalled();
expect(result).toEqual([]);
});
it('should return unhandled causes when no match is found', () => {
const error = new APIError('client error', {
cause: ['Unhandled error'],
status: 400,
});
const result = parseAPIError({
error,
errorParams: {
name: {
causes: ['Mail domain with this name already exists.'],
handleError: handleErrorMock,
},
},
serverErrorParams: {
defaultMessage: 'Server error',
handleError: handleServerErrorMock,
},
});
expect(handleErrorMock).not.toHaveBeenCalled();
expect(result).toEqual(['Unhandled error']);
});
it('should handle server errors correctly and prepend server error message', () => {
const error = new APIError('server error', { status: 500 });
const result = parseAPIError({
error,
errorParams: undefined,
serverErrorParams: {
defaultMessage: 'Server error occurred',
handleError: handleServerErrorMock,
},
});
expect(handleServerErrorMock).toHaveBeenCalled();
expect(result).toEqual(['Server error occurred']);
});
it('should handle absence of errors gracefully', () => {
const result = parseAPIError({
error: null,
errorParams: {
name: {
causes: ['Mail domain with this name already exists.'],
handleError: handleErrorMock,
},
},
serverErrorParams: {
defaultMessage: 'Server error',
handleError: handleServerErrorMock,
},
});
expect(result).toBeUndefined();
});
});
describe('parseAPIErrorCause', () => {
it('should handle specific errors and call handleError', () => {
const handleErrorMock = jest.fn();
const causes = ['Mail domain with this name already exists.'];
const errorParams = {
name: {
causes: ['Mail domain with this name already exists.'],
handleError: handleErrorMock,
},
};
const result = parseAPIErrorCause({ causes, errorParams });
expect(handleErrorMock).toHaveBeenCalled();
expect(result).toEqual([]);
});
it('should handle multiple causes and return unhandled causes', () => {
const handleErrorMock = jest.fn();
const causes = [
'Mail domain with this name already exists.',
'Unhandled error',
];
const errorParams = {
name: {
causes: ['Mail domain with this name already exists.'],
handleError: handleErrorMock,
},
};
const result = parseAPIErrorCause({ causes, errorParams });
expect(handleErrorMock).toHaveBeenCalled();
expect(result).toEqual(['Unhandled error']);
});
});
describe('parseServerAPIError', () => {
it('should prepend the server error message when there are other causes', () => {
const causes = ['Some other error'];
const serverErrorParams = {
defaultMessage: 'Server error',
};
const result = parseServerAPIError({ causes, serverErrorParams });
expect(result).toEqual(['Server error', 'Some other error']);
});
it('should only return server error message when no other causes exist', () => {
const causes: string[] = [];
const serverErrorParams = {
defaultMessage: 'Server error',
};
const result = parseServerAPIError({ causes, serverErrorParams });
expect(result).toEqual(['Server error']);
});
it('should call handleError when provided as a param', () => {
const handleErrorMock = jest.fn();
const causes: string[] = [];
const serverErrorParams = {
defaultMessage: 'Server error',
handleError: handleErrorMock,
};
parseServerAPIError({ causes, serverErrorParams });
expect(handleErrorMock).toHaveBeenCalled();
});
});

View File

@@ -1,85 +1,106 @@
import { APIError } from '@/api/index';
type ErrorParams = {
[fieldName: string]: {
causes: string[];
causeShown?: string;
handleError: () => void;
};
};
type ErrorCallback = () => void;
type ServerErrorParams = {
defaultMessage: string;
handleError?: () => void;
};
// Type for the error tuple format [causes, message, handleError]
type ErrorTuple = [string[], string, ErrorCallback | undefined];
export type parseAPIErrorParams = {
error: APIError | null;
errorParams?: ErrorParams;
serverErrorParams: ServerErrorParams;
};
// Server error tuple [defaultMessage, handleError]
type ServerErrorTuple = [string, ErrorCallback | undefined];
/**
* @function parseAPIError
* @description function to centralize APIError handling to treat discovered errors
* and error type 500 with default behavior using a simplified tuple structure.
* @param error - APIError object
* @param errorParams - Array of tuples: each contains an array of causes, a message, and an optional callback function.
* @param serverErrorParams - A tuple for server error handling: [defaultMessage, handleError]
* @returns Array of error messages or undefined
*/
export const parseAPIError = ({
error,
errorParams,
serverErrorParams,
}: parseAPIErrorParams) => {
if (!error || !serverErrorParams?.defaultMessage) {
}: {
error: APIError | null;
errorParams?: ErrorTuple[];
serverErrorParams?: ServerErrorTuple;
}): string[] | undefined => {
if (!error) {
return;
}
let causes: string[] =
// Parse known error causes using the tuple structure
const errorCauses =
error.cause?.length && errorParams
? parseAPIErrorCause({ causes: error.cause, errorParams })
: [];
? parseAPIErrorCause(error, errorParams)
: undefined;
if (error?.status === 500 || !error?.status) {
causes = parseServerAPIError({ causes, serverErrorParams });
// Check if it's a server error (500) and handle that case
const serverErrorCause =
(error?.status === 500 || !error?.status) && serverErrorParams
? parseServerAPIError(serverErrorParams)
: undefined;
// Combine the causes and return
const causes: string[] = errorCauses ? [...errorCauses] : [];
if (serverErrorCause) {
causes.unshift(serverErrorCause);
}
return causes;
return causes.length ? causes : undefined;
};
export const parseAPIErrorCause = ({
causes,
errorParams,
}: {
causes: string[];
errorParams: ErrorParams;
}): string[] =>
causes.reduce((arrayCauses, cause) => {
const foundErrorParams = Object.values(errorParams).find((params) =>
params.causes.find((knownCause) =>
new RegExp(knownCause, 'i').test(cause),
),
/**
* @function parseAPIErrorCause
* @description Processes known API error causes using the tuple structure.
* @param error - APIError object
* @param errorParams - Array of tuples: each contains an array of causes, a message, and an optional callback function.
* @returns Array of error messages
*/
export const parseAPIErrorCause = (
error: APIError,
errorParams: ErrorTuple[],
): string[] | undefined => {
if (!error.cause) {
return;
}
return error.cause.reduce((causes: string[], cause: string) => {
// Find the matching error tuple
const matchedError = errorParams.find(([errorCauses]) =>
errorCauses.some((knownCause) => new RegExp(knownCause, 'i').test(cause)),
);
if (!foundErrorParams) {
arrayCauses.push(cause);
if (matchedError) {
const [, message, handleError] = matchedError;
causes.push(message);
if (handleError) {
handleError();
}
} else {
// If no match is found, add the original cause
causes.push(cause);
}
if (foundErrorParams?.causeShown) {
arrayCauses.push(foundErrorParams.causeShown);
}
return causes;
}, []);
};
if (typeof foundErrorParams?.handleError === 'function') {
foundErrorParams.handleError();
}
return arrayCauses;
}, [] as string[]);
export const parseServerAPIError = ({
causes,
serverErrorParams,
}: {
causes: string[];
serverErrorParams: ServerErrorParams;
}): string[] => {
causes.unshift(serverErrorParams.defaultMessage);
if (typeof serverErrorParams?.handleError === 'function') {
serverErrorParams.handleError();
/**
* @function parseServerAPIError
* @description Handles server errors (500) and adds the default message.
* @param serverErrorParams - Tuple [defaultMessage, handleError]
* @returns Server error message
*/
export const parseServerAPIError = ([
defaultMessage,
handleError,
]: ServerErrorTuple): string => {
if (handleError) {
handleError();
}
return causes;
return defaultMessage;
};

View File

@@ -42,13 +42,14 @@ export const ModalAddMailDomain = () => {
onError: (error) => {
const unhandledCauses = parseAPIError({
error,
errorParams: {
name: {
causes: [
errorParams: [
[
[
'Mail domain with this name already exists.',
'Mail domain with this Slug already exists.',
],
handleError: () => {
'',
() => {
if (methods.formState.errors.name) {
return;
}
@@ -61,17 +62,17 @@ export const ModalAddMailDomain = () => {
});
methods.setFocus('name');
},
},
},
serverErrorParams: {
handleError: () => {
methods.setFocus('name');
},
defaultMessage: t(
],
],
serverErrorParams: [
t(
'Your request cannot be processed because the server is experiencing an error. If the problem ' +
'persists, please contact our support to resolve the issue: suiteterritoriale@anct.gouv.fr',
),
},
() => {
methods.setFocus('name');
},
],
});
setErrorCauses((prevState) =>

View File

@@ -90,48 +90,44 @@ export const ModalCreateMailbox = ({
closeModal();
},
onError: (error) => {
const unhandledCauses = parseAPIError({
const causes = parseAPIError({
error,
errorParams: {
local_part: {
causes: ['Mailbox with this Local_part and Domain already exists.'],
handleError: () => {
errorParams: [
[
['Mailbox with this Local_part and Domain already exists.'],
'',
() => {
methods.setError('local_part', {
type: 'manual',
message: t('This email prefix is already used.'),
});
methods.setFocus('local_part');
},
},
secret: {
causes: [
],
[
[
"Please configure your domain's secret before creating any mailbox.",
`Secret not valid for this domain`,
'Secret not valid for this domain',
],
causeShown: t(
t(
'The mail domain secret is misconfigured. Please, contact ' +
'our support team to solve the issue: suiteterritoriale@anct.gouv.fr',
),
handleError: () => {
methods.setFocus('first_name');
},
},
},
serverErrorParams: {
handleError: () => {
methods.setFocus('first_name');
},
defaultMessage: t(
() => methods.setFocus('first_name'),
],
],
serverErrorParams: [
t(
'Your request cannot be processed because the server is experiencing an error. If the problem ' +
'persists, please contact our support to resolve the issue: suiteterritoriale@anct.gouv.fr',
),
},
() => methods.setFocus('first_name'),
],
});
setErrorCauses((prevState) =>
unhandledCauses &&
JSON.stringify(unhandledCauses) !== JSON.stringify(prevState)
? unhandledCauses
causes && JSON.stringify(causes) !== JSON.stringify(prevState)
? causes
: prevState,
);
},

View File

@@ -14,6 +14,7 @@ import { InputTeamName } from './InputTeamName';
export const CardCreateTeam = () => {
const { t } = useTranslation();
const router = useRouter();
const {
mutate: createTeam,
isError,
@@ -24,6 +25,7 @@ export const CardCreateTeam = () => {
router.push(`/teams/${team.id}`);
},
});
const [teamName, setTeamName] = useState('');
const { colorsTokens } = useCunninghamTheme();

View File

@@ -1,7 +1,9 @@
import { Input, Loader } from '@openfun/cunningham-react';
import { useEffect, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { APIError } from '@/api';
import { parseAPIError } from '@/api/parseAPIError';
import { Box, TextErrors } from '@/components';
interface InputTeamNameProps {
@@ -21,6 +23,7 @@ export const InputTeamName = ({
label,
setTeamName,
}: InputTeamNameProps) => {
const { t } = useTranslation();
const [isInputError, setIsInputError] = useState(isError);
useEffect(() => {
@@ -29,6 +32,28 @@ export const InputTeamName = ({
}
}, [isError]);
const causes = error
? parseAPIError({
error,
errorParams: [
[
['Team with this Slug already exists.'],
t(
'This name is already used for another group. Please enter another one.',
),
undefined,
],
],
serverErrorParams: [
t(
'Your request cannot be processed because the server is experiencing an error. If the problem ' +
'persists, please contact our support to resolve the issue: suiteterritoriale@anct.gouv.fr',
),
undefined,
],
})
: undefined;
return (
<>
<Input
@@ -42,7 +67,7 @@ export const InputTeamName = ({
}}
state={isInputError ? 'error' : 'default'}
/>
{isError && error && <TextErrors causes={error.cause} />}
{isError && causes && <TextErrors causes={causes} />}
{isPending && (
<Box $align="center">
<Loader />

View File

@@ -0,0 +1,149 @@
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import fetchMock from 'fetch-mock';
import React from 'react';
import { AppWrapper } from '@/tests/utils';
import { CardCreateTeam } from '../CardCreateTeam';
const mockPush = jest.fn();
jest.mock('next/navigation', () => ({
useRouter: () => ({
push: mockPush,
}),
}));
describe('CardCreateTeam', () => {
const renderCardCreateTeam = () =>
render(<CardCreateTeam />, { wrapper: AppWrapper });
beforeEach(() => {
jest.clearAllMocks();
});
afterEach(() => {
fetchMock.restore();
});
it('renders all the elements', () => {
renderCardCreateTeam();
expect(screen.getByLabelText('Create new team card')).toBeInTheDocument();
expect(screen.getByText('Create a new group')).toBeInTheDocument();
expect(screen.getByLabelText('Team name')).toBeInTheDocument();
expect(screen.getByText('Cancel')).toBeInTheDocument();
expect(screen.getByText('Create the team')).toBeInTheDocument();
});
it('handles input for team name and enables submit button', async () => {
const user = userEvent.setup();
renderCardCreateTeam();
const teamNameInput = screen.getByLabelText('Team name');
const createButton = screen.getByText('Create the team');
expect(createButton).toBeDisabled();
await user.type(teamNameInput, 'New Team');
expect(createButton).toBeEnabled();
});
it('creates a team successfully and redirects', async () => {
fetchMock.post('end:teams/', {
id: '270328ea-c2c0-4f74-a449-5cdc976dcdb6',
name: 'New Team',
});
const user = userEvent.setup();
renderCardCreateTeam();
const teamNameInput = screen.getByLabelText('Team name');
const createButton = screen.getByText('Create the team');
await user.type(teamNameInput, 'New Team');
await user.click(createButton);
await waitFor(() => {
expect(mockPush).toHaveBeenCalledWith(
'/teams/270328ea-c2c0-4f74-a449-5cdc976dcdb6',
);
});
expect(fetchMock.calls()).toHaveLength(1);
expect(fetchMock.lastCall()?.[0]).toContain('/teams/');
expect(fetchMock.lastCall()?.[1]?.body).toEqual(
JSON.stringify({ name: 'New Team' }),
);
});
it('displays an error message when team name already exists', async () => {
fetchMock.post('end:teams/', {
body: {
cause: ['Team with this Slug already exists.'],
},
status: 400,
});
const user = userEvent.setup();
renderCardCreateTeam();
const teamNameInput = screen.getByLabelText('Team name');
const createButton = screen.getByText('Create the team');
await user.type(teamNameInput, 'Existing Team');
await user.click(createButton);
await waitFor(() => {
expect(
screen.getByText(/This name is already used for another group/i),
).toBeInTheDocument();
});
});
it('handles server error gracefully', async () => {
fetchMock.post('end:/teams/', {
body: {},
status: 500,
});
const user = userEvent.setup();
renderCardCreateTeam();
const teamNameInput = screen.getByLabelText('Team name');
const createButton = screen.getByText('Create the team');
await user.type(teamNameInput, 'Server Error Team');
await user.click(createButton);
await waitFor(() => {
expect(
screen.getByText(
/Your request cannot be processed because the server is experiencing an error/i,
),
).toBeInTheDocument();
});
expect(fetchMock.calls()).toHaveLength(1);
expect(fetchMock.lastCall()?.[0]).toContain('/teams/');
expect(fetchMock.lastCall()?.[1]?.body).toEqual(
JSON.stringify({ name: 'Server Error Team' }),
);
});
it('disables create button when API request is pending', async () => {
// Never resolves
fetchMock.post('end:teams/', new Promise(() => {}));
const user = userEvent.setup();
renderCardCreateTeam();
const teamNameInput = screen.getByLabelText('Team name');
const createButton = screen.getByText('Create the team');
await user.type(teamNameInput, 'Pending Team');
await user.click(createButton);
expect(createButton).toBeDisabled();
});
});

View File

@@ -147,6 +147,7 @@
"This domain name is deactivated. No new mailboxes can be created.": "Ce nom de domaine est désactivé. Aucune nouvelle boîte mail ne peut être créée.",
"This email prefix is already used.": "Ce préfixe d'email est déjà utilisé.",
"This mail domain is already used. Please, choose another one.": "Ce domaine de messagerie est déjà utilisé. Veuillez en choisir un autre.",
"This name is already used for another group. Please enter another one.": "Un autre groupe utilise déjà ce nom. Veuillez en saisir un autre.",
"This procedure is to be used in the following case: you have reported to the website \n manager an accessibility defect which prevents you from accessing content or one of the \n portal's services and you have not obtained a satisfactory response.": "Cette procédure est à utiliser dans le cas suivant : vous avez signalé au responsable du site internet un défaut daccessibilité qui vous empêche daccéder à un contenu ou à un des services du portail et vous navez pas obtenu de réponse satisfaisante.",
"This site does not display a cookie consent banner, why?": "Ce site n'affiche pas de bannière de consentement des cookies, pourquoi?",
"This site places a small text file (a \"cookie\") on your computer when you visit it.": "Ce site place un petit fichier texte (un « cookie ») sur votre ordinateur lorsque vous le visitez.",

View File

@@ -108,7 +108,9 @@ test.describe('Teams Create', () => {
await page.getByRole('button', { name: 'Create the team' }).click();
await expect(
page.getByText('Team with this Slug already exists.'),
page.getByText(
'This name is already used for another group. Please enter another one.',
),
).toBeVisible();
});