Skip to content

Commit

Permalink
🎨 [#4608] Finetune ObjectsAPIGroup refactor
Browse files Browse the repository at this point in the history
After moving the ObjectsAPIGroup component to a shared field layer
where the form data structure is unknown at the component level, the
code to reset dependent fields was no longer properly 'generic' and the
approach with a prefix was not the most elegant:

* it assumes the other (dependent) field names are the same in
  registration and prefill (they are, but deliberately, though there's
  no guarantee they will be in the future)
* it assumes a certain structure, but dependent fields may live in
  another namespace or the reset behaviour could require additional
  changes in the form state to be made

Instead, the reset behaviour can now be passed in as a prop, so that
the caller/user of the ObjectsAPIGroup component is in control of
which state exactly gets reset.

This already reflects nicely since the variableMapping reset in the
Legacy Objects API registration configuration didn't make any sense,
it's a V2 option, so now we have tailored reset functions to the
expected data structure.
  • Loading branch information
sergei-maertens committed Oct 7, 2024
1 parent 60e52cb commit 2fec7dc
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,19 @@ import {
UploadSubmissionCsv,
} from './fields';

/**
* Callback to invoke when the API group changes - used to reset the dependent fields.
*/
const onApiGroupChange = prevValues => ({
...prevValues,
objecttype: '',
objecttypeVersion: undefined,
});

const LegacyConfigFields = ({apiGroupChoices}) => (
<>
<Fieldset>
<ObjectsAPIGroup apiGroupChoices={apiGroupChoices} />
<ObjectsAPIGroup apiGroupChoices={apiGroupChoices} onApiGroupChange={onApiGroupChange} />
<ErrorBoundary
errorMessage={
<FormattedMessage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ import {
UploadSubmissionCsv,
} from './fields';

/**
* Callback to invoke when the API group changes - used to reset the dependent fields.
*/
const onApiGroupChange = prevValues => ({
...prevValues,
objecttype: '',
objecttypeVersion: undefined,
variablesMapping: [],
});

const V2ConfigFields = ({apiGroupChoices}) => {
const intl = useIntl();
const {
Expand All @@ -44,6 +54,7 @@ const V2ConfigFields = ({apiGroupChoices}) => {
setFieldValue('variablesMapping', []);
return true;
}}
onApiGroupChange={onApiGroupChange}
/>
<ErrorBoundary
errorMessage={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ const EditableVariableRow = ({index, variable, onDelete, onChange, onFieldChange
attribute={variable.prefillAttribute}
identifierRole={variable.prefillIdentifierRole}
errors={variable.errors}
prefillOptions={variable.prefillOptions}
options={variable.prefillOptions}
onChange={({plugin, attribute, identifierRole}) =>
onChange(variable.key, '', {
prefillPlugin: plugin,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {Formik, useField, useFormikContext} from 'formik';
import PropTypes from 'prop-types';
import React, {useContext, useEffect, useState} from 'react';
import React, {useContext} from 'react';
import {FormattedMessage, useIntl} from 'react-intl';
import useAsync from 'react-use/esm/useAsync';
import useUpdateEffect from 'react-use/esm/useUpdateEffect';
Expand Down Expand Up @@ -30,7 +30,7 @@ const PrefillConfigurationForm = ({
attribute = '',
identifierRole = 'main',
// TODO: find a better way to specify this based on the selected plugin
prefillOptions = {
options = {
objectsApiGroup: '',
objecttype: '',
objecttypeVersion: null,
Expand All @@ -44,7 +44,7 @@ const PrefillConfigurationForm = ({
plugin,
attribute,
identifierRole,
prefillOptions,
options,
}}
onSubmit={(values, actions) => {
// TODO should be implemented in https://github.com/open-formulieren/open-forms/issues/4693
Expand Down Expand Up @@ -205,6 +205,19 @@ const PrefillFields = ({values, errors}) => {
);
};

/**
* Callback to invoke when the API group changes - used to reset the dependent fields.
*/
const onApiGroupChange = prevValues => ({
...prevValues,
options: {
...prevValues.options,
objecttype: '',
objecttypeVersion: undefined,
variablesMapping: [],
},
});

const ObjectsAPIPrefillFields = ({values, errors}) => {
const intl = useIntl();
const plugin = values.plugin;
Expand All @@ -217,7 +230,7 @@ const ObjectsAPIPrefillFields = ({values, errors}) => {

const apiGroups = objectsPlugin.configurationContext.apiGroups;

const {objecttype, objecttypeVersion, objectsApiGroup} = values.prefillOptions;
const {objecttype, objecttypeVersion, objectsApiGroup} = values.options;

// Load the possible prefill properties
// XXX: this would benefit from client-side caching
Expand Down Expand Up @@ -259,12 +272,11 @@ const ObjectsAPIPrefillFields = ({values, errors}) => {
</FormRow>

<ObjectsAPIGroup
prefix="prefillOptions"
apiGroupName="prefillOptions.objectsApiGroup"
errors={errors['prefillOptions.objectsApiGroup']}
name="options.objectsApiGroup"
prefix="options"
apiGroupChoices={apiGroups}
onChangeCheck={() => {
if (values.prefillOptions.variablesMapping.length === 0) return true;
if (values.options.variablesMapping.length === 0) return true;
const confirmSwitch = window.confirm(
intl.formatMessage({
description:
Expand All @@ -274,10 +286,12 @@ const ObjectsAPIPrefillFields = ({values, errors}) => {
})
);
if (!confirmSwitch) return false;
setFieldValue('prefillOptions.variablesMapping', []);
setFieldValue('options.variablesMapping', []);
return true;
}}
onApiGroupChange={onApiGroupChange}
/>

<ErrorBoundary
errorMessage={
<FormattedMessage
Expand All @@ -287,11 +301,11 @@ const ObjectsAPIPrefillFields = ({values, errors}) => {
}
>
<ObjectTypeSelect
objectTypeName="prefillOptions.objecttype"
objectTypeVersionName="prefillOptions.objecttypeVersion"
apiGroupName="prefillOptions.objectsApiGroup"
objectTypeName="options.objecttype"
objectTypeVersionName="options.objecttypeVersion"
apiGroupName="options.objectsApiGroup"
onChangeCheck={() => {
if (values.prefillOptions.variablesMapping.length === 0) return true;
if (values.options.variablesMapping.length === 0) return true;
const confirmSwitch = window.confirm(
intl.formatMessage({
description:
Expand All @@ -301,7 +315,7 @@ const ObjectsAPIPrefillFields = ({values, errors}) => {
})
);
if (!confirmSwitch) return false;
setFieldValue('prefillOptions.variablesMapping', []);
setFieldValue('options.variablesMapping', []);
return true;
}}
/>
Expand All @@ -316,9 +330,9 @@ const ObjectsAPIPrefillFields = ({values, errors}) => {
}
>
<ObjectTypeVersionSelect
objectTypeName="prefillOptions.objecttype"
objectTypeVersionName="prefillOptions.objecttypeVersion"
apiGroupName="prefillOptions.objectsApiGroup"
objectTypeName="options.objecttype"
objectTypeVersionName="options.objecttypeVersion"
apiGroupName="options.objectsApiGroup"
/>
</ErrorBoundary>
</Fieldset>
Expand All @@ -333,7 +347,7 @@ const ObjectsAPIPrefillFields = ({values, errors}) => {
>
<FormRow>
<VariableMapping
name="prefillOptions.variablesMapping"
name="options.variablesMapping"
loading={loading}
directionIcon={<FAIcon icon="arrow-left-long" aria-hidden="true" />}
propertyName="prefillProperty"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const PrefillSummary = ({
plugin = '',
attribute = '',
identifierRole = 'main',
prefillOptions = undefined,
options = undefined,
onChange = undefined,
errors = {},
}) => {
Expand Down Expand Up @@ -91,7 +91,7 @@ const PrefillSummary = ({
plugin={plugin}
attribute={attribute}
identifierRole={identifierRole}
prefillOptions={prefillOptions}
options={options}
onSubmit={values => {
onChange(values);
setModalOpen(false);
Expand Down Expand Up @@ -119,6 +119,7 @@ PrefillSummary.propTypes = {
plugin: PropTypes.string,
attribute: PropTypes.string,
identifierRole: PropTypes.string,
options: PropTypes.objects,
onChange: PropTypes.func, // if defined, we can edit it in a modal
errors: PropTypes.objectOf(AnyError),
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {useField, useFormikContext} from 'formik';
import _ from 'lodash';
import PropTypes from 'prop-types';
import {FormattedMessage} from 'react-intl';
import {useUpdateEffect} from 'react-use';
Expand All @@ -11,32 +10,24 @@ import ReactSelect from 'components/admin/forms/ReactSelect';
const ObjectsAPIGroup = ({
apiGroupChoices,
onChangeCheck,
apiGroupName = 'objectsApiGroup',
prefix = undefined,
name = 'objectsApiGroup',
onApiGroupChange,
}) => {
const [{onChange: onChangeFormik, ...fieldProps}, , {setValue}] = useField(apiGroupName);
const [{onChange: onChangeFormik, ...fieldProps}, , {setValue}] = useField(name);
const {setValues} = useFormikContext();
const {value} = fieldProps;

// reset the objecttype specific-configuration whenever the API group changes
// Call `onApiGroupChange` to get the 'reset' values whenever the API group changes.
useUpdateEffect(() => {
setValues(prevValues => {
const targetObject = prefix ? prevValues[prefix] : prevValues;
const newObject = {
...targetObject,
objecttype: '',
objecttypeVersion: undefined,
variablesMapping: [],
};
return prefix ? {...prevValues, [prefix]: newObject} : newObject;
});
}, [setValues, value]);
if (!onApiGroupChange) return;
setValues(onApiGroupChange);
}, [setValues, onApiGroupChange, value]);

const options = apiGroupChoices.map(([value, label]) => ({value, label}));
return (
<FormRow>
<Field
name={apiGroupName}
name={name}
required
label={
<FormattedMessage
Expand All @@ -53,7 +44,7 @@ const ObjectsAPIGroup = ({
noManageChildProps
>
<ReactSelect
name={apiGroupName}
name={name}
options={options}
required
onChange={selectedOption => {
Expand All @@ -75,7 +66,31 @@ ObjectsAPIGroup.propTypes = {
])
)
).isRequired,

/**
* Optional callback to confirm the change. Return `true` to continue with the change,
* return `false` to abort it.
*/
onChangeCheck: PropTypes.func,

/**
* Name to use for the form field, is passed down to Formik.
*/
name: PropTypes.string,

/**
* Callback to invoke when the API group value changes, e.g. to reset any dependent fields.
*
* The function will be called with Formik's previous values so you can construct a new
* values state from that.
*
* **NOTE**
*
* It's best to define this callback at the module level, or make use of `useCallback`
* to obtain a stable reference to the callback, otherwise the callback will likely
* fire unexpectedly during re-renders.
*/
onApiGroupChange: PropTypes.func,
};

export default ObjectsAPIGroup;

0 comments on commit 2fec7dc

Please sign in to comment.