diff --git a/app/services/supplementary-billing/fetch-invoice-account-numbers.service.js b/app/services/supplementary-billing/fetch-invoice-account-numbers.service.js new file mode 100644 index 0000000000..92a45011ce --- /dev/null +++ b/app/services/supplementary-billing/fetch-invoice-account-numbers.service.js @@ -0,0 +1,52 @@ +'use strict' + +/** + * Fetches all invoice account numbers for the supplied charge versions + * @module FetchInvoiceAccountNumbersService + */ + +const InvoiceAccountModel = require('../../models/crm-v2/invoice-account.model.js') + +/** + * Fetch all invoice account numbers for the supplied charge versions + * + * @param {module:ChargeVersionModel[]} chargeVersions An array of charge versions + * + * @returns {Object[]} Array of objects in the format { invoiceAccountId: '...', invoiceAccountNumber: '...' } + */ +async function go (chargeVersions) { + const uniqueInvoiceAccountIds = _extractUniqueInvoiceAccountIds(chargeVersions) + const invoiceAccountModels = await _fetch(uniqueInvoiceAccountIds) + + // The results come back from Objection as InvoiceAccountModels. Since we want to be clear that these are not + // full-blown models, we turn them into plain objects using Objection's .toJSON() method + const invoiceAccountObjects = _makeObjects(invoiceAccountModels) + + return invoiceAccountObjects +} + +function _extractUniqueInvoiceAccountIds (chargeVersions) { + const allInvoiceAccountIds = chargeVersions.map((chargeVersion) => { + return chargeVersion.invoiceAccountId + }) + + // Creating a new set from allInvoiceAccountIds gives us just the unique ids. Objection does not accept sets in + // .findByIds() so we spread it into an array + return [...new Set(allInvoiceAccountIds)] +} + +function _fetch (uniqueInvoiceAccountIds) { + return InvoiceAccountModel.query() + .select('invoiceAccountId', 'invoiceAccountNumber') + .findByIds([...uniqueInvoiceAccountIds]) +} + +function _makeObjects (models) { + return models.map(model => { + return model.toJSON() + }) +} + +module.exports = { + go +} diff --git a/app/services/supplementary-billing/generate-billing-invoice.service.js b/app/services/supplementary-billing/generate-billing-invoice.service.js index f41f96fa57..dafec846b7 100644 --- a/app/services/supplementary-billing/generate-billing-invoice.service.js +++ b/app/services/supplementary-billing/generate-billing-invoice.service.js @@ -7,44 +7,20 @@ const { randomUUID } = require('crypto') -const InvoiceAccountModel = require('../../models/crm-v2/invoice-account.model.js') - /** - * Return either a new billing invoice object ready for persisting or an existing one if it exists - * - * This first checks whether the invoice account ID of `currentBillingInvoice` matches the one passed to this service. - * If it does, we return that instance. - * - * If it doesn't, we generate a new instance and return it. - * - * For context, this is all to avoid creating `billing_invoice` and `billing_invoice_licence` records unnecessarily. - * The legacy service will create them first, then determine if there are any transactions to be billed. If there - * aren't, it then has to go back and delete the records it created. + * Return a billing invoice object ready for persisting * - * Our intent is to only call the DB when we have records that need persisting. So, we start at the transaction level - * and only persist `billing_invoice` and `billing_invoice_licence` records that are linked to billable transactions. - * But to persist the billing transactions we need the foreign keys. So, we generate our billing invoice and billing - * licence data in memory along with ID's, and use this service to provide the right record when persisting the - * transaction. - * - * @param {module:BillingInvoiceModel} currentBillingInvoice A billing invoice object - * @param {String} invoiceAccountId UUID of the invoice account this billing invoice will be linked to if persisted - * @param {String} billingBatchId UUID of the billing batch this billing invoice will be linked to if persisted + * @param {module:InvoiceAccountModel} invoiceAccount The invoice account this billing invoice will be linked to + * @param {String} billingBatchId UUID of the billing batch this billing invoice will be linked to * @param {Number} financialYearEnding A value that must exist in the persisted record * - * @returns {Object} The current or newly-generated billing invoice object + * @returns {Object} The billing invoice object ready to be persisted */ -async function go (currentBillingInvoice, invoiceAccountId, billingBatchId, financialYearEnding) { - if (currentBillingInvoice?.invoiceAccountId === invoiceAccountId) { - return currentBillingInvoice - } - - const invoiceAccount = await InvoiceAccountModel.query().findById(invoiceAccountId) - +function go (invoiceAccount, billingBatchId, financialYearEnding) { const billingInvoice = { billingBatchId, financialYearEnding, - invoiceAccountId, + invoiceAccountId: invoiceAccount.invoiceAccountId, billingInvoiceId: randomUUID({ disableEntropyCache: true }), address: {}, // Address is set to an empty object for SROC billing invoices invoiceAccountNumber: invoiceAccount.invoiceAccountNumber, diff --git a/app/services/supplementary-billing/process-billing-batch.service.js b/app/services/supplementary-billing/process-billing-batch.service.js index 18544b30e2..50b8cd1aaf 100644 --- a/app/services/supplementary-billing/process-billing-batch.service.js +++ b/app/services/supplementary-billing/process-billing-batch.service.js @@ -15,6 +15,7 @@ const CreateBillingTransactionService = require('./create-billing-transaction.se const DetermineChargePeriodService = require('./determine-charge-period.service.js') const DetermineMinimumChargeService = require('./determine-minimum-charge.service.js') const FetchChargeVersionsService = require('./fetch-charge-versions.service.js') +const FetchInvoiceAccountNumbersService = require('./fetch-invoice-account-numbers.service.js') const GenerateBillingTransactionsService = require('./generate-billing-transactions.service.js') const GenerateBillingInvoiceService = require('./generate-billing-invoice.service.js') const GenerateBillingInvoiceLicenceService = require('./generate-billing-invoice-licence.service.js') @@ -49,13 +50,16 @@ async function go (billingBatch, billingPeriod) { await _updateStatus(billingBatchId, 'processing') const chargeVersions = await _fetchChargeVersions(billingBatch, billingPeriod) + const invoiceAccounts = await _fetchInvoiceData(chargeVersions, billingBatch.billingBatchId) for (const chargeVersion of chargeVersions) { - const { billingInvoice, billingInvoiceLicence } = await _generateInvoiceData( + const billingInvoice = _generateInvoiceData(invoiceAccounts, chargeVersion, billingBatchId, billingPeriod) + + const billingInvoiceLicence = _generateInvoiceLicenceData( currentBillingData, billingBatch, chargeVersion, - billingPeriod + billingInvoice ) // If we've moved on to the next invoice licence then we need to finalise the previous one before we can continue @@ -86,6 +90,44 @@ async function go (billingBatch, billingPeriod) { } } +function _generateInvoiceData (invoiceAccounts, chargeVersion, billingBatchId, billingPeriod) { + // Pull the invoice account from our previously-fetched accounts + const invoiceAccount = invoiceAccounts[chargeVersion.invoiceAccountId] + + const billingInvoice = GenerateBillingInvoiceService.go( + invoiceAccount, + billingBatchId, + billingPeriod.endDate.getFullYear() + ) + + return billingInvoice +} + +async function _fetchInvoiceData (chargeVersions, billingBatchId) { + try { + const invoiceAccountsArray = await FetchInvoiceAccountNumbersService.go(chargeVersions) + + // We create a keyed object from the array so we can quickly retrieve the required invoice account later. This will + // be in the format: + // { + // 'uuid-1': { invoiceAccountId: 'uuid-1', ... }, + // 'uuid-2': { invoiceAccountId: 'uuid-2', ... } + // } + const invoiceAccountsObject = invoiceAccountsArray.reduce((acc, item) => { + return { + ...acc, + [item.invoiceAccountId]: item + } + }, {}) + + return invoiceAccountsObject + } catch (error) { + HandleErroredBillingBatchService.go(billingBatchId) + + throw error + } +} + function _generateTransactionsIfStatusIsCurrent (chargeVersion, billingPeriod, billingBatchId, billingInvoiceLicence, currentBillingData) { // If the charge version status isn't 'current' then we don't need to add any new debit lines to the bill if (chargeVersion.status !== 'current') { @@ -255,19 +297,15 @@ async function _finaliseCurrentInvoiceLicence (currentBillingData, billingPeriod } } -async function _generateInvoiceData (currentBillingData, billingBatch, chargeVersion, billingPeriod) { +function _generateInvoiceLicenceData (currentBillingData, billingBatch, chargeVersion, billingInvoice) { try { - const { invoiceAccountId, licence } = chargeVersion - const { billingBatchId } = billingBatch - const financialYearEnding = billingPeriod.endDate.getFullYear() - - const billingInvoice = await GenerateBillingInvoiceService.go(currentBillingData.billingInvoice, invoiceAccountId, billingBatchId, financialYearEnding) - const billingInvoiceLicence = GenerateBillingInvoiceLicenceService.go(currentBillingData.billingInvoiceLicence, billingInvoice.billingInvoiceId, licence) + const billingInvoiceLicence = GenerateBillingInvoiceLicenceService.go( + currentBillingData.billingInvoiceLicence, + billingInvoice.billingInvoiceId, + chargeVersion.licence + ) - return { - billingInvoice, - billingInvoiceLicence - } + return billingInvoiceLicence } catch (error) { HandleErroredBillingBatchService.go(billingBatch.billingBatchId) diff --git a/test/services/supplementary-billing/fetch-invoice-account-numbers.service.test.js b/test/services/supplementary-billing/fetch-invoice-account-numbers.service.test.js new file mode 100644 index 0000000000..c52daca57e --- /dev/null +++ b/test/services/supplementary-billing/fetch-invoice-account-numbers.service.test.js @@ -0,0 +1,55 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') + +const { describe, it, beforeEach } = exports.lab = Lab.script() +const { expect } = Code + +// Test helpers +const DatabaseHelper = require('../../support/helpers/database.helper.js') +const InvoiceAccountHelper = require('../../support/helpers/crm-v2/invoice-account.helper.js') + +// Thing under test +const FetchInvoiceAccountNumbersService = require('../../../app/services/supplementary-billing/fetch-invoice-account-numbers.service.js') + +describe('Fetch Invoice Account Numbers service', () => { + beforeEach(async () => { + await DatabaseHelper.clean() + }) + + describe('when the service is called with an array of charge versions', () => { + let expectedResult + let invoiceAccounts + + beforeEach(async () => { + // We create three invoice accounts but we will only be fetching the first two + invoiceAccounts = await Promise.all([ + InvoiceAccountHelper.add(), + InvoiceAccountHelper.add(), + InvoiceAccountHelper.add() + ]) + + expectedResult = [ + { + invoiceAccountId: invoiceAccounts[0].invoiceAccountId, + invoiceAccountNumber: invoiceAccounts[0].invoiceAccountNumber + }, + { + invoiceAccountId: invoiceAccounts[1].invoiceAccountId, + invoiceAccountNumber: invoiceAccounts[1].invoiceAccountNumber + } + ] + }) + + it('fetches the invoice accounts that the charge versions link to', async () => { + const result = await FetchInvoiceAccountNumbersService.go([ + { invoiceAccountId: invoiceAccounts[0].invoiceAccountId }, + { invoiceAccountId: invoiceAccounts[1].invoiceAccountId } + ]) + + expect(result).to.have.length(2).and.contain(expectedResult) + }) + }) +}) diff --git a/test/services/supplementary-billing/generate-billing-invoice.service.test.js b/test/services/supplementary-billing/generate-billing-invoice.service.test.js index 7d5d45c79a..ed22be8f5e 100644 --- a/test/services/supplementary-billing/generate-billing-invoice.service.test.js +++ b/test/services/supplementary-billing/generate-billing-invoice.service.test.js @@ -18,7 +18,6 @@ describe('Generate billing invoice service', () => { const billingBatchId = 'f4fb6257-c50f-46ea-80b0-7533423d6efd' const financialYearEnding = 2023 - let currentBillingInvoice let expectedResult let invoiceAccount @@ -26,74 +25,28 @@ describe('Generate billing invoice service', () => { await DatabaseHelper.clean() invoiceAccount = await InvoiceAccountHelper.add() - }) - describe('when `currentBillingInvoice` is null', () => { - beforeEach(async () => { - currentBillingInvoice = null - expectedResult = _billingInvoiceGenerator(invoiceAccount, billingBatchId, financialYearEnding) - }) + expectedResult = { + invoiceAccountId: invoiceAccount.invoiceAccountId, + address: {}, + invoiceAccountNumber: invoiceAccount.invoiceAccountNumber, + billingBatchId, + financialYearEnding, + isCredit: false + } + }) - it('returns a new billing invoice with the provided values', async () => { - const result = await GenerateBillingInvoiceService.go( - currentBillingInvoice, - invoiceAccount.invoiceAccountId, + describe('when called', () => { + it('returns a new billing invoice with the provided values', () => { + const result = GenerateBillingInvoiceService.go( + invoiceAccount, billingBatchId, financialYearEnding ) expect(result).to.equal(expectedResult, { skip: 'billingInvoiceId' }) - }) - }) - - describe('when `currentBillingInvoice` is set', () => { - beforeEach(async () => { - currentBillingInvoice = _billingInvoiceGenerator(invoiceAccount, billingBatchId, financialYearEnding) - }) - - describe('and the invoice account ID matches', () => { - it('returns the `currentBillingInvoice`', async () => { - const result = await GenerateBillingInvoiceService.go( - currentBillingInvoice, - currentBillingInvoice.invoiceAccountId, - billingBatchId, - financialYearEnding - ) - - expect(result).to.equal(currentBillingInvoice) - }) - }) - - describe('and the invoice account ID does not match', () => { - let otherInvoiceAccount - - beforeEach(async () => { - otherInvoiceAccount = await InvoiceAccountHelper.add() - }) - - it('returns a new billing invoice with the provided values', async () => { - const result = await GenerateBillingInvoiceService.go( - currentBillingInvoice, - otherInvoiceAccount.invoiceAccountId, - billingBatchId, - financialYearEnding - ) - - expect(result).not.to.equal(currentBillingInvoice) - expect(result.invoiceAccountId).to.equal(otherInvoiceAccount.invoiceAccountId) - }) + // Separate check for billingInvoiceId as it will be a random UUID + expect(result.billingInvoiceId).to.be.a.string().and.to.have.length(36) }) }) }) - -function _billingInvoiceGenerator (invoiceAccount, billingBatchId, financialYearEnding) { - return { - billingInvoiceId: 'fa0c763e-3976-42df-ae2c-e93a954701dd', - invoiceAccountId: invoiceAccount.invoiceAccountId, - address: {}, - invoiceAccountNumber: invoiceAccount.invoiceAccountNumber, - billingBatchId, - financialYearEnding, - isCredit: false - } -} diff --git a/test/services/supplementary-billing/process-billing-batch.service.test.js b/test/services/supplementary-billing/process-billing-batch.service.test.js index d79591063b..1781c2d4ee 100644 --- a/test/services/supplementary-billing/process-billing-batch.service.test.js +++ b/test/services/supplementary-billing/process-billing-batch.service.test.js @@ -27,7 +27,7 @@ const BillingInvoiceLicenceModel = require('../../../app/models/water/billing-in const ChargingModuleCreateTransactionService = require('../../../app/services/charging-module/create-transaction.service.js') const ChargingModuleGenerateService = require('../../../app/services/charging-module/generate-bill-run.service.js') const FetchChargeVersionsService = require('../../../app/services/supplementary-billing/fetch-charge-versions.service.js') -const GenerateBillingInvoiceService = require('../../../app/services/supplementary-billing/generate-billing-invoice.service.js') +const FetchInvoiceAccountNumbersService = require('../../../app/services/supplementary-billing/fetch-invoice-account-numbers.service.js') const GenerateBillingTransactionsService = require('../../../app/services/supplementary-billing/generate-billing-transactions.service.js') const HandleErroredBillingBatchService = require('../../../app/services/supplementary-billing/handle-errored-billing-batch.service.js') @@ -244,6 +244,21 @@ describe('Process billing batch service', () => { }) }) + describe('because fetching the invoice accounts fails', () => { + beforeEach(() => { + Sinon.stub(FetchInvoiceAccountNumbersService, 'go').rejects() + }) + + it('sets no error code', async () => { + await ProcessBillingBatchService.go(billingBatch, billingPeriod) + + const handlerArgs = handleErroredBillingBatchStub.firstCall.args + + // Check that only the billing batch id was passed and not an error code as well + expect(handlerArgs).to.have.length(1) + }) + }) + describe('because generating the calculated transactions fails', () => { beforeEach(async () => { const { chargeVersionId } = await ChargeVersionHelper.add({ @@ -309,31 +324,6 @@ describe('Process billing batch service', () => { }) }) - describe('because generating the invoice data fails', () => { - beforeEach(async () => { - const { chargeVersionId } = await ChargeVersionHelper.add({ - changeReasonId: changeReason.changeReasonId, - invoiceAccountId: invoiceAccount.invoiceAccountId, - licenceId: licence.licenceId - }) - const { chargeElementId } = await ChargeElementHelper.add( - { billingChargeCategoryId: billingChargeCategory.billingChargeCategoryId, chargeVersionId } - ) - await ChargePurposeHelper.add({ chargeElementId }) - - Sinon.stub(GenerateBillingInvoiceService, 'go').rejects() - }) - - it('sets no error code', async () => { - await ProcessBillingBatchService.go(billingBatch, billingPeriod) - - const handlerArgs = handleErroredBillingBatchStub.firstCall.args - - // Check that only the billing batch id was passed and not an error code as well - expect(handlerArgs).to.have.length(1) - }) - }) - describe('because finalising the billing batch fails', () => { beforeEach(async () => { const { chargeVersionId } = await ChargeVersionHelper.add({