From ec70854876ef4be9bc7e288ed80fae905105bb7c Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 28 Mar 2024 08:23:29 +0000 Subject: [PATCH] Remove redundant fetch for region code in service (#866) https://eaflood.atlassian.net/browse/WATER-4375 Spotted whilst working on [Exclude ended charge versions from 2PT billing](https://github.com/DEFRA/water-abstraction-system/pull/865) that we pass in the region ID to `FetchChargeVersionsService` but then do an additional query to identify the NALD region code for that region. We then use the NALD region code to filter the charge versions when we could use the region ID directly. This change removes the redundant fetch. --- .../fetch-charge-versions.service.js | 24 +++++++------------ .../fetch-charge-versions.service.test.js | 22 ++++++++--------- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/app/services/bill-runs/two-part-tariff/fetch-charge-versions.service.js b/app/services/bill-runs/two-part-tariff/fetch-charge-versions.service.js index 4ebd988a0f..8153b17784 100644 --- a/app/services/bill-runs/two-part-tariff/fetch-charge-versions.service.js +++ b/app/services/bill-runs/two-part-tariff/fetch-charge-versions.service.js @@ -9,11 +9,10 @@ const { ref } = require('objection') const ChargeReferenceModel = require('../../../models/charge-reference.model.js') const ChargeVersionModel = require('../../../models/charge-version.model.js') -const RegionModel = require('../../../models/region.model.js') const Workflow = require('../../../models/workflow.model.js') /** - * Fetches SROC charge versions based on region and billing period + * Fetches two-part tariff charge versions for the region and billing period being billed * * To be selected for billing charge versions must * @@ -26,18 +25,17 @@ const Workflow = require('../../../models/workflow.model.js') * - have a status of current * - be linked to a charge reference that is marked as two-part-tariff * - * @param {String} regionId UUID of the region being billed that the charge version must have - * @param {Object} billingPeriod Object with a `startDate` and `endDate` property representing the period being billed + * @param {String} regionId - UUID of the region being billed + * @param {Object} billingPeriod - Object with a `startDate` and `endDate` property representing the period being billed * - * @returns {Promise} Contains an array of SROC charge versions with linked licences, charge references, charge elements and related purpose + * @returns {Promise} Contains an array of two-part tariff charge versions with linked licences, charge + * references, charge elements and related purpose */ async function go (regionId, billingPeriod) { - const regionCode = await _regionCode(regionId) - - return _fetch(regionCode, billingPeriod) + return _fetch(regionId, billingPeriod) } -async function _fetch (regionCode, billingPeriod) { +async function _fetch (regionId, billingPeriod) { const chargeVersions = await ChargeVersionModel.query() .select([ 'chargeVersions.id', @@ -46,10 +44,10 @@ async function _fetch (regionCode, billingPeriod) { 'chargeVersions.status' ]) .innerJoinRelated('licence') + .where('licence.regionId', regionId) .where('chargeVersions.scheme', 'sroc') .where('chargeVersions.startDate', '<=', billingPeriod.endDate) .where('chargeVersions.status', 'current') - .where('chargeVersions.regionCode', regionCode) .where((builder) => { builder .whereNull('chargeVersions.endDate') @@ -153,12 +151,6 @@ async function _fetch (regionCode, billingPeriod) { return chargeVersions } -async function _regionCode (regionId) { - const { naldRegionId } = await RegionModel.query().findById(regionId).select('naldRegionId') - - return naldRegionId -} - module.exports = { go } diff --git a/test/services/bill-runs/two-part-tariff/fetch-charge-versions.service.test.js b/test/services/bill-runs/two-part-tariff/fetch-charge-versions.service.test.js index 96760debed..34dd7fd9f6 100644 --- a/test/services/bill-runs/two-part-tariff/fetch-charge-versions.service.test.js +++ b/test/services/bill-runs/two-part-tariff/fetch-charge-versions.service.test.js @@ -29,7 +29,6 @@ describe('Fetch Charge Versions service', () => { startDate: new Date('2023-04-01'), endDate: new Date('2024-03-31') } - const regionCode = 5 const licenceId = 'cee9ff5f-813a-49c7-ba04-c65cfecf67dd' const licenceRef = '01/128' @@ -42,7 +41,7 @@ describe('Fetch Charge Versions service', () => { const chargeCategory = await ChargeCategoryHelper.add({ reference: '4.3.41' }) chargeCategoryId = chargeCategory.id - const region = await RegionHelper.add({ naldRegionId: regionCode }) + const region = await RegionHelper.add() regionId = region.id }) @@ -57,7 +56,7 @@ describe('Fetch Charge Versions service', () => { // NOTE: The first part of the setup creates a charge version we will test exactly matches what we expect. The // second part is to create another charge version with a different licence ref so we can test the order of the // results - await ChargeVersionHelper.add({ id: chargeVersionId, licenceId, licenceRef, regionCode, changeReasonId }) + await ChargeVersionHelper.add({ id: chargeVersionId, licenceId, licenceRef, changeReasonId }) const { id: chargeReferenceId } = await ChargeReferenceHelper.add({ id: 'a86837fa-cf25-42fe-8216-ea8c2d2c939d', @@ -86,7 +85,7 @@ describe('Fetch Charge Versions service', () => { // Second charge version to test ordering const otherLicence = await LicenceHelper.add({ licenceRef: '01/130', regionId }) const chargeVersion = await ChargeVersionHelper.add( - { licenceId: otherLicence.id, licenceRef: '01/130', regionCode, changeReasonId } + { licenceId: otherLicence.id, licenceRef: '01/130', changeReasonId } ) const chargeReference = await ChargeReferenceHelper.add({ chargeVersionId: chargeVersion.id, @@ -205,7 +204,7 @@ describe('Fetch Charge Versions service', () => { describe("because the scheme is 'presroc'", () => { beforeEach(async () => { const { id: chargeVersionId } = await ChargeVersionHelper.add( - { scheme: 'alcs', licenceId, licenceRef, regionCode: 5 } + { scheme: 'alcs', licenceId, licenceRef } ) await ChargeReferenceHelper.add({ @@ -225,7 +224,7 @@ describe('Fetch Charge Versions service', () => { describe('because the start date is after the billing period ends', () => { beforeEach(async () => { const { id: chargeVersionId } = await ChargeVersionHelper.add( - { startDate: new Date('2024-04-01'), licenceId, licenceRef, regionCode } + { startDate: new Date('2024-04-01'), licenceId, licenceRef } ) await ChargeReferenceHelper.add({ @@ -245,7 +244,7 @@ describe('Fetch Charge Versions service', () => { describe('because the end date is before the billing period starts', () => { beforeEach(async () => { const { id: chargeVersionId } = await ChargeVersionHelper.add( - { endDate: new Date('2023-03-31'), licenceId, licenceRef, regionCode } + { endDate: new Date('2023-03-31'), licenceId, licenceRef } ) await ChargeReferenceHelper.add({ @@ -265,7 +264,7 @@ describe('Fetch Charge Versions service', () => { describe("because the status is not 'current'", () => { beforeEach(async () => { const { id: chargeVersionId } = await ChargeVersionHelper.add( - { licenceId, licenceRef, regionCode, status: 'superseded' } + { licenceId, licenceRef, status: 'superseded' } ) await ChargeReferenceHelper.add({ @@ -284,8 +283,9 @@ describe('Fetch Charge Versions service', () => { describe('because the region is different', () => { beforeEach(async () => { + const { id: otherLicenceId, licenceRef: otherLicenceRef } = await LicenceHelper.add({ regionId: 'eee44502-dd6f-4b13-8885-ebc50a7f54dc' }) const { id: chargeVersionId } = await ChargeVersionHelper.add( - { licenceId, licenceRef, regionCode: 9 } + { licenceId: otherLicenceId, licenceRef: otherLicenceRef } ) await ChargeReferenceHelper.add({ @@ -305,7 +305,7 @@ describe('Fetch Charge Versions service', () => { describe('because the licence is linked to a workflow', () => { beforeEach(async () => { const { id: chargeVersionId } = await ChargeVersionHelper.add( - { licenceId, licenceRef, regionCode } + { licenceId, licenceRef } ) await ChargeReferenceHelper.add({ @@ -334,7 +334,7 @@ describe('Fetch Charge Versions service', () => { .where('id', licenceId) const { id: chargeVersionId } = await ChargeVersionHelper.add( - { licenceId, licenceRef, regionCode } + { licenceId, licenceRef } ) await ChargeReferenceHelper.add({