From 9b95146211b64248d9bb1c3799fdd392bb8e086e Mon Sep 17 00:00:00 2001 From: Phil Benson Date: Wed, 6 Nov 2024 18:01:35 +0000 Subject: [PATCH 1/3] Fix bug where transaction id is not recognised https://eaflood.atlassian.net/browse/IWTF-4352 On return from GovPay, the transaction is not retrieved and the transaction id has changed From 7da1374cdd120c77a2d7233d2ac2d596b9822f50 Mon Sep 17 00:00:00 2001 From: Phil Benson Date: Wed, 6 Nov 2024 18:05:39 +0000 Subject: [PATCH 2/3] Only set a transaction id if one hasn't already been set --- .../agreed-handler-recurring-payments.spec.js | 22 ++++++++++++++++++- .../src/handlers/agreed-handler.js | 4 +++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js b/packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js index 3e913708f..a3fecb944 100644 --- a/packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js +++ b/packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js @@ -36,7 +36,7 @@ describe('The agreed handler', () => { cache: () => ({ helpers: { transaction: { - get: async () => ({ id: 'id-111', cost: 0 }), + get: async () => ({ cost: 0 }), set: async () => {} }, status: { @@ -90,6 +90,26 @@ describe('The agreed handler', () => { expect(transactionPayload.id).toBe(v4guid) }) + it("doesn't overwrite transaction id if one is already set", async () => { + const setTransaction = jest.fn() + const transactionId = 'abc-123-def-456' + uuidv4.mockReturnValue('def-789-ghi-012') + const mockRequest = getMockRequest({ + transaction: { + get: async () => ({ cost: 0, id: transactionId }), + set: setTransaction + } + }) + + await agreedHandler(mockRequest, getRequestToolkit()) + + expect(setTransaction).toHaveBeenCalledWith( + expect.objectContaining({ + id: transactionId + }) + ) + }) + it('sends a recurring payment creation request to Gov.UK Pay', async () => { const preparedPayment = Symbol('preparedPayment') prepareRecurringPayment.mockResolvedValueOnce(preparedPayment) diff --git a/packages/gafl-webapp-service/src/handlers/agreed-handler.js b/packages/gafl-webapp-service/src/handlers/agreed-handler.js index 1c0e505d7..9acb830d8 100644 --- a/packages/gafl-webapp-service/src/handlers/agreed-handler.js +++ b/packages/gafl-webapp-service/src/handlers/agreed-handler.js @@ -241,7 +241,9 @@ const finaliseTransaction = async (request, transaction, status) => { export default async (request, h) => { const status = await request.cache().helpers.status.get() const transaction = await request.cache().helpers.transaction.get() - transaction.id = uuidv4() + if (!transaction.id) { + transaction.id = uuidv4() + } // If the agreed flag is not set to true then throw an exception if (!status[COMPLETION_STATUS.agreed]) { From f74b75a96ddd1358eee251c2abf17a8abbc83bd6 Mon Sep 17 00:00:00 2001 From: Phil Benson Date: Thu, 7 Nov 2024 09:21:54 +0000 Subject: [PATCH 3/3] Amend transaction id overwrite test to be closer to what was observed - i.e. the finalise transaction call failing because the wrong transaction id was sent --- .../agreed-handler-recurring-payments.spec.js | 33 ++++++++++++++----- .../src/handlers/agreed-handler.js | 1 + 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js b/packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js index a3fecb944..4355907d1 100644 --- a/packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js +++ b/packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js @@ -94,19 +94,34 @@ describe('The agreed handler', () => { const setTransaction = jest.fn() const transactionId = 'abc-123-def-456' uuidv4.mockReturnValue('def-789-ghi-012') - const mockRequest = getMockRequest({ - transaction: { - get: async () => ({ cost: 0, id: transactionId }), - set: setTransaction - } + salesApi.finaliseTransaction.mockReturnValueOnce({ + permissions: [] }) + const mockRequest = { + cache: () => ({ + helpers: { + status: { + get: async () => ({ + [COMPLETION_STATUS.agreed]: true, + [COMPLETION_STATUS.posted]: true, + [COMPLETION_STATUS.finalised]: false, + [RECURRING_PAYMENT]: false + }), + set: () => {} + }, + transaction: { + get: async () => ({ cost: 0, id: transactionId }), + set: setTransaction + } + } + }) + } await agreedHandler(mockRequest, getRequestToolkit()) - expect(setTransaction).toHaveBeenCalledWith( - expect.objectContaining({ - id: transactionId - }) + expect(salesApi.finaliseTransaction).toHaveBeenCalledWith( + transactionId, + undefined // prepareApiFinalisationPayload has no mocked return value ) }) diff --git a/packages/gafl-webapp-service/src/handlers/agreed-handler.js b/packages/gafl-webapp-service/src/handlers/agreed-handler.js index 9acb830d8..a4589dd7e 100644 --- a/packages/gafl-webapp-service/src/handlers/agreed-handler.js +++ b/packages/gafl-webapp-service/src/handlers/agreed-handler.js @@ -281,6 +281,7 @@ export default async (request, h) => { // If the transaction has already been finalised then redirect to the order completed page if (!status[COMPLETION_STATUS.finalised]) { + console.log('finalising transaction', request, transaction, status) await finaliseTransaction(request, transaction, status) } else { debug('Transaction %s already finalised, redirect to order complete: %s', transaction.id)