From 417f6edc013fadaa7796d15fcc4b194fc8d69318 Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 15:17:21 +0000
Subject: [PATCH 01/10] Do not create SROC bill run if one already exists

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

The legacy system will not create a PRESROC bill run if one exists with the same region, financial year and type. There are some exceptions, for example, if the previous bill run has a status of EMPTY or ERROR.

The same logic needs to be implemented for SROC supplementary bill runs.

From e4ea453361a2a0f826d0647759c270ac5197d369 Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 13:09:43 +0000
Subject: [PATCH 02/10] Create skeleton service and failing tests

---
 .../check-live-bill-run.service.js            | 26 ++++++++
 .../check-live-bill-run.service.test.js       | 64 +++++++++++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 app/services/supplementary-billing/check-live-bill-run.service.js
 create mode 100644 test/services/supplementary-billing/check-live-bill-run.service.test.js

diff --git a/app/services/supplementary-billing/check-live-bill-run.service.js b/app/services/supplementary-billing/check-live-bill-run.service.js
new file mode 100644
index 0000000000..a91d04e0b4
--- /dev/null
+++ b/app/services/supplementary-billing/check-live-bill-run.service.js
@@ -0,0 +1,26 @@
+'use strict'
+
+/**
+ * Checks whether a "live" bill run exists for the specified region, scheme, type and financial year
+ * @module CheckLiveBillRunService
+ */
+
+/**
+ * Check whether a "live" bill run exists for the specified region, scheme, type and financial year
+ *
+ * We define "live" as having the status `processing`, `ready` or `review`
+ *
+ * @param {*} region
+ * @param {*} scheme
+ * @param {*} type
+ * @param {*} financialYeara
+ *
+ * @returns {Boolean} Whether a "live" bill run exists
+ */
+async function go (region, scheme, type, financialYear) {
+  return true
+}
+
+module.exports = {
+  go
+}
diff --git a/test/services/supplementary-billing/check-live-bill-run.service.test.js b/test/services/supplementary-billing/check-live-bill-run.service.test.js
new file mode 100644
index 0000000000..23e6717e8b
--- /dev/null
+++ b/test/services/supplementary-billing/check-live-bill-run.service.test.js
@@ -0,0 +1,64 @@
+'use strict'
+
+// Test framework dependencies
+const Lab = require('@hapi/lab')
+const Code = require('@hapi/code')
+
+const { describe, it, beforeEach } = exports.lab = Lab.script()
+const { expect } = Code
+
+// Test helpers
+const BillingBatchHelper = require('../../support/helpers/water/billing-batch.helper.js')
+const DatabaseHelper = require('../../support/helpers/database.helper.js')
+
+// Thing under test
+const CheckLiveBillRunService = require('../../../app/services/supplementary-billing/check-live-bill-run.service.js')
+
+describe.only('Check Live Bill Run service', () => {
+  let billRunRegion
+
+  beforeEach(async () => {
+    await DatabaseHelper.clean()
+  })
+
+  describe('when an sroc supplementary bill run exists for this region and financial year', () => {
+    describe('with a status considered to be "live"', () => {
+      beforeEach(async () => {
+        const billRun = await BillingBatchHelper.add()
+        billRunRegion = billRun.regionId
+      })
+
+      it('returns `true`', async () => {
+        const result = await CheckLiveBillRunService.go(billRunRegion, 'sroc', 'supplementary', 2023)
+
+        expect(result).to.be.true()
+      })
+    })
+
+    describe('with a status not considered to be "live"', () => {
+      beforeEach(async () => {
+        const billRun = await BillingBatchHelper.add({ status: 'sent' })
+        billRunRegion = billRun.regionId
+      })
+
+      it('returns `false`', async () => {
+        const result = await CheckLiveBillRunService.go(billRunRegion, 'sroc', 'supplementary', 2023)
+
+        expect(result).to.be.false()
+      })
+    })
+  })
+
+  describe('when an sroc supplementary bill run does not exist for this region and financial year', () => {
+    beforeEach(async () => {
+      const billRun = await BillingBatchHelper.add({ fromFinancialYearEnding: 2024, toFinancialYearEnding: 2024 })
+      billRunRegion = billRun.regionId
+    })
+
+    it('returns `false`', async () => {
+      const result = await CheckLiveBillRunService.go(billRunRegion, 'sroc', 'supplementary', 2023)
+
+      expect(result).to.be.false()
+    })
+  })
+})

From e39bd79baf1fb2907fb4d16b0aaa0c449a831d78 Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 14:13:50 +0000
Subject: [PATCH 03/10] Develop `CheckLiveBillRunService`

---
 .../check-live-bill-run.service.js            | 24 ++++++++++++++-----
 .../check-live-bill-run.service.test.js       |  8 +++----
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/app/services/supplementary-billing/check-live-bill-run.service.js b/app/services/supplementary-billing/check-live-bill-run.service.js
index a91d04e0b4..ebc0bc5efb 100644
--- a/app/services/supplementary-billing/check-live-bill-run.service.js
+++ b/app/services/supplementary-billing/check-live-bill-run.service.js
@@ -5,20 +5,32 @@
  * @module CheckLiveBillRunService
  */
 
+const BillingBatchModel = require('../../models/water/billing-batch.model.js')
+
+const LIVE_STATUSES = ['processing', 'ready', 'review']
+
 /**
  * Check whether a "live" bill run exists for the specified region, scheme, type and financial year
  *
  * We define "live" as having the status `processing`, `ready` or `review`
  *
- * @param {*} region
- * @param {*} scheme
- * @param {*} type
- * @param {*} financialYeara
+ * @param {*} regionId The id of the region to be checked
+ * @param {*} financialYear The financial year to be checked
  *
  * @returns {Boolean} Whether a "live" bill run exists
  */
-async function go (region, scheme, type, financialYear) {
-  return true
+async function go (regionId, financialYear) {
+  const numberOfLiveBillRuns = await BillingBatchModel.query()
+    .where({
+      regionId,
+      toFinancialYearEnding: financialYear,
+      scheme: 'sroc',
+      batchType: 'supplementary'
+    })
+    .whereIn('status', LIVE_STATUSES)
+    .resultSize()
+
+  return numberOfLiveBillRuns !== 0
 }
 
 module.exports = {
diff --git a/test/services/supplementary-billing/check-live-bill-run.service.test.js b/test/services/supplementary-billing/check-live-bill-run.service.test.js
index 23e6717e8b..4313150c48 100644
--- a/test/services/supplementary-billing/check-live-bill-run.service.test.js
+++ b/test/services/supplementary-billing/check-live-bill-run.service.test.js
@@ -14,7 +14,7 @@ const DatabaseHelper = require('../../support/helpers/database.helper.js')
 // Thing under test
 const CheckLiveBillRunService = require('../../../app/services/supplementary-billing/check-live-bill-run.service.js')
 
-describe.only('Check Live Bill Run service', () => {
+describe('Check Live Bill Run service', () => {
   let billRunRegion
 
   beforeEach(async () => {
@@ -29,7 +29,7 @@ describe.only('Check Live Bill Run service', () => {
       })
 
       it('returns `true`', async () => {
-        const result = await CheckLiveBillRunService.go(billRunRegion, 'sroc', 'supplementary', 2023)
+        const result = await CheckLiveBillRunService.go(billRunRegion, 2023)
 
         expect(result).to.be.true()
       })
@@ -42,7 +42,7 @@ describe.only('Check Live Bill Run service', () => {
       })
 
       it('returns `false`', async () => {
-        const result = await CheckLiveBillRunService.go(billRunRegion, 'sroc', 'supplementary', 2023)
+        const result = await CheckLiveBillRunService.go(billRunRegion, 2023)
 
         expect(result).to.be.false()
       })
@@ -56,7 +56,7 @@ describe.only('Check Live Bill Run service', () => {
     })
 
     it('returns `false`', async () => {
-      const result = await CheckLiveBillRunService.go(billRunRegion, 'sroc', 'supplementary', 2023)
+      const result = await CheckLiveBillRunService.go(billRunRegion, 2023)
 
       expect(result).to.be.false()
     })

From a6d52b21ce95ff50aed7662ed7fbaf389856117e Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 14:14:39 +0000
Subject: [PATCH 04/10] Fix unit test typo

---
 .../initiate-billing-batch.service.test.js                      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/services/supplementary-billing/initiate-billing-batch.service.test.js b/test/services/supplementary-billing/initiate-billing-batch.service.test.js
index 708cc0a2c6..fe65b10456 100644
--- a/test/services/supplementary-billing/initiate-billing-batch.service.test.js
+++ b/test/services/supplementary-billing/initiate-billing-batch.service.test.js
@@ -107,7 +107,7 @@ describe('Initiate Billing Batch service', () => {
       })
     })
 
-    describe('and the error doesn\'t include a messge', () => {
+    describe('and the error doesn\'t include a message', () => {
       beforeEach(() => {
         Sinon.stub(ChargingModuleCreateBillRunService, 'go').resolves({
           succeeded: false,

From a799c7f8e8b67c66b870a113c733416add560598 Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 14:58:40 +0000
Subject: [PATCH 05/10] Initially add `CheckLiveBillRunService` to
 `InitiateBillingBatchService`

---
 .../initiate-billing-batch.service.js           |  8 ++++++++
 .../initiate-billing-batch.service.test.js      | 17 ++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/app/services/supplementary-billing/initiate-billing-batch.service.js b/app/services/supplementary-billing/initiate-billing-batch.service.js
index ac1a613d66..c494c4a4b1 100644
--- a/app/services/supplementary-billing/initiate-billing-batch.service.js
+++ b/app/services/supplementary-billing/initiate-billing-batch.service.js
@@ -7,6 +7,7 @@
 
 const BillingPeriodService = require('./billing-period.service.js')
 const ChargingModuleCreateBillRunService = require('../charging-module/create-bill-run.service.js')
+const CheckLiveBillRunService = require('./check-live-bill-run.service.js')
 const CreateBillingBatchPresenter = require('../../presenters/supplementary-billing/create-billing-batch.presenter.js')
 const CreateBillingBatchService = require('./create-billing-batch.service.js')
 const CreateBillingBatchEventService = require('./create-billing-batch-event.service.js')
@@ -28,6 +29,13 @@ async function go (billRunRequestData) {
 
   const { region, scheme, type, user } = billRunRequestData
 
+  const financialYear = billingPeriod.endDate.getFullYear()
+  const liveBillRunExists = await CheckLiveBillRunService.go(region, financialYear)
+
+  if (liveBillRunExists) {
+    throw Error()
+  }
+
   const chargingModuleBillRun = await ChargingModuleCreateBillRunService.go(region, 'sroc')
 
   // A failed response will be due to a failed `RequestLib` request so format an error message accordingly and throw it
diff --git a/test/services/supplementary-billing/initiate-billing-batch.service.test.js b/test/services/supplementary-billing/initiate-billing-batch.service.test.js
index fe65b10456..b4715ed462 100644
--- a/test/services/supplementary-billing/initiate-billing-batch.service.test.js
+++ b/test/services/supplementary-billing/initiate-billing-batch.service.test.js
@@ -17,11 +17,12 @@ const RegionHelper = require('../../support/helpers/water/region.helper.js')
 // Things we need to stub
 const BillingPeriodService = require('../../../app/services/supplementary-billing/billing-period.service.js')
 const ChargingModuleCreateBillRunService = require('../../../app/services/charging-module/create-bill-run.service.js')
+const CheckLiveBillRunService = require('../../../app/services/supplementary-billing/check-live-bill-run.service.js')
 
 // Thing under test
 const InitiateBillingBatchService = require('../../../app//services/supplementary-billing/initiate-billing-batch.service.js')
 
-describe('Initiate Billing Batch service', () => {
+describe.only('Initiate Billing Batch service', () => {
   const currentBillingPeriod = {
     startDate: new Date('2022-04-01'),
     endDate: new Date('2023-03-31')
@@ -40,6 +41,7 @@ describe('Initiate Billing Batch service', () => {
     }
 
     Sinon.stub(BillingPeriodService, 'go').returns([currentBillingPeriod])
+    Sinon.stub(CheckLiveBillRunService, 'go').resolves(false)
   })
 
   afterEach(() => {
@@ -107,6 +109,19 @@ describe('Initiate Billing Batch service', () => {
       })
     })
 
+    describe('because a bill run already exists for this region and financial year', () => {
+      beforeEach(() => {
+        CheckLiveBillRunService.go.restore()
+        Sinon.stub(CheckLiveBillRunService, 'go').resolves(true)
+      })
+
+      it('rejects with an appropriate error', async () => {
+        const err = await expect(InitiateBillingBatchService.go(validatedRequestData)).to.reject()
+
+        expect(err).to.be.an.error()
+      })
+    })
+
     describe('and the error doesn\'t include a message', () => {
       beforeEach(() => {
         Sinon.stub(ChargingModuleCreateBillRunService, 'go').resolves({

From 919ea27b9c53af584249bb5dbd840c9f5e4e2695 Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 14:59:42 +0000
Subject: [PATCH 06/10] Tidier method of stubbing in unit test

---
 .../initiate-billing-batch.service.test.js                     | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/test/services/supplementary-billing/initiate-billing-batch.service.test.js b/test/services/supplementary-billing/initiate-billing-batch.service.test.js
index b4715ed462..6984c63343 100644
--- a/test/services/supplementary-billing/initiate-billing-batch.service.test.js
+++ b/test/services/supplementary-billing/initiate-billing-batch.service.test.js
@@ -111,8 +111,7 @@ describe.only('Initiate Billing Batch service', () => {
 
     describe('because a bill run already exists for this region and financial year', () => {
       beforeEach(() => {
-        CheckLiveBillRunService.go.restore()
-        Sinon.stub(CheckLiveBillRunService, 'go').resolves(true)
+        CheckLiveBillRunService.go.resolves(true)
       })
 
       it('rejects with an appropriate error', async () => {

From 7b69ecc90336e3d0abfe7aa567e71dba41e59dc1 Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 15:12:23 +0000
Subject: [PATCH 07/10] Add message to thrown error

---
 .../supplementary-billing/initiate-billing-batch.service.js    | 2 +-
 .../initiate-billing-batch.service.test.js                     | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/services/supplementary-billing/initiate-billing-batch.service.js b/app/services/supplementary-billing/initiate-billing-batch.service.js
index c494c4a4b1..cba3394c97 100644
--- a/app/services/supplementary-billing/initiate-billing-batch.service.js
+++ b/app/services/supplementary-billing/initiate-billing-batch.service.js
@@ -33,7 +33,7 @@ async function go (billRunRequestData) {
   const liveBillRunExists = await CheckLiveBillRunService.go(region, financialYear)
 
   if (liveBillRunExists) {
-    throw Error()
+    throw Error(`Batch already live for region ${region}`)
   }
 
   const chargingModuleBillRun = await ChargingModuleCreateBillRunService.go(region, 'sroc')
diff --git a/test/services/supplementary-billing/initiate-billing-batch.service.test.js b/test/services/supplementary-billing/initiate-billing-batch.service.test.js
index 6984c63343..f38212f6d2 100644
--- a/test/services/supplementary-billing/initiate-billing-batch.service.test.js
+++ b/test/services/supplementary-billing/initiate-billing-batch.service.test.js
@@ -22,7 +22,7 @@ const CheckLiveBillRunService = require('../../../app/services/supplementary-bil
 // Thing under test
 const InitiateBillingBatchService = require('../../../app//services/supplementary-billing/initiate-billing-batch.service.js')
 
-describe.only('Initiate Billing Batch service', () => {
+describe('Initiate Billing Batch service', () => {
   const currentBillingPeriod = {
     startDate: new Date('2022-04-01'),
     endDate: new Date('2023-03-31')
@@ -118,6 +118,7 @@ describe.only('Initiate Billing Batch service', () => {
         const err = await expect(InitiateBillingBatchService.go(validatedRequestData)).to.reject()
 
         expect(err).to.be.an.error()
+        expect(err.message).to.equal(`Batch already live for region ${validatedRequestData.region}`)
       })
     })
 

From cf98d6f5508fa224fc4fd5949844796d2f637470 Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 15:41:44 +0000
Subject: [PATCH 08/10] Add `queued` status to `LIVE_STATUSES`

We started this work on an incorrect, outdated branch. When we realised our mistake, we created a new branch and cherry-picked the commits across to it. However we'd overlooked the fact that we recently added the additional status `queued`, which we should also consider to be "live". We therefore update our `LIVE_STATUSES` list to include it
---
 .../supplementary-billing/check-live-bill-run.service.js      | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/services/supplementary-billing/check-live-bill-run.service.js b/app/services/supplementary-billing/check-live-bill-run.service.js
index ebc0bc5efb..18b71f7a6d 100644
--- a/app/services/supplementary-billing/check-live-bill-run.service.js
+++ b/app/services/supplementary-billing/check-live-bill-run.service.js
@@ -7,12 +7,12 @@
 
 const BillingBatchModel = require('../../models/water/billing-batch.model.js')
 
-const LIVE_STATUSES = ['processing', 'ready', 'review']
+const LIVE_STATUSES = ['processing', 'ready', 'review', 'queued']
 
 /**
  * Check whether a "live" bill run exists for the specified region, scheme, type and financial year
  *
- * We define "live" as having the status `processing`, `ready` or `review`
+ * We define "live" as having the status `processing`, `ready`, `review` or `queued`
  *
  * @param {*} regionId The id of the region to be checked
  * @param {*} financialYear The financial year to be checked

From e6f33e6842835e24b036ad3839e1ecd15b07f0af Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 15:42:05 +0000
Subject: [PATCH 09/10] Tidy unit tests

---
 .../check-live-bill-run.service.test.js         | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/test/services/supplementary-billing/check-live-bill-run.service.test.js b/test/services/supplementary-billing/check-live-bill-run.service.test.js
index 4313150c48..0b5859c744 100644
--- a/test/services/supplementary-billing/check-live-bill-run.service.test.js
+++ b/test/services/supplementary-billing/check-live-bill-run.service.test.js
@@ -15,7 +15,7 @@ const DatabaseHelper = require('../../support/helpers/database.helper.js')
 const CheckLiveBillRunService = require('../../../app/services/supplementary-billing/check-live-bill-run.service.js')
 
 describe('Check Live Bill Run service', () => {
-  let billRunRegion
+  let billRun
 
   beforeEach(async () => {
     await DatabaseHelper.clean()
@@ -24,12 +24,11 @@ describe('Check Live Bill Run service', () => {
   describe('when an sroc supplementary bill run exists for this region and financial year', () => {
     describe('with a status considered to be "live"', () => {
       beforeEach(async () => {
-        const billRun = await BillingBatchHelper.add()
-        billRunRegion = billRun.regionId
+        billRun = await BillingBatchHelper.add()
       })
 
       it('returns `true`', async () => {
-        const result = await CheckLiveBillRunService.go(billRunRegion, 2023)
+        const result = await CheckLiveBillRunService.go(billRun.regionId, 2023)
 
         expect(result).to.be.true()
       })
@@ -37,12 +36,11 @@ describe('Check Live Bill Run service', () => {
 
     describe('with a status not considered to be "live"', () => {
       beforeEach(async () => {
-        const billRun = await BillingBatchHelper.add({ status: 'sent' })
-        billRunRegion = billRun.regionId
+        billRun = await BillingBatchHelper.add({ status: 'sent' })
       })
 
       it('returns `false`', async () => {
-        const result = await CheckLiveBillRunService.go(billRunRegion, 2023)
+        const result = await CheckLiveBillRunService.go(billRun.regionId, 2023)
 
         expect(result).to.be.false()
       })
@@ -51,12 +49,11 @@ describe('Check Live Bill Run service', () => {
 
   describe('when an sroc supplementary bill run does not exist for this region and financial year', () => {
     beforeEach(async () => {
-      const billRun = await BillingBatchHelper.add({ fromFinancialYearEnding: 2024, toFinancialYearEnding: 2024 })
-      billRunRegion = billRun.regionId
+      billRun = await BillingBatchHelper.add({ fromFinancialYearEnding: 2024, toFinancialYearEnding: 2024 })
     })
 
     it('returns `false`', async () => {
-      const result = await CheckLiveBillRunService.go(billRunRegion, 2023)
+      const result = await CheckLiveBillRunService.go(billRun.regionId, 2023)
 
       expect(result).to.be.false()
     })

From d3af5a394191eeeaae1d8b9e1cf78d7a15248183 Mon Sep 17 00:00:00 2001
From: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Date: Wed, 25 Jan 2023 17:04:49 +0000
Subject: [PATCH 10/10] Select just `billing_batch_id` when querying db

Although `.resultSize()` only gives us the number of rows that a query would return and not the actual data itself, the query still has to be run and the data returned from the db. We therefore select just the `billing_batch_id` to ensure only the minimum required data is returned from the db
---
 .../supplementary-billing/check-live-bill-run.service.js         | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/services/supplementary-billing/check-live-bill-run.service.js b/app/services/supplementary-billing/check-live-bill-run.service.js
index 18b71f7a6d..671a56edd0 100644
--- a/app/services/supplementary-billing/check-live-bill-run.service.js
+++ b/app/services/supplementary-billing/check-live-bill-run.service.js
@@ -21,6 +21,7 @@ const LIVE_STATUSES = ['processing', 'ready', 'review', 'queued']
  */
 async function go (regionId, financialYear) {
   const numberOfLiveBillRuns = await BillingBatchModel.query()
+    .select('billing_batch_id')
     .where({
       regionId,
       toFinancialYearEnding: financialYear,