From 135564175c8b618247a0bc186307d74fda73ea9f Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 4 Feb 2025 17:41:06 +0100 Subject: [PATCH] :sparkles: [open-formulieren/open-forms#4510] Display backend validation errors 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. --- src/api-mocks/submissions.js | 18 ++- src/components/Summary/GenericSummary.jsx | 8 +- src/components/Summary/SubmissionSummary.jsx | 26 +++- .../Summary/SubmissionSummary.stories.jsx | 101 ++++++++++++++ src/components/Summary/ValidationErrors.jsx | 131 ++++++++++++++++++ src/i18n/compiled/en.json | 50 +++++++ src/i18n/compiled/nl.json | 50 +++++++ src/i18n/messages/en.json | 25 ++++ src/i18n/messages/nl.json | 25 ++++ src/scss/components/_card.scss | 6 + 10 files changed, 434 insertions(+), 6 deletions(-) create mode 100644 src/components/Summary/SubmissionSummary.stories.jsx create mode 100644 src/components/Summary/ValidationErrors.jsx diff --git a/src/api-mocks/submissions.js b/src/api-mocks/submissions.js index f40eb5764..e2a2191c8 100644 --- a/src/api-mocks/submissions.js +++ b/src/api-mocks/submissions.js @@ -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], }, ], @@ -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. */ diff --git a/src/components/Summary/GenericSummary.jsx b/src/components/Summary/GenericSummary.jsx index e369adfa0..545584d8f 100644 --- a/src/components/Summary/GenericSummary.jsx +++ b/src/components/Summary/GenericSummary.jsx @@ -34,8 +34,10 @@ const GenericSummary = ({ return ( - {errors.map(error => ( - {error} + {errors.map((error, index) => ( +
+ {error} +
))} { @@ -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', @@ -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; } @@ -86,6 +94,20 @@ const SubmissionSummary = () => { navigate(getPreviousPage()); }; + const submitError = + submitErrors && + (typeof submitErrors === 'string' ? ( + submitErrors + ) : ( + <> + + + + )); + const errorMessages = [location.state?.errorMessage, submitError].filter(Boolean); return ( diff --git a/src/components/Summary/SubmissionSummary.stories.jsx b/src/components/Summary/SubmissionSummary.stories.jsx new file mode 100644 index 000000000..fb7d939af --- /dev/null +++ b/src/components/Summary/SubmissionSummary.stories.jsx @@ -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}) => ( + + + + ), + 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(); + }); + }, +}; diff --git a/src/components/Summary/ValidationErrors.jsx b/src/components/Summary/ValidationErrors.jsx new file mode 100644 index 000000000..ba6288c5a --- /dev/null +++ b/src/components/Summary/ValidationErrors.jsx @@ -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 ( + <> + + + {allErrorMessages.map(error => ( + {error} + ))} + + + ); +}; + +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 ( + + {steps.map((stepErrors, index) => ( + + + + ))} + + ); +}; + +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; diff --git a/src/i18n/compiled/en.json b/src/i18n/compiled/en.json index a8d22545c..129ac4200 100644 --- a/src/i18n/compiled/en.json +++ b/src/i18n/compiled/en.json @@ -663,6 +663,12 @@ "value": "The verification code may only contain letters (A-Z) and numbers (0-9)." } ], + "FKWQuw": [ + { + "type": 0, + "value": "Unfortunately, this form is currently unavailable due to an outage. Please try again later." + } + ], "FrFeZj": [ { "type": 0, @@ -1419,6 +1425,20 @@ "value": "." } ], + "YwnY5d": [ + { + "type": 0, + "value": "Problems in form step '" + }, + { + "type": 1, + "value": "name" + }, + { + "type": 0, + "value": "'" + } + ], "ZJoaby": [ { "children": [ @@ -1585,6 +1605,24 @@ "value": "Loading..." } ], + "eEC1P4": [ + { + "type": 0, + "value": "Field '" + }, + { + "type": 1, + "value": "label" + }, + { + "type": 0, + "value": "': " + }, + { + "type": 1, + "value": "error" + } + ], "eO6Ysb": [ { "type": 0, @@ -1663,6 +1701,12 @@ "value": "House letter must be a single letter." } ], + "hcw9Gf": [ + { + "type": 0, + "value": "There are problems with the submitted data." + } + ], "hkZ8N1": [ { "type": 0, @@ -2183,6 +2227,12 @@ "value": "Postcode must be four digits followed by two letters (e.g. 1234 AB)." } ], + "rWbce4": [ + { + "type": 0, + "value": "Form unavailable" + } + ], "riuSfc": [ { "type": 0, diff --git a/src/i18n/compiled/nl.json b/src/i18n/compiled/nl.json index d1524570a..48709b3ec 100644 --- a/src/i18n/compiled/nl.json +++ b/src/i18n/compiled/nl.json @@ -663,6 +663,12 @@ "value": "De bevestigingscode bestaat uit hoofdletters (A-Z) en getallen (0-9)." } ], + "FKWQuw": [ + { + "type": 0, + "value": "Unfortunately, this form is currently unavailable due to an outage. Please try again later." + } + ], "FrFeZj": [ { "type": 0, @@ -1419,6 +1425,20 @@ "value": " zijn." } ], + "YwnY5d": [ + { + "type": 0, + "value": "Problems in form step '" + }, + { + "type": 1, + "value": "name" + }, + { + "type": 0, + "value": "'" + } + ], "ZJoaby": [ { "children": [ @@ -1589,6 +1609,24 @@ "value": "Laden..." } ], + "eEC1P4": [ + { + "type": 0, + "value": "Field '" + }, + { + "type": 1, + "value": "label" + }, + { + "type": 0, + "value": "': " + }, + { + "type": 1, + "value": "error" + } + ], "eO6Ysb": [ { "type": 0, @@ -1667,6 +1705,12 @@ "value": "Huisletter moet een enkele letter zijn." } ], + "hcw9Gf": [ + { + "type": 0, + "value": "There are problems with the submitted data." + } + ], "hkZ8N1": [ { "type": 0, @@ -2187,6 +2231,12 @@ "value": "Postcode moet bestaan uit vier cijfers gevolgd door twee letters (bijv. 1234 AB)." } ], + "rWbce4": [ + { + "type": 0, + "value": "Form unavailable" + } + ], "riuSfc": [ { "type": 0, diff --git a/src/i18n/messages/en.json b/src/i18n/messages/en.json index c075c5ed0..f7204bc7e 100644 --- a/src/i18n/messages/en.json +++ b/src/i18n/messages/en.json @@ -369,6 +369,11 @@ "description": "Validation error message for verification code pattern", "originalDefault": "The verification code may only contain letters (A-Z) and numbers (0-9)." }, + "FKWQuw": { + "defaultMessage": "Unfortunately, this form is currently unavailable due to an outage. Please try again later.", + "description": "Open Forms service unavailable error message", + "originalDefault": "Unfortunately, this form is currently unavailable due to an outage. Please try again later." + }, "FrFeZj": { "defaultMessage": "You haven't entered an email address yet.", "description": "Email verification modal: warning that no email is specified", @@ -714,6 +719,11 @@ "description": "ZOD 'invalid_union_discriminator' error message", "originalDefault": "Invalid discriminator value. Expected {expected}." }, + "YwnY5d": { + "defaultMessage": "Problems in form step ''{name}''", + "description": "Overview page, title before listing the validation errors for a step", + "originalDefault": "Problems in form step ''{name}''" + }, "ZJoaby": { "defaultMessage": "

Thank you for cosigning. We have received your \"{formName}\" submission. {confirmationEmailEnabled, select, true {You will receive a confirmation email.} other {} }

Do not forget to download your summary, we do not send it via email.

", "description": "Cosign confirmation page body", @@ -804,6 +814,11 @@ "description": "Loading content text", "originalDefault": "Loading..." }, + "eEC1P4": { + "defaultMessage": "Field ''{label}'': {error}", + "description": "Overview page, validation error message for step field.", + "originalDefault": "Field ''{label}'': {error}" + }, "eO6Ysb": { "defaultMessage": "Your payment is currently processing.", "description": "payment processing status", @@ -859,6 +874,11 @@ "description": "ZOD error message when AddressNL house letter does not match the house letter regular expression", "originalDefault": "House letter must be a single letter." }, + "hcw9Gf": { + "defaultMessage": "There are problems with the submitted data.", + "description": "Summary page generic validation error message", + "originalDefault": "There are problems with the submitted data." + }, "hkZ8N1": { "defaultMessage": "Invalid input.", "description": "ZOD 'custom' error message", @@ -1079,6 +1099,11 @@ "description": "ZOD error message when AddressNL postcode does not match the postcode regular expression", "originalDefault": "Postcode must be four digits followed by two letters (e.g. 1234 AB)." }, + "rWbce4": { + "defaultMessage": "Form unavailable", + "description": "Open Forms service unavailable error title", + "originalDefault": "Form unavailable" + }, "riuSfc": { "defaultMessage": "Send code", "description": "Email verification: send code button text", diff --git a/src/i18n/messages/nl.json b/src/i18n/messages/nl.json index b2e835870..75d095836 100644 --- a/src/i18n/messages/nl.json +++ b/src/i18n/messages/nl.json @@ -373,6 +373,11 @@ "description": "Validation error message for verification code pattern", "originalDefault": "The verification code may only contain letters (A-Z) and numbers (0-9)." }, + "FKWQuw": { + "defaultMessage": "Unfortunately, this form is currently unavailable due to an outage. Please try again later.", + "description": "Open Forms service unavailable error message", + "originalDefault": "Unfortunately, this form is currently unavailable due to an outage. Please try again later." + }, "FrFeZj": { "defaultMessage": "Je hebt nog geen e-mailadres ingevuld.", "description": "Email verification modal: warning that no email is specified", @@ -722,6 +727,11 @@ "description": "ZOD 'invalid_union_discriminator' error message", "originalDefault": "Invalid discriminator value. Expected {expected}." }, + "YwnY5d": { + "defaultMessage": "Problems in form step ''{name}''", + "description": "Overview page, title before listing the validation errors for a step", + "originalDefault": "Problems in form step ''{name}''" + }, "ZJoaby": { "defaultMessage": "

Bedankt voor het medeondertekenen. Wij hebben je inzending \"{formName}\" ontvangen. {confirmationEmailEnabled, select, true {Je ontvangt per mail een bevestiging.} other {} }

Vergeet niet je aanvraag te downloaden, deze versturen wij niet per mail.

", "description": "Cosign confirmation page body", @@ -813,6 +823,11 @@ "description": "Loading content text", "originalDefault": "Loading..." }, + "eEC1P4": { + "defaultMessage": "Field ''{label}'': {error}", + "description": "Overview page, validation error message for step field.", + "originalDefault": "Field ''{label}'': {error}" + }, "eO6Ysb": { "defaultMessage": "De betaling wordt momenteel verwerkt.", "description": "payment processing status", @@ -868,6 +883,11 @@ "description": "ZOD error message when AddressNL house letter does not match the house letter regular expression", "originalDefault": "House letter must be a single letter." }, + "hcw9Gf": { + "defaultMessage": "There are problems with the submitted data.", + "description": "Summary page generic validation error message", + "originalDefault": "There are problems with the submitted data." + }, "hkZ8N1": { "defaultMessage": "Ongeldige invoer.", "description": "ZOD 'custom' error message", @@ -1091,6 +1111,11 @@ "description": "ZOD error message when AddressNL postcode does not match the postcode regular expression", "originalDefault": "Postcode must be four digits followed by two letters (e.g. 1234 AB)." }, + "rWbce4": { + "defaultMessage": "Form unavailable", + "description": "Open Forms service unavailable error title", + "originalDefault": "Form unavailable" + }, "riuSfc": { "defaultMessage": "Verstuur code", "description": "Email verification: send code button text", diff --git a/src/scss/components/_card.scss b/src/scss/components/_card.scss index aa74fa84f..040e97b39 100644 --- a/src/scss/components/_card.scss +++ b/src/scss/components/_card.scss @@ -47,4 +47,10 @@ @include rows(0, margin-top); } } + + @include bem.element('alert') { + &:not(:last-child) { + margin-block-end: 20px; + } + } }