Fix supp. billing not crediting old accounts

app/services/supplementary-billing/determine-charge-period.service.js

@@ -23,7 +23,7 @@ function go (chargeVersion, financialYearEnding) {
   const financialYearStartDate = new Date(financialYearEnding - 1, 3, 1)
   const financialYearEndDate = new Date(financialYearEnding, 2, 31)
 
-  if (_periodIsInvalid(chargeVersion, financialYearStartDate, financialYearEndDate)) {
+  if (_periodIsInvalid(chargeVersion, financialYearEndDate)) {
     throw new Error(`Charge version is outside billing period ${financialYearEnding}`)
   }
 
@@ -51,22 +51,26 @@ function go (chargeVersion, financialYearEnding) {
 }
 
 /**
- * Determine if the charge version starts after or ends before the billing period
+ * Determine if the charge version is valid for the billing period billing period
  *
- * We never expect a charge version outside the financial period but we do this just to ensure we never return
- * nonsense and all possible scenarios are covered in our tests 😁
+ * With having to support multi-year charging the only way a charge version could be invalid is when it's start date
+ * is after the billing period.
+ *
+ * Any that start before _might_ be valid. For example, assume the billing period is 2023-24
+ *
+ * - a charge version that starts on 2023-04-01 is valid
+ * - a charge version that starts on 2022-04-01 is valid (with no end date it applies to 2023-24)
+ * - a charge version that starts on 2022-04-01 and ends on 2022-06-01 is valid (the charge period will be determined
+ *   as outside the billing period. But we still need to process these charge versions in case they have any previous
+ *   transactions that need crediting)
  *
  * @param {Object} chargeVersion chargeVersion being billed
- * @param {Date} financialYearStartDate billing period (financial year) start date
  * @param {Date} financialYearEndDate billing period (financial year) end date
  *
  * @returns {boolean} true if invalid else false
  */
-function _periodIsInvalid (chargeVersion, financialYearStartDate, financialYearEndDate) {
-  const chargeVersionStartsAfterFinancialYear = chargeVersion.startDate > financialYearEndDate
-  const chargeVersionEndsBeforeFinancialYear = chargeVersion.endDate && chargeVersion.endDate < financialYearStartDate
-
-  return chargeVersionStartsAfterFinancialYear || chargeVersionEndsBeforeFinancialYear
+function _periodIsInvalid (chargeVersion, financialYearEndDate) {
+  return chargeVersion.startDate > financialYearEndDate
 }
 
 module.exports = {

app/services/supplementary-billing/fetch-charge-versions.service.js

@@ -11,22 +11,33 @@ const ChargeVersion = require('../../models/water/charge-version.model.js')
 const ChargeVersionWorkflow = require('../../models/water/charge-version-workflow.model.js')
 
 /**
- * Fetch all SROC charge versions linked to licences flagged for supplementary billing that are in the period being
- * billed
+ * Fetch all SROC charge versions to be processed as part of supplementary billing
+ *
+ * To be selected for billing charge versions must
+ *
+ * - be linked to a licence flagged for supplementary billing
+ * - be linked to a licence which is linked to the selected region
+ * - have the scheme 'sroc'
+ * - have a start date before the end of the billing period
+ * - not have a status of draft
+ * - not be linked to a licence in the workflow
+ *
+ * From this initial result we extract an array of unique licence IDs and then remove any that are non-chargeable (we
+ * need to know about them in order to unset the licence's supplementary billing flag).
  *
  * @param {String} regionId UUID of the region being billed that the licences must be linked to
  * @param {Object} billingPeriod Object with a `startDate` and `endDate` property representing the period being billed
  *
- * @returns {Object[]} An array of matching charge versions
+ * @returns {Object} Contains an array of unique licence IDs and array of charge versions to be processed
  */
 async function go (regionId, billingPeriod) {
-  const uncleansedChargeVersions = await _fetch(regionId, billingPeriod)
+  const allChargeVersions = await _fetch(regionId, billingPeriod)
 
-  return _extractLicencesAndCleanseChargeVersions(uncleansedChargeVersions)
+  return _extractLicenceIdsThenRemoveNonChargeableChargeVersions(allChargeVersions)
 }
 
 async function _fetch (regionId, billingPeriod) {
-  const uncleansedChargeVersions = await ChargeVersion.query()
+  const allChargeVersions = await ChargeVersion.query()
     .select([
       'chargeVersionId',
       'scheme',
@@ -40,10 +51,6 @@ async function _fetch (regionId, billingPeriod) {
     .where('regionId', regionId)
     .where('chargeVersions.scheme', 'sroc')
     .where('chargeVersions.startDate', '<=', billingPeriod.endDate)
-    .where(builder => {
-      builder.whereNull('chargeVersions.endDate')
-        .orWhere('chargeVersions.endDate', '>=', billingPeriod.startDate)
-    })
     .whereNot('chargeVersions.status', 'draft')
     .whereNotExists(
       ChargeVersionWorkflow.query()
@@ -112,22 +119,21 @@ async function _fetch (regionId, billingPeriod) {
       ])
     })
 
-  return uncleansedChargeVersions
+  return allChargeVersions
 }
 
 /**
- * Extract the `licence_id`s from the charge versions before removing charge versions that have no `invoice_account_id`
+ * Extract the `licenceId`s from all the charge versions before removing non-chargeable charge versions
  *
  * When a licence is made "non-chargeable" the supplementary billing flag gets set and a charge version created that has
- * no `invoice_account_id`. For the purpose of billing we are not interested in charge versions with no
- * `invoice_account_id` as these will not be charged. We are however interested in the associated licences to ensure
- * that "non-chargeable" licences have their supplementary billing flag un-set.
+ * no `invoice_account_id`. For the purpose of billing we are not interested in non-chargeable charge versions. We are
+ * interested in the associated licences to ensure that their supplementary billing flag is unset.
  */
-function _extractLicencesAndCleanseChargeVersions (uncleansedChargeVersions) {
+function _extractLicenceIdsThenRemoveNonChargeableChargeVersions (allChargeVersions) {
   const licenceIdsForPeriod = []
   const chargeVersions = []
 
-  for (const chargeVersion of uncleansedChargeVersions) {
+  for (const chargeVersion of allChargeVersions) {
     licenceIdsForPeriod.push(chargeVersion.licence.licenceId)
 
     if (chargeVersion.invoiceAccountId) {

test/services/supplementary-billing/fetch-charge-versions.service.test.js

@@ -39,8 +39,10 @@ describe('Fetch Charge Versions service', () => {
   describe('when there are charge versions that should be considered for the next supplementary billing', () => {
     let billingChargeCategory
     let chargeElement2023
+    let chargeElement2023And24
     let chargeElement2024
     let chargePurpose2023
+    let chargePurpose2023And24
     let chargePurpose2024
     let changeReason
     let licence
@@ -60,16 +62,21 @@ describe('Fetch Charge Versions service', () => {
       const { licenceId } = licence
       changeReason = await ChangeReasonHelper.add({ triggersMinimumCharge: true })
 
-      // This creates a 'current' SROC charge version valid only FYE 2024
+      // This creates a 'current' SROC charge version which covers only FYE 2024
       const sroc2024ChargeVersion = await ChargeVersionHelper.add(
         { startDate: new Date('2023-11-01'), changeReasonId: changeReason.changeReasonId, licenceId }
       )
 
-      // This creates a 'current' SROC charge version valid in both FYE 2023 and 2024
-      const sroc2023ChargeVersion = await ChargeVersionHelper.add(
+      // This creates a 'current' SROC charge version which covers both FYE 2023 and 2024
+      const sroc2023And24ChargeVersion = await ChargeVersionHelper.add(
         { endDate: new Date('2023-10-31'), changeReasonId: changeReason.changeReasonId, licenceId }
       )
 
+      // This creates a 'current' SROC charge version which covers only FYE 2023
+      const sroc2023ChargeVersion = await ChargeVersionHelper.add(
+        { endDate: new Date('2022-10-31'), changeReasonId: changeReason.changeReasonId, licenceId }
+      )
+
       // This creates a 'superseded' SROC charge version
       const srocSupersededChargeVersion = await ChargeVersionHelper.add(
         { changeReasonId: changeReason.changeReasonId, status: 'superseded', licenceId }
@@ -80,7 +87,13 @@ describe('Fetch Charge Versions service', () => {
         { scheme: 'alcs', licenceId }
       )
 
-      testRecords = [sroc2024ChargeVersion, sroc2023ChargeVersion, srocSupersededChargeVersion, alcsChargeVersion]
+      testRecords = [
+        sroc2024ChargeVersion,
+        sroc2023And24ChargeVersion,
+        sroc2023ChargeVersion,
+        srocSupersededChargeVersion,
+        alcsChargeVersion
+      ]
 
       // We test that related data is returned in the results. So, we create and link it to the srocChargeVersion
       // ready for testing
@@ -95,6 +108,15 @@ describe('Fetch Charge Versions service', () => {
         chargeElementId: chargeElement2024.chargeElementId
       })
 
+      chargeElement2023And24 = await ChargeElementHelper.add({
+        chargeVersionId: sroc2023And24ChargeVersion.chargeVersionId,
+        billingChargeCategoryId: billingChargeCategory.billingChargeCategoryId
+      })
+
+      chargePurpose2023And24 = await ChargePurposeHelper.add({
+        chargeElementId: chargeElement2023And24.chargeElementId
+      })
+
       chargeElement2023 = await ChargeElementHelper.add({
         chargeVersionId: sroc2023ChargeVersion.chargeVersionId,
         billingChargeCategoryId: billingChargeCategory.billingChargeCategoryId
@@ -113,29 +135,32 @@ describe('Fetch Charge Versions service', () => {
       it('returns the SROC charge versions that are applicable', async () => {
         const result = await FetchChargeVersionsService.go(regionId, billingPeriod)
 
-        expect(result.chargeVersions).to.have.length(3)
+        expect(result.chargeVersions).to.have.length(4)
         expect(result.chargeVersions[0].chargeVersionId).to.equal(testRecords[0].chargeVersionId)
         expect(result.chargeVersions[1].chargeVersionId).to.equal(testRecords[1].chargeVersionId)
         expect(result.chargeVersions[2].chargeVersionId).to.equal(testRecords[2].chargeVersionId)
+        expect(result.chargeVersions[3].chargeVersionId).to.equal(testRecords[3].chargeVersionId)
       })
 
       it('returns the licenceIds from SROC charge versions that are applicable', async () => {
         const result = await FetchChargeVersionsService.go(regionId, billingPeriod)
 
-        expect(result.licenceIdsForPeriod).to.have.length(3)
+        expect(result.licenceIdsForPeriod).to.have.length(4)
         expect(result.licenceIdsForPeriod[0]).to.equal(licence.licenceId)
         expect(result.licenceIdsForPeriod[1]).to.equal(licence.licenceId)
         expect(result.licenceIdsForPeriod[2]).to.equal(licence.licenceId)
+        expect(result.licenceIdsForPeriod[3]).to.equal(licence.licenceId)
       })
     })
 
     it("returns both 'current' and 'superseded' SROC charge versions that are applicable", async () => {
       const result = await FetchChargeVersionsService.go(regionId, billingPeriod)
 
-      expect(result.chargeVersions).to.have.length(3)
+      expect(result.chargeVersions).to.have.length(4)
       expect(result.chargeVersions[0].chargeVersionId).to.equal(testRecords[0].chargeVersionId)
       expect(result.chargeVersions[1].chargeVersionId).to.equal(testRecords[1].chargeVersionId)
       expect(result.chargeVersions[2].chargeVersionId).to.equal(testRecords[2].chargeVersionId)
+      expect(result.chargeVersions[3].chargeVersionId).to.equal(testRecords[3].chargeVersionId)
     })
 
     it('includes the related licence and region', async () => {
@@ -179,6 +204,27 @@ describe('Fetch Charge Versions service', () => {
         }]
       }
 
+      const expectedResult2023And24 = {
+        chargeElementId: chargeElement2023And24.chargeElementId,
+        source: chargeElement2023And24.source,
+        loss: chargeElement2023And24.loss,
+        volume: chargeElement2023And24.volume,
+        adjustments: chargeElement2023And24.adjustments,
+        additionalCharges: chargeElement2023And24.additionalCharges,
+        description: chargeElement2023And24.description,
+        billingChargeCategory: {
+          reference: billingChargeCategory.reference,
+          shortDescription: billingChargeCategory.shortDescription
+        },
+        chargePurposes: [{
+          chargePurposeId: chargePurpose2023And24.chargePurposeId,
+          abstractionPeriodStartDay: chargePurpose2023And24.abstractionPeriodStartDay,
+          abstractionPeriodStartMonth: chargePurpose2023And24.abstractionPeriodStartMonth,
+          abstractionPeriodEndDay: chargePurpose2023And24.abstractionPeriodEndDay,
+          abstractionPeriodEndMonth: chargePurpose2023And24.abstractionPeriodEndMonth
+        }]
+      }
+
       const expectedResult2023 = {
         chargeElementId: chargeElement2023.chargeElementId,
         source: chargeElement2023.source,
@@ -201,7 +247,8 @@ describe('Fetch Charge Versions service', () => {
       }
 
       expect(result.chargeVersions[0].chargeElements[0]).to.equal(expectedResult2024)
-      expect(result.chargeVersions[1].chargeElements[0]).to.equal(expectedResult2023)
+      expect(result.chargeVersions[1].chargeElements[0]).to.equal(expectedResult2023And24)
+      expect(result.chargeVersions[2].chargeElements[0]).to.equal(expectedResult2023)
     })
   })
 
@@ -306,61 +353,31 @@ describe('Fetch Charge Versions service', () => {
       })
     })
 
-    describe('because none of them are in the billing period', () => {
-      describe('as they all have end dates before the billing period', () => {
-        beforeEach(async () => {
-          billingPeriod = {
-            startDate: new Date('2023-04-01'),
-            endDate: new Date('2024-03-31')
-          }
-
-          const { licenceId } = await LicenceHelper.add({
-            regionId,
-            includeInSrocSupplementaryBilling: true
-          })
-
-          // This creates an SROC charge version with an end date before the billing period. This would have been
-          // picked up by a previous years bill run
-          const srocChargeVersion = await ChargeVersionHelper.add(
-            { startDate: new Date('2022-04-01'), endDate: new Date('2022-10-01'), licenceId }
-          )
-          testRecords = [srocChargeVersion]
-        })
-
-        it('returns no applicable licenceIds or charge versions', async () => {
-          const result = await FetchChargeVersionsService.go(regionId, billingPeriod)
+    describe('because 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.chargeVersions).to.be.empty()
-          expect(result.licenceIdsForPeriod).to.be.empty()
+        const { licenceId } = await LicenceHelper.add({
+          regionId,
+          includeInSrocSupplementaryBilling: true
         })
-      })
 
-      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')
-          }
-
-          const { licenceId } = await LicenceHelper.add({
-            regionId,
-            includeInSrocSupplementaryBilling: true
-          })
-
-          // This creates an SROC charge version with a start date after the billing period. This will be picked in
-          // next years bill runs
-          const srocChargeVersion = await ChargeVersionHelper.add(
-            { startDate: new Date('2023-04-01'), licenceId }
-          )
-          testRecords = [srocChargeVersion]
-        })
+        // This creates an SROC charge version with a start date after the billing period. This will be picked in
+        // next years bill runs
+        const srocChargeVersion = await ChargeVersionHelper.add(
+          { startDate: new Date('2023-04-01'), licenceId }
+        )
+        testRecords = [srocChargeVersion]
+      })
 
-        it('returns no applicable licenceIds or charge versions', async () => {
-          const result = await FetchChargeVersionsService.go(regionId, billingPeriod)
+      it('returns no applicable licenceIds or charge versions', async () => {
+        const result = await FetchChargeVersionsService.go(regionId, billingPeriod)
 
-          expect(result.chargeVersions).to.be.empty()
-          expect(result.licenceIdsForPeriod).to.be.empty()
-        })
+        expect(result.chargeVersions).to.be.empty()
+        expect(result.licenceIdsForPeriod).to.be.empty()
       })
     })