From ecd5c73d8938cf4ec5bb2e558b95360d2d02ddbd Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 10 Feb 2026 11:43:07 +0000 Subject: [PATCH 1/4] Additional logging --- .../plugins/engine/components/PaymentField.ts | 6 ++- .../plugins/engine/routes/payment-helper.js | 32 +++++++++++- src/server/plugins/engine/routes/payment.js | 46 ++++++++++++++++- src/server/plugins/payment/service.js | 51 ++++++++++--------- src/server/plugins/payment/service.test.js | 10 +++- 5 files changed, 117 insertions(+), 28 deletions(-) diff --git a/src/server/plugins/engine/components/PaymentField.ts b/src/server/plugins/engine/components/PaymentField.ts index c1d49a907..4be399263 100644 --- a/src/server/plugins/engine/components/PaymentField.ts +++ b/src/server/plugins/engine/components/PaymentField.ts @@ -225,6 +225,7 @@ export class PaymentField extends FormComponent { description, payCallbackUrl, reference, + isLivePayment, { formId, slug } ) @@ -276,7 +277,10 @@ export class PaymentField extends FormComponent { /** * @see https://docs.payments.service.gov.uk/api_reference/#payment-status-lifecycle */ - const status = await paymentService.getPaymentStatus(paymentId) + const status = await paymentService.getPaymentStatus( + paymentId, + isLivePayment + ) PaymentSubmissionError.checkPaymentAmount( status.amount, diff --git a/src/server/plugins/engine/routes/payment-helper.js b/src/server/plugins/engine/routes/payment-helper.js index f2ce83b24..cd8da316a 100644 --- a/src/server/plugins/engine/routes/payment-helper.js +++ b/src/server/plugins/engine/routes/payment-helper.js @@ -28,11 +28,41 @@ export async function getPaymentContext(request, uuid) { const apiKey = getPaymentApiKey(isLivePayment, formId) const paymentService = new PaymentService(apiKey) - const paymentStatus = await paymentService.getPaymentStatus(paymentId) + const paymentStatus = await paymentService.getPaymentStatus( + paymentId, + isLivePayment + ) return { session, sessionKey, paymentStatus } } +/** + * Builds an object for logging payment information + * @param {string} action + * @param {string} outcome + * @param {string} reason + * @param {boolean} isLivePayment + * @param {string} paymentId + */ +export function buildPaymentInfo( + action, + outcome, + reason, + isLivePayment, + paymentId +) { + return { + event: { + category: 'payment', + action, + outcome, + reason, + type: isLivePayment ? 'live' : 'test', + reference: paymentId + } + } +} + /** * @import { Request } from '@hapi/hapi' * @import { GetPaymentResponse, PaymentSessionData } from '~/src/server/plugins/payment/types.js' diff --git a/src/server/plugins/engine/routes/payment.js b/src/server/plugins/engine/routes/payment.js index 5c360e38e..d6d9168a7 100644 --- a/src/server/plugins/engine/routes/payment.js +++ b/src/server/plugins/engine/routes/payment.js @@ -2,12 +2,18 @@ import Boom from '@hapi/boom' import { StatusCodes } from 'http-status-codes' import Joi from 'joi' +import { createLogger } from '~/src/server/common/helpers/logging/logger.js' import { EXTERNAL_STATE_APPENDAGE } from '~/src/server/constants.js' -import { getPaymentContext } from '~/src/server/plugins/engine/routes/payment-helper.js' +import { + buildPaymentInfo, + getPaymentContext +} from '~/src/server/plugins/engine/routes/payment-helper.js' export const PAYMENT_RETURN_PATH = '/payment-callback' export const PAYMENT_SESSION_PREFIX = 'payment-' +const logger = createLogger() + /** * Flash form component state after successful payment * @param {Request} request - the request @@ -48,6 +54,42 @@ export function getRoutes() { return [getReturnRoute()] } +/** + * Logs successful payment + * @param {PaymentSessionData} session - the session data + * @param {GetPaymentResponse} paymentStatus - the payment status from GOV.UK Pay + */ +function logPaymentSuccess(session, paymentStatus) { + logger.info( + buildPaymentInfo( + 'pre-auth', + 'success', + `${paymentStatus.state.status} amount=${paymentStatus.amount}`, + session.isLivePayment, + paymentStatus.paymentId + ), + `[payment] Successful pre-auth for paymentId=${paymentStatus.paymentId}` + ) +} + +/** + * Logs failed/cancelled payment + * @param {PaymentSessionData} session - the session data + * @param {GetPaymentResponse} paymentStatus - the payment status from GOV.UK Pay + */ +function logPaymentFailure(session, paymentStatus) { + logger.info( + buildPaymentInfo( + 'pre-auth', + 'failed/cancelled', + `${paymentStatus.state.status} amount=${paymentStatus.amount}`, + session.isLivePayment, + paymentStatus.paymentId + ), + `[payment] Failed/cancelled pre-auth for paymentId=${paymentStatus.paymentId}` + ) +} + /** * Handles successful payment states (capturable/success) * @param {Request} request - the request @@ -98,6 +140,7 @@ function getReturnRoute() { switch (status) { case 'capturable': case 'success': + logPaymentSuccess(session, paymentStatus) return handlePaymentSuccess( request, h, @@ -109,6 +152,7 @@ function getReturnRoute() { case 'cancelled': case 'failed': case 'error': + logPaymentFailure(session, paymentStatus) return handlePaymentFailure(request, h, session, sessionKey) case 'created': diff --git a/src/server/plugins/payment/service.js b/src/server/plugins/payment/service.js index 24d9c2f5c..acb2ea1c9 100644 --- a/src/server/plugins/payment/service.js +++ b/src/server/plugins/payment/service.js @@ -1,6 +1,7 @@ import { StatusCodes } from 'http-status-codes' import { createLogger } from '~/src/server/common/helpers/logging/logger.js' +import { buildPaymentInfo } from '~/src/server/plugins/engine/routes/payment-helper.js' import { get, post, postJson } from '~/src/server/services/httpService.js' const PAYMENT_BASE_URL = 'https://publicapi.payments.service.gov.uk' @@ -35,9 +36,17 @@ export class PaymentService { * @param {string} description * @param {string} returnUrl * @param {string} reference + * @param {boolean} isLivePayment * @param {{ formId: string, slug: string }} metadata */ - async createPayment(amount, description, returnUrl, reference, metadata) { + async createPayment( + amount, + description, + returnUrl, + reference, + isLivePayment, + metadata + ) { const response = await this.postToPayProvider({ amount, description, @@ -48,15 +57,13 @@ export class PaymentService { }) logger.info( - { - event: { - category: 'payment', - action: 'create-payment', - outcome: 'success', - reason: `amount=${amount}`, - reference: response.payment_id - } - }, + buildPaymentInfo( + 'create-payment', + 'success', + `amount=${amount}`, + isLivePayment, + response.payment_id + ), `[payment] Created payment and user taken to enter pre-auth details for paymentId=${response.payment_id}` ) @@ -68,9 +75,10 @@ export class PaymentService { /** * @param {string} paymentId + * @param {boolean} isLivePayment * @returns {Promise} */ - async getPaymentStatus(paymentId) { + async getPaymentStatus(paymentId, isLivePayment) { const getByType = /** @type {typeof get} */ (get) try { @@ -92,18 +100,15 @@ export class PaymentService { const state = response.payload.state logger.info( - { - event: { - category: 'payment', - action: 'get-payment-status', - outcome: - state.status === 'capturable' || state.status === 'success' - ? 'success' - : 'failure', - reason: `status:${state.status} code:${state.code ?? 'N/A'} message:${state.message ?? 'N/A'}`, - reference: paymentId - } - }, + buildPaymentInfo( + 'get-payment-status', + state.status === 'capturable' || state.status === 'success' + ? 'success' + : 'failure', + `status:${state.status} code:${state.code ?? 'N/A'} message:${state.message ?? 'N/A'}`, + isLivePayment, + paymentId + ), `[payment] Got payment status for paymentId=${paymentId}: status=${state.status}` ) diff --git a/src/server/plugins/payment/service.test.js b/src/server/plugins/payment/service.test.js index d48d6cba7..ccd4db80b 100644 --- a/src/server/plugins/payment/service.test.js +++ b/src/server/plugins/payment/service.test.js @@ -41,6 +41,7 @@ describe('payment service', () => { 'Payment description', returnUrl, referenceNumber, + false, metadata ) expect(payment.paymentId).toBe('payment-id-12345') @@ -61,6 +62,7 @@ describe('payment service', () => { 'Payment description', returnUrl, referenceNumber, + false, metadata ) ).rejects.toThrow('internal creation error') @@ -90,6 +92,7 @@ describe('payment service', () => { 'Payment description', returnUrl, referenceNumber, + false, metadata ) ).rejects.toThrow('Failed to create payment') @@ -119,7 +122,10 @@ describe('payment service', () => { error: undefined }) - const paymentStatus = await service.getPaymentStatus('payment-id-12345') + const paymentStatus = await service.getPaymentStatus( + 'payment-id-12345', + false + ) expect(paymentStatus.paymentId).toBe('payment-id-12345') expect(paymentStatus._links.next_url?.href).toBe( 'http://next-url-href/payment' @@ -137,7 +143,7 @@ describe('payment service', () => { }) await expect(() => - service.getPaymentStatus('payment-id-12345') + service.getPaymentStatus('payment-id-12345', false) ).rejects.toThrow('Failed to get payment status: some-error') }) }) From b26324a6e6a9bbfd140b07967c10bbb89b5fe04f Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 10 Feb 2026 11:52:41 +0000 Subject: [PATCH 2/4] Extra coverage --- .../engine/routes/payment-helper.test.js | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/server/plugins/engine/routes/payment-helper.test.js b/src/server/plugins/engine/routes/payment-helper.test.js index d18fb3aa4..d96821539 100644 --- a/src/server/plugins/engine/routes/payment-helper.test.js +++ b/src/server/plugins/engine/routes/payment-helper.test.js @@ -1,4 +1,7 @@ -import { getPaymentContext } from '~/src/server/plugins/engine/routes/payment-helper.js' +import { + buildPaymentInfo, + getPaymentContext +} from '~/src/server/plugins/engine/routes/payment-helper.js' import { get } from '~/src/server/services/httpService.js' jest.mock('~/src/server/services/httpService.ts') @@ -83,6 +86,46 @@ describe('payment helper', () => { sessionKey: 'payment-5a54c2fe-da49-4202-8cd3-2121eaca03c3' }) }) + + it('should create logging info for a test payment', () => { + const res = buildPaymentInfo( + 'action1', + 'outcome1', + 'reason1', + false, + 'pay-123' + ) + expect(res).toEqual({ + event: { + category: 'payment', + action: 'action1', + outcome: 'outcome1', + reason: 'reason1', + type: 'test', + reference: 'pay-123' + } + }) + }) + + it('should create logging info for a live payment', () => { + const res = buildPaymentInfo( + 'action2', + 'outcome2', + 'reason2', + true, + 'pay-123' + ) + expect(res).toEqual({ + event: { + category: 'payment', + action: 'action2', + outcome: 'outcome2', + reason: 'reason2', + type: 'live', + reference: 'pay-123' + } + }) + }) }) /** From 390650467e650a970c88612edc837785ab373c13 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 10 Feb 2026 12:03:57 +0000 Subject: [PATCH 3/4] Fixed pounds vs pence --- src/server/plugins/engine/routes/payment.js | 4 ++-- src/server/plugins/payment/service.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server/plugins/engine/routes/payment.js b/src/server/plugins/engine/routes/payment.js index d6d9168a7..c1c5ed3b8 100644 --- a/src/server/plugins/engine/routes/payment.js +++ b/src/server/plugins/engine/routes/payment.js @@ -64,7 +64,7 @@ function logPaymentSuccess(session, paymentStatus) { buildPaymentInfo( 'pre-auth', 'success', - `${paymentStatus.state.status} amount=${paymentStatus.amount}`, + `${paymentStatus.state.status} amount=${paymentStatus.amount / 100}`, session.isLivePayment, paymentStatus.paymentId ), @@ -82,7 +82,7 @@ function logPaymentFailure(session, paymentStatus) { buildPaymentInfo( 'pre-auth', 'failed/cancelled', - `${paymentStatus.state.status} amount=${paymentStatus.amount}`, + `${paymentStatus.state.status} amount=${paymentStatus.amount / 100}`, session.isLivePayment, paymentStatus.paymentId ), diff --git a/src/server/plugins/payment/service.js b/src/server/plugins/payment/service.js index acb2ea1c9..ae2cc00ea 100644 --- a/src/server/plugins/payment/service.js +++ b/src/server/plugins/payment/service.js @@ -60,7 +60,7 @@ export class PaymentService { buildPaymentInfo( 'create-payment', 'success', - `amount=${amount}`, + `amount=${amount / 100}`, isLivePayment, response.payment_id ), @@ -156,7 +156,7 @@ export class PaymentService { category: 'payment', action: 'capture-payment', outcome: 'success', - reason: `amount=${amount}`, + reason: `amount=${amount / 100}`, reference: paymentId } }, From e8caa51281d5212c20b1075c342c20a24406fef6 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Wed, 11 Feb 2026 12:11:03 +0000 Subject: [PATCH 4/4] Created function to do pence to pounds conversion --- src/server/plugins/engine/routes/payment-helper.js | 7 +++++++ src/server/plugins/engine/routes/payment.js | 5 +++-- src/server/plugins/payment/service.js | 9 ++++++--- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/server/plugins/engine/routes/payment-helper.js b/src/server/plugins/engine/routes/payment-helper.js index cd8da316a..bfa0ab0d9 100644 --- a/src/server/plugins/engine/routes/payment-helper.js +++ b/src/server/plugins/engine/routes/payment-helper.js @@ -63,6 +63,13 @@ export function buildPaymentInfo( } } +/** + * @param {number} amount + */ +export function convertPenceToPounds(amount) { + return `${amount / 100}` +} + /** * @import { Request } from '@hapi/hapi' * @import { GetPaymentResponse, PaymentSessionData } from '~/src/server/plugins/payment/types.js' diff --git a/src/server/plugins/engine/routes/payment.js b/src/server/plugins/engine/routes/payment.js index c1c5ed3b8..50ba8becc 100644 --- a/src/server/plugins/engine/routes/payment.js +++ b/src/server/plugins/engine/routes/payment.js @@ -6,6 +6,7 @@ import { createLogger } from '~/src/server/common/helpers/logging/logger.js' import { EXTERNAL_STATE_APPENDAGE } from '~/src/server/constants.js' import { buildPaymentInfo, + convertPenceToPounds, getPaymentContext } from '~/src/server/plugins/engine/routes/payment-helper.js' @@ -64,7 +65,7 @@ function logPaymentSuccess(session, paymentStatus) { buildPaymentInfo( 'pre-auth', 'success', - `${paymentStatus.state.status} amount=${paymentStatus.amount / 100}`, + `${paymentStatus.state.status} amount=${convertPenceToPounds(paymentStatus.amount)}`, session.isLivePayment, paymentStatus.paymentId ), @@ -82,7 +83,7 @@ function logPaymentFailure(session, paymentStatus) { buildPaymentInfo( 'pre-auth', 'failed/cancelled', - `${paymentStatus.state.status} amount=${paymentStatus.amount / 100}`, + `${paymentStatus.state.status} amount=${convertPenceToPounds(paymentStatus.amount)}`, session.isLivePayment, paymentStatus.paymentId ), diff --git a/src/server/plugins/payment/service.js b/src/server/plugins/payment/service.js index ae2cc00ea..b643fe6a4 100644 --- a/src/server/plugins/payment/service.js +++ b/src/server/plugins/payment/service.js @@ -1,7 +1,10 @@ import { StatusCodes } from 'http-status-codes' import { createLogger } from '~/src/server/common/helpers/logging/logger.js' -import { buildPaymentInfo } from '~/src/server/plugins/engine/routes/payment-helper.js' +import { + buildPaymentInfo, + convertPenceToPounds +} from '~/src/server/plugins/engine/routes/payment-helper.js' import { get, post, postJson } from '~/src/server/services/httpService.js' const PAYMENT_BASE_URL = 'https://publicapi.payments.service.gov.uk' @@ -60,7 +63,7 @@ export class PaymentService { buildPaymentInfo( 'create-payment', 'success', - `amount=${amount / 100}`, + `amount=${convertPenceToPounds(amount)}`, isLivePayment, response.payment_id ), @@ -156,7 +159,7 @@ export class PaymentService { category: 'payment', action: 'capture-payment', outcome: 'success', - reason: `amount=${amount / 100}`, + reason: `amount=${convertPenceToPounds(amount)}`, reference: paymentId } },