Supplementary billing housekeeping

app/services/bill-runs/annual/process-bill-run.service.js

@@ -9,7 +9,7 @@ const BillRunModel = require('../../../models/bill-run.model.js')
 const BillRunError = require('../../../errors/bill-run.error.js')
 const ChargingModuleGenerateBillRunRequest = require('../../../requests/charging-module/generate-bill-run.request.js')
 const FetchBillingAccountsService = require('./fetch-billing-accounts.service.js')
-const { calculateAndLogTimeTaken } = require('../../../lib/general.lib.js')
+const { calculateAndLogTimeTaken, currentTimeInNanoseconds } = require('../../../lib/general.lib.js')
 const LegacyRefreshBillRunRequest = require('../../../requests/legacy/refresh-bill-run.request.js')
 const ProcessBillingPeriodService = require('./process-billing-period.service.js')
 const HandleErroredBillRunService = require('../handle-errored-bill-run.service.js')
@@ -41,7 +41,7 @@ async function go (billRun, billingPeriods) {
   const billingPeriod = billingPeriods[0]
 
   try {
-    const startTime = process.hrtime.bigint()
+    const startTime = currentTimeInNanoseconds()
 
     await _updateStatus(billRunId, 'processing')
 

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

@@ -1,32 +1,26 @@
 'use strict'
 
 /**
- * Fetches the previously billed transactions that match the bill, licence and year provided, removing any debits
- * which are cancelled out by previous credits.
+ * Fetches the previously billed transactions that match, removing any debits which cancelled out by previous credits
  * @module FetchPreviousTransactionsService
  */
 
 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 the bill, licence and year provided, removing any debits
- * which are cancelled out by previous credits.
+ * Fetches the previously billed transactions that match, removing any debits which cancelled out by previous credits
  *
- * @param {Object} bill A generated bill that identifies the billing account ID we need to match
- *  against
- * @param {Object} billLicence A generated bill licence that identifies the licence we need to
- *  match against
- * @param {Number} financialYearEnding The year the financial billing period ends that we need to match against
+ * @param {string} billingAccountId - The UUID that identifies the billing account we need to fetch transactions for
+ * @param {string} licenceId - The UUID that identifies the licence we need to fetch transactions for
+ * @param {Number} financialYearEnding - The year the financial billing period ends that we need to fetch transactions
+ * for
  *
- * @returns {Promise<Object>} The resulting matched transactions
+ * @returns {Promise<Object[]>} The resulting matched transactions
  */
-async function go (bill, billLicence, financialYearEnding) {
-  const transactions = await _fetch(
-    billLicence.licenceId,
-    bill.billingAccountId,
-    financialYearEnding
-  )
+async function go (billingAccountId, licenceId, financialYearEnding) {
+  const transactions = await _fetch(billingAccountId, licenceId, financialYearEnding)
 
   return _cleanse(transactions)
 }
@@ -54,68 +48,55 @@ function _cleanse (transactions) {
   return debits
 }
 
-async function _fetch (licenceId, billingAccountId, financialYearEnding) {
-  return db
-    .select(
-      't.authorisedDays',
-      't.billableDays',
-      't.waterUndertaker',
-      't.chargeReferenceId',
-      't.startDate',
-      't.endDate',
-      't.source',
-      't.season',
-      't.loss',
-      't.credit',
-      't.chargeType',
-      't.authorisedQuantity',
-      't.billableQuantity',
-      't.description',
-      't.volume',
-      't.section126Factor',
-      't.section127Agreement',
+async function _fetch (billingAccountId, licenceId, financialYearEnding) {
+  return TransactionModel.query()
+    .select([
+      'transactions.authorisedDays',
+      'transactions.billableDays',
+      'transactions.waterUndertaker',
+      'transactions.chargeReferenceId',
+      'transactions.startDate',
+      'transactions.endDate',
+      'transactions.source',
+      'transactions.season',
+      'transactions.loss',
+      'transactions.credit',
+      'transactions.chargeType',
+      'transactions.authorisedQuantity',
+      'transactions.billableQuantity',
+      'transactions.description',
+      'transactions.volume',
+      'transactions.section126Factor',
+      'transactions.section127Agreement',
+      'transactions.secondPartCharge',
+      'transactions.scheme',
+      'transactions.aggregateFactor',
+      'transactions.adjustmentFactor',
+      'transactions.chargeCategoryCode',
+      'transactions.chargeCategoryDescription',
+      'transactions.supportedSource',
+      'transactions.supportedSourceName',
+      'transactions.newLicence',
+      'transactions.waterCompanyCharge',
+      'transactions.winterOnly',
+      'transactions.purposes',
       // NOTE: The section130Agreement field is a varchar in the DB for historic reasons. It seems some early PRESROC
       // transactions recorded values other than 'true' or 'false'. For SROC though, it will only ever be true/false. We
       // generate our calculated billing transaction lines based on the Section130 flag against charge_elements which is
       // always a boolean. So, to avoid issues when we need to compare the values we cast this to a boolean when
       // fetching the data.
-      db.raw('t.section_130_agreement::boolean'),
-      't.secondPartCharge',
-      't.scheme',
-      't.aggregateFactor',
-      't.adjustmentFactor',
-      't.chargeCategoryCode',
-      't.chargeCategoryDescription',
-      't.supportedSource',
-      't.supportedSourceName',
-      't.newLicence',
-      't.waterCompanyCharge',
-      't.winterOnly',
-      't.purposes',
-      'validBills.billingAccountId',
-      'validBills.accountNumber'
-    )
-    .from('transactions as t')
-    .innerJoin(
-      db
-        .select(
-          'bl.id',
-          'b.billingAccountId',
-          'b.accountNumber'
-        )
-        .from('billLicences as bl')
-        .innerJoin('bills as b', 'bl.billId', 'b.id')
-        .innerJoin('billRuns as br', 'br.id', 'b.billRunId')
-        .where({
-          'bl.licenceId': licenceId,
-          'b.billingAccountId': billingAccountId,
-          'b.financialYearEnding': financialYearEnding,
-          'br.status': 'sent',
-          'br.scheme': 'sroc'
-        })
-        .as('validBills'),
-      't.billLicenceId', 'validBills.id'
-    )
+      db.raw('transactions.section_130_agreement::boolean')
+    ])
+    .innerJoin('billLicences', 'transactions.billLicenceId', 'billLicences.id')
+    .innerJoin('bills', 'billLicences.billId', 'bills.id')
+    .innerJoin('billRuns', 'bills.billRunId', 'billRuns.id')
+    .where({
+      'billLicences.licenceId': licenceId,
+      'bills.billingAccountId': billingAccountId,
+      'bills.financialYearEnding': financialYearEnding,
+      'billRuns.status': 'sent',
+      'billRuns.scheme': 'sroc'
+    })
 }
 
 module.exports = {

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

@@ -176,7 +176,7 @@ async function _cleanseTransactions (currentBillingData, billingPeriod) {
 
   const cleansedTransactions = await ProcessTransactionsService.go(
     currentBillingData.calculatedTransactions,
-    currentBillingData.bill,
+    currentBillingData.bill.billingAccountId,
     currentBillingData.billLicence,
     billingPeriod
   )

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

@@ -1,9 +1,7 @@
 'use strict'
 
 /**
- * Fetches the matching debit transactions from a previous bill run and reverses them as credits; removes any which
- * would be cancelled out by the supplied calculated debit transactions; combines the remaining transactions and returns
- * them all
+ * Fetches the matching debit transactions from a previous bill run and reverses them as credits
  * @module ProcessTransactionsService
  */
 
@@ -19,22 +17,23 @@ const ReverseTransactionsService = require('./reverse-transactions.service.js')
  * 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.
  *
- * @param {Object[]} calculatedTransactions The calculated transactions to be processed
- * @param {Object} bill A generated bill that identifies the invoice account ID we need to match against
- * @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
+ * @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
  *
- * @returns {Promise<Object[]>} An array of the remaining calculated transactions (ie. those which were not cancelled out by a
- *  previous matching credit)
+ * @returns {Promise<Object[]>} An array of the remaining calculated transactions (ie. those which were not cancelled
+ *  out by a previous matching credit)
  */
-async function go (calculatedTransactions, bill, billLicence, billingPeriod) {
-  const previousTransactions = await _fetchPreviousTransactions(bill, billLicence, billingPeriod)
+async function go (calculatedTransactions, billingAccountId, billLicence, billingPeriod) {
+  const { id: billLicenceId, licenceId } = billLicence
+  const previousTransactions = await _fetchPreviousTransactions(billingAccountId, licenceId, billingPeriod)
 
   if (previousTransactions.length === 0) {
     return calculatedTransactions
   }
 
-  const reversedTransactions = ReverseTransactionsService.go(previousTransactions, billLicence)
+  const reversedTransactions = ReverseTransactionsService.go(previousTransactions, billLicenceId)
 
   return _cleanseTransactions(calculatedTransactions, reversedTransactions)
 }
@@ -88,10 +87,10 @@ function _cleanseTransactions (calculatedTransactions, reverseTransactions) {
   return cleansedTransactionLines
 }
 
-async function _fetchPreviousTransactions (bill, billLicence, billingPeriod) {
+async function _fetchPreviousTransactions (billingAccountId, licenceId, billingPeriod) {
   const financialYearEnding = billingPeriod.endDate.getFullYear()
 
-  const transactions = await FetchPreviousTransactionsService.go(bill, billLicence, financialYearEnding)
+  const transactions = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding)
 
   return transactions
 }

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

@@ -11,45 +11,31 @@ const { generateUUID } = require('../../../lib/general.lib.js')
  * Takes an array of transactions and returns an array of transactions which will reverse them.
  *
  * In some situations we need to "reverse" transactions; this is done by issuing new transactions which cancel them out.
- * This service takes an array of transactions and a bill licence, and returns an array of transactions which
- * will reverse the original transactions, with their bill licence id set to the id of the supplied billing licence.
+ * This service takes an array of transactions and a bill licence, and returns an array of transactions which will
+ * reverse the original transactions, with their bill licence id set to the ID of the supplied billing licence.
  *
- * @param {module:TransactionModel[]} transactions Array of transactions to be reversed
- * @param {module:BillLicenceModel} billLicence The bill licence these transactions are intended to be added to
+ * @param {module:TransactionModel[]} transactions - Array of transactions to be reversed
+ * @param {string} billLicenceId - The bill licence UUID these transactions are to be added to
  *
- * @returns {Object[]} Array of reversing transactions with `billLicenceId` set to the id of the supplied
- *  `billLicence`
+ * @returns {Object[]} Array of reversed transactions with `billLicenceId` set to the id of the supplied `billLicence`
  */
-function go (transactions, billLicence) {
-  return _reverseTransactions(transactions, billLicence)
+function go (transactions, billLicenceId) {
+  return _reverseTransactions(transactions, billLicenceId)
 }
 
 /**
  * Receives an array of debit transactions and returns transactions that will reverse them. These transactions are
- * identical except the `credit` flag is set to 'true', the status is set to `candidate`, the `billLicenceId` is set
- * to the id of the supplied bill licence, and a new transaction ID is generated.
+ * identical except the `credit` flag is set to 'true', the status is set to `candidate`, the `billLicenceId` is for the
+ * bill licence we are creating, and a new transaction ID is generated.
  */
-function _reverseTransactions (transactions, billLicence) {
+function _reverseTransactions (transactions, billLicenceId) {
   return transactions.map((transaction) => {
-    // TODO: The FetchTransactionsService which we use to get the transactions to reverse adds the billing account ID
-    // and number to each transaction returned. This is a performance measure to avoid an extra query to the DB. But if
-    // we don't strip them from the result when we try to persist our reversed versions, they fail because the
-    // transactions table doesn't have these fields. We do the stripping here to avoid iterating through the
-    // collection multiple times. Ideally, we'd look to return a result from FetchTransactionsService that avoids us
-    // having to do this.
-    const { billingAccountId, accountNumber, ...propertiesToKeep } = transaction
-
     return {
-      ...propertiesToKeep,
+      ...transaction,
       id: generateUUID(),
-      billLicenceId: billLicence.id,
+      billLicenceId,
       credit: true,
-      status: 'candidate',
-      // TODO: Our query result seems to return the transaction's `purposes:` property as [Object]. Clearly, we need
-      // to re-jig something or give Knex some more instructions on dealing with this JSONB field. But just to prove
-      // the process is working we use this service to deal with the issue and extract the JSON from the array we've
-      // been provided.
-      purposes: transaction.purposes[0]
+      status: 'candidate'
     }
   })
 }

test/services/bill-runs/supplementary/handle-errored-bill-run.service.test.js → test/services/bill-runs/handle-errored-bill-run.service.test.js

@@ -9,10 +9,10 @@ const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script()
 const { expect } = Code
 
 // Test helpers
-const BillRunHelper = require('../../../support/helpers/bill-run.helper.js')
+const BillRunHelper = require('../../support/helpers/bill-run.helper.js')
 
 // Thing under test
-const HandleErroredBillRunService = require('../../../../app/services/bill-runs/handle-errored-bill-run.service.js')
+const HandleErroredBillRunService = require('../../../app/services/bill-runs/handle-errored-bill-run.service.js')
 
 describe('Handle Errored Bill Run service', () => {
   let billRun