Fix bill licences with no transactions in bill run (#765)

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

We are working to replace the legacy SROC annual billing engine with one in this project that exploits what we did with SROC supplementary billing.

The first pass of testing has highlighted a discrepancy with how many licences are being included in some bills. For example, the legacy bill run will only display 1 but the new engine will display 3.

When we looked into it we found it was because only 1 of the 3 licences had applicable transactions. Two of the licences had no billable days.

The legacy is correct in this case, we shouldn't be creating bill licence records where there were no transactions. This change fixes the annual billing engine to deal with this scenario.

> For reference the licences had no billable days because they were revoked before the abstraction period on the charge reference i.e. revoked before the abstraction period so the charge period is null.

app/services/bill-runs/annual/process-billing-period.service.js

@@ -87,25 +87,28 @@ async function go (billRun, billingPeriod, billingAccounts) {
  * we have to create a new one.
  */
 async function _createBillLicencesAndTransactions (billId, billingAccount, billRunExternalId, billingPeriod) {
-  const billLicences = []
+  const allBillLicences = []
   const transactions = []
 
-  const { accountNumber } = billingAccount
-
   for (const chargeVersion of billingAccount.chargeVersions) {
-    const billLicence = _findOrCreateBillLicence(billLicences, chargeVersion.licence, billId)
+    const billLicence = _findOrCreateBillLicence(allBillLicences, chargeVersion.licence, billId)
 
     const createdTransactions = await _createTransactions(
       billLicence.id,
       billingPeriod,
       chargeVersion,
       billRunExternalId,
-      accountNumber
+      billingAccount.accountNumber
     )
 
-    transactions.push(...createdTransactions)
+    if (createdTransactions.length > 0) {
+      billLicence.billable = true
+      transactions.push(...createdTransactions)
+    }
   }
 
+  const billLicences = _extractBillableLicences(allBillLicences)
+
   return { billLicences, transactions }
 }
 
@@ -124,6 +127,27 @@ async function _createTransactions (billLicenceId, billingPeriod, chargeVersion,
   return SendTransactionsService.go(generatedTransactions, billRunExternalId, accountNumber, chargeVersion.licence)
 }
 
+/**
+ * Intended to be used in conjunction with _createBillLicencesAndTransactions() it extracts only those bill licences
+ * where we generated transactions. This avoids us persisting a bill licence record with no transaction records.
+ *
+ * A billing account can be linked to multiple licences but not all of them may be billable. We add a flag to each
+ * one that demotes if transactions were generated. So we can easily filter the billable ones out. But we also need
+ * to remove that flag because it doesn't exist in the DB and will cause issues if we try and persist the object.
+ */
+function _extractBillableLicences (allBillLicences) {
+  const billableBillLicences = []
+
+  allBillLicences.forEach((billLicence) => {
+    const { id, billId, licenceId, licenceRef, billable } = billLicence
+    if (billable) {
+      billableBillLicences.push({ id, billId, licenceId, licenceRef })
+    }
+  })
+
+  return billableBillLicences
+}
+
 /**
  * Use to either find or create the bill licence for a given bill (billing account) and licence
  *
@@ -160,7 +184,8 @@ function _findOrCreateBillLicence (billLicences, licence, billId) {
       id: generateUUID(),
       billId,
       licenceId,
-      licenceRef
+      licenceRef,
+      billable: false
     }
 
     billLicences.push(billLicence)

test/services/bill-runs/annual/process-billing-period.service.test.js

@@ -14,6 +14,7 @@ const { generateUUID } = require('../../../../app/lib/general.lib.js')
 const { currentFinancialYear } = require('../../../support/helpers/general.helper.js')
 
 // Things we need to stub
+const BillModel = require('../../../../app/models/bill.model.js')
 const BillRunError = require('../../../../app/errors/bill-run.error.js')
 const BillRunModel = require('../../../../app/models/bill-run.model.js')
 const ChargingModuleCreateTransactionService = require('../../../../app/services/charging-module/create-transaction.service.js')
@@ -54,33 +55,68 @@ describe('Annual Process billing period service', () => {
     })
 
     describe('and there are billing accounts to process', () => {
+      beforeEach(async () => {
+        billingAccount = _testBillingAccount()
+
+        chargingModuleCreateTransactionServiceStub.onFirstCall().resolves({
+          ..._chargingModuleResponse('7e752fa6-a19c-4779-b28c-6e536f028795')
+        })
+        chargingModuleCreateTransactionServiceStub.onSecondCall().resolves({
+          ..._chargingModuleResponse('a2086da4-e3b6-4b83-afe1-0e2e5255efaf')
+        })
+      })
+
       describe('and they are billable', () => {
         beforeEach(async () => {
-          billingAccount = _testBillingAccount()
-
           // We want to ensure there is coverage of the functionality that finds an existing bill licence or creates a
           // new one when processing a billing account. To to that we need a billing account with 2 charge versions
           // linked to the same licence
           billingAccount.chargeVersions = [_testChargeVersion(billingAccount.id), _testChargeVersion(billingAccount.id)]
+        })
 
-          chargingModuleCreateTransactionServiceStub.onFirstCall().resolves({
-            ..._chargingModuleResponse('7e752fa6-a19c-4779-b28c-6e536f028795')
-          })
-          chargingModuleCreateTransactionServiceStub.onSecondCall().resolves({
-            ..._chargingModuleResponse('a2086da4-e3b6-4b83-afe1-0e2e5255efaf')
-          })
+        it('returns true (bill run is not empty)', async () => {
+          const result = await ProcessBillingPeriodService.go(billRun, billingPeriod, [billingAccount])
+
+          expect(result).to.be.true()
+        })
+      })
+
+      describe('and they are partially billable (some bill licences generate 0 transactions)', () => {
+        beforeEach(() => {
+          // Create a charge version with an abstraction period that starts in June but which is linked to a licence
+          // that was revoked at the start of May. The engine should calculate 0 billable days and therefore not attempt
+          // to create a bill licence for this record.
+          const unbillableChargeVersion = _testChargeVersion(billingAccount.id)
+          unbillableChargeVersion.licence.id = 'c3726e99-935e-4a36-ab2f-eef8bda9293a'
+          unbillableChargeVersion.licence.revokedDate = new Date(billingPeriod.startDate.getFullYear(), 4, 1)
+          unbillableChargeVersion.chargeReferences[0].chargeElements[0].abstractionPeriodStartDay = 1
+          unbillableChargeVersion.chargeReferences[0].chargeElements[0].abstractionPeriodStartMonth = 6
+
+          billingAccount.chargeVersions = [
+            _testChargeVersion(billingAccount.id),
+            _testChargeVersion(billingAccount.id),
+            unbillableChargeVersion
+          ]
         })
 
         it('returns true (bill run is not empty)', async () => {
           const result = await ProcessBillingPeriodService.go(billRun, billingPeriod, [billingAccount])
 
           expect(result).to.be.true()
         })
+
+        it('only persists the bill licences with transactions', async () => {
+          await ProcessBillingPeriodService.go(billRun, billingPeriod, [billingAccount])
+
+          const result = await BillModel.query().findOne('billRunId', billRun.id).withGraphFetched('billLicences')
+
+          expect(result.billLicences.length).to.equal(1)
+          expect(result.billLicences[0].licenceId).to.equal(billingAccount.chargeVersions[0].licence.id)
+        })
       })
 
       describe('but they are not billable', () => {
         beforeEach(async () => {
-          billingAccount = _testBillingAccount()
           const chargeVersion = _testChargeVersion(billingAccount.id)
 
           // We update the billing account's charge information so that the engine calculates a charge period that