Skip to content

Commit

Permalink
♻️ [#4398] Refactor the TargetPathSelect component
Browse files Browse the repository at this point in the history
The TargetPathSelect component is now decoupled from its possible
FieldArray parent, and the option label display is incorporated in
the component itself so that options no longer need to be pre-processed.

On top of that, it's now refactored to be based on react-select for
consistency in the UI, and separate stories have been added so we can
do (visual) regression testing/isolated development.

We use this component in a number of places now:

* the path to the auth attribute (no variablesMapping parent context),
  used in registration and prefill plugins for the Objects API
* the path for the generic Object Types V2 registration mapping, here
  a variablesMapping parent container/context is relevant and different
  form state management semantics apply
* the path to specialized AddressNL subfield mappings - a parent is
  relevant, but not within a bigger variable mapping so we can use the
  standard field assignment semantics of Formik.
  • Loading branch information
sergei-maertens committed Dec 2, 2024
1 parent addb139 commit 3ceac3e
Show file tree
Hide file tree
Showing 16 changed files with 303 additions and 128 deletions.
6 changes: 6 additions & 0 deletions src/openforms/js/compiled-lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -6427,6 +6427,12 @@
"value": "Variable"
}
],
"xBb5YI": [
{
"type": 0,
"value": "Select an object type and version before you can pick a source path."
}
],
"xI6md8": [
{
"type": 0,
Expand Down
6 changes: 6 additions & 0 deletions src/openforms/js/compiled-lang/nl.json
Original file line number Diff line number Diff line change
Expand Up @@ -6449,6 +6449,12 @@
"value": "Variabele"
}
],
"xBb5YI": [
{
"type": 0,
"value": "Select an object type and version before you can pick a source path."
}
],
"xI6md8": [
{
"type": 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
mockCataloguesGet as mockObjectsApiCataloguesGet,
mockObjecttypeVersionsGet,
mockObjecttypesGet,
mockTargetPathsPost,
} from 'components/admin/form_design/registrations/objectsapi/mocks';
import {
mockCaseTypesGet,
Expand Down Expand Up @@ -512,6 +513,15 @@ export default {
]),
mockObjectsApiCataloguesGet(),
mockDocumentTypesGet(),
mockTargetPathsPost({
string: [
{
targetPath: ['path', 'to.the', 'target'],
isRequired: true,
jsonSchema: {type: 'string'},
},
],
}),
],
zgwMocks: [
mockZGWApisCataloguesGet(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {useFormikContext} from 'formik';
import {FieldArray, useFormikContext} from 'formik';
import isEqual from 'lodash/isEqual';
import PropTypes from 'prop-types';
import React, {useContext} from 'react';
import {FormattedMessage} from 'react-intl';
import {useAsync, useToggle} from 'react-use';
Expand All @@ -9,14 +10,74 @@ import {REGISTRATION_OBJECTS_TARGET_PATHS} from 'components/admin/form_design/co
import Field from 'components/admin/forms/Field';
import FormRow from 'components/admin/forms/FormRow';
import {Checkbox} from 'components/admin/forms/Inputs';
import Select, {LOADING_OPTION} from 'components/admin/forms/Select';
import {TargetPathSelect} from 'components/admin/forms/objects_api';
import {TargetPathDisplay} from 'components/admin/forms/objects_api';
import ErrorMessage from 'components/errors/ErrorMessage';
import {post} from 'utils/fetch';

import {asJsonSchema} from './utils';

/**
* Hack-ish way to manage the variablesMapping state for one particular entry.
*
* We ensure that an item is added to `variablesMapping` by using the `FieldArray`
* helper component if it doesn't exist yet, otherwise we update it.
*/
const MappedVariableTargetPathSelect = ({
name,
index,
mappedVariable,
isLoading = false,
targetPaths = [],
isDisabled = false,
}) => {
const {
values: {variablesMapping = []},
setFieldValue,
} = useFormikContext();
const isNew = variablesMapping.length === index;
return (
<FieldArray
name="variablesMapping"
render={arrayHelpers => (
<TargetPathSelect
name={name}
isLoading={isLoading}
targetPaths={targetPaths}
isDisabled={isDisabled}
onChange={newValue => {
// Clearing the select means we need to remove the record from the mapping,
// otherwise it's not a valid item for the backend.
if (newValue === null) {
arrayHelpers.remove(index);
return;
}

// otherwise, either add a new item, or update the existing
if (isNew) {
const newMapping = {...mappedVariable, targetPath: newValue.targetPath};
arrayHelpers.push(newMapping);
} else {
setFieldValue(name, newValue.targetPath);
}
}}
/>
)}
/>
);
};

MappedVariableTargetPathSelect.propTypes = {
name: PropTypes.string.isRequired,
index: PropTypes.number.isRequired,
mappedVariable: PropTypes.shape({
variableKey: PropTypes.string.isRequired,
targetPath: PropTypes.arrayOf(PropTypes.string),
options: PropTypes.object,
}).isRequired,
isLoading: PropTypes.bool,
isDisabled: PropTypes.bool,
};

export const GenericEditor = ({
variable,
components,
Expand Down Expand Up @@ -56,11 +117,6 @@ export const GenericEditor = ({

const getTargetPath = pathSegment => targetPaths.find(t => isEqual(t.targetPath, pathSegment));

const choices =
loading || error
? LOADING_OPTION
: targetPaths.map(t => [JSON.stringify(t.targetPath), <TargetPathDisplay target={t} />]);

if (error)
return (
<ErrorMessage>
Expand Down Expand Up @@ -108,12 +164,13 @@ export const GenericEditor = ({
}
disabled={isGeometry}
>
<TargetPathSelect
<MappedVariableTargetPathSelect
name={`${namePrefix}.targetPath`}
index={index}
choices={choices}
mappedVariable={mappedVariable}
disabled={isGeometry}
isDisabled={isGeometry}
isLoading={loading}
targetPaths={targetPaths}
/>
</Field>
</FormRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,17 @@ const onApiGroupChange = prevValues => ({
...prevValues,
objecttype: '',
objecttypeVersion: undefined,
authAttributePath: undefined,
});

const LegacyConfigFields = ({apiGroupChoices}) => {
const [updateExistingObject] = useField('updateExistingObject');
const authAttributePathDisabled = !updateExistingObject.value;

const {
values: {objecttype, objecttypeVersion, objectsApiGroup},
values: {
objectsApiGroup = null,
objecttype = '',
objecttypeVersion = null,
updateExistingObject = false,
},
} = useFormikContext();

return (
Expand Down Expand Up @@ -117,11 +120,11 @@ const LegacyConfigFields = ({apiGroupChoices}) => {
>
<UpdateExistingObject />
<AuthAttributePath
name={'authAttributePath'}
name="authAttributePath"
objectsApiGroup={objectsApiGroup}
objecttypeUuid={objecttype}
objecttypeVersion={objecttypeVersion}
disabled={authAttributePathDisabled}
disabled={!updateExistingObject}
/>
</Fieldset>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
mockObjecttypeVersionsGet,
mockObjecttypesError,
mockObjecttypesGet,
mockTargetPathsPost,
} from './mocks';

const NAME = 'form.registrationBackends.0.options';
Expand Down Expand Up @@ -62,6 +63,15 @@ export default {
]),
mockCataloguesGet(),
mockDocumentTypesGet(),
mockTargetPathsPost({
string: [
{
targetPath: ['path', 'to.the', 'target'],
isRequired: true,
jsonSchema: {type: 'string'},
},
],
}),
],
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import Field from 'components/admin/forms/Field';
import Fieldset from 'components/admin/forms/Fieldset';
import FormRow from 'components/admin/forms/FormRow';
import {TextInput} from 'components/admin/forms/Inputs';
import {TargetPathDisplay} from 'components/admin/forms/objects_api';

import {AddressNlEditor} from './AddressNlObjectsApiVariableConfigurationEditor';
import {GenericEditor} from './GenericObjectsApiVariableConfigurationEditor';
Expand Down Expand Up @@ -37,7 +36,7 @@ const VARIABLE_CONFIGURATION_OPTIONS = {
* @returns {JSX.Element} - The configuration form for the Objects API
*/
const ObjectsApiVariableConfigurationEditor = ({variable}) => {
const {values: backendOptions, getFieldProps, setFieldValue} = useFormikContext();
const {values: backendOptions, getFieldProps} = useFormikContext();
const {components} = useContext(FormContext);

/** @type {ObjectsAPIRegistrationBackendOptions} */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {useField, useFormikContext} from 'formik';
import {useFormikContext} from 'formik';
import PropTypes from 'prop-types';
import {FormattedMessage} from 'react-intl';

Expand Down Expand Up @@ -27,15 +27,19 @@ const onApiGroupChange = prevValues => ({
...prevValues,
objecttype: '',
objecttypeVersion: undefined,
authAttributePath: undefined,
variablesMapping: [],
});

const V2ConfigFields = ({apiGroupChoices}) => {
const [updateExistingObject] = useField('updateExistingObject');
const authAttributePathDisabled = !updateExistingObject.value;

const {
values: {objecttype, objecttypeVersion, objectsApiGroup, variablesMapping = []},
values: {
objectsApiGroup = null,
objecttype = '',
objecttypeVersion = null,
variablesMapping = [],
updateExistingObject = false,
},
setFieldValue,
} = useFormikContext();

Expand Down Expand Up @@ -143,7 +147,7 @@ const V2ConfigFields = ({apiGroupChoices}) => {
objectsApiGroup={objectsApiGroup}
objecttypeUuid={objecttype}
objecttypeVersion={objecttypeVersion}
disabled={authAttributePathDisabled}
disabled={!updateExistingObject}
/>
</Fieldset>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {expect, fn, screen, userEvent, waitFor, within} from '@storybook/test';
import selectEvent from 'react-select-event';

import {
mockObjectsAPIPrefillPropertiesGet,
Expand All @@ -7,9 +8,10 @@ import {
import {BACKEND_OPTIONS_FORMS} from 'components/admin/form_design/registrations';
import {mockTargetPathsPost} from 'components/admin/form_design/registrations/objectsapi/mocks';

import {findReactSelectMenu} from '../../../../utils/storybookTestHelpers';
import {serializeValue} from '../../forms/VariableMapping';
import {mockObjecttypeVersionsGet, mockObjecttypesGet} from '../registrations/objectsapi/mocks';
import {FormDecorator, withReactSelectDecorator} from '../story-decorators';
import {FormDecorator} from '../story-decorators';
import VariablesEditor from './VariablesEditor';

BACKEND_OPTIONS_FORMS.testPlugin = {
Expand Down Expand Up @@ -385,9 +387,7 @@ export const FilesMappingAndObjectAPIRegistration = {
pluginVerboseName: 'Objects API registration',
},
],
onFieldChange: data => {
console.log(data);
},
onFieldChange: fn(),
},
parameters: {
msw: {
Expand Down Expand Up @@ -426,38 +426,47 @@ export const FilesMappingAndObjectAPIRegistration = {
],
},
},
play: async ({canvasElement}) => {
play: async ({canvasElement, step}) => {
const canvas = within(canvasElement);

const editIcons = canvas.getAllByTitle('Registratie-instellingen bewerken');
expect(editIcons).toHaveLength(3);

await step('Single file component', async () => {
// The second icon is for the single file upload component variable
await userEvent.click(editIcons[1]);

const targetSchemaDropdown = await canvas.findByRole('combobox', {name: 'Bestemmingspad'});
await expect(targetSchemaDropdown).toBeVisible();
selectEvent.openMenu(targetSchemaDropdown);

// Only the targets of type string should appear
const targetSelectMenu = within(await findReactSelectMenu(canvas));
expect(
await targetSelectMenu.findByRole('option', {name: 'path > to.the > target (verplicht)'})
).toBeVisible();
await expect(
await targetSelectMenu.findByRole('option', {name: 'path > to > uri (verplicht)'})
).toBeVisible();

const saveButton = canvas.getByRole('button', {name: 'Opslaan'});
userEvent.click(saveButton);
});

// The second icon is for the single file upload component variable
userEvent.click(editIcons[1]);

const targetSchemaDropdown = await screen.findByRole('combobox');

await expect(targetSchemaDropdown).toBeInTheDocument();

// Only the targets of type string should appear
await expect(
await screen.findByRole('option', {name: 'path > to.the > target (verplicht)'})
).toBeVisible();
await expect(
await screen.findByRole('option', {name: 'path > to > uri (verplicht)'})
).toBeVisible();

const saveButton = screen.getByRole('button', {name: 'Opslaan'});
userEvent.click(saveButton);

// The third icon is for the multiple file upload component variable
userEvent.click(editIcons[2]);
await step('Multi file component', async () => {
// The third icon is for the multiple file upload component variable
await userEvent.click(editIcons[2]);

const dropdown = await screen.findByRole('combobox');
const targetSchemaDropdown = await canvas.findByRole('combobox', {name: 'Bestemmingspad'});
await expect(targetSchemaDropdown).toBeVisible();
selectEvent.openMenu(targetSchemaDropdown);

await expect(dropdown).toBeInTheDocument();
await expect(
await screen.findByRole('option', {name: 'path > to > array (verplicht)'})
).toBeVisible();
// Only the targets of type array should appear
const targetSelectMenu = within(await findReactSelectMenu(canvas));
expect(
await targetSelectMenu.findByRole('option', {name: 'path > to > array (verplicht)'})
).toBeVisible();
});
},
};

Expand Down
Loading

0 comments on commit 3ceac3e

Please sign in to comment.