From ac3c204a19df9834efc93af795213707d179d931 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 30 Sep 2025 10:33:56 +0100 Subject: [PATCH 01/25] Stash --- .../forms/register-as-a-unicorn-breeder.yaml | 1 + .../engine/components/FormComponent.ts | 3 +- .../pageControllers/SummaryPageController.ts | 60 +++++++++++++++++++ src/server/plugins/engine/views/summary.html | 7 +++ 4 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/server/forms/register-as-a-unicorn-breeder.yaml b/src/server/forms/register-as-a-unicorn-breeder.yaml index 345dd4930..3c20764c8 100644 --- a/src/server/forms/register-as-a-unicorn-breeder.yaml +++ b/src/server/forms/register-as-a-unicorn-breeder.yaml @@ -1,5 +1,6 @@ --- name: Register as a unicorn breeder +declaration: "

All the answers you have provided are true to the best of your knowledge.

" pages: - path: '/whats-your-name' title: What's your name? diff --git a/src/server/plugins/engine/components/FormComponent.ts b/src/server/plugins/engine/components/FormComponent.ts index ff04cccab..f903d6013 100644 --- a/src/server/plugins/engine/components/FormComponent.ts +++ b/src/server/plugins/engine/components/FormComponent.ts @@ -149,7 +149,8 @@ export class FormComponent extends ComponentBase { return { ...viewModel, label: { - text: label + text: label, + classes: 'labelClasses' in options ? options.labelClasses : '' }, id: name, name, diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 14d4ecae4..6d8a8f62b 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -1,4 +1,5 @@ import { + ComponentType, hasComponentsEvenIfNoNext, type FormMetadata, type Page, @@ -6,8 +7,10 @@ import { } from '@defra/forms-model' import Boom from '@hapi/boom' import { type RouteOptions } from '@hapi/hapi' +import Joi from 'joi' import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' +import { EmailAddressField } from '~/src/server/plugins/engine/components/EmailAddressField.js' import { FileUploadField } from '~/src/server/plugins/engine/components/FileUploadField.js' import { getAnswer } from '~/src/server/plugins/engine/components/helpers/components.js' import { @@ -27,6 +30,7 @@ import { QuestionPageController } from '~/src/server/plugins/engine/pageControll import { type FormContext, type FormContextRequest, + type FormSubmissionError, type FormSubmissionState } from '~/src/server/plugins/engine/types.js' import { @@ -37,6 +41,14 @@ import { type FormResponseToolkit } from '~/src/server/routes/types.js' +const schema = Joi.object({ + crumb: Joi.string().required(), + action: Joi.string().required(), + citizenEmail: Joi.string().email().optional().allow('').allow(null).messages({ + '*': 'Enter your email address in the correct format' + }) +}) + export class SummaryPageController extends QuestionPageController { declare pageDef: Page allowSaveAndExit = true @@ -54,6 +66,25 @@ export class SummaryPageController extends QuestionPageController { hasComponentsEvenIfNoNext(pageDef) ? pageDef.components : [], { model, page: this } ) + + // TODO - insert just before 'declaration' (if one is defined) + this.collection.components.push( + new EmailAddressField( + { + title: 'Confirmation email', + shortDescription: 'Email address', + id: 'citizenEmail', + name: 'citizenEmail', + type: ComponentType.EmailAddressField, + hint: 'Enter your email address to get an email confirming your form has been submitted', + options: { + required: false, + labelClasses: 'govuk-label--m' + } + }, + { model } + ) + ) } getSummaryViewModel( @@ -73,10 +104,27 @@ export class SummaryPageController extends QuestionPageController { viewModel.phaseTag = this.phaseTag viewModel.components = components viewModel.allowSaveAndExit = this.shouldShowSaveAndExit(request.server) + viewModel.errors = errors return viewModel } + validateSummary( + request: FormContextRequest + ): FormSubmissionError[] | undefined { + const { error } = schema.validate(request.payload, { abortEarly: false }) + if (error) { + return error.details.map((detail) => ({ + message: detail.message, + path: detail.path, + text: detail.message, + href: `#${detail.context?.key}`, + name: detail.context?.key ?? 'unknown' + })) + } + return undefined + } + /** * Returns an async function. This is called in plugin.ts when there is a GET request at `/{id}/{path*}`, */ @@ -127,6 +175,18 @@ export class SummaryPageController extends QuestionPageController { const { isPreview } = checkFormStatus(request.params) const emailAddress = notificationEmail ?? this.model.def.outputEmail + const errors = this.validateSummary(request) + if (errors) { + cacheService.setFlash(request, { errors }) + const { viewName } = this + const viewModel = this.getSummaryViewModel(request, { + ...context, + errors, + payload: request.payload + }) + return h.view(viewName, viewModel) + } + checkEmailAddressForLiveFormSubmission(emailAddress, isPreview) // Send submission email diff --git a/src/server/plugins/engine/views/summary.html b/src/server/plugins/engine/views/summary.html index 22a112dc9..43c47d94c 100644 --- a/src/server/plugins/engine/views/summary.html +++ b/src/server/plugins/engine/views/summary.html @@ -12,6 +12,13 @@ {% include "partials/preview-banner.html" %} {% endif %} + {% if errors %} + {{ govukErrorSummary({ + titleText: "There is a problem", + errorList: checkErrorTemplates(errors) + }) }} + {% endif %} + {% if hasMissingNotificationEmail %} {% include "partials/warn-missing-notification-email.html" %} {% endif %} From cc2797a4092da62f2f498287edc79e708a3d96a3 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 3 Oct 2025 11:12:46 +0100 Subject: [PATCH 02/25] Stash --- .../forms/register-as-a-unicorn-breeder.yaml | 2 +- .../engine/components/FormComponent.ts | 5 +- src/server/plugins/engine/models/FormModel.ts | 5 +- .../SummaryPageController.test.ts | 64 +++++++++++++ .../pageControllers/SummaryPageController.ts | 89 ++++--------------- ...maryPageWithConfirmationEmailController.ts | 58 ++++++++++++ .../pageControllers/helpers/helpers.test.ts | 5 ++ .../engine/pageControllers/helpers/pages.ts | 8 ++ .../plugins/engine/pageControllers/index.ts | 1 + .../plugins/engine/routes/questions.test.ts | 4 + src/server/plugins/engine/views/summary.html | 4 +- 11 files changed, 168 insertions(+), 77 deletions(-) create mode 100644 src/server/plugins/engine/pageControllers/SummaryPageController.test.ts create mode 100644 src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts diff --git a/src/server/forms/register-as-a-unicorn-breeder.yaml b/src/server/forms/register-as-a-unicorn-breeder.yaml index 3c20764c8..955050c1e 100644 --- a/src/server/forms/register-as-a-unicorn-breeder.yaml +++ b/src/server/forms/register-as-a-unicorn-breeder.yaml @@ -19,7 +19,7 @@ pages: section: section - title: Summary path: '/summary' - controller: './pages/summary.js' + controller: './pages/summary-with-confirmation-email.js' components: [] next: [] - path: '/whats-your-email-address' diff --git a/src/server/plugins/engine/components/FormComponent.ts b/src/server/plugins/engine/components/FormComponent.ts index f903d6013..6152fab13 100644 --- a/src/server/plugins/engine/components/FormComponent.ts +++ b/src/server/plugins/engine/components/FormComponent.ts @@ -146,11 +146,14 @@ export class FormComponent extends ComponentBase { } } + const labelClasses = + 'labelClasses' in options ? { classes: options.labelClasses } : {} + return { ...viewModel, label: { text: label, - classes: 'labelClasses' in options ? options.labelClasses : '' + ...labelClasses }, id: name, name, diff --git a/src/server/plugins/engine/models/FormModel.ts b/src/server/plugins/engine/models/FormModel.ts index 3b380bcd4..65a45f088 100644 --- a/src/server/plugins/engine/models/FormModel.ts +++ b/src/server/plugins/engine/models/FormModel.ts @@ -547,7 +547,10 @@ function validateFormPayload( // Skip validation GET requests or other actions if ( !request.payload || - (action && ![FormAction.Validate, FormAction.SaveAndExit].includes(action)) + (action && + ![FormAction.Validate, FormAction.Send, FormAction.SaveAndExit].includes( + action + )) ) { return context } diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts new file mode 100644 index 000000000..6947fed16 --- /dev/null +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts @@ -0,0 +1,64 @@ +import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' +import { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' +import { type FormSubmissionState } from '~/src/server/plugins/engine/types.js' +import { + type FormRequestPayload, + type FormResponseToolkit +} from '~/src/server/routes/types.js' +import { type CacheService } from '~/src/server/services/cacheService.js' +import definition from '~/test/form/definitions/basic.js' + +describe('SummaryPageController', () => { + let model: FormModel + let controller: SummaryPageController + + beforeEach(() => { + model = new FormModel(definition, { + basePath: 'test' + }) + + // Create a mock page for SummaryPageController + const mockPage = { + ...definition.pages[0], + controller: 'summary' + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + controller = new SummaryPageController(model, mockPage as any) + }) + + describe('handle errors', () => { + it('should display errors including summary', () => { + const requestPage1 = { + abc: '123' + } + const h = {} as unknown as FormResponseToolkit + const saveAndExitMock = jest.fn(() => ({})) + const state: FormSubmissionState = { + $$__referenceNumber: 'foobar', + yesNoField: true + } + const request = { + ...requestPage1, + server: { + plugins: { + 'forms-engine-plugin': { + saveAndExit: saveAndExitMock, + cacheService: { + clearState: jest.fn() + } as unknown as CacheService + } + } + }, + method: 'post', + payload: { yesNoField: true, action: 'save-and-exit' } + } as unknown as FormRequestPayload + + const context = model.getFormContext(request, state) + + controller.handleSaveAndExit(request, context, h) + + expect(saveAndExitMock).toHaveBeenCalledWith(request, h, context) + }) + }) +}) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 6d8a8f62b..3dc79dc06 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -1,16 +1,12 @@ import { - ComponentType, hasComponentsEvenIfNoNext, type FormMetadata, type Page, type SubmitPayload } from '@defra/forms-model' import Boom from '@hapi/boom' -import { type RouteOptions } from '@hapi/hapi' -import Joi from 'joi' import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' -import { EmailAddressField } from '~/src/server/plugins/engine/components/EmailAddressField.js' import { FileUploadField } from '~/src/server/plugins/engine/components/FileUploadField.js' import { getAnswer } from '~/src/server/plugins/engine/components/helpers/components.js' import { @@ -30,24 +26,15 @@ import { QuestionPageController } from '~/src/server/plugins/engine/pageControll import { type FormContext, type FormContextRequest, - type FormSubmissionError, type FormSubmissionState } from '~/src/server/plugins/engine/types.js' import { FormAction, type FormRequest, type FormRequestPayload, - type FormRequestPayloadRefs, type FormResponseToolkit } from '~/src/server/routes/types.js' - -const schema = Joi.object({ - crumb: Joi.string().required(), - action: Joi.string().required(), - citizenEmail: Joi.string().email().optional().allow('').allow(null).messages({ - '*': 'Enter your email address in the correct format' - }) -}) +import { actionSchema, crumbSchema } from '~/src/server/schemas/index.js' export class SummaryPageController extends QuestionPageController { declare pageDef: Page @@ -67,24 +54,10 @@ export class SummaryPageController extends QuestionPageController { { model, page: this } ) - // TODO - insert just before 'declaration' (if one is defined) - this.collection.components.push( - new EmailAddressField( - { - title: 'Confirmation email', - shortDescription: 'Email address', - id: 'citizenEmail', - name: 'citizenEmail', - type: ComponentType.EmailAddressField, - hint: 'Enter your email address to get an email confirming your form has been submitted', - options: { - required: false, - labelClasses: 'govuk-label--m' - } - }, - { model } - ) - ) + this.collection.formSchema = this.collection.formSchema.keys({ + crumb: crumbSchema, + action: actionSchema + }) } getSummaryViewModel( @@ -109,22 +82,6 @@ export class SummaryPageController extends QuestionPageController { return viewModel } - validateSummary( - request: FormContextRequest - ): FormSubmissionError[] | undefined { - const { error } = schema.validate(request.payload, { abortEarly: false }) - if (error) { - return error.details.map((detail) => ({ - message: detail.message, - path: detail.path, - text: detail.message, - href: `#${detail.context?.key}`, - name: detail.context?.key ?? 'unknown' - })) - } - return undefined - } - /** * Returns an async function. This is called in plugin.ts when there is a GET request at `/{id}/{path*}`, */ @@ -157,6 +114,8 @@ export class SummaryPageController extends QuestionPageController { ) => { const { model } = this const { params } = request + const { viewName } = this + const { isForceAccess } = context // Check if this is a save-and-exit action const { action } = request.payload @@ -164,6 +123,16 @@ export class SummaryPageController extends QuestionPageController { return this.handleSaveAndExit(request, context, h) } + /** + * If there are any errors, render the page with the parsed errors + * @todo Refactor to match POST REDIRECT GET pattern + */ + if (context.errors || isForceAccess) { + const viewModel = this.getSummaryViewModel(request, context) + viewModel.errors = this.collection.getViewErrors(viewModel.errors) + return h.view(viewName, viewModel) + } + const cacheService = getCacheService(request.server) const { formsService } = this.model.services @@ -175,18 +144,6 @@ export class SummaryPageController extends QuestionPageController { const { isPreview } = checkFormStatus(request.params) const emailAddress = notificationEmail ?? this.model.def.outputEmail - const errors = this.validateSummary(request) - if (errors) { - cacheService.setFlash(request, { errors }) - const { viewName } = this - const viewModel = this.getSummaryViewModel(request, { - ...context, - errors, - payload: request.payload - }) - return h.view(viewName, viewModel) - } - checkEmailAddressForLiveFormSubmission(emailAddress, isPreview) // Send submission email @@ -210,18 +167,6 @@ export class SummaryPageController extends QuestionPageController { return this.proceed(request, h, this.getStatusPath()) } } - - get postRouteOptions(): RouteOptions { - return { - ext: { - onPreHandler: { - method(request, h) { - return h.continue - } - } - } - } - } } async function submitForm( diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts new file mode 100644 index 000000000..866fc8b68 --- /dev/null +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts @@ -0,0 +1,58 @@ +import { + ComponentType, + hasComponentsEvenIfNoNext, + type ComponentDef, + type Page +} from '@defra/forms-model' +import findLastIndex from 'lodash/findLastIndex.js' + +import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' +import { type FormModel } from '~/src/server/plugins/engine/models/index.js' +import { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' +import { actionSchema, crumbSchema } from '~/src/server/schemas/index.js' + +export class SummaryPageWithConfirmationEmailController extends SummaryPageController { + constructor(model: FormModel, pageDef: Page) { + super(model, pageDef) + + this.addConfirmationEmailAddress(pageDef) + + // Components collection + this.collection = new ComponentCollection( + hasComponentsEvenIfNoNext(pageDef) ? pageDef.components : [], + { model, page: this } + ) + + this.collection.formSchema = this.collection.formSchema.keys({ + crumb: crumbSchema, + action: actionSchema + }) + } + + addConfirmationEmailAddress(pageDef: Page) { + const confirmationEmailAddress = { + title: 'Confirmation email', + shortDescription: 'Email address', + id: 'confirmationEmailAddress', + name: 'confirmationEmailAddress', + type: ComponentType.EmailAddressField, + hint: 'Enter your email address to get an email confirming your form has been submitted', + classes: 'my-label-override-class', + options: { + required: false + } + } as ComponentDef + + if (hasComponentsEvenIfNoNext(pageDef)) { + const declarationPos = findLastIndex( + pageDef.components, + (comp) => comp.type === ComponentType.Markdown + ) + if (declarationPos > -1) { + pageDef.components.splice(declarationPos, 0, confirmationEmailAddress) + } else { + pageDef.components.push(confirmationEmailAddress) + } + } + } +} diff --git a/src/server/plugins/engine/pageControllers/helpers/helpers.test.ts b/src/server/plugins/engine/pageControllers/helpers/helpers.test.ts index 65d774708..b9474b054 100644 --- a/src/server/plugins/engine/pageControllers/helpers/helpers.test.ts +++ b/src/server/plugins/engine/pageControllers/helpers/helpers.test.ts @@ -21,6 +21,7 @@ import { StartPageController, StatusPageController, SummaryPageController, + SummaryPageWithConfirmationEmailController, TerminalPageController } from '~/src/server/plugins/engine/pageControllers/index.js' import definition from '~/test/form/definitions/blank.js' @@ -67,6 +68,10 @@ describe('Page controller helpers', () => { controller = SummaryPageController break + case ControllerType.SummaryWithConfirmationEmail: + controller = SummaryPageWithConfirmationEmailController + break + case ControllerType.Status: controller = StatusPageController break diff --git a/src/server/plugins/engine/pageControllers/helpers/pages.ts b/src/server/plugins/engine/pageControllers/helpers/pages.ts index a49389887..166b350ba 100644 --- a/src/server/plugins/engine/pageControllers/helpers/pages.ts +++ b/src/server/plugins/engine/pageControllers/helpers/pages.ts @@ -52,6 +52,14 @@ export function createPage(model: FormModel, pageDef: Page) { controller = new PageControllers.SummaryPageController(model, pageDef) break + case ControllerType.SummaryWithConfirmationEmail: + controller = + new PageControllers.SummaryPageWithConfirmationEmailController( + model, + pageDef + ) + break + case ControllerType.Status: controller = new PageControllers.StatusPageController(model, pageDef) break diff --git a/src/server/plugins/engine/pageControllers/index.ts b/src/server/plugins/engine/pageControllers/index.ts index 7a24af16c..954552280 100644 --- a/src/server/plugins/engine/pageControllers/index.ts +++ b/src/server/plugins/engine/pageControllers/index.ts @@ -5,6 +5,7 @@ export { } from '~/src/server/plugins/engine/pageControllers/QuestionPageController.js' export { TerminalPageController } from '~/src/server/plugins/engine/pageControllers/TerminalPageController.js' export { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' +export { SummaryPageWithConfirmationEmailController } from '~/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.js' export { StatusPageController } from '~/src/server/plugins/engine/pageControllers/StatusPageController.js' export { FileUploadPageController } from '~/src/server/plugins/engine/pageControllers/FileUploadPageController.js' export { RepeatPageController } from '~/src/server/plugins/engine/pageControllers/RepeatPageController.js' diff --git a/src/server/plugins/engine/routes/questions.test.ts b/src/server/plugins/engine/routes/questions.test.ts index c9940ed5d..6b56873e8 100644 --- a/src/server/plugins/engine/routes/questions.test.ts +++ b/src/server/plugins/engine/routes/questions.test.ts @@ -25,9 +25,13 @@ jest.mock('~/src/server/plugins/engine/models/SummaryViewModel', () => ({ } })) +// eslint-disable-next-line @typescript-eslint/no-unsafe-return jest.mock( '~/src/server/plugins/engine/pageControllers/SummaryPageController', () => ({ + ...jest.requireActual( + '~/src/server/plugins/engine/pageControllers/SummaryPageController' + ), getFormSubmissionData: jest.fn().mockReturnValue([]) }) ) diff --git a/src/server/plugins/engine/views/summary.html b/src/server/plugins/engine/views/summary.html index 43c47d94c..093a2897e 100644 --- a/src/server/plugins/engine/views/summary.html +++ b/src/server/plugins/engine/views/summary.html @@ -40,6 +40,8 @@

+ {{ componentList(components) }} + {% if declaration %}

Declaration

@@ -47,8 +49,6 @@

Declaration

{% endif %} - {{ componentList(components) }} -
{% set isDeclaration = declaration or components | length %} From 28c5991f148386d8dadd8ca733268bcedca7c321 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 3 Oct 2025 14:19:30 +0100 Subject: [PATCH 03/25] Reworked and added unit tests --- .../engine/components/FormComponent.ts | 6 +- .../pageControllers/QuestionPageController.ts | 25 ++- .../SummaryPageController.test.ts | 67 ++++++- .../pageControllers/SummaryPageController.ts | 2 + ...ageWithConfirmationEmailController.test.ts | 188 ++++++++++++++++++ ...maryPageWithConfirmationEmailController.ts | 50 ++--- .../plugins/engine/routes/questions.test.ts | 2 +- 7 files changed, 289 insertions(+), 51 deletions(-) create mode 100644 src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts diff --git a/src/server/plugins/engine/components/FormComponent.ts b/src/server/plugins/engine/components/FormComponent.ts index 6152fab13..ff04cccab 100644 --- a/src/server/plugins/engine/components/FormComponent.ts +++ b/src/server/plugins/engine/components/FormComponent.ts @@ -146,14 +146,10 @@ export class FormComponent extends ComponentBase { } } - const labelClasses = - 'labelClasses' in options ? { classes: options.labelClasses } : {} - return { ...viewModel, label: { - text: label, - ...labelClasses + text: label }, id: name, name, diff --git a/src/server/plugins/engine/pageControllers/QuestionPageController.ts b/src/server/plugins/engine/pageControllers/QuestionPageController.ts index b912a6449..ca4d30b27 100644 --- a/src/server/plugins/engine/pageControllers/QuestionPageController.ts +++ b/src/server/plugins/engine/pageControllers/QuestionPageController.ts @@ -14,7 +14,10 @@ import { type ValidationErrorItem } from 'joi' import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' import { optionalText } from '~/src/server/plugins/engine/components/constants.js' -import { type BackLink } from '~/src/server/plugins/engine/components/types.js' +import { + type BackLink, + type ComponentViewModel +} from '~/src/server/plugins/engine/components/types.js' import { getCacheService, getErrors, @@ -162,14 +165,7 @@ export class QuestionPageController extends PageController { } else if (formComponents.length > 1) { // When there is more than one form component, // adjust the label/legends to give equal prominence - for (const { model } of formComponents) { - if (model.fieldset?.legend) { - model.fieldset.legend.classes = 'govuk-fieldset__legend--m' - } - if (model.label) { - model.label.classes = 'govuk-label--m' - } - } + this.applyLabelOrLegendClass(formComponents) } return { @@ -183,6 +179,17 @@ export class QuestionPageController extends PageController { } } + applyLabelOrLegendClass(formComponents: ComponentViewModel[]) { + for (const { model } of formComponents) { + if (model.fieldset?.legend) { + model.fieldset.legend.classes = 'govuk-fieldset__legend--m' + } + if (model.label) { + model.label.classes = 'govuk-label--m' + } + } + } + getRelevantPath(request: AnyFormRequest, context: FormContext) { const { paths } = context diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts index 6947fed16..8a8ced1f9 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts @@ -1,7 +1,9 @@ import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' +import { buildFormRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' import { type FormSubmissionState } from '~/src/server/plugins/engine/types.js' import { + type FormRequest, type FormRequestPayload, type FormResponseToolkit } from '~/src/server/routes/types.js' @@ -11,6 +13,15 @@ import definition from '~/test/form/definitions/basic.js' describe('SummaryPageController', () => { let model: FormModel let controller: SummaryPageController + let requestPage: FormRequest + + const response = { + code: jest.fn().mockImplementation(() => response) + } + const h: FormResponseToolkit = { + redirect: jest.fn().mockReturnValue(response), + view: jest.fn() + } beforeEach(() => { model = new FormModel(definition, { @@ -25,21 +36,60 @@ describe('SummaryPageController', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any controller = new SummaryPageController(model, mockPage as any) + + requestPage = buildFormRequest({ + method: 'get', + url: new URL('http://example.com/test/summary'), + path: '/test/summary', + params: { + path: 'summary', + slug: 'test' + }, + query: {}, + app: { model } + } as FormRequest) }) describe('handle errors', () => { - it('should display errors including summary', () => { - const requestPage1 = { - abc: '123' + it('should display errors including summary', async () => { + const state: FormSubmissionState = { + $$__referenceNumber: 'foobar', + licenceLength: 365, + fullName: 'John Smith' } - const h = {} as unknown as FormResponseToolkit + const request = { + ...requestPage, + method: 'post', + payload: { invalid: '123', action: 'send' } + } as unknown as FormRequestPayload + + const context = model.getFormContext(request, state) + + jest.spyOn(controller, 'getState').mockResolvedValue({}) + jest.spyOn(controller, 'setState').mockResolvedValue(state) + + const postHandler = controller.makePostRouteHandler() + await postHandler(request, context, h) + + const viewModel = controller.getSummaryViewModel(request, context) + + expect(h.view).toHaveBeenCalledWith('summary', expect.anything()) + expect(viewModel.errors).toHaveLength(1) + const errorText = viewModel.errors ? viewModel.errors[0].text : '' + expect(errorText).toBe('invalid is not allowed') + }) + }) + + describe('handleSaveAndExit', () => { + it('should invoke saveAndExit plugin option', async () => { const saveAndExitMock = jest.fn(() => ({})) const state: FormSubmissionState = { $$__referenceNumber: 'foobar', - yesNoField: true + licenceLength: 365, + fullName: 'John Smith' } const request = { - ...requestPage1, + ...requestPage, server: { plugins: { 'forms-engine-plugin': { @@ -51,12 +101,13 @@ describe('SummaryPageController', () => { } }, method: 'post', - payload: { yesNoField: true, action: 'save-and-exit' } + payload: { fullName: 'John Smith', action: 'save-and-exit' } } as unknown as FormRequestPayload const context = model.getFormContext(request, state) - controller.handleSaveAndExit(request, context, h) + const postHandler = controller.makePostRouteHandler() + await postHandler(request, context, h) expect(saveAndExitMock).toHaveBeenCalledWith(request, h, context) }) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 3dc79dc06..66ff7625a 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -70,6 +70,8 @@ export class SummaryPageController extends QuestionPageController { const { payload, errors } = context const components = this.collection.getViewModel(payload, errors, query) + this.applyLabelOrLegendClass(components) + // We already figure these out in the base page controller. Take them and apply them to our page-specific model. // This is a stop-gap until we can add proper inheritance in place. viewModel.backLink = this.getBackLink(request, context) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts new file mode 100644 index 000000000..bbcc242b5 --- /dev/null +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts @@ -0,0 +1,188 @@ +import { + ControllerType, + type PageSummaryWithConfirmationEmail +} from '@defra/forms-model' + +import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' +import { + SummaryPageWithConfirmationEmailController, + addConfirmationEmailAddress +} from '~/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.js' +import { buildFormRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' +import { type FormSubmissionState } from '~/src/server/plugins/engine/types.js' +import { + type FormRequest, + type FormRequestPayload, + type FormResponseToolkit +} from '~/src/server/routes/types.js' +import { type CacheService } from '~/src/server/services/cacheService.js' +import definition from '~/test/form/definitions/basic.js' + +describe('SummaryPageWithConfirmationEmailController', () => { + let model: FormModel + let controller: SummaryPageWithConfirmationEmailController + let requestPage: FormRequest + + const response = { + code: jest.fn().mockImplementation(() => response) + } + const h: FormResponseToolkit = { + redirect: jest.fn().mockReturnValue(response), + view: jest.fn() + } + + beforeEach(() => { + model = new FormModel(definition, { + basePath: 'test' + }) + + // Create a mock page for SummaryPageWithConfirmationEmailController + const mockPage = { + ...definition.pages[0], + controller: ControllerType.SummaryWithConfirmationEmail + } as unknown as PageSummaryWithConfirmationEmail + + controller = new SummaryPageWithConfirmationEmailController(model, mockPage) + + requestPage = buildFormRequest({ + method: 'get', + url: new URL('http://example.com/test/summary'), + path: '/test/summary', + params: { + path: 'summary', + slug: 'test' + }, + query: {}, + app: { model } + } as FormRequest) + }) + + describe('handle errors', () => { + it('should display errors including summary', async () => { + const state: FormSubmissionState = { + $$__referenceNumber: 'foobar', + licenceLength: 365, + fullName: 'John Smith' + } + const request = { + ...requestPage, + method: 'post', + payload: { invalid: '123', action: 'send' } + } as unknown as FormRequestPayload + + const context = model.getFormContext(request, state) + + jest.spyOn(controller, 'getState').mockResolvedValue({}) + jest.spyOn(controller, 'setState').mockResolvedValue(state) + + const postHandler = controller.makePostRouteHandler() + await postHandler(request, context, h) + + const viewModel = controller.getSummaryViewModel(request, context) + + expect(h.view).toHaveBeenCalledWith('summary', expect.anything()) + expect(viewModel.errors).toHaveLength(1) + const errorText = viewModel.errors ? viewModel.errors[0].text : '' + expect(errorText).toBe('invalid is not allowed') + }) + }) + + describe('handleSaveAndExit', () => { + it('should invoke saveAndExit plugin option', async () => { + const saveAndExitMock = jest.fn(() => ({})) + const state: FormSubmissionState = { + $$__referenceNumber: 'foobar', + licenceLength: 365, + fullName: 'John Smith' + } + const request = { + ...requestPage, + server: { + plugins: { + 'forms-engine-plugin': { + saveAndExit: saveAndExitMock, + cacheService: { + clearState: jest.fn() + } as unknown as CacheService + } + } + }, + method: 'post', + payload: { fullName: 'John Smith', action: 'save-and-exit' } + } as unknown as FormRequestPayload + + const context = model.getFormContext(request, state) + + const postHandler = controller.makePostRouteHandler() + await postHandler(request, context, h) + + expect(saveAndExitMock).toHaveBeenCalledWith(request, h, context) + }) + }) + + describe('addConfirmationEmailAddress', () => { + const confirmationEmailField = { + hint: 'Enter your email address to get an email confirming your form has been submitted', + id: '20f50a94-2c35-466c-b802-9215753b383b', + name: 'confirmationEmailAddress', + options: { + required: false + }, + shortDescription: 'Email address', + title: 'Confirmation email', + type: 'EmailAddressField' + } + + test('should add confirmation email', () => { + const pageDef = { + components: [], + path: '/summary', + controller: ControllerType.SummaryWithConfirmationEmail, + title: 'Summary' + } as PageSummaryWithConfirmationEmail + addConfirmationEmailAddress(pageDef) + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const components = pageDef.components ?? [] + expect(components).toHaveLength(1) + expect(components).toEqual([confirmationEmailField]) + }) + + test('should not add confirmation email if already exists', () => { + const pageDef = { + components: [confirmationEmailField], + path: '/summary', + controller: ControllerType.SummaryWithConfirmationEmail, + title: 'Summary' + } as PageSummaryWithConfirmationEmail + addConfirmationEmailAddress(pageDef) + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const components = pageDef.components ?? [] + expect(components).toHaveLength(1) + }) + + test('should insert just before declaration', () => { + const pageDef = { + components: [ + { type: 'TextField' }, + { type: 'RadiosField' }, + { type: 'Markdown' } + ], + path: '/summary', + controller: ControllerType.SummaryWithConfirmationEmail, + title: 'Summary' + } as PageSummaryWithConfirmationEmail + addConfirmationEmailAddress(pageDef) + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const components = pageDef.components ?? [] + expect(components).toHaveLength(4) + // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return + const fieldTypes = components.map((x) => x.type) + expect(fieldTypes).toEqual([ + 'TextField', + 'RadiosField', + 'EmailAddressField', + 'Markdown' + ]) + }) + }) +}) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts index 866fc8b68..01c6b3bfd 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts @@ -6,53 +6,47 @@ import { } from '@defra/forms-model' import findLastIndex from 'lodash/findLastIndex.js' -import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' import { type FormModel } from '~/src/server/plugins/engine/models/index.js' import { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' -import { actionSchema, crumbSchema } from '~/src/server/schemas/index.js' + +export const CONFIRMATION_EMAIL_GUID = '20f50a94-2c35-466c-b802-9215753b383b' export class SummaryPageWithConfirmationEmailController extends SummaryPageController { constructor(model: FormModel, pageDef: Page) { + addConfirmationEmailAddress(pageDef) super(model, pageDef) - - this.addConfirmationEmailAddress(pageDef) - - // Components collection - this.collection = new ComponentCollection( - hasComponentsEvenIfNoNext(pageDef) ? pageDef.components : [], - { model, page: this } - ) - - this.collection.formSchema = this.collection.formSchema.keys({ - crumb: crumbSchema, - action: actionSchema - }) } +} + +export function addConfirmationEmailAddress(pageDef: Page) { + if (hasComponentsEvenIfNoNext(pageDef)) { + // Abort if already added + if ( + pageDef.components.find((comp) => comp.id === CONFIRMATION_EMAIL_GUID) + ) { + return + } - addConfirmationEmailAddress(pageDef: Page) { const confirmationEmailAddress = { title: 'Confirmation email', shortDescription: 'Email address', - id: 'confirmationEmailAddress', + id: CONFIRMATION_EMAIL_GUID, name: 'confirmationEmailAddress', type: ComponentType.EmailAddressField, hint: 'Enter your email address to get an email confirming your form has been submitted', - classes: 'my-label-override-class', options: { required: false } } as ComponentDef - if (hasComponentsEvenIfNoNext(pageDef)) { - const declarationPos = findLastIndex( - pageDef.components, - (comp) => comp.type === ComponentType.Markdown - ) - if (declarationPos > -1) { - pageDef.components.splice(declarationPos, 0, confirmationEmailAddress) - } else { - pageDef.components.push(confirmationEmailAddress) - } + const declarationPos = findLastIndex( + pageDef.components, + (comp) => comp.type === ComponentType.Markdown + ) + if (declarationPos > -1) { + pageDef.components.splice(declarationPos, 0, confirmationEmailAddress) + } else { + pageDef.components.push(confirmationEmailAddress) } } } diff --git a/src/server/plugins/engine/routes/questions.test.ts b/src/server/plugins/engine/routes/questions.test.ts index 6b56873e8..bff753bcf 100644 --- a/src/server/plugins/engine/routes/questions.test.ts +++ b/src/server/plugins/engine/routes/questions.test.ts @@ -25,9 +25,9 @@ jest.mock('~/src/server/plugins/engine/models/SummaryViewModel', () => ({ } })) -// eslint-disable-next-line @typescript-eslint/no-unsafe-return jest.mock( '~/src/server/plugins/engine/pageControllers/SummaryPageController', + // eslint-disable-next-line @typescript-eslint/no-unsafe-return () => ({ ...jest.requireActual( '~/src/server/plugins/engine/pageControllers/SummaryPageController' From d7ccfca588ed1f330747ae4b6598862f24513ece Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 3 Oct 2025 15:00:17 +0100 Subject: [PATCH 04/25] Uses updated forms-model version --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index b8a693b48..f82391786 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@defra/forms-model": "^3.0.555", + "@defra/forms-model": "^3.0.558", "@defra/hapi-tracing": "^1.26.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", @@ -2272,9 +2272,9 @@ } }, "node_modules/@defra/forms-model": { - "version": "3.0.555", - "resolved": "https://registry.npmjs.org/@defra/forms-model/-/forms-model-3.0.555.tgz", - "integrity": "sha512-RgPYOiwdzxI3WSNp+WD/GKZam72GUlZEpUDbciejQBdkpWMbMcKQkPbyOLuHjHp+/ZyR3T7xySlGEkUtPc9Gzw==", + "version": "3.0.558", + "resolved": "https://registry.npmjs.org/@defra/forms-model/-/forms-model-3.0.558.tgz", + "integrity": "sha512-+c3qYFnh57g7xw24+osaCSMB08tqSPAUzOz6K942qJT4ZR/RjGEefb4t1FszMpAKjytRGqIAy40dzNdkYwSwMQ==", "license": "OGL-UK-3.0", "dependencies": { "@joi/date": "^2.1.1", diff --git a/package.json b/package.json index 94f7540e2..e260b5e94 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,7 @@ }, "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@defra/forms-model": "^3.0.555", + "@defra/forms-model": "^3.0.558", "@defra/hapi-tracing": "^1.26.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", From 7160dd7cfdebf503d3284b32bd6316eabf6cf08f Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 3 Oct 2025 15:05:34 +0100 Subject: [PATCH 05/25] Sonar fix --- .../SummaryPageWithConfirmationEmailController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts index 01c6b3bfd..dac5e439c 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts @@ -22,7 +22,7 @@ export function addConfirmationEmailAddress(pageDef: Page) { if (hasComponentsEvenIfNoNext(pageDef)) { // Abort if already added if ( - pageDef.components.find((comp) => comp.id === CONFIRMATION_EMAIL_GUID) + pageDef.components.some((comp) => comp.id === CONFIRMATION_EMAIL_GUID) ) { return } From cafc7dc9698338864f09983868260e91f4d71003 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 3 Oct 2025 15:22:29 +0100 Subject: [PATCH 06/25] Changed field name --- ...ummaryPageWithConfirmationEmailController.test.ts | 12 ++++++------ .../SummaryPageWithConfirmationEmailController.ts | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts index bbcc242b5..bb0e3e334 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts @@ -6,7 +6,7 @@ import { import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { SummaryPageWithConfirmationEmailController, - addConfirmationEmailAddress + addUserConfirmationEmailAddress } from '~/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.js' import { buildFormRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' import { type FormSubmissionState } from '~/src/server/plugins/engine/types.js' @@ -120,11 +120,11 @@ describe('SummaryPageWithConfirmationEmailController', () => { }) }) - describe('addConfirmationEmailAddress', () => { + describe('addUserConfirmationEmailAddress', () => { const confirmationEmailField = { hint: 'Enter your email address to get an email confirming your form has been submitted', id: '20f50a94-2c35-466c-b802-9215753b383b', - name: 'confirmationEmailAddress', + name: 'userConfirmationEmailAddress', options: { required: false }, @@ -140,7 +140,7 @@ describe('SummaryPageWithConfirmationEmailController', () => { controller: ControllerType.SummaryWithConfirmationEmail, title: 'Summary' } as PageSummaryWithConfirmationEmail - addConfirmationEmailAddress(pageDef) + addUserConfirmationEmailAddress(pageDef) // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access const components = pageDef.components ?? [] expect(components).toHaveLength(1) @@ -154,7 +154,7 @@ describe('SummaryPageWithConfirmationEmailController', () => { controller: ControllerType.SummaryWithConfirmationEmail, title: 'Summary' } as PageSummaryWithConfirmationEmail - addConfirmationEmailAddress(pageDef) + addUserConfirmationEmailAddress(pageDef) // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access const components = pageDef.components ?? [] expect(components).toHaveLength(1) @@ -171,7 +171,7 @@ describe('SummaryPageWithConfirmationEmailController', () => { controller: ControllerType.SummaryWithConfirmationEmail, title: 'Summary' } as PageSummaryWithConfirmationEmail - addConfirmationEmailAddress(pageDef) + addUserConfirmationEmailAddress(pageDef) // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access const components = pageDef.components ?? [] expect(components).toHaveLength(4) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts index dac5e439c..85d7a0afc 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts @@ -13,12 +13,12 @@ export const CONFIRMATION_EMAIL_GUID = '20f50a94-2c35-466c-b802-9215753b383b' export class SummaryPageWithConfirmationEmailController extends SummaryPageController { constructor(model: FormModel, pageDef: Page) { - addConfirmationEmailAddress(pageDef) + addUserConfirmationEmailAddress(pageDef) super(model, pageDef) } } -export function addConfirmationEmailAddress(pageDef: Page) { +export function addUserConfirmationEmailAddress(pageDef: Page) { if (hasComponentsEvenIfNoNext(pageDef)) { // Abort if already added if ( @@ -27,11 +27,11 @@ export function addConfirmationEmailAddress(pageDef: Page) { return } - const confirmationEmailAddress = { + const userConfirmationEmailAddress = { title: 'Confirmation email', shortDescription: 'Email address', id: CONFIRMATION_EMAIL_GUID, - name: 'confirmationEmailAddress', + name: 'userConfirmationEmailAddress', type: ComponentType.EmailAddressField, hint: 'Enter your email address to get an email confirming your form has been submitted', options: { @@ -44,9 +44,9 @@ export function addConfirmationEmailAddress(pageDef: Page) { (comp) => comp.type === ComponentType.Markdown ) if (declarationPos > -1) { - pageDef.components.splice(declarationPos, 0, confirmationEmailAddress) + pageDef.components.splice(declarationPos, 0, userConfirmationEmailAddress) } else { - pageDef.components.push(confirmationEmailAddress) + pageDef.components.push(userConfirmationEmailAddress) } } } From 3a19b5dc5854e544184f699b71b036bff7e5c69e Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Mon, 6 Oct 2025 09:20:12 +0100 Subject: [PATCH 07/25] Added const --- .../SummaryPageWithConfirmationEmailController.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts index 85d7a0afc..234c42d97 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts @@ -10,6 +10,7 @@ import { type FormModel } from '~/src/server/plugins/engine/models/index.js' import { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' export const CONFIRMATION_EMAIL_GUID = '20f50a94-2c35-466c-b802-9215753b383b' +export const CONFIRMATION_EMAIL_FIELD_NAME = 'userConfirmationEmailAddress' export class SummaryPageWithConfirmationEmailController extends SummaryPageController { constructor(model: FormModel, pageDef: Page) { @@ -31,7 +32,7 @@ export function addUserConfirmationEmailAddress(pageDef: Page) { title: 'Confirmation email', shortDescription: 'Email address', id: CONFIRMATION_EMAIL_GUID, - name: 'userConfirmationEmailAddress', + name: CONFIRMATION_EMAIL_FIELD_NAME, type: ComponentType.EmailAddressField, hint: 'Enter your email address to get an email confirming your form has been submitted', options: { From 13c57018ffafee01a09d1b32243e765a3ef5ae41 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Mon, 6 Oct 2025 16:29:17 +0100 Subject: [PATCH 08/25] Reworked to avoid framework restrictions --- .../plugins/engine/models/SummaryViewModel.ts | 3 +- .../pageControllers/SummaryPageController.ts | 99 ++++++-------- ...maryPageWithConfirmationEmailController.ts | 128 ++++++++++++------ src/server/plugins/engine/views/summary.html | 6 +- src/server/schemas/index.ts | 1 + src/server/types.ts | 3 +- 6 files changed, 142 insertions(+), 98 deletions(-) diff --git a/src/server/plugins/engine/models/SummaryViewModel.ts b/src/server/plugins/engine/models/SummaryViewModel.ts index 26308ab4f..23ff64a9b 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.ts @@ -1,4 +1,4 @@ -import { type Section } from '@defra/forms-model' +import { type GovukField, type Section } from '@defra/forms-model' import { getAnswer, @@ -52,6 +52,7 @@ export class SummaryViewModel { hasMissingNotificationEmail?: boolean components?: ComponentViewModel[] allowSaveAndExit = false + userConfirmationEmailField?: GovukField constructor( request: FormContextRequest, diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 66ff7625a..2a74ce503 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -34,7 +34,6 @@ import { type FormRequestPayload, type FormResponseToolkit } from '~/src/server/routes/types.js' -import { actionSchema, crumbSchema } from '~/src/server/schemas/index.js' export class SummaryPageController extends QuestionPageController { declare pageDef: Page @@ -53,11 +52,6 @@ export class SummaryPageController extends QuestionPageController { hasComponentsEvenIfNoNext(pageDef) ? pageDef.components : [], { model, page: this } ) - - this.collection.formSchema = this.collection.formSchema.keys({ - crumb: crumbSchema, - action: actionSchema - }) } getSummaryViewModel( @@ -70,8 +64,6 @@ export class SummaryPageController extends QuestionPageController { const { payload, errors } = context const components = this.collection.getViewModel(payload, errors, query) - this.applyLabelOrLegendClass(components) - // We already figure these out in the base page controller. Take them and apply them to our page-specific model. // This is a stop-gap until we can add proper inheritance in place. viewModel.backLink = this.getBackLink(request, context) @@ -114,70 +106,68 @@ export class SummaryPageController extends QuestionPageController { context: FormContext, h: FormResponseToolkit ) => { - const { model } = this - const { params } = request - const { viewName } = this - const { isForceAccess } = context - // Check if this is a save-and-exit action const { action } = request.payload if (action === FormAction.SaveAndExit) { return this.handleSaveAndExit(request, context, h) } - /** - * If there are any errors, render the page with the parsed errors - * @todo Refactor to match POST REDIRECT GET pattern - */ - if (context.errors || isForceAccess) { - const viewModel = this.getSummaryViewModel(request, context) - viewModel.errors = this.collection.getViewErrors(viewModel.errors) - return h.view(viewName, viewModel) - } + return this.handleFormSubmit(request, context, h) + } + } - const cacheService = getCacheService(request.server) - - const { formsService } = this.model.services - const { getFormMetadata } = formsService - - // Get the form metadata using the `slug` param - const formMetadata = await getFormMetadata(params.slug) - const { notificationEmail } = formMetadata - const { isPreview } = checkFormStatus(request.params) - const emailAddress = notificationEmail ?? this.model.def.outputEmail - - checkEmailAddressForLiveFormSubmission(emailAddress, isPreview) - - // Send submission email - if (emailAddress) { - const viewModel = this.getSummaryViewModel(request, context) - await submitForm( - context, - request, - viewModel, - model, - emailAddress, - formMetadata - ) - } + async handleFormSubmit( + request: FormRequestPayload, + context: FormContext, + h: FormResponseToolkit + ) { + const { model } = this + const { params } = request + + const cacheService = getCacheService(request.server) - await cacheService.setConfirmationState(request, { confirmed: true }) + const { formsService } = this.model.services + const { getFormMetadata } = formsService - // Clear all form data - await cacheService.clearState(request) + // Get the form metadata using the `slug` param + const formMetadata = await getFormMetadata(params.slug) + const { notificationEmail } = formMetadata + const { isPreview } = checkFormStatus(request.params) + const emailAddress = notificationEmail ?? this.model.def.outputEmail - return this.proceed(request, h, this.getStatusPath()) + checkEmailAddressForLiveFormSubmission(emailAddress, isPreview) + + // Send submission email + if (emailAddress) { + const viewModel = this.getSummaryViewModel(request, context) + await submitForm( + context, + request, + viewModel, + model, + emailAddress, + formMetadata, + viewModel.userConfirmationEmailField?.value as string + ) } + + await cacheService.setConfirmationState(request, { confirmed: true }) + + // Clear all form data + await cacheService.clearState(request) + + return this.proceed(request, h, this.getStatusPath()) } } -async function submitForm( +export async function submitForm( context: FormContext, request: FormRequestPayload, summaryViewModel: SummaryViewModel, model: FormModel, emailAddress: string, - formMetadata: FormMetadata + formMetadata: FormMetadata, + userConfirmationEmailAddress?: string ) { await extendFileRetention(model, context.state, emailAddress) @@ -212,7 +202,8 @@ async function submitForm( emailAddress, items, submitResponse, - formMetadata + formMetadata, + userConfirmationEmailAddress ) } diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts index 234c42d97..fb5f1778a 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts @@ -1,53 +1,101 @@ -import { - ComponentType, - hasComponentsEvenIfNoNext, - type ComponentDef, - type Page -} from '@defra/forms-model' -import findLastIndex from 'lodash/findLastIndex.js' - -import { type FormModel } from '~/src/server/plugins/engine/models/index.js' +import { ComponentType, type GovukField } from '@defra/forms-model' +import Joi from 'joi' + +import { type SummaryViewModel } from '~/src/server/plugins/engine/models/index.js' import { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' +import { + type FormContext, + type FormContextRequest, + type FormPayload, + type FormSubmissionError +} from '~/src/server/plugins/engine/types.js' +import { + FormAction, + type FormRequestPayload, + type FormResponseToolkit +} from '~/src/server/routes/types.js' +import { + actionSchema, + crumbSchema, + userConfirmationEmailSchema +} from '~/src/server/schemas/index.js' -export const CONFIRMATION_EMAIL_GUID = '20f50a94-2c35-466c-b802-9215753b383b' export const CONFIRMATION_EMAIL_FIELD_NAME = 'userConfirmationEmailAddress' +const schema = Joi.object().keys({ + crumb: crumbSchema, + action: actionSchema, + userConfirmationEmailAddress: userConfirmationEmailSchema.messages({ + '*': 'Enter an email address in the correct format' + }) +}) + export class SummaryPageWithConfirmationEmailController extends SummaryPageController { - constructor(model: FormModel, pageDef: Page) { - addUserConfirmationEmailAddress(pageDef) - super(model, pageDef) + getSummaryViewModel( + request: FormContextRequest, + context: FormContext + ): SummaryViewModel { + const viewModel = super.getSummaryViewModel(request, context) + viewModel.userConfirmationEmailField = getUserConfirmationEmailAddress( + request.payload, + context.errors + ) + return viewModel } -} -export function addUserConfirmationEmailAddress(pageDef: Page) { - if (hasComponentsEvenIfNoNext(pageDef)) { - // Abort if already added - if ( - pageDef.components.some((comp) => comp.id === CONFIRMATION_EMAIL_GUID) - ) { - return - } + /** + * Returns an async function. This is called in plugin.ts when there is a POST request at `/{id}/{path*}`. + * If a form is incomplete, a user will be redirected to the start page. + */ + makePostRouteHandler() { + return async ( + request: FormRequestPayload, + context: FormContext, + h: FormResponseToolkit + ) => { + const { viewName } = this + const { isForceAccess } = context - const userConfirmationEmailAddress = { - title: 'Confirmation email', - shortDescription: 'Email address', - id: CONFIRMATION_EMAIL_GUID, - name: CONFIRMATION_EMAIL_FIELD_NAME, - type: ComponentType.EmailAddressField, - hint: 'Enter your email address to get an email confirming your form has been submitted', - options: { - required: false + // Check if this is a save-and-exit action + const { action } = request.payload + if (action === FormAction.SaveAndExit) { + return this.handleSaveAndExit(request, context, h) } - } as ComponentDef - const declarationPos = findLastIndex( - pageDef.components, - (comp) => comp.type === ComponentType.Markdown - ) - if (declarationPos > -1) { - pageDef.components.splice(declarationPos, 0, userConfirmationEmailAddress) - } else { - pageDef.components.push(userConfirmationEmailAddress) + /** + * If there are any errors, render the page with the parsed errors + * @todo Refactor to match POST REDIRECT GET pattern + */ + const { error } = schema.validate(request.payload, { abortEarly: false }) + if (error || isForceAccess) { + context.errors = this.getErrors(error?.details) + const viewModel = this.getSummaryViewModel(request, context) + return h.view(viewName, viewModel) + } + + return this.handleFormSubmit(request, context, h) } } } + +export function getUserConfirmationEmailAddress( + payload?: FormPayload, + errors?: FormSubmissionError[] +) { + return { + label: { + text: 'Confirmation email (optional)', + classes: 'govuk-label--m' + }, + shortDescription: 'Email address', + id: CONFIRMATION_EMAIL_FIELD_NAME, + name: CONFIRMATION_EMAIL_FIELD_NAME, + type: ComponentType.EmailAddressField, + hint: { + text: 'Enter your email address to get an email confirming your form has been submitted' + }, + attributes: {}, + value: payload ? payload[CONFIRMATION_EMAIL_FIELD_NAME] : undefined, + errorMessage: errors?.length ? errors[0].text : undefined + } as GovukField +} diff --git a/src/server/plugins/engine/views/summary.html b/src/server/plugins/engine/views/summary.html index 093a2897e..548259bc1 100644 --- a/src/server/plugins/engine/views/summary.html +++ b/src/server/plugins/engine/views/summary.html @@ -3,7 +3,7 @@ {% from "govuk/components/error-summary/macro.njk" import govukErrorSummary %} {% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} {% from "govuk/components/button/macro.njk" import govukButton %} -{% from "partials/components.html" import componentList with context %} +{% from "govuk/components/input/macro.njk" import govukInput %} {% block content %}
@@ -40,7 +40,9 @@

- {{ componentList(components) }} + {% if userConfirmationEmailField %} + {{ govukInput(userConfirmationEmailField) }} + {% endif %} {% if declaration %}

Declaration

diff --git a/src/server/schemas/index.ts b/src/server/schemas/index.ts index f181e7f01..630a0c70d 100644 --- a/src/server/schemas/index.ts +++ b/src/server/schemas/index.ts @@ -23,6 +23,7 @@ export const pathSchema = Joi.string().required() export const itemIdSchema = Joi.string().uuid().required() export const crumbSchema = Joi.string().optional().allow('') export const confirmSchema = Joi.boolean().empty(false) +export const userConfirmationEmailSchema = Joi.string().trim().email().allow('') export const paramsSchema = Joi.object() .keys({ diff --git a/src/server/types.ts b/src/server/types.ts index b5179ae00..8ecc462ab 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -63,6 +63,7 @@ export interface OutputService { emailAddress: string, items: DetailItem[], submitResponse: SubmitResponsePayload, - formMetadata?: FormMetadata + formMetadata?: FormMetadata, + userConfirmationEmailAddress?: string ) => Promise } From e2006b3c48de97c4a9dd0b2602d657543bb5a069 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Mon, 6 Oct 2025 16:32:13 +0100 Subject: [PATCH 09/25] Reverted method --- .../pageControllers/QuestionPageController.ts | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/server/plugins/engine/pageControllers/QuestionPageController.ts b/src/server/plugins/engine/pageControllers/QuestionPageController.ts index ca4d30b27..b912a6449 100644 --- a/src/server/plugins/engine/pageControllers/QuestionPageController.ts +++ b/src/server/plugins/engine/pageControllers/QuestionPageController.ts @@ -14,10 +14,7 @@ import { type ValidationErrorItem } from 'joi' import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' import { optionalText } from '~/src/server/plugins/engine/components/constants.js' -import { - type BackLink, - type ComponentViewModel -} from '~/src/server/plugins/engine/components/types.js' +import { type BackLink } from '~/src/server/plugins/engine/components/types.js' import { getCacheService, getErrors, @@ -165,7 +162,14 @@ export class QuestionPageController extends PageController { } else if (formComponents.length > 1) { // When there is more than one form component, // adjust the label/legends to give equal prominence - this.applyLabelOrLegendClass(formComponents) + for (const { model } of formComponents) { + if (model.fieldset?.legend) { + model.fieldset.legend.classes = 'govuk-fieldset__legend--m' + } + if (model.label) { + model.label.classes = 'govuk-label--m' + } + } } return { @@ -179,17 +183,6 @@ export class QuestionPageController extends PageController { } } - applyLabelOrLegendClass(formComponents: ComponentViewModel[]) { - for (const { model } of formComponents) { - if (model.fieldset?.legend) { - model.fieldset.legend.classes = 'govuk-fieldset__legend--m' - } - if (model.label) { - model.label.classes = 'govuk-label--m' - } - } - } - getRelevantPath(request: AnyFormRequest, context: FormContext) { const { paths } = context From ef146c2965f86cb8d406cb632f93a3b57e26f573 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Mon, 6 Oct 2025 17:06:14 +0100 Subject: [PATCH 10/25] Fixed tests --- .../SummaryPageController.test.ts | 30 -------- .../pageControllers/SummaryPageController.ts | 14 ++++ ...ageWithConfirmationEmailController.test.ts | 76 ++++--------------- ...maryPageWithConfirmationEmailController.ts | 5 +- 4 files changed, 28 insertions(+), 97 deletions(-) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts index 8a8ced1f9..3e3a367ef 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts @@ -50,36 +50,6 @@ describe('SummaryPageController', () => { } as FormRequest) }) - describe('handle errors', () => { - it('should display errors including summary', async () => { - const state: FormSubmissionState = { - $$__referenceNumber: 'foobar', - licenceLength: 365, - fullName: 'John Smith' - } - const request = { - ...requestPage, - method: 'post', - payload: { invalid: '123', action: 'send' } - } as unknown as FormRequestPayload - - const context = model.getFormContext(request, state) - - jest.spyOn(controller, 'getState').mockResolvedValue({}) - jest.spyOn(controller, 'setState').mockResolvedValue(state) - - const postHandler = controller.makePostRouteHandler() - await postHandler(request, context, h) - - const viewModel = controller.getSummaryViewModel(request, context) - - expect(h.view).toHaveBeenCalledWith('summary', expect.anything()) - expect(viewModel.errors).toHaveLength(1) - const errorText = viewModel.errors ? viewModel.errors[0].text : '' - expect(errorText).toBe('invalid is not allowed') - }) - }) - describe('handleSaveAndExit', () => { it('should invoke saveAndExit plugin option', async () => { const saveAndExitMock = jest.fn(() => ({})) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 2a74ce503..e35e55206 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -5,6 +5,7 @@ import { type SubmitPayload } from '@defra/forms-model' import Boom from '@hapi/boom' +import { type RouteOptions } from '@hapi/hapi' import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' import { FileUploadField } from '~/src/server/plugins/engine/components/FileUploadField.js' @@ -32,6 +33,7 @@ import { FormAction, type FormRequest, type FormRequestPayload, + type FormRequestPayloadRefs, type FormResponseToolkit } from '~/src/server/routes/types.js' @@ -158,6 +160,18 @@ export class SummaryPageController extends QuestionPageController { return this.proceed(request, h, this.getStatusPath()) } + + get postRouteOptions(): RouteOptions { + return { + ext: { + onPreHandler: { + method(request, h) { + return h.continue + } + } + } + } + } } export async function submitForm( diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts index bb0e3e334..e5cbb40b6 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts @@ -6,7 +6,7 @@ import { import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { SummaryPageWithConfirmationEmailController, - addUserConfirmationEmailAddress + getUserConfirmationEmailAddress } from '~/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.js' import { buildFormRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' import { type FormSubmissionState } from '~/src/server/plugins/engine/types.js' @@ -83,7 +83,7 @@ describe('SummaryPageWithConfirmationEmailController', () => { expect(h.view).toHaveBeenCalledWith('summary', expect.anything()) expect(viewModel.errors).toHaveLength(1) const errorText = viewModel.errors ? viewModel.errors[0].text : '' - expect(errorText).toBe('invalid is not allowed') + expect(errorText).toBe('"invalid" is not allowed') }) }) @@ -120,69 +120,19 @@ describe('SummaryPageWithConfirmationEmailController', () => { }) }) - describe('addUserConfirmationEmailAddress', () => { - const confirmationEmailField = { - hint: 'Enter your email address to get an email confirming your form has been submitted', - id: '20f50a94-2c35-466c-b802-9215753b383b', - name: 'userConfirmationEmailAddress', - options: { - required: false - }, - shortDescription: 'Email address', - title: 'Confirmation email', - type: 'EmailAddressField' - } - - test('should add confirmation email', () => { - const pageDef = { - components: [], - path: '/summary', - controller: ControllerType.SummaryWithConfirmationEmail, - title: 'Summary' - } as PageSummaryWithConfirmationEmail - addUserConfirmationEmailAddress(pageDef) - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const components = pageDef.components ?? [] - expect(components).toHaveLength(1) - expect(components).toEqual([confirmationEmailField]) - }) - - test('should not add confirmation email if already exists', () => { - const pageDef = { - components: [confirmationEmailField], - path: '/summary', - controller: ControllerType.SummaryWithConfirmationEmail, - title: 'Summary' - } as PageSummaryWithConfirmationEmail - addUserConfirmationEmailAddress(pageDef) - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const components = pageDef.components ?? [] - expect(components).toHaveLength(1) + describe('getUserConfirmationEmailAddress', () => { + test('should get confirmation email', () => { + const field = getUserConfirmationEmailAddress() + expect(field.name).toBe('userConfirmationEmailAddress') + expect(field.value).toBeUndefined() }) - test('should insert just before declaration', () => { - const pageDef = { - components: [ - { type: 'TextField' }, - { type: 'RadiosField' }, - { type: 'Markdown' } - ], - path: '/summary', - controller: ControllerType.SummaryWithConfirmationEmail, - title: 'Summary' - } as PageSummaryWithConfirmationEmail - addUserConfirmationEmailAddress(pageDef) - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const components = pageDef.components ?? [] - expect(components).toHaveLength(4) - // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return - const fieldTypes = components.map((x) => x.type) - expect(fieldTypes).toEqual([ - 'TextField', - 'RadiosField', - 'EmailAddressField', - 'Markdown' - ]) + test('should get confirmation email with retained value', () => { + const field = getUserConfirmationEmailAddress({ + userConfirmationEmailAddress: 'emailval' + }) + expect(field.name).toBe('userConfirmationEmailAddress') + expect(field.value).toBe('emailval') }) }) }) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts index fb5f1778a..4829ff168 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts @@ -1,4 +1,4 @@ -import { ComponentType, type GovukField } from '@defra/forms-model' +import { type GovukField } from '@defra/forms-model' import Joi from 'joi' import { type SummaryViewModel } from '~/src/server/plugins/engine/models/index.js' @@ -87,14 +87,11 @@ export function getUserConfirmationEmailAddress( text: 'Confirmation email (optional)', classes: 'govuk-label--m' }, - shortDescription: 'Email address', id: CONFIRMATION_EMAIL_FIELD_NAME, name: CONFIRMATION_EMAIL_FIELD_NAME, - type: ComponentType.EmailAddressField, hint: { text: 'Enter your email address to get an email confirming your form has been submitted' }, - attributes: {}, value: payload ? payload[CONFIRMATION_EMAIL_FIELD_NAME] : undefined, errorMessage: errors?.length ? errors[0].text : undefined } as GovukField From a9567786b1c103f41d804791582f6343592bc6ed Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 7 Oct 2025 08:29:53 +0100 Subject: [PATCH 11/25] Fixed test --- src/server/plugins/engine/views/summary.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/server/plugins/engine/views/summary.html b/src/server/plugins/engine/views/summary.html index 548259bc1..2716d7197 100644 --- a/src/server/plugins/engine/views/summary.html +++ b/src/server/plugins/engine/views/summary.html @@ -3,6 +3,7 @@ {% from "govuk/components/error-summary/macro.njk" import govukErrorSummary %} {% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} {% from "govuk/components/button/macro.njk" import govukButton %} +{% from "partials/components.html" import componentList with context %} {% from "govuk/components/input/macro.njk" import govukInput %} {% block content %} @@ -40,6 +41,8 @@

+ {{ componentList(components) }} + {% if userConfirmationEmailField %} {{ govukInput(userConfirmationEmailField) }} {% endif %} From 334b2bb61d925a1c4066700eeb9168993e7a87e6 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 7 Oct 2025 13:50:27 +0100 Subject: [PATCH 12/25] Reverted --- .../forms/register-as-a-unicorn-breeder.yaml | 2 +- .../pageControllers/SummaryPageController.ts | 23 +-- ...ageWithConfirmationEmailController.test.ts | 138 ------------------ ...maryPageWithConfirmationEmailController.ts | 98 ------------- .../pageControllers/helpers/helpers.test.ts | 5 - .../engine/pageControllers/helpers/pages.ts | 8 - .../plugins/engine/pageControllers/index.ts | 1 - .../plugins/engine/routes/questions.test.ts | 4 - .../engine/services/notifyService.test.ts | 12 +- .../plugins/engine/services/notifyService.ts | 6 +- src/server/types.ts | 8 +- 11 files changed, 30 insertions(+), 275 deletions(-) delete mode 100644 src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts delete mode 100644 src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts diff --git a/src/server/forms/register-as-a-unicorn-breeder.yaml b/src/server/forms/register-as-a-unicorn-breeder.yaml index 955050c1e..3c20764c8 100644 --- a/src/server/forms/register-as-a-unicorn-breeder.yaml +++ b/src/server/forms/register-as-a-unicorn-breeder.yaml @@ -19,7 +19,7 @@ pages: section: section - title: Summary path: '/summary' - controller: './pages/summary-with-confirmation-email.js' + controller: './pages/summary.js' components: [] next: [] - path: '/whats-your-email-address' diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index e35e55206..4f32af0a3 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -135,19 +135,20 @@ export class SummaryPageController extends QuestionPageController { const formMetadata = await getFormMetadata(params.slug) const { notificationEmail } = formMetadata const { isPreview } = checkFormStatus(request.params) - const emailAddress = notificationEmail ?? this.model.def.outputEmail + const submissionEmailAddress = + notificationEmail ?? this.model.def.outputEmail - checkEmailAddressForLiveFormSubmission(emailAddress, isPreview) + checkEmailAddressForLiveFormSubmission(submissionEmailAddress, isPreview) // Send submission email - if (emailAddress) { + if (submissionEmailAddress) { const viewModel = this.getSummaryViewModel(request, context) await submitForm( context, request, viewModel, model, - emailAddress, + submissionEmailAddress, formMetadata, viewModel.userConfirmationEmailField?.value as string ) @@ -179,11 +180,11 @@ export async function submitForm( request: FormRequestPayload, summaryViewModel: SummaryViewModel, model: FormModel, - emailAddress: string, + submissionEmailAddress: string, formMetadata: FormMetadata, userConfirmationEmailAddress?: string ) { - await extendFileRetention(model, context.state, emailAddress) + await extendFileRetention(model, context.state, submissionEmailAddress) const formStatus = checkFormStatus(request.params) const logTags = ['submit', 'submissionApi'] @@ -201,7 +202,7 @@ export async function submitForm( const submitResponse = await submitData( model, items, - emailAddress, + submissionEmailAddress, request.yar.id ) @@ -213,11 +214,13 @@ export async function submitForm( context, request, model, - emailAddress, + { + submissionEmailAddress, + userConfirmationEmailAddress + }, items, submitResponse, - formMetadata, - userConfirmationEmailAddress + formMetadata ) } diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts deleted file mode 100644 index e5cbb40b6..000000000 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.test.ts +++ /dev/null @@ -1,138 +0,0 @@ -import { - ControllerType, - type PageSummaryWithConfirmationEmail -} from '@defra/forms-model' - -import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' -import { - SummaryPageWithConfirmationEmailController, - getUserConfirmationEmailAddress -} from '~/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.js' -import { buildFormRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' -import { type FormSubmissionState } from '~/src/server/plugins/engine/types.js' -import { - type FormRequest, - type FormRequestPayload, - type FormResponseToolkit -} from '~/src/server/routes/types.js' -import { type CacheService } from '~/src/server/services/cacheService.js' -import definition from '~/test/form/definitions/basic.js' - -describe('SummaryPageWithConfirmationEmailController', () => { - let model: FormModel - let controller: SummaryPageWithConfirmationEmailController - let requestPage: FormRequest - - const response = { - code: jest.fn().mockImplementation(() => response) - } - const h: FormResponseToolkit = { - redirect: jest.fn().mockReturnValue(response), - view: jest.fn() - } - - beforeEach(() => { - model = new FormModel(definition, { - basePath: 'test' - }) - - // Create a mock page for SummaryPageWithConfirmationEmailController - const mockPage = { - ...definition.pages[0], - controller: ControllerType.SummaryWithConfirmationEmail - } as unknown as PageSummaryWithConfirmationEmail - - controller = new SummaryPageWithConfirmationEmailController(model, mockPage) - - requestPage = buildFormRequest({ - method: 'get', - url: new URL('http://example.com/test/summary'), - path: '/test/summary', - params: { - path: 'summary', - slug: 'test' - }, - query: {}, - app: { model } - } as FormRequest) - }) - - describe('handle errors', () => { - it('should display errors including summary', async () => { - const state: FormSubmissionState = { - $$__referenceNumber: 'foobar', - licenceLength: 365, - fullName: 'John Smith' - } - const request = { - ...requestPage, - method: 'post', - payload: { invalid: '123', action: 'send' } - } as unknown as FormRequestPayload - - const context = model.getFormContext(request, state) - - jest.spyOn(controller, 'getState').mockResolvedValue({}) - jest.spyOn(controller, 'setState').mockResolvedValue(state) - - const postHandler = controller.makePostRouteHandler() - await postHandler(request, context, h) - - const viewModel = controller.getSummaryViewModel(request, context) - - expect(h.view).toHaveBeenCalledWith('summary', expect.anything()) - expect(viewModel.errors).toHaveLength(1) - const errorText = viewModel.errors ? viewModel.errors[0].text : '' - expect(errorText).toBe('"invalid" is not allowed') - }) - }) - - describe('handleSaveAndExit', () => { - it('should invoke saveAndExit plugin option', async () => { - const saveAndExitMock = jest.fn(() => ({})) - const state: FormSubmissionState = { - $$__referenceNumber: 'foobar', - licenceLength: 365, - fullName: 'John Smith' - } - const request = { - ...requestPage, - server: { - plugins: { - 'forms-engine-plugin': { - saveAndExit: saveAndExitMock, - cacheService: { - clearState: jest.fn() - } as unknown as CacheService - } - } - }, - method: 'post', - payload: { fullName: 'John Smith', action: 'save-and-exit' } - } as unknown as FormRequestPayload - - const context = model.getFormContext(request, state) - - const postHandler = controller.makePostRouteHandler() - await postHandler(request, context, h) - - expect(saveAndExitMock).toHaveBeenCalledWith(request, h, context) - }) - }) - - describe('getUserConfirmationEmailAddress', () => { - test('should get confirmation email', () => { - const field = getUserConfirmationEmailAddress() - expect(field.name).toBe('userConfirmationEmailAddress') - expect(field.value).toBeUndefined() - }) - - test('should get confirmation email with retained value', () => { - const field = getUserConfirmationEmailAddress({ - userConfirmationEmailAddress: 'emailval' - }) - expect(field.name).toBe('userConfirmationEmailAddress') - expect(field.value).toBe('emailval') - }) - }) -}) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts b/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts deleted file mode 100644 index 4829ff168..000000000 --- a/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.ts +++ /dev/null @@ -1,98 +0,0 @@ -import { type GovukField } from '@defra/forms-model' -import Joi from 'joi' - -import { type SummaryViewModel } from '~/src/server/plugins/engine/models/index.js' -import { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' -import { - type FormContext, - type FormContextRequest, - type FormPayload, - type FormSubmissionError -} from '~/src/server/plugins/engine/types.js' -import { - FormAction, - type FormRequestPayload, - type FormResponseToolkit -} from '~/src/server/routes/types.js' -import { - actionSchema, - crumbSchema, - userConfirmationEmailSchema -} from '~/src/server/schemas/index.js' - -export const CONFIRMATION_EMAIL_FIELD_NAME = 'userConfirmationEmailAddress' - -const schema = Joi.object().keys({ - crumb: crumbSchema, - action: actionSchema, - userConfirmationEmailAddress: userConfirmationEmailSchema.messages({ - '*': 'Enter an email address in the correct format' - }) -}) - -export class SummaryPageWithConfirmationEmailController extends SummaryPageController { - getSummaryViewModel( - request: FormContextRequest, - context: FormContext - ): SummaryViewModel { - const viewModel = super.getSummaryViewModel(request, context) - viewModel.userConfirmationEmailField = getUserConfirmationEmailAddress( - request.payload, - context.errors - ) - return viewModel - } - - /** - * Returns an async function. This is called in plugin.ts when there is a POST request at `/{id}/{path*}`. - * If a form is incomplete, a user will be redirected to the start page. - */ - makePostRouteHandler() { - return async ( - request: FormRequestPayload, - context: FormContext, - h: FormResponseToolkit - ) => { - const { viewName } = this - const { isForceAccess } = context - - // Check if this is a save-and-exit action - const { action } = request.payload - if (action === FormAction.SaveAndExit) { - return this.handleSaveAndExit(request, context, h) - } - - /** - * If there are any errors, render the page with the parsed errors - * @todo Refactor to match POST REDIRECT GET pattern - */ - const { error } = schema.validate(request.payload, { abortEarly: false }) - if (error || isForceAccess) { - context.errors = this.getErrors(error?.details) - const viewModel = this.getSummaryViewModel(request, context) - return h.view(viewName, viewModel) - } - - return this.handleFormSubmit(request, context, h) - } - } -} - -export function getUserConfirmationEmailAddress( - payload?: FormPayload, - errors?: FormSubmissionError[] -) { - return { - label: { - text: 'Confirmation email (optional)', - classes: 'govuk-label--m' - }, - id: CONFIRMATION_EMAIL_FIELD_NAME, - name: CONFIRMATION_EMAIL_FIELD_NAME, - hint: { - text: 'Enter your email address to get an email confirming your form has been submitted' - }, - value: payload ? payload[CONFIRMATION_EMAIL_FIELD_NAME] : undefined, - errorMessage: errors?.length ? errors[0].text : undefined - } as GovukField -} diff --git a/src/server/plugins/engine/pageControllers/helpers/helpers.test.ts b/src/server/plugins/engine/pageControllers/helpers/helpers.test.ts index b9474b054..65d774708 100644 --- a/src/server/plugins/engine/pageControllers/helpers/helpers.test.ts +++ b/src/server/plugins/engine/pageControllers/helpers/helpers.test.ts @@ -21,7 +21,6 @@ import { StartPageController, StatusPageController, SummaryPageController, - SummaryPageWithConfirmationEmailController, TerminalPageController } from '~/src/server/plugins/engine/pageControllers/index.js' import definition from '~/test/form/definitions/blank.js' @@ -68,10 +67,6 @@ describe('Page controller helpers', () => { controller = SummaryPageController break - case ControllerType.SummaryWithConfirmationEmail: - controller = SummaryPageWithConfirmationEmailController - break - case ControllerType.Status: controller = StatusPageController break diff --git a/src/server/plugins/engine/pageControllers/helpers/pages.ts b/src/server/plugins/engine/pageControllers/helpers/pages.ts index 166b350ba..a49389887 100644 --- a/src/server/plugins/engine/pageControllers/helpers/pages.ts +++ b/src/server/plugins/engine/pageControllers/helpers/pages.ts @@ -52,14 +52,6 @@ export function createPage(model: FormModel, pageDef: Page) { controller = new PageControllers.SummaryPageController(model, pageDef) break - case ControllerType.SummaryWithConfirmationEmail: - controller = - new PageControllers.SummaryPageWithConfirmationEmailController( - model, - pageDef - ) - break - case ControllerType.Status: controller = new PageControllers.StatusPageController(model, pageDef) break diff --git a/src/server/plugins/engine/pageControllers/index.ts b/src/server/plugins/engine/pageControllers/index.ts index 954552280..7a24af16c 100644 --- a/src/server/plugins/engine/pageControllers/index.ts +++ b/src/server/plugins/engine/pageControllers/index.ts @@ -5,7 +5,6 @@ export { } from '~/src/server/plugins/engine/pageControllers/QuestionPageController.js' export { TerminalPageController } from '~/src/server/plugins/engine/pageControllers/TerminalPageController.js' export { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' -export { SummaryPageWithConfirmationEmailController } from '~/src/server/plugins/engine/pageControllers/SummaryPageWithConfirmationEmailController.js' export { StatusPageController } from '~/src/server/plugins/engine/pageControllers/StatusPageController.js' export { FileUploadPageController } from '~/src/server/plugins/engine/pageControllers/FileUploadPageController.js' export { RepeatPageController } from '~/src/server/plugins/engine/pageControllers/RepeatPageController.js' diff --git a/src/server/plugins/engine/routes/questions.test.ts b/src/server/plugins/engine/routes/questions.test.ts index bff753bcf..c9940ed5d 100644 --- a/src/server/plugins/engine/routes/questions.test.ts +++ b/src/server/plugins/engine/routes/questions.test.ts @@ -27,11 +27,7 @@ jest.mock('~/src/server/plugins/engine/models/SummaryViewModel', () => ({ jest.mock( '~/src/server/plugins/engine/pageControllers/SummaryPageController', - // eslint-disable-next-line @typescript-eslint/no-unsafe-return () => ({ - ...jest.requireActual( - '~/src/server/plugins/engine/pageControllers/SummaryPageController' - ), getFormSubmissionData: jest.fn().mockReturnValue([]) }) ) diff --git a/src/server/plugins/engine/services/notifyService.test.ts b/src/server/plugins/engine/services/notifyService.test.ts index 1a9caf79f..fe283d0df 100644 --- a/src/server/plugins/engine/services/notifyService.test.ts +++ b/src/server/plugins/engine/services/notifyService.test.ts @@ -70,7 +70,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - 'test@defra.gov.uk', + { submissionEmailAddress: 'test@defra.gov.uk' }, items, submitResponse ) @@ -106,7 +106,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - 'test@defra.gov.uk', + { submissionEmailAddress: 'test@defra.gov.uk' }, items, submitResponse ) @@ -144,7 +144,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - 'test@defra.gov.uk', + { submissionEmailAddress: 'test@defra.gov.uk' }, items, submitResponse ) @@ -190,7 +190,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - 'test@defra.gov.uk', + { submissionEmailAddress: 'test@defra.gov.uk' }, items, submitResponse, formMetadata @@ -242,7 +242,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - 'test@defra.gov.uk', + { submissionEmailAddress: 'test@defra.gov.uk' }, items, submitResponse ) @@ -294,7 +294,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - 'test@defra.gov.uk', + { submissionEmailAddress: 'test@defra.gov.uk' }, items, submitResponse ) diff --git a/src/server/plugins/engine/services/notifyService.ts b/src/server/plugins/engine/services/notifyService.ts index e6bd3ee0a..93c52f692 100644 --- a/src/server/plugins/engine/services/notifyService.ts +++ b/src/server/plugins/engine/services/notifyService.ts @@ -25,7 +25,10 @@ export async function submit( context: FormContext, request: FormRequestPayload, model: FormModel, - emailAddress: string, + emailAddresses: { + submissionEmailAddress: string + userConfirmationEmailAddress?: string + }, items: DetailItem[], submitResponse: SubmitResponsePayload, formMetadata?: FormMetadata @@ -40,6 +43,7 @@ export async function submit( // Get submission email personalisation request.logger.info(logTags, 'Getting personalisation data') + const emailAddress = emailAddresses.submissionEmailAddress const formName = escapeMarkdown(model.name) const subject = formStatus.isPreview ? `TEST FORM SUBMISSION: ${formName}` diff --git a/src/server/types.ts b/src/server/types.ts index 8ecc462ab..2e13f09b2 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -60,10 +60,12 @@ export interface OutputService { context: FormContext, request: FormRequestPayload, model: FormModel, - emailAddress: string, + emailAddresses: { + submissionEmailAddress: string + userConfirmationEmailAddress?: string + }, items: DetailItem[], submitResponse: SubmitResponsePayload, - formMetadata?: FormMetadata, - userConfirmationEmailAddress?: string + formMetadata?: FormMetadata ) => Promise } From 661bb7f0a0301166777416040742b5ea49e9479b Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 7 Oct 2025 14:35:02 +0100 Subject: [PATCH 13/25] Changed to 'options' --- src/server/plugins/engine/services/notifyService.ts | 4 ++-- src/server/types.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/plugins/engine/services/notifyService.ts b/src/server/plugins/engine/services/notifyService.ts index 93c52f692..193c62243 100644 --- a/src/server/plugins/engine/services/notifyService.ts +++ b/src/server/plugins/engine/services/notifyService.ts @@ -25,7 +25,7 @@ export async function submit( context: FormContext, request: FormRequestPayload, model: FormModel, - emailAddresses: { + options: { submissionEmailAddress: string userConfirmationEmailAddress?: string }, @@ -43,7 +43,7 @@ export async function submit( // Get submission email personalisation request.logger.info(logTags, 'Getting personalisation data') - const emailAddress = emailAddresses.submissionEmailAddress + const emailAddress = options.submissionEmailAddress const formName = escapeMarkdown(model.name) const subject = formStatus.isPreview ? `TEST FORM SUBMISSION: ${formName}` diff --git a/src/server/types.ts b/src/server/types.ts index 2e13f09b2..61a4e0c58 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -60,7 +60,7 @@ export interface OutputService { context: FormContext, request: FormRequestPayload, model: FormModel, - emailAddresses: { + options: { submissionEmailAddress: string userConfirmationEmailAddress?: string }, From 288edfcd5c84d8e1305fbd6a5e325027f7e0821d Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 7 Oct 2025 15:18:52 +0100 Subject: [PATCH 14/25] Upversioned forms model --- package-lock.json | 8 ++--- package.json | 2 +- .../pageControllers/SummaryPageController.ts | 30 +++++++++++-------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5195fff36..1daec55bc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@defra/forms-model": "^3.0.558", + "@defra/forms-model": "^3.0.559", "@defra/hapi-tracing": "^1.26.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", @@ -2272,9 +2272,9 @@ } }, "node_modules/@defra/forms-model": { - "version": "3.0.558", - "resolved": "https://registry.npmjs.org/@defra/forms-model/-/forms-model-3.0.558.tgz", - "integrity": "sha512-+c3qYFnh57g7xw24+osaCSMB08tqSPAUzOz6K942qJT4ZR/RjGEefb4t1FszMpAKjytRGqIAy40dzNdkYwSwMQ==", + "version": "3.0.559", + "resolved": "https://registry.npmjs.org/@defra/forms-model/-/forms-model-3.0.559.tgz", + "integrity": "sha512-dSMrTnhUXnapflHKdeQLMGDwK2QlFhp/08XwzLNHzLHmgx7pqHAgelzVeRsyHtzYDu7B7tF4r5cyR+SxI4UmXw==", "license": "OGL-UK-3.0", "dependencies": { "@joi/date": "^2.1.1", diff --git a/package.json b/package.json index 994398313..18a94e715 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,7 @@ }, "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@defra/forms-model": "^3.0.558", + "@defra/forms-model": "^3.0.559", "@defra/hapi-tracing": "^1.26.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 4f32af0a3..6e6a7dd0b 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -148,9 +148,12 @@ export class SummaryPageController extends QuestionPageController { request, viewModel, model, - submissionEmailAddress, - formMetadata, - viewModel.userConfirmationEmailField?.value as string + { + submissionEmailAddress, + userConfirmationEmailAddress: viewModel.userConfirmationEmailField + ?.value as string + }, + formMetadata ) } @@ -180,11 +183,17 @@ export async function submitForm( request: FormRequestPayload, summaryViewModel: SummaryViewModel, model: FormModel, - submissionEmailAddress: string, - formMetadata: FormMetadata, - userConfirmationEmailAddress?: string + options: { + submissionEmailAddress: string + userConfirmationEmailAddress?: string + }, + formMetadata: FormMetadata ) { - await extendFileRetention(model, context.state, submissionEmailAddress) + await extendFileRetention( + model, + context.state, + options.submissionEmailAddress + ) const formStatus = checkFormStatus(request.params) const logTags = ['submit', 'submissionApi'] @@ -202,7 +211,7 @@ export async function submitForm( const submitResponse = await submitData( model, items, - submissionEmailAddress, + options.submissionEmailAddress, request.yar.id ) @@ -214,10 +223,7 @@ export async function submitForm( context, request, model, - { - submissionEmailAddress, - userConfirmationEmailAddress - }, + options, items, submitResponse, formMetadata From d967450612eb9f6dc58ac7e60d1a70f0546c90d7 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 7 Oct 2025 15:54:54 +0100 Subject: [PATCH 15/25] Passes confirmation email address --- src/server/plugins/engine/types.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/plugins/engine/types.ts b/src/server/plugins/engine/types.ts index 993fa148e..e72942255 100644 --- a/src/server/plugins/engine/types.ts +++ b/src/server/plugins/engine/types.ts @@ -408,6 +408,7 @@ export interface FormAdapterSubmissionMessageMeta { status: FormStatus isPreview: boolean notificationEmail: string + userConfirmationEmail?: string versionMetadata?: FormVersionMetadata } From a05a90de7c04b2c0257da89bcaef907cc808185b Mon Sep 17 00:00:00 2001 From: Alex Luckett Date: Wed, 8 Oct 2025 12:11:12 +0100 Subject: [PATCH 16/25] Remove confirmation email from plugin, now delegated to Runner --- .../pageControllers/SummaryPageController.ts | 21 +++++-------------- .../plugins/engine/services/notifyService.ts | 6 +----- src/server/plugins/engine/types.ts | 2 +- src/server/schemas/index.ts | 1 - src/server/types.ts | 5 +---- 5 files changed, 8 insertions(+), 27 deletions(-) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 6e6a7dd0b..e9e128c79 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -148,11 +148,7 @@ export class SummaryPageController extends QuestionPageController { request, viewModel, model, - { - submissionEmailAddress, - userConfirmationEmailAddress: viewModel.userConfirmationEmailField - ?.value as string - }, + submissionEmailAddress, formMetadata ) } @@ -183,17 +179,10 @@ export async function submitForm( request: FormRequestPayload, summaryViewModel: SummaryViewModel, model: FormModel, - options: { - submissionEmailAddress: string - userConfirmationEmailAddress?: string - }, + submissionEmailAddress: string, formMetadata: FormMetadata ) { - await extendFileRetention( - model, - context.state, - options.submissionEmailAddress - ) + await extendFileRetention(model, context.state, submissionEmailAddress) const formStatus = checkFormStatus(request.params) const logTags = ['submit', 'submissionApi'] @@ -211,7 +200,7 @@ export async function submitForm( const submitResponse = await submitData( model, items, - options.submissionEmailAddress, + submissionEmailAddress, request.yar.id ) @@ -223,7 +212,7 @@ export async function submitForm( context, request, model, - options, + submissionEmailAddress, items, submitResponse, formMetadata diff --git a/src/server/plugins/engine/services/notifyService.ts b/src/server/plugins/engine/services/notifyService.ts index 193c62243..e6bd3ee0a 100644 --- a/src/server/plugins/engine/services/notifyService.ts +++ b/src/server/plugins/engine/services/notifyService.ts @@ -25,10 +25,7 @@ export async function submit( context: FormContext, request: FormRequestPayload, model: FormModel, - options: { - submissionEmailAddress: string - userConfirmationEmailAddress?: string - }, + emailAddress: string, items: DetailItem[], submitResponse: SubmitResponsePayload, formMetadata?: FormMetadata @@ -43,7 +40,6 @@ export async function submit( // Get submission email personalisation request.logger.info(logTags, 'Getting personalisation data') - const emailAddress = options.submissionEmailAddress const formName = escapeMarkdown(model.name) const subject = formStatus.isPreview ? `TEST FORM SUBMISSION: ${formName}` diff --git a/src/server/plugins/engine/types.ts b/src/server/plugins/engine/types.ts index e72942255..3af534cf2 100644 --- a/src/server/plugins/engine/types.ts +++ b/src/server/plugins/engine/types.ts @@ -408,8 +408,8 @@ export interface FormAdapterSubmissionMessageMeta { status: FormStatus isPreview: boolean notificationEmail: string - userConfirmationEmail?: string versionMetadata?: FormVersionMetadata + [key: string]: unknown } export type FormAdapterSubmissionMessageMetaSerialised = Omit< diff --git a/src/server/schemas/index.ts b/src/server/schemas/index.ts index 630a0c70d..f181e7f01 100644 --- a/src/server/schemas/index.ts +++ b/src/server/schemas/index.ts @@ -23,7 +23,6 @@ export const pathSchema = Joi.string().required() export const itemIdSchema = Joi.string().uuid().required() export const crumbSchema = Joi.string().optional().allow('') export const confirmSchema = Joi.boolean().empty(false) -export const userConfirmationEmailSchema = Joi.string().trim().email().allow('') export const paramsSchema = Joi.object() .keys({ diff --git a/src/server/types.ts b/src/server/types.ts index 61a4e0c58..a21c53def 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -60,10 +60,7 @@ export interface OutputService { context: FormContext, request: FormRequestPayload, model: FormModel, - options: { - submissionEmailAddress: string - userConfirmationEmailAddress?: string - }, + submissionEmailAddress: string, items: DetailItem[], submitResponse: SubmitResponsePayload, formMetadata?: FormMetadata From bc8f22c8c5532a430d398474f59839c3c349116f Mon Sep 17 00:00:00 2001 From: Alex Luckett Date: Wed, 8 Oct 2025 12:26:42 +0100 Subject: [PATCH 17/25] Don't skip summary submit --- src/server/plugins/engine/models/FormModel.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/server/plugins/engine/models/FormModel.ts b/src/server/plugins/engine/models/FormModel.ts index e5b3253ff..e6fa7506b 100644 --- a/src/server/plugins/engine/models/FormModel.ts +++ b/src/server/plugins/engine/models/FormModel.ts @@ -551,10 +551,7 @@ function validateFormPayload( // Skip validation GET requests or other actions if ( !request.payload || - (action && - ![FormAction.Validate, FormAction.Send, FormAction.SaveAndExit].includes( - action - )) + (action && ![FormAction.Validate, FormAction.SaveAndExit].includes(action)) ) { return context } From e0bd9c8af2a3c5560a3875c1abb7f0d71ba6a427 Mon Sep 17 00:00:00 2001 From: Alex Luckett Date: Wed, 8 Oct 2025 12:36:40 +0100 Subject: [PATCH 18/25] Support custom page content on summary pages --- src/server/plugins/engine/views/summary.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/server/plugins/engine/views/summary.html b/src/server/plugins/engine/views/summary.html index 2716d7197..1ce7a5e83 100644 --- a/src/server/plugins/engine/views/summary.html +++ b/src/server/plugins/engine/views/summary.html @@ -43,9 +43,7 @@

{{ componentList(components) }} - {% if userConfirmationEmailField %} - {{ govukInput(userConfirmationEmailField) }} - {% endif %} + {% block customPageContent %}{% endblock %} {% if declaration %}

Declaration

From 6ecde8d85566ec2fc9cc7d7f5b36cd283c02b11c Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Wed, 8 Oct 2025 12:43:56 +0100 Subject: [PATCH 19/25] Removed confirmation email to prevent bleed from runner --- src/server/plugins/engine/models/SummaryViewModel.ts | 3 +-- src/server/plugins/engine/types/schema.ts | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/server/plugins/engine/models/SummaryViewModel.ts b/src/server/plugins/engine/models/SummaryViewModel.ts index 23ff64a9b..26308ab4f 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.ts @@ -1,4 +1,4 @@ -import { type GovukField, type Section } from '@defra/forms-model' +import { type Section } from '@defra/forms-model' import { getAnswer, @@ -52,7 +52,6 @@ export class SummaryViewModel { hasMissingNotificationEmail?: boolean components?: ComponentViewModel[] allowSaveAndExit = false - userConfirmationEmailField?: GovukField constructor( request: FormContextRequest, diff --git a/src/server/plugins/engine/types/schema.ts b/src/server/plugins/engine/types/schema.ts index 396d965cb..f8801b663 100644 --- a/src/server/plugins/engine/types/schema.ts +++ b/src/server/plugins/engine/types/schema.ts @@ -4,7 +4,8 @@ import { idSchema, notificationEmailAddressSchema, slugSchema, - titleSchema + titleSchema, + userConfirmationEmailAddressSchema } from '@defra/forms-model' import Joi from 'joi' @@ -31,6 +32,7 @@ export const formAdapterSubmissionMessageMetaSchema = .required(), isPreview: Joi.boolean().required(), notificationEmail: notificationEmailAddressSchema.required(), + userConfirmationEmail: userConfirmationEmailAddressSchema.optional(), versionMetadata: formVersionMetadataSchema.optional() }) From 5c01fa03b2033cff0e4492bdf7a9850872cd8e62 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Wed, 8 Oct 2025 13:01:37 +0100 Subject: [PATCH 20/25] Reverted variable name --- .../pageControllers/SummaryPageController.ts | 17 ++++++++--------- .../engine/services/notifyService.test.ts | 12 ++++++------ src/server/types.ts | 2 +- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index e9e128c79..aac9d3b55 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -135,20 +135,19 @@ export class SummaryPageController extends QuestionPageController { const formMetadata = await getFormMetadata(params.slug) const { notificationEmail } = formMetadata const { isPreview } = checkFormStatus(request.params) - const submissionEmailAddress = - notificationEmail ?? this.model.def.outputEmail + const emailAddress = notificationEmail ?? this.model.def.outputEmail - checkEmailAddressForLiveFormSubmission(submissionEmailAddress, isPreview) + checkEmailAddressForLiveFormSubmission(emailAddress, isPreview) // Send submission email - if (submissionEmailAddress) { + if (emailAddress) { const viewModel = this.getSummaryViewModel(request, context) await submitForm( context, request, viewModel, model, - submissionEmailAddress, + emailAddress, formMetadata ) } @@ -179,10 +178,10 @@ export async function submitForm( request: FormRequestPayload, summaryViewModel: SummaryViewModel, model: FormModel, - submissionEmailAddress: string, + emailAddress: string, formMetadata: FormMetadata ) { - await extendFileRetention(model, context.state, submissionEmailAddress) + await extendFileRetention(model, context.state, emailAddress) const formStatus = checkFormStatus(request.params) const logTags = ['submit', 'submissionApi'] @@ -200,7 +199,7 @@ export async function submitForm( const submitResponse = await submitData( model, items, - submissionEmailAddress, + emailAddress, request.yar.id ) @@ -212,7 +211,7 @@ export async function submitForm( context, request, model, - submissionEmailAddress, + emailAddress, items, submitResponse, formMetadata diff --git a/src/server/plugins/engine/services/notifyService.test.ts b/src/server/plugins/engine/services/notifyService.test.ts index fe283d0df..1a9caf79f 100644 --- a/src/server/plugins/engine/services/notifyService.test.ts +++ b/src/server/plugins/engine/services/notifyService.test.ts @@ -70,7 +70,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - { submissionEmailAddress: 'test@defra.gov.uk' }, + 'test@defra.gov.uk', items, submitResponse ) @@ -106,7 +106,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - { submissionEmailAddress: 'test@defra.gov.uk' }, + 'test@defra.gov.uk', items, submitResponse ) @@ -144,7 +144,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - { submissionEmailAddress: 'test@defra.gov.uk' }, + 'test@defra.gov.uk', items, submitResponse ) @@ -190,7 +190,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - { submissionEmailAddress: 'test@defra.gov.uk' }, + 'test@defra.gov.uk', items, submitResponse, formMetadata @@ -242,7 +242,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - { submissionEmailAddress: 'test@defra.gov.uk' }, + 'test@defra.gov.uk', items, submitResponse ) @@ -294,7 +294,7 @@ describe('notifyService', () => { formContext, mockRequest, model, - { submissionEmailAddress: 'test@defra.gov.uk' }, + 'test@defra.gov.uk', items, submitResponse ) diff --git a/src/server/types.ts b/src/server/types.ts index a21c53def..b5179ae00 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -60,7 +60,7 @@ export interface OutputService { context: FormContext, request: FormRequestPayload, model: FormModel, - submissionEmailAddress: string, + emailAddress: string, items: DetailItem[], submitResponse: SubmitResponsePayload, formMetadata?: FormMetadata From a7b1ed30c7bc8f10cfd781f65535d4881a671201 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Wed, 8 Oct 2025 14:04:25 +0100 Subject: [PATCH 21/25] Tidy up --- src/server/plugins/engine/types/schema.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/server/plugins/engine/types/schema.ts b/src/server/plugins/engine/types/schema.ts index f8801b663..396d965cb 100644 --- a/src/server/plugins/engine/types/schema.ts +++ b/src/server/plugins/engine/types/schema.ts @@ -4,8 +4,7 @@ import { idSchema, notificationEmailAddressSchema, slugSchema, - titleSchema, - userConfirmationEmailAddressSchema + titleSchema } from '@defra/forms-model' import Joi from 'joi' @@ -32,7 +31,6 @@ export const formAdapterSubmissionMessageMetaSchema = .required(), isPreview: Joi.boolean().required(), notificationEmail: notificationEmailAddressSchema.required(), - userConfirmationEmail: userConfirmationEmailAddressSchema.optional(), versionMetadata: formVersionMetadataSchema.optional() }) From 8846475b5b8fa133565fe4d4e9ae3bc58a1c0395 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Wed, 8 Oct 2025 14:18:19 +0100 Subject: [PATCH 22/25] Allow custom key/values in meta --- src/server/plugins/engine/types/schema.ts | 34 ++++++++++++----------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/server/plugins/engine/types/schema.ts b/src/server/plugins/engine/types/schema.ts index 396d965cb..2582d10ef 100644 --- a/src/server/plugins/engine/types/schema.ts +++ b/src/server/plugins/engine/types/schema.ts @@ -17,22 +17,24 @@ import { } from '~/src/server/plugins/engine/types.js' export const formAdapterSubmissionMessageMetaSchema = - Joi.object().keys({ - schemaVersion: Joi.string().valid( - ...Object.values(FormAdapterSubmissionSchemaVersion) - ), - timestamp: Joi.date().required(), - referenceNumber: Joi.string().required(), - formName: titleSchema, - formId: idSchema, - formSlug: slugSchema, - status: Joi.string() - .valid(...Object.values(FormStatus)) - .required(), - isPreview: Joi.boolean().required(), - notificationEmail: notificationEmailAddressSchema.required(), - versionMetadata: formVersionMetadataSchema.optional() - }) + Joi.object() + .keys({ + schemaVersion: Joi.string().valid( + ...Object.values(FormAdapterSubmissionSchemaVersion) + ), + timestamp: Joi.date().required(), + referenceNumber: Joi.string().required(), + formName: titleSchema, + formId: idSchema, + formSlug: slugSchema, + status: Joi.string() + .valid(...Object.values(FormStatus)) + .required(), + isPreview: Joi.boolean().required(), + notificationEmail: notificationEmailAddressSchema.required(), + versionMetadata: formVersionMetadataSchema.optional() + }) + .unknown(true) // Allow custom key/values export const formAdapterSubmissionMessageDataSchema = Joi.object().keys({ From a4959fcd143bfae472b4596497a5a2e3fcf59ca9 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Wed, 8 Oct 2025 17:29:22 +0100 Subject: [PATCH 23/25] Changed to use custom property in meta --- src/server/plugins/engine/types.ts | 2 +- src/server/plugins/engine/types/schema.ts | 38 ++++++++++++----------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/server/plugins/engine/types.ts b/src/server/plugins/engine/types.ts index 3af534cf2..1ff21d013 100644 --- a/src/server/plugins/engine/types.ts +++ b/src/server/plugins/engine/types.ts @@ -409,7 +409,7 @@ export interface FormAdapterSubmissionMessageMeta { isPreview: boolean notificationEmail: string versionMetadata?: FormVersionMetadata - [key: string]: unknown + custom?: Record } export type FormAdapterSubmissionMessageMetaSerialised = Omit< diff --git a/src/server/plugins/engine/types/schema.ts b/src/server/plugins/engine/types/schema.ts index 2582d10ef..c40a43e53 100644 --- a/src/server/plugins/engine/types/schema.ts +++ b/src/server/plugins/engine/types/schema.ts @@ -17,24 +17,26 @@ import { } from '~/src/server/plugins/engine/types.js' export const formAdapterSubmissionMessageMetaSchema = - Joi.object() - .keys({ - schemaVersion: Joi.string().valid( - ...Object.values(FormAdapterSubmissionSchemaVersion) - ), - timestamp: Joi.date().required(), - referenceNumber: Joi.string().required(), - formName: titleSchema, - formId: idSchema, - formSlug: slugSchema, - status: Joi.string() - .valid(...Object.values(FormStatus)) - .required(), - isPreview: Joi.boolean().required(), - notificationEmail: notificationEmailAddressSchema.required(), - versionMetadata: formVersionMetadataSchema.optional() - }) - .unknown(true) // Allow custom key/values + Joi.object().keys({ + schemaVersion: Joi.string().valid( + ...Object.values(FormAdapterSubmissionSchemaVersion) + ), + timestamp: Joi.date().required(), + referenceNumber: Joi.string().required(), + formName: titleSchema, + formId: idSchema, + formSlug: slugSchema, + status: Joi.string() + .valid(...Object.values(FormStatus)) + .required(), + isPreview: Joi.boolean().required(), + notificationEmail: notificationEmailAddressSchema.required(), + versionMetadata: formVersionMetadataSchema.optional(), + custom: Joi.object({ a: Joi.any() }) + .unknown() + .optional() + .description('Custom properties for the message') + }) export const formAdapterSubmissionMessageDataSchema = Joi.object().keys({ From d7bb1841499d083d38f6fcd970150d0ee5b37432 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Thu, 9 Oct 2025 08:24:32 +0100 Subject: [PATCH 24/25] Corrected custom property schema --- .../plugins/engine/types/schema.test.ts | 34 +++++++++++++++++++ src/server/plugins/engine/types/schema.ts | 3 +- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/server/plugins/engine/types/schema.test.ts b/src/server/plugins/engine/types/schema.test.ts index b07a48392..a77c9257a 100644 --- a/src/server/plugins/engine/types/schema.test.ts +++ b/src/server/plugins/engine/types/schema.test.ts @@ -78,6 +78,29 @@ describe('Schema validation', () => { expect(error).toBeUndefined() }) + it('should validate valid meta object with valid custom properties', () => { + const validMetaWithCustom = { + ...validMeta, + custom: { + property1: 'value 1', + property2: 'value2' + } + } + const { error } = + formAdapterSubmissionMessageMetaSchema.validate(validMetaWithCustom) + expect(error).toBeUndefined() + }) + + it('should validate valid meta object with empty custom properties', () => { + const validMetaWithCustom = { + ...validMeta, + custom: {} + } + const { error } = + formAdapterSubmissionMessageMetaSchema.validate(validMetaWithCustom) + expect(error).toBeUndefined() + }) + it('should reject invalid schema version', () => { const invalidMeta = { ...validMeta, schemaVersion: 'invalid' } const { error } = @@ -92,6 +115,17 @@ describe('Schema validation', () => { formAdapterSubmissionMessageMetaSchema.validate(metaWithoutTimestamp) expect(error).toBeDefined() }) + + it('should reject invalid custom structure', () => { + const validMetaWithInvalidCustom = { + ...validMeta, + custom: 'invalid' + } + const { error } = formAdapterSubmissionMessageMetaSchema.validate( + validMetaWithInvalidCustom + ) + expect(error).toBeDefined() + }) }) describe('formAdapterSubmissionMessageDataSchema', () => { diff --git a/src/server/plugins/engine/types/schema.ts b/src/server/plugins/engine/types/schema.ts index c40a43e53..203c4f1ec 100644 --- a/src/server/plugins/engine/types/schema.ts +++ b/src/server/plugins/engine/types/schema.ts @@ -32,7 +32,8 @@ export const formAdapterSubmissionMessageMetaSchema = isPreview: Joi.boolean().required(), notificationEmail: notificationEmailAddressSchema.required(), versionMetadata: formVersionMetadataSchema.optional(), - custom: Joi.object({ a: Joi.any() }) + custom: Joi.object() + .pattern(/^/, Joi.any()) .unknown() .optional() .description('Custom properties for the message') From ade367c9e7e2df3a5be33d9fc69acfcdd32aaf77 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Thu, 9 Oct 2025 09:50:17 +0100 Subject: [PATCH 25/25] chore(release): #minor