Merge pull request #309 from DEFRA/feat/df-634-enhanced-logging

jbarnsley10 · web-flow · commit cc63848ed73c · 2026-02-11T12:15:27.000Z
feat/df-634: Additional logging
diff --git 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
@@ -28,11 +28,48 @@ 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
+    }
+  }
+}
+
+/**
+ * @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-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'
+      }
+    })
+  })
 })
 
 /**
diff --git a/src/server/plugins/engine/routes/payment.js b/src/server/plugins/engine/routes/payment.js
@@ -2,12 +2,19 @@ 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,
+  convertPenceToPounds,
+  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 +55,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=${convertPenceToPounds(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=${convertPenceToPounds(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 +141,7 @@ function getReturnRoute() {
       switch (status) {
         case 'capturable':
         case 'success':
+          logPaymentSuccess(session, paymentStatus)
           return handlePaymentSuccess(
             request,
             h,
@@ -109,6 +153,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
@@ -1,6 +1,10 @@
 import { StatusCodes } from 'http-status-codes'
 
 import { createLogger } from '~/src/server/common/helpers/logging/logger.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'
@@ -35,9 +39,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 +60,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=${convertPenceToPounds(amount)}`,
+        isLivePayment,
+        response.payment_id
+      ),
       `[payment] Created payment and user taken to enter pre-auth details for paymentId=${response.payment_id}`
     )
 
@@ -68,9 +78,10 @@ export class PaymentService {
 
   /**
    * @param {string} paymentId
+   * @param {boolean} isLivePayment
    * @returns {Promise<GetPaymentResponse>}
    */
-  async getPaymentStatus(paymentId) {
+  async getPaymentStatus(paymentId, isLivePayment) {
     const getByType = /** @type {typeof get<GetPaymentApiResponse>} */ (get)
 
     try {
@@ -92,18 +103,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}`
       )
 
@@ -151,7 +159,7 @@ export class PaymentService {
               category: 'payment',
               action: 'capture-payment',
               outcome: 'success',
-              reason: `amount=${amount}`,
+              reason: `amount=${convertPenceToPounds(amount)}`,
               reference: paymentId
             }
           },
diff --git 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')
     })
   })
