Rework after review - part 1

Author: jbarnsley10

jest.setup.cjs

@@ -13,4 +13,4 @@ process.env.UPLOADER_BUCKET_NAME = 'dummy-bucket'
 process.env.GOOGLE_ANALYTICS_TRACKING_ID = 'G-123456789'
 process.env.SUBMISSION_EMAIL_ADDRESS = 'dummy@defra.gov.uk'
 process.env.ORDNANCE_SURVEY_API_KEY = 'dummy'
-process.env.PAYMENT_PROVIDER_API_KEY_TEST = 'test-api-key'
+process.env.PAYMENT_PROVIDER_API_KEY_TEST_formid = 'test-api-key'

src/config/index.ts

@@ -260,14 +260,6 @@ export const config = convict({
     nullable: true,
     default: undefined,
     env: 'ORDNANCE_SURVEY_API_KEY'
-  } as SchemaObj<string | undefined>,
-
-  paymentProviderApiKeyTest: {
-    doc: 'A test API key for integrating with a payment provider',
-    format: String,
-    nullable: true,
-    default: undefined,
-    env: 'PAYMENT_PROVIDER_API_KEY_TEST'
   } as SchemaObj<string | undefined>
 })
 

src/server/plugins/engine/components/FormComponent.ts

@@ -5,6 +5,7 @@ import {
 } from '@defra/forms-model'
 
 import { ComponentBase } from '~/src/server/plugins/engine/components/ComponentBase.js'
+import { type PaymentState } from '~/src/server/plugins/engine/components/PaymentField.types.js'
 import { optionalText } from '~/src/server/plugins/engine/components/constants.js'
 import {
   type FormContext,
@@ -179,7 +180,7 @@ export class FormComponent extends ComponentBase {
 
   getContextValueFromFormValue(
     value: FormValue | FormPayload
-  ): Item['value'] | Item['value'][] | null {
+  ): Item['value'] | Item['value'][] | PaymentState | null {
     // Filter object field values
     if (this.isState(value)) {
       const values = Object.values(value).filter(isFormValue)
@@ -191,12 +192,12 @@ export class FormComponent extends ComponentBase {
       return value.filter(isFormValue)
     }
 
-    return this.isValue(value) ? (value as Item['value']) : null
+    return this.isValue(value) ? value : null
   }
 
   getContextValueFromState(
     state: FormSubmissionState
-  ): Item['value'] | Item['value'][] | null {
+  ): Item['value'] | Item['value'][] | PaymentState | null {
     const value = this.getFormValueFromState(state)
 
     return this.getContextValueFromFormValue(value)

src/server/plugins/engine/components/PaymentField.test.ts

@@ -170,7 +170,7 @@ describe('PaymentField', () => {
         amount: 150,
         description: 'payment description',
         uuid: 'ee501106-4ce1-4947-91a7-7cc1a335ccd8',
-        formId: 'form-id',
+        formId: 'formid',
         isLivePayment: false
       }
       it('returns text from state', () => {
@@ -206,7 +206,7 @@ describe('PaymentField', () => {
           paymentId: 'payment-id',
           reference: 'payment-ref',
           uuid: 'ee501106-4ce1-4947-91a7-7cc1a335ccd8',
-          formId: 'form-id',
+          formId: 'formid',
           amount: 100,
           description: 'Test payment description',
           isLivePayment: false
@@ -273,7 +273,7 @@ describe('PaymentField', () => {
         const args = {
           controller: {
             model: {
-              formId: 'form-id',
+              formId: 'formid',
               basePath: 'base-path',
               name: 'PaymentModel'
             },
@@ -308,7 +308,7 @@ describe('PaymentField', () => {
           componentName: 'myComponent',
           description: 'Test payment description',
           failureUrl: 'http://localhost:3009/test-payment',
-          formId: 'form-id',
+          formId: 'formid',
           isLivePayment: false,
           paymentId: 'new-payment-id',
           reference: 'pay-ref-123',
@@ -338,7 +338,7 @@ describe('PaymentField', () => {
         const args = {
           controller: {
             model: {
-              formId: 'form-id',
+              formId: 'formid',
               basePath: 'base-path',
               name: 'PaymentModel'
             },
@@ -432,7 +432,7 @@ describe('PaymentField', () => {
                 amount: 123,
                 description: 'Payment desc',
                 isLivePayment: false,
-                formId: 'form-id'
+                formId: 'formid'
               }
             }
           } as unknown as FormContext
@@ -458,7 +458,7 @@ describe('PaymentField', () => {
                   amount: 123,
                   description: 'Payment desc',
                   isLivePayment: false,
-                  formId: 'form-id'
+                  formId: 'formid'
                 }
               }
             } as unknown as FormContext
@@ -495,7 +495,7 @@ describe('PaymentField', () => {
                   amount: 123,
                   description: 'Payment desc',
                   isLivePayment: false,
-                  formId: 'form-id'
+                  formId: 'formid'
                 }
               }
             } as unknown as FormContext
@@ -531,7 +531,7 @@ describe('PaymentField', () => {
                 amount: 123,
                 description: 'Payment desc',
                 isLivePayment: false,
-                formId: 'form-id'
+                formId: 'formid'
               }
             }
           } as unknown as FormContext

src/server/plugins/engine/routes/payment-helper.test.js

@@ -35,7 +35,7 @@ describe('payment helper', () => {
         get: jest.fn().mockReturnValueOnce({
           paymentId: 'payment-id',
           isLivePayment: false,
-          formId: 'form-id'
+          formId: 'formid'
         })
       }
     }
@@ -76,7 +76,7 @@ describe('payment helper', () => {
         }
       },
       session: {
-        formId: 'form-id',
+        formId: 'formid',
         isLivePayment: false,
         paymentId: 'payment-id'
       },

src/server/plugins/payment/helper.js

@@ -1,23 +1,22 @@
 import { format } from 'date-fns'
 
-import { config } from '~/src/config/index.js'
 import { PaymentService } from '~/src/server/plugins/payment/service.js'
 
 export const DEFAULT_PAYMENT_HELP_URL =
   'https://www.gov.uk/government/organisations/department-for-environment-food-rural-affairs'
 
 /**
  * Determine which payment API key value to use.
- * If a non-live non-preview form, use the TEST API key value.
- * If a live (non-preview) form, read the API key value specific to that form.
+ * If a draft preview form or a live preview form, read the TEST API key value specific to that form.
+ * If a live (non-preview) form, read the LIVE API key value specific to that form.
  * @param {boolean} isLivePayment - true if this is a live payment (as opposed to a test one)
  * @param {string} formId - id of the form
  * @returns {string}
  */
 export function getPaymentApiKey(isLivePayment, formId) {
   const apiKeyValue = isLivePayment
     ? process.env[`PAYMENT_PROVIDER_API_KEY_LIVE_${formId}`]
-    : config.get('paymentProviderApiKeyTest')
+    : process.env[`PAYMENT_PROVIDER_API_KEY_TEST_${formId}`]
 
   if (!apiKeyValue) {
     throw new Error(

src/server/plugins/payment/helper.test.js

@@ -9,6 +9,7 @@ describe('getPaymentApiKey', () => {
   config.set('paymentProviderApiKeyTest', 'TEST-API-KEY')
   const formId = 'form-id'
   process.env['PAYMENT_PROVIDER_API_KEY_LIVE_form-id'] = 'LIVE-API-KEY'
+  process.env['PAYMENT_PROVIDER_API_KEY_TEST_form-id'] = 'TEST-API-KEY'
 
   it('should read test key when non-live form', () => {
     const apiKey = getPaymentApiKey(false, formId)
@@ -20,7 +21,13 @@ describe('getPaymentApiKey', () => {
     expect(apiKey).toBe('LIVE-API-KEY')
   })
 
-  it('should throw if key is missing', () => {
+  it('should throw if TEST key is missing', () => {
+    expect(() => getPaymentApiKey(false, 'form-id-missing')).toThrow(
+      'Missing payment api key for test form id form-id-missing'
+    )
+  })
+
+  it('should throw if LIVE key is missing', () => {
     expect(() => getPaymentApiKey(true, 'form-id-missing')).toThrow(
       'Missing payment api key for live form id form-id-missing'
     )