From cac29ef6b4bf90fc05c5a1e3800276bf47c5665c Mon Sep 17 00:00:00 2001 From: Philip Benson Date: Mon, 13 Apr 2026 15:33:14 +0100 Subject: [PATCH 1/3] Use HTTPRequestBatcher for Gov.UK Pay API calls https://eaflood.atlassian.net/browse/IWTF-4598 Modify the recurring payments processor to route all Gov.UK Pay API interactions (payment creation and payment status checks) through an HTTPRequestBatcher instance. This introduces configurable batch size and inter-batch delay via environment variables (RCP_BATCHER_BATCH_SIZE, RCP_BATCHER_DELAY_MS), enabling rate-limit-aware batching with automatic 429 retry handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> From 51665085baaa9eff306682f8ad1018d6de4fae43 Mon Sep 17 00:00:00 2001 From: Philip Benson Date: Mon, 13 Apr 2026 16:25:34 +0100 Subject: [PATCH 2/3] Route Gov.UK Pay calls through HTTPRequestBatcher Use an HTTPRequestBatcher instance for payment creation and payment status check requests so that all Gov.UK Pay interactions are batched and benefit from automatic 429 back-pressure handling. - Replace sendPayment/getPaymentStatus with two HTTPRequestBatcher instances (one per phase: creation, status-check). - Add govPayRecurringHeaders() helper and createBatcher() factory. - Batch size and inter-batch delay are configurable via env vars: RCP_BATCHER_BATCH_SIZE (default 50) and RCP_BATCHER_DELAY_MS (default 1000). - Correlate creation responses via body.reference; status responses via response.url path segment. - Rewrite tests to mock HTTPRequestBatcher; 100% code coverage. https://eaflood.atlassian.net/browse/IWTF-4598 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../recurring-payments-processor.spec.js | 1235 +++++++++-------- .../src/recurring-payments-processor.js | 197 ++- 2 files changed, 773 insertions(+), 659 deletions(-) diff --git a/packages/recurring-payments-job/src/__tests__/recurring-payments-processor.spec.js b/packages/recurring-payments-job/src/__tests__/recurring-payments-processor.spec.js index 765e1bd903..70a756d23d 100644 --- a/packages/recurring-payments-job/src/__tests__/recurring-payments-processor.spec.js +++ b/packages/recurring-payments-job/src/__tests__/recurring-payments-processor.spec.js @@ -1,7 +1,7 @@ -import { airbrake, salesApi } from '@defra-fish/connectors-lib' +import { airbrake, salesApi, HTTPRequestBatcher } from '@defra-fish/connectors-lib' import { PAYMENT_STATUS, PAYMENT_JOURNAL_STATUS_CODES } from '@defra-fish/business-rules-lib' import { execute } from '../recurring-payments-processor.js' -import { getPaymentStatus, isGovPayUp, sendPayment } from '../services/govuk-pay-service.js' +import { isGovPayUp } from '../services/govuk-pay-service.js' import db from 'debug' jest.mock('@defra-fish/business-rules-lib', () => ({ @@ -18,6 +18,7 @@ jest.mock('@defra-fish/business-rules-lib', () => ({ Completed: 'completed payment' } })) + jest.mock('@defra-fish/connectors-lib', () => ({ airbrake: { initialise: jest.fn(), @@ -29,9 +30,7 @@ jest.mock('@defra-fish/connectors-lib', () => ({ createTransaction: jest.fn(() => ({ id: 'test-transaction-id', cost: 30, - recurringPayment: { - id: 'recurring-payment-1' - } + recurringPayment: { id: 'recurring-payment-1' } })), getDueRecurringPayments: jest.fn(() => []), getPaymentJournal: jest.fn(), @@ -40,52 +39,137 @@ jest.mock('@defra-fish/connectors-lib', () => ({ })), processRPResult: jest.fn(), updatePaymentJournal: jest.fn() - } + }, + HTTPRequestBatcher: jest.fn() })) jest.mock('../services/govuk-pay-service.js', () => ({ - sendPayment: jest.fn(() => ({ payment_id: 'payment_id', created_date: '2025-07-18T09:00:00.000Z' })), - getPaymentStatus: jest.fn(), isGovPayUp: jest.fn(() => true) })) jest.mock('debug', () => jest.fn(() => jest.fn())) +const GOV_PAY_API_URL = 'https://publicapi.payments.service.gov.uk/v1/payments' +const GOV_PAY_RECURRING_APIKEY = 'test-recurring-api-key' const PAYMENT_STATUS_DELAY = 60000 -const getPaymentStatusSuccess = () => ({ state: { status: 'payment status success' } }) -const getPaymentStatusFailure = () => ({ state: { status: 'payment status failure' } }) -const getPaymentStatusError = () => ({ state: { status: 'payment status error' } }) + +// ── Response factories ──────────────────────────────────────────────────────── + +const mockCreationOkResponse = ({ + payment_id = 'pay-1', + reference = 'test-transaction-id', + created_date = '2025-01-01T00:00:00.000Z' +} = {}) => ({ + ok: true, + status: 200, + url: GOV_PAY_API_URL, + json: jest.fn().mockResolvedValue({ payment_id, reference, created_date }) +}) + +const mockCreationErrorResponse = ({ status = 422, description = 'An error occurred' } = {}) => ({ + ok: false, + status, + url: GOV_PAY_API_URL, + json: jest.fn().mockResolvedValue({ description }) +}) + +const mock429Response = (url = GOV_PAY_API_URL) => ({ + ok: false, + status: 429, + url, + json: jest.fn() +}) + +const mockStatusOkResponse = (paymentId, status) => ({ + ok: true, + status: 200, + url: `${GOV_PAY_API_URL}/${paymentId}`, + json: jest.fn().mockResolvedValue({ state: { status } }) +}) + +const mockStatusErrorResponse = (paymentId, httpStatus) => ({ + ok: false, + status: httpStatus, + url: `${GOV_PAY_API_URL}/${paymentId}`, + json: jest.fn().mockResolvedValue({ code: 'ERR', description: 'error' }) +}) + +// ── Batcher mock factory ────────────────────────────────────────────────────── + +const makeBatcherMock = (responses = []) => ({ + addRequest: jest.fn(), + fetch: jest.fn().mockResolvedValue(undefined), + responses +}) + +// ── Data factories ──────────────────────────────────────────────────────────── + +const getMockDueRecurringPayment = ({ agreementId = 'test-agreement-id', id = 'abc-123', referenceNumber = '123' } = {}) => ({ + entity: { id, agreementId }, + expanded: { activePermission: { entity: { referenceNumber } } } +}) + const getMockPaymentRequestResponse = () => [ { entity: { agreementId: 'agreement-1' }, - expanded: { - activePermission: { - entity: { - referenceNumber: 'ref-1' - } - } - } + expanded: { activePermission: { entity: { referenceNumber: 'ref-1' } } } } ] -const getMockDueRecurringPayment = ({ agreementId = 'test-agreement-id', id = 'abc-123', referenceNumber = '123' } = {}) => ({ - entity: { id, agreementId }, - expanded: { activePermission: { entity: { referenceNumber } } } -}) +const getPaymentStatusSuccess = () => ({ state: { status: 'payment status success' } }) +const getPaymentStatusFailure = () => ({ state: { status: 'payment status failure' } }) +const getPaymentStatusError = () => ({ state: { status: 'payment status error' } }) -// eslint-disable-next-line camelcase -const getMockSendPaymentResponse = ({ payment_id = 'pay-1', agreementId = 'agr-1', created_date = '2025-01-01T00:00:00.000Z' } = {}) => ({ - payment_id, - agreementId, - created_date -}) +// ── Test helpers ────────────────────────────────────────────────────────────── + +/** + * Configures the HTTPRequestBatcher mock and salesApi mocks for a single-payment + * happy-path scenario, returning the batcher mock instances for assertions. + */ +const setupSinglePayment = ({ + agreementId = 'test-agreement-id', + id = 'abc-123', + referenceNumber = '123', + transactionId = 'test-transaction-id', + paymentId = 'pay-1', + created_date = '2025-01-01T00:00:00.000Z', + paymentStatus = 'payment status success', + permissionData = { licensee: { countryCode: 'GB-ENG' } } +} = {}) => { + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment({ agreementId, id, referenceNumber })]) + salesApi.preparePermissionDataForRenewal.mockReturnValueOnce(permissionData) + salesApi.createTransaction.mockReturnValueOnce({ id: transactionId, cost: 30, recurringPayment: { id } }) + + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: paymentId, reference: transactionId, created_date })]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse(paymentId, paymentStatus)]) + + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + + return { creationBatcher, statusBatcher, paymentId, transactionId, agreementId, created_date } +} + +// ── Tests ───────────────────────────────────────────────────────────────────── describe('recurring-payments-processor', () => { const [{ value: debugLogger }] = db.mock.results + beforeAll(() => { + process.env.GOV_PAY_API_URL = GOV_PAY_API_URL + process.env.GOV_PAY_RECURRING_APIKEY = GOV_PAY_RECURRING_APIKEY + }) + beforeEach(() => { - jest.clearAllMocks() + jest.restoreAllMocks() + jest.resetAllMocks() + // Restore default implementations that tests depend on + isGovPayUp.mockReturnValue(true) + salesApi.getDueRecurringPayments.mockReturnValue([]) + salesApi.createTransaction.mockReturnValue({ id: 'test-transaction-id', cost: 30, recurringPayment: { id: 'recurring-payment-1' } }) + salesApi.preparePermissionDataForRenewal.mockReturnValue({ licensee: { countryCode: 'GB-ENG' } }) + HTTPRequestBatcher.mockImplementation(() => makeBatcherMock()) process.env.RUN_RECURRING_PAYMENTS = 'true' + delete process.env.RCP_BATCHER_BATCH_SIZE + delete process.env.RCP_BATCHER_DELAY_MS global.setTimeout = jest.fn((cb, ms) => cb()) }) @@ -117,7 +201,6 @@ describe('recurring-payments-processor', () => { ['SIGTERM', 137] ])('flushes airbrake on %s signal', (signal, code) => { jest.isolateModules(() => { - // setup a delay so script doesn't call processRecurringPayments and exit naturally process.env.RECURRING_PAYMENTS_LOCAL_DELAY = '1' const signalCallbacks = {} jest.spyOn(process, 'on') @@ -125,9 +208,7 @@ describe('recurring-payments-processor', () => { process.on.mockImplementation((signalToken, callback) => { signalCallbacks[signalToken] = callback }) - process.exit.mockImplementation(() => { - // so we don't crash out of the tests! - }) + process.exit.mockImplementation(() => {}) require('../recurring-payments-processor.js') signalCallbacks[signal]() @@ -149,9 +230,7 @@ describe('recurring-payments-processor', () => { process.on.mockImplementation((signalToken, callback) => { signalCallbacks[signalToken] = callback }) - process.exit.mockImplementation(() => { - // so we don't crash out of the tests! - }) + process.exit.mockImplementation(() => {}) require('../recurring-payments-job.js') signalCallbacks[signal]() @@ -218,167 +297,257 @@ describe('recurring-payments-processor', () => { }) }) - describe('When payment request throws an error...', () => { - it('console.error is called with error message', async () => { - jest.spyOn(console, 'error') - salesApi.getDueRecurringPayments.mockReturnValueOnce(getMockPaymentRequestResponse()) - const oopsie = new Error('payment gate down') - sendPayment.mockRejectedValueOnce(oopsie) + describe('HTTPRequestBatcher configuration', () => { + it('creates batcher with default config when env vars are not set', async () => { + setupSinglePayment() - try { - await execute() - } catch {} + await execute() - expect(console.error).toHaveBeenCalledWith(expect.any(String), oopsie) + expect(HTTPRequestBatcher).toHaveBeenCalledWith({ batchSize: undefined, delay: undefined }) }) - it('prepares and sends all payment requests, even if some fail', async () => { - const agreementIds = [Symbol('agreementId1'), Symbol('agreementId2'), Symbol('agreementId3'), Symbol('agreementId4')] - salesApi.getDueRecurringPayments.mockReturnValueOnce([ - getMockDueRecurringPayment({ referenceNumber: 'fee', agreementId: agreementIds[0] }), - getMockDueRecurringPayment({ referenceNumber: 'fi', agreementId: agreementIds[1] }), - getMockDueRecurringPayment({ referenceNumber: 'foe', agreementId: agreementIds[2] }), - getMockDueRecurringPayment({ referenceNumber: 'fum', agreementId: agreementIds[3] }) - ]) + it('creates batcher with RCP_BATCHER_BATCH_SIZE from env when set', async () => { + process.env.RCP_BATCHER_BATCH_SIZE = '10' + setupSinglePayment() - const permissionData = { licensee: { countryCode: 'GB-ENG' } } - for (let x = 0; x < agreementIds.length; x++) { - salesApi.preparePermissionDataForRenewal.mockReturnValueOnce(permissionData) - salesApi.createTransaction.mockReturnValueOnce({ - cost: 50, - id: `transaction-id-${x + 1}` + await execute() + + expect(HTTPRequestBatcher).toHaveBeenCalledWith(expect.objectContaining({ batchSize: 10 })) + }) + + it('creates batcher with RCP_BATCHER_DELAY_MS from env when set', async () => { + process.env.RCP_BATCHER_DELAY_MS = '2000' + setupSinglePayment() + + await execute() + + expect(HTTPRequestBatcher).toHaveBeenCalledWith(expect.objectContaining({ delay: 2000 })) + }) + + it('creates two separate batcher instances — one for creation, one for status', async () => { + setupSinglePayment() + + await execute() + + expect(HTTPRequestBatcher).toHaveBeenCalledTimes(2) + }) + }) + + describe('Payment creation via batcher', () => { + it('calls addRequest for each due payment with the correct URL', async () => { + const { creationBatcher } = setupSinglePayment() + + await execute() + + expect(creationBatcher.addRequest).toHaveBeenCalledWith(GOV_PAY_API_URL, expect.any(Object)) + }) + + it('calls addRequest with correct method and headers', async () => { + const { creationBatcher } = setupSinglePayment() + + await execute() + + expect(creationBatcher.addRequest).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + method: 'post', + headers: expect.objectContaining({ + accept: 'application/json', + authorization: `Bearer ${GOV_PAY_RECURRING_APIKEY}`, + 'content-type': 'application/json' + }) }) + ) + }) - if (x === 1) { - const err = new Error('Payment request failed') - sendPayment.mockRejectedValueOnce(err) - } else { - sendPayment.mockResolvedValueOnce({ payment_id: `test-payment-id-${x + 1}`, agreementId: agreementIds[x] }) - } - if (x < 3) { - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) - } - } - const expectedData = { + it('calls addRequest with the correct payment body', async () => { + const agreementId = Symbol('agreementId') + const transactionId = 'transactionId' + + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment({ agreementId, referenceNumber: 'foo' })]) + salesApi.createTransaction.mockReturnValueOnce({ cost: 50, id: transactionId, recurringPayment: { id: 'rp-1' } }) + + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ reference: transactionId })]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse('pay-1', 'payment status success')]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + + await execute() + + const expectedBody = JSON.stringify({ amount: 5000, description: 'The recurring card payment for your rod fishing licence', - reference: 'transactionId', - authorisation_mode: 'agreement' - } + reference: transactionId, + authorisation_mode: 'agreement', + agreement_id: agreementId + }) + + expect(creationBatcher.addRequest).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ body: expectedBody }) + ) + }) + + it('calls batcher.fetch() for payment creation', async () => { + const { creationBatcher } = setupSinglePayment() await execute() - expect(sendPayment).toHaveBeenCalledTimes(4) - expect(sendPayment).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ ...expectedData, reference: 'transaction-id-1', agreement_id: agreementIds[0] }) - ) - expect(sendPayment).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ ...expectedData, reference: 'transaction-id-2', agreement_id: agreementIds[1] }) + expect(creationBatcher.fetch).toHaveBeenCalledTimes(1) + }) + + it('calls addRequest for all due payments, even if batch size is smaller', async () => { + const agreementIds = ['agr-1', 'agr-2', 'agr-3', 'agr-4'] + salesApi.getDueRecurringPayments.mockReturnValueOnce( + agreementIds.map((id, i) => getMockDueRecurringPayment({ agreementId: id, referenceNumber: `ref-${i}` })) ) - expect(sendPayment).toHaveBeenNthCalledWith( - 3, - expect.objectContaining({ ...expectedData, reference: 'transaction-id-3', agreement_id: agreementIds[2] }) + + const permissionData = { licensee: { countryCode: 'GB-ENG' } } + agreementIds.forEach((_, i) => { + salesApi.preparePermissionDataForRenewal.mockReturnValueOnce(permissionData) + salesApi.createTransaction.mockReturnValueOnce({ cost: 50, id: `trans-${i + 1}`, recurringPayment: { id: `rp-${i}` } }) + }) + + const creationBatcher = makeBatcherMock( + agreementIds.map((_, i) => mockCreationOkResponse({ payment_id: `pay-${i + 1}`, reference: `trans-${i + 1}` })) ) - expect(sendPayment).toHaveBeenNthCalledWith( - 4, - expect.objectContaining({ ...expectedData, reference: 'transaction-id-4', agreement_id: agreementIds[3] }) + const statusBatcher = makeBatcherMock( + agreementIds.map((_, i) => mockStatusOkResponse(`pay-${i + 1}`, 'payment status success')) ) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + + await execute() + + expect(creationBatcher.addRequest).toHaveBeenCalledTimes(4) }) - it('logs an error for every failure', async () => { - jest.spyOn(console, 'error') - const errors = [new Error('error 1'), new Error('error 2'), new Error('error 3')] - salesApi.getDueRecurringPayments.mockReturnValueOnce([ - getMockDueRecurringPayment({ referenceNumber: 'fee', agreementId: 'a1' }), - getMockDueRecurringPayment({ referenceNumber: 'fi', agreementId: 'a2' }), - getMockDueRecurringPayment({ referenceNumber: 'foe', agreementId: 'a3' }) + it('skips retry response if it does not exist in batcher responses', async () => { + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + + // Only the 429 exists at position 0; no retry response at position N+0 + const creationBatcher = makeBatcherMock([mock429Response()]) + const statusBatcher = makeBatcherMock([]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + + await execute() + + expect(salesApi.createPaymentJournal).not.toHaveBeenCalled() + }) + + it('falls back to position-based metadata when body.reference is not in the transaction map', async () => { + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + salesApi.createTransaction.mockReturnValueOnce({ id: 'trans-1', cost: 30, recurringPayment: { id: 'rp-1' } }) + + // body.reference does not match 'trans-1' → falls back to positional metadata + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: 'pay-1', reference: 'unexpected-ref' })]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse('pay-1', PAYMENT_STATUS.Success)]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + + await execute() + + expect(salesApi.createPaymentJournal).toHaveBeenCalledWith('trans-1', expect.any(Object)) + }) + + it('skips 429 responses and processes the retry response', async () => { + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + + // index 0: 429 (first attempt), index 1 (= N+0 retry): ok response + const creationBatcher = makeBatcherMock([ + mock429Response(), + mockCreationOkResponse({ payment_id: 'pay-retry', reference: 'test-transaction-id' }) ]) - const permissionData = { licensee: { countryCode: 'GB-ENG' } } - salesApi.preparePermissionDataForRenewal - .mockRejectedValueOnce(errors[0]) - .mockReturnValueOnce(permissionData) - .mockReturnValueOnce(permissionData) - salesApi.createTransaction.mockRejectedValueOnce(errors[1]).mockReturnValueOnce({ cost: 50, id: 'transaction-id-3' }) - sendPayment.mockRejectedValueOnce(errors[2]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse('pay-retry', 'payment status success')]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() - expect(console.error).toHaveBeenCalledWith(expect.any(String), ...errors) + expect(salesApi.createPaymentJournal).toHaveBeenCalledWith( + 'test-transaction-id', + expect.objectContaining({ paymentReference: 'pay-retry' }) + ) }) + }) - describe('when the error is caused by an invalid agreementId', () => { - it('logs out the ids', async () => { - jest.spyOn(console, 'log') - salesApi.getDueRecurringPayments.mockReturnValueOnce(getMockPaymentRequestResponse()) - const oopsie = new Error('Invalid attribute value: agreement_id. Agreement does not exist') - sendPayment.mockRejectedValueOnce(oopsie) + describe('Payment status checks via batcher', () => { + it('calls addRequest for each payment with the correct status check URL', async () => { + const { statusBatcher, paymentId } = setupSinglePayment() - try { - await execute() - } catch (e) {} + await execute() - expect(console.log).toHaveBeenCalledWith( - '%s is an invalid agreementId. Recurring payment %s will be cancelled', - 'agreement-1', - 'recurring-payment-1' - ) - }) + expect(statusBatcher.addRequest).toHaveBeenCalledWith(`${GOV_PAY_API_URL}/${paymentId}`, expect.any(Object)) + }) - it('cancels the recurring payment', async () => { - salesApi.getDueRecurringPayments.mockReturnValueOnce(getMockPaymentRequestResponse()) - const oopsie = new Error('Invalid attribute value: agreement_id. Agreement does not exist') - sendPayment.mockRejectedValueOnce(oopsie) + it('calls addRequest for status check with correct method and headers', async () => { + const { statusBatcher } = setupSinglePayment() + + await execute() - try { - await execute() - } catch (e) {} + expect(statusBatcher.addRequest).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + method: 'get', + headers: expect.objectContaining({ + accept: 'application/json', + authorization: `Bearer ${GOV_PAY_RECURRING_APIKEY}`, + 'content-type': 'application/json' + }) + }) + ) + }) - expect(salesApi.cancelRecurringPayment).toHaveBeenCalledWith('recurring-payment-1') - }) + it('calls batcher.fetch() for status checks', async () => { + const { statusBatcher } = setupSinglePayment() + + await execute() + + expect(statusBatcher.fetch).toHaveBeenCalledTimes(1) }) - describe('when the error is caused by a reason other than invalid agreementId', () => { - it('does not try to cancel the recurring payment', async () => { - salesApi.getDueRecurringPayments.mockReturnValueOnce(getMockPaymentRequestResponse()) - const oopsie = new Error('The moon blew up without warning and for no apparent reason') - sendPayment.mockRejectedValueOnce(oopsie) + it('skips 429 responses during status check', async () => { + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + const paymentId = 'pay-1' + salesApi.createTransaction.mockReturnValueOnce({ id: 'trans-1', cost: 30, recurringPayment: { id: 'rp-1' } }) - try { - await execute() - } catch (e) { - expect(salesApi.cancelRecurringPayment).not.toHaveBeenCalledWith('recurring-payment-1') - } - }) + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: paymentId, reference: 'trans-1' })]) + const statusBatcher = makeBatcherMock([mock429Response(`${GOV_PAY_API_URL}/${paymentId}`)]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + + await execute() + + expect(salesApi.processRPResult).not.toHaveBeenCalled() + expect(salesApi.cancelRecurringPayment).not.toHaveBeenCalled() }) - }) - describe('When payment status request throws an error...', () => { - it('processRecurringPayments requests payment status for all payments, even if some throw errors', async () => { - const dueRecurringPayments = [] - for (let x = 0; x < 6; x++) { - dueRecurringPayments.push(getMockDueRecurringPayment()) - if ([1, 3].includes(x)) { - getPaymentStatus.mockRejectedValueOnce(new Error(`status failure ${x}`)) - } else { - getPaymentStatus.mockReturnValueOnce(getPaymentStatusSuccess()) - } - } + it('processes status for all payments, even if some responses indicate errors', async () => { + const dueRecurringPayments = Array.from({ length: 6 }, () => getMockDueRecurringPayment()) + salesApi.getDueRecurringPayments.mockReturnValueOnce(dueRecurringPayments) + salesApi.preparePermissionDataForRenewal.mockResolvedValue({ licensee: { countryCode: 'GB-ENG' } }) + + const paymentIds = dueRecurringPayments.map((_, i) => `pay-${i + 1}`) + const transactionIds = dueRecurringPayments.map((_, i) => `trans-${i + 1}`) + + transactionIds.forEach((id, i) => { + salesApi.createTransaction.mockReturnValueOnce({ id, cost: 30, recurringPayment: { id: `rp-${i}` } }) + }) + + const creationBatcher = makeBatcherMock( + paymentIds.map((pid, i) => mockCreationOkResponse({ payment_id: pid, reference: transactionIds[i] })) + ) + const statusBatcher = makeBatcherMock( + paymentIds.map((pid, i) => ([1, 3].includes(i) ? mockStatusErrorResponse(pid, 500) : mockStatusOkResponse(pid, 'payment status success'))) + ) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() - expect(getPaymentStatus).toHaveBeenCalledTimes(6) + // 4 successes → processRPResult called 4 times + expect(salesApi.processRPResult).toHaveBeenCalledTimes(4) }) }) it('prepares the data for found recurring payments', async () => { const referenceNumber = Symbol('reference') - salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment({ referenceNumber })]) - const mockPaymentResponse = { payment_id: 'test-payment-id', created_date: '2025-01-01T00:00:00.000Z' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) + setupSinglePayment({ referenceNumber }) await execute() @@ -388,6 +557,7 @@ describe('recurring-payments-processor', () => { it('creates a transaction with the correct data', async () => { const id = Symbol('recurring-payment-id') const agreementId = Symbol('agreement-id') + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment({ agreementId, id })]) const isLicenceForYou = Symbol('isLicenceForYou') @@ -400,68 +570,60 @@ describe('recurring-payments-processor', () => { salesApi.preparePermissionDataForRenewal.mockReturnValueOnce({ isLicenceForYou, isRenewal, - licensee: { - firstName, - lastName, - country, - countryCode: 'GB-ENG' - }, + licensee: { firstName, lastName, country, countryCode: 'GB-ENG' }, licenceStartDate: '2020-01-01', licenceStartTime: 3, permitId }) + const mockTransaction = { id: 'trans-1', cost: 30, recurringPayment: { id } } + salesApi.createTransaction.mockReturnValueOnce(mockTransaction) + + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ reference: 'trans-1' })]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse('pay-1', 'payment status success')]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + const expectedData = { dataSource: 'Recurring Payment', - recurringPayment: { - agreementId, - id - }, + recurringPayment: { agreementId, id }, permissions: [ { isLicenceForYou, isRenewal, issueDate: null, - licensee: { - firstName, - lastName, - country - }, + licensee: { firstName, lastName, country }, permitId, startDate: '2020-01-01T03:00:00.000Z' } ] } - const mockPaymentResponse = { payment_id: 'test-payment-id', agreementId: 'test-agreement-id' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) - await execute() expect(salesApi.createTransaction).toHaveBeenCalledWith(expectedData) }) it('creates a payment journal entry', async () => { + const paymentId = 'unique-payment-id-under-test' + const created_date = Symbol('created-date') + const transactionId = Symbol('transaction-id') + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) - const samplePayment = { - payment_id: Symbol('payment-id'), - created_date: Symbol('created-date') - } - const sampleTransaction = { - id: Symbol('transaction-id'), - cost: 99 - } - sendPayment.mockResolvedValueOnce(samplePayment) - salesApi.createTransaction.mockResolvedValueOnce(sampleTransaction) + salesApi.createTransaction.mockReturnValueOnce({ id: transactionId, cost: 99, recurringPayment: { id: 'rp-1' } }) + + const creationBatcher = makeBatcherMock([ + mockCreationOkResponse({ payment_id: paymentId, reference: transactionId, created_date }) + ]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse(paymentId, 'payment status success')]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() expect(salesApi.createPaymentJournal).toHaveBeenCalledWith( - sampleTransaction.id, + transactionId, expect.objectContaining({ - paymentReference: samplePayment.payment_id, - paymentTimestamp: samplePayment.created_date, + paymentReference: paymentId, + paymentTimestamp: created_date, paymentStatus: PAYMENT_JOURNAL_STATUS_CODES.InProgress }) ) @@ -471,21 +633,13 @@ describe('recurring-payments-processor', () => { salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) salesApi.preparePermissionDataForRenewal.mockReturnValueOnce({ - licensee: { - countryCode: 'GB-ENG' - }, - concessions: [ - { - id: 'abc-123', - name: 'concession-type-1', - proof: { type: 'NO-PROOF' } - } - ] + licensee: { countryCode: 'GB-ENG' }, + concessions: [{ id: 'abc-123', name: 'concession-type-1', proof: { type: 'NO-PROOF' } }] }) - const mockPaymentResponse = { payment_id: 'test-payment-id' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) + const creationBatcher = makeBatcherMock([mockCreationOkResponse()]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse('pay-1', 'payment status success')]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() @@ -493,11 +647,7 @@ describe('recurring-payments-processor', () => { expect.objectContaining({ permissions: expect.arrayContaining([ expect.objectContaining({ - concessions: expect.arrayContaining([ - expect.not.objectContaining({ - name: 'concession-type-1' - }) - ]) + concessions: expect.arrayContaining([expect.not.objectContaining({ name: 'concession-type-1' })]) }) ]) }) @@ -506,16 +656,15 @@ describe('recurring-payments-processor', () => { it('assigns the correct startDate when licenceStartTime is present', async () => { salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) - salesApi.preparePermissionDataForRenewal.mockReturnValueOnce({ licensee: { countryCode: 'GB-ENG' }, licenceStartDate: '2020-03-14', licenceStartTime: 15 }) - const mockPaymentResponse = { payment_id: 'test-payment-id' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) + const creationBatcher = makeBatcherMock([mockCreationOkResponse()]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse('pay-1', 'payment status success')]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() @@ -528,15 +677,14 @@ describe('recurring-payments-processor', () => { it('assigns the correct startDate when licenceStartTime is not present', async () => { salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) - salesApi.preparePermissionDataForRenewal.mockReturnValueOnce({ licensee: { countryCode: 'GB-ENG' }, licenceStartDate: '2020-03-14' }) - const mockPaymentResponse = { payment_id: 'test-payment-id' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) + const creationBatcher = makeBatcherMock([mockCreationOkResponse()]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse('pay-1', 'payment status success')]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() @@ -547,125 +695,159 @@ describe('recurring-payments-processor', () => { ) }) - it('prepares and sends the payment request', async () => { - const agreementId = Symbol('agreementId') - const transactionId = 'transactionId' + it('should log payment status for recurring payment', async () => { + const { paymentId } = setupSinglePayment({ paymentId: 'test-payment-id' }) - salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment({ referenceNumber: 'foo', agreementId: agreementId })]) + await execute() - salesApi.preparePermissionDataForRenewal.mockReturnValueOnce({ - licensee: { countryCode: 'GB-ENG' } - }) + expect(debugLogger).toHaveBeenCalledWith(`Payment status for ${paymentId}: ${PAYMENT_STATUS.Success}`) + }) - salesApi.createTransaction.mockReturnValueOnce({ - cost: 50, - id: transactionId + it('logs an error if createTransaction fails', async () => { + jest.spyOn(console, 'error') + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + const error = new Error('Wuh-oh!') + salesApi.createTransaction.mockImplementationOnce(() => { + throw error }) - const mockPaymentResponse = { payment_id: 'test-payment-id', agreementId } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) - - const expectedData = { - amount: 5000, - description: 'The recurring card payment for your rod fishing licence', - reference: transactionId, - authorisation_mode: 'agreement', - agreement_id: agreementId - } - await execute() - expect(sendPayment).toHaveBeenCalledWith(expectedData) + expect(console.error).toHaveBeenCalledWith(expect.any(String), error) }) - it('should call getPaymentStatus with payment id', async () => { - const mockResponse = [ - { - entity: { agreementId: 'agreement-1' }, - expanded: { - activePermission: { - entity: { - referenceNumber: 'ref-1' - } - } - } - } - ] - salesApi.getDueRecurringPayments.mockResolvedValueOnce(mockResponse) - salesApi.createTransaction.mockResolvedValueOnce({ - id: 'payment-id-1' + describe('When payment creation request results in an error response', () => { + it('logs an error for a non-ok, non-Agreement response', async () => { + jest.spyOn(console, 'error') + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + + const creationBatcher = makeBatcherMock([mockCreationErrorResponse({ status: 500, description: 'Internal Server Error' })]) + const statusBatcher = makeBatcherMock([]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + + await execute() + + expect(console.error).toHaveBeenCalledWith(expect.stringContaining('Unexpected response from GOV.UK Pay API')) }) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) - const mockPaymentResponse = { payment_id: 'test-payment-id', agreementId: 'agreement-1' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - await execute() + it('prepares and sends all payment requests, even if some fail', async () => { + const agreementIds = ['agr-1', 'agr-2', 'agr-3', 'agr-4'] + salesApi.getDueRecurringPayments.mockReturnValueOnce( + agreementIds.map((id, i) => getMockDueRecurringPayment({ agreementId: id, referenceNumber: `ref-${i}` })) + ) - expect(getPaymentStatus).toHaveBeenCalledWith('test-payment-id') - }) + const permissionData = { licensee: { countryCode: 'GB-ENG' } } + agreementIds.forEach((_, i) => { + salesApi.preparePermissionDataForRenewal.mockReturnValueOnce(permissionData) + salesApi.createTransaction.mockReturnValueOnce({ cost: 50, id: `trans-${i + 1}`, recurringPayment: { id: `rp-${i}` } }) + }) - it('should log payment status for recurring payment', async () => { - const mockPaymentId = 'test-payment-id' - const mockResponse = [ - { - entity: { agreementId: 'agreement-1' }, - expanded: { - activePermission: { - entity: { - referenceNumber: 'ref-1' - } - } - } - } - ] - salesApi.getDueRecurringPayments.mockResolvedValueOnce(mockResponse) - salesApi.createTransaction.mockResolvedValueOnce({ - id: mockPaymentId + const creationBatcher = makeBatcherMock([ + mockCreationOkResponse({ payment_id: 'pay-1', reference: 'trans-1' }), + mockCreationErrorResponse({ status: 500, description: 'Gateway down' }), + mockCreationOkResponse({ payment_id: 'pay-3', reference: 'trans-3' }), + mockCreationOkResponse({ payment_id: 'pay-4', reference: 'trans-4' }) + ]) + const statusBatcher = makeBatcherMock([ + mockStatusOkResponse('pay-1', 'payment status success'), + mockStatusOkResponse('pay-3', 'payment status success'), + mockStatusOkResponse('pay-4', 'payment status success') + ]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) + + await execute() + + expect(creationBatcher.addRequest).toHaveBeenCalledTimes(4) }) - const mockPaymentResponse = { payment_id: mockPaymentId, agreementId: 'agreement-1' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) - await execute() + it('logs errors for every failed transaction creation', async () => { + jest.spyOn(console, 'error') + const errors = [new Error('error 1'), new Error('error 2'), new Error('error 3')] + salesApi.getDueRecurringPayments.mockReturnValueOnce([ + getMockDueRecurringPayment({ referenceNumber: 'fee', agreementId: 'a1' }), + getMockDueRecurringPayment({ referenceNumber: 'fi', agreementId: 'a2' }), + getMockDueRecurringPayment({ referenceNumber: 'foe', agreementId: 'a3' }) + ]) + const permissionData = { licensee: { countryCode: 'GB-ENG' } } + salesApi.preparePermissionDataForRenewal + .mockRejectedValueOnce(errors[0]) + .mockReturnValueOnce(permissionData) + .mockReturnValueOnce(permissionData) + salesApi.createTransaction.mockRejectedValueOnce(errors[1]).mockRejectedValueOnce(errors[2]) - console.log(debugLogger.mock.calls) - expect(debugLogger).toHaveBeenCalledWith(`Payment status for ${mockPaymentId}: ${PAYMENT_STATUS.Success}`) - }) + await execute() - it('logs an error if createTransaction fails', async () => { - jest.spyOn(console, 'error') - salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) - const error = new Error('Wuh-oh!') - salesApi.createTransaction.mockImplementationOnce(() => { - throw error + expect(console.error).toHaveBeenCalledWith(expect.any(String), ...errors) }) - await execute() + describe('when the error is caused by an invalid agreementId', () => { + it('logs out the ids', async () => { + jest.spyOn(console, 'log') + salesApi.getDueRecurringPayments.mockReturnValueOnce(getMockPaymentRequestResponse()) - expect(console.error).toHaveBeenCalledWith(expect.any(String), error) - }) + const creationBatcher = makeBatcherMock([ + mockCreationErrorResponse({ + status: 422, + description: 'Invalid attribute value: agreement_id. Agreement does not exist' + }) + ]) + const statusBatcher = makeBatcherMock([]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) - // --- // + await execute() - it('should log errors from await salesApi.processRPResult', async () => { - salesApi.getDueRecurringPayments.mockResolvedValueOnce([getMockDueRecurringPayment()]) - salesApi.createTransaction.mockResolvedValueOnce({ id: 'trans-1', cost: 30 }) + expect(console.log).toHaveBeenCalledWith( + '%s is an invalid agreementId. Recurring payment %s will be cancelled', + 'agreement-1', + 'recurring-payment-1' + ) + }) + + it('cancels the recurring payment', async () => { + salesApi.getDueRecurringPayments.mockReturnValueOnce(getMockPaymentRequestResponse()) + + const creationBatcher = makeBatcherMock([ + mockCreationErrorResponse({ + status: 422, + description: 'Invalid attribute value: agreement_id. Agreement does not exist' + }) + ]) + const statusBatcher = makeBatcherMock([]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) - const payment = getMockSendPaymentResponse() - sendPayment.mockResolvedValueOnce(payment) + await execute() - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) + expect(salesApi.cancelRecurringPayment).toHaveBeenCalledWith('recurring-payment-1') + }) + }) - const boom = new Error('boom') + describe('when the error response is NOT caused by an invalid agreementId', () => { + it('does not try to cancel the recurring payment', async () => { + salesApi.getDueRecurringPayments.mockReturnValueOnce(getMockPaymentRequestResponse()) + + const creationBatcher = makeBatcherMock([ + mockCreationErrorResponse({ status: 500, description: 'The moon blew up without warning' }) + ]) + const statusBatcher = makeBatcherMock([]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) - salesApi.processRPResult.mockImplementation(transId => (transId === 'trans-1' ? Promise.reject(boom) : Promise.resolve())) + await execute() + + expect(salesApi.cancelRecurringPayment).not.toHaveBeenCalled() + }) + }) + }) + + it('should log errors from await salesApi.processRPResult', async () => { + const { paymentId, transactionId } = setupSinglePayment({ transactionId: 'trans-1', paymentId: 'pay-1' }) + const boom = new Error('boom') + salesApi.processRPResult.mockImplementation(transId => (transId === transactionId ? Promise.reject(boom) : Promise.resolve())) const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}) await execute() - expect(errorSpy).toHaveBeenCalledWith('Failed to process Recurring Payment for trans-1', boom) + expect(errorSpy).toHaveBeenCalledWith(`Failed to process Recurring Payment for ${transactionId}`, boom) errorSpy.mockRestore() }) @@ -673,51 +855,38 @@ describe('recurring-payments-processor', () => { describe('handling failures for multiple due payments', () => { beforeEach(() => { salesApi.getDueRecurringPayments.mockResolvedValueOnce([getMockDueRecurringPayment(), getMockDueRecurringPayment()]) - salesApi.preparePermissionDataForRenewal.mockResolvedValueOnce({ licensee: { countryCode: 'GB-ENG' } }) - - salesApi.createTransaction.mockResolvedValueOnce({ id: 'trans-1', cost: 30 }).mockResolvedValueOnce({ id: 'trans-2', cost: 30 }) + salesApi.createTransaction + .mockResolvedValueOnce({ id: 'trans-1', cost: 30, recurringPayment: { id: 'rp-1' } }) + .mockResolvedValueOnce({ id: 'trans-2', cost: 30, recurringPayment: { id: 'rp-2' } }) }) - it('continues when one sendPayment rejects (Promise.allSettled check)', async () => { - const secondPayment = getMockSendPaymentResponse({ - payment_id: 'test-payment-second', - agreementId: 'agr-2', - created_date: '2025-01-01T00:00:00.000Z' - }) + it('continues when one creation batcher response is an error', async () => { + const creationBatcher = makeBatcherMock([ + mockCreationErrorResponse({ status: 500, description: 'gateway down' }), + mockCreationOkResponse({ payment_id: 'pay-2', reference: 'trans-2', created_date: '2025-01-01T00:01:00.000Z' }) + ]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse('pay-2', 'payment status success')]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) - const gatewayDown = new Error('gateway down') - sendPayment.mockRejectedValueOnce(gatewayDown).mockResolvedValueOnce(secondPayment) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) salesApi.processRPResult.mockResolvedValueOnce() await execute() - const summary = { - statusArgs: getPaymentStatus.mock.calls, - rpResultArgs: salesApi.processRPResult.mock.calls - } - - expect(summary).toEqual({ - statusArgs: [[secondPayment.payment_id]], - rpResultArgs: [['trans-2', secondPayment.payment_id, secondPayment.created_date]] - }) + expect(salesApi.processRPResult).toHaveBeenCalledWith('trans-2', 'pay-2', '2025-01-01T00:01:00.000Z') + expect(salesApi.processRPResult).toHaveBeenCalledTimes(1) }) it('continues when processRPResult rejects for one payment', async () => { - const firstPayment = getMockSendPaymentResponse({ - payment_id: 'pay-1', - agreementId: 'agr-1', - created_date: '2025-01-01T00:00:00.000Z' - }) - const secondPayment = getMockSendPaymentResponse({ - payment_id: 'pay-2', - agreementId: 'agr-2', - created_date: '2025-01-01T00:01:00.000Z' - }) - - sendPayment.mockResolvedValueOnce(firstPayment).mockResolvedValueOnce(secondPayment) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()).mockResolvedValueOnce(getPaymentStatusSuccess()) + const creationBatcher = makeBatcherMock([ + mockCreationOkResponse({ payment_id: 'pay-1', reference: 'trans-1', created_date: '2025-01-01T00:00:00.000Z' }), + mockCreationOkResponse({ payment_id: 'pay-2', reference: 'trans-2', created_date: '2025-01-01T00:01:00.000Z' }) + ]) + const statusBatcher = makeBatcherMock([ + mockStatusOkResponse('pay-1', 'payment status success'), + mockStatusOkResponse('pay-2', 'payment status success') + ]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) const boom = new Error('boom') salesApi.processRPResult.mockImplementation(transId => (transId === 'trans-1' ? Promise.reject(boom) : Promise.resolve())) @@ -726,59 +895,32 @@ describe('recurring-payments-processor', () => { await execute() - const summary = { - rpResultArgs: salesApi.processRPResult.mock.calls, - rpCount: salesApi.processRPResult.mock.calls.length, - firstError: errorSpy.mock.calls[0] - } + expect(salesApi.processRPResult).toHaveBeenCalledTimes(2) + expect(errorSpy).toHaveBeenCalledWith('Failed to process Recurring Payment for trans-1', boom) errorSpy.mockRestore() - - expect(summary).toEqual({ - rpResultArgs: expect.arrayContaining([ - ['trans-1', firstPayment.payment_id, firstPayment.created_date], - ['trans-2', secondPayment.payment_id, secondPayment.created_date] - ]), - rpCount: 2, - firstError: ['Failed to process Recurring Payment for trans-1', boom] - }) }) - it('does not abort when getPaymentStatus rejects for one payment (allSettled at status stage)', async () => { - const p1 = getMockSendPaymentResponse({ payment_id: 'pay-1', created_date: '2025-01-01T00:00:00.000Z' }) - const p2 = getMockSendPaymentResponse({ payment_id: 'pay-2', created_date: '2025-01-01T00:01:00.000Z' }) - - sendPayment.mockResolvedValueOnce(p1).mockResolvedValueOnce(p2) - - getPaymentStatus.mockImplementation(async id => { - if (id === p1.payment_id) { - throw Object.assign(new Error('HTTP 500'), { response: { status: 500, data: 'boom' } }) - } - return getPaymentStatusSuccess() - }) + it('does not abort when a status response is non-ok for one payment', async () => { + const creationBatcher = makeBatcherMock([ + mockCreationOkResponse({ payment_id: 'pay-1', reference: 'trans-1', created_date: '2025-01-01T00:00:00.000Z' }), + mockCreationOkResponse({ payment_id: 'pay-2', reference: 'trans-2', created_date: '2025-01-01T00:01:00.000Z' }) + ]) + const statusBatcher = makeBatcherMock([ + mockStatusErrorResponse('pay-1', 500), + mockStatusOkResponse('pay-2', 'payment status success') + ]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) salesApi.processRPResult.mockResolvedValueOnce() await execute() - const summary = { - statusArgs: getPaymentStatus.mock.calls, - statusCount: getPaymentStatus.mock.calls.length, - rpResultArgs: salesApi.processRPResult.mock.calls, - rpCount: salesApi.processRPResult.mock.calls.length - } - - expect(summary).toEqual({ - statusArgs: expect.arrayContaining([[p1.payment_id], [p2.payment_id]]), - statusCount: 2, - rpResultArgs: expect.arrayContaining([['trans-2', p2.payment_id, p2.created_date]]), - rpCount: 1 - }) + expect(salesApi.processRPResult).toHaveBeenCalledWith('trans-2', 'pay-2', '2025-01-01T00:01:00.000Z') + expect(salesApi.processRPResult).toHaveBeenCalledTimes(1) }) }) - // --- // - it.each([ [400, 'Failed to fetch status for payment test-payment-id, error 400'], [486, 'Failed to fetch status for payment test-payment-id, error 486'], @@ -786,54 +928,47 @@ describe('recurring-payments-processor', () => { [500, 'Payment status API error for test-payment-id, error 500'], [512, 'Payment status API error for test-payment-id, error 512'], [599, 'Payment status API error for test-payment-id, error 599'] - ])('logs the correct message when getPaymentStatus rejects with HTTP %i', async (statusCode, expectedMessage) => { + ])('logs the correct message when status response is HTTP %i', async (statusCode, expectedMessage) => { jest.spyOn(console, 'error') - const mockPaymentId = 'test-payment-id' - const mockResponse = [ - { entity: { agreementId: 'agreement-1' }, expanded: { activePermission: { entity: { referenceNumber: 'ref-1' } } } } - ] - salesApi.getDueRecurringPayments.mockResolvedValueOnce(mockResponse) - salesApi.createTransaction.mockResolvedValueOnce({ id: mockPaymentId }) - sendPayment.mockResolvedValueOnce({ - payment_id: mockPaymentId, - agreementId: 'agreement-1', - created_date: '2025-04-30T12:00:00Z' - }) + const paymentId = 'test-payment-id' + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + salesApi.createTransaction.mockReturnValueOnce({ id: 'trans-1', cost: 30, recurringPayment: { id: 'rp-1' } }) - const apiError = { response: { status: statusCode, data: 'boom' } } - getPaymentStatus.mockRejectedValueOnce(apiError) + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: paymentId, reference: 'trans-1' })]) + const statusBatcher = makeBatcherMock([mockStatusErrorResponse(paymentId, statusCode)]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() expect(console.error).toHaveBeenCalledWith(expectedMessage) }) - it('logs the generic unexpected-error message and still rejects', async () => { + it('logs the generic unexpected-error message when status response throws unexpectedly', async () => { jest.spyOn(console, 'error') - const mockPaymentId = 'test-payment-id' - salesApi.getDueRecurringPayments.mockResolvedValueOnce(getMockPaymentRequestResponse()) - salesApi.createTransaction.mockResolvedValueOnce({ id: mockPaymentId }) - sendPayment.mockResolvedValueOnce({ - payment_id: mockPaymentId, - agreementId: 'agreement-1', - created_date: '2025-04-30T12:00:00.000Z' - }) + const paymentId = 'test-payment-id' + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + salesApi.createTransaction.mockReturnValueOnce({ id: 'trans-1', cost: 30, recurringPayment: { id: 'rp-1' } }) + + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: paymentId, reference: 'trans-1' })]) - const networkError = new Error('network meltdown') - getPaymentStatus.mockRejectedValueOnce(networkError) + // Status response's json() throws unexpectedly (no response.status on error) + const statusBatcher = makeBatcherMock([ + { + ok: true, + status: 200, + url: `${GOV_PAY_API_URL}/${paymentId}`, + json: jest.fn().mockRejectedValue(new Error('network meltdown')) + } + ]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() - expect(console.error).toHaveBeenCalledWith(`Unexpected error fetching payment status for ${mockPaymentId}.`) + expect(console.error).toHaveBeenCalledWith(`Unexpected error fetching payment status for ${paymentId}.`) }) it('should call setTimeout with correct delay when there are recurring payments', async () => { - const referenceNumber = Symbol('reference') - salesApi.getDueRecurringPayments.mockResolvedValueOnce([getMockDueRecurringPayment({ referenceNumber })]) - const mockPaymentResponse = { payment_id: 'test-payment-id' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) - + setupSinglePayment() const setTimeoutSpy = jest.spyOn(global, 'setTimeout').mockImplementation(cb => cb()) await execute() @@ -843,7 +978,6 @@ describe('recurring-payments-processor', () => { it('should not call setTimeout when there are no recurring payments', async () => { salesApi.getDueRecurringPayments.mockResolvedValueOnce([]) - const setTimeoutSpy = jest.spyOn(global, 'setTimeout').mockImplementation(cb => cb()) await execute() @@ -852,29 +986,19 @@ describe('recurring-payments-processor', () => { }) it('calls processRPResult with transaction id, payment id and created date when payment is successful', async () => { - debugLogger.mockImplementation(function () { - console.log(...arguments) + const { paymentId, transactionId, created_date } = setupSinglePayment({ + transactionId: 'test-transaction-id', + paymentId: 'test-payment-id', + created_date: '2025-01-01T00:00:00.000Z' }) - const mockTransactionId = 'test-transaction-id' - const mockPaymentId = 'test-payment-id' - const mockPaymentCreatedDate = '2025-01-01T00:00:00.000Z' - salesApi.getDueRecurringPayments.mockResolvedValueOnce(getMockPaymentRequestResponse()) - salesApi.createTransaction.mockResolvedValueOnce({ id: mockTransactionId, cost: 30 }) - sendPayment.mockResolvedValueOnce({ payment_id: mockPaymentId, agreementId: 'agreement-1', created_date: mockPaymentCreatedDate }) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusSuccess()) await execute() - console.log(salesApi.processRPResult.mock.calls, mockTransactionId, mockPaymentId, mockPaymentCreatedDate) - expect(salesApi.processRPResult).toHaveBeenCalledWith(mockTransactionId, mockPaymentId, mockPaymentCreatedDate) + expect(salesApi.processRPResult).toHaveBeenCalledWith(transactionId, paymentId, created_date) }) it("doesn't call processRPResult if payment status is not successful", async () => { - const mockPaymentId = 'test-payment-id' - salesApi.getDueRecurringPayments.mockResolvedValueOnce(getMockPaymentRequestResponse()) - salesApi.createTransaction.mockResolvedValueOnce({ id: mockPaymentId, cost: 30 }) - sendPayment.mockResolvedValueOnce({ payment_id: mockPaymentId, agreementId: 'agreement-1' }) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusFailure()) + setupSinglePayment({ paymentStatus: 'payment status failure' }) await execute() @@ -893,9 +1017,17 @@ describe('recurring-payments-processor', () => { async (agreementId, mockStatus, status) => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(jest.fn()) salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment({ agreementId })]) - const mockPaymentResponse = { payment_id: 'test-payment-id', created_date: '2025-01-01T00:00:00.000Z' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(mockStatus) + + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: 'pay-1', reference: 'test-transaction-id' })]) + const statusBatcher = makeBatcherMock([ + { + ok: true, + status: 200, + url: `${GOV_PAY_API_URL}/pay-1`, + json: jest.fn().mockResolvedValue(mockStatus) + } + ]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() @@ -906,214 +1038,93 @@ describe('recurring-payments-processor', () => { ) it.each([ - ['a failure', 'agreement-id', getPaymentStatusFailure()], - ['a failure', 'test-agreement-id', getPaymentStatusFailure()], - ['a failure', 'another-agreement-id', getPaymentStatusFailure()], - ['an error', 'agreement-id', getPaymentStatusError()], - ['an error', 'test-agreement-id', getPaymentStatusError()], - ['an error', 'another-agreement-id', getPaymentStatusError()] - ])('cancelRecurringPayment is called when payment is %s', async (_status, agreementId, mockStatus) => { + ['agreement-id', getPaymentStatusFailure()], + ['test-agreement-id', getPaymentStatusError()] + ])('updates payment journal to Failed when payment agreement %s has a failure/error status', async (agreementId, mockStatus) => { salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment({ agreementId })]) - const id = Symbol('recurring-payment-id') - salesApi.createTransaction.mockResolvedValueOnce({ - recurringPayment: { - id - } - }) - const mockPaymentResponse = { payment_id: 'test-payment-id', created_date: '2025-01-01T00:00:00.000Z' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(mockStatus) + salesApi.getPaymentJournal.mockResolvedValueOnce({ id: 'journal-1' }) + + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: 'pay-1', reference: 'test-transaction-id' })]) + const statusBatcher = makeBatcherMock([ + { ok: true, status: 200, url: `${GOV_PAY_API_URL}/pay-1`, json: jest.fn().mockResolvedValue(mockStatus) } + ]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() - expect(salesApi.cancelRecurringPayment).toHaveBeenCalledWith(id) + expect(salesApi.updatePaymentJournal).toHaveBeenCalledWith( + 'test-transaction-id', + expect.objectContaining({ paymentStatus: PAYMENT_JOURNAL_STATUS_CODES.Failed }) + ) }) - it('updatePaymentJournal is called with transaction id and failed status code payment is not succesful and payment journal exists', async () => { - salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) - const transactionId = 'test-transaction-id' - salesApi.createTransaction.mockReturnValueOnce({ - cost: 50, - id: transactionId - }) - const mockPaymentResponse = { payment_id: 'test-payment-id', created_date: '2025-01-01T00:00:00.000Z' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusFailure()) - salesApi.getPaymentJournal.mockResolvedValueOnce(true) + it.each([ + ['agreement-id', getPaymentStatusFailure()], + ['test-agreement-id', getPaymentStatusError()] + ])('cancels the recurring payment when agreement %s has a failure/error status', async (agreementId, mockStatus) => { + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment({ agreementId })]) + salesApi.getPaymentJournal.mockResolvedValueOnce({ id: 'journal-1' }) + + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: 'pay-1', reference: 'test-transaction-id' })]) + const statusBatcher = makeBatcherMock([ + { ok: true, status: 200, url: `${GOV_PAY_API_URL}/pay-1`, json: jest.fn().mockResolvedValue(mockStatus) } + ]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() - expect(salesApi.updatePaymentJournal).toHaveBeenCalledWith(transactionId, { paymentStatus: PAYMENT_JOURNAL_STATUS_CODES.Failed }) + expect(salesApi.cancelRecurringPayment).toHaveBeenCalledWith('recurring-payment-1') }) - it('updatePaymentJournal is not called when failed status code payment is not succesful but payment journal does not exist', async () => { + it('logs an error if a status response URL contains an unknown paymentId', async () => { + jest.spyOn(console, 'error') salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) - const transactionId = 'test-transaction-id' - salesApi.createTransaction.mockReturnValueOnce({ - cost: 50, - id: transactionId - }) - const mockPaymentResponse = { payment_id: 'test-payment-id', created_date: '2025-01-01T00:00:00.000Z' } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - getPaymentStatus.mockResolvedValueOnce(getPaymentStatusFailure()) - salesApi.getPaymentJournal.mockResolvedValueOnce(undefined) + salesApi.createTransaction.mockReturnValueOnce({ id: 'trans-1', cost: 30, recurringPayment: { id: 'rp-1' } }) + + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: 'pay-1', reference: 'trans-1' })]) + const statusBatcher = makeBatcherMock([ + { ok: true, status: 200, url: `${GOV_PAY_API_URL}/pay-unknown`, json: jest.fn().mockResolvedValue({ state: { status: 'success' } }) } + ]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() - expect(salesApi.updatePaymentJournal).not.toHaveBeenCalled() + expect(console.error).toHaveBeenCalledWith('Could not find payment data for paymentId: pay-unknown') }) - describe.each([2, 3, 10])('if there are %d recurring payments', count => { - it('prepares the data for each one', async () => { - const references = [] - for (let i = 0; i < count; i++) { - references.push(Symbol('reference' + i)) - } - const mockGetDueRecurringPayments = [] - references.forEach(referenceNumber => { - mockGetDueRecurringPayments.push(getMockDueRecurringPayment({ referenceNumber })) - }) - salesApi.getDueRecurringPayments.mockReturnValueOnce(mockGetDueRecurringPayments) - const mockPaymentResponse = { payment_id: 'test-payment-id' } - sendPayment.mockResolvedValue(mockPaymentResponse) - const mockPaymentStatus = getPaymentStatusSuccess() - getPaymentStatus.mockResolvedValue(mockPaymentStatus) - - const expectedData = [] - references.forEach(reference => { - expectedData.push([reference]) - }) - - await execute() - - expect(salesApi.preparePermissionDataForRenewal.mock.calls).toEqual(expectedData) - }) - - it('creates a transaction for each one', async () => { - const mockGetDueRecurringPayments = [] - const agreementIds = [] - const ids = [] - for (let i = 0; i < count; i++) { - const agreementId = Symbol(`agreement-id-${i}`) - const id = Symbol(`recurring-payment-${i}`) - agreementIds.push(agreementId) - ids.push(id) - mockGetDueRecurringPayments.push(getMockDueRecurringPayment({ agreementId, id, referenceNumber: i })) - } - salesApi.getDueRecurringPayments.mockReturnValueOnce(mockGetDueRecurringPayments) - - const permits = [] - for (let i = 0; i < count; i++) { - permits.push(Symbol(`permit${i}`)) - } - - permits.forEach(permit => { - salesApi.preparePermissionDataForRenewal.mockReturnValueOnce({ - licensee: { countryCode: 'GB-ENG' }, - permitId: permit - }) - }) - - const expectedData = [] - permits.forEach((permit, i) => { - expectedData.push([ - { - dataSource: 'Recurring Payment', - recurringPayment: { - agreementId: agreementIds[i], - id: ids[i] - }, - permissions: [expect.objectContaining({ permitId: permit })] - } - ]) - }) - - await execute() - - expect(salesApi.createTransaction.mock.calls).toEqual(expectedData) - }) - - it('sends a payment for each one', async () => { - const mockGetDueRecurringPayments = [] - const agreementIds = [] - for (let i = 0; i < count; i++) { - const agreementId = Symbol(`agreementId${1}`) - agreementIds.push(agreementId) - mockGetDueRecurringPayments.push(getMockDueRecurringPayment({ agreementId })) - } - salesApi.getDueRecurringPayments.mockReturnValueOnce(mockGetDueRecurringPayments) - - const permits = [] - for (let i = 0; i < count; i++) { - permits.push(Symbol(`permit${i}`)) - } - - permits.forEach((permit, i) => { - salesApi.preparePermissionDataForRenewal.mockReturnValueOnce({ - licensee: { countryCode: 'GB-ENG' } - }) - - salesApi.createTransaction.mockReturnValueOnce({ - cost: i, - id: permit - }) - }) + it('logs a generic error when a status response has an unexpected HTTP status (not 4xx or 5xx)', async () => { + jest.spyOn(console, 'error') + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + salesApi.createTransaction.mockReturnValueOnce({ id: 'trans-1', cost: 30, recurringPayment: { id: 'rp-1' } }) - const expectedData = [] - permits.forEach((permit, i) => { - expectedData.push([ - { - amount: i * 100, - description: 'The recurring card payment for your rod fishing licence', - reference: permit, - authorisation_mode: 'agreement', - agreement_id: agreementIds[i] - } - ]) - }) + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: 'pay-1', reference: 'trans-1' })]) + const statusBatcher = makeBatcherMock([{ ok: false, status: 304, url: `${GOV_PAY_API_URL}/pay-1`, json: jest.fn() }]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) - await execute() - expect(sendPayment.mock.calls).toEqual(expectedData) - }) + await execute() - it('gets the payment status for each one', async () => { - const mockGetDueRecurringPayments = [] - const agreementIds = [] - for (let i = 0; i < count; i++) { - const agreementId = Symbol(`agreementId${1}`) - agreementIds.push(agreementId) - mockGetDueRecurringPayments.push(getMockDueRecurringPayment({ agreementId })) - } - salesApi.getDueRecurringPayments.mockReturnValueOnce(mockGetDueRecurringPayments) + expect(console.error).toHaveBeenCalledWith('Unexpected error fetching payment status for pay-1.') + }) - const permits = [] - for (let i = 0; i < count; i++) { - permits.push(Symbol(`permit${i}`)) - } + it.each([ + [400, 'Failed to fetch status for payment test-payment-id, error 400'], + [500, 'Payment status API error for test-payment-id, error 500'] + ])('logs correct error when processRPResult throws with error.response.status %i', async (responseStatus, expectedMessage) => { + jest.spyOn(console, 'error') + const paymentId = 'test-payment-id' + salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) + salesApi.createTransaction.mockReturnValueOnce({ id: 'trans-1', cost: 30, recurringPayment: { id: 'rp-1' } }) - permits.forEach((permit, i) => { - salesApi.preparePermissionDataForRenewal.mockReturnValueOnce({ - licensee: { countryCode: 'GB-ENG' } - }) + const err = new Error('API error') + err.response = { status: responseStatus } + salesApi.processRPResult.mockRejectedValueOnce(err) - salesApi.createTransaction.mockReturnValueOnce({ - cost: i, - id: permit - }) - }) + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: paymentId, reference: 'trans-1' })]) + const statusBatcher = makeBatcherMock([mockStatusOkResponse(paymentId, PAYMENT_STATUS.Success)]) + HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) - const expectedData = [] - permits.forEach((_, index) => { - const paymentId = `payment-id-${index}` - expectedData.push(paymentId) - const mockPaymentResponse = { payment_id: paymentId } - sendPayment.mockResolvedValueOnce(mockPaymentResponse) - }) + await execute() - await execute() - expectedData.forEach(paymentId => { - expect(getPaymentStatus).toHaveBeenCalledWith(paymentId) - }) - }) + expect(console.error).toHaveBeenCalledWith(expectedMessage) }) }) diff --git a/packages/recurring-payments-job/src/recurring-payments-processor.js b/packages/recurring-payments-job/src/recurring-payments-processor.js index 57133229f0..e38903ae5a 100644 --- a/packages/recurring-payments-job/src/recurring-payments-processor.js +++ b/packages/recurring-payments-job/src/recurring-payments-processor.js @@ -1,7 +1,7 @@ import moment from 'moment-timezone' import { PAYMENT_STATUS, SERVICE_LOCAL_TIME, PAYMENT_JOURNAL_STATUS_CODES } from '@defra-fish/business-rules-lib' -import { salesApi, airbrake } from '@defra-fish/connectors-lib' -import { getPaymentStatus, sendPayment, isGovPayUp } from './services/govuk-pay-service.js' +import { salesApi, airbrake, HTTPRequestBatcher } from '@defra-fish/connectors-lib' +import { isGovPayUp } from './services/govuk-pay-service.js' import db from 'debug' const debug = db('recurring-payments:processor') @@ -9,6 +9,8 @@ const debug = db('recurring-payments:processor') const SIGINT_CODE = 130 const SIGTERM_CODE = 137 const PAYMENT_STATUS_DELAY = 60000 +const GOV_PAY_REQUEST_TIMEOUT_MS_DEFAULT = 10000 +const INVALID_AGREEMENT_MESSAGE = 'Agreement does not exist' const MIN_CLIENT_ERROR = 400 const MAX_CLIENT_ERROR = 499 const MIN_SERVER_ERROR = 500 @@ -17,6 +19,18 @@ const MAX_SERVER_ERROR = 599 const isClientError = code => code >= MIN_CLIENT_ERROR && code <= MAX_CLIENT_ERROR const isServerError = code => code >= MIN_SERVER_ERROR && code <= MAX_SERVER_ERROR +const govPayRecurringHeaders = () => ({ + accept: 'application/json', + authorization: `Bearer ${process.env.GOV_PAY_RECURRING_APIKEY}`, + 'content-type': 'application/json' +}) + +const createBatcher = () => + new HTTPRequestBatcher({ + batchSize: process.env.RCP_BATCHER_BATCH_SIZE ? Number(process.env.RCP_BATCHER_BATCH_SIZE) : undefined, + delay: process.env.RCP_BATCHER_DELAY_MS ? Number(process.env.RCP_BATCHER_DELAY_MS) : undefined + }) + export const execute = async () => { airbrake.initialise() try { @@ -51,7 +65,7 @@ const processRecurringPayments = async () => { await new Promise(resolve => setTimeout(resolve, PAYMENT_STATUS_DELAY)) - await Promise.allSettled(payments.map(p => processRecurringPaymentStatus(p))) + await processAllPaymentStatuses(payments) } const fetchDueRecurringPayments = async date => { @@ -65,56 +79,105 @@ const fetchDueRecurringPayments = async date => { } } +const createNewTransaction = async (referenceNumber, recurringPayment) => { + const transactionData = await processPermissionData(referenceNumber, recurringPayment) + return salesApi.createTransaction(transactionData) +} + const requestPayments = async dueRCPayments => { - const paymentRequestResults = await Promise.allSettled(dueRCPayments.map(processRecurringPayment)) - const payments = paymentRequestResults.filter(prr => prr.status === 'fulfilled').map(p => p.value) - const failures = paymentRequestResults.filter(prr => prr.status === 'rejected').map(f => f.reason) + const transactionResults = await Promise.allSettled( + dueRCPayments.map(async record => { + const referenceNumber = record.expanded.activePermission.entity.referenceNumber + const { agreementId, id } = record.entity + const transaction = await createNewTransaction(referenceNumber, { agreementId, id }) + return { agreementId, transaction } + }) + ) + + const failures = transactionResults.filter(r => r.status === 'rejected').map(f => f.reason) if (failures.length) { console.error('Error requesting payments:', ...failures) } - return payments -} -const processRecurringPayment = async record => { - const referenceNumber = record.expanded.activePermission.entity.referenceNumber - const { agreementId, id } = record.entity - const transaction = await createNewTransaction(referenceNumber, { agreementId, id }) - return takeRecurringPayment(agreementId, transaction) -} + const validTransactions = transactionResults.filter(r => r.status === 'fulfilled').map(r => r.value) + if (!validTransactions.length) { + return [] + } -const createNewTransaction = async (referenceNumber, recurringPayment) => { - const transactionData = await processPermissionData(referenceNumber, recurringPayment) - return salesApi.createTransaction(transactionData) + return batchCreatePayments(validTransactions) } -const takeRecurringPayment = async (agreementId, transaction) => { - const preparedPayment = preparePayment(agreementId, transaction) - const payment = await takePaymentIfValid(preparedPayment, agreementId, transaction) +const batchCreatePayments = async validTransactions => { + const batcher = createBatcher() + const transactionMap = new Map() + const requestsMetadata = [] - await salesApi.createPaymentJournal(transaction.id, { - paymentReference: payment.payment_id, - paymentTimestamp: payment.created_date, - paymentStatus: PAYMENT_JOURNAL_STATUS_CODES.InProgress - }) + for (const { agreementId, transaction } of validTransactions) { + const preparedPayment = preparePayment(agreementId, transaction) + batcher.addRequest(process.env.GOV_PAY_API_URL, { + headers: govPayRecurringHeaders(), + method: 'post', + body: JSON.stringify(preparedPayment), + timeout: process.env.GOV_PAY_REQUEST_TIMEOUT_MS || GOV_PAY_REQUEST_TIMEOUT_MS_DEFAULT + }) + transactionMap.set(transaction.id, { agreementId, transaction }) + requestsMetadata.push({ agreementId, transaction }) + } - return { - agreementId, - paymentId: payment.payment_id, - created_date: payment.created_date, - transaction + await batcher.fetch() + + const retriedIndices = [] + const payments = [] + + for (let i = 0; i < requestsMetadata.length; i++) { + const response = batcher.responses[i] + if (response.status === 429) { + retriedIndices.push(i) + continue + } + await processPaymentCreationResponse(response, requestsMetadata[i], transactionMap, payments) + } + + for (let j = 0; j < retriedIndices.length; j++) { + const response = batcher.responses[requestsMetadata.length + j] + if (response) { + await processPaymentCreationResponse(response, requestsMetadata[retriedIndices[j]], transactionMap, payments) + } } + + return payments } -const takePaymentIfValid = async (preparedPayment, agreementId, transaction) => { - try { - return await sendPayment(preparedPayment) - } catch (error) { - if (error.message.includes('Invalid attribute value: agreement_id. Agreement does not exist')) { - console.log('%s is an invalid agreementId. Recurring payment %s will be cancelled', agreementId, transaction.recurringPayment.id) - await salesApi.cancelRecurringPayment(transaction.recurringPayment.id) +const processPaymentCreationResponse = async (response, metadata, transactionMap, payments) => { + const { agreementId, transaction } = metadata + const body = await response.json() + + if (!response.ok) { + if (body.description?.includes(INVALID_AGREEMENT_MESSAGE)) { + console.log('%s is an invalid agreementId. Recurring payment %s will be cancelled', agreementId, transaction.recurringPayment?.id) + await salesApi.cancelRecurringPayment(transaction.recurringPayment?.id) + } else { + console.error( + `Unexpected response from GOV.UK Pay API. Status: ${response.status}, Response: ${JSON.stringify(body)}, Transaction ID: ${transaction.id}` + ) } - throw error + return } + + const correlatedMetadata = transactionMap.get(body.reference) ?? metadata + + await salesApi.createPaymentJournal(correlatedMetadata.transaction.id, { + paymentReference: body.payment_id, + paymentTimestamp: body.created_date, + paymentStatus: PAYMENT_JOURNAL_STATUS_CODES.InProgress + }) + + payments.push({ + agreementId: correlatedMetadata.agreementId, + paymentId: body.payment_id, + created_date: body.created_date, + transaction: correlatedMetadata.transaction + }) } const processPermissionData = async (referenceNumber, recurringPayment) => { @@ -159,17 +222,57 @@ const preparePayment = (agreementId, transaction) => { return result } -const processRecurringPaymentStatus = async payment => { +const processAllPaymentStatuses = async payments => { + const batcher = createBatcher() + const paymentMap = new Map() + + for (const payment of payments) { + batcher.addRequest(`${process.env.GOV_PAY_API_URL}/${payment.paymentId}`, { + headers: govPayRecurringHeaders(), + method: 'get', + timeout: process.env.GOV_PAY_REQUEST_TIMEOUT_MS || GOV_PAY_REQUEST_TIMEOUT_MS_DEFAULT + }) + paymentMap.set(payment.paymentId, payment) + } + + await batcher.fetch() + + await Promise.allSettled(batcher.responses.map(response => processPaymentStatusResponse(response, paymentMap))) +} + +const processPaymentStatusResponse = async (response, paymentMap) => { + if (response.status === 429) { + return + } + + const paymentId = response.url.split('/').pop() + const payment = paymentMap.get(paymentId) + + if (!payment) { + console.error(`Could not find payment data for paymentId: ${paymentId}`) + return + } + try { - const { - state: { status } - } = await getPaymentStatus(payment.paymentId) + if (!response.ok) { + const status = response.status + if (isClientError(status)) { + console.error(`Failed to fetch status for payment ${paymentId}, error ${status}`) + } else if (isServerError(status)) { + console.error(`Payment status API error for ${paymentId}, error ${status}`) + } else { + console.error(`Unexpected error fetching payment status for ${paymentId}.`) + } + return + } + + const { state: { status } } = await response.json() - debug(`Payment status for ${payment.paymentId}: ${status}`) + debug(`Payment status for ${paymentId}: ${status}`) if (status === PAYMENT_STATUS.Success) { try { - await salesApi.processRPResult(payment.transaction.id, payment.paymentId, payment.created_date) + await salesApi.processRPResult(payment.transaction.id, paymentId, payment.created_date) debug(`Processed Recurring Payment for ${payment.transaction.id}`) } catch (err) { console.error(`Failed to process Recurring Payment for ${payment.transaction.id}`, err) @@ -191,11 +294,11 @@ const processRecurringPaymentStatus = async payment => { const status = error.response?.status if (isClientError(status)) { - console.error(`Failed to fetch status for payment ${payment.paymentId}, error ${status}`) + console.error(`Failed to fetch status for payment ${paymentId}, error ${status}`) } else if (isServerError(status)) { - console.error(`Payment status API error for ${payment.paymentId}, error ${status}`) + console.error(`Payment status API error for ${paymentId}, error ${status}`) } else { - console.error(`Unexpected error fetching payment status for ${payment.paymentId}.`) + console.error(`Unexpected error fetching payment status for ${paymentId}.`) } } } From 568f97600efae10c2c35c5f4f5268b697f49c0ff Mon Sep 17 00:00:00 2001 From: Philip Benson Date: Mon, 13 Apr 2026 16:55:40 +0100 Subject: [PATCH 3/3] Lint fixes --- package-lock.json | 1 - .../recurring-payments-processor.spec.js | 26 +++++++++---------- .../src/recurring-payments-processor.js | 8 ++++-- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/package-lock.json b/package-lock.json index e0d0d502d1..d121f5a449 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16208,7 +16208,6 @@ "version": "2.3.3", "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz", "integrity": "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==", - "dev": true, "hasInstallScript": true, "optional": true, "os": [ diff --git a/packages/recurring-payments-job/src/__tests__/recurring-payments-processor.spec.js b/packages/recurring-payments-job/src/__tests__/recurring-payments-processor.spec.js index 70a756d23d..2cbeb908cc 100644 --- a/packages/recurring-payments-job/src/__tests__/recurring-payments-processor.spec.js +++ b/packages/recurring-payments-job/src/__tests__/recurring-payments-processor.spec.js @@ -56,8 +56,10 @@ const PAYMENT_STATUS_DELAY = 60000 // ── Response factories ──────────────────────────────────────────────────────── const mockCreationOkResponse = ({ + // eslint-disable-next-line camelcase payment_id = 'pay-1', reference = 'test-transaction-id', + // eslint-disable-next-line camelcase created_date = '2025-01-01T00:00:00.000Z' } = {}) => ({ ok: true, @@ -116,7 +118,6 @@ const getMockPaymentRequestResponse = () => [ } ] -const getPaymentStatusSuccess = () => ({ state: { status: 'payment status success' } }) const getPaymentStatusFailure = () => ({ state: { status: 'payment status failure' } }) const getPaymentStatusError = () => ({ state: { status: 'payment status error' } }) @@ -132,6 +133,7 @@ const setupSinglePayment = ({ referenceNumber = '123', transactionId = 'test-transaction-id', paymentId = 'pay-1', + // eslint-disable-next-line camelcase created_date = '2025-01-01T00:00:00.000Z', paymentStatus = 'payment status success', permissionData = { licensee: { countryCode: 'GB-ENG' } } @@ -381,10 +383,7 @@ describe('recurring-payments-processor', () => { agreement_id: agreementId }) - expect(creationBatcher.addRequest).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({ body: expectedBody }) - ) + expect(creationBatcher.addRequest).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ body: expectedBody })) }) it('calls batcher.fetch() for payment creation', async () => { @@ -410,9 +409,7 @@ describe('recurring-payments-processor', () => { const creationBatcher = makeBatcherMock( agreementIds.map((_, i) => mockCreationOkResponse({ payment_id: `pay-${i + 1}`, reference: `trans-${i + 1}` })) ) - const statusBatcher = makeBatcherMock( - agreementIds.map((_, i) => mockStatusOkResponse(`pay-${i + 1}`, 'payment status success')) - ) + const statusBatcher = makeBatcherMock(agreementIds.map((_, i) => mockStatusOkResponse(`pay-${i + 1}`, 'payment status success'))) HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) await execute() @@ -534,7 +531,9 @@ describe('recurring-payments-processor', () => { paymentIds.map((pid, i) => mockCreationOkResponse({ payment_id: pid, reference: transactionIds[i] })) ) const statusBatcher = makeBatcherMock( - paymentIds.map((pid, i) => ([1, 3].includes(i) ? mockStatusErrorResponse(pid, 500) : mockStatusOkResponse(pid, 'payment status success'))) + paymentIds.map((pid, i) => + [1, 3].includes(i) ? mockStatusErrorResponse(pid, 500) : mockStatusOkResponse(pid, 'payment status success') + ) ) HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) @@ -605,15 +604,14 @@ describe('recurring-payments-processor', () => { it('creates a payment journal entry', async () => { const paymentId = 'unique-payment-id-under-test' + // eslint-disable-next-line camelcase const created_date = Symbol('created-date') const transactionId = Symbol('transaction-id') salesApi.getDueRecurringPayments.mockReturnValueOnce([getMockDueRecurringPayment()]) salesApi.createTransaction.mockReturnValueOnce({ id: transactionId, cost: 99, recurringPayment: { id: 'rp-1' } }) - const creationBatcher = makeBatcherMock([ - mockCreationOkResponse({ payment_id: paymentId, reference: transactionId, created_date }) - ]) + const creationBatcher = makeBatcherMock([mockCreationOkResponse({ payment_id: paymentId, reference: transactionId, created_date })]) const statusBatcher = makeBatcherMock([mockStatusOkResponse(paymentId, 'payment status success')]) HTTPRequestBatcher.mockImplementationOnce(() => creationBatcher).mockImplementationOnce(() => statusBatcher) @@ -839,7 +837,7 @@ describe('recurring-payments-processor', () => { }) it('should log errors from await salesApi.processRPResult', async () => { - const { paymentId, transactionId } = setupSinglePayment({ transactionId: 'trans-1', paymentId: 'pay-1' }) + const { transactionId } = setupSinglePayment({ transactionId: 'trans-1', paymentId: 'pay-1' }) const boom = new Error('boom') salesApi.processRPResult.mockImplementation(transId => (transId === transactionId ? Promise.reject(boom) : Promise.resolve())) @@ -986,9 +984,11 @@ describe('recurring-payments-processor', () => { }) it('calls processRPResult with transaction id, payment id and created date when payment is successful', async () => { + // eslint-disable-next-line camelcase const { paymentId, transactionId, created_date } = setupSinglePayment({ transactionId: 'test-transaction-id', paymentId: 'test-payment-id', + // eslint-disable-next-line camelcase created_date: '2025-01-01T00:00:00.000Z' }) diff --git a/packages/recurring-payments-job/src/recurring-payments-processor.js b/packages/recurring-payments-job/src/recurring-payments-processor.js index e38903ae5a..4c7dc6944f 100644 --- a/packages/recurring-payments-job/src/recurring-payments-processor.js +++ b/packages/recurring-payments-job/src/recurring-payments-processor.js @@ -158,7 +158,9 @@ const processPaymentCreationResponse = async (response, metadata, transactionMap await salesApi.cancelRecurringPayment(transaction.recurringPayment?.id) } else { console.error( - `Unexpected response from GOV.UK Pay API. Status: ${response.status}, Response: ${JSON.stringify(body)}, Transaction ID: ${transaction.id}` + `Unexpected response from GOV.UK Pay API. Status: ${response.status}, Response: ${JSON.stringify(body)}, Transaction ID: ${ + transaction.id + }` ) } return @@ -266,7 +268,9 @@ const processPaymentStatusResponse = async (response, paymentMap) => { return } - const { state: { status } } = await response.json() + const { + state: { status } + } = await response.json() debug(`Payment status for ${paymentId}: ${status}`)