diff --git a/src/server/index.test.ts b/src/server/index.test.ts index 919d7a5d9..2fee15ca8 100644 --- a/src/server/index.test.ts +++ b/src/server/index.test.ts @@ -259,7 +259,9 @@ describe('Model cache', () => { // Assert the live/live cache item has the correct updatedAt timestamp expect( - getCacheItem(`${fixtures.form.metadata.id}_live_false`)?.updatedAt + getCacheItem( + `${fixtures.form.metadata.id}_live_false_${fixtures.form.metadata.notificationEmail}` + )?.updatedAt ).toBe(now2) // Expect the cache size to remain unchanged diff --git a/src/server/plugins/engine/beta/form-context.test.ts b/src/server/plugins/engine/beta/form-context.test.ts index aed997266..8196a78aa 100644 --- a/src/server/plugins/engine/beta/form-context.test.ts +++ b/src/server/plugins/engine/beta/form-context.test.ts @@ -292,7 +292,8 @@ describe('resolveFormModel helper', () => { expect(mockFormsService.getFormDefinition).toHaveBeenCalledTimes(2) expect(mockCheckEmailAddressForLiveFormSubmission).toHaveBeenCalledWith( undefined, - true + true, + false ) expect(FormModel).toHaveBeenCalledWith( definition, diff --git a/src/server/plugins/engine/beta/form-context.ts b/src/server/plugins/engine/beta/form-context.ts index 88b06839d..089852f2e 100644 --- a/src/server/plugins/engine/beta/form-context.ts +++ b/src/server/plugins/engine/beta/form-context.ts @@ -1,3 +1,4 @@ +import { hasPaymentQuestionInForm } from '@defra/forms-model' import Boom from '@hapi/boom' import { type Request, type Server } from '@hapi/hapi' import { isEqual } from 'date-fns' @@ -73,7 +74,8 @@ export async function getFormModel( options.basePath ?? buildBasePath(options.routePrefix ?? '', slug, formState, isPreview), ordnanceSurveyApiKey: options.ordnanceSurveyApiKey, - formId: options.formId ?? metadata.id + formId: options.formId ?? metadata.id, + notificationEmail: metadata.notificationEmail }, services, options.controllers @@ -153,7 +155,7 @@ export async function resolveFormModel( const cache = server.app.models - const cacheKey = `${metadata.id}_${formState}_${isPreview}` + const cacheKey = `${metadata.id}_${formState}_${isPreview}_${metadata.notificationEmail}` let entry = cache.get(cacheKey) if (!entry || !isEqual(entry.updatedAt, stateMetadata.updatedAt)) { @@ -170,7 +172,8 @@ export async function resolveFormModel( checkEmailAddressForLiveFormSubmission( metadata.notificationEmail, - isPreview + isPreview, + hasPaymentQuestionInForm(definition) ) const routePrefix = @@ -183,7 +186,8 @@ export async function resolveFormModel( options.basePath ?? buildBasePath(routePrefix, slug, formState, isPreview), ordnanceSurveyApiKey: options.ordnanceSurveyApiKey, - formId: options.formId ?? metadata.id + formId: options.formId ?? metadata.id, + notificationEmail: metadata.notificationEmail }, services, options.controllers diff --git a/src/server/plugins/engine/helpers.test.ts b/src/server/plugins/engine/helpers.test.ts index b75d0f537..4487d62f0 100644 --- a/src/server/plugins/engine/helpers.test.ts +++ b/src/server/plugins/engine/helpers.test.ts @@ -401,7 +401,7 @@ describe('Helpers', () => { describe('checkEmailAddressForLiveFormSubmission', () => { it('should throw an error if emailAddress is undefined and isPreview is false', () => { expect(() => - checkEmailAddressForLiveFormSubmission(undefined, false) + checkEmailAddressForLiveFormSubmission(undefined, false, false) ).toThrow( Boom.internal( 'An email address is required to complete the form submission' @@ -411,19 +411,25 @@ describe('Helpers', () => { it('should not throw an error if emailAddress is defined and isPreview is false', () => { expect(() => - checkEmailAddressForLiveFormSubmission('test@example.com', false) + checkEmailAddressForLiveFormSubmission('test@example.com', false, false) ).not.toThrow() }) it('should not throw an error if emailAddress is undefined and isPreview is true', () => { expect(() => - checkEmailAddressForLiveFormSubmission(undefined, true) + checkEmailAddressForLiveFormSubmission(undefined, true, false) ).not.toThrow() }) it('should not throw an error if emailAddress is defined and isPreview is true', () => { expect(() => - checkEmailAddressForLiveFormSubmission('test@example.com', true) + checkEmailAddressForLiveFormSubmission('test@example.com', true, false) + ).not.toThrow() + }) + + it('should not throw when emailAddress is undefined and the form has a PaymentField', () => { + expect(() => + checkEmailAddressForLiveFormSubmission(undefined, false, true) ).not.toThrow() }) }) diff --git a/src/server/plugins/engine/helpers.ts b/src/server/plugins/engine/helpers.ts index 3be9f98bd..bdcf683b4 100644 --- a/src/server/plugins/engine/helpers.ts +++ b/src/server/plugins/engine/helpers.ts @@ -282,9 +282,11 @@ export function checkFormStatus(params?: FormParams) { export function checkEmailAddressForLiveFormSubmission( emailAddress: string | undefined, - isPreview: boolean + isPreview: boolean, + hasPayment: boolean ) { - if (!emailAddress && !isPreview) { + // Payment-only forms submit via GOV.UK Pay without a notification email. + if (!emailAddress && !isPreview && !hasPayment) { throw Boom.internal( 'An email address is required to complete the form submission' ) diff --git a/src/server/plugins/engine/models/FormModel.ts b/src/server/plugins/engine/models/FormModel.ts index 30f6c2463..b5890c44e 100644 --- a/src/server/plugins/engine/models/FormModel.ts +++ b/src/server/plugins/engine/models/FormModel.ts @@ -92,6 +92,7 @@ export class FormModel { pageMap: Map componentMap: Map + notificationEmail?: string constructor( def: typeof this.def, @@ -99,6 +100,7 @@ export class FormModel { basePath: string ordnanceSurveyApiKey?: string formId?: string + notificationEmail?: string }, services: Services = defaultServices, controllers?: Record @@ -155,6 +157,7 @@ export class FormModel { this.values = result.value this.basePath = options.basePath this.ordnanceSurveyApiKey = options.ordnanceSurveyApiKey + this.notificationEmail = options.notificationEmail this.conditions = {} this.services = services this.controllers = controllers @@ -357,7 +360,8 @@ export class FormModel { componentDefMap: this.componentDefMap, pageMap: this.pageMap, componentMap: this.componentMap, - referenceNumber: getReferenceNumber(state) + referenceNumber: getReferenceNumber(state), + notificationEmail: this.notificationEmail } // Validate current page diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts index 4c704d111..b41cf5a57 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts @@ -1,3 +1,4 @@ +import { PaymentField } from '~/src/server/plugins/engine/components/PaymentField.js' import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { SummaryPageController, @@ -429,4 +430,114 @@ describe('SummaryPageController - Payment (DF-832)', () => { ).rejects.toBe(err) }) }) + + describe('handleFormSubmit - notification email gate', () => { + function buildHandleFormSubmitHarness({ + notificationEmail + }: { + notificationEmail?: string + }) { + const state = { + $$__referenceNumber: 'ref', + yesNoField: false + } as unknown as FormSubmissionState + + const cacheService = { + setConfirmationState: jest.fn().mockResolvedValue(undefined), + clearState: jest.fn().mockResolvedValue(undefined), + resetComponentStates: jest.fn().mockResolvedValue(undefined) + } as unknown as CacheService + + const getFormMetadata = jest.fn().mockResolvedValue({ + notificationEmail + }) + + model.services = { + ...model.services, + formsService: { ...model.services.formsService, getFormMetadata } + } as typeof model.services + + const request = { + ...requestPage, + method: 'post', + params: { ...requestPage.params, path: 'summary' }, + server: { + plugins: { 'forms-engine-plugin': { cacheService } } + }, + yar: { id: 'session-id' }, + logger: { info: jest.fn(), error: jest.fn() } + } as unknown as FormRequestPayload + + const context = model.getFormContext(request, state) + jest + .spyOn(controller, 'proceed') + .mockReturnValue( + undefined as unknown as ReturnType + ) + + return { request, context, cacheService } + } + + it('runs PaymentField onSubmit when notificationEmail is empty', async () => { + const onSubmitSpy = jest + .spyOn(PaymentField.prototype, 'onSubmit') + .mockResolvedValue(undefined) + + const { request, context } = buildHandleFormSubmitHarness({ + notificationEmail: '' + }) + + await controller.handleFormSubmit(request, context, h) + + expect(onSubmitSpy).toHaveBeenCalled() + }) + + it('routes through submitForm when notificationEmail is set', async () => { + jest + .spyOn(PaymentField.prototype, 'onSubmit') + .mockResolvedValue(undefined) + + model.services = { + ...model.services, + formSubmissionService: { + ...model.services.formSubmissionService, + submit: jest.fn().mockResolvedValue({ data: { reference: 'r' } }) + }, + outputService: { + ...model.services.outputService, + submit: jest.fn().mockResolvedValue(undefined) + } + } as typeof model.services + + const { request, context } = buildHandleFormSubmitHarness({ + notificationEmail: 'notify@example.com' + }) + + await controller.handleFormSubmit(request, context, h) + + expect(model.services.outputService.submit).toHaveBeenCalled() + }) + + it('routes errors through handleSubmissionError when finalisePaymentFields throws', async () => { + const err = new Error('pay boom') + jest.spyOn(PaymentField.prototype, 'onSubmit').mockRejectedValue(err) + + const { request, context } = buildHandleFormSubmitHarness({ + notificationEmail: '' + }) + + const handleSubmissionErrorSpy = jest + .spyOn( + controller as unknown as { + handleSubmissionError: (...args: unknown[]) => unknown + }, + 'handleSubmissionError' + ) + .mockReturnValue(undefined) + + await controller.handleFormSubmit(request, context, h) + + expect(handleSubmissionErrorSpy).toHaveBeenCalledWith(err, request, h) + }) + }) }) diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 2863e51ba..d97ffb7ae 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -267,12 +267,19 @@ export class SummaryPageController extends QuestionPageController { const { notificationEmail } = formMetadata const { isPreview } = checkFormStatus(request.params) - checkEmailAddressForLiveFormSubmission(notificationEmail, isPreview) + const hasPayment = this.findPaymentField() !== undefined + checkEmailAddressForLiveFormSubmission( + notificationEmail, + isPreview, + hasPayment + ) - if (notificationEmail) { - const viewModel = this.getSummaryViewModel(request, context) + const viewModel = notificationEmail + ? this.getSummaryViewModel(request, context) + : undefined - try { + try { + if (notificationEmail && viewModel) { await submitForm( context, formMetadata, @@ -281,9 +288,12 @@ export class SummaryPageController extends QuestionPageController { model, notificationEmail ) - } catch (error) { - return this.handleSubmissionError(error, request, h) + } else { + // Payment-only submission path — runs PaymentField capture hooks only. + await finalisePaymentFields(request, formMetadata, context, model) } + } catch (error) { + return this.handleSubmissionError(error, request, h) } await cacheService.setConfirmationState(request, { @@ -516,6 +526,24 @@ async function finaliseComponents( } } +/** + * Runs PaymentField capture hooks for the no-email submission path. + */ +async function finalisePaymentFields( + request: FormRequestPayload, + metadata: FormMetadata, + context: FormContext, + model: FormModel +) { + for (const page of model.pages) { + for (const field of page.collection.fields) { + if (field instanceof PaymentField) { + await field.onSubmit(request, metadata, context) + } + } + } +} + function submitData( model: FormModel, items: DetailItem[], diff --git a/src/server/plugins/engine/types.ts b/src/server/plugins/engine/types.ts index 9d9fecd75..24e8bd7c3 100644 --- a/src/server/plugins/engine/types.ts +++ b/src/server/plugins/engine/types.ts @@ -199,6 +199,7 @@ export interface FormContext { pageMap: Map componentMap: Map referenceNumber: string + notificationEmail?: string } export type FormContextRequest = ( diff --git a/src/server/plugins/engine/views/partials/preview-banner.html b/src/server/plugins/engine/views/partials/preview-banner.html index 66dfb75e6..bddfd3f19 100644 --- a/src/server/plugins/engine/views/partials/preview-banner.html +++ b/src/server/plugins/engine/views/partials/preview-banner.html @@ -11,10 +11,22 @@ {% call govukNotificationBanner() %} {% if not context.isForceAccess %}

- This is a preview of a {{ previewMode }} form. Do not enter personal information. + This is a preview of a {{ previewMode }} form. Do not enter personal information.

{{ _closeLink() }} + + {% if not context.notificationEmail %} +

+ You can complete this form to test the user journey. The form will appear to submit successfully, but because a destination email address has not been set, the following features will not trigger: +

+
    +
  • email delivery
    • +
  • file uploads
    • +
  • payments
    • +
  • SQS deliveries
    • +
+ {% endif %} {% else %}

This is a preview of a {{ previewMode }} form page you are editing. diff --git a/src/server/plugins/engine/views/partials/preview-banner.test.js b/src/server/plugins/engine/views/partials/preview-banner.test.js index 6bb6153bb..98cc0c5b0 100644 --- a/src/server/plugins/engine/views/partials/preview-banner.test.js +++ b/src/server/plugins/engine/views/partials/preview-banner.test.js @@ -13,7 +13,7 @@ describe('Preview banner partial', () => { let $component = /** @type {HTMLElement | null} */ (null) let $paragraphs = /** @type {HTMLElement[]} */ ([]) - describe('Form preview', () => { + describe('Form preview (no notification email)', () => { beforeEach(() => { const { container } = renderView('partials/preview-banner.html', { context: { @@ -29,6 +29,48 @@ describe('Preview banner partial', () => { $paragraphs = container.getAllByRole('paragraph') }) + it('should render contents', () => { + expect($component).toBeInTheDocument() + expect($component).toContainElement($paragraphs[0]) + expect($component).toHaveClass('govuk-notification-banner') + + expect($paragraphs).toHaveLength(2) + expect($paragraphs[0]).toHaveClass('govuk-notification-banner__heading') + expect($paragraphs[1]).toHaveClass('govuk-body') + }) + + it('should have accessible name', () => { + expect($component).toHaveAccessibleName('Important') + }) + + it('should have text content', () => { + expect($paragraphs[0]).toHaveTextContent( + `This is a preview of a ${status} form. Do not enter personal information.` + ) + + expect($paragraphs[1]).toHaveTextContent( + 'You can complete this form to test the user journey. The form will appear to submit successfully, but because a destination email address has not been set, the following features will not trigger:' + ) + }) + }) + + describe('Form preview (with notification email)', () => { + beforeEach(() => { + const { container } = renderView('partials/preview-banner.html', { + context: { + previewMode: status, + context: { + isForceAccess: false, + notificationEmail: 'test@example.com', + relevantPages: [{ title: 'Page 1' }] + } + } + }) + + $component = container.getByRole('region') + $paragraphs = container.getAllByRole('paragraph') + }) + it('should render contents', () => { expect($component).toBeInTheDocument() expect($component).toContainElement($paragraphs[0])