Skip to content

Commit

Permalink
✨ [open-formulieren/open-forms#4510] Display backend validation errors
Browse files Browse the repository at this point in the history
I've opted to collect all the errors at the top and group them by step,
rather than trying to weave this in the summary table. User research
from Utrecht showed that this was the best way to show validation
errors.

Ideally, there would be some aria-describedby options, but that requires
a lot more work to link everything together.
  • Loading branch information
sergei-maertens committed Feb 4, 2025
1 parent 775bb98 commit 1355641
Show file tree
Hide file tree
Showing 10 changed files with 434 additions and 6 deletions.
18 changes: 17 additions & 1 deletion src/api-mocks/submissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export const mockSubmissionSummaryGet = () =>
data: [
{
name: SUBMISSION_STEP_DETAILS.formStep.configuration.components[0].label,
value: 'Compnent 1 value',
value: 'Component 1 value',
component: SUBMISSION_STEP_DETAILS.formStep.configuration.components[0],
},
],
Expand All @@ -119,6 +119,22 @@ export const mockSubmissionCompletePost = () =>
})
);

export const mockSubmissionCompleteInvalidPost = invalidParams =>
http.post(`${BASE_URL}submissions/:uuid/_complete`, () =>
HttpResponse.json(
{
type: 'http://localhost:8000/fouten/ValidationError/',
code: 'invalid',
title: 'Does not validate.',
status: 400,
detail: '',
instance: 'urn:uuid:41e0174a-efc2-4cc0-9bf2-8366242a4e75',
invalidParams,
},
{status: 400}
)
);

/**
* Simulate a successful backend processing status without payment.
*/
Expand Down
8 changes: 5 additions & 3 deletions src/components/Summary/GenericSummary.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ const GenericSummary = ({

return (
<Card title={title}>
{errors.map(error => (
<ErrorMessage key={error}>{error}</ErrorMessage>
{errors.map((error, index) => (
<div className="openforms-card__alert" key={`error-${index}`}>
<ErrorMessage>{error}</ErrorMessage>
</div>
))}
<Formik
initialValues={{privacyPolicyAccepted: false, statementOfTruthAccepted: false}}
Expand Down Expand Up @@ -102,7 +104,7 @@ GenericSummary.propTypes = {
editStepText: PropTypes.string,
isLoading: PropTypes.bool,
isAuthenticated: PropTypes.bool,
errors: PropTypes.arrayOf(PropTypes.string),
errors: PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.node, PropTypes.string])),
prevPage: PropTypes.string,
onSubmit: PropTypes.func.isRequired,
onDestroySession: PropTypes.func.isRequired,
Expand Down
26 changes: 24 additions & 2 deletions src/components/Summary/SubmissionSummary.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import {LiteralsProvider} from 'components/Literal';
import {useSubmissionContext} from 'components/SubmissionProvider';
import {SUBMISSION_ALLOWED} from 'components/constants';
import {findPreviousApplicableStep} from 'components/utils';
import {ValidationError} from 'errors';
import useFormContext from 'hooks/useFormContext';
import useRefreshSubmission from 'hooks/useRefreshSubmission';
import useTitle from 'hooks/useTitle';

import GenericSummary from './GenericSummary';
import ValidationErrors from './ValidationErrors';
import {loadSummaryData} from './data';

const completeSubmission = async (submission, statementValues) => {
Expand All @@ -29,7 +31,7 @@ const SubmissionSummary = () => {
const {submission, onDestroySession, removeSubmissionId} = useSubmissionContext();
const refreshedSubmission = useRefreshSubmission(submission);

const [submitError, setSubmitError] = useState('');
const [submitErrors, setSubmitErrors] = useState(null);

const pageTitle = intl.formatMessage({
description: 'Summary page title',
Expand All @@ -52,11 +54,17 @@ const SubmissionSummary = () => {

const onSubmit = async statementValues => {
if (refreshedSubmission.submissionAllowed !== SUBMISSION_ALLOWED.yes) return;

let statusUrl;
try {
statusUrl = await completeSubmission(refreshedSubmission, statementValues);
} catch (e) {
setSubmitError(e.message);
if (e instanceof ValidationError) {
const {initialErrors} = e.asFormikProps();
setSubmitErrors(initialErrors);
} else {
setSubmitErrors(e.message);
}
return;
}

Expand Down Expand Up @@ -86,6 +94,20 @@ const SubmissionSummary = () => {
navigate(getPreviousPage());
};

const submitError =
submitErrors &&
(typeof submitErrors === 'string' ? (
submitErrors
) : (
<>
<FormattedMessage
description="Summary page generic validation error message"
defaultMessage="There are problems with the submitted data."
/>
<ValidationErrors errors={submitErrors} summaryData={summaryData} />
</>
));

const errorMessages = [location.state?.errorMessage, submitError].filter(Boolean);

return (
Expand Down
101 changes: 101 additions & 0 deletions src/components/Summary/SubmissionSummary.stories.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import {expect, fn, userEvent, within} from '@storybook/test';
import {withRouter} from 'storybook-addon-remix-react-router';

import {buildForm} from 'api-mocks';
import {
buildSubmission,
mockSubmissionCompleteInvalidPost,
mockSubmissionGet,
mockSubmissionSummaryGet,
} from 'api-mocks/submissions';
import SubmissionProvider from 'components/SubmissionProvider';
import {ConfigDecorator, withForm} from 'story-utils/decorators';

import SubmissionSummary from './SubmissionSummary';

const form = buildForm();
const submission = buildSubmission();

export default {
title: 'Private API / SubmissionSummary',
component: SubmissionSummary,
decorators: [
(Story, {args}) => (
<SubmissionProvider
submission={args.submission}
onSubmissionObtained={fn()}
onDestroySession={fn()}
removeSubmissionId={fn()}
>
<Story />
</SubmissionProvider>
),
withRouter,
ConfigDecorator,
withForm,
],
args: {
form,
submission,
},
argTypes: {
form: {table: {disable: true}},
submission: {table: {disable: true}},
},
parameters: {
msw: {
handlers: {
loadSubmission: [mockSubmissionGet(submission), mockSubmissionSummaryGet()],
},
},
},
};

export const Overview = {};

export const BackendValidationErrors = {
parameters: {
msw: {
handlers: {
completeSubmission: [
mockSubmissionCompleteInvalidPost([
{
name: 'steps.0.nonFieldErrors.0',
code: 'invalid',
reason: 'Your carpet is ugly.',
},
{
name: 'steps.0.nonFieldErrors.1',
code: 'invalid',
reason: "And your veg ain't in season.",
},
{
name: 'steps.0.data.component1',
code: 'existential-nightmare',
reason: 'I was waiting in line for ten whole minutes.',
},
]),
],
},
},
},
play: async ({canvasElement, step}) => {
const canvas = within(canvasElement);

await step('Submit form submission to backend', async () => {
await userEvent.click(
await canvas.findByRole('checkbox', {name: /I accept the privacy policy/})
);
await userEvent.click(canvas.getByRole('button', {name: 'Confirm'}));
});

await step('Check validation errors from backend', async () => {
const genericMessage = await canvas.findByText('There are problems with the submitted data.');
expect(genericMessage).toBeVisible();

expect(await canvas.findByText(/Your carpet is ugly/)).toBeVisible();
expect(await canvas.findByText(/And your veg/)).toBeVisible();
expect(await canvas.findByText(/I was waiting in line for ten whole minutes/)).toBeVisible();
});
},
};
131 changes: 131 additions & 0 deletions src/components/Summary/ValidationErrors.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import {UnorderedList, UnorderedListItem} from '@utrecht/component-library-react';
import PropTypes from 'prop-types';
import {FormattedMessage, useIntl} from 'react-intl';

const ErrorType = PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)]);

const normalizeError = error => (Array.isArray(error) ? error : [error]);

const StepValidationErrors = ({errors, name, stepData}) => {
const intl = useIntl();
const {nonFieldErrors = [], data = {}} = errors;

const allErrorMessages = [...normalizeError(nonFieldErrors)];

// related the data errors to their matching components
Object.entries(data).map(([key, errorMessage]) => {
const normalizedErrorMessage = normalizeError(errorMessage);
const stepDataItem = stepData.find(item => item.component.key === key);

for (const error of normalizedErrorMessage) {
const message = intl.formatMessage(
{
description: 'Overview page, validation error message for step field.',
defaultMessage: "Field ''{label}'': {error}",
},
{
label: stepDataItem.name,
error: error,
}
);
allErrorMessages.push(message);
}
});

return (
<>
<FormattedMessage
description="Overview page, title before listing the validation errors for a step"
defaultMessage="Problems in form step ''{name}''"
values={{name}}
/>
<UnorderedList className="utrecht-unordered-list--distanced">
{allErrorMessages.map(error => (
<UnorderedListItem key={error}>{error}</UnorderedListItem>
))}
</UnorderedList>
</>
);
};

StepValidationErrors.propTypes = {
errors: PropTypes.shape({
nonFieldErrors: ErrorType,
// keys are the component key names
data: PropTypes.objectOf(ErrorType),
}).isRequired,
name: PropTypes.string.isRequired,
stepData: PropTypes.arrayOf(
PropTypes.shape({
name: PropTypes.string.isRequired,
value: PropTypes.oneOfType([
PropTypes.string,
PropTypes.object,
PropTypes.array,
PropTypes.number,
PropTypes.bool,
]),
component: PropTypes.object.isRequired,
})
).isRequired,
};

/**
* Render the validation errors received from the backend.
*/
const ValidationErrors = ({errors, summaryData}) => {
const {steps = []} = errors;
if (steps.length === 0) return null;
return (
<UnorderedList className="utrecht-unordered-list--distanced">
{steps.map((stepErrors, index) => (
<UnorderedListItem key={summaryData[index].slug}>
<StepValidationErrors
errors={stepErrors}
name={summaryData[index].name}
stepData={summaryData[index].data}
/>
</UnorderedListItem>
))}
</UnorderedList>
);
};

ValidationErrors.propTypes = {
/**
* Validation errors as reconstructed from the backend invalidParams
*/
errors: PropTypes.shape({
steps: PropTypes.arrayOf(
PropTypes.shape({
nonFieldErrors: ErrorType,
// keys are the component key names
data: PropTypes.objectOf(ErrorType),
})
),
}),
/**
* Array of summary data per step.
*/
summaryData: PropTypes.arrayOf(
PropTypes.shape({
name: PropTypes.string.isRequired,
slug: PropTypes.string.isRequired,
data: PropTypes.arrayOf(
PropTypes.shape({
name: PropTypes.string.isRequired,
value: PropTypes.oneOfType([
PropTypes.string,
PropTypes.object,
PropTypes.array,
PropTypes.number,
PropTypes.bool,
]),
component: PropTypes.object.isRequired,
})
).isRequired,
})
),
};

export default ValidationErrors;
Loading

0 comments on commit 1355641

Please sign in to comment.