Refactor Supplementary Billing Flags (#2624)

https://eaflood.atlassian.net/browse/WATER-4137 https://eaflood.atlassian.net/browse/WATER-4199 https://eaflood.atlassian.net/browse/WATER-4622 https://eaflood.atlassian.net/browse/WATER-4588 Upon completion of the above two-part tariff supplementary tickets, it was noted that there are now multiple places in the code that handles flagging for pre-sroc supplementary, sroc supplementary and now two-part tariff supplementary. The UI has its own flagging mechanisms as well as the service repo and now our new code in system. To keep things consistent, and easy to maintain it was decided that all the code for flagging a supplementary bill run be consolidated into the system repo. All other flagging mechanisms can be removed meaning there is only 1 place to update and maintain flagging. This PR is a housekeeping task to reduce the code.
DEFRA · Dec 2, 2024 · 495b777 · 495b777
1 parent a75057b
commit 495b777
Show file tree

Hide file tree

Showing 10 changed files with 21 additions and 129 deletions.
diff --git a/...tors/services/system/LicencesApiClient.js → ...ctors/services/system/ReturnsApiClient.js b/...tors/services/system/LicencesApiClient.js → ...ctors/services/system/ReturnsApiClient.js
@@ -2,7 +2,7 @@
 
 const ServiceClient = require('../../../../../shared/lib/connectors/services/ServiceClient')
 
-class LicencesApiClient extends ServiceClient {
+class ReturnsApiClient extends ServiceClient {
   /**
    * Flags a licence for the supplementary bill run
    *
@@ -25,4 +25,4 @@ class LicencesApiClient extends ServiceClient {
   }
 }
 
-module.exports = LicencesApiClient
+module.exports = ReturnsApiClient
diff --git a/src/internal/lib/connectors/services/system/WorkflowApiClient.js b/src/internal/lib/connectors/services/system/WorkflowApiClient.js
diff --git a/src/internal/lib/connectors/services/system/index.js b/src/internal/lib/connectors/services/system/index.js
@@ -3,14 +3,12 @@
 // Internal services
 const BillingAccountsApiClient = require('./BillingAccountsApiClient.js')
 const BillRunsApiClient = require('./BillRunsApiClient.js')
-const LicencesApiClient = require('./LicencesApiClient.js')
-const WorkflowApiClient = require('./WorkflowApiClient.js')
+const ReturnsApiClient = require('./ReturnsApiClient.js')
 
 const { logger } = require('../../../../logger')
 
 module.exports = config => ({
   billingAccounts: new BillingAccountsApiClient(config.services.system, logger),
   billRuns: new BillRunsApiClient(config.services.system, logger),
-  licences: new LicencesApiClient(config.services.system, logger),
-  workflow: new WorkflowApiClient(config.services.system, logger)
+  returns: new ReturnsApiClient(config.services.system, logger)
 })
diff --git a/src/internal/modules/charge-information/controllers/charge-information-workflow.js b/src/internal/modules/charge-information/controllers/charge-information-workflow.js
@@ -5,7 +5,6 @@ const { deleteChargeInfo } = require('../forms')
 const services = require('../../../lib/connectors/services')
 const { sortBy } = require('lodash')
 const { clearNoteSessionData } = require('internal/modules/charge-information/lib/helpers')
-const { logger } = require('./../../../logger.js')
 
 const getChargeInformationWorkflow = async (request, h) => {
   const toSetUp = request.pre.chargeInformationWorkflows
@@ -49,13 +48,6 @@ const getRemoveChargeInformationWorkflow = (request, h) => {
 const postRemoveChargeInformationWorkflow = async (request, h) => {
   const { chargeVersionWorkflowId } = request.params
 
-  try {
-    const cookie = request.headers.cookie
-    await services.system.workflow.supplementary(chargeVersionWorkflowId, cookie)
-  } catch (error) {
-    logger.error('Flag supplementary request to system failed', error.stack)
-  }
-
   await services.water.chargeVersionWorkflows.deleteChargeVersionWorkflow(chargeVersionWorkflowId)
   clearNoteSessionData(request)
   return h.redirect('/charge-information-workflow')

diff --git a/src/internal/modules/view-licences/controller.js b/src/internal/modules/view-licences/controller.js
@@ -148,26 +148,25 @@ const getMarkLicenceForSupplementaryBilling = (request, h) => {
  * to this function. This enables the system repo to determine whether the licence needs flagging based on the updated
  * return.
  *
- * In the case of the legacy "Recalculate Bills" link, only the licence ID is needed to flag the licence
- * for pre-SROC and SROC supplementary billing.
+ * We no longer support the legacy recalculate bills page for flagging a licence. This means this route should only be
+ * used for editing a return and as a result of that we only call the system repo for flagging when a returnId is
+ * present
  */
 const postMarkLicenceForSupplementaryBilling = async (request, h) => {
   const { licenceId } = request.params
   const { returnId } = request.payload
   const { document } = request.pre
   const { system_external_id: licenceRef } = document
 
-  if (returnId) {
-    try {
-      const cookie = request.headers.cookie
-
-      await services.system.licences.supplementary(returnId, cookie)
-    } catch (error) {
-      logger.error('Flag supplementary request to system failed', error.stack)
+  try {
+    // If there is a returnId, it means we are flagging for a new return being added/edited
+    if (returnId) {
+      await services.system.returns.supplementary(returnId)
     }
-  } else {
-    // Call backend to mark the licence for supplementary billing
-    await services.water.licences.postMarkLicenceForSupplementaryBilling(licenceId)
+    // Otherwise it means the user has gone through the legacy recalculate bills link (they shouldn't do this).
+    // This functionality has been migrated to our new licence page so nothing needs to happen here
+  } catch (error) {
+    logger.error('Flag supplementary request to system failed', error.stack)
   }
 
   return h.view('nunjucks/billing/marked-licence-for-supplementary-billing', {

diff --git a/src/shared/lib/connectors/services/water/LicencesService.js b/src/shared/lib/connectors/services/water/LicencesService.js
@@ -166,11 +166,6 @@ class LicencesService extends ServiceClient {
       }
     })
   }
-
-  postMarkLicenceForSupplementaryBilling (licenceId) {
-    const uri = this.joinUrl('licences', licenceId, 'mark-for-supplementary-billing')
-    return this.serviceRequest.post(uri)
-  }
 }
 
 module.exports = LicencesService
diff --git a/...tors/services/system/LicencesApiClient.js → .../services/system/ReturnsApiClient.test.js b/...tors/services/system/LicencesApiClient.js → .../services/system/ReturnsApiClient.test.js
@@ -13,14 +13,14 @@ const sandbox = Sinon.createSandbox()
 const { serviceRequest } = require('@envage/water-abstraction-helpers')
 
 // Thing under test
-const LicencesApiClient = require('../../../../../../src/internal/lib/connectors/services/system/LicencesApiClient.js')
+const ReturnsApiClient = require('../../../../../../src/internal/lib/connectors/services/system/ReturnsApiClient.js')
 
-experiment('services/system/LicencesApiClient', () => {
+experiment('services/system/ReturnsApiClient', () => {
   let service
 
   beforeEach(async () => {
     sandbox.stub(serviceRequest, 'post')
-    service = new LicencesApiClient('http://127.0.0.1:8013')
+    service = new ReturnsApiClient('http://127.0.0.1:8013')
   })
 
   afterEach(async () => {

diff --git a/test/internal/lib/connectors/services/system/WorkflowApiClient.test.js b/test/internal/lib/connectors/services/system/WorkflowApiClient.test.js
diff --git a/test/internal/modules/charge-information/controllers/charge-information-workflow.test.js b/test/internal/modules/charge-information/controllers/charge-information-workflow.test.js
@@ -64,7 +64,6 @@ experiment('internal/modules/charge-information/controller', () => {
 
     sandbox.stub(services.water.chargeVersionWorkflows, 'postChargeVersionWorkflow').resolves()
     sandbox.stub(services.water.chargeVersionWorkflows, 'deleteChargeVersionWorkflow').resolves()
-    sandbox.stub(services.system.workflow, 'supplementary').resolves()
   })
 
   afterEach(async () => {
@@ -196,11 +195,6 @@ experiment('internal/modules/charge-information/controller', () => {
       await controller.postRemoveChargeInformationWorkflow(request, h)
     })
 
-    test('calls the system repo to flag for supplementary billing', async () => {
-      const [workflowId] = services.system.workflow.supplementary.lastCall.args
-      expect(workflowId).to.equal('test-charge-version-workflow-id')
-    })
-
     test('deletes the charge version workflow', async () => {
       const [workflowId] = services.water.chargeVersionWorkflows.deleteChargeVersionWorkflow.lastCall.args
       expect(workflowId).to.equal('test-charge-version-workflow-id')

diff --git a/test/internal/modules/view-licences/controller.test.js b/test/internal/modules/view-licences/controller.test.js
@@ -27,8 +27,7 @@ experiment('internal/modules/billing/controllers/bills-tab', () => {
       response: sandbox.stub().returns(),
       redirect: sandbox.stub()
     }
-    sandbox.stub(services.water.licences, 'postMarkLicenceForSupplementaryBilling').resolves()
-    sandbox.stub(services.system.licences, 'supplementary').resolves()
+    sandbox.stub(services.system.returns, 'supplementary').resolves()
     sandbox.stub(services.water.licences, 'getDocumentByLicenceId').resolves({
       metadata: {},
       system_external_id: 'test id'
@@ -421,9 +420,6 @@ experiment('internal/modules/billing/controllers/bills-tab', () => {
         params: {
           licenceId: tempLicenceId
         },
-        headers: {
-          cookie: 'taste=yummy'
-        },
         payload: {
           returnId: tempReturnId
         },
@@ -439,8 +435,8 @@ experiment('internal/modules/billing/controllers/bills-tab', () => {
         await controller.postMarkLicenceForSupplementaryBilling(request, h)
       })
 
-      test('calls system', () => {
-        expect(services.system.licences.supplementary.calledWith(tempReturnId)).to.be.true()
+      test('calls system with the returnId', () => {
+        expect(services.system.returns.supplementary.calledWith(tempReturnId)).to.be.true()
       })
     })
 
@@ -452,9 +448,6 @@ experiment('internal/modules/billing/controllers/bills-tab', () => {
         params: {
           licenceId: tempLicenceId
         },
-        headers: {
-          cookie: 'taste=yummy'
-        },
         payload: {},
         pre: {
           document: {
@@ -468,10 +461,6 @@ experiment('internal/modules/billing/controllers/bills-tab', () => {
         await controller.postMarkLicenceForSupplementaryBilling(request, h)
       })
 
-      test('calls the backend', () => {
-        expect(services.water.licences.postMarkLicenceForSupplementaryBilling.calledWith(tempLicenceId)).to.be.true()
-      })
-
       test('returns the correct view data objects', async () => {
         const keys = Object.keys(h.view.lastCall.args[1])