From 54f509ee38e33a75e342e971222dfef23188c5dc Mon Sep 17 00:00:00 2001 From: "kasia.oczkowska" Date: Fri, 22 Nov 2024 14:48:25 +0000 Subject: [PATCH 1/4] feat: introduce inline error messages for InstallationForm and SignUpForm --- packages/web/src/components/Form/index.jsx | 7 +- .../src/components/InstallationForm/index.jsx | 207 ++++++++++-------- .../src/components/SignUpForm/index.ee.jsx | 156 +++++++------ packages/web/src/helpers/errors.js | 18 ++ packages/web/src/locales/en.json | 2 + 5 files changed, 222 insertions(+), 168 deletions(-) create mode 100644 packages/web/src/helpers/errors.js diff --git a/packages/web/src/components/Form/index.jsx b/packages/web/src/components/Form/index.jsx index 614873f7..352b0a69 100644 --- a/packages/web/src/components/Form/index.jsx +++ b/packages/web/src/components/Form/index.jsx @@ -46,7 +46,12 @@ function Form(props) { return ( -
+ + onSubmit?.(data, event, methods.setError), + )} + {...formProps} + > {render ? render(methods) : children}
diff --git a/packages/web/src/components/InstallationForm/index.jsx b/packages/web/src/components/InstallationForm/index.jsx index 80d66b6c..6e443302 100644 --- a/packages/web/src/components/InstallationForm/index.jsx +++ b/packages/web/src/components/InstallationForm/index.jsx @@ -2,35 +2,55 @@ import * as React from 'react'; import { Link as RouterLink } from 'react-router-dom'; import Paper from '@mui/material/Paper'; import Typography from '@mui/material/Typography'; -import { Alert } from '@mui/material'; +import Alert from '@mui/material/Alert'; import LoadingButton from '@mui/lab/LoadingButton'; import * as yup from 'yup'; import { yupResolver } from '@hookform/resolvers/yup'; -import { enqueueSnackbar } from 'notistack'; import { useQueryClient } from '@tanstack/react-query'; import Link from '@mui/material/Link'; +import { getGeneralErrorMessage } from 'helpers/errors'; import useFormatMessage from 'hooks/useFormatMessage'; import useInstallation from 'hooks/useInstallation'; import * as URLS from 'config/urls'; import Form from 'components/Form'; import TextField from 'components/TextField'; -const validationSchema = yup.object().shape({ - fullName: yup.string().trim().required('installationForm.mandatoryInput'), - email: yup - .string() - .trim() - .email('installationForm.validateEmail') - .required('installationForm.mandatoryInput'), - password: yup.string().required('installationForm.mandatoryInput'), - confirmPassword: yup - .string() - .required('installationForm.mandatoryInput') - .oneOf([yup.ref('password')], 'installationForm.passwordsMustMatch'), -}); +const getValidationSchema = (formatMessage) => { + const getMandatoryInputMessage = (inputNameId) => + formatMessage('installationForm.mandatoryInput', { + inputName: formatMessage(inputNameId), + }); -const initialValues = { + return yup.object().shape({ + fullName: yup + .string() + .trim() + .required( + getMandatoryInputMessage('installationForm.fullNameFieldLabel'), + ), + email: yup + .string() + .trim() + .required(getMandatoryInputMessage('installationForm.emailFieldLabel')) + .email(formatMessage('installationForm.validateEmail')), + password: yup + .string() + .required(getMandatoryInputMessage('installationForm.passwordFieldLabel')) + .min(6, formatMessage('installationForm.passwordMinLength')), + confirmPassword: yup + .string() + .required( + getMandatoryInputMessage('installationForm.confirmPasswordFieldLabel'), + ) + .oneOf( + [yup.ref('password')], + formatMessage('installationForm.passwordsMustMatch'), + ), + }); +}; + +const defaultValues = { fullName: '', email: '', password: '', @@ -39,7 +59,7 @@ const initialValues = { function InstallationForm() { const formatMessage = useFormatMessage(); - const install = useInstallation(); + const { mutateAsync: install, isSuccess, isPending } = useInstallation(); const queryClient = useQueryClient(); const handleOnRedirect = () => { @@ -48,21 +68,38 @@ function InstallationForm() { }); }; - const handleSubmit = async (values) => { - const { fullName, email, password } = values; + const handleSubmit = async ({ fullName, email, password }, e, setError) => { try { - await install.mutateAsync({ + await install({ fullName, email, password, }); } catch (error) { - enqueueSnackbar( - error?.message || formatMessage('installationForm.error'), - { - variant: 'error', - }, - ); + const errors = error?.response?.data?.errors; + if (errors) { + const fieldNames = Object.keys(defaultValues); + Object.entries(errors).forEach(([fieldName, fieldErrors]) => { + if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { + setError(fieldName, { + type: 'fieldRequestError', + message: fieldErrors.join(', '), + }); + } + }); + } + + const generalError = getGeneralErrorMessage({ + error, + fallbackMessage: formatMessage('installationForm.error'), + }); + + if (generalError) { + setError('root.general', { + type: 'requestError', + message: generalError, + }); + } } }; @@ -82,11 +119,13 @@ function InstallationForm() { {formatMessage('installationForm.title')}
( + render={({ formState: { errors } }) => ( <> + + + + {errors?.root?.general && ( + + {errors.root.general.message} + + )} + + {isSuccess && ( + + {formatMessage('installationForm.success', { + link: (str) => ( + + {str} + + ), + })} + + )} {formatMessage('installationForm.submit')} )} /> - {install.isSuccess && ( - - {formatMessage('installationForm.success', { - link: (str) => ( - - {str} - - ), - })} - - )} ); } diff --git a/packages/web/src/components/SignUpForm/index.ee.jsx b/packages/web/src/components/SignUpForm/index.ee.jsx index e931d394..3cbc1033 100644 --- a/packages/web/src/components/SignUpForm/index.ee.jsx +++ b/packages/web/src/components/SignUpForm/index.ee.jsx @@ -3,33 +3,52 @@ import { useNavigate } from 'react-router-dom'; import Paper from '@mui/material/Paper'; import Typography from '@mui/material/Typography'; import LoadingButton from '@mui/lab/LoadingButton'; +import Alert from '@mui/material/Alert'; import * as yup from 'yup'; import { yupResolver } from '@hookform/resolvers/yup'; +import { getGeneralErrorMessage } from 'helpers/errors'; import useAuthentication from 'hooks/useAuthentication'; import * as URLS from 'config/urls'; import Form from 'components/Form'; import TextField from 'components/TextField'; import useFormatMessage from 'hooks/useFormatMessage'; import useCreateAccessToken from 'hooks/useCreateAccessToken'; -import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; import useRegisterUser from 'hooks/useRegisterUser'; -const validationSchema = yup.object().shape({ - fullName: yup.string().trim().required('signupForm.mandatoryInput'), - email: yup - .string() - .trim() - .email('signupForm.validateEmail') - .required('signupForm.mandatoryInput'), - password: yup.string().required('signupForm.mandatoryInput'), - confirmPassword: yup - .string() - .required('signupForm.mandatoryInput') - .oneOf([yup.ref('password')], 'signupForm.passwordsMustMatch'), -}); +const getValidationSchema = (formatMessage) => { + const getMandatoryInputMessage = (inputNameId) => + formatMessage('signupForm.mandatoryInput', { + inputName: formatMessage(inputNameId), + }); -const initialValues = { + return yup.object().shape({ + fullName: yup + .string() + .trim() + .required(getMandatoryInputMessage('signupForm.fullNameFieldLabel')), + email: yup + .string() + .trim() + .required(getMandatoryInputMessage('signupForm.emailFieldLabel')) + .email(formatMessage('signupForm.validateEmail')), + password: yup + .string() + .required(getMandatoryInputMessage('signupForm.passwordFieldLabel')) + .min(6, formatMessage('signupForm.passwordMinLength')), + confirmPassword: yup + .string() + .required( + getMandatoryInputMessage('signupForm.confirmPasswordFieldLabel'), + ) + .oneOf( + [yup.ref('password')], + formatMessage('signupForm.passwordsMustMatch'), + ), + }); +}; + +const defaultValues = { fullName: '', email: '', password: '', @@ -40,7 +59,6 @@ function SignUpForm() { const navigate = useNavigate(); const authentication = useAuthentication(); const formatMessage = useFormatMessage(); - const enqueueSnackbar = useEnqueueSnackbar(); const { mutateAsync: registerUser, isPending: isRegisterUserPending } = useRegisterUser(); const { mutateAsync: createAccessToken, isPending: loginLoading } = @@ -52,7 +70,7 @@ function SignUpForm() { } }, [authentication.isAuthenticated]); - const handleSubmit = async (values) => { + const handleSubmit = async (values, e, setError) => { try { const { fullName, email, password } = values; await registerUser({ @@ -67,25 +85,28 @@ function SignUpForm() { const { token } = data; authentication.updateToken(token); } catch (error) { - const errors = error?.response?.data?.errors - ? Object.values(error.response.data.errors) - : []; + const errors = error?.response?.data?.errors; + if (errors) { + const fieldNames = Object.keys(defaultValues); + Object.entries(errors).forEach(([fieldName, fieldErrors]) => { + if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { + setError(fieldName, { + type: 'fieldRequestError', + message: fieldErrors.join(', '), + }); + } + }); + } - if (errors.length) { - for (const [error] of errors) { - enqueueSnackbar(error, { - variant: 'error', - SnackbarProps: { - 'data-test': 'snackbar-sign-up-error', - }, - }); - } - } else { - enqueueSnackbar(error?.message || formatMessage('signupForm.error'), { - variant: 'error', - SnackbarProps: { - 'data-test': 'snackbar-sign-up-error', - }, + const generalError = getGeneralErrorMessage({ + error, + fallbackMessage: formatMessage('signupForm.error'), + }); + + if (generalError) { + setError('root.general', { + type: 'requestError', + message: generalError, }); } } @@ -108,11 +129,13 @@ function SignUpForm() { ( + render={({ formState: { errors } }) => ( <> + {errors?.root?.general && ( + + {errors.root.general.message} + + )} + { + if (!error) { + return; + } + + const errors = error?.response?.data?.errors; + const generalError = errors?.general; + + if (generalError && Array.isArray(generalError)) { + return generalError.join(' '); + } + + if (!errors) { + return error?.message || fallbackMessage; + } +}; diff --git a/packages/web/src/locales/en.json b/packages/web/src/locales/en.json index b121f5e2..941af1fa 100644 --- a/packages/web/src/locales/en.json +++ b/packages/web/src/locales/en.json @@ -138,6 +138,7 @@ "installationForm.submit": "Create admin", "installationForm.validateEmail": "Email must be valid.", "installationForm.passwordsMustMatch": "Passwords must match.", + "installationForm.passwordMinLength": "Password must be at least 6 characters long.", "installationForm.mandatoryInput": "{inputName} is required.", "installationForm.success": "The admin account has been created, and thus, the installation has been completed. You can now log in here.", "installationForm.error": "Something went wrong. Please try again.", @@ -149,6 +150,7 @@ "signupForm.submit": "Sign up", "signupForm.validateEmail": "Email must be valid.", "signupForm.passwordsMustMatch": "Passwords must match.", + "signupForm.passwordMinLength": "Password must be at least 6 characters long.", "signupForm.mandatoryInput": "{inputName} is required.", "signupForm.error": "Something went wrong. Please try again.", "loginForm.title": "Login", From 3de18ab46f8fc84553660b9addf32ca51ea400e2 Mon Sep 17 00:00:00 2001 From: "Jakub P." Date: Tue, 10 Dec 2024 00:44:03 +0100 Subject: [PATCH 2/4] test: use updated selector for create admin button in installation form --- .../e2e-tests/fixtures/admin-setup-page.js | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/e2e-tests/fixtures/admin-setup-page.js b/packages/e2e-tests/fixtures/admin-setup-page.js index 704a9caf..6d5b85c2 100644 --- a/packages/e2e-tests/fixtures/admin-setup-page.js +++ b/packages/e2e-tests/fixtures/admin-setup-page.js @@ -1,4 +1,4 @@ -import { BasePage } from "./base-page"; +import { BasePage } from './base-page'; const { faker } = require('@faker-js/faker'); const { expect } = require('@playwright/test'); @@ -6,16 +6,18 @@ export class AdminSetupPage extends BasePage { path = '/installation'; /** - * @param {import('@playwright/test').Page} page - */ + * @param {import('@playwright/test').Page} page + */ constructor(page) { super(page); this.fullNameTextField = this.page.getByTestId('fullName-text-field'); this.emailTextField = this.page.getByTestId('email-text-field'); this.passwordTextField = this.page.getByTestId('password-text-field'); - this.repeatPasswordTextField = this.page.getByTestId('repeat-password-text-field'); - this.createAdminButton = this.page.getByTestId('signUp-button'); + this.repeatPasswordTextField = this.page.getByTestId( + 'repeat-password-text-field' + ); + this.createAdminButton = this.page.getByTestId('installation-button'); this.invalidFields = this.page.locator('p.Mui-error'); this.successAlert = this.page.getByTestId('success-alert'); } @@ -46,7 +48,7 @@ export class AdminSetupPage extends BasePage { await this.repeatPasswordTextField.fill(testUser.wronglyRepeatedPassword); } - async submitAdminForm() { + async submitAdminForm() { await this.createAdminButton.click(); } @@ -59,7 +61,10 @@ export class AdminSetupPage extends BasePage { } async expectSuccessMessageToContainLoginLink() { - await expect(await this.successAlert.locator('a')).toHaveAttribute('href', '/login'); + await expect(await this.successAlert.locator('a')).toHaveAttribute( + 'href', + '/login' + ); } generateUser() { @@ -69,7 +74,7 @@ export class AdminSetupPage extends BasePage { fullName: faker.person.fullName(), email: faker.internet.email(), password: faker.internet.password(), - wronglyRepeatedPassword: faker.internet.password() + wronglyRepeatedPassword: faker.internet.password(), }; } -}; +} From ce4e4b48854b49c6faba780ccd17f7fd62483c05 Mon Sep 17 00:00:00 2001 From: Ali BARIN Date: Tue, 17 Dec 2024 17:32:31 +0000 Subject: [PATCH 3/4] refactor(Form): centralize error management --- packages/web/src/components/Form/index.jsx | 50 +++++++++++++++++-- .../src/components/InstallationForm/index.jsx | 25 +--------- packages/web/src/hooks/useFormatMessage.js | 10 +++- packages/web/src/locales/en.json | 2 +- 4 files changed, 59 insertions(+), 28 deletions(-) diff --git a/packages/web/src/components/Form/index.jsx b/packages/web/src/components/Form/index.jsx index 352b0a69..d501e5a8 100644 --- a/packages/web/src/components/Form/index.jsx +++ b/packages/web/src/components/Form/index.jsx @@ -2,6 +2,7 @@ import * as React from 'react'; import { FormProvider, useForm, useWatch } from 'react-hook-form'; import PropTypes from 'prop-types'; import isEqual from 'lodash/isEqual'; +import useFormatMessage from 'hooks/useFormatMessage'; const noop = () => null; @@ -18,6 +19,8 @@ function Form(props) { ...formProps } = props; + const formatMessage = useFormatMessage(); + const methods = useForm({ defaultValues, reValidateMode, @@ -25,6 +28,8 @@ function Form(props) { mode, }); + const { setError } = methods; + const form = useWatch({ control: methods.control }); const prevDefaultValues = React.useRef(defaultValues); @@ -44,12 +49,51 @@ function Form(props) { } }, [defaultValues]); + const handleErrors = React.useCallback( + function (errors) { + if (!errors) return; + + let shouldSetGenericGeneralError = true; + const fieldNames = Object.keys(defaultValues); + + Object.entries(errors).forEach(([fieldName, fieldErrors]) => { + if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { + shouldSetGenericGeneralError = false; + setError(fieldName, { + type: 'fieldRequestError', + message: fieldErrors.join(', '), + }); + } + }); + + // in case of general errors + if (Array.isArray(errors.general)) { + for (const error of errors.general) { + shouldSetGenericGeneralError = false; + setError('root.general', { type: 'requestError', message: error }); + } + } + + if (shouldSetGenericGeneralError) { + setError('root.general', { + type: 'requestError', + message: formatMessage('form.genericError'), + }); + } + }, + [defaultValues, formatMessage, setError], + ); + return ( - onSubmit?.(data, event, methods.setError), - )} + onSubmit={methods.handleSubmit(async (data, event) => { + try { + return await onSubmit?.(data); + } catch (errors) { + handleErrors(errors); + } + })} {...formProps} > {render ? render(methods) : children} diff --git a/packages/web/src/components/InstallationForm/index.jsx b/packages/web/src/components/InstallationForm/index.jsx index 6e443302..f1e30051 100644 --- a/packages/web/src/components/InstallationForm/index.jsx +++ b/packages/web/src/components/InstallationForm/index.jsx @@ -68,7 +68,7 @@ function InstallationForm() { }); }; - const handleSubmit = async ({ fullName, email, password }, e, setError) => { + const handleSubmit = async ({ fullName, email, password }) => { try { await install({ fullName, @@ -77,29 +77,8 @@ function InstallationForm() { }); } catch (error) { const errors = error?.response?.data?.errors; - if (errors) { - const fieldNames = Object.keys(defaultValues); - Object.entries(errors).forEach(([fieldName, fieldErrors]) => { - if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { - setError(fieldName, { - type: 'fieldRequestError', - message: fieldErrors.join(', '), - }); - } - }); - } - const generalError = getGeneralErrorMessage({ - error, - fallbackMessage: formatMessage('installationForm.error'), - }); - - if (generalError) { - setError('root.general', { - type: 'requestError', - message: generalError, - }); - } + throw errors; } }; diff --git a/packages/web/src/hooks/useFormatMessage.js b/packages/web/src/hooks/useFormatMessage.js index ef76fe0c..62e95e28 100644 --- a/packages/web/src/hooks/useFormatMessage.js +++ b/packages/web/src/hooks/useFormatMessage.js @@ -1,5 +1,13 @@ +import * as React from 'react'; import { useIntl } from 'react-intl'; + export default function useFormatMessage() { const { formatMessage } = useIntl(); - return (id, values = {}) => formatMessage({ id }, values); + + const customFormatMessage = React.useCallback( + (id, values = {}) => formatMessage({ id }, values), + [formatMessage], + ); + + return customFormatMessage; } diff --git a/packages/web/src/locales/en.json b/packages/web/src/locales/en.json index 941af1fa..19c8aec4 100644 --- a/packages/web/src/locales/en.json +++ b/packages/web/src/locales/en.json @@ -130,6 +130,7 @@ "webhookUrlInfo.description": "You'll need to configure your application with this webhook URL.", "webhookUrlInfo.helperText": "We've generated a custom webhook URL for you to send requests to. Learn more about webhooks.", "webhookUrlInfo.copy": "Copy", + "form.genericError": "Something went wrong. Please try again.", "installationForm.title": "Installation", "installationForm.fullNameFieldLabel": "Full name", "installationForm.emailFieldLabel": "Email", @@ -141,7 +142,6 @@ "installationForm.passwordMinLength": "Password must be at least 6 characters long.", "installationForm.mandatoryInput": "{inputName} is required.", "installationForm.success": "The admin account has been created, and thus, the installation has been completed. You can now log in here.", - "installationForm.error": "Something went wrong. Please try again.", "signupForm.title": "Sign up", "signupForm.fullNameFieldLabel": "Full name", "signupForm.emailFieldLabel": "Email", From ebc21e90acfba87293261dd7c515616587e2a6bf Mon Sep 17 00:00:00 2001 From: "kasia.oczkowska" Date: Thu, 19 Dec 2024 13:23:33 +0000 Subject: [PATCH 4/4] refactor: remove redundant errors setting in SignUpForm --- .../src/components/InstallationForm/index.jsx | 4 +-- .../src/components/SignUpForm/index.ee.jsx | 27 ++----------------- packages/web/src/helpers/errors.js | 18 ------------- packages/web/src/locales/en.json | 1 - 4 files changed, 3 insertions(+), 47 deletions(-) delete mode 100644 packages/web/src/helpers/errors.js diff --git a/packages/web/src/components/InstallationForm/index.jsx b/packages/web/src/components/InstallationForm/index.jsx index f1e30051..bba4d314 100644 --- a/packages/web/src/components/InstallationForm/index.jsx +++ b/packages/web/src/components/InstallationForm/index.jsx @@ -9,7 +9,6 @@ import { yupResolver } from '@hookform/resolvers/yup'; import { useQueryClient } from '@tanstack/react-query'; import Link from '@mui/material/Link'; -import { getGeneralErrorMessage } from 'helpers/errors'; import useFormatMessage from 'hooks/useFormatMessage'; import useInstallation from 'hooks/useInstallation'; import * as URLS from 'config/urls'; @@ -77,8 +76,7 @@ function InstallationForm() { }); } catch (error) { const errors = error?.response?.data?.errors; - - throw errors; + throw errors || error; } }; diff --git a/packages/web/src/components/SignUpForm/index.ee.jsx b/packages/web/src/components/SignUpForm/index.ee.jsx index 3cbc1033..a7daa326 100644 --- a/packages/web/src/components/SignUpForm/index.ee.jsx +++ b/packages/web/src/components/SignUpForm/index.ee.jsx @@ -7,7 +7,6 @@ import Alert from '@mui/material/Alert'; import * as yup from 'yup'; import { yupResolver } from '@hookform/resolvers/yup'; -import { getGeneralErrorMessage } from 'helpers/errors'; import useAuthentication from 'hooks/useAuthentication'; import * as URLS from 'config/urls'; import Form from 'components/Form'; @@ -70,7 +69,7 @@ function SignUpForm() { } }, [authentication.isAuthenticated]); - const handleSubmit = async (values, e, setError) => { + const handleSubmit = async (values) => { try { const { fullName, email, password } = values; await registerUser({ @@ -86,29 +85,7 @@ function SignUpForm() { authentication.updateToken(token); } catch (error) { const errors = error?.response?.data?.errors; - if (errors) { - const fieldNames = Object.keys(defaultValues); - Object.entries(errors).forEach(([fieldName, fieldErrors]) => { - if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { - setError(fieldName, { - type: 'fieldRequestError', - message: fieldErrors.join(', '), - }); - } - }); - } - - const generalError = getGeneralErrorMessage({ - error, - fallbackMessage: formatMessage('signupForm.error'), - }); - - if (generalError) { - setError('root.general', { - type: 'requestError', - message: generalError, - }); - } + throw errors || error; } }; diff --git a/packages/web/src/helpers/errors.js b/packages/web/src/helpers/errors.js deleted file mode 100644 index dc73867d..00000000 --- a/packages/web/src/helpers/errors.js +++ /dev/null @@ -1,18 +0,0 @@ -// Helpers to extract errors received from the API - -export const getGeneralErrorMessage = ({ error, fallbackMessage }) => { - if (!error) { - return; - } - - const errors = error?.response?.data?.errors; - const generalError = errors?.general; - - if (generalError && Array.isArray(generalError)) { - return generalError.join(' '); - } - - if (!errors) { - return error?.message || fallbackMessage; - } -}; diff --git a/packages/web/src/locales/en.json b/packages/web/src/locales/en.json index 19c8aec4..373521e0 100644 --- a/packages/web/src/locales/en.json +++ b/packages/web/src/locales/en.json @@ -152,7 +152,6 @@ "signupForm.passwordsMustMatch": "Passwords must match.", "signupForm.passwordMinLength": "Password must be at least 6 characters long.", "signupForm.mandatoryInput": "{inputName} is required.", - "signupForm.error": "Something went wrong. Please try again.", "loginForm.title": "Login", "loginForm.emailFieldLabel": "Email", "loginForm.passwordFieldLabel": "Password",