Refactor Supplementary output to test presenter (#32)

https://eaflood.atlassian.net/browse/WATER-3787 https://eaflood.atlassian.net/browse/WATER-3802 https://eaflood.atlassian.net/browse/WATER-3826 We're trying to make it easy to test that we are selecting the right values from the DB when generating the SROC supplementary bill run. Our test endpoint currently lists the charge versions, which includes the licence reference. In [Add new SROC Billing Period service](#30) we add the financial years. But it would be easier to validate the licences that have been selected if the response included a unique list of them rather than having to search through the charge version results. This gives us a prime opportunity to split out the presentation logic from what the service is doing, especially as it is likely to be discarded when we are happy the process is working. So, in this change, we move formatting the response to a new 'presenter'. As part of the change, we'll add a unique list of the Licences being used. Example response following this change ```json { "billingPeriods": [ { "startDate": "2022-04-01T00:00:00.000Z", "endDate": "2023-03-31T00:00:00.000Z" } ], "licences": [ { "licenceId": "2627a306-23a3-432f-9c71-a71663888285", "licenceRef": "AT/SROC/SUPB/01" } ], "chargeVersions": [ { "chargeVersionId": "4b5cbe04-a0e2-468c-909e-1e2d93810ba8", "licenceRef": "AT/SROC/SUPB/01", "licenceId": "2627a306-23a3-432f-9c71-a71663888285", "scheme": "sroc", "endDate": null }, { "chargeVersionId": "732fde85-fd3b-44e8-811f-8e6f4eb8cf6f", "licenceRef": "AT/SROC/SUPB/01", "licenceId": "2627a306-23a3-432f-9c71-a71663888285", "scheme": "sroc", "endDate": null } ] } ```
DEFRA · Nov 23, 2022 · 895b071 · 895b071
1 parent 6894ea0
commit 895b071
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 12 deletions.
diff --git a/app/presenters/supplementary.presenter.js b/app/presenters/supplementary.presenter.js
@@ -0,0 +1,72 @@
+'use strict'
+
+/**
+ * @module SupplementaryPresenter
+ */
+
+class SupplementaryPresenter {
+  constructor (data) {
+    this._data = data
+  }
+
+  go () {
+    return this._presentation(this._data)
+  }
+
+  /**
+   * Creates an array of unique licence objects from the combined data received
+   *
+   * We know the main query in `SupplementaryService` is based on charge versions. As a licence may be linked to
+   * multiple charge versions, it is possible that the licence ID and reference will feature multiple times in the
+   * results.
+   *
+   * To make it easier to confirm we are including the right licences in the Supplementary bill run process we want to
+   * extract a unique list of licence ID's and references.
+   *
+   * @param {Object[]} chargeVersions Results of a call to SupplementaryService and the charge version info it returns
+   *
+   * @returns {Object[]} Array of objects representing the unique licence details in the charge versions passed in
+   */
+  _licences (chargeVersions) {
+    // Use map to generate an array of just licence IDs
+    const justLicenceIds = chargeVersions.map((chargeVersion) => chargeVersion.licenceId)
+    // Set is used to store unique values. So, if you what you pass in contains duplicates Set will return just the
+    // unique ones
+    const uniqueLicenceIds = new Set(justLicenceIds)
+
+    const licences = []
+    for (const id of uniqueLicenceIds) {
+      // Iterate through our unique Licence Id's and use them to find the index for the first matching entry in the
+      // charge versions data
+      const index = chargeVersions.findIndex((chargeVersion) => chargeVersion.licenceId === id)
+      // Destructure the licenceId and licenceRef from the matching object
+      const { licenceId, licenceRef } = chargeVersions[index]
+
+      // Push a new object into our array of licences
+      licences.push({ licenceId, licenceRef })
+    }
+
+    return licences
+  }
+
+  _presentation (data) {
+    const licences = this._licences(data.chargeVersions)
+    const chargeVersions = data.chargeVersions.map((chargeVersion) => {
+      return {
+        chargeVersionId: chargeVersion.chargeVersionId,
+        licenceRef: chargeVersion.licenceRef,
+        licenceId: chargeVersion.licenceId,
+        scheme: chargeVersion.scheme,
+        endDate: chargeVersion.endDate
+      }
+    })
+
+    return {
+      billingPeriods: data.billingPeriods,
+      licences,
+      chargeVersions
+    }
+  }
+}
+
+module.exports = SupplementaryPresenter
diff --git a/app/services/supplementary_billing/fetch_charge_versions.service.js b/app/services/supplementary_billing/fetch_charge_versions.service.js
@@ -2,7 +2,6 @@
 
 /**
  * @module FetchChargeVersionsService
- *
  */
 
 const { db } = require('../../../db/db')
@@ -16,16 +15,16 @@ class FetchChargeVersionsService {
 
   static async _fetch (regionId) {
     const chargeVersions = db
-      .select('chargeVersionId', 'licences.licenceRef')
-      .from('water.charge_versions')
-      .innerJoin('water.licences', 'charge_versions.licence_id', 'licences.licence_id')
+      .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({
-        'licences.include_in_supplementary_billing': 'yes',
-        'licences.region_id': regionId
+        'lic.include_in_supplementary_billing': 'yes',
+        'lic.region_id': regionId
       })
 
     return chargeVersions

diff --git a/app/services/supplementary_billing/supplementary.service.js b/app/services/supplementary_billing/supplementary.service.js
@@ -4,16 +4,22 @@
  * @module SupplementaryService
  */
 
-const FetchChargeVersionsService = require('./fetch_charge_versions.service.js')
 const BillingPeriodService = require('./billing_period.service')
+const FetchChargeVersionsService = require('./fetch_charge_versions.service.js')
+const SupplementaryPresenter = require('../../presenters/supplementary.presenter.js')
 
 class SupplementaryService {
   static async go (regionId) {
     const chargeVersions = await FetchChargeVersionsService.go(regionId)
-    const financialYears = BillingPeriodService.go()
-    const response = { chargeVersions, financialYears }
+    const billingPeriods = BillingPeriodService.go()
+
+    return this._response(chargeVersions, billingPeriods)
+  }
+
+  static _response (chargeVersions, billingPeriods) {
+    const presenter = new SupplementaryPresenter({ chargeVersions, billingPeriods })
 
-    return response
+    return presenter.go()
   }
 }
 

diff --git a/test/presenters/supplementary.presenter.test.js b/test/presenters/supplementary.presenter.test.js
@@ -0,0 +1,76 @@
+'use strict'
+
+// Test framework dependencies
+const Lab = require('@hapi/lab')
+const Code = require('@hapi/code')
+
+const { describe, it, beforeEach } = exports.lab = Lab.script()
+const { expect } = Code
+
+// Thing under test
+const SupplementaryPresenter = require('../../app/presenters/supplementary.presenter.js')
+
+describe('Supplementary presenter', () => {
+  let data
+
+  describe('when there are results', () => {
+    beforeEach(() => {
+      data = {
+        billingPeriods: [
+          { startDate: new Date(2022, 3, 1), endDate: new Date(2023, 2, 31) }
+        ],
+        chargeVersions: [
+          {
+            chargeVersionId: '4b5cbe04-a0e2-468c-909e-1e2d93810ba8',
+            scheme: 'sroc',
+            endDate: null,
+            licenceRef: 'AT/SROC/SUPB/01',
+            licenceId: '0000579f-0f8f-4e21-b63a-063384ad32c8'
+          },
+          {
+            chargeVersionId: '732fde85-fd3b-44e8-811f-8e6f4eb8cf6f',
+            scheme: 'sroc',
+            endDate: null,
+            licenceRef: 'AT/SROC/SUPB/01',
+            licenceId: '0000579f-0f8f-4e21-b63a-063384ad32c8'
+          }
+        ]
+      }
+    })
+
+    it('correctly presents the data', () => {
+      const presenter = new SupplementaryPresenter(data)
+      const result = presenter.go()
+
+      expect(result.billingPeriods).to.have.length(1)
+      expect(result.billingPeriods[0]).to.equal(data.billingPeriods[0])
+
+      expect(result.licences).to.have.length(1)
+      expect(result.licences[0]).to.equal({
+        licenceId: data.chargeVersions[0].licenceId,
+        licenceRef: data.chargeVersions[0].licenceRef
+      })
+
+      expect(result.chargeVersions).to.have.length(2)
+      expect(result.chargeVersions[0]).to.equal(data.chargeVersions[0])
+    })
+  })
+
+  describe('when there are no results', () => {
+    beforeEach(() => {
+      data = {
+        billingPeriods: [],
+        chargeVersions: []
+      }
+    })
+
+    it('correctly presents the data', () => {
+      const presenter = new SupplementaryPresenter(data)
+      const result = presenter.go()
+
+      expect(result.billingPeriods).to.be.empty()
+      expect(result.licences).to.be.empty()
+      expect(result.chargeVersions).to.be.empty()
+    })
+  })
+})
diff --git a/test/services/supplementary_billing/supplementary.service.test.js b/test/services/supplementary_billing/supplementary.service.test.js
@@ -20,7 +20,13 @@ describe('Supplementary service', () => {
   })
 
   describe('when there are licences to be included in supplementary billing', () => {
-    const testRecords = [{ charge_version_id: '878bc903-836d-4549-83f5-4e20ccf87d2f' }]
+    const testRecords = [{
+      chargeVersionId: '4b5cbe04-a0e2-468c-909e-1e2d93810ba8',
+      scheme: 'sroc',
+      endDate: null,
+      licenceId: '2627a306-23a3-432f-9c71-a71663888285',
+      licenceRef: 'AT/SROC/SUPB/01'
+    }]
 
     beforeEach(async () => {
       Sinon.stub(FetchChargeVersionsService, 'go').resolves(testRecords)
@@ -30,7 +36,7 @@ describe('Supplementary service', () => {
       const result = await SupplementaryService.go('regionId')
 
       expect(result.chargeVersions.length).to.equal(1)
-      expect(result.chargeVersions[0].charge_version_id).to.equal(testRecords[0].charge_version_id)
+      expect(result.chargeVersions[0].chargeVersionId).to.equal(testRecords[0].chargeVersionId)
     })
   })