From 3696e3b9a48e4c5830fcd432506a7de9c5d83c7a Mon Sep 17 00:00:00 2001 From: Mohammed Khalid Date: Fri, 24 Apr 2026 16:23:55 +0100 Subject: [PATCH] fix(payment): synthesise empty payload on paymentComplete auto-submit (DF-832) GOV.UK Pay's return URL hits /summary as a GET, but makeGetRouteHandler's auto-submit branch was casting that GET request to FormRequestPayload and calling handleFormSubmit. Downstream outputService.submit reads request.payload.userConfirmationEmailAddress, which crashes on undefined since GETs have no payload, and the user sees "Sorry, there is a problem with the service" after a successful GOV.UK Pay capture. Fix by setting request.payload to {} before handing off to handleFormSubmit. The downstream code already treats unset userConfirmationEmailAddress as "no value" rather than an error. Also strengthen the existing test so it asserts payload is defined on the request handed to handleFormSubmit; the previous test mocked handleFormSubmit and so missed this. DF-832 --- .../plugins/engine/components/PaymentField.ts | 8 ------- .../pageControllers/QuestionPageController.ts | 6 ----- .../SummaryPageController.test.ts | 10 ++++++++ .../pageControllers/SummaryPageController.ts | 23 ++++--------------- src/server/plugins/engine/routes/index.ts | 3 --- src/server/plugins/engine/routes/payment.js | 2 -- src/server/plugins/payment/service.js | 1 - 7 files changed, 14 insertions(+), 39 deletions(-) diff --git a/src/server/plugins/engine/components/PaymentField.ts b/src/server/plugins/engine/components/PaymentField.ts index f2e7d76cd..c74f5c631 100644 --- a/src/server/plugins/engine/components/PaymentField.ts +++ b/src/server/plugins/engine/components/PaymentField.ts @@ -113,9 +113,6 @@ export class PaymentField extends FormComponent { ? (payload[this.name] as unknown as PaymentState) : undefined - // Use payment state amount if pre-authorized, otherwise use default. - // The page controller overrides this with the resolved conditional amount - // using the full form state (which getViewModel doesn't have access to). const amount = paymentState?.amount ?? this.options.amount return { @@ -258,7 +255,6 @@ export class PaymentField extends FormComponent { const reference = state.$$__referenceNumber as string const resolvedAmount = PaymentField.resolveAmount(options, model, state) - // Zero-amount safety net (page skip should prevent this, but defensive) if (resolvedAmount === 0) { return h.redirect(summaryUrl).code(StatusCodes.SEE_OTHER) } @@ -270,9 +266,6 @@ export class PaymentField extends FormComponent { const payCallbackUrl = `${baseUrl}/payment-callback?uuid=${uuid}` const paymentPageUrl = args.sourceUrl - // Prepopulate GOV.UK Pay email if emailField is configured. - // The referenced EmailAddressField validates with joi.string().email() - // at input time, so the value in state is already validated. let prefilledEmail: string | undefined if (options.emailField) { const emailValue = state[options.emailField] @@ -330,7 +323,6 @@ export class PaymentField extends FormComponent { _metadata: FormMetadata, context: FormContext ): Promise { - // Zero-amount bypass — no capture needed const resolvedAmount = PaymentField.resolveAmount( this.options, this.model, diff --git a/src/server/plugins/engine/pageControllers/QuestionPageController.ts b/src/server/plugins/engine/pageControllers/QuestionPageController.ts index 1b2398001..e65656de0 100644 --- a/src/server/plugins/engine/pageControllers/QuestionPageController.ts +++ b/src/server/plugins/engine/pageControllers/QuestionPageController.ts @@ -185,9 +185,6 @@ export class QuestionPageController extends PageController { } } - // Override payment amount display with resolved conditional amount - // getViewModel() only has the current page's payload, not full form state. - // The full state is available via context.evaluationState. for (const comp of components) { if ('amount' in comp.model && 'paymentState' in comp.model) { const paymentField = this.collection.fields.find( @@ -270,9 +267,6 @@ export class QuestionPageController extends PageController { } } - // Skip payment pages in the normal page walk. - // Users reach the payment page via "Pay and submit" on CYA, - // not by navigating forward through the form. if (isPaymentPage(page.pageDef)) { return false } diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts index 4c704d111..006bee0c9 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts @@ -153,6 +153,16 @@ describe('SummaryPageController - Payment (DF-832)', () => { expect(handleFormSubmitSpy).toHaveBeenCalledTimes(1) // The GET handler should short-circuit before rendering the view expect(h.view).not.toHaveBeenCalled() + + // GOV.UK Pay callback is a GET so request.payload is undefined. + // Downstream submit code (e.g. outputService) reads + // request.payload.* fields, so the controller must synthesise an + // empty payload before calling handleFormSubmit, otherwise + // request.payload.userConfirmationEmailAddress crashes with + // "Cannot read properties of undefined". + const passedRequest = handleFormSubmitSpy.mock.calls[0][0] + expect(passedRequest.payload).toBeDefined() + expect(passedRequest.payload).toEqual({}) }) it('follows the normal CYA render path when paymentComplete is absent', async () => { diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 2863e51ba..3543385c6 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -62,14 +62,6 @@ export class SummaryPageController extends QuestionPageController { declare pageDef: Page allowSaveAndExit = true - /** - * The controller which is used when Page["controller"] is defined as "./pages/summary.js" - */ - - /** - * Finds the PaymentField component across all pages in the model. - * Payment pages are skipped in the normal page walk - */ private findPaymentField(): PaymentField | undefined { return this.model.pages .flatMap((page) => page.collection.fields) @@ -183,9 +175,11 @@ export class SummaryPageController extends QuestionPageController { ) => { const { viewName } = this - // After GOV.UK Pay callback, auto-submit the form instead of - // showing CYA again. The payment is already pre-authorized. if (request.query.paymentComplete === 'true') { + const requestWithPayload = request as unknown as { + payload: FormRequestPayload['payload'] + } + requestWithPayload.payload = {} as FormRequestPayload['payload'] return this.handleFormSubmit( request as unknown as FormRequestPayload, context, @@ -204,10 +198,6 @@ export class SummaryPageController extends QuestionPageController { } } - /** - * Checks if the resolved payment amount has changed since pre-auth - * and invalidates stale payment state if so. - */ private async reconcilePaymentState( request: FormRequest, context: FormContext @@ -358,9 +348,6 @@ export class SummaryPageController extends QuestionPageController { } } - // Payment page is skipped in the page walk, so proceed() would redirect - // to an earlier page. For PaymentIncomplete, redirect directly to the - // payment page using its href. if ( error.errorType === PaymentErrorTypes.PaymentIncomplete && error.component.page @@ -502,8 +489,6 @@ async function finaliseComponents( context: FormContext, model: FormModel ) { - // Get fields from relevant pages (normal components) - // plus PaymentField from all pages (payment page is skipped in the page walk) const allFields = new Set([ ...context.relevantPages.flatMap((page) => page.collection.fields), ...model.pages diff --git a/src/server/plugins/engine/routes/index.ts b/src/server/plugins/engine/routes/index.ts index ef324cd6b..f1c51409f 100644 --- a/src/server/plugins/engine/routes/index.ts +++ b/src/server/plugins/engine/routes/index.ts @@ -103,9 +103,6 @@ export async function redirectOrMakeHandler( return proceed(request, h, resumeInRepeaterUrl) } - // Return handler for relevant pages, payment pages, or preview URL direct access. - // Payment pages are skipped in the normal page walk but must render when the user - // is redirected there from CYA "Pay and submit". if ( relevantPath.startsWith(page.path) || isPaymentPage(page.pageDef) || diff --git a/src/server/plugins/engine/routes/payment.js b/src/server/plugins/engine/routes/payment.js index 692f623fe..33c904147 100644 --- a/src/server/plugins/engine/routes/payment.js +++ b/src/server/plugins/engine/routes/payment.js @@ -104,8 +104,6 @@ function handlePaymentSuccess(request, h, session, sessionKey, paymentStatus) { flashComponentState(request, session, paymentStatus) request.yar.clear(sessionKey) - // Append paymentComplete flag so the summary page auto-submits - // instead of showing CYA again const separator = session.returnUrl.includes('?') ? '&' : '?' const returnUrl = `${session.returnUrl}${separator}paymentComplete=true` diff --git a/src/server/plugins/payment/service.js b/src/server/plugins/payment/service.js index 32ae24c8b..244fd83af 100644 --- a/src/server/plugins/payment/service.js +++ b/src/server/plugins/payment/service.js @@ -63,7 +63,6 @@ export class PaymentService { delayed_capture: true } - // Prepopulate email on GOV.UK Pay if provided if (email) { payload.email = email }