diff --git a/app/services/bill-runs/annual/process-bill-run.service.js b/app/services/bill-runs/annual/process-bill-run.service.js index 149ebc06d1..6980637a9a 100644 --- a/app/services/bill-runs/annual/process-bill-run.service.js +++ b/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') diff --git a/app/services/bill-runs/supplementary/fetch-previous-transactions.service.js b/app/services/bill-runs/supplementary/fetch-previous-transactions.service.js index 73d138fffa..c7fc2db59c 100644 --- a/app/services/bill-runs/supplementary/fetch-previous-transactions.service.js +++ b/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} The resulting matched transactions + * @returns {Promise} 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 = { diff --git a/app/services/bill-runs/supplementary/process-billing-period.service.js b/app/services/bill-runs/supplementary/process-billing-period.service.js index 819eff7dd0..b7b148c4e7 100644 --- a/app/services/bill-runs/supplementary/process-billing-period.service.js +++ b/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 ) diff --git a/app/services/bill-runs/supplementary/process-transactions.service.js b/app/services/bill-runs/supplementary/process-transactions.service.js index 203653ac69..730e68ad8a 100644 --- a/app/services/bill-runs/supplementary/process-transactions.service.js +++ b/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} An array of the remaining calculated transactions (ie. those which were not cancelled out by a - * previous matching credit) + * @returns {Promise} 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 } diff --git a/app/services/bill-runs/supplementary/reverse-transactions.service.js b/app/services/bill-runs/supplementary/reverse-transactions.service.js index bfd0e37a5a..f7dbc3a5aa 100644 --- a/app/services/bill-runs/supplementary/reverse-transactions.service.js +++ b/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' } }) } diff --git a/test/services/bill-runs/supplementary/handle-errored-bill-run.service.test.js b/test/services/bill-runs/handle-errored-bill-run.service.test.js similarity index 93% rename from test/services/bill-runs/supplementary/handle-errored-bill-run.service.test.js rename to test/services/bill-runs/handle-errored-bill-run.service.test.js index 3ad287019c..1a2368bc54 100644 --- a/test/services/bill-runs/supplementary/handle-errored-bill-run.service.test.js +++ b/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 diff --git a/test/services/bill-runs/supplementary/fetch-previous-transactions.service.test.js b/test/services/bill-runs/supplementary/fetch-previous-transactions.service.test.js index 2d90261599..31208a9f0d 100644 --- a/test/services/bill-runs/supplementary/fetch-previous-transactions.service.test.js +++ b/test/services/bill-runs/supplementary/fetch-previous-transactions.service.test.js @@ -35,11 +35,7 @@ describe('Fetch Previous Transactions service', () => { describe('when there are no transactions', () => { it('returns no results', async () => { - const result = await FetchPreviousTransactionsService.go( - { billingAccountId }, - { licenceId }, - financialYearEnding - ) + const result = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding) expect(result).to.be.empty() }) @@ -53,11 +49,7 @@ describe('Fetch Previous Transactions service', () => { }) it('returns results', async () => { - const results = await FetchPreviousTransactionsService.go( - { billingAccountId }, - { licenceId }, - financialYearEnding - ) + const results = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding) expect(results).to.have.length(1) expect(results[0].credit).to.be.false() @@ -81,11 +73,7 @@ describe('Fetch Previous Transactions service', () => { }) it('returns no results', async () => { - const results = await FetchPreviousTransactionsService.go( - { billingAccountId }, - { licenceId }, - financialYearEnding - ) + const results = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding) expect(results).to.be.empty() }) @@ -102,11 +90,7 @@ describe('Fetch Previous Transactions service', () => { }) it('returns the debits', async () => { - const results = await FetchPreviousTransactionsService.go( - { billingAccountId }, - { licenceId }, - financialYearEnding - ) + const results = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding) expect(results).to.have.length(1) expect(results[0].credit).to.be.false() @@ -132,11 +116,7 @@ describe('Fetch Previous Transactions service', () => { }) it('returns only the follow up debit', async () => { - const results = await FetchPreviousTransactionsService.go( - { billingAccountId }, - { licenceId }, - financialYearEnding - ) + const results = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding) expect(results).to.have.length(1) expect(results[0].credit).to.be.false() @@ -155,11 +135,7 @@ describe('Fetch Previous Transactions service', () => { }) it('returns both debits', async () => { - const results = await FetchPreviousTransactionsService.go( - { billingAccountId }, - { licenceId }, - financialYearEnding - ) + const results = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding) expect(results).to.have.length(2) expect(results.every((transaction) => !transaction.credit)).to.be.true() @@ -181,11 +157,7 @@ describe('Fetch Previous Transactions service', () => { }) it('returns no results', async () => { - const results = await FetchPreviousTransactionsService.go( - { billingAccountId }, - { licenceId }, - financialYearEnding - ) + const results = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding) expect(results).to.be.empty() }) @@ -202,11 +174,7 @@ describe('Fetch Previous Transactions service', () => { }) it('returns no results', async () => { - const results = await FetchPreviousTransactionsService.go( - { billingAccountId }, - { licenceId }, - financialYearEnding - ) + const results = await FetchPreviousTransactionsService.go(billingAccountId, licenceId, financialYearEnding) expect(results).to.be.empty() }) diff --git a/test/services/bill-runs/supplementary/process-transactions.service.test.js b/test/services/bill-runs/supplementary/process-transactions.service.test.js index 8cc7a5bdd0..baea512a63 100644 --- a/test/services/bill-runs/supplementary/process-transactions.service.test.js +++ b/test/services/bill-runs/supplementary/process-transactions.service.test.js @@ -15,8 +15,11 @@ const FetchPreviousTransactionsService = require('../../../../app/services/bill- const ProcessTransactionsService = require('../../../../app/services/bill-runs/supplementary/process-transactions.service.js') describe('Process Transactions service', () => { - const bill = { id: 'a56ef6d9-370a-4224-b6ec-0fca8bfa4d1f' } - const billLicence = { id: '110ab2e2-6076-4d5a-a56f-b17a048eb269' } + const billingAccountId = 'a56ef6d9-370a-4224-b6ec-0fca8bfa4d1f' + const billLicence = { + id: '110ab2e2-6076-4d5a-a56f-b17a048eb269', + licenceId: '9d587a65-aa00-4be6-969e-5bbb9fc6c885' + } const billingPeriod = { startDate: new Date('2022-04-01'), @@ -55,7 +58,7 @@ describe('Process Transactions service', () => { it('returns the unmatched calculated transactions', async () => { const result = await ProcessTransactionsService.go( calculatedTransactions, - bill, + billingAccountId, billLicence, billingPeriod ) @@ -79,7 +82,7 @@ describe('Process Transactions service', () => { it('returns no transactions', async () => { const result = await ProcessTransactionsService.go( calculatedTransactions, - bill, + billingAccountId, billLicence, billingPeriod ) @@ -101,14 +104,14 @@ describe('Process Transactions service', () => { it('returns only the previous transactions', async () => { const result = await ProcessTransactionsService.go( [], - bill, + billingAccountId, billLicence, billingPeriod ) expect(result).to.have.length(2) - expect(result[0].purposes).to.equal('I_WILL_NOT_BE_REMOVED_1') - expect(result[1].purposes).to.equal('I_WILL_NOT_BE_REMOVED_2') + expect(result[0].purposes).to.equal(['I_WILL_NOT_BE_REMOVED_1']) + expect(result[1].purposes).to.equal(['I_WILL_NOT_BE_REMOVED_2']) }) }) @@ -126,14 +129,14 @@ describe('Process Transactions service', () => { it('returns the unmatched calculated transactions and previous transactions (reversed)', async () => { const result = await ProcessTransactionsService.go( calculatedTransactions, - bill, + billingAccountId, billLicence, billingPeriod ) expect(result).to.have.length(2) expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION_3') - expect(result[1].purposes).to.equal('I_WILL_NOT_BE_REMOVED') + expect(result[1].purposes).to.equal(['I_WILL_NOT_BE_REMOVED']) expect(result[1].credit).to.be.true() }) }) @@ -148,7 +151,7 @@ describe('Process Transactions service', () => { it('returns the calculated transactions unchanged', async () => { const result = await ProcessTransactionsService.go( calculatedTransactions, - bill, + billingAccountId, billLicence, billingPeriod ) diff --git a/test/services/bill-runs/supplementary/reverse-transactions.service.test.js b/test/services/bill-runs/supplementary/reverse-transactions.service.test.js index 755422a5a4..ec7fa8cc98 100644 --- a/test/services/bill-runs/supplementary/reverse-transactions.service.test.js +++ b/test/services/bill-runs/supplementary/reverse-transactions.service.test.js @@ -13,22 +13,24 @@ const ReverseTransactionsService = require('../../../../app/services/bill-runs/s describe('Reverse Transactions service', () => { const transactions = [ { - billingAccountId: '7190937e-e176-4d50-ae4f-c00c5e76938a', - accountNumber: 'B12345678A', name: 'DEBIT', credit: false, status: 'TO_BE_OVERWRITTEN', - purposes: ['foo'] + purposes: [{ + id: '04cbede8-45cf-433e-b4f5-f33dc911ced0', + abstractionPeriodStartDay: 1, + abstractionPeriodStartMonth: 4, + abstractionPeriodEndDay: 31, + abstractionPeriodEndMonth: 3 + }] } ] - const billLicence = { - id: '8affaa71-c185-4b6c-9814-4c615c235611' - } + const billLicenceId = '8affaa71-c185-4b6c-9814-4c615c235611' describe('when the service is called', () => { it('returns reversing transactions', () => { - const result = ReverseTransactionsService.go(transactions, billLicence) + const result = ReverseTransactionsService.go(transactions, billLicenceId) expect(result).to.have.length(transactions.length) @@ -38,9 +40,15 @@ describe('Reverse Transactions service', () => { expect(result[0].name).to.equal('DEBIT') expect(result[0].credit).to.be.true() expect(result[0].status).to.equal('candidate') - expect(result[0].billLicenceId).to.equal(billLicence.id) + expect(result[0].billLicenceId).to.equal('8affaa71-c185-4b6c-9814-4c615c235611') expect(result[0].id).to.exist().and.to.be.a.string() - expect(result[0].purposes).to.equal('foo') + expect(result[0].purposes).to.equal([{ + id: '04cbede8-45cf-433e-b4f5-f33dc911ced0', + abstractionPeriodStartDay: 1, + abstractionPeriodStartMonth: 4, + abstractionPeriodEndDay: 31, + abstractionPeriodEndMonth: 3 + }]) }) }) })