Skip to content

Commit

Permalink
♻️ [#4398] Refactor validation error handling to use context
Browse files Browse the repository at this point in the history
Instead of prop-drilling, make sure to process validation errors at the
locations where the shape/structure is known and rely on the generic
mechanisms to pass/display the validation errors.

This also adds some JSDoc type hinting to the helpers because I keep
getting lost on what the expected shape of data/input is.
  • Loading branch information
sergei-maertens committed Dec 3, 2024
1 parent c77276c commit 3f25d66
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Field from 'components/admin/forms/Field';
import Fieldset from 'components/admin/forms/Fieldset';
import FormRow from 'components/admin/forms/FormRow';
import SubmitRow from 'components/admin/forms/SubmitRow';
import {ValidationErrorsProvider} from 'components/admin/forms/ValidationErrors';

import PluginField from './PluginField';
import PLUGIN_COMPONENT_MAPPING from './constants';
Expand All @@ -17,11 +18,12 @@ const PrefillConfigurationForm = ({
plugin = '',
attribute = '',
identifierRole = 'main',
// TODO: find a better way to specify this based on the selected plugin
// Plugins are responsible for setting up the default values, since we don't know the
// plugin-specific shape here.
options = {},
errors,
}) => {
return (
}) => (
<ValidationErrorsProvider errors={Object.entries(errors)}>
<Formik
initialValues={{
plugin,
Expand Down Expand Up @@ -61,7 +63,6 @@ const PrefillConfigurationForm = ({
defaultMessage="Plugin"
/>
}
errors={errors.plugin}
>
<>
<PluginField />
Expand All @@ -76,7 +77,6 @@ const PrefillConfigurationForm = ({
</Fieldset>

<PluginFormComponent
errors={errors}
{...(ToggleCopyComponent && {
showCopyButton: showCopyButton,
setShowCopyButton: setShowCopyButton,
Expand All @@ -95,8 +95,8 @@ const PrefillConfigurationForm = ({
);
}}
</Formik>
);
};
</ValidationErrorsProvider>
);

PrefillConfigurationForm.propTypes = {
onSubmit: PropTypes.func.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const getAttributes = async plugin => {
* Default (legacy) prefill configuration - after selecting the plugin, the user
* selects which attribute to use to grab the prefill value from.
*/
const DefaultFields = ({errors}) => {
const DefaultFields = () => {
const {
values: {plugin = ''},
} = useFormikContext();
Expand Down Expand Up @@ -60,7 +60,6 @@ const DefaultFields = ({errors}) => {
defaultMessage="Attribute"
/>
}
errors={errors.attribute}
>
<AttributeField prefillAttributes={prefillAttributes} />
</Field>
Expand All @@ -75,7 +74,6 @@ const DefaultFields = ({errors}) => {
defaultMessage="Identifier role"
/>
}
errors={errors.identifierRole}
>
<IdentifierRoleField />
</Field>
Expand All @@ -84,12 +82,6 @@ const DefaultFields = ({errors}) => {
);
};

DefaultFields.propTypes = {
errors: PropTypes.shape({
plugin: ErrorsType,
attribute: ErrorsType,
identifierRole: ErrorsType,
}).isRequired,
};
DefaultFields.propTypes = {};

export default DefaultFields;
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import useAsync from 'react-use/esm/useAsync';

import {FormContext} from 'components/admin/form_design/Context';
import useConfirm from 'components/admin/form_design/useConfirm';
import {normalizeErrors} from 'components/admin/forms/Field';
import Fieldset from 'components/admin/forms/Fieldset';
import FormRow from 'components/admin/forms/FormRow';
import {LOADING_OPTION} from 'components/admin/forms/Select';
import {ValidationErrorContext} from 'components/admin/forms/ValidationErrors';
import VariableMapping from 'components/admin/forms/VariableMapping';
import {
AuthAttributePath,
Expand All @@ -26,7 +26,7 @@ import {FAIcon} from 'components/admin/icons';
import ErrorBoundary from 'components/errors/ErrorBoundary';
import {get} from 'utils/fetch';

import {ErrorsType} from '../types';
import ValidationErrorsProvider from '../../../../forms/ValidationErrors';
import CopyConfigurationFromRegistrationBackend from './CopyConfigurationFromRegistrationBackend';

const PLUGIN_ID = 'objects_api';
Expand All @@ -40,7 +40,7 @@ const onApiGroupChange = prevValues => ({
...prevValues.options,
objecttypeUuid: '',
objecttypeVersion: undefined,
authAttributePath: [],
authAttributePath: undefined,
variablesMapping: [],
},
});
Expand All @@ -56,40 +56,34 @@ const getProperties = async (objectsApiGroup, objecttypeUuid, objecttypeVersion)
return response.data.map(property => [property.targetPath, property.targetPath.join(' > ')]);
};

const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
const ObjectsAPIFields = ({showCopyButton, setShowCopyButton}) => {
const intl = useIntl();
// Object with keys the plugin/attribute/options, we process these further to set up
// the required context for the fields.
const errors = Object.fromEntries(useContext(ValidationErrorContext));
const optionsErrors = Object.entries(errors.options ?? {}).map(([key, errs]) => [
`options.${key}`,
errs,
]);

const {values, setFieldValue, setValues} = useFormikContext();
const {
values,
values: {
plugin,
options: {
objecttypeUuid,
objecttypeVersion,
objectsApiGroup,
authAttributePath,
variablesMapping,
},
},
setFieldValue,
setValues,
} = useFormikContext();
plugin,
options: {objecttypeUuid, objecttypeVersion, objectsApiGroup},
} = values;

const defaults = {
objectsApiGroup: null,
objecttypeUuid: '',
objecttypeVersion: null,
authAttributePath: [],
authAttributePath: undefined,
variablesMapping: [],
};

// Merge defaults into options if not already set
useEffect(() => {
if (!values.options) {
setFieldValue('options', defaults);
} else {
setFieldValue('options', {...defaults, ...values.options});
}
const options = values.options ?? {};
setFieldValue('options', {...defaults, ...options});
}, []);

const {
Expand Down Expand Up @@ -129,13 +123,8 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
if (error) throw error;
const prefillProperties = loading ? LOADING_OPTION : value;

const [, objectsApiGroupErrors] = normalizeErrors(errors.options?.objectsApiGroup, intl);
const [, objecttypeUuidErrors] = normalizeErrors(errors.options?.objecttypeUuid, intl);
const [, objecttypeVersionErrors] = normalizeErrors(errors.options?.objecttypeVersion, intl);
const [, authAttributePathErrors] = normalizeErrors(errors.options?.authAttributePath, intl);

return (
<>
<ValidationErrorsProvider errors={optionsErrors}>
{showCopyButton ? (
<CopyConfigurationFromRegistrationBackend
backends={backends}
Expand All @@ -145,7 +134,6 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
<Fieldset>
<ObjectsAPIGroup
apiGroupChoices={apiGroups}
errors={objectsApiGroupErrors}
onChangeCheck={async () => {
if (!objecttypeUuid) return true;
const confirmSwitch = await openApiGroupConfirmationModal();
Expand Down Expand Up @@ -180,7 +168,6 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
name="options.objecttypeUuid"
apiGroupFieldName="options.objectsApiGroup"
versionFieldName="options.objecttypeVersion"
errors={objecttypeUuidErrors}
label={
<FormattedMessage
description="Objects API prefill options 'Objecttype' label"
Expand Down Expand Up @@ -230,7 +217,6 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
defaultMessage="Version"
/>
}
errors={objecttypeVersionErrors}
apiGroupFieldName="options.objectsApiGroup"
objectTypeFieldName="options.objecttypeUuid"
/>
Expand All @@ -241,7 +227,6 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
objecttypeUuid={objecttypeUuid}
objecttypeVersion={objecttypeVersion}
style={{maxWidth: '10em'}}
errors={authAttributePathErrors}
/>
</Fieldset>

Expand Down Expand Up @@ -294,14 +279,13 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
/>
}
/>
</>
</ValidationErrorsProvider>
);
};

ObjectsAPIFields.propTypes = {
errors: PropTypes.shape({
plugin: ErrorsType,
}).isRequired,
showCopyButton: PropTypes.bool.isRequired,
setShowCopyButton: PropTypes.func.isRequired,
};

export default ObjectsAPIFields;
23 changes: 23 additions & 0 deletions src/openforms/js/components/admin/forms/Field.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@ import {useIntl} from 'react-intl';
import {PrefixContext} from './Context';
import ErrorList from './ErrorList';

/**
* @typedef {Object} IntlErrorMessage
* @property {string} defaultMessage
* @property {string} description
*/

/**
* @typedef {[string, string]} NamedErrorMessage
* @property {string} 0 - The form field name with the error.
* @property {string} 1 - The error message itself.
*/

/**
* @typedef {string | IntlErrorMessage | NamedErrorMessage} ErrorMessage
*/

/**
*
* @param {ErrorMessage | ErrorMessage[]} errors A single error instance or array of errors.
* @param {IntlShape} intl The intl object from react-intl (useIntl() hook return value).
* @return {[boolean, string[]]} A tuple indicating if there are errors and the list of error messages.
*/
export const normalizeErrors = (errors = [], intl) => {
if (!Array.isArray(errors)) {
errors = [errors];
Expand Down Expand Up @@ -55,6 +77,7 @@ const Field = ({
modifiedChildren = React.cloneElement(children, childProps);
}
const [hasErrors, formattedErrors] = normalizeErrors(errors, intl);
// console.log(name, hasErrors, formattedErrors);
const className = classNames({
fieldBox: fieldBox,
errors: hasErrors,
Expand Down
6 changes: 6 additions & 0 deletions src/openforms/js/components/admin/forms/ValidationErrors.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ ValidationErrorsProvider.propTypes = {
errors: PropTypes.arrayOf(PropTypes.arrayOf(errorArray)),
};

/**
* Only return the errors that have the $name prefix.
* @param {string} name Field name prefix
* @param {Array<[string, T>} errors List of all validation errors.
* @return {Array<[string, T>} List of errors that match the name prefix.
*/
const filterErrors = (name, errors) => {
return errors
.filter(([key]) => key.startsWith(`${name}.`))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const ObjectTypeSelect = ({
label,
helpText,
versionFieldName = 'objecttypeVersion',
errors = [],
}) => {
const [fieldProps, , fieldHelpers] = useField(name);
const {
Expand Down Expand Up @@ -69,14 +68,7 @@ const ObjectTypeSelect = ({

return (
<FormRow>
<Field
name={name}
required
label={label}
helpText={helpText}
errors={errors}
noManageChildProps
>
<Field name={name} required label={label} helpText={helpText} noManageChildProps>
<ReactSelect
name={name}
options={options}
Expand Down Expand Up @@ -121,10 +113,6 @@ ObjectTypeSelect.propTypes = {
* changes, the version will be reset/unset.
*/
versionFieldName: PropTypes.string,
/**
* List of errors to be displayed on the field
*/
errors: PropTypes.arrayOf(PropTypes.string),
};

export default ObjectTypeSelect;
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const ObjectTypeVersionSelect = ({
apiGroupFieldName = 'objectsApiGroup',
objectTypeFieldName = 'objecttype',
label,
errors = [],
}) => {
const {getFieldProps} = useFormikContext();

Expand All @@ -56,7 +55,7 @@ const ObjectTypeVersionSelect = ({
const options = choices.map(([value, label]) => ({value, label}));
return (
<FormRow>
<Field name={name} required label={label} errors={errors} noManageChildProps>
<Field name={name} required label={label} noManageChildProps>
<ReactSelect
name={name}
required
Expand Down Expand Up @@ -88,10 +87,6 @@ ObjectTypeVersionSelect.propTypes = {
* The label that will be shown before the field
*/
label: PropTypes.node.isRequired,
/**
* List of errors to be displayed on the field
*/
errors: PropTypes.arrayOf(PropTypes.string),
};

export default ObjectTypeVersionSelect;
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const ObjectsAPIGroup = ({
onApiGroupChange,
isClearable = false,
required = true,
errors = [],
}) => {
const [{onChange: onChangeFormik, ...fieldProps}, , {setValue}] = useField(name);
const {setValues} = useFormikContext();
Expand All @@ -41,7 +40,6 @@ const ObjectsAPIGroup = ({
<Field
name={name}
required={required}
errors={errors}
label={
<FormattedMessage
description="Objects API group field label"
Expand Down Expand Up @@ -121,11 +119,6 @@ ObjectsAPIGroup.propTypes = {
* Indicate if the field is required or optional.
*/
required: PropTypes.bool,

/**
* List of errors to be displayed on the field
*/
errors: PropTypes.arrayOf(PropTypes.string),
};

export default ObjectsAPIGroup;

0 comments on commit 3f25d66

Please sign in to comment.