Replace live bill run checking in engine (#843)

https://eaflood.atlassian.net/browse/WATER-4416
https://eaflood.atlassian.net/browse/WATER-4379

We spotted an issue with removing a bill from a bill run and it not setting the supplementary flags on the licences involved correctly. We've fixed that, but it's led to our QA team rigorously re-creating bill runs over and over.

Doing this they spotted we did have an issue with the `CheckLiveBillRunService` that we fixed in [Fix broken CheckLiveBillRunService](#841).

Now they've spotted something else. `CheckLiveBillRunService` includes batch type as a filter. This means it _will_ allow you to create a supplementary bill run, for example, even if another type of bill run is 'in progress' (queued, processing, or ready). At the time we thought that was ok but now we know better.

We spotted this in our work to migrate the setup bill run journey (see [Handle bill run setup matches an existing bill run](#810)). We hoped we'd be using our version of the journey by now and we could quietly retire `CheckLiveBillRunService` as part of ongoing maintenance and no one would be any the wiser (it has been live for 7 months now!)

However, our QA team would rather clear the issues currently found before bringing in the new setup journey for testing.

So, rather than fix the service, we'll replace it with `DetermineBlockingBillRunService` which matches what the legacy service does and deals with this scenario.

app/services/bill-runs/setup/determine-blocking-bill-run.service.js → app/services/bill-runs/determine-blocking-bill-run.service.js

@@ -2,7 +2,7 @@
 
 /**
  * Determines if an existing bill run will block the one a user is trying to setup
- * @module ExistsService
+ * @module DetermineBlockingBillRunService
  */
 
 const FetchLiveBillRunsService = require('./fetch-live-bill-runs.service.js')

app/services/bill-runs/setup/fetch-live-bill-runs.service.js → app/services/bill-runs/fetch-live-bill-runs.service.js

@@ -5,7 +5,7 @@
  * @module FetchBillRunsForRegionService
  */
 
-const BillRunModel = require('../../../models/bill-run.model.js')
+const BillRunModel = require('../../models/bill-run.model.js')
 
 const LAST_PRESROC_YEAR = 2022
 

app/services/bill-runs/setup/fetch-matching-bill-run.service.js → app/services/bill-runs/fetch-matching-bill-run.service.js

@@ -5,7 +5,7 @@
  * @module ExistsService
  */
 
-const BillRunModel = require('../../../models/bill-run.model.js')
+const BillRunModel = require('../../models/bill-run.model.js')
 
 /**
  * Fetch bill run(s) that match the options provided

app/services/bill-runs/initiate-bill-run.service.js

@@ -7,9 +7,9 @@
 
 const BillRunModel = require('../../models/bill-run.model.js')
 const ChargingModuleCreateBillRunRequest = require('../../requests/charging-module/create-bill-run.request.js')
-const CheckLiveBillRunService = require('./check-live-bill-run.service.js')
 const CreateBillRunService = require('./create-bill-run.service.js')
 const CreateBillRunEventService = require('./create-bill-run-event.service.js')
+const DetermineBlockingBillRunService = require('./determine-blocking-bill-run.service.js')
 const ExpandedError = require('../../errors/expanded.error.js')
 
 /**
@@ -26,11 +26,7 @@ const ExpandedError = require('../../errors/expanded.error.js')
  * @returns {Promise<module:BillRunModel>} The newly created bill run instance
  */
 async function go (financialYearEndings, regionId, batchType, userEmail) {
-  const liveBillRunExists = await CheckLiveBillRunService.go(regionId, financialYearEndings.toFinancialYearEnding, batchType)
-
-  if (liveBillRunExists) {
-    throw new ExpandedError('Batch already live for region', { regionId })
-  }
+  await _billRunBlocked(regionId, batchType, financialYearEndings.toFinancialYearEnding)
 
   const chargingModuleResult = await ChargingModuleCreateBillRunRequest.send(regionId, 'sroc')
 
@@ -61,6 +57,23 @@ function _billRunOptions (chargingModuleResult, batchType) {
   return options
 }
 
+async function _billRunBlocked (regionId, batchType, financialEndYear) {
+  const matchResults = await DetermineBlockingBillRunService.go(regionId, batchType, financialEndYear)
+
+  // No matches so we can create the bill run
+  if (matchResults.length === 0) {
+    return
+  }
+
+  // You can only have one SROC and PRESROC supplementary being processed at any time. If less than 2 then we can create
+  // a bill run
+  if (batchType === 'supplementary' && matchResults.length < 2) {
+    return
+  }
+
+  throw new ExpandedError('Batch already live for region', { billRunId: matchResults[0].id })
+}
+
 module.exports = {
   go
 }

app/services/bill-runs/setup/exists.service.js

@@ -6,7 +6,7 @@
  */
 
 const CreatePresenter = require('../../../presenters/bill-runs/setup/create.presenter.js')
-const DetermineBlockingBillRunService = require('./determine-blocking-bill-run.service.js')
+const DetermineBlockingBillRunService = require('../determine-blocking-bill-run.service.js')
 const SessionModel = require('../../../models/session.model.js')
 
 const { determineCurrentFinancialYear } = require('../../../lib/general.lib.js')

test/services/bill-runs/setup/determine-blocking-bill-run.service.test.js → test/services/bill-runs/determine-blocking-bill-run.service.test.js

@@ -8,15 +8,15 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
 const { expect } = Code
 
 // Test helpers
-const BillRunHelper = require('../../../support/helpers/bill-run.helper.js')
-const DatabaseSupport = require('../../../support/database.js')
-const { determineCurrentFinancialYear } = require('../../../../app/lib/general.lib.js')
-const RegionHelper = require('../../../support/helpers/region.helper.js')
+const BillRunHelper = require('../../support/helpers/bill-run.helper.js')
+const DatabaseSupport = require('../../support/database.js')
+const { determineCurrentFinancialYear } = require('../../../app/lib/general.lib.js')
+const RegionHelper = require('../../support/helpers/region.helper.js')
 
 // Thing under test
-const DetermineBlockingBillRunService = require('../../../../app/services/bill-runs/setup/determine-blocking-bill-run.service.js')
+const DetermineBlockingBillRunService = require('../../../app/services/bill-runs/determine-blocking-bill-run.service.js')
 
-describe('Bill Runs Setup Determine Blocking Bill Run service', () => {
+describe('Determine Blocking Bill Run service', () => {
   let batchType
   let financialEndYear
   let regionId

test/services/bill-runs/setup/fetch-live-bill-runs.service.test.js → test/services/bill-runs/fetch-live-bill-runs.service.test.js

@@ -8,14 +8,14 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
 const { expect } = Code
 
 // Test helpers
-const BillRunHelper = require('../../../support/helpers/bill-run.helper.js')
-const DatabaseSupport = require('../../../support/database.js')
-const RegionHelper = require('../../../support/helpers/region.helper.js')
+const BillRunHelper = require('../../support/helpers/bill-run.helper.js')
+const DatabaseSupport = require('../../support/database.js')
+const RegionHelper = require('../../support/helpers/region.helper.js')
 
 // Thing under test
-const FetchLiveBillRunsService = require('../../../../app/services/bill-runs/setup/fetch-live-bill-runs.service.js')
+const FetchLiveBillRunsService = require('../../../app/services/bill-runs/fetch-live-bill-runs.service.js')
 
-describe('Bill Runs Setup Fetch Live Bill Runs service', () => {
+describe('Fetch Live Bill Runs service', () => {
   let financialYearEnding
   let regionId
 

test/services/bill-runs/setup/fetch-matching-bill-run.service.test.js → test/services/bill-runs/fetch-matching-bill-run.service.test.js

@@ -8,14 +8,14 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
 const { expect } = Code
 
 // Test helpers
-const BillRunHelper = require('../../../support/helpers/bill-run.helper.js')
-const DatabaseSupport = require('../../../support/database.js')
-const RegionHelper = require('../../../support/helpers/region.helper.js')
+const BillRunHelper = require('../../support/helpers/bill-run.helper.js')
+const DatabaseSupport = require('../../support/database.js')
+const RegionHelper = require('../../support/helpers/region.helper.js')
 
 // Thing under test
-const FetchMatchingBillRunService = require('../../../../app/services/bill-runs/setup/fetch-matching-bill-run.service.js')
+const FetchMatchingBillRunService = require('../../../app/services/bill-runs/fetch-matching-bill-run.service.js')
 
-describe('Bill Runs Setup Fetch Matching Bill Run service', () => {
+describe('Fetch Matching Bill Run service', () => {
   let matchingBillRunId
   let regionId
 

test/services/bill-runs/initiate-bill-run.service.test.js

@@ -16,16 +16,17 @@ const RegionHelper = require('../../support/helpers/region.helper.js')
 
 // Things we need to stub
 const ChargingModuleCreateBillRunRequest = require('../../../app/requests/charging-module/create-bill-run.request.js')
-const CheckLiveBillRunService = require('../../../app/services/bill-runs/check-live-bill-run.service.js')
+const DetermineBlockingBillRunService = require('../../../app/services/bill-runs/determine-blocking-bill-run.service.js')
 const SupplementaryProcessBillRunService = require('../../../app/services/bill-runs/supplementary/process-bill-run.service.js')
 
 // Thing under test
 const InitiateBillRunService = require('../../../app/services/bill-runs/initiate-bill-run.service.js')
 
 describe('Initiate Bill Run service', () => {
-  const batchType = 'supplementary'
   const financialYearEndings = { fromFinancialYearEnding: 2023, toFinancialYearEnding: 2024 }
   const user = '[email protected]'
+
+  let batchType
   let regionId
 
   beforeEach(async () => {
@@ -34,8 +35,6 @@ describe('Initiate Bill Run service', () => {
     const region = await RegionHelper.add()
     regionId = region.id
 
-    Sinon.stub(CheckLiveBillRunService, 'go').resolves(false)
-
     // The InitiateBillRun service does not await the call to the ProcessBillRunService. It is intended to
     // kick of the process and then move on. This is why we simply stub it in the tests.
     Sinon.stub(SupplementaryProcessBillRunService, 'go')
@@ -54,6 +53,10 @@ describe('Initiate Bill Run service', () => {
     }
 
     beforeEach(() => {
+      batchType = 'supplementary'
+
+      Sinon.stub(DetermineBlockingBillRunService, 'go').resolves([])
+
       Sinon.stub(ChargingModuleCreateBillRunRequest, 'send').resolves({
         succeeded: true,
         response: {
@@ -101,6 +104,8 @@ describe('Initiate Bill Run service', () => {
   describe('when initiating a bill run fails', () => {
     describe('because a bill run could not be created in the Charging Module', () => {
       beforeEach(() => {
+        Sinon.stub(DetermineBlockingBillRunService, 'go').resolves([])
+
         Sinon.stub(ChargingModuleCreateBillRunRequest, 'send').resolves({
           succeeded: false,
           response: {
@@ -132,17 +137,19 @@ describe('Initiate Bill Run service', () => {
       })
     })
 
-    describe('because a bill run already exists for this region, financial year and type', () => {
+    describe('because a live bill run already exists for this region, financial year and type', () => {
       beforeEach(() => {
-        CheckLiveBillRunService.go.resolves(true)
+        batchType = 'annual'
+
+        Sinon.stub(DetermineBlockingBillRunService, 'go').resolves([{ id: 'becf430d-f6dd-45a3-b943-42683f7bb889' }])
       })
 
       it('rejects with an appropriate error', async () => {
         const err = await expect(InitiateBillRunService.go(financialYearEndings, regionId, batchType, user)).to.reject()
 
         expect(err).to.be.an.error()
         expect(err.message).to.equal('Batch already live for region')
-        expect(err.regionId).to.equal(regionId)
+        expect(err.billRunId).to.equal('becf430d-f6dd-45a3-b943-42683f7bb889')
       })
     })
   })

test/services/bill-runs/setup/exists.service.test.js

@@ -14,7 +14,7 @@ const { determineCurrentFinancialYear } = require('../../../../app/lib/general.l
 const SessionHelper = require('../../../support/helpers/session.helper.js')
 
 // Things we need to stub
-const DetermineBlockingBillRunService = require('../../../../app/services/bill-runs/setup/determine-blocking-bill-run.service.js')
+const DetermineBlockingBillRunService = require('../../../../app/services/bill-runs/determine-blocking-bill-run.service.js')
 
 // Thing under test
 const ExistsService = require('../../../../app/services/bill-runs/setup/exists.service.js')