Add agreement_id to existing GOV.UK Pay request (#2074)

https://eaflood.atlassian.net/browse/IWTF-4261

If the request to prepare a payment is part of setting up a new recurring payment agreement, then the agreement_id should be included.

I also renamed prepareRecurringPayment() to prepareRecurringPaymentAgreement() as I think this is slightly more accurate - it is setting up the agreement rather than the payment - and it initially threw me off when working out what needed to be changed.

* Store agreementId in DynamoDb

* Fix agreementId validation in schema

* Resolve linting issue

* Add arg to payment connector for API key usage

* Add recurring payment arg to govuk-pay-service

Also added a lot more tests (cribbed from the sendRecurringPayment() ones) as the existing ones are testing through the payment processor for some reason.

* Use recurring flag in agreed handler

Because we need to check the recurring status at this level now, we can also remove the status check from the payment processor.

* Update rest of pay connector to toggle headers

* Add recurring flag to getPaymentStatus in service

* Add recurring arg when checking payment status in agreed handler

* Add optional() to schema

* Use agreementId in payment processor logic

* Fix mocking

* Remove unnecessary default

docker/env/gafl_webapp.secrets.env.example

@@ -3,5 +3,6 @@
 ####################################################################################
 
 GOV_PAY_APIKEY=<insert here>
+GOV_PAY_RECURRING_APIKEY=<insert here>
 SESSION_COOKIE_PASSWORD=<insert here>
 ANALYTICS_PROPERTY_API=<insert here>

packages/connectors-lib/README.md

@@ -18,6 +18,7 @@ Provides connectivity to the resources/infrastructure used in the rod licensing
 | SALES_API_TIMEOUT_MS                | Request timeout for the requests to the sales API                  |    no    | 20000 (20s)     |                                                                                                |                                                                             |
 | GOV_PAY_API_URL                     | The GOV.UK Pay API base url                                        |   yes    |                 |                                                                                                |                                                                             |
 | GOV_PAY_APIKEY                      | GOV pay access identifier                                          |   yes    |                 |                                                                                                |                                                                             |
+| GOV_PAY_RECURRING_APIKEY            | GOV pay access identifier for recurring payments                   |   yes    |                 |                                                                                                |                                                                             |
 | GOV_PAY_REQUEST_TIMEOUT_MS          | Timeout in milliseconds for API requests                           |    no    | 10000           |                                                                                                |                                                                             |
 | GOV_PAY_RCP_API_URL                 | The GOV.UK Pay API url for agreements                              |   yes    |                 |
 

packages/connectors-lib/src/__tests__/govuk-pay-api.spec.js

@@ -5,13 +5,20 @@ const fetch = require('node-fetch')
 process.env.GOV_PAY_API_URL = 'http://0.0.0.0/payment'
 process.env.GOV_PAY_RCP_API_URL = 'http://0.0.0.0/agreement'
 process.env.GOV_PAY_APIKEY = 'key'
+process.env.GOV_PAY_RECURRING_APIKEY = 'recurringkey'
 
 const headers = {
   accept: 'application/json',
   authorization: `Bearer ${process.env.GOV_PAY_APIKEY}`,
   'content-type': 'application/json'
 }
 
+const recurringHeaders = {
+  accept: 'application/json',
+  authorization: `Bearer ${process.env.GOV_PAY_RECURRING_APIKEY}`,
+  'content-type': 'application/json'
+}
+
 describe('govuk-pay-api-connector', () => {
   beforeEach(jest.clearAllMocks)
 
@@ -41,6 +48,17 @@ describe('govuk-pay-api-connector', () => {
       })
       expect(consoleErrorSpy).toHaveBeenCalled()
     })
+
+    it('uses the correct API key if recurring arg is set to true', async () => {
+      fetch.mockReturnValue({ ok: true, status: 200 })
+      await expect(govUkPayApi.createPayment({ cost: 0 }, true)).resolves.toEqual({ ok: true, status: 200 })
+      expect(fetch).toHaveBeenCalledWith('http://0.0.0.0/payment', {
+        body: JSON.stringify({ cost: 0 }),
+        headers: recurringHeaders,
+        method: 'post',
+        timeout: 10000
+      })
+    })
   })
 
   describe('fetchPaymentStatus', () => {
@@ -63,6 +81,16 @@ describe('govuk-pay-api-connector', () => {
       expect(fetch).toHaveBeenCalledWith('http://0.0.0.0/payment/123', { headers, method: 'get', timeout: 10000 })
       expect(consoleErrorSpy).toHaveBeenCalled()
     })
+
+    it('uses the correct API key if recurring arg is set to true', async () => {
+      fetch.mockReturnValue({ ok: true, status: 200, json: () => {} })
+      await expect(govUkPayApi.fetchPaymentStatus(123, true)).resolves.toEqual(expect.objectContaining({ ok: true, status: 200 }))
+      expect(fetch).toHaveBeenCalledWith('http://0.0.0.0/payment/123', {
+        headers: recurringHeaders,
+        method: 'get',
+        timeout: 10000
+      })
+    })
   })
 
   describe('fetchPaymentEvents', () => {
@@ -80,6 +108,16 @@ describe('govuk-pay-api-connector', () => {
       await expect(govUkPayApi.fetchPaymentEvents(123)).rejects.toEqual(Error('test event error'))
       expect(consoleErrorSpy).toHaveBeenCalled()
     })
+
+    it('uses the correct API key if recurring arg is set to true', async () => {
+      fetch.mockReturnValue({ ok: true, status: 200, json: () => {} })
+      await expect(govUkPayApi.fetchPaymentEvents(123, true)).resolves.toEqual(expect.objectContaining({ ok: true, status: 200 }))
+      expect(fetch).toHaveBeenCalledWith('http://0.0.0.0/payment/123/events', {
+        headers: recurringHeaders,
+        method: 'get',
+        timeout: 10000
+      })
+    })
   })
 
   describe('createRecurringPayment', () => {
@@ -88,7 +126,7 @@ describe('govuk-pay-api-connector', () => {
       await expect(govUkPayApi.createRecurringPayment({ cost: 0 })).resolves.toEqual({ ok: true, status: 200 })
       expect(fetch).toHaveBeenCalledWith('http://0.0.0.0/agreement', {
         body: JSON.stringify({ cost: 0 }),
-        headers,
+        headers: recurringHeaders,
         method: 'post',
         timeout: 10000
       })
@@ -102,7 +140,7 @@ describe('govuk-pay-api-connector', () => {
       expect(govUkPayApi.createRecurringPayment({ reference: '123' })).rejects.toEqual(Error(''))
       expect(fetch).toHaveBeenCalledWith('http://0.0.0.0/agreement', {
         body: JSON.stringify({ reference: '123' }),
-        headers,
+        headers: recurringHeaders,
         method: 'post',
         timeout: 10000
       })

packages/connectors-lib/src/govuk-pay-api.js

@@ -4,9 +4,9 @@
 import fetch from 'node-fetch'
 const GOV_PAY_REQUEST_TIMEOUT_MS_DEFAULT = 10000
 
-const headers = () => ({
+const headers = recurring => ({
   accept: 'application/json',
-  authorization: `Bearer ${process.env.GOV_PAY_APIKEY}`,
+  authorization: `Bearer ${recurring ? process.env.GOV_PAY_RECURRING_APIKEY : process.env.GOV_PAY_APIKEY}`,
   'content-type': 'application/json'
 })
 
@@ -18,7 +18,7 @@ const headers = () => ({
 export const createRecurringPayment = async preparedPayment => {
   try {
     return fetch(process.env.GOV_PAY_RCP_API_URL, {
-      headers: headers(),
+      headers: headers(true),
       method: 'post',
       body: JSON.stringify(preparedPayment),
       timeout: process.env.GOV_PAY_REQUEST_TIMEOUT_MS || GOV_PAY_REQUEST_TIMEOUT_MS_DEFAULT
@@ -37,10 +37,10 @@ export const createRecurringPayment = async preparedPayment => {
  * @param preparedPayment - see the GOV.UK pay API reference for details
  * @returns {Promise<*>}
  */
-export const createPayment = async preparedPayment => {
+export const createPayment = async (preparedPayment, recurring = false) => {
   try {
     return fetch(process.env.GOV_PAY_API_URL, {
-      headers: headers(),
+      headers: headers(recurring),
       method: 'post',
       body: JSON.stringify(preparedPayment),
       timeout: process.env.GOV_PAY_REQUEST_TIMEOUT_MS || GOV_PAY_REQUEST_TIMEOUT_MS_DEFAULT
@@ -56,10 +56,10 @@ export const createPayment = async preparedPayment => {
  * @param paymentId
  * @returns {Promise<unknown>}
  */
-export const fetchPaymentStatus = async paymentId => {
+export const fetchPaymentStatus = async (paymentId, recurring = false) => {
   try {
     return fetch(`${process.env.GOV_PAY_API_URL}/${paymentId}`, {
-      headers: headers(),
+      headers: headers(recurring),
       method: 'get',
       timeout: process.env.GOV_PAY_REQUEST_TIMEOUT_MS || GOV_PAY_REQUEST_TIMEOUT_MS_DEFAULT
     })
@@ -74,10 +74,10 @@ export const fetchPaymentStatus = async paymentId => {
  * @param paymentId
  * @returns {Promise<unknown>}
  */
-export const fetchPaymentEvents = async paymentId => {
+export const fetchPaymentEvents = async (paymentId, recurring = false) => {
   try {
     return fetch(`${process.env.GOV_PAY_API_URL}/${paymentId}/events`, {
-      headers: headers(),
+      headers: headers(recurring),
       method: 'get',
       timeout: process.env.GOV_PAY_REQUEST_TIMEOUT_MS || GOV_PAY_REQUEST_TIMEOUT_MS_DEFAULT
     })

packages/gafl-webapp-service/src/handlers/__tests__/agreed-handler-recurring-payments.spec.js

@@ -1,16 +1,22 @@
 import { salesApi } from '@defra-fish/connectors-lib'
 import { COMPLETION_STATUS, RECURRING_PAYMENT } from '../../constants.js'
 import agreedHandler from '../agreed-handler.js'
-import { prepareRecurringPayment } from '../../processors/payment.js'
-import { sendRecurringPayment } from '../../services/payment/govuk-pay-service.js'
+import { preparePayment, prepareRecurringPaymentAgreement } from '../../processors/payment.js'
+import { sendPayment, sendRecurringPayment, getPaymentStatus } from '../../services/payment/govuk-pay-service.js'
 import { prepareApiTransactionPayload } from '../../processors/api-transaction.js'
 import { v4 as uuidv4 } from 'uuid'
 import db from 'debug'
 
 jest.mock('@defra-fish/connectors-lib')
 jest.mock('../../processors/payment.js')
 jest.mock('../../services/payment/govuk-pay-service.js', () => ({
-  sendPayment: jest.fn(),
+  sendPayment: jest.fn(() => ({
+    payment_id: 'payment-id-1',
+    _links: {
+      next_url: { href: 'next-url' },
+      self: { href: 'self-url' }
+    }
+  })),
   getPaymentStatus: jest.fn(),
   sendRecurringPayment: jest.fn(() => ({ agreementId: 'agr-eem-ent-id1' }))
 }))
@@ -32,12 +38,12 @@ describe('The agreed handler', () => {
   })
   beforeEach(jest.clearAllMocks)
 
-  const getMockRequest = (overrides = {}) => ({
+  const getMockRequest = ({ overrides = {}, transactionSet = () => {} } = {}) => ({
     cache: () => ({
       helpers: {
         transaction: {
           get: async () => ({ cost: 0 }),
-          set: async () => {}
+          set: transactionSet
         },
         status: {
           get: async () => ({
@@ -54,25 +60,28 @@ describe('The agreed handler', () => {
   })
 
   const getRequestToolkit = () => ({
+    redirect: jest.fn(),
     redirectWithLanguageCode: jest.fn()
   })
 
   describe('recurring card payments', () => {
     it('sends the request and transaction to prepare the recurring payment', async () => {
       const transaction = { cost: 0 }
       const mockRequest = getMockRequest({
-        transaction: {
-          get: async () => transaction,
-          set: () => {}
+        overrides: {
+          transaction: {
+            get: async () => transaction,
+            set: () => {}
+          }
         }
       })
       await agreedHandler(mockRequest, getRequestToolkit())
-      expect(prepareRecurringPayment).toHaveBeenCalledWith(mockRequest, transaction)
+      expect(prepareRecurringPaymentAgreement).toHaveBeenCalledWith(mockRequest, transaction)
     })
 
     it('adds a v4 guid to the transaction as an id', async () => {
       let transactionPayload = null
-      prepareRecurringPayment.mockImplementationOnce((_p1, tp) => {
+      prepareRecurringPaymentAgreement.mockImplementationOnce((_p1, tp) => {
         transactionPayload = { ...tp }
       })
       const v4guid = Symbol('v4guid')
@@ -90,46 +99,73 @@ 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')
-      salesApi.finaliseTransaction.mockReturnValueOnce({
-        permissions: []
+    it('sends a recurring payment agreement creation request to Gov.UK Pay', async () => {
+      const preparedPayment = Symbol('preparedPayment')
+      prepareRecurringPaymentAgreement.mockResolvedValueOnce(preparedPayment)
+      await agreedHandler(getMockRequest(), getRequestToolkit())
+      expect(sendRecurringPayment).toHaveBeenCalledWith(preparedPayment)
+    })
+
+    describe('when there is a cost and recurringAgreement status is set to true', () => {
+      beforeEach(() => {
+        salesApi.createTransaction.mockResolvedValueOnce({
+          id: 'transaction-id-1',
+          cost: 100
+        })
+      })
+
+      it('calls preparePayment', async () => {
+        const transaction = { id: Symbol('transaction') }
+        const request = getMockRequest({
+          overrides: {
+            transaction: {
+              get: async () => transaction,
+              set: () => {}
+            }
+          }
+        })
+        const toolkit = getRequestToolkit()
+
+        await agreedHandler(request, toolkit)
+        expect(preparePayment).toHaveBeenCalledWith(request, transaction)
+      })
+
+      it('calls sendPayment with recurring as true', async () => {
+        const preparedPayment = Symbol('preparedPayment')
+        preparePayment.mockReturnValueOnce(preparedPayment)
+
+        await agreedHandler(getMockRequest(), getRequestToolkit())
+        expect(sendPayment).toHaveBeenCalledWith(preparedPayment, true)
       })
-      const mockRequest = {
-        cache: () => ({
-          helpers: {
+
+      it('calls getPaymentStatus with recurring as true', async () => {
+        const id = Symbol('paymentId')
+        const transaction = { id: '123', payment: { payment_id: id } }
+        const request = getMockRequest({
+          overrides: {
+            transaction: {
+              get: async () => transaction,
+              set: () => {}
+            },
             status: {
               get: async () => ({
                 [COMPLETION_STATUS.agreed]: true,
-                [COMPLETION_STATUS.posted]: true,
-                [COMPLETION_STATUS.finalised]: false,
-                [RECURRING_PAYMENT]: false
+                [COMPLETION_STATUS.posted]: false,
+                [COMPLETION_STATUS.finalised]: true,
+                [RECURRING_PAYMENT]: true,
+                [COMPLETION_STATUS.paymentCreated]: true
               }),
               set: () => {}
-            },
-            transaction: {
-              get: async () => ({ cost: 0, id: transactionId }),
-              set: setTransaction
             }
           }
         })
-      }
+        const toolkit = getRequestToolkit()
 
-      await agreedHandler(mockRequest, getRequestToolkit())
+        getPaymentStatus.mockReturnValueOnce({ state: { finished: true, status: 'success' } })
 
-      expect(salesApi.finaliseTransaction).toHaveBeenCalledWith(
-        transactionId,
-        undefined // prepareApiFinalisationPayload has no mocked return value
-      )
-    })
-
-    it('sends a recurring payment creation request to Gov.UK Pay', async () => {
-      const preparedPayment = Symbol('preparedPayment')
-      prepareRecurringPayment.mockResolvedValueOnce(preparedPayment)
-      await agreedHandler(getMockRequest(), getRequestToolkit())
-      expect(sendRecurringPayment).toHaveBeenCalledWith(preparedPayment)
+        await agreedHandler(request, toolkit)
+        expect(getPaymentStatus).toHaveBeenCalledWith(id, true)
+      })
     })
 
     // this doesn't really belong here, but until the other agreed handler tests are refactored to
@@ -140,7 +176,7 @@ describe('The agreed handler', () => {
 
       await agreedHandler(getMockRequest(), getRequestToolkit())
 
-      expect(prepareApiTransactionPayload).toHaveBeenCalledWith(expect.any(Object), v4guid)
+      expect(prepareApiTransactionPayload).toHaveBeenCalledWith(expect.any(Object), v4guid, undefined)
     })
 
     it.each(['zxy-098-wvu-765', '467482f1-099d-403d-b6b3-8db7e70d19e3'])(
@@ -158,5 +194,23 @@ describe('The agreed handler', () => {
         expect(debugMock).toHaveBeenCalledWith(`Created agreement with id ${agreement_id}`)
       }
     )
+
+    it.each(['zxy-098-wvu-765', '467482f1-099d-403d-b6b3-8db7e70d19e3'])(
+      "assigns agreement id '%s' to the transaction when recurring payment agreement created",
+      async agreementId => {
+        const mockTransactionCacheSet = jest.fn()
+        sendRecurringPayment.mockResolvedValueOnce({
+          agreement_id: agreementId
+        })
+
+        await agreedHandler(getMockRequest({ transactionSet: mockTransactionCacheSet }), getRequestToolkit())
+
+        expect(mockTransactionCacheSet).toHaveBeenCalledWith(
+          expect.objectContaining({
+            agreementId
+          })
+        )
+      }
+    )
   })
 })