Move reusable supplementary billing services (#1760)

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

As we write this description, the long outstanding annual SROC two-part tariff bill runs are being 'sent' in production.

The final part of two-part tariff is to add support for supplementary billing; when changes are made to a licence or the returns previously submitted and billed, we need to generate supplementary two-part tariff bill runs.

We've already made several changes to support it, off the back of a [spike](#1412).

That spike has shown us there are existing billing services, especially in supplementary billing, that we can reuse for two-part supplementary. This change moves them to make their reuse more transparent, while tweaking a few things to allow reuse.

Author: Cruikshanks

app/lib/general.lib.js

@@ -200,15 +200,15 @@ function timestampForPostgres() {
 /**
  * Compare key properties of 2 transactions and determine if they are a 'match'
  *
- * We compare those properties which determine the charge value calculated by the charging module. If the properties
- * are the same we return true. Else we return false.
+ * We compare those properties which determine the charge value calculated by the charging module. If the properties are
+ * the same we return true. Else we return false.
  *
- * This is used in the billing engines to determine 2 transactions within the same bill, often a debit and a credit,
- * and whether they match. If they do we don't send either to the charge module or include them in the bill as they
- * 'cancel' each other out.
+ * This is used in the billing engines to determine 2 transactions within the same bill, often a debit and a credit, and
+ * whether they match. If they do we don't send either to the charge module or include them in the bill as they 'cancel'
+ * each other out.
  *
- * The key properties are charge type, category code, and billable days. But we also need to compare agreements and
- * additional charges because if those have changed, we need to credit the previous transaction and calculate the
+ * The key properties are charge type, category code, billable days, and volume. But we also need to compare agreements
+ * and additional charges because if those have changed, we need to credit the previous transaction and calculate the
  * new debit value.
  *
  * Because what we are checking does not match up to what you see in the UI we have this reference
@@ -242,6 +242,7 @@ function transactionsMatch(left, right) {
     left.chargeType === right.chargeType &&
     left.chargeCategoryCode === right.chargeCategoryCode &&
     left.billableDays === right.billableDays &&
+    left.volume === right.volume &&
     left.section126Factor === right.section126Factor &&
     left.section127Agreement === right.section127Agreement &&
     left.section130Agreement === right.section130Agreement &&

app/services/bill-runs/supplementary/fetch-previous-transactions.service.js renamed to app/services/bill-runs/fetch-previous-transactions.service.js

@@ -5,9 +5,9 @@
  * @module FetchPreviousTransactionsService
  */
 
-const { db } = require('../../../../db/db.js')
-const { transactionsMatch } = require('../../../lib/general.lib.js')
-const TransactionModel = require('../../../models/transaction.model.js')
+const { db } = require('../../../db/db.js')
+const { transactionsMatch } = require('../../lib/general.lib.js')
+const TransactionModel = require('../../models/transaction.model.js')
 
 /**
  * Fetches the previously billed transactions that match, removing any debits which cancelled out by previous credits

app/services/bill-runs/two-part-tariff/fetch-billing-accounts.service.js renamed to app/services/bill-runs/fetch-two-part-tariff-billing-accounts.service.js

@@ -2,13 +2,13 @@
 
 /**
  * Fetches all billing accounts linked to a bill run to be processed as part of two-part tariff billing
- * @module FetchBillingAccountsService
+ * @module FetchTwoPartTariffBillingAccountsService
  */
 
 const { ref } = require('objection')
 
-const BillingAccountModel = require('../../../models/billing-account.model.js')
-const ChargeVersionModel = require('../../../models/charge-version.model.js')
+const BillingAccountModel = require('../../models/billing-account.model.js')
+const ChargeVersionModel = require('../../models/charge-version.model.js')
 
 /**
  * Fetches all billing accounts linked to a bill run to be processed as part of two-part tariff billing

app/services/bill-runs/supplementary/process-transactions.service.js renamed to app/services/bill-runs/process-supplementary-transactions.service.js

@@ -1,41 +1,56 @@
 'use strict'
 
 /**
- * Fetches the matching debit transactions from a previous bill run and reverses them as credits
- * @module ProcessTransactionsService
+ * Fetches matching debit transactions from previous bill runs, then compares them as credits to those just generated
+ * @module ProcessSupplementaryTransactionsService
  */
 
 const FetchPreviousTransactionsService = require('./fetch-previous-transactions.service.js')
-const { transactionsMatch } = require('../../../lib/general.lib.js')
-const ReverseTransactionsService = require('./reverse-transactions.service.js')
+const { transactionsMatch } = require('../../lib/general.lib.js')
+const ReverseTransactionsService = require('./reverse-supplementary-transactions.service.js')
 
 /**
- * Fetches debit-only transactions from the previous bill run for the billing account and licence provided and reverses
- * them as credits.
+ * Fetches matching debit transactions from previous bill runs, then compares them as credits to those just generated
  *
- * These credits are compared with the supplied calculated debit transactions (ie. debit transactions which are to be
- * sent to the Charging Module) and any matching pairs of transactions which would cancel each other out are removed.
- * Any remaining reversed credits and calculated debits are returned.
+ * The `FetchPreviousTransactionsService` finds _all_ previous debit and credit transactions. It then compares the
+ * debits to the credits. If they match, the debit transaction is dropped. This is because it must have been dealt with
+ * by a previous supplementary bill run.
  *
- * @param {object[]} calculatedTransactions - The calculated transactions to be processed
- * @param {string} billingAccountId - The UUID that identifies the billing account we are processing transactions for
- * @param {object} billLicence - A generated bill licence that identifies the licence we need to match against
- * @param {object} billingPeriod - Object with a `startDate` and `endDate` property representing the period being billed
+ * Any debits that remain are then reversed as credits. These are then compared against the ones we've generated for the
+ * bill run in progress.
  *
- * @returns {Promise<object[]>} An array of the remaining calculated transactions (ie. those which were not cancelled
- * out by a previous matching credit)
+ * If any matches occur here, both transactions are dropped. There is no point in sending them to the Charging Module
+ * API and including them in the bill, as the result will just be £0, and we've been asked to limit zero-value bills
+ * where we can.
+ *
+ * Any generated transactions and reversed debits (now credits) that didn't match are returned as the transactions to be
+ * sent to the Charging Module API, and included in the bill.
+ *
+ * @param {object[]} generatedTransactions - The generated transactions for the bill licence being processed
+ * @param {string} billLicenceId - The UUID that will be used for the bill licence we are processing transactions for
+ * @param {string} billingAccountId - The UUID for the billing account we are processing transactions for and for which
+ * we need to fetch previous transactions
+ * @param {string} licenceId - The UUID for the licence we are processing transactions for and  and for which we need to
+ * fetch previous transactions
+ * @param {number} financialYearEnding - Which financial year to look for previous transactions in
+ *
+ * @returns {Promise<object[]>} An array of the remaining generated transactions and reversed debits from previous
+ * transactions (ie. those which were not cancelled out when the generated and reversed were compared)
  */
-async function go(calculatedTransactions, billingAccountId, billLicence, billingPeriod) {
-  const { id: billLicenceId, licenceId } = billLicence
-  const previousTransactions = await _fetchPreviousTransactions(billingAccountId, licenceId, billingPeriod)
+async function go(generatedTransactions, billLicenceId, billingAccountId, licenceId, financialYearEnding) {
+  const previousTransactions = await FetchPreviousTransactionsService.go(
+    billingAccountId,
+    licenceId,
+    financialYearEnding
+  )
 
   if (previousTransactions.length === 0) {
-    return calculatedTransactions
+    return generatedTransactions
   }
 
   const reversedTransactions = ReverseTransactionsService.go(previousTransactions, billLicenceId)
 
-  return _cleanseTransactions(calculatedTransactions, reversedTransactions)
+  return _cleanseTransactions(generatedTransactions, reversedTransactions)
 }
 
 /**
@@ -91,14 +106,6 @@ function _cleanseTransactions(calculatedTransactions, reverseTransactions) {
   return cleansedTransactionLines
 }
 
-async function _fetchPreviousTransactions(billingAccountId, licenceId, billingPeriod) {
-  const financialYearEnding = billingPeriod.endDate.getFullYear()
-
-  const transactions = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding)
-
-  return transactions
-}
-
 module.exports = {
   go
 }

app/services/bill-runs/reverse-supplementary-transactions.service.js

@@ -0,0 +1,51 @@
+'use strict'
+
+/**
+ * Takes previously billed transactions and returns reversed and cleansed versions of them for supplementary billing
+ * @module ReverseSupplementaryTransactionsService
+ */
+
+const { generateUUID } = require('../../lib/general.lib.js')
+
+/**
+ * Takes previously billed transactions and returns reversed and cleansed versions of them for supplementary billing
+ *
+ * In supplementary we need to "reverse" transactions. We start by identifying previous debit transactions that have not
+ * been cancelled out by a previous credit transaction.
+ *
+ * We then "reverse" these previous debits as credits and compare them to the transactions generated as part of the
+ * bill run currently in progress.
+ *
+ * If the "reversed" transaction matches the generated transaction, we know including both will just result in £0.
+ * If like the legacy service we included them, it could lead to a zero value bill which our users hate! So, we drop\
+ * both from the bill run.
+ *
+ * Those reversed and generated transactions that don't match, are what gets included and eventually billed.
+ *
+ * This service takes the previous debit transactions and a bill licence ID, and returns a copy of them, but with
+ *
+ * - `credit` set to `true`
+ * - `status` set to `candidate`
+ * - `billLicenceId` set to the supplied ID
+ * - `id` set to a new UUID
+ *
+ * @param {module:TransactionModel[]} transactions - The transactions to be reversed
+ * @param {string} billLicenceId - The bill licence UUID these transactions are to be added to
+ *
+ * @returns {object[]} The "reversed" transactions
+ */
+function go(transactions, billLicenceId) {
+  return transactions.map((transaction) => {
+    return {
+      ...transaction,
+      id: generateUUID(),
+      billLicenceId,
+      credit: true,
+      status: 'candidate'
+    }
+  })
+}
+
+module.exports = {
+  go
+}

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

@@ -13,7 +13,7 @@ const DetermineChargePeriodService = require('../determine-charge-period.service
 const DetermineMinimumChargeService = require('../determine-minimum-charge.service.js')
 const GenerateTransactionsService = require('../generate-transactions.service.js')
 const PreGenerateBillingDataService = require('./pre-generate-billing-data.service.js')
-const ProcessTransactionsService = require('./process-transactions.service.js')
+const ProcessSupplementaryTransactionsService = require('../process-supplementary-transactions.service.js')
 const SendTransactionsService = require('../send-transactions.service.js')
 const TransactionModel = require('../../../models/transaction.model.js')
 
@@ -179,11 +179,12 @@ async function _cleanseTransactions(currentBillingData, billingPeriod) {
     return []
   }
 
-  const cleansedTransactions = await ProcessTransactionsService.go(
+  const cleansedTransactions = await ProcessSupplementaryTransactionsService.go(
     currentBillingData.calculatedTransactions,
+    currentBillingData.billLicence.id,
     currentBillingData.bill.billingAccountId,
-    currentBillingData.billLicence,
-    billingPeriod
+    currentBillingData.billLicence.licenceId,
+    billingPeriod.endDate.getFullYear()
   )
 
   return cleansedTransactions

app/services/bill-runs/supplementary/reverse-transactions.service.js

@@ -14,7 +14,7 @@ const {
   currentTimeInNanoseconds,
   timestampForPostgres
 } = require('../../../lib/general.lib.js')
-const FetchBillingAccountsService = require('./fetch-billing-accounts.service.js')
+const FetchTwoPartTariffBillingAccountsService = require('../fetch-two-part-tariff-billing-accounts.service.js')
 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')
@@ -77,7 +77,7 @@ function _billingPeriod(billRun) {
 
 async function _fetchBillingAccounts(billRunId) {
   try {
-    return await FetchBillingAccountsService.go(billRunId)
+    return await FetchTwoPartTariffBillingAccountsService.go(billRunId)
   } catch (error) {
     // We know we're saying we failed to process charge versions. But we're stuck with the legacy error codes and this
     // is the closest one related to what stage we're at in the process

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

@@ -332,7 +332,7 @@ describe('GeneralLib', () => {
     let rightTransaction
 
     beforeEach(() => {
-      // NOTE: The properties the function is comparing are; chargeType, chargeCategoryCode, billableDays,
+      // NOTE: The properties the function is comparing are; chargeType, chargeCategoryCode, billableDays, volume,
       // section126Factor, section127Agreement, section130Agreement, aggregateFactor, adjustmentFactor, winterOnly,
       // supportedSource, supportedSourceName, waterCompanyCharge.
       //
@@ -398,6 +398,18 @@ describe('GeneralLib', () => {
         })
       })
 
+      describe('because the volume is different', () => {
+        beforeEach(() => {
+          rightTransaction.volume = 10
+        })
+
+        it('returns false', () => {
+          const result = GeneralLib.transactionsMatch(leftTransaction, rightTransaction)
+
+          expect(result).to.be.false()
+        })
+      })
+
       describe('because the canal and river trust agreement (section 130) is different', () => {
         beforeEach(() => {
           rightTransaction.section130Agreement = true

test/lib/general.lib.test.js

@@ -8,18 +8,18 @@ const { describe, it, beforeEach } = (exports.lab = Lab.script())
 const { expect } = Code
 
 // Test helpers
-const BillHelper = require('../../../support/helpers/bill.helper.js')
-const BillingAccountHelper = require('../../../support/helpers/billing-account.helper.js')
-const BillLicenceHelper = require('../../../support/helpers/bill-licence.helper.js')
-const BillRunHelper = require('../../../support/helpers/bill-run.helper.js')
-const { generateUUID } = require('../../../../app/lib/general.lib.js')
-const LicenceHelper = require('../../../support/helpers/licence.helper.js')
-const TransactionHelper = require('../../../support/helpers/transaction.helper.js')
+const BillHelper = require('../../support/helpers/bill.helper.js')
+const BillingAccountHelper = require('../../support/helpers/billing-account.helper.js')
+const BillLicenceHelper = require('../../support/helpers/bill-licence.helper.js')
+const BillRunHelper = require('../../support/helpers/bill-run.helper.js')
+const { generateUUID } = require('../../../app/lib/general.lib.js')
+const LicenceHelper = require('../../support/helpers/licence.helper.js')
+const TransactionHelper = require('../../support/helpers/transaction.helper.js')
 
 // Thing under test
-const FetchPreviousTransactionsService = require('../../../../app/services/bill-runs/supplementary/fetch-previous-transactions.service.js')
+const FetchPreviousTransactionsService = require('../../../app/services/bill-runs/fetch-previous-transactions.service.js')
 
-describe('Fetch Previous Transactions service', () => {
+describe('Bill Runs - Fetch Previous Transactions service', () => {
   const chargeCategoryCode = '4.3.1'
   const financialYearEnding = 2023