Skip to content

Commit

Permalink
Remove redundant fetch for region code in service (#866)
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-4375

Spotted whilst working on [Exclude ended charge versions from 2PT billing](#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.
  • Loading branch information
Cruikshanks authored Mar 28, 2024
1 parent af943d3 commit ec70854
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand All @@ -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<Object>} Contains an array of SROC charge versions with linked licences, charge references, charge elements and related purpose
* @returns {Promise<Object>} 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',
Expand All @@ -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')
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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
})

Expand All @@ -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',
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand Down Expand Up @@ -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({
Expand Down

0 comments on commit ec70854

Please sign in to comment.