From c2f296aa73e620c4556081f6dcfc4c4a222d1313 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 3 Apr 2024 22:30:39 +0100 Subject: [PATCH 1/6] Fix send bill run unflagging licences https://eaflood.atlassian.net/browse/WATER-4390 In a recent change we [Migrated legacy send bill run functionality](https://github.com/DEFRA/water-abstraction-system/pull/771) to our service. We though all was well but then recently spotted an issue. When we are 'sending' a bill run (telling the CHA to generate the transaction file for SOP that results in the bills being sent to customers) we are calling our `SubmitSendBillRunService` which in turn is calling `UnflagBilledLicencesService`. Only it shouldn't be doing this _all_ the time. It should only be calling that service if the bill run is supplementary. This change fixes the issue. From cee9de186079bcef75fcdba326909c779f8cf1dd Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 3 Apr 2024 22:36:49 +0100 Subject: [PATCH 2/6] Housekeeping - correct type in comment --- .../bill-runs/supplementary/unflag-billed-licences.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/bill-runs/supplementary/unflag-billed-licences.service.js b/app/services/bill-runs/supplementary/unflag-billed-licences.service.js index 7475a4a696..cd98a70248 100644 --- a/app/services/bill-runs/supplementary/unflag-billed-licences.service.js +++ b/app/services/bill-runs/supplementary/unflag-billed-licences.service.js @@ -2,7 +2,7 @@ /** * Unflag all licences in a bill run that resulted in a billing invoice (they are billed) - * @module UnflagUnbilledLicencesService + * @module UnflagBilledLicencesService */ const LicenceModel = require('../../../models/licence.model.js') From 61845b35d0a4240f21f92ea2a3da027d3b08cd5e Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 3 Apr 2024 22:42:09 +0100 Subject: [PATCH 3/6] Housekeeping - correct test file name for service --- ...end.service.test.js => submit-send-bill-run.service.test.js} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename test/services/bill-runs/{submit-send.service.test.js => submit-send-bill-run.service.test.js} (99%) diff --git a/test/services/bill-runs/submit-send.service.test.js b/test/services/bill-runs/submit-send-bill-run.service.test.js similarity index 99% rename from test/services/bill-runs/submit-send.service.test.js rename to test/services/bill-runs/submit-send-bill-run.service.test.js index 15c93b5bb5..8f078fa16f 100644 --- a/test/services/bill-runs/submit-send.service.test.js +++ b/test/services/bill-runs/submit-send-bill-run.service.test.js @@ -23,7 +23,7 @@ const ChargingModuleViewBillRunRequest = require('../../../app/requests/charging // Thing under test const SubmitSendBillBunService = require('../../../app/services/bill-runs/submit-send-bill-run.service.js') -describe('Submit Cancel Bill Run service', () => { +describe('Submit Send Bill Run service', () => { // NOTE: introducing a delay in the tests is not ideal. But the service is written such that the send happens in the // background and is not awaited. We want to confirm things like the records have been updated. But the only way to do // so is to give the background process time to complete. From 4ebb4b81b24cfdc85730948e3cfd79ad2d7e24d7 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 3 Apr 2024 22:59:50 +0100 Subject: [PATCH 4/6] Add failing unit test --- .../submit-send-bill-run.service.test.js | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/services/bill-runs/submit-send-bill-run.service.test.js b/test/services/bill-runs/submit-send-bill-run.service.test.js index 8f078fa16f..075d4cc93f 100644 --- a/test/services/bill-runs/submit-send-bill-run.service.test.js +++ b/test/services/bill-runs/submit-send-bill-run.service.test.js @@ -19,6 +19,7 @@ const ExpandedError = require('../../../app/errors/expanded.error.js') // Things we need to stub const ChargingModuleSendBillRunRequest = require('../../../app/requests/charging-module/send-bill-run.request.js') const ChargingModuleViewBillRunRequest = require('../../../app/requests/charging-module/view-bill-run.request.js') +const UnflagBilledLicencesService = require('../../../app/services/bill-runs/supplementary/unflag-billed-licences.service.js') // Thing under test const SubmitSendBillBunService = require('../../../app/services/bill-runs/submit-send-bill-run.service.js') @@ -32,12 +33,14 @@ describe('Submit Send Bill Run service', () => { let chargingModuleSendBillRunRequestStub let chargingModuleViewBillRunRequestStub let notifierStub + let unflagBilledLicencesServiceStub beforeEach(async () => { await DatabaseSupport.clean() chargingModuleSendBillRunRequestStub = Sinon.stub(ChargingModuleSendBillRunRequest, 'send').resolves() chargingModuleViewBillRunRequestStub = Sinon.stub(ChargingModuleViewBillRunRequest, 'send') + unflagBilledLicencesServiceStub = Sinon.stub(UnflagBilledLicencesService, 'go').resolves() // The service depends on GlobalNotifier to have been set. This happens in app/plugins/global-notifier.plugin.js // when the app starts up and the plugin is registered. As we're not creating an instance of Hapi server in this @@ -123,6 +126,34 @@ describe('Submit Send Bill Run service', () => { expect(logDataArg.timeTakenSs).to.exist() expect(logDataArg.billRunId).to.exist() }) + + describe("when the bill run's batch type is 'supplementary'", () => { + it('removes the SROC supplementary billing flag from those licences billed', async () => { + await SubmitSendBillBunService.go(billRun.id) + + await setTimeout(delay) + + expect(unflagBilledLicencesServiceStub.called).to.be.true() + }) + }) + + describe("when the bill run's batch type is not 'supplementary'", () => { + let annualBillRun + + beforeEach(async () => { + annualBillRun = await BillRunHelper.add({ + batchType: 'annual', externalId: '76ed78bd-c104-4ad7-8842-4b660df02331', status: 'ready' + }) + }) + + it('leaves the SROC supplementary billing flag for those licences billed', async () => { + await SubmitSendBillBunService.go(annualBillRun.id) + + await setTimeout(delay) + + expect(unflagBilledLicencesServiceStub.called).to.be.false() + }) + }) }) describe('but the request to view the Charging Module bill run fails', () => { From 3ca3738d1c33a61a5b0116fe46170be79eee51a5 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 3 Apr 2024 23:02:57 +0100 Subject: [PATCH 5/6] Implement fix --- app/services/bill-runs/submit-send-bill-run.service.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/services/bill-runs/submit-send-bill-run.service.js b/app/services/bill-runs/submit-send-bill-run.service.js index e2ddaefa09..b43fd855e9 100644 --- a/app/services/bill-runs/submit-send-bill-run.service.js +++ b/app/services/bill-runs/submit-send-bill-run.service.js @@ -63,6 +63,7 @@ async function _fetchBillRun (id) { .findById(id) .select([ 'id', + 'batchType', 'createdAt', 'externalId', 'regionId', @@ -94,7 +95,9 @@ async function _sendBillRun (billRun) { await _updateBillRunData(billRun, externalBillRun) - await UnflagBilledLicencesService.go(billRun) + if (billRun.batchType === 'supplementary') { + await UnflagBilledLicencesService.go(billRun) + } calculateAndLogTimeTaken(startTime, 'Send bill run complete', { billRunId }) } From b84eba8f8bd1393c39423f7c075ee12f1e6db4e7 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 3 Apr 2024 23:03:30 +0100 Subject: [PATCH 6/6] Housekeeping - fix service name in comment --- app/services/bill-runs/submit-send-bill-run.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/bill-runs/submit-send-bill-run.service.js b/app/services/bill-runs/submit-send-bill-run.service.js index b43fd855e9..eec39d6fed 100644 --- a/app/services/bill-runs/submit-send-bill-run.service.js +++ b/app/services/bill-runs/submit-send-bill-run.service.js @@ -2,7 +2,7 @@ /** * Orchestrates the sending of a bill run - * @module SubmitCancelBillRunService + * @module SubmitSendBillRunService */ const BillModel = require('../../models/bill.model.js')