From 6c2c500e53c3b66283d59d897bb589103b21e048 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Mon, 5 Dec 2022 17:59:30 +0000 Subject: [PATCH 1/7] Amend the supplementary charge version query https://eaflood.atlassian.net/browse/WATER-3826 The query as we have it is to simplistic. We need to review all charge versions within a given financial year to accurately calculate the bill run. We can't just look at the 'current' charge version. So, this change updates the query to consider the creation and effective dates for the charge versions linked to a licence. From 06234148207d13de406ad7af5498afd80541f355 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 6 Dec 2022 08:49:28 +0000 Subject: [PATCH 2/7] Add migration for start date Start date becomes a consideration in our query so we need to add it to the table. --- .../20221206084612_alter_charge_versions.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 db/migrations/20221206084612_alter_charge_versions.js diff --git a/db/migrations/20221206084612_alter_charge_versions.js b/db/migrations/20221206084612_alter_charge_versions.js new file mode 100644 index 0000000000..560c58c5d7 --- /dev/null +++ b/db/migrations/20221206084612_alter_charge_versions.js @@ -0,0 +1,21 @@ +'use strict' + +const tableName = 'charge_versions' + +exports.up = async function (knex) { + await knex + .schema + .withSchema('water') + .alterTable(tableName, table => { + table.date('start_date') + }) +} + +exports.down = function (knex) { + return knex + .schema + .withSchema('water') + .alterTable(tableName, table => { + table.dropColumns('start_date') + }) +} From 453cb624f7f93aa880374c29e26abf1062be0069 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 6 Dec 2022 09:36:14 +0000 Subject: [PATCH 3/7] Implement change to query We now pass through the billing period in order to use it in our query. We've not yet updated the tests (other than to ensure we pass through a billing period) in order to highlight as expected they fail. --- .../fetch-charge-versions.service.js | 11 +++--- .../supplementary.service.js | 2 +- .../fetch-charge-versions.service.test.js | 36 ++++++++++++++++--- test/support/helpers/charge-version.helper.js | 3 +- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/app/services/supplementary-billing/fetch-charge-versions.service.js b/app/services/supplementary-billing/fetch-charge-versions.service.js index 4d64e48eff..2391afefcb 100644 --- a/app/services/supplementary-billing/fetch-charge-versions.service.js +++ b/app/services/supplementary-billing/fetch-charge-versions.service.js @@ -8,25 +8,24 @@ const { db } = require('../../../db/db.js') class FetchChargeVersionsService { - static async go (regionId) { - const chargeVersions = await this._fetch(regionId) + static async go (regionId, billingPeriod) { + const chargeVersions = await this._fetch(regionId, billingPeriod) return chargeVersions } - static async _fetch (regionId) { + static async _fetch (regionId, billingPeriod) { const chargeVersions = db .select('chv.chargeVersionId', 'chv.scheme', 'chv.endDate', 'lic.licenceId', 'lic.licenceRef') .from({ chv: 'water.charge_versions' }) .innerJoin({ lic: 'water.licences' }, 'chv.licence_id', 'lic.licence_id') .where({ scheme: 'sroc', - end_date: null - }) - .andWhere({ 'lic.include_in_supplementary_billing': 'yes', 'lic.region_id': regionId }) + .andWhere('start_date', '>=', billingPeriod.startDate) + .andWhere('start_date', '<=', billingPeriod.endDate) return chargeVersions } diff --git a/app/services/supplementary-billing/supplementary.service.js b/app/services/supplementary-billing/supplementary.service.js index ca0ca73db5..ea2f093ccb 100644 --- a/app/services/supplementary-billing/supplementary.service.js +++ b/app/services/supplementary-billing/supplementary.service.js @@ -16,7 +16,7 @@ class SupplementaryService { const region = await FetchRegionService.go(naldRegionId) const billingPeriods = BillingPeriodService.go() const licences = await FetchLicencesService.go(region) - const chargeVersions = await FetchChargeVersionsService.go(region.regionId) + const chargeVersions = await FetchChargeVersionsService.go(region.regionId, billingPeriods[0]) return this._response({ billingPeriods, licences, chargeVersions }) } 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 1d7426d5bf..ce673b842a 100644 --- a/test/services/supplementary-billing/fetch-charge-versions.service.test.js +++ b/test/services/supplementary-billing/fetch-charge-versions.service.test.js @@ -18,6 +18,7 @@ const FetchChargeVersionsService = require('../../../app/services/supplementary- describe('Fetch Charge Versions service', () => { const { region_id: regionId } = LicenceHelper.defaults() let testRecords + let billingPeriod beforeEach(async () => { await DatabaseHelper.clean() @@ -25,6 +26,11 @@ describe('Fetch Charge Versions service', () => { 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( {}, @@ -41,7 +47,7 @@ describe('Fetch Charge Versions service', () => { }) it('returns only the current SROC charge versions that are applicable', async () => { - const result = await FetchChargeVersionsService.go(regionId) + const result = await FetchChargeVersionsService.go(regionId, billingPeriod) expect(result.length).to.equal(1) expect(result[0].charge_version_id).to.equal(testRecords[0].charge_version_id) @@ -51,6 +57,11 @@ describe('Fetch Charge Versions service', () => { describe('when there are no licences to be included in supplementary billing', () => { describe("because none of them are marked 'include_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. But the licence won't be marked for supplementary // billing const srocChargeVersion = await ChargeVersionHelper.add() @@ -58,7 +69,7 @@ describe('Fetch Charge Versions service', () => { }) it('returns no applicable charge versions', async () => { - const result = await FetchChargeVersionsService.go(regionId) + const result = await FetchChargeVersionsService.go(regionId, billingPeriod) expect(result.length).to.equal(0) }) @@ -66,6 +77,11 @@ describe('Fetch Charge Versions service', () => { 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 alcsChargeVersion = await ChargeVersionHelper.add( { scheme: 'alcs' }, @@ -75,7 +91,7 @@ describe('Fetch Charge Versions service', () => { }) it('returns no applicable charge versions', async () => { - const result = await FetchChargeVersionsService.go(regionId) + const result = await FetchChargeVersionsService.go(regionId, billingPeriod) expect(result.length).to.equal(0) }) @@ -83,6 +99,11 @@ describe('Fetch Charge Versions service', () => { describe('because there are no current charge versions (they all have end dates)', () => { beforeEach(async () => { + billingPeriod = { + startDate: new Date('2022-04-01'), + endDate: new Date('2023-03-31') + } + // This creates an SROC charge version with an end date linked to a licence marked for supplementary billing const alcsChargeVersion = await ChargeVersionHelper.add( { end_date: new Date(2022, 2, 1) }, // 2022-03-01 - Months are zero indexed :-) @@ -92,7 +113,7 @@ describe('Fetch Charge Versions service', () => { }) it('returns no applicable charge versions', async () => { - const result = await FetchChargeVersionsService.go(regionId) + const result = await FetchChargeVersionsService.go(regionId, billingPeriod) expect(result.length).to.equal(0) }) @@ -100,6 +121,11 @@ describe('Fetch Charge Versions service', () => { 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 otherRegionChargeVersion = await ChargeVersionHelper.add( {}, @@ -112,7 +138,7 @@ describe('Fetch Charge Versions service', () => { }) it('returns no applicable charge versions', async () => { - const result = await FetchChargeVersionsService.go(regionId) + const result = await FetchChargeVersionsService.go(regionId, billingPeriod) expect(result.length).to.equal(0) }) diff --git a/test/support/helpers/charge-version.helper.js b/test/support/helpers/charge-version.helper.js index f8537ab2b2..0ae64ebd21 100644 --- a/test/support/helpers/charge-version.helper.js +++ b/test/support/helpers/charge-version.helper.js @@ -49,7 +49,8 @@ class ChargeVersionHelper { static defaults (data = {}) { const defaults = { licence_ref: '01/123', - scheme: 'sroc' + scheme: 'sroc', + start_date: new Date('2022-04-01') } return { From e0187b1ae10dcbc260254017a2beda8ff154bd5b Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 6 Dec 2022 09:47:55 +0000 Subject: [PATCH 4/7] Update tests to match expectations --- .../fetch-charge-versions.service.test.js | 54 +++++++++++++------ 1 file changed, 39 insertions(+), 15 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 ce673b842a..f61a8ff71b 100644 --- a/test/services/supplementary-billing/fetch-charge-versions.service.test.js +++ b/test/services/supplementary-billing/fetch-charge-versions.service.test.js @@ -97,25 +97,49 @@ describe('Fetch Charge Versions service', () => { }) }) - describe('because there are no current charge versions (they all have end dates)', () => { - beforeEach(async () => { - billingPeriod = { - startDate: new Date('2022-04-01'), - endDate: new Date('2023-03-31') - } + describe('because there are no current charge versions', () => { + 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 an end date linked to a licence marked for supplementary billing - const alcsChargeVersion = await ChargeVersionHelper.add( - { end_date: new Date(2022, 2, 1) }, // 2022-03-01 - Months are zero indexed :-) - { include_in_supplementary_billing: 'yes' } - ) - testRecords = [alcsChargeVersion] + // This creates an SROC charge version with an end date linked to a licence marked for supplementary billing + const alcsChargeVersion = await ChargeVersionHelper.add( + { start_date: new Date(2022, 2, 31) }, // 2022-03-01 - Months are zero indexed :-) + { include_in_supplementary_billing: 'yes' } + ) + testRecords = [alcsChargeVersion] + }) + + it('returns no applicable charge versions', async () => { + const result = await FetchChargeVersionsService.go(regionId, billingPeriod) + + expect(result.length).to.equal(0) + }) }) - it('returns no applicable charge versions', async () => { - const result = await FetchChargeVersionsService.go(regionId, billingPeriod) + 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') + } - expect(result.length).to.equal(0) + // This creates an SROC charge version with an end date linked to a licence marked for supplementary billing + const alcsChargeVersion = await ChargeVersionHelper.add( + { start_date: new Date(2023, 3, 1) }, // 2023-04-01 - Months are zero indexed :-) + { include_in_supplementary_billing: 'yes' } + ) + testRecords = [alcsChargeVersion] + }) + + it('returns no applicable charge versions', async () => { + const result = await FetchChargeVersionsService.go(regionId, billingPeriod) + + expect(result.length).to.equal(0) + }) }) }) From 4844204ff0932946a32063a592db50477b80d1d5 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 6 Dec 2022 11:04:40 +0000 Subject: [PATCH 5/7] Fix comments --- .../fetch-charge-versions.service.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 f61a8ff71b..b5d7fd6a5f 100644 --- a/test/services/supplementary-billing/fetch-charge-versions.service.test.js +++ b/test/services/supplementary-billing/fetch-charge-versions.service.test.js @@ -105,7 +105,8 @@ describe('Fetch Charge Versions service', () => { endDate: new Date('2023-03-31') } - // This creates an SROC charge version with an end date linked to a licence marked for supplementary billing + // 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 alcsChargeVersion = await ChargeVersionHelper.add( { start_date: new Date(2022, 2, 31) }, // 2022-03-01 - Months are zero indexed :-) { include_in_supplementary_billing: 'yes' } @@ -127,7 +128,8 @@ describe('Fetch Charge Versions service', () => { endDate: new Date('2023-03-31') } - // This creates an SROC charge version with an end date linked to a licence marked for supplementary billing + // This creates an SROC charge version with a start date after the billing period. This will be picked in + // next years bill runs const alcsChargeVersion = await ChargeVersionHelper.add( { start_date: new Date(2023, 3, 1) }, // 2023-04-01 - Months are zero indexed :-) { include_in_supplementary_billing: 'yes' } From df6cbd44beed6b36ab1ddff01d37138a51557b02 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 6 Dec 2022 17:07:13 +0000 Subject: [PATCH 6/7] Add comments Without the context some of the changes look 'odd'! So, we've added some comments to help make sense of things. --- .../fetch-charge-versions.service.js | 12 ++++++++++++ .../supplementary-billing/supplementary.service.js | 9 +++++++++ 2 files changed, 21 insertions(+) diff --git a/app/services/supplementary-billing/fetch-charge-versions.service.js b/app/services/supplementary-billing/fetch-charge-versions.service.js index 2391afefcb..ebe7df995b 100644 --- a/app/services/supplementary-billing/fetch-charge-versions.service.js +++ b/app/services/supplementary-billing/fetch-charge-versions.service.js @@ -8,6 +8,18 @@ const { db } = require('../../../db/db.js') class FetchChargeVersionsService { + /** + * Fetch all 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 + * + * @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 + */ static async go (regionId, billingPeriod) { const chargeVersions = await this._fetch(regionId, billingPeriod) diff --git a/app/services/supplementary-billing/supplementary.service.js b/app/services/supplementary-billing/supplementary.service.js index ea2f093ccb..5aba11a7bf 100644 --- a/app/services/supplementary-billing/supplementary.service.js +++ b/app/services/supplementary-billing/supplementary.service.js @@ -11,11 +11,20 @@ const FetchLicencesService = require('./fetch-licences.service.js') const FetchRegionService = require('./fetch-region.service.js') const SupplementaryPresenter = require('../../presenters/supplementary.presenter.js') +/** + * WIP: This is currently being used to generate testing data to confirm we are understanding SROC supplementary + * billing. We intend to refactor things so that the service starts representing what is actually required. + */ class SupplementaryService { static async go (naldRegionId) { const region = await FetchRegionService.go(naldRegionId) const billingPeriods = BillingPeriodService.go() const licences = await FetchLicencesService.go(region) + + // We know in the future we will be calculating multiple billing periods and so will have to iterate through each, + // generating bill runs and reviewing if there is anything to bill. For now, whilst our knowledge of the process + // is low we are focusing on just the current financial year, and intending to ship a working version for just it. + // This is why we are only passing through the first billing period; we know there is only one! const chargeVersions = await FetchChargeVersionsService.go(region.regionId, billingPeriods[0]) return this._response({ billingPeriods, licences, chargeVersions }) From bf8c8d088a29e17f7aac77a2fae0af06f45781f6 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 6 Dec 2022 17:12:07 +0000 Subject: [PATCH 7/7] Add start date to charge version output Now it's a critical factor in our query we should be outputting the start date to make it easier to confirm the results are correct. --- app/presenters/supplementary.presenter.js | 1 + .../supplementary-billing/fetch-charge-versions.service.js | 2 +- test/presenters/supplementary.presenter.test.js | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/presenters/supplementary.presenter.js b/app/presenters/supplementary.presenter.js index 675e85af34..e318099a4e 100644 --- a/app/presenters/supplementary.presenter.js +++ b/app/presenters/supplementary.presenter.js @@ -27,6 +27,7 @@ class SupplementaryPresenter { licenceRef: chargeVersion.licenceRef, licenceId: chargeVersion.licenceId, scheme: chargeVersion.scheme, + startDate: chargeVersion.startDate, endDate: chargeVersion.endDate } }) diff --git a/app/services/supplementary-billing/fetch-charge-versions.service.js b/app/services/supplementary-billing/fetch-charge-versions.service.js index ebe7df995b..5dc0d8616d 100644 --- a/app/services/supplementary-billing/fetch-charge-versions.service.js +++ b/app/services/supplementary-billing/fetch-charge-versions.service.js @@ -28,7 +28,7 @@ class FetchChargeVersionsService { static async _fetch (regionId, billingPeriod) { const chargeVersions = db - .select('chv.chargeVersionId', 'chv.scheme', 'chv.endDate', 'lic.licenceId', 'lic.licenceRef') + .select('chv.chargeVersionId', 'chv.scheme', 'chv.startDate', 'chv.endDate', 'lic.licenceId', 'lic.licenceRef') .from({ chv: 'water.charge_versions' }) .innerJoin({ lic: 'water.licences' }, 'chv.licence_id', 'lic.licence_id') .where({ diff --git a/test/presenters/supplementary.presenter.test.js b/test/presenters/supplementary.presenter.test.js index 991a0aa20c..dc5dcfb55c 100644 --- a/test/presenters/supplementary.presenter.test.js +++ b/test/presenters/supplementary.presenter.test.js @@ -26,6 +26,7 @@ describe('Supplementary presenter', () => { { chargeVersionId: '4b5cbe04-a0e2-468c-909e-1e2d93810ba8', scheme: 'sroc', + startDate: new Date('2022-04-01'), endDate: null, licenceRef: 'AT/SROC/SUPB/01', licenceId: '0000579f-0f8f-4e21-b63a-063384ad32c8' @@ -33,6 +34,7 @@ describe('Supplementary presenter', () => { { chargeVersionId: '732fde85-fd3b-44e8-811f-8e6f4eb8cf6f', scheme: 'sroc', + startDate: new Date('2022-04-01'), endDate: null, licenceRef: 'AT/SROC/SUPB/01', licenceId: '0000579f-0f8f-4e21-b63a-063384ad32c8'