From e784215e3ccc5470cc08fd805f3af7feef7b5107 Mon Sep 17 00:00:00 2001 From: "kasia.oczkowska" Date: Fri, 13 Dec 2024 11:12:04 +0000 Subject: [PATCH 1/2] feat: introduce inline error messages in SamlConfiguration and RoleMappings forms --- packages/web/src/helpers/errors.js | 18 + .../hooks/useAdminCreateSamlAuthProvider.js | 15 - .../hooks/useAdminUpdateSamlAuthProvider.js | 15 - packages/web/src/hooks/useSamlAuthProvider.js | 2 +- packages/web/src/locales/en.json | 5 +- .../src/pages/Authentication/RoleMappings.jsx | 95 +++-- .../Authentication/SamlConfiguration.jsx | 338 ++++++++++++------ 7 files changed, 314 insertions(+), 174 deletions(-) create mode 100644 packages/web/src/helpers/errors.js diff --git a/packages/web/src/helpers/errors.js b/packages/web/src/helpers/errors.js new file mode 100644 index 00000000..dc73867d --- /dev/null +++ b/packages/web/src/helpers/errors.js @@ -0,0 +1,18 @@ +// 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/hooks/useAdminCreateSamlAuthProvider.js b/packages/web/src/hooks/useAdminCreateSamlAuthProvider.js index 5cf8c6bc..33fa7e79 100644 --- a/packages/web/src/hooks/useAdminCreateSamlAuthProvider.js +++ b/packages/web/src/hooks/useAdminCreateSamlAuthProvider.js @@ -1,6 +1,5 @@ import { useMutation, useQueryClient } from '@tanstack/react-query'; import api from 'helpers/api'; -import { enqueueSnackbar } from 'notistack'; export default function useAdminCreateSamlAuthProvider() { const queryClient = useQueryClient(); @@ -16,20 +15,6 @@ export default function useAdminCreateSamlAuthProvider() { queryKey: ['admin', 'samlAuthProviders'], }); }, - onError: (error) => { - const errors = Object.entries( - error.response.data.errors || [['', 'Failed while saving!']], - ); - - for (const error of errors) { - enqueueSnackbar(`${error[0] ? error[0] + ': ' : ''} ${error[1]}`, { - variant: 'error', - SnackbarProps: { - 'data-test': 'snackbar-create-saml-auth-provider-error', - }, - }); - } - }, }); return query; diff --git a/packages/web/src/hooks/useAdminUpdateSamlAuthProvider.js b/packages/web/src/hooks/useAdminUpdateSamlAuthProvider.js index ab7dd2fa..9e6600f4 100644 --- a/packages/web/src/hooks/useAdminUpdateSamlAuthProvider.js +++ b/packages/web/src/hooks/useAdminUpdateSamlAuthProvider.js @@ -1,6 +1,5 @@ import { useMutation, useQueryClient } from '@tanstack/react-query'; import api from 'helpers/api'; -import { enqueueSnackbar } from 'notistack'; export default function useAdminUpdateSamlAuthProvider(samlAuthProviderId) { const queryClient = useQueryClient(); @@ -19,20 +18,6 @@ export default function useAdminUpdateSamlAuthProvider(samlAuthProviderId) { queryKey: ['admin', 'samlAuthProviders'], }); }, - onError: (error) => { - const errors = Object.entries( - error.response.data.errors || [['', 'Failed while saving!']], - ); - - for (const error of errors) { - enqueueSnackbar(`${error[0] ? error[0] + ': ' : ''} ${error[1]}`, { - variant: 'error', - SnackbarProps: { - 'data-test': 'snackbar-update-saml-auth-provider-error', - }, - }); - } - }, }); return query; diff --git a/packages/web/src/hooks/useSamlAuthProvider.js b/packages/web/src/hooks/useSamlAuthProvider.js index c293eafd..ee575beb 100644 --- a/packages/web/src/hooks/useSamlAuthProvider.js +++ b/packages/web/src/hooks/useSamlAuthProvider.js @@ -4,7 +4,7 @@ import api from 'helpers/api'; export default function useSamlAuthProvider({ samlAuthProviderId } = {}) { const query = useQuery({ - queryKey: ['samlAuthProviders', samlAuthProviderId], + queryKey: ['admin', 'samlAuthProviders', samlAuthProviderId], queryFn: async ({ signal }) => { const { data } = await api.get( `/v1/admin/saml-auth-providers/${samlAuthProviderId}`, diff --git a/packages/web/src/locales/en.json b/packages/web/src/locales/en.json index c9422557..b03c96e0 100644 --- a/packages/web/src/locales/en.json +++ b/packages/web/src/locales/en.json @@ -282,13 +282,16 @@ "authenticationForm.defaultRole": "Default role", "authenticationForm.successfullySaved": "The provider has been saved.", "authenticationForm.save": "Save", + "authenticationForm.mandatoryInput": "{inputName} is required.", + "authenticationForm.error": "Failed while saving!", "roleMappingsForm.title": "Role mappings", "roleMappingsForm.remoteRoleName": "Remote role name", "roleMappingsForm.role": "Role", "roleMappingsForm.appendRoleMapping": "Append", "roleMappingsForm.save": "Save", - "roleMappingsForm.notFound": "No role mappings have found.", + "roleMappingsForm.notFound": "No role mappings have been found.", "roleMappingsForm.successfullySaved": "Role mappings have been saved.", + "roleMappingsForm.error": "Failed while saving!", "adminApps.title": "Apps", "adminApps.connections": "Connections", "adminApps.oauthClients": "OAuth clients", diff --git a/packages/web/src/pages/Authentication/RoleMappings.jsx b/packages/web/src/pages/Authentication/RoleMappings.jsx index 4440177b..8fd19c94 100644 --- a/packages/web/src/pages/Authentication/RoleMappings.jsx +++ b/packages/web/src/pages/Authentication/RoleMappings.jsx @@ -3,7 +3,7 @@ import LoadingButton from '@mui/lab/LoadingButton'; import Divider from '@mui/material/Divider'; import Stack from '@mui/material/Stack'; import Typography from '@mui/material/Typography'; -import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; +import Alert from '@mui/material/Alert'; import { useMemo } from 'react'; import { yupResolver } from '@hookform/resolvers/yup'; import * as yup from 'yup'; @@ -13,6 +13,7 @@ import useFormatMessage from 'hooks/useFormatMessage'; import useAdminSamlAuthProviderRoleMappings from 'hooks/useAdminSamlAuthProviderRoleMappings'; import useAdminUpdateSamlAuthProviderRoleMappings from 'hooks/useAdminUpdateSamlAuthProviderRoleMappings'; import RoleMappingsFieldArray from './RoleMappingsFieldsArray'; +import { getGeneralErrorMessage } from 'helpers/errors'; function generateFormRoleMappings(roleMappings) { if (roleMappings?.length === 0) { @@ -63,11 +64,11 @@ const getValidationSchema = (formatMessage) => function RoleMappings({ provider, providerLoading }) { const formatMessage = useFormatMessage(); - const enqueueSnackbar = useEnqueueSnackbar(); const { mutateAsync: updateRoleMappings, isPending: isUpdateRoleMappingsPending, + isSuccess: isUpdateRoleMappingsSuccess, } = useAdminUpdateSamlAuthProviderRoleMappings(provider?.id); const { data, isLoading: isAdminSamlAuthProviderRoleMappingsLoading } = @@ -75,8 +76,9 @@ function RoleMappings({ provider, providerLoading }) { adminSamlAuthProviderId: provider?.id, }); const roleMappings = data?.data; + const fieldNames = ['remoteRoleName', 'roleId']; - const handleRoleMappingsUpdate = async (values) => { + const handleRoleMappingsUpdate = async (values, e, setError) => { try { if (provider?.id) { await updateRoleMappings( @@ -85,29 +87,31 @@ function RoleMappings({ provider, providerLoading }) { remoteRoleName, })), ); - - enqueueSnackbar(formatMessage('roleMappingsForm.successfullySaved'), { - variant: 'success', - SnackbarProps: { - 'data-test': 'snackbar-update-role-mappings-success', - }, - }); } } catch (error) { - const errors = Object.values( - error.response.data.errors || [['Failed while saving!']], - ); - - for (const [error] of errors) { - enqueueSnackbar(error, { - variant: 'error', - SnackbarProps: { - 'data-test': 'snackbar-update-role-mappings-error', - }, + const errors = error?.response?.data?.errors; + if (errors) { + Object.entries(errors).forEach(([fieldName, fieldErrors]) => { + if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { + setError(`root.${fieldName}`, { + type: 'fieldRequestError', + message: `${fieldName}: ${fieldErrors.join(', ')}`, + }); + } }); } - throw new Error('Failed while saving!'); + const generalError = getGeneralErrorMessage({ + error, + fallbackMessage: formatMessage('roleMappingsForm.error'), + }); + + if (generalError) { + setError('root.general', { + type: 'requestError', + message: generalError, + }); + } } }; @@ -118,6 +122,17 @@ function RoleMappings({ provider, providerLoading }) { [roleMappings], ); + const renderErrors = (errors) => { + const rootErrors = errors?.root; + if (rootErrors) { + return Object.values(rootErrors).map((error, index) => ( + + {error.message} + + )); + } + }; + if ( providerLoading || !provider?.id || @@ -140,27 +155,35 @@ function RoleMappings({ provider, providerLoading }) { reValidateMode="onChange" noValidate automaticValidation={false} - > - - - - {formatMessage('roleMappingsForm.save')} - - - + render={({ formState: { errors, isDirty } }) => ( + + + {renderErrors(errors)} + {isUpdateRoleMappingsSuccess && !isDirty && ( + + {formatMessage('roleMappingsForm.successfullySaved')} + + )} + + {formatMessage('roleMappingsForm.save')} + + + )} + /> ); } RoleMappings.propTypes = { provider: PropTypes.shape({ - id: PropTypes.oneOf([PropTypes.number, PropTypes.string]).isRequired, + id: PropTypes.oneOfType([PropTypes.number, PropTypes.string]).isRequired, }), providerLoading: PropTypes.bool, }; diff --git a/packages/web/src/pages/Authentication/SamlConfiguration.jsx b/packages/web/src/pages/Authentication/SamlConfiguration.jsx index 47488241..d9d82e56 100644 --- a/packages/web/src/pages/Authentication/SamlConfiguration.jsx +++ b/packages/web/src/pages/Authentication/SamlConfiguration.jsx @@ -2,9 +2,11 @@ import PropTypes from 'prop-types'; import LoadingButton from '@mui/lab/LoadingButton'; import Stack from '@mui/material/Stack'; import MuiTextField from '@mui/material/TextField'; +import Alert from '@mui/material/Alert'; import * as React from 'react'; +import { yupResolver } from '@hookform/resolvers/yup'; +import * as yup from 'yup'; -import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; import ControlledAutocomplete from 'components/ControlledAutocomplete'; import Form from 'components/Form'; import Switch from 'components/Switch'; @@ -13,6 +15,7 @@ import useFormatMessage from 'hooks/useFormatMessage'; import useAdminCreateSamlAuthProvider from 'hooks/useAdminCreateSamlAuthProvider'; import useAdminUpdateSamlAuthProvider from 'hooks/useAdminUpdateSamlAuthProvider'; import useRoles from 'hooks/useRoles.ee'; +import { getGeneralErrorMessage } from 'helpers/errors'; const defaultValues = { active: false, @@ -28,45 +31,126 @@ const defaultValues = { defaultRoleId: '', }; +const getValidationSchema = (formatMessage) => { + const getMandatoryFieldMessage = (fieldTranslationId) => + formatMessage('authenticationForm.mandatoryInput', { + inputName: formatMessage(fieldTranslationId), + }); + + return yup.object().shape({ + active: yup.boolean(), + name: yup + .string() + .trim() + .required(getMandatoryFieldMessage('authenticationForm.name')), + certificate: yup + .string() + .trim() + .required(getMandatoryFieldMessage('authenticationForm.certificate')), + signatureAlgorithm: yup + .string() + .trim() + .required( + getMandatoryFieldMessage('authenticationForm.signatureAlgorithm'), + ), + issuer: yup + .string() + .trim() + .required(getMandatoryFieldMessage('authenticationForm.issuer')), + entryPoint: yup + .string() + .trim() + .required(getMandatoryFieldMessage('authenticationForm.entryPoint')), + firstnameAttributeName: yup + .string() + .trim() + .required( + getMandatoryFieldMessage('authenticationForm.firstnameAttributeName'), + ), + surnameAttributeName: yup + .string() + .trim() + .required( + getMandatoryFieldMessage('authenticationForm.surnameAttributeName'), + ), + emailAttributeName: yup + .string() + .trim() + .required( + getMandatoryFieldMessage('authenticationForm.emailAttributeName'), + ), + roleAttributeName: yup + .string() + .trim() + .required( + getMandatoryFieldMessage('authenticationForm.roleAttributeName'), + ), + defaultRoleId: yup + .string() + .trim() + .required(getMandatoryFieldMessage('authenticationForm.defaultRole')), + }); +}; + function generateRoleOptions(roles) { return roles?.map(({ name: label, id: value }) => ({ label, value })); } function SamlConfiguration({ provider, providerLoading }) { const formatMessage = useFormatMessage(); - const { data, loading: isRolesLoading } = useRoles(); + const { data, isLoading: isRolesLoading } = useRoles(); const roles = data?.data; - const enqueueSnackbar = useEnqueueSnackbar(); const { mutateAsync: createSamlAuthProvider, isPending: isCreateSamlAuthProviderPending, + isSuccess: isCreateSamlAuthProviderSuccess, } = useAdminCreateSamlAuthProvider(); const { mutateAsync: updateSamlAuthProvider, isPending: isUpdateSamlAuthProviderPending, + isSuccess: isUpdateSamlAuthProviderSuccess, } = useAdminUpdateSamlAuthProvider(provider?.id); const isPending = isCreateSamlAuthProviderPending || isUpdateSamlAuthProviderPending; - const handleSubmit = async (providerData) => { + const isSuccess = + isCreateSamlAuthProviderSuccess || isUpdateSamlAuthProviderSuccess; + + const handleSubmit = async (providerData, e, setError) => { try { if (provider?.id) { await updateSamlAuthProvider(providerData); } else { await createSamlAuthProvider(providerData); } + } 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(', '), + }); + } + }); + } - enqueueSnackbar(formatMessage('authenticationForm.successfullySaved'), { - variant: 'success', - SnackbarProps: { - 'data-test': 'snackbar-save-saml-provider-success', - }, + const generalError = getGeneralErrorMessage({ + error, + fallbackMessage: formatMessage('authenticationForm.error'), }); - } catch { - throw new Error('Failed while saving!'); + + if (generalError) { + setError('root.general', { + type: 'requestError', + message: generalError, + }); + } } }; @@ -75,103 +159,145 @@ function SamlConfiguration({ provider, providerLoading }) { } return ( -
- - - - - ( - + ( + + + + + ( + + )} + /> + + + + + + + ( + + )} + loading={isRolesLoading} + /> + {errors?.root?.general && ( + + {errors.root.general.message} + )} - /> - - - - - - - ( - + {isSuccess && !isDirty && ( + + {formatMessage('authenticationForm.successfullySaved')} + )} - loading={isRolesLoading} - /> - - {formatMessage('authenticationForm.save')} - - - + + {formatMessage('authenticationForm.save')} + +
+ )} + /> ); } From 19e2ff94f311f3a8c1e3b1a5bd41b7630ddebac7 Mon Sep 17 00:00:00 2001 From: "kasia.oczkowska" Date: Fri, 20 Dec 2024 09:50:34 +0000 Subject: [PATCH 2/2] refactor: use form's centralized error management --- packages/web/src/helpers/errors.js | 18 -------- packages/web/src/locales/en.json | 2 - .../src/pages/Authentication/RoleMappings.jsx | 44 +++++++++---------- .../Authentication/SamlConfiguration.jsx | 27 +----------- 4 files changed, 23 insertions(+), 68 deletions(-) delete mode 100644 packages/web/src/helpers/errors.js 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 b03c96e0..6a1bd9f6 100644 --- a/packages/web/src/locales/en.json +++ b/packages/web/src/locales/en.json @@ -283,7 +283,6 @@ "authenticationForm.successfullySaved": "The provider has been saved.", "authenticationForm.save": "Save", "authenticationForm.mandatoryInput": "{inputName} is required.", - "authenticationForm.error": "Failed while saving!", "roleMappingsForm.title": "Role mappings", "roleMappingsForm.remoteRoleName": "Remote role name", "roleMappingsForm.role": "Role", @@ -291,7 +290,6 @@ "roleMappingsForm.save": "Save", "roleMappingsForm.notFound": "No role mappings have been found.", "roleMappingsForm.successfullySaved": "Role mappings have been saved.", - "roleMappingsForm.error": "Failed while saving!", "adminApps.title": "Apps", "adminApps.connections": "Connections", "adminApps.oauthClients": "OAuth clients", diff --git a/packages/web/src/pages/Authentication/RoleMappings.jsx b/packages/web/src/pages/Authentication/RoleMappings.jsx index 8fd19c94..f306a0f7 100644 --- a/packages/web/src/pages/Authentication/RoleMappings.jsx +++ b/packages/web/src/pages/Authentication/RoleMappings.jsx @@ -4,7 +4,7 @@ import Divider from '@mui/material/Divider'; import Stack from '@mui/material/Stack'; import Typography from '@mui/material/Typography'; import Alert from '@mui/material/Alert'; -import { useMemo } from 'react'; +import { useMemo, useState } from 'react'; import { yupResolver } from '@hookform/resolvers/yup'; import * as yup from 'yup'; @@ -13,7 +13,6 @@ import useFormatMessage from 'hooks/useFormatMessage'; import useAdminSamlAuthProviderRoleMappings from 'hooks/useAdminSamlAuthProviderRoleMappings'; import useAdminUpdateSamlAuthProviderRoleMappings from 'hooks/useAdminUpdateSamlAuthProviderRoleMappings'; import RoleMappingsFieldArray from './RoleMappingsFieldsArray'; -import { getGeneralErrorMessage } from 'helpers/errors'; function generateFormRoleMappings(roleMappings) { if (roleMappings?.length === 0) { @@ -77,9 +76,11 @@ function RoleMappings({ provider, providerLoading }) { }); const roleMappings = data?.data; const fieldNames = ['remoteRoleName', 'roleId']; + const [fieldErrors, setFieldErrors] = useState(null); - const handleRoleMappingsUpdate = async (values, e, setError) => { + const handleRoleMappingsUpdate = async (values) => { try { + setFieldErrors(null); if (provider?.id) { await updateRoleMappings( values.roleMappings.map(({ roleId, remoteRoleName }) => ({ @@ -93,25 +94,14 @@ function RoleMappings({ provider, providerLoading }) { if (errors) { Object.entries(errors).forEach(([fieldName, fieldErrors]) => { if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { - setError(`root.${fieldName}`, { - type: 'fieldRequestError', - message: `${fieldName}: ${fieldErrors.join(', ')}`, - }); + setFieldErrors((prevErrors) => [ + ...(prevErrors || []), + `${fieldName}: ${fieldErrors.join(', ')}`, + ]); } }); } - - const generalError = getGeneralErrorMessage({ - error, - fallbackMessage: formatMessage('roleMappingsForm.error'), - }); - - if (generalError) { - setError('root.general', { - type: 'requestError', - message: generalError, - }); - } + throw errors || error; } }; @@ -123,14 +113,22 @@ function RoleMappings({ provider, providerLoading }) { ); const renderErrors = (errors) => { - const rootErrors = errors?.root; - if (rootErrors) { - return Object.values(rootErrors).map((error, index) => ( + const generalError = errors?.root?.general?.message; + if (fieldErrors) { + return fieldErrors.map((error, index) => ( - {error.message} + {error} )); } + + if (generalError) { + return ( + + {generalError} + + ); + } }; if ( diff --git a/packages/web/src/pages/Authentication/SamlConfiguration.jsx b/packages/web/src/pages/Authentication/SamlConfiguration.jsx index d9d82e56..73ff8c2d 100644 --- a/packages/web/src/pages/Authentication/SamlConfiguration.jsx +++ b/packages/web/src/pages/Authentication/SamlConfiguration.jsx @@ -15,7 +15,6 @@ import useFormatMessage from 'hooks/useFormatMessage'; import useAdminCreateSamlAuthProvider from 'hooks/useAdminCreateSamlAuthProvider'; import useAdminUpdateSamlAuthProvider from 'hooks/useAdminUpdateSamlAuthProvider'; import useRoles from 'hooks/useRoles.ee'; -import { getGeneralErrorMessage } from 'helpers/errors'; const defaultValues = { active: false, @@ -119,7 +118,7 @@ function SamlConfiguration({ provider, providerLoading }) { const isSuccess = isCreateSamlAuthProviderSuccess || isUpdateSamlAuthProviderSuccess; - const handleSubmit = async (providerData, e, setError) => { + const handleSubmit = async (providerData) => { try { if (provider?.id) { await updateSamlAuthProvider(providerData); @@ -128,29 +127,7 @@ function SamlConfiguration({ provider, providerLoading }) { } } 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('authenticationForm.error'), - }); - - if (generalError) { - setError('root.general', { - type: 'requestError', - message: generalError, - }); - } + throw errors || error; } };