From 75d45422078eec7b2ca916d0fd16cd005593da4a Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 18 Apr 2023 14:28:13 +0100 Subject: [PATCH 01/10] Fix re-processing REPLACED charge versions https://eaflood.atlassian.net/browse/WATER-3950 > TBD From 4799305f9bfe8bc1f2e7924648f3b31b15576e0b Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 18 Apr 2023 14:30:38 +0100 Subject: [PATCH 02/10] Implement possible fix Though have confirmed manually locally via a number of scenarios this does the trick. --- .../fetch-charge-versions.service.js | 5 +++-- .../process-billing-batch.service.js | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/services/supplementary-billing/fetch-charge-versions.service.js b/app/services/supplementary-billing/fetch-charge-versions.service.js index afe3cefc20..8b3e65bc07 100644 --- a/app/services/supplementary-billing/fetch-charge-versions.service.js +++ b/app/services/supplementary-billing/fetch-charge-versions.service.js @@ -34,14 +34,15 @@ async function _fetch (regionId, billingPeriod) { 'scheme', 'chargeVersions.startDate', 'chargeVersions.endDate', - 'invoiceAccountId' + 'invoiceAccountId', + 'status' ]) .innerJoinRelated('licence') .where('scheme', 'sroc') .whereNotNull('invoiceAccountId') .where('includeInSrocSupplementaryBilling', true) .where('regionId', regionId) - .where('chargeVersions.status', 'current') + // .where('chargeVersions.status', 'current') .where('chargeVersions.startDate', '>=', billingPeriod.startDate) .where('chargeVersions.startDate', '<=', billingPeriod.endDate) .whereNotExists( diff --git a/app/services/supplementary-billing/process-billing-batch.service.js b/app/services/supplementary-billing/process-billing-batch.service.js index 625cd629b0..5c5618408c 100644 --- a/app/services/supplementary-billing/process-billing-batch.service.js +++ b/app/services/supplementary-billing/process-billing-batch.service.js @@ -73,12 +73,14 @@ async function go (billingBatch, billingPeriod) { currentBillingData.billingInvoice = billingInvoice currentBillingData.billingInvoiceLicence = billingInvoiceLicence - const calculatedTransactions = _generateCalculatedTransactions(billingPeriod, chargeVersion, billingBatchId, billingInvoiceLicence) - currentBillingData.calculatedTransactions.push(...calculatedTransactions) + if (chargeVersion.status === 'current') { + const calculatedTransactions = _generateCalculatedTransactions(billingPeriod, chargeVersion, billingBatchId, billingInvoiceLicence) + currentBillingData.calculatedTransactions.push(...calculatedTransactions) + } } await _finaliseCurrentInvoiceLicence(currentBillingData, billingPeriod, billingBatch) - await _processReplacedChargeVersions(currentBillingData, billingBatch, billingPeriod) + // await _processReplacedChargeVersions(currentBillingData, billingBatch, billingPeriod) await _finaliseBillingBatch(billingBatch, chargeVersions, currentBillingData.isEmpty) From 36e7bc88528090722597793b4df9919150c24e36 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 18 Apr 2023 17:18:50 +0100 Subject: [PATCH 03/10] Clean up FetchChargeVersionsService Remove the commented out line and update the documentation to reflect what is now going on. Also tidied up the imports. --- .../fetch-charge-versions.service.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/app/services/supplementary-billing/fetch-charge-versions.service.js b/app/services/supplementary-billing/fetch-charge-versions.service.js index 8b3e65bc07..d55fe8748f 100644 --- a/app/services/supplementary-billing/fetch-charge-versions.service.js +++ b/app/services/supplementary-billing/fetch-charge-versions.service.js @@ -1,25 +1,23 @@ 'use strict' -const { ref } = require('objection') /** - * Fetches SROC charge versions that might be included in a supplementary bill run + * Fetches SROC charge versions linked to licences flagged for inclusion in next SROC supplementary billing * @module FetchChargeVersionsService */ +const { ref } = require('objection') + const ChargeVersion = require('../../models/water/charge-version.model.js') const ChargeVersionWorkflow = require('../../models/water/charge-version-workflow.model.js') /** - * Fetch all 'current' SROC charge versions linked to licences flagged for supplementary billing that are in the period - * being billed - * - * > This is not the final form of the service. It is a 'work in progress' as we implement tickets that gradually - * > build up our understanding of SROC supplementary billing + * Fetch all SROC charge versions linked to licences flagged for supplementary billing that are in the period being + * billed * - * @param {string} regionId GUID of the region which the licences will be linked to + * @param {string} regionId UUID of the region being billed that the licences must be linked to * @param {Object} billingPeriod Object with a `startDate` and `endDate` property representing the period being billed * - * @returns an array of Objects containing the relevant charge versions + * @returns {Object[]} an array of matching charge versions */ async function go (regionId, billingPeriod) { const chargeVersions = await _fetch(regionId, billingPeriod) @@ -42,7 +40,6 @@ async function _fetch (regionId, billingPeriod) { .whereNotNull('invoiceAccountId') .where('includeInSrocSupplementaryBilling', true) .where('regionId', regionId) - // .where('chargeVersions.status', 'current') .where('chargeVersions.startDate', '>=', billingPeriod.startDate) .where('chargeVersions.startDate', '<=', billingPeriod.endDate) .whereNotExists( From 137e59defa6ef9f0d9cd7612934a71e1bbe6281c Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 19 Apr 2023 11:38:18 +0100 Subject: [PATCH 04/10] Update FetchChargeVersionsService test desc. We're going to have to make some changes to the tests. But now we know better what we expect the service to do, and that it is more charge version than licence focused, we take the opportunity to update the test descriptions. --- .../fetch-charge-versions.service.test.js | 43 +++++++++++-------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/test/services/supplementary-billing/fetch-charge-versions.service.test.js b/test/services/supplementary-billing/fetch-charge-versions.service.test.js index 4e83a22b9f..6b377691e4 100644 --- a/test/services/supplementary-billing/fetch-charge-versions.service.test.js +++ b/test/services/supplementary-billing/fetch-charge-versions.service.test.js @@ -36,7 +36,7 @@ describe('Fetch Charge Versions service', () => { regionId = region.regionId }) - describe('when there are licences to be included in supplementary billing', () => { + describe('when there are charge versions that should be considered for the next supplementary billing', () => { let billingChargeCategory let chargeElement let chargePurpose @@ -48,29 +48,36 @@ describe('Fetch Charge Versions service', () => { endDate: new Date('2023-03-31') } + const { licenceId } = await LicenceHelper.add({ + regionId, + isWaterUndertaker: true, + includeInSrocSupplementaryBilling: true, + includeInSupplementaryBilling: 'yes' + }) changeReason = await ChangeReasonHelper.add({ triggersMinimumCharge: true }) - // This creates an SROC charge version linked to a licence marked for supplementary billing + // This creates a 'current' SROC charge version const srocChargeVersion = await ChargeVersionHelper.add( { changeReasonId: changeReason.changeReasonId }, - { regionId, isWaterUndertaker: true, includeInSrocSupplementaryBilling: true } + { licenceId } ) - // This creates an SROC charge version linked to a licence marked for supplementary billing - // with a status of 'superseded' + // This creates a 'superseded' SROC charge version const srocSupersededChargeVersion = await ChargeVersionHelper.add( { changeReasonId: changeReason.changeReasonId, status: 'superseded' }, - { regionId, isWaterUndertaker: true, includeInSrocSupplementaryBilling: true } + { licenceId } ) - // This creates an ALCS (presroc) charge version linked to a licence marked for supplementary billing + // This creates an ALCS (presroc) charge version const alcsChargeVersion = await ChargeVersionHelper.add( { scheme: 'alcs' }, - { regionId, includeInSupplementaryBilling: 'yes' } + { licenceId } ) testRecords = [srocChargeVersion, srocSupersededChargeVersion, alcsChargeVersion] + // We test that related data is returned in the results. So, we create and link it to the srocChargeVersion + // ready for testing billingChargeCategory = await BillingChargeCategoryHelper.add() chargeElement = await ChargeElementHelper.add({ @@ -90,7 +97,7 @@ describe('Fetch Charge Versions service', () => { expect(result[0].chargeVersionId).to.equal(testRecords[0].chargeVersionId) }) - it('includes related licence and region', async () => { + it('includes the related licence and region', async () => { const result = await FetchChargeVersionsService.go(regionId, billingPeriod) expect(result[0].licence.licenceRef).to.equal(licenceDefaults.licenceRef) @@ -101,13 +108,13 @@ describe('Fetch Charge Versions service', () => { expect(result[0].licence.region.chargeRegionId).to.equal(region.chargeRegionId) }) - it('includes related change reason', async () => { + it('includes the related change reason', async () => { const result = await FetchChargeVersionsService.go(regionId, billingPeriod) expect(result[0].changeReason.triggersMinimumCharge).to.equal(changeReason.triggersMinimumCharge) }) - it('includes related charge elements, billing charge category and charge purposes', async () => { + it('includes the related charge elements, billing charge category and charge purposes', async () => { const result = await FetchChargeVersionsService.go(regionId, billingPeriod) const expectedResult = { @@ -137,8 +144,8 @@ describe('Fetch Charge Versions service', () => { }) }) - describe('when there are no licences to be included in supplementary billing', () => { - describe("because none of them are marked 'includeInSrocSupplementaryBilling'", () => { + describe('when there are no charge versions that should be considered for the next supplementary billing', () => { + describe("because none of them are linked to a licence flagged 'includeInSrocSupplementaryBilling'", () => { beforeEach(async () => { billingPeriod = { startDate: new Date('2022-04-01'), @@ -183,7 +190,7 @@ describe('Fetch Charge Versions service', () => { }) }) - describe("because all the applicable charge versions are 'alcs' (presroc)", () => { + describe("because all of them are for the 'alcs' (presroc) scheme", () => { beforeEach(async () => { billingPeriod = { startDate: new Date('2022-04-01'), @@ -205,7 +212,7 @@ describe('Fetch Charge Versions service', () => { }) }) - describe('because all the applicable charge versions have no `invoiceAccountId`', () => { + describe('because none of them have an `invoiceAccountId`', () => { beforeEach(async () => { billingPeriod = { startDate: new Date('2022-04-01'), @@ -227,7 +234,7 @@ describe('Fetch Charge Versions service', () => { }) }) - describe('because there are no charge versions in the billing period', () => { + describe('because none of them are in the billing period', () => { describe('as they all have start dates before the billing period', () => { beforeEach(async () => { billingPeriod = { @@ -275,7 +282,7 @@ describe('Fetch Charge Versions service', () => { }) }) - describe('because there are no licences linked to the selected region', () => { + describe('because the licences flagged for supplementary billing are linked to a different region', () => { beforeEach(async () => { billingPeriod = { startDate: new Date('2022-04-01'), @@ -300,7 +307,7 @@ describe('Fetch Charge Versions service', () => { }) }) - describe('because the licence is in workflow', () => { + describe('because they are all linked to licences in workflow', () => { beforeEach(async () => { billingPeriod = { startDate: new Date('2022-04-01'), From 1d2d7001bf053698bda5b5fbac075683d947c892 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 19 Apr 2023 11:52:24 +0100 Subject: [PATCH 05/10] Fix 'current' tests Fix those tests that focused on checking we were only returning 'current' (APPROVED) charge versions. --- .../fetch-charge-versions.service.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/services/supplementary-billing/fetch-charge-versions.service.test.js b/test/services/supplementary-billing/fetch-charge-versions.service.test.js index 6b377691e4..7cbd2138e4 100644 --- a/test/services/supplementary-billing/fetch-charge-versions.service.test.js +++ b/test/services/supplementary-billing/fetch-charge-versions.service.test.js @@ -90,11 +90,12 @@ describe('Fetch Charge Versions service', () => { }) }) - it("returns only the 'current' SROC charge versions that are applicable", async () => { + it("returns both 'current' and 'superseded' SROC charge versions that are applicable", async () => { const result = await FetchChargeVersionsService.go(regionId, billingPeriod) - expect(result).to.have.length(1) + expect(result).to.have.length(2) expect(result[0].chargeVersionId).to.equal(testRecords[0].chargeVersionId) + expect(result[1].chargeVersionId).to.equal(testRecords[1].chargeVersionId) }) it('includes the related licence and region', async () => { @@ -138,8 +139,6 @@ describe('Fetch Charge Versions service', () => { }] } - expect(result).to.have.length(1) - expect(result[0].chargeVersionId).to.equal(testRecords[0].chargeVersionId) expect(result[0].chargeElements[0]).to.equal(expectedResult) }) }) From 4cb1ea5113889d8f744992007a783560eca7b092 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 19 Apr 2023 11:53:19 +0100 Subject: [PATCH 06/10] Fix 'draft' tests One of our failing tests highlighted we had been a bit to eager when deleting the status WHERE condition. Though we've not seen much evidence of its use, charge versions do have a status of 'draft'. So, just in case we add that WHERE condition back in and update the tests appropriately. --- .../fetch-charge-versions.service.js | 1 + .../fetch-charge-versions.service.test.js | 8 ++------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/services/supplementary-billing/fetch-charge-versions.service.js b/app/services/supplementary-billing/fetch-charge-versions.service.js index d55fe8748f..99fac7aad1 100644 --- a/app/services/supplementary-billing/fetch-charge-versions.service.js +++ b/app/services/supplementary-billing/fetch-charge-versions.service.js @@ -42,6 +42,7 @@ async function _fetch (regionId, billingPeriod) { .where('regionId', regionId) .where('chargeVersions.startDate', '>=', billingPeriod.startDate) .where('chargeVersions.startDate', '<=', billingPeriod.endDate) + .whereNot('chargeVersions.status', 'draft') .whereNotExists( ChargeVersionWorkflow.query() .select(1) diff --git a/test/services/supplementary-billing/fetch-charge-versions.service.test.js b/test/services/supplementary-billing/fetch-charge-versions.service.test.js index 7cbd2138e4..ec938492e3 100644 --- a/test/services/supplementary-billing/fetch-charge-versions.service.test.js +++ b/test/services/supplementary-billing/fetch-charge-versions.service.test.js @@ -164,22 +164,18 @@ describe('Fetch Charge Versions service', () => { }) }) - describe("because all the applicable charge versions do not have a 'current' status", () => { + describe("because all the applicable charge versions have a 'draft' status", () => { beforeEach(async () => { billingPeriod = { startDate: new Date('2022-04-01'), endDate: new Date('2023-03-31') } - const srocSupersededChargeVersion = await ChargeVersionHelper.add( - { status: 'superseded' }, - { regionId, isWaterUndertaker: true, includeInSrocSupplementaryBilling: true } - ) const srocDraftChargeVersion = await ChargeVersionHelper.add( { status: 'draft' }, { regionId, isWaterUndertaker: true, includeInSrocSupplementaryBilling: true } ) - testRecords = [srocSupersededChargeVersion, srocDraftChargeVersion] + testRecords = [srocDraftChargeVersion] }) it('returns no applicable charge versions', async () => { From 1be03b049b41910c81cd3321e370c1bd2d12b6f0 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 20 Apr 2023 09:30:23 +0100 Subject: [PATCH 07/10] Make the WHERE clauses more explicit We have shied away from adding the table name to the licence fields in the WHERE clauses on the basis if it works don't fix it. But by adding `chargeVersions.` to all the charge version based ones we hope to make it clearer where the source of each comes from. --- .../supplementary-billing/fetch-charge-versions.service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/supplementary-billing/fetch-charge-versions.service.js b/app/services/supplementary-billing/fetch-charge-versions.service.js index 99fac7aad1..b936998bcd 100644 --- a/app/services/supplementary-billing/fetch-charge-versions.service.js +++ b/app/services/supplementary-billing/fetch-charge-versions.service.js @@ -36,10 +36,10 @@ async function _fetch (regionId, billingPeriod) { 'status' ]) .innerJoinRelated('licence') - .where('scheme', 'sroc') - .whereNotNull('invoiceAccountId') .where('includeInSrocSupplementaryBilling', true) .where('regionId', regionId) + .where('chargeVersions.scheme', 'sroc') + .whereNotNull('chargeVersions.invoiceAccountId') .where('chargeVersions.startDate', '>=', billingPeriod.startDate) .where('chargeVersions.startDate', '<=', billingPeriod.endDate) .whereNot('chargeVersions.status', 'draft') From 271f9a7ff21cb2af212451ce701e1147e2067bdd Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 20 Apr 2023 09:39:08 +0100 Subject: [PATCH 08/10] Remove the process replaced step from process Now REPLACED charge versions are included in the initial result set and processed in our main loop we don't need to deal with REPLACED charge versions as an extra step. --- .../process-billing-batch.service.js | 21 +++++-------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/app/services/supplementary-billing/process-billing-batch.service.js b/app/services/supplementary-billing/process-billing-batch.service.js index 5c5618408c..7150eda33d 100644 --- a/app/services/supplementary-billing/process-billing-batch.service.js +++ b/app/services/supplementary-billing/process-billing-batch.service.js @@ -22,7 +22,6 @@ const HandleErroredBillingBatchService = require('./handle-errored-billing-batch const LegacyRequestLib = require('../../lib/legacy-request.lib.js') const LicenceModel = require('../../models/water/licence.model.js') const ProcessBillingTransactionsService = require('./process-billing-transactions.service.js') -const ProcessReplacedChargeVersionsService = require('./process-replaced-charge-versions.service.js') /** * Creates the invoices and transactions in both WRLS and the Charging Module API @@ -73,6 +72,11 @@ async function go (billingBatch, billingPeriod) { currentBillingData.billingInvoice = billingInvoice currentBillingData.billingInvoiceLicence = billingInvoiceLicence + // If the charge version has a status of 'current' (APPROVED) then it is likely to be something we have never + // billed previously so we need to calculate a debit line. + // Else the charge version has been 'superseded' (REPLACED). So, we won't be adding a new debit line to the bill + // for it. But we still need to process it to understand what, if anything, needs to be credited back or if our + // calculated debit line has already been billed. if (chargeVersion.status === 'current') { const calculatedTransactions = _generateCalculatedTransactions(billingPeriod, chargeVersion, billingBatchId, billingInvoiceLicence) currentBillingData.calculatedTransactions.push(...calculatedTransactions) @@ -80,8 +84,6 @@ async function go (billingBatch, billingPeriod) { } await _finaliseCurrentInvoiceLicence(currentBillingData, billingPeriod, billingBatch) - // await _processReplacedChargeVersions(currentBillingData, billingBatch, billingPeriod) - await _finaliseBillingBatch(billingBatch, chargeVersions, currentBillingData.isEmpty) // Log how long the process took @@ -287,19 +289,6 @@ function _logError (billingBatch, error) { }) } -async function _processReplacedChargeVersions (currentBillingData, billingBatch, billingPeriod) { - try { - const anythingGenerated = await ProcessReplacedChargeVersionsService.go(billingBatch, billingPeriod) - if (anythingGenerated) { - currentBillingData.isEmpty = false - } - } catch (error) { - HandleErroredBillingBatchService.go(billingBatch.billingBatchId) - - throw error - } -} - async function _updateStatus (billingBatchId, status) { try { await BillingBatchModel.query() From f560e4f47ec47a51ca9ffbb1244f3d191ff6355a Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 20 Apr 2023 09:42:18 +0100 Subject: [PATCH 09/10] Remove our fetch replaced service This is now redundant based on the other changes we made. --- .../fetch-replaced-charge-versions.service.js | 83 ----- ...h-replaced-charge-versions.service.test.js | 283 ------------------ 2 files changed, 366 deletions(-) delete mode 100644 app/services/supplementary-billing/fetch-replaced-charge-versions.service.js delete mode 100644 test/services/supplementary-billing/fetch-replaced-charge-versions.service.test.js diff --git a/app/services/supplementary-billing/fetch-replaced-charge-versions.service.js b/app/services/supplementary-billing/fetch-replaced-charge-versions.service.js deleted file mode 100644 index 13299fda00..0000000000 --- a/app/services/supplementary-billing/fetch-replaced-charge-versions.service.js +++ /dev/null @@ -1,83 +0,0 @@ -'use strict' - -/** - * Fetches replaced SROC charge versions that have not already been included in the billing batch being processed - * @module FetchReplacedChargeVersionsService - */ - -const { ref } = require('objection') - -const BillingInvoiceModel = require('../../models/water/billing-invoice.model.js') -const ChargeVersion = require('../../models/water/charge-version.model.js') -const ChargeVersionWorkflow = require('../../models/water/charge-version-workflow.model.js') - -/** - * Fetch all 'replaced' (superseded) SROC charge versions linked to licences flagged for supplementary billing that are - * in the period being billed - * - * @param {string} regionId GUID of the region which the licences will be linked to - * @param {Object} billingPeriod Object with a `startDate` and `endDate` property representing the period being billed - * - * @returns an array of Objects containing the relevant charge versions - */ -async function go (regionId, billingPeriod, billingBatchId) { - const chargeVersions = await _fetch(regionId, billingPeriod, billingBatchId) - - return chargeVersions -} - -async function _fetch (regionId, billingPeriod, billingBatchId) { - const chargeVersions = await ChargeVersion.query() - .select([ - 'chargeVersionId', - 'scheme', - 'chargeVersions.startDate', - 'chargeVersions.endDate', - 'invoiceAccountId' - ]) - .innerJoinRelated('licence') - .where('scheme', 'sroc') - .where('includeInSrocSupplementaryBilling', true) - .where('regionId', regionId) - .where('chargeVersions.status', 'superseded') - .where('chargeVersions.startDate', '>=', billingPeriod.startDate) - .where('chargeVersions.startDate', '<=', billingPeriod.endDate) - .whereNotExists( - ChargeVersionWorkflow.query() - .select(1) - .whereColumn('chargeVersions.licenceId', 'chargeVersionWorkflows.licenceId') - ) - .whereNotIn( - 'chargeVersions.invoiceAccountId', - BillingInvoiceModel.query() - .distinct('invoiceAccountId') - .where('billingBatchId', billingBatchId) - ) - .orderBy([ - { column: 'chargeVersions.invoiceAccountId' }, - { column: 'chargeVersions.licenceId' } - ]) - .withGraphFetched('licence') - .modifyGraph('licence', builder => { - builder.select([ - 'licenceId', - 'licenceRef', - 'isWaterUndertaker', - ref('licences.regions:historicalAreaCode').castText().as('historicalAreaCode'), - ref('licences.regions:regionalChargeArea').castText().as('regionalChargeArea') - ]) - }) - .withGraphFetched('licence.region') - .modifyGraph('licence.region', builder => { - builder.select([ - 'regionId', - 'chargeRegionId' - ]) - }) - - return chargeVersions -} - -module.exports = { - go -} diff --git a/test/services/supplementary-billing/fetch-replaced-charge-versions.service.test.js b/test/services/supplementary-billing/fetch-replaced-charge-versions.service.test.js deleted file mode 100644 index bb8b53b2b9..0000000000 --- a/test/services/supplementary-billing/fetch-replaced-charge-versions.service.test.js +++ /dev/null @@ -1,283 +0,0 @@ -'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 BillingInvoiceHelper = require('../../support/helpers/water/billing-invoice.helper.js') -const ChargeVersionHelper = require('../../support/helpers/water/charge-version.helper.js') -const ChargeVersionWorkflowHelper = require('../../support/helpers/water/charge-version-workflow.helper.js') -const DatabaseHelper = require('../../support/helpers/database.helper.js') -const LicenceHelper = require('../../support/helpers/water/licence.helper.js') -const RegionHelper = require('../../support/helpers/water/region.helper.js') - -// Thing under test -const FetchReplacedChargeVersionsService = require('../../../app/services/supplementary-billing/fetch-replaced-charge-versions.service.js') - -describe('Fetch Replaced Charge Versions service', () => { - const billingBatchId = '245b47a3-69be-4b9d-aac3-04f1f7125385' - const licenceDefaults = LicenceHelper.defaults() - - let testRecords - let billingPeriod - let region - let regionId - - beforeEach(async () => { - await DatabaseHelper.clean() - - region = await RegionHelper.add() - regionId = region.regionId - }) - - describe('when there are licences to be included in supplementary billing', () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - // This creates an SROC charge version linked to a licence marked for supplementary billing - const srocChargeVersion = await ChargeVersionHelper.add( - {}, - { regionId, includeInSrocSupplementaryBilling: true } - ) - - // This creates an SROC charge version linked to a licence marked for supplementary billing - // with a status of 'superseded' - const srocSupersededChargeVersion = await ChargeVersionHelper.add( - { status: 'superseded' }, - { regionId, isWaterUndertaker: true, includeInSrocSupplementaryBilling: true } - ) - - // This creates an ALCS (presroc) charge version linked to a licence marked for supplementary billing - // with a status of 'superseded' - const alcsChargeVersion = await ChargeVersionHelper.add( - { scheme: 'alcs', status: 'superseded' }, - { regionId, includeInSupplementaryBilling: 'yes' } - ) - - testRecords = [srocChargeVersion, srocSupersededChargeVersion, alcsChargeVersion] - }) - - it("returns only the 'superseded' SROC charge versions that are applicable", async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.have.length(1) - expect(result[0].chargeVersionId).to.equal(testRecords[1].chargeVersionId) - }) - - it('includes related licence and region', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result[0].licence.licenceRef).to.equal(licenceDefaults.licenceRef) - expect(result[0].licence.isWaterUndertaker).to.equal(true) - expect(result[0].licence.historicalAreaCode).to.equal(licenceDefaults.regions.historicalAreaCode) - expect(result[0].licence.regionalChargeArea).to.equal(licenceDefaults.regions.regionalChargeArea) - expect(result[0].licence.region.regionId).to.equal(regionId) - expect(result[0].licence.region.chargeRegionId).to.equal(region.chargeRegionId) - }) - }) - - describe('when there are no licences to be included in supplementary billing', () => { - describe("because none of them are marked 'includeInSrocSupplementaryBilling'", () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - // This creates an SROC charge version linked to a licence. But the licence won't be marked for supplementary - // billing - const chargeVersion = await ChargeVersionHelper.add( - { status: 'superseded' }, - { regionId, includeInSrocSupplementaryBilling: false } - ) - testRecords = [chargeVersion] - }) - - it('returns no applicable charge versions', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.be.empty() - }) - }) - - describe("because all the applicable charge versions do not have a 'superseded' status", () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - const currentChargeVersion = await ChargeVersionHelper.add( - { status: 'current' }, - { regionId, isWaterUndertaker: true, includeInSrocSupplementaryBilling: true } - ) - const draftChargeVersion = await ChargeVersionHelper.add( - { status: 'draft' }, - { regionId, isWaterUndertaker: true, includeInSrocSupplementaryBilling: true } - ) - testRecords = [currentChargeVersion, draftChargeVersion] - }) - - it('returns no applicable charge versions', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.be.empty() - }) - }) - - describe("because all the applicable charge versions are 'alcs' (presroc)", () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - // This creates an ALCS (presroc) charge version linked to a licence marked for supplementary billing - const chargeVersion = await ChargeVersionHelper.add( - { status: 'superseded', scheme: 'alcs' }, - { regionId, includeInSrocSupplementaryBilling: true } - ) - testRecords = [chargeVersion] - }) - - it('returns no applicable charge versions', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.be.empty() - }) - }) - - describe('because there are no charge versions in the billing period', () => { - describe('as they all have start dates before the billing period', () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - // This creates an SROC charge version with a start date before the billing period. This would have been - // picked up by a previous bill run - const chargeVersion = await ChargeVersionHelper.add( - { status: 'superseded', startDate: new Date(2022, 2, 31) }, // 2022-03-01 - Months are zero indexed :-) - { regionId, includeInSrocSupplementaryBilling: true } - ) - testRecords = [chargeVersion] - }) - - it('returns no applicable charge versions', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.be.empty() - }) - }) - - describe('as they all have start dates after the billing period', () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - // This creates an SROC charge version with a start date after the billing period. This will be picked in - // next years bill runs - const srocChargeVersion = await ChargeVersionHelper.add( - { status: 'superseded', startDate: new Date(2023, 3, 1) }, // 2023-04-01 - Months are zero indexed :-) - { regionId, includeInSrocSupplementaryBilling: true } - ) - testRecords = [srocChargeVersion] - }) - - it('returns no applicable charge versions', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.be.empty() - }) - }) - }) - - describe('because there are no licences linked to the selected region', () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - // This creates an SROC charge version linked to a licence with an different region than selected - const chargeVersion = await ChargeVersionHelper.add( - { status: 'superseded' }, - { - regionId: 'e117b501-e3c1-4337-ad35-21c60ed9ad73', - includeInSrocSupplementaryBilling: true - } - ) - testRecords = [chargeVersion] - }) - - it('returns no applicable charge versions', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.be.empty() - }) - }) - - describe('because the licence is in workflow', () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - const chargeVersion = await ChargeVersionHelper.add( - { status: 'superseded' }, - { - regionId, - includeInSrocSupplementaryBilling: true - } - ) - await ChargeVersionWorkflowHelper.add({ licenceId: chargeVersion.licenceId }) - - testRecords = [chargeVersion] - }) - - it('returns no applicable charge versions', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.be.empty() - }) - }) - - describe('because the replaced charge versions invoice accounts have already been processed', () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - const chargeVersion = await ChargeVersionHelper.add( - { status: 'superseded' }, - { - regionId, - includeInSrocSupplementaryBilling: true - } - ) - const { invoiceAccountId } = chargeVersion - await BillingInvoiceHelper.add({ invoiceAccountId }, { billingBatchId }) - - testRecords = [chargeVersion] - }) - - it('returns no applicable charge versions', async () => { - const result = await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) - - expect(result).to.be.empty() - }) - }) - }) -}) From ee2e4128c65cf70a765d225a1dbf04dc45af0dbe Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 20 Apr 2023 09:43:35 +0100 Subject: [PATCH 10/10] Remove redundant process replaced service Again, with the changes we have made to the process this is no longer needed. --- ...rocess-replaced-charge-versions.service.js | 151 ------------------ ...s-replaced-charge-versions.service.test.js | 143 ----------------- 2 files changed, 294 deletions(-) delete mode 100644 app/services/supplementary-billing/process-replaced-charge-versions.service.js delete mode 100644 test/services/supplementary-billing/process-replaced-charge-versions.service.test.js diff --git a/app/services/supplementary-billing/process-replaced-charge-versions.service.js b/app/services/supplementary-billing/process-replaced-charge-versions.service.js deleted file mode 100644 index cec0d0b50e..0000000000 --- a/app/services/supplementary-billing/process-replaced-charge-versions.service.js +++ /dev/null @@ -1,151 +0,0 @@ -'use strict' - -/** - * Processes the replaced charge versions of licences to be included in supplementary billing - * @module ProcessReplacedChargeVersionsService - */ - -const BillingInvoiceModel = require('../../models/water/billing-invoice.model.js') -const BillingInvoiceLicenceModel = require('../../models/water/billing-invoice-licence.model.js') -const ChargingModuleCreateTransactionService = require('../charging-module/create-transaction.service.js') -const ChargingModuleCreateTransactionPresenter = require('../../presenters/charging-module/create-transaction.presenter.js') -const CreateBillingTransactionService = require('./create-billing-transaction.service.js') -const FetchReplacedChargeVersionsService = require('./fetch-replaced-charge-versions.service.js') -const GenerateBillingInvoiceService = require('./generate-billing-invoice.service.js') -const GenerateBillingInvoiceLicenceService = require('./generate-billing-invoice-licence.service.js') -const ProcessPreviousBillingTransactionsService = require('./process-previous-billing-transactions.service.js') - -/** - * Finds any replaced charge versions and creates any transactions required to cancel out the replaced transactions, - * plus any invoices they require, in both WRLS and the Charging Module API - * - * @param {module:BillingBatchModel} billingBatch The billing batch we need to process - * @param {Object} billingPeriod an object representing the financial year the transaction is for - * @returns {Boolean} Returns `true` if any transactions were generated, otherwise `false` - */ -async function go (billingBatch, billingPeriod) { - const currentBillingData = { - isEmpty: true, - licence: null, - billingInvoice: null, - billingInvoiceLicence: null - } - - const replacedChargeVersions = await _fetchReplacedChargeVersions(billingBatch, billingPeriod) - - for (const chargeVersion of replacedChargeVersions) { - const { billingInvoice, billingInvoiceLicence } = await _generateInvoiceData( - currentBillingData, - billingBatch, - chargeVersion, - billingPeriod - ) - - // We need to deal with the very first iteration when currentBillingData is all nulls! So, we check both there is - // a billingInvoiceLicence and that its ID is different - if ( - currentBillingData.billingInvoiceLicence && - currentBillingData.billingInvoiceLicence.billingInvoiceLicenceId !== billingInvoiceLicence.billingInvoiceLicenceId - ) { - await _finaliseCurrentInvoiceLicence(currentBillingData, billingPeriod, billingBatch) - } - - currentBillingData.licence = chargeVersion.licence - currentBillingData.billingInvoice = billingInvoice - currentBillingData.billingInvoiceLicence = billingInvoiceLicence - } - await _finaliseCurrentInvoiceLicence(currentBillingData, billingPeriod, billingBatch) - - // We return `true` if we generated any transactions and `false` if we didn't - return !currentBillingData.isEmpty -} - -async function _fetchReplacedChargeVersions (billingBatch, billingPeriod) { - const { billingBatchId, regionId } = billingBatch - - return await FetchReplacedChargeVersionsService.go(regionId, billingPeriod, billingBatchId) -} - -async function _createBillingInvoiceLicence (currentBillingData) { - const { billingInvoice, billingInvoiceLicence } = currentBillingData - - if (!billingInvoice.persisted) { - await BillingInvoiceModel.query().insert(billingInvoice) - billingInvoice.persisted = true - } - - await BillingInvoiceLicenceModel.query().insert(billingInvoiceLicence) -} - -async function _createBillingTransactions (currentBillingData, billingBatch, billingTransactions, billingPeriod) { - const { licence, billingInvoice, billingInvoiceLicence } = currentBillingData - - for (const transaction of billingTransactions) { - const chargingModuleRequest = ChargingModuleCreateTransactionPresenter.go( - transaction, - billingPeriod, - billingInvoice.invoiceAccountNumber, - licence - ) - - const chargingModuleResponse = await ChargingModuleCreateTransactionService.go(billingBatch.externalId, chargingModuleRequest) - - transaction.status = 'charge_created' - transaction.externalId = chargingModuleResponse.response.body.transaction.id - transaction.billingInvoiceLicenceId = billingInvoiceLicence.billingInvoiceLicenceId - - await CreateBillingTransactionService.go(transaction) - } -} - -/** - * Reverses any debit transactions from the previous billing batch, creates them in the db, and persists the billing - * invoice and billing invoice licence as needed - */ -async function _finaliseCurrentInvoiceLicence (currentBillingData, billingPeriod, billingBatch) { - // Guard clause which is most likely to hit in the event that no charge versions were 'fetched' to be billed in the - // first place - if (!currentBillingData.billingInvoice) { - return - } - - const reversingPreviousTransactions = await ProcessPreviousBillingTransactionsService.go( - currentBillingData.billingInvoice, - currentBillingData.billingInvoiceLicence, - billingPeriod - ) - - if (reversingPreviousTransactions.length > 0) { - currentBillingData.isEmpty = false - - await _createBillingTransactions(currentBillingData, billingBatch, reversingPreviousTransactions, billingPeriod) - await _createBillingInvoiceLicence(currentBillingData) - } -} - -async function _generateInvoiceData (currentBillingData, billingBatch, chargeVersion, billingPeriod) { - 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 - ) - - return { - billingInvoice, - billingInvoiceLicence - } -} - -module.exports = { - go -} diff --git a/test/services/supplementary-billing/process-replaced-charge-versions.service.test.js b/test/services/supplementary-billing/process-replaced-charge-versions.service.test.js deleted file mode 100644 index dc05c26f9d..0000000000 --- a/test/services/supplementary-billing/process-replaced-charge-versions.service.test.js +++ /dev/null @@ -1,143 +0,0 @@ -'use strict' - -// Test framework dependencies -const Lab = require('@hapi/lab') -const Code = require('@hapi/code') -const Sinon = require('sinon') - -const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() -const { expect } = Code - -// Test helpers -const BillingBatchHelper = require('../../support/helpers/water/billing-batch.helper.js') -const BillingInvoiceHelper = require('../../support/helpers/water/billing-invoice.helper.js') -const BillingInvoiceLicenceHelper = require('../../support/helpers/water/billing-invoice-licence.helper.js') -const BillingInvoiceModel = require('../../../app/models/water/billing-invoice.model.js') -const BillingTransactionHelper = require('../../support/helpers/water/billing-transaction.helper.js') -const ChargeVersionHelper = require('../../support/helpers/water/charge-version.helper.js') -const InvoiceAccountHelper = require('../../support/helpers/crm-v2/invoice-account.helper.js') -const LicenceHelper = require('../../support/helpers/water/licence.helper.js') -const DatabaseHelper = require('../../support/helpers/database.helper.js') -const RegionHelper = require('../../support/helpers/water/region.helper.js') - -// Things we need to stub -const ChargingModuleCreateTransactionService = require('../../../app/services/charging-module/create-transaction.service.js') -const FetchReplacedChargeVersionsService = require('../../../app/services/supplementary-billing/fetch-replaced-charge-versions.service.js') - -// Thing under test -const ProcessReplacedChargeVersionsService = require('../../../app/services/supplementary-billing/process-replaced-charge-versions.service.js') - -describe('Process replaced charge versions service', () => { - const billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } - - let licence - let billingBatch - - beforeEach(async () => { - await DatabaseHelper.clean() - - const region = await RegionHelper.add() - licence = await LicenceHelper.add({ includeInSrocSupplementaryBilling: true, regionId: region.regionId }) - billingBatch = await BillingBatchHelper.add({ regionId: region.regionId }) - }) - - afterEach(() => { - Sinon.restore() - }) - - describe('when the service is called', () => { - describe('and there are no replaced charge versions to process', () => { - beforeEach(() => { - Sinon.stub(FetchReplacedChargeVersionsService, 'go').resolves([]) - }) - - it('returns `false`', async () => { - const result = await ProcessReplacedChargeVersionsService.go(billingBatch, billingPeriod) - - expect(result).to.be.false() - }) - }) - - describe('and there are replaced charge versions to process', () => { - let supersededInvoiceAccount - - beforeEach(async () => { - Sinon.stub(ChargingModuleCreateTransactionService, 'go').resolves({ - succeeded: true, - response: { - body: { transaction: { id: '7e752fa6-a19c-4779-b28c-6e536f028795' } } - } - }) - - supersededInvoiceAccount = await InvoiceAccountHelper.add() - - await ChargeVersionHelper.add( - { - endDate: new Date('2022-04-30'), - status: 'superseded', - invoiceAccountId: supersededInvoiceAccount.invoiceAccountId - }, - licence - ) - - const supersededBillingInvoice = await BillingInvoiceHelper.add( - { invoiceAccountId: supersededInvoiceAccount.invoiceAccountId }, - { status: 'sent' } - ) - const supersededBillingInvoiceLicence = await BillingInvoiceLicenceHelper.add( - {}, - licence, - supersededBillingInvoice - ) - await BillingTransactionHelper.add({ - billingInvoiceLicenceId: supersededBillingInvoiceLicence.billingInvoiceLicenceId, - purposes: [] - }) - }) - - it('returns `true`', async () => { - const result = await ProcessReplacedChargeVersionsService.go(billingBatch, billingPeriod) - - expect(result).to.be.true() - }) - - it('creates a new billingInvoice record', async () => { - await ProcessReplacedChargeVersionsService.go(billingBatch, billingPeriod) - - const newBillingInvoice = await BillingInvoiceModel.query().where('billingBatchId', billingBatch.billingBatchId) - - expect(newBillingInvoice).to.have.length(1) - expect(newBillingInvoice[0].invoiceAccountId).to.equal(supersededInvoiceAccount.invoiceAccountId) - }) - - it('creates a new billingInvoiceLicence record', async () => { - await ProcessReplacedChargeVersionsService.go(billingBatch, billingPeriod) - - const billingInvoice = await BillingInvoiceModel.query() - .findOne('billingBatchId', billingBatch.billingBatchId) - .withGraphFetched('billingInvoiceLicences') - - expect(billingInvoice.billingInvoiceLicences).to.have.length(1) - expect(billingInvoice.billingInvoiceLicences[0].licenceId).to.equal(licence.licenceId) - }) - - it('creates a new billingTransaction record', async () => { - await ProcessReplacedChargeVersionsService.go(billingBatch, billingPeriod) - - const billingInvoice = await BillingInvoiceModel.query() - .findOne('billingBatchId', billingBatch.billingBatchId) - .withGraphFetched('billingInvoiceLicences.billingTransactions') - const { billingTransactions } = billingInvoice.billingInvoiceLicences[0] - - expect(billingTransactions).to.have.length(1) - expect(billingTransactions[0].isCredit).to.be.true() - expect(billingTransactions[0].status).to.equal('charge_created') - expect(billingTransactions[0].externalId).to.equal('7e752fa6-a19c-4779-b28c-6e536f028795') - expect(billingTransactions[0].billableDays).to.equal(BillingTransactionHelper.defaults().billableDays) - }) - }) - }) -})