Fix error in generate 2PT bill run transactions (#1188)

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

> Part of the two-part tariff annual billing work

We recently completed [implementation of the annual two-part tariff billing engine](#1172). But we've immediately spotted an error with the results.

We are not populating the correct field in order to see the amended billing volume being sent to the Charging Module and persisting against the transaction.

However, that uncovered something else we'd overlooked. Now that we were using the review allocated amount, some transactions were being returned from `GenerateTransactionService` with a 0 volume, which caused the Charging Module to error.

Of course! If a return was submitted as a 'Nil return' then we should not be trying to generate a charge for those charge elements linked to it. 🤦 

So, this change corrects which value we are assigning to the generated transaction and therefore passing to the charging module. But it also updates the engine to handle ignoring transactions with no volume rather than attempting to send them to the Charging Module API.

app/services/bill-runs/two-part-tariff/generate-transaction.service.js

@@ -35,6 +35,10 @@ const { generateUUID } = require('../../../lib/general.lib.js')
 function go (billLicenceId, chargeReference, chargePeriod, newLicence, waterUndertaker) {
   const billableQuantity = _billableQuantity(chargeReference.chargeElements)
 
+  if (billableQuantity === 0) {
+    return null
+  }
+
   return _standardTransaction(
     billLicenceId,
     billableQuantity,
@@ -105,7 +109,7 @@ function _standardTransaction (
     billableQuantity,
     status: 'candidate',
     description: _description(chargeReference),
-    volume: chargeReference.volume,
+    volume: billableQuantity,
     section126Factor: Number(chargeReference.adjustments.s126) || 1,
     section127Agreement: !!chargeReference.adjustments.s127,
     section130Agreement: !!chargeReference.adjustments.s130,

app/services/bill-runs/two-part-tariff/process-billing-period.service.js

@@ -127,6 +127,10 @@ async function _createTransactions (billLicenceId, billingPeriod, chargeVersion,
 
   const generatedTransactions = _generateTransactionData(billLicenceId, chargePeriod, chargeVersion)
 
+  if (generatedTransactions.length === 0) {
+    return []
+  }
+
   return SendTransactionsService.go(generatedTransactions, billRunExternalId, accountNumber, chargeVersion.licence)
 }
 
@@ -221,7 +225,9 @@ function _generateTransactionData (billLicenceId, chargePeriod, chargeVersion) {
         chargeVersion.licence.waterUndertaker
       )
 
-      transactions.push(transaction)
+      if (transaction) {
+        transactions.push(transaction)
+      }
     })
 
     return transactions

test/services/bill-runs/two-part-tariff/generate-transaction.service.test.js

@@ -30,63 +30,80 @@ describe('Generate Transaction service', () => {
   })
 
   describe('when called', () => {
-    it('returns a two-part tariff transaction ready to be persisted', () => {
-      const result = GenerateTransactionService.go(
-        billLicenceId, chargeReference, chargePeriod, newLicence, waterUndertaker
-      )
-
-      expect(result).to.equal({
-        billLicenceId,
-        authorisedDays: 0,
-        billableDays: 0,
-        newLicence,
-        waterUndertaker,
-        chargeReferenceId: '89450220-0f7f-4280-a946-1fdbe9b789c1',
-        startDate: chargePeriod.startDate,
-        endDate: chargePeriod.endDate,
-        source: 'non-tidal',
-        season: 'all year',
-        loss: 'low',
-        credit: false,
-        chargeType: 'standard',
-        authorisedQuantity: 20,
-        billableQuantity: 15,
-        status: 'candidate',
-        description: 'Water abstraction charge: Lower Queenstown - Pittisham',
-        volume: 15,
-        section126Factor: 1,
-        section127Agreement: false,
-        section130Agreement: false,
-        secondPartCharge: true,
-        scheme: 'sroc',
-        aggregateFactor: 0.75,
-        adjustmentFactor: 0.6,
-        chargeCategoryCode: '4.4.5',
-        chargeCategoryDescription: 'Low loss, non-tidal, restricted water, up to and including 5,000 ML/yr, Tier 1 model',
-        supportedSource: false,
-        supportedSourceName: null,
-        waterCompanyCharge: false,
-        winterOnly: false
-      },
-      {
-        // We skip the ID because it is a UUID generated by the service. We skip purposes because it is the result of
-        // calling `toJSON()` on the charge references child ChargeElementModel instances. Including it would just be us
-        // testing Objection
-        skip: ['id', 'purposes']
+    describe('with a charge reference that has volume to be billed', () => {
+      it('returns a two-part tariff transaction ready to be persisted', () => {
+        const result = GenerateTransactionService.go(
+          billLicenceId, chargeReference, chargePeriod, newLicence, waterUndertaker
+        )
+
+        expect(result).to.equal({
+          billLicenceId,
+          authorisedDays: 0,
+          billableDays: 0,
+          newLicence,
+          waterUndertaker,
+          chargeReferenceId: '89450220-0f7f-4280-a946-1fdbe9b789c1',
+          startDate: chargePeriod.startDate,
+          endDate: chargePeriod.endDate,
+          source: 'non-tidal',
+          season: 'all year',
+          loss: 'low',
+          credit: false,
+          chargeType: 'standard',
+          authorisedQuantity: 20,
+          billableQuantity: 15,
+          status: 'candidate',
+          description: 'Water abstraction charge: Lower Queenstown - Pittisham',
+          volume: 15,
+          section126Factor: 1,
+          section127Agreement: false,
+          section130Agreement: false,
+          secondPartCharge: true,
+          scheme: 'sroc',
+          aggregateFactor: 0.75,
+          adjustmentFactor: 0.6,
+          chargeCategoryCode: '4.4.5',
+          chargeCategoryDescription: 'Low loss, non-tidal, restricted water, up to and including 5,000 ML/yr, Tier 1 model',
+          supportedSource: false,
+          supportedSourceName: null,
+          waterCompanyCharge: false,
+          winterOnly: false
+        },
+        {
+          // We skip the ID because it is a UUID generated by the service. We skip purposes because it is the result of
+          // calling `toJSON()` on the charge references child ChargeElementModel instances. Including it would just be
+          // us testing Objection
+          skip: ['id', 'purposes']
+        })
+      })
+
+      describe('and the charge reference has a two-part tariff agreement (section 127)', () => {
+        beforeEach(() => {
+          chargeReference.adjustments.s127 = true
+        })
+
+        it('returns the two-part tariff prefixed description', () => {
+          const result = GenerateTransactionService.go(
+            billLicenceId, chargeReference, chargePeriod, newLicence, waterUndertaker
+          )
+
+          expect(result.description).to.equal('Two-part tariff basic water abstraction charge: Lower Queenstown - Pittisham')
+        })
       })
     })
 
-    describe('and the charge reference has a two-part tariff agreement (section 127)', () => {
+    describe('with a charge reference that has no volume to be billed', () => {
       beforeEach(() => {
-        chargeReference.adjustments.s127 = true
+        chargeReference.chargeElements[0].reviewChargeElements[0].amendedAllocated = 0
+        chargeReference.chargeElements[1].reviewChargeElements[0].amendedAllocated = 0
       })
 
-      it('returns the two-part tariff prefixed description', () => {
+      it('returns null', () => {
         const result = GenerateTransactionService.go(
           billLicenceId, chargeReference, chargePeriod, newLicence, waterUndertaker
         )
 
-        expect(result.description).to.equal('Two-part tariff basic water abstraction charge: Lower Queenstown - Pittisham')
+        expect(result).to.be.null()
       })
     })
   })
@@ -137,6 +154,6 @@ function _chargeReference () {
       amendedChargeAdjustment: 0.6
     }],
     source: 'non-tidal',
-    volume: 15
+    volume: 20
   }
 }

test/services/bill-runs/two-part-tariff/process-billing-period.service.test.js

@@ -156,15 +156,12 @@ describe('Two-part Tariff Process Billing Period service', () => {
 
       describe('but they are not billable', () => {
         beforeEach(() => {
-          // Create a licence that has a revoked date before the billing period and then link it to a charge version
-          // that is also linked to our billing account. The engine will determine that the charge period for the charge
-          // version is invalid so won't attempt to generate a transaction. If we did try, the Charging Module would
-          // only reject it.
-          const unbillableLicence = _licence()
-
-          unbillableLicence.revokedDate = new Date('2019-01-01')
+          // This time we update the charge version so that nothing is allocated in the charge references. This means
+          // the service will not generate any transactions and therefore no bills leading to bills being empty
+          const unbillableChargeVersion = _chargeVersion(billingAccount.id, licence)
 
-          const unbillableChargeVersion = _chargeVersion(billingAccount.id, unbillableLicence)
+          unbillableChargeVersion.chargeReferences[0].chargeElements[0].reviewChargeElements[0].amendedAllocated = 0
+          unbillableChargeVersion.chargeReferences[0].chargeElements[1].reviewChargeElements[0].amendedAllocated = 0
 
           billingAccount.chargeVersions = [unbillableChargeVersion]
         })
@@ -289,7 +286,7 @@ function _chargeVersion (billingAccountId, licence) {
           amendedChargeAdjustment: 0.6
         }],
         source: 'non-tidal',
-        volume: 15
+        volume: 20
       }
     ]
   }