Move supplementary unflagging services (#1769)

https://eaflood.atlassian.net/browse/WATER-4201

> Part of the work to support two-part tariff supplementary bill runs

In [Move reusable supplementary billing services](#1760) we shifted some existing supplementary billing services to make them reusable by the two-part tariff supplementary billing engine we're building.

Then we spotted [some two-part tariff billing services we could remove and reuse](#1765).

The final step of supporting two-part tariff supplementary, like standard supplementary, is the flagging.

When changes are made to licences, they are flagged for supplementary billing, and it is these 'flags' that drive whether a licence is considered part of a supplementary bill run. Just as complex as setting the flags is unsetting them. Various actions, either driven by the users or from logic in the engines, trigger licences becoming unflagged.

Two scenarios a two-part tariff supplementary shares with a standard supplementary, are

- when a licence after processing is found _not_ to have generated a bill
- when the bill has been sent and the licence can be considered as having been 'processed' for supplementary billing

We already have services that handle these scenarios, which are called at the appropriate times. So, before adding the two-part tariff supplementary flagging logic, we prepare the existing services for reuse.

Author: Cruikshanks

app/services/bill-runs/send/update-invoice-numbers.service.js

@@ -11,7 +11,7 @@ const ExpandedError = require('../../../errors/expanded.error.js')
 const { calculateAndLogTimeTaken, timestampForPostgres } = require('../../../lib/general.lib.js')
 const ChargingModuleSendBillRunRequest = require('../../../requests/charging-module/send-bill-run.request.js')
 const ChargingModuleViewBillRunRequest = require('../../../requests/charging-module/view-bill-run.request.js')
-const UnflagBilledLicencesService = require('../supplementary/unflag-billed-licences.service.js')
+const UnflagBilledSupplementaryLicencesService = require('../unflag-billed-supplementary-licences.service.js')
 
 /**
  * Updates a bill run and its bill records with the invoice numbers generated by the Charging Module API
@@ -41,7 +41,7 @@ async function go(billRun) {
     await _updateBillRun(billRunId, externalBillRun)
 
     if (billRun.batchType === 'supplementary') {
-      await UnflagBilledLicencesService.go(billRun)
+      await UnflagBilledSupplementaryLicencesService.go(billRun)
     }
 
     calculateAndLogTimeTaken(startTime, 'Send bill run complete', { billRun })

app/services/bill-runs/supplementary/process-bill-run.service.js

@@ -13,7 +13,7 @@ const { calculateAndLogTimeTaken, currentTimeInNanoseconds } = require('../../..
 const HandleErroredBillRunService = require('../handle-errored-bill-run.service.js')
 const LegacyRefreshBillRunRequest = require('../../../requests/legacy/refresh-bill-run.request.js')
 const ProcessBillingPeriodService = require('./process-billing-period.service.js')
-const UnflagUnbilledLicencesService = require('./unflag-unbilled-licences.service.js')
+const UnflagUnbilledSupplementaryLicencesService = require('../unflag-unbilled-supplementary-licences.service.js')
 
 /**
  * Process a given bill run for the given billing periods. In this case, "process" means that we create the
@@ -81,7 +81,7 @@ async function _finaliseBillRun(billRun, accumulatedLicenceIds, resultsOfProcess
   // .findByIds() so we spread it into an array
   const allLicenceIds = [...new Set(accumulatedLicenceIds)]
 
-  await UnflagUnbilledLicencesService.go(billRun, allLicenceIds)
+  await UnflagUnbilledSupplementaryLicencesService.go(billRun, allLicenceIds)
 
   // We set `isPopulated` to `true` if at least one processing result was truthy
   const isPopulated = resultsOfProcessing.some((result) => {

app/services/bill-runs/supplementary/unflag-billed-licences.service.js renamed to app/services/bill-runs/unflag-billed-supplementary-licences.service.js

@@ -2,10 +2,10 @@
 
 /**
  * Unflag all licences in a bill run that resulted in a billing invoice (they are billed)
- * @module UnflagBilledLicencesService
+ * @module UnflagBilledSupplementaryLicencesService
  */
 
-const LicenceModel = require('../../../models/licence.model.js')
+const LicenceModel = require('../../models/licence.model.js')
 
 /**
  * Unflag any licences that were billed as part of a bill run

app/services/bill-runs/supplementary/unflag-unbilled-licences.service.js renamed to app/services/bill-runs/unflag-unbilled-supplementary-licences.service.js

@@ -5,7 +5,7 @@
  * @module UnflagUnbilledLicencesService
  */
 
-const LicenceModel = require('../../../models/licence.model.js')
+const LicenceModel = require('../../models/licence.model.js')
 
 /**
  * Unflag any licences that were not billed as part of a bill run

test/services/bill-runs/send/update-invoice-numbers.service.test.js

@@ -15,12 +15,12 @@ const ExpandedError = require('../../../../app/errors/expanded.error.js')
 // Things we need to stub
 const ChargingModuleSendBillRunRequest = require('../../../../app/requests/charging-module/send-bill-run.request.js')
 const ChargingModuleViewBillRunRequest = require('../../../../app/requests/charging-module/view-bill-run.request.js')
-const UnflagBilledLicencesService = require('../../../../app/services/bill-runs/supplementary/unflag-billed-licences.service.js')
+const UnflagBilledSupplementaryLicencesService = require('../../../../app/services/bill-runs/unflag-billed-supplementary-licences.service.js')
 
 // Thing under test
 const UpdateInvoiceNumbersService = require('../../../../app/services/bill-runs/send/update-invoice-numbers.service.js')
 
-describe('Bill Runs - Update Invoice Numbers service', () => {
+describe('Bill Runs - Send- Update Invoice Numbers service', () => {
   let billRun
   let chargingModuleSendBillRunRequestStub
   let chargingModuleViewBillRunRequestStub
@@ -33,7 +33,7 @@ describe('Bill Runs - Update Invoice Numbers service', () => {
   beforeEach(async () => {
     chargingModuleSendBillRunRequestStub = Sinon.stub(ChargingModuleSendBillRunRequest, 'send')
     chargingModuleViewBillRunRequestStub = Sinon.stub(ChargingModuleViewBillRunRequest, 'send')
-    unflagBilledLicencesServiceStub = Sinon.stub(UnflagBilledLicencesService, 'go').resolves()
+    unflagBilledLicencesServiceStub = Sinon.stub(UnflagBilledSupplementaryLicencesService, 'go').resolves()
 
     billPatchStub = Sinon.stub().resolves()
     billRunPatchStub = Sinon.stub().resolves()

test/services/bill-runs/supplementary/process-bill-run.service.test.js

@@ -10,35 +10,40 @@ const { expect } = Code
 
 // Test helpers
 const BillRunError = require('../../../../app/errors/bill-run.error.js')
-const BillRunHelper = require('../../../support/helpers/bill-run.helper.js')
-const BillRunModel = require('../../../../app/models/bill-run.model.js')
 
 // Things we need to stub
+const BillRunModel = require('../../../../app/models/bill-run.model.js')
 const ChargingModuleGenerateBillRunRequest = require('../../../../app/requests/charging-module/generate-bill-run.request.js')
 const FeatureFlagsConfig = require('../../../../config/feature-flags.config.js')
 const FetchChargeVersionsService = require('../../../../app/services/bill-runs/supplementary/fetch-charge-versions.service.js')
 const HandleErroredBillRunService = require('../../../../app/services/bill-runs/handle-errored-bill-run.service.js')
 const LegacyRefreshBillRunRequest = require('../../../../app/requests/legacy/refresh-bill-run.request.js')
 const ProcessBillingPeriodService = require('../../../../app/services/bill-runs/supplementary/process-billing-period.service.js')
-const UnflagUnbilledLicencesService = require('../../../../app/services/bill-runs/supplementary/unflag-unbilled-licences.service.js')
+const UnflagUnbilledSupplementaryLicencesService = require('../../../../app/services/bill-runs/unflag-unbilled-supplementary-licences.service.js')
 
 // Thing under test
-const SupplementaryProcessBillRunService = require('../../../../app/services/bill-runs/supplementary/process-bill-run.service.js')
+const ProcessBillRunService = require('../../../../app/services/bill-runs/supplementary/process-bill-run.service.js')
 
-describe('Supplementary Process Bill Run service', () => {
+describe('Bill Runs - Supplementary - Process Bill Run service', () => {
   const billingPeriods = [
     { startDate: new Date('2023-04-01'), endDate: new Date('2024-03-31') },
     { startDate: new Date('2022-04-01'), endDate: new Date('2023-03-31') }
   ]
+  const billRun = { id: '410c84a5-39d3-441a-97ca-6104e14d00a2' }
 
-  let billRun
+  let billRunPatchStub
   let chargingModuleGenerateBillRunRequestStub
   let handleErroredBillRunStub
   let legacyRefreshBillRunRequestStub
   let notifierStub
 
   beforeEach(async () => {
-    billRun = await BillRunHelper.add()
+    billRunPatchStub = Sinon.stub().resolves()
+
+    Sinon.stub(BillRunModel, 'query').returns({
+      findById: Sinon.stub().returnsThis(),
+      patch: billRunPatchStub
+    })
 
     handleErroredBillRunStub = Sinon.stub(HandleErroredBillRunService, 'go')
     chargingModuleGenerateBillRunRequestStub = Sinon.stub(ChargingModuleGenerateBillRunRequest, 'send')
@@ -62,20 +67,31 @@ describe('Supplementary Process Bill Run service', () => {
   describe('when the service is called', () => {
     beforeEach(() => {
       Sinon.stub(FetchChargeVersionsService, 'go').resolves({ chargeVersions: [], licenceIdsForPeriod: [] })
-      Sinon.stub(UnflagUnbilledLicencesService, 'go')
+      Sinon.stub(UnflagUnbilledSupplementaryLicencesService, 'go')
     })
 
     describe('and nothing is billed', () => {
       beforeEach(() => {
         Sinon.stub(ProcessBillingPeriodService, 'go').resolves(false)
       })
 
-      it('sets the Bill Run status to empty', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+      it('sets the bill run status first to "processing" and then to "empty"', async () => {
+        await ProcessBillRunService.go(billRun, billingPeriods)
+
+        expect(billRunPatchStub.calledTwice).to.be.true()
+        expect(billRunPatchStub.firstCall.firstArg).to.equal({ status: 'processing' })
+        expect(billRunPatchStub.secondCall.firstArg).to.equal({ status: 'empty' })
+      })
+
+      it('logs the time taken', async () => {
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
-        const result = await BillRunModel.query().findById(billRun.id)
+        const args = notifierStub.omg.firstCall.args
 
-        expect(result.status).to.equal('empty')
+        expect(args[0]).to.equal('Process bill run complete')
+        expect(args[1].timeTakenMs).to.exist()
+        expect(args[1].billRunId).to.equal(billRun.id)
+        expect(args[1].type).to.equal('supplementary')
       })
     })
 
@@ -84,28 +100,27 @@ describe('Supplementary Process Bill Run service', () => {
         Sinon.stub(ProcessBillingPeriodService, 'go').resolves(true)
       })
 
-      it('sets the Bill Run status to processing', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
-
-        const result = await BillRunModel.query().findById(billRun.id)
+      it('sets the bill run status to "processing"', async () => {
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
-        expect(result.status).to.equal('processing')
+        expect(billRunPatchStub.calledOnce).to.be.true()
+        expect(billRunPatchStub.firstCall.firstArg).to.equal({ status: 'processing' }, { skip: ['updatedAt'] })
       })
 
       it('tells the charging module API to "generate" the bill run', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
         expect(chargingModuleGenerateBillRunRequestStub.called).to.be.true()
       })
 
       it('tells the legacy service to start its refresh job', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
         expect(legacyRefreshBillRunRequestStub.called).to.be.true()
       })
 
-      it('it logs the time taken', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+      it('logs the time taken', async () => {
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
         const args = notifierStub.omg.firstCall.args
 
@@ -127,15 +142,15 @@ describe('Supplementary Process Bill Run service', () => {
       })
 
       it('calls HandleErroredBillRunService with appropriate error code', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
         const handlerArgs = handleErroredBillRunStub.firstCall.args
 
         expect(handlerArgs[1]).to.equal(BillRunModel.errorCodes.failedToProcessChargeVersions)
       })
 
       it('logs the error', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
         const args = notifierStub.omfg.firstCall.args
 
@@ -158,15 +173,15 @@ describe('Supplementary Process Bill Run service', () => {
         })
 
         it('calls HandleErroredBillRunService with the error code', async () => {
-          await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+          await ProcessBillRunService.go(billRun, billingPeriods)
 
           const handlerArgs = handleErroredBillRunStub.firstCall.args
 
           expect(handlerArgs[1]).to.equal(BillRunModel.errorCodes.failedToPrepareTransactions)
         })
 
         it('logs the error', async () => {
-          await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+          await ProcessBillRunService.go(billRun, billingPeriods)
 
           const args = notifierStub.omfg.firstCall.args
 
@@ -186,19 +201,19 @@ describe('Supplementary Process Bill Run service', () => {
 
         Sinon.stub(FetchChargeVersionsService, 'go').resolves({ chargeVersions: [], licenceIdsForPeriod: [] })
         Sinon.stub(ProcessBillingPeriodService, 'go').resolves(false)
-        Sinon.stub(UnflagUnbilledLicencesService, 'go').rejects(thrownError)
+        Sinon.stub(UnflagUnbilledSupplementaryLicencesService, 'go').rejects(thrownError)
       })
 
       it('calls HandleErroredBillRunService without an error code', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
         const handlerArgs = handleErroredBillRunStub.firstCall.args
 
         expect(handlerArgs[1]).to.be.undefined()
       })
 
       it('logs the error', async () => {
-        await SupplementaryProcessBillRunService.go(billRun, billingPeriods)
+        await ProcessBillRunService.go(billRun, billingPeriods)
 
         const args = notifierStub.omfg.firstCall.args
 

test/services/bill-runs/supplementary/unflag-billed-licences.service.test.js renamed to test/services/bill-runs/unflag-billed-supplementary-licences.service.test.js

@@ -8,16 +8,16 @@ const { describe, it, beforeEach } = (exports.lab = Lab.script())
 const { expect } = Code
 
 // Test helpers
-const BillHelper = require('../../../support/helpers/bill.helper.js')
-const BillLicenceHelper = require('../../../support/helpers/bill-licence.helper.js')
-const LicenceHelper = require('../../../support/helpers/licence.helper.js')
-const RegionHelper = require('../../../support/helpers/region.helper.js')
-const WorkflowHelper = require('../../../support/helpers/workflow.helper.js')
+const BillHelper = require('../../support/helpers/bill.helper.js')
+const BillLicenceHelper = require('../../support/helpers/bill-licence.helper.js')
+const LicenceHelper = require('../../support/helpers/licence.helper.js')
+const RegionHelper = require('../../support/helpers/region.helper.js')
+const WorkflowHelper = require('../../support/helpers/workflow.helper.js')
 
 // Thing under test
-const UnflagBilledLicencesService = require('../../../../app/services/bill-runs/supplementary/unflag-billed-licences.service.js')
+const UnflagBilledSupplementaryLicencesService = require('../../../app/services/bill-runs/unflag-billed-supplementary-licences.service.js')
 
-describe('Unflag Billed Licences service', () => {
+describe('Bill Runs - Unflag Billed Supplementary Licences service', () => {
   const { id: regionId } = RegionHelper.select(RegionHelper.TEST_REGION_INDEX)
 
   let billRun
@@ -64,7 +64,7 @@ describe('Unflag Billed Licences service', () => {
     })
 
     it('unflags those in the same region, not in workflow, and that were last updated before the bill run was created', async () => {
-      await UnflagBilledLicencesService.go(billRun)
+      await UnflagBilledSupplementaryLicencesService.go(billRun)
 
       let licenceBeingChecked
 
@@ -120,7 +120,7 @@ describe('Unflag Billed Licences service', () => {
     })
 
     it('unflags only those licences in the bill run that are not in workflow, and that were last updated before the bill run was created)', async () => {
-      await UnflagBilledLicencesService.go(billRun)
+      await UnflagBilledSupplementaryLicencesService.go(billRun)
 
       let licenceBeingChecked
 

test/services/bill-runs/supplementary/unflag-unbilled-licences.service.test.js renamed to test/services/bill-runs/unflag-unbilled-supplementary-licences.service.test.js

@@ -8,15 +8,15 @@ const { describe, it, beforeEach } = (exports.lab = Lab.script())
 const { expect } = Code
 
 // Test helpers
-const BillHelper = require('../../../support/helpers/bill.helper.js')
-const BillLicenceHelper = require('../../../support/helpers/bill-licence.helper.js')
-const LicenceHelper = require('../../../support/helpers/licence.helper.js')
-const WorkflowHelper = require('../../../support/helpers/workflow.helper.js')
+const BillHelper = require('../../support/helpers/bill.helper.js')
+const BillLicenceHelper = require('../../support/helpers/bill-licence.helper.js')
+const LicenceHelper = require('../../support/helpers/licence.helper.js')
+const WorkflowHelper = require('../../support/helpers/workflow.helper.js')
 
 // Thing under test
-const UnflagUnbilledLicencesService = require('../../../../app/services/bill-runs/supplementary/unflag-unbilled-licences.service.js')
+const UnflagUnbilledSupplementaryLicencesService = require('../../../app/services/bill-runs/unflag-unbilled-supplementary-licences.service.js')
 
-describe('Unflag unbilled licences service', () => {
+describe('Bill Runs - Unflag Unbilled Supplementary Licences service', () => {
   let billRun
 
   describe('when there are licences flagged for SROC supplementary billing', () => {
@@ -59,7 +59,7 @@ describe('Unflag unbilled licences service', () => {
       describe('which were not billed', () => {
         describe('and are not in workflow or updated after the bill run was created', () => {
           it('unflags them for SROC supplementary billing', async () => {
-            await UnflagUnbilledLicencesService.go(billRun, allLicenceIds)
+            await UnflagUnbilledSupplementaryLicencesService.go(billRun, allLicenceIds)
 
             const licenceToBeChecked = await licenceNotBilledInBillRun.$query()
 
@@ -69,7 +69,7 @@ describe('Unflag unbilled licences service', () => {
 
         describe('but are in workflow', () => {
           it('leaves flagged for SROC supplementary billing', async () => {
-            await UnflagUnbilledLicencesService.go(billRun, allLicenceIds)
+            await UnflagUnbilledSupplementaryLicencesService.go(billRun, allLicenceIds)
 
             const licenceToBeChecked = await licenceNotBilledInBillRunAndWorkflow.$query()
 
@@ -79,7 +79,7 @@ describe('Unflag unbilled licences service', () => {
 
         describe('but were updated after the bill run was created', () => {
           it('leaves flagged for SROC supplementary billing', async () => {
-            await UnflagUnbilledLicencesService.go(billRun, allLicenceIds)
+            await UnflagUnbilledSupplementaryLicencesService.go(billRun, allLicenceIds)
 
             const licenceToBeChecked = await licenceNotBilledInBillRunAndUpdated.$query()
 
@@ -96,7 +96,7 @@ describe('Unflag unbilled licences service', () => {
         })
 
         it('are left flagged (include_in_sroc_billing still true)', async () => {
-          await UnflagUnbilledLicencesService.go(billRun, allLicenceIds)
+          await UnflagUnbilledSupplementaryLicencesService.go(billRun, allLicenceIds)
 
           const licenceToBeChecked = await licenceBilledInBillRun.$query()
 
@@ -107,7 +107,7 @@ describe('Unflag unbilled licences service', () => {
 
     describe('those licences not in the current bill run', () => {
       it('leaves flagged (include_in_sroc_billing still true)', async () => {
-        await UnflagUnbilledLicencesService.go(billRun, allLicenceIds)
+        await UnflagUnbilledSupplementaryLicencesService.go(billRun, allLicenceIds)
 
         const licenceToBeChecked = await licenceNotInBillRun.$query()