diff --git a/.env.example b/.env.example index 1f53595d93..9d19ab843a 100644 --- a/.env.example +++ b/.env.example @@ -50,3 +50,6 @@ AWS_REGION=eu-west-1 AWS_ACCESS_KEY_ID=uploadaccesskey AWS_SECRET_ACCESS_KEY=uploadsecretkey AWS_MAINTENANCE_BUCKET=upload-bucket-gov-uk + +# Feature flags +ENABLE_REISSUING_BILLING_BATCHES=false diff --git a/app/services/supplementary-billing/process-billing-batch.service.js b/app/services/supplementary-billing/process-billing-batch.service.js index aaa8d5fed1..2c53c07bf4 100644 --- a/app/services/supplementary-billing/process-billing-batch.service.js +++ b/app/services/supplementary-billing/process-billing-batch.service.js @@ -1,5 +1,10 @@ 'use strict' +/** + * Process a given billing batch for the given billing periods + * @module ProcessBillingBatchService + */ + const BillingBatchError = require('../../errors/billing-batch.error.js') const BillingBatchModel = require('../../models/water/billing-batch.model.js') const ChargingModuleGenerateService = require('../charging-module/generate-bill-run.service.js') @@ -7,38 +12,32 @@ const FetchChargeVersionsService = require('./fetch-charge-versions.service.js') const HandleErroredBillingBatchService = require('./handle-errored-billing-batch.service.js') const LegacyRequestLib = require('../../lib/legacy-request.lib.js') const ProcessBillingPeriodService = require('./process-billing-period.service.js') +const ReissueInvoicesService = require('./reissue-invoices.service.js') const UnflagUnbilledLicencesService = require('./unflag-unbilled-licences.service.js') +const FeatureFlagsConfig = require('../../../config/feature-flags.config.js') + +/** + * Process a given billing batch for the given billing periods. In this case, "process" means that we create the + * required invoices and transactions for it in both this service and the Charging Module. + * + * TODO: flesh out these docs + * + * @param {module:BillingBatchModel} billingBatch + * @param {Object[]} billingPeriods An array of billing periods each containing a `startDate` and `endDate` + */ async function go (billingBatch, billingPeriods) { const { billingBatchId } = billingBatch try { - // Mark the start time for later logging const startTime = process.hrtime.bigint() - const accumulatedLicenceIds = [] - - let isBatchPopulated = false await _updateStatus(billingBatchId, 'processing') - for (const billingPeriod of billingPeriods) { - const { chargeVersions, licenceIdsForPeriod } = await _fetchChargeVersions(billingBatch, billingPeriod) - const isPeriodPopulated = await ProcessBillingPeriodService.go(billingBatch, billingPeriod, chargeVersions) + const resultOfReissuing = await _reissueInvoices(billingBatch) - accumulatedLicenceIds.push(...licenceIdsForPeriod) + await _processBillingPeriods(billingPeriods, billingBatch, billingBatchId, resultOfReissuing) - if (isPeriodPopulated) { - isBatchPopulated = true - } - } - - // Creating a new set from accumulatedLicenceIds gives us just the unique ids. Objection does not accept sets in - // .findByIds() so we spread it into an array - const allLicenceIds = [...new Set(accumulatedLicenceIds)] - - await _finaliseBillingBatch(billingBatch, allLicenceIds, isBatchPopulated) - - // Log how long the process took _calculateAndLogTime(billingBatchId, startTime) } catch (error) { await HandleErroredBillingBatchService.go(billingBatchId, error.code) @@ -46,6 +45,40 @@ async function go (billingBatch, billingPeriods) { } } +async function _processBillingPeriods (billingPeriods, billingBatch, billingBatchId, resultOfReissuing) { + const accumulatedLicenceIds = [] + + // We use `results` to check if any db changes have been made (which is indicated by a billing period being processed + // and returning `true`). We populate it with the result of reissuing as this also indicates whether db changes have + // been made. + const results = [resultOfReissuing] + + for (const billingPeriod of billingPeriods) { + const { chargeVersions, licenceIdsForPeriod } = await _fetchChargeVersions(billingBatch, billingPeriod) + const isPeriodPopulated = await ProcessBillingPeriodService.go(billingBatch, billingPeriod, chargeVersions) + + accumulatedLicenceIds.push(...licenceIdsForPeriod) + results.push(isPeriodPopulated) + } + + await _finaliseBillingBatch(billingBatch, accumulatedLicenceIds, results) +} + +/** + * Call `ReissueInvoicesService` and log the time taken. We return `true` if any invoices have been reissued (which will + * have resulted in db changes), otherwise we return `false` (to indicate there have been no db changes) + */ +async function _reissueInvoices (billingBatch) { + // If reissuing isn't enabled then simply return `false` to indicate no db change has been made + if (!FeatureFlagsConfig.enableReissuingBillingBatches) { + return false + } + + const result = await ReissueInvoicesService.go(billingBatch) + + return result +} + /** * Log the time taken to process the billing batch * @@ -81,16 +114,24 @@ async function _fetchChargeVersions (billingBatch, billingPeriod) { * process, and refreshes the billing batch locally. However if there were no resulting invoice licences then we simply * unflag the unbilled licences and mark the billing batch with `empty` status */ -async function _finaliseBillingBatch (billingBatch, allLicenceIds, isPopulated) { +async function _finaliseBillingBatch (billingBatch, accumulatedLicenceIds, resultsOfProcessing) { + // Creating a new set from accumulatedLicenceIds gives us just the unique ids. Objection does not accept sets in + // .findByIds() so we spread it into an array + const allLicenceIds = [...new Set(accumulatedLicenceIds)] + await UnflagUnbilledLicencesService.go(billingBatch.billingBatchId, allLicenceIds) + // We set `isPopulated` to `true` if at least one processing result was truthy + const isPopulated = resultsOfProcessing.some(result => result) + // If there are no billing invoice licences then the bill run is considered empty. We just need to set the status to // indicate this in the UI if (!isPopulated) { - return _updateStatus(billingBatch.billingBatchId, 'empty') + await _updateStatus(billingBatch.billingBatchId, 'empty') + return } - // We then need to tell the Charging Module to run its generate process. This is where the Charging module finalises + // We now need to tell the Charging Module to run its generate process. This is where the Charging module finalises // the debit and credit amounts, and adds any additional transactions needed, for example, minimum charge await ChargingModuleGenerateService.go(billingBatch.externalId) diff --git a/app/services/supplementary-billing/unflag-unbilled-licences.service.js b/app/services/supplementary-billing/unflag-unbilled-licences.service.js index 50c4fea426..1ec8fd83bb 100644 --- a/app/services/supplementary-billing/unflag-unbilled-licences.service.js +++ b/app/services/supplementary-billing/unflag-unbilled-licences.service.js @@ -26,12 +26,12 @@ const LicenceModel = require('../../models/water/licence.model.js') * The query we run during the **SEND** process unflags only those which are linked to the bill run being sent. We can * do this because we know this service has handled anything that was unbilled and not represented. * - * @param {*} billingBatchId The ID of the bill run (billing batch) being processed + * @param {String} billingBatchId The ID of the bill run (billing batch) being processed * @param {String[]} allLicenceIds All licence IDs being processed in the bill run * @returns {Number} count of records updated */ async function go (billingBatchId, allLicenceIds) { - return LicenceModel.query() + const result = await LicenceModel.query() .patch({ includeInSrocSupplementaryBilling: false }) .whereIn('licenceId', allLicenceIds) .whereNotExists( @@ -39,6 +39,8 @@ async function go (billingBatchId, allLicenceIds) { .join('billingInvoices', 'billingInvoices.billingInvoiceId', '=', 'billingInvoiceLicences.billingInvoiceId') .where('billingInvoices.billingBatchId', '=', billingBatchId) ) + + return result } module.exports = { diff --git a/config/feature-flags.config.js b/config/feature-flags.config.js new file mode 100644 index 0000000000..5d0439c087 --- /dev/null +++ b/config/feature-flags.config.js @@ -0,0 +1,18 @@ +'use strict' + +/** + * Config values used to enable feature flags + * @module FeatureFlagsConfig + */ + +// We require dotenv directly in each config file to support unit tests that depend on this this subset of config. +// Requiring dotenv in multiple places has no effect on the app when running for real. +require('dotenv').config() + +const config = { + // Credit to https://stackoverflow.com/a/323546/6117745 for how to handle + // converting the env var to a boolean + enableReissuingBillingBatches: (String(process.env.ENABLE_REISSUING_BILLING_BATCHES) === 'true') || false +} + +module.exports = config 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 b5bdd032ce..439bacd8ac 100644 --- a/test/services/supplementary-billing/process-billing-batch.service.test.js +++ b/test/services/supplementary-billing/process-billing-batch.service.test.js @@ -16,10 +16,12 @@ const DatabaseHelper = require('../../support/helpers/database.helper.js') // Things we need to stub const ChargingModuleGenerateService = require('../../../app/services/charging-module/generate-bill-run.service.js') +const FeatureFlagsConfig = require('../../../config/feature-flags.config.js') const FetchChargeVersionsService = require('../../../app/services/supplementary-billing/fetch-charge-versions.service.js') const HandleErroredBillingBatchService = require('../../../app/services/supplementary-billing/handle-errored-billing-batch.service.js') const LegacyRequestLib = require('../../../app/lib/legacy-request.lib.js') const ProcessBillingPeriodService = require('../../../app/services/supplementary-billing/process-billing-period.service.js') +const ReissueInvoicesService = require('../../../app/services/supplementary-billing/reissue-invoices.service.js') const UnflagUnbilledLicencesService = require('../../../app/services/supplementary-billing/unflag-unbilled-licences.service.js') // Thing under test @@ -51,6 +53,9 @@ describe('Process billing batch service', () => { // test we recreate the condition by setting it directly with our own stub notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } global.GlobalNotifier = notifierStub + + // We set the `enableReissuingBillingBatches` feature flag to `true` to ensure that we always perform reissuing + Sinon.replace(FeatureFlagsConfig, 'enableReissuingBillingBatches', true) }) afterEach(() => { @@ -69,18 +74,39 @@ describe('Process billing batch service', () => { Sinon.stub(ProcessBillingPeriodService, 'go').resolves(false) }) - it('sets the Billing Batch status to empty', async () => { - await ProcessBillingBatchService.go(billingBatch, billingPeriods) + describe('and there are no invoices to reissue', () => { + beforeEach(() => { + Sinon.stub(ReissueInvoicesService, 'go').resolves(false) + }) - const result = await BillingBatchModel.query().findById(billingBatch.billingBatchId) + it('sets the Billing Batch status to empty', async () => { + await ProcessBillingBatchService.go(billingBatch, billingPeriods) - expect(result.status).to.equal('empty') + const result = await BillingBatchModel.query().findById(billingBatch.billingBatchId) + + expect(result.status).to.equal('empty') + }) + }) + + describe('and there are invoices to reissue', () => { + beforeEach(() => { + Sinon.stub(ReissueInvoicesService, 'go').resolves(true) + }) + + it('sets the Billing Batch status to processing', async () => { + await ProcessBillingBatchService.go(billingBatch, billingPeriods) + + const result = await BillingBatchModel.query().findById(billingBatch.billingBatchId) + + expect(result.status).to.equal('processing') + }) }) }) describe('and some charge versions are billed', () => { beforeEach(() => { Sinon.stub(ProcessBillingPeriodService, 'go').resolves(true) + Sinon.stub(ReissueInvoicesService, 'go').resolves(true) }) it('sets the Billing Batch status to processing', async () => { @@ -102,22 +128,26 @@ describe('Process billing batch service', () => { expect(legacyRequestLibStub.called).to.be.true() }) - }) - it('logs the time taken to process the billing batch', async () => { - await ProcessBillingBatchService.go(billingBatch, billingPeriods) + it('it logs the time taken', async () => { + await ProcessBillingBatchService.go(billingBatch, billingPeriods) - const args = notifierStub.omg.firstCall.args + const args = notifierStub.omg.firstCall.args - expect(args[0]).to.equal('Process billing batch complete') - expect(args[1].timeTakenMs).to.exist() - expect(args[1].billingBatchId).to.equal(billingBatch.billingBatchId) + expect(args[0]).to.equal('Process billing batch complete') + expect(args[1].timeTakenMs).to.exist() + expect(args[1].billingBatchId).to.equal(billingBatch.billingBatchId) + }) }) }) describe('when the service errors', () => { let thrownError + beforeEach(() => { + Sinon.stub(ReissueInvoicesService, 'go') + }) + describe('because fetching the charge versions fails', () => { beforeEach(() => { thrownError = new Error('ERROR')