Update points page to use point id as the value (#1016)

https://eaflood.atlassian.net/browse/WATER-4295 As part of our work to get the requirements for returns set up we need to know which of the points the user has selected in the manual journey so that we can then get the information later at the check your answers page. This PR changes the logic to store the ID rather than the string which describes the point. that way we can look it up again.
DEFRA · May 15, 2024 · d2bced8 · d2bced8
1 parent d4f0c50
commit d2bced8
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 78 deletions.
diff --git a/app/lib/general.lib.js b/app/lib/general.lib.js
@@ -98,6 +98,43 @@ function determineCurrentFinancialYear () {
   return { startDate: new Date(startYear, 3, 1), endDate: new Date(endYear, 2, 31) }
 }
 
+/**
+ * Generate a string that represents an abstraction point
+ *
+ * When abstracting water the point at which this is done can be described in several ways depending on the number of
+ * Nation Grid References are saved against the abstraction point. This function checks for these references and builds
+ * a string that defines the details of the abstraction point.
+ *
+ * @param {Object} pointDetail - Object containing all the details for the point
+ *
+ * @returns {String} a description of the abstraction point
+ */
+function generateAbstractionPointDetail (pointDetail) {
+  let abstractionPoint = null
+
+  if (pointDetail.NGR4_SHEET && pointDetail.NGR4_NORTH !== 'null') {
+    const point1 = `${pointDetail.NGR1_SHEET} ${pointDetail.NGR1_EAST} ${pointDetail.NGR1_NORTH}`
+    const point2 = `${pointDetail.NGR2_SHEET} ${pointDetail.NGR2_EAST} ${pointDetail.NGR2_NORTH}`
+    const point3 = `${pointDetail.NGR3_SHEET} ${pointDetail.NGR3_EAST} ${pointDetail.NGR3_NORTH}`
+    const point4 = `${pointDetail.NGR4_SHEET} ${pointDetail.NGR4_EAST} ${pointDetail.NGR4_NORTH}`
+
+    abstractionPoint = `Within the area formed by the straight lines running between National Grid References ${point1} ${point2} ${point3} and ${point4}`
+  } else if (pointDetail.NGR2_SHEET && pointDetail.NGR2_NORTH !== 'null') {
+    const point1 = `${pointDetail.NGR1_SHEET} ${pointDetail.NGR1_EAST} ${pointDetail.NGR1_NORTH}`
+    const point2 = `${pointDetail.NGR2_SHEET} ${pointDetail.NGR2_EAST} ${pointDetail.NGR2_NORTH}`
+
+    abstractionPoint = `Between National Grid References ${point1} and ${point2}`
+  } else {
+    const point1 = `${pointDetail.NGR1_SHEET} ${pointDetail.NGR1_EAST} ${pointDetail.NGR1_NORTH}`
+
+    abstractionPoint = `At National Grid Reference ${point1}`
+  }
+
+  abstractionPoint += pointDetail.LOCAL_NAME !== undefined ? ` (${pointDetail.LOCAL_NAME})` : ''
+
+  return abstractionPoint
+}
+
 /**
  * Generate a Universally Unique Identifier (UUID)
  *
@@ -239,6 +276,7 @@ module.exports = {
   calculateAndLogTimeTaken,
   currentTimeInNanoseconds,
   determineCurrentFinancialYear,
+  generateAbstractionPointDetail,
   generateUUID,
   periodsOverlap,
   timestampForPostgres,

diff --git a/app/presenters/licences/view-licence-summary.presenter.js b/app/presenters/licences/view-licence-summary.presenter.js
@@ -6,6 +6,7 @@
  */
 
 const { formatLongDate, formatAbstractionDate } = require('../base.presenter')
+const { generateAbstractionPointDetail } = require('../../lib/general.lib.js')
 
 /**
  * Formats data for the `/licences/{id}/summary` page's summary tab
@@ -110,32 +111,6 @@ function _endDate (expiredDate) {
   return formatLongDate(expiredDate)
 }
 
-function _generateAbstractionContent (pointDetail) {
-  let abstractionPoint = null
-
-  if (pointDetail.NGR4_SHEET && pointDetail.NGR4_NORTH !== 'null') {
-    const point1 = `${pointDetail.NGR1_SHEET} ${pointDetail.NGR1_EAST} ${pointDetail.NGR1_NORTH}`
-    const point2 = `${pointDetail.NGR2_SHEET} ${pointDetail.NGR2_EAST} ${pointDetail.NGR2_NORTH}`
-    const point3 = `${pointDetail.NGR3_SHEET} ${pointDetail.NGR3_EAST} ${pointDetail.NGR3_NORTH}`
-    const point4 = `${pointDetail.NGR4_SHEET} ${pointDetail.NGR4_EAST} ${pointDetail.NGR4_NORTH}`
-
-    abstractionPoint = `Within the area formed by the straight lines running between National Grid References ${point1} ${point2} ${point3} and ${point4}`
-  } else if (pointDetail.NGR2_SHEET && pointDetail.NGR2_NORTH !== 'null') {
-    const point1 = `${pointDetail.NGR1_SHEET} ${pointDetail.NGR1_EAST} ${pointDetail.NGR1_NORTH}`
-    const point2 = `${pointDetail.NGR2_SHEET} ${pointDetail.NGR2_EAST} ${pointDetail.NGR2_NORTH}`
-
-    abstractionPoint = `Between National Grid References ${point1} and ${point2}`
-  } else {
-    const point1 = `${pointDetail.NGR1_SHEET} ${pointDetail.NGR1_EAST} ${pointDetail.NGR1_NORTH}`
-
-    abstractionPoint = `At National Grid Reference ${point1}`
-  }
-
-  abstractionPoint += pointDetail.LOCAL_NAME !== undefined ? ` (${pointDetail.LOCAL_NAME})` : ''
-
-  return abstractionPoint
-}
-
 function _generateAbstractionPeriods (licenceVersions) {
   if (!licenceVersions ||
     licenceVersions.length === 0 ||
@@ -222,7 +197,7 @@ function _parseAbstractionsAndSourceOfSupply (permitLicence) {
     purpose.purposePoints.forEach((point) => {
       const pointDetail = point.point_detail
       if (pointDetail) {
-        abstractionPoints.push(_generateAbstractionContent(pointDetail))
+        abstractionPoints.push(generateAbstractionPointDetail(pointDetail))
       }
     })
     abstractionQuantities = _setAbstractionAmountDetails(abstractionQuantities, purpose)

diff --git a/app/presenters/return-requirements/points.presenter.js b/app/presenters/return-requirements/points.presenter.js
@@ -5,6 +5,8 @@
  * @module PointsPresenter
 */
 
+const { generateAbstractionPointDetail } = require('../../lib/general.lib.js')
+
 /**
  * Formats data for the `/return-requirements/{sessionId}/points` page
  *
@@ -38,32 +40,6 @@ function _backLink (session, requirementIndex) {
   return `/system/return-requirements/${id}/purpose/${requirementIndex}`
 }
 
-function _generateAbstractionContent (pointDetails) {
-  let abstractionPoints = null
-
-  if (pointDetails.NGR4_SHEET && pointDetails.NGR4_NORTH !== 'null') {
-    const point1 = `${pointDetails.NGR1_SHEET} ${pointDetails.NGR1_EAST} ${pointDetails.NGR1_NORTH}`
-    const point2 = `${pointDetails.NGR2_SHEET} ${pointDetails.NGR2_EAST} ${pointDetails.NGR2_NORTH}`
-    const point3 = `${pointDetails.NGR3_SHEET} ${pointDetails.NGR3_EAST} ${pointDetails.NGR3_NORTH}`
-    const point4 = `${pointDetails.NGR4_SHEET} ${pointDetails.NGR4_EAST} ${pointDetails.NGR4_NORTH}`
-
-    abstractionPoints = `Within the area formed by the straight lines running between National Grid References ${point1} ${point2} ${point3} and ${point4}`
-  } else if (pointDetails.NGR2_SHEET && pointDetails.NGR2_NORTH !== 'null') {
-    const point1 = `${pointDetails.NGR1_SHEET} ${pointDetails.NGR1_EAST} ${pointDetails.NGR1_NORTH}`
-    const point2 = `${pointDetails.NGR2_SHEET} ${pointDetails.NGR2_EAST} ${pointDetails.NGR2_NORTH}`
-
-    abstractionPoints = `Between National Grid References ${point1} and ${point2}`
-  } else {
-    const point1 = `${pointDetails.NGR1_SHEET} ${pointDetails.NGR1_EAST} ${pointDetails.NGR1_NORTH}`
-
-    abstractionPoints = `At National Grid Reference ${point1}`
-  }
-
-  abstractionPoints += pointDetails.LOCAL_NAME !== undefined ? ` (${pointDetails.LOCAL_NAME})` : ''
-
-  return abstractionPoints
-}
-
 function _licencePoints (pointsData) {
   const abstractionPoints = []
 
@@ -72,12 +48,23 @@ function _licencePoints (pointsData) {
   }
 
   pointsData.forEach((pointDetail) => {
-    abstractionPoints.push(_generateAbstractionContent(pointDetail))
+    const point = {
+      id: pointDetail.ID,
+      description: generateAbstractionPointDetail(pointDetail)
+    }
+
+    if (_pointNotInArray(abstractionPoints, point)) {
+      abstractionPoints.push(point)
+    }
   })
 
-  const uniqueAbstractionPoints = [...new Set(abstractionPoints)]
+  return abstractionPoints
+}
 
-  return uniqueAbstractionPoints
+function _pointNotInArray (abstractionPointsArray, abstractionPoint) {
+  return !abstractionPointsArray.some((point) => {
+    return point.id === abstractionPoint.id
+  })
 }
 
 module.exports = {

diff --git a/app/views/return-requirements/points.njk b/app/views/return-requirements/points.njk
@@ -42,9 +42,9 @@
       {% for point in licencePoints %}
 
         {% set checkBoxItem = {
-          value: point,
-          text: point,
-          checked: point in points
+          value: point.id,
+          text: point.description,
+          checked: point.id in points
         } %}
 
         {% set checkBoxItems = (checkBoxItems.push(checkBoxItem), checkBoxItems) %}

diff --git a/test/presenters/return-requirements/points.presenter.test.js b/test/presenters/return-requirements/points.presenter.test.js
@@ -44,11 +44,16 @@ describe('Return Requirements - Points presenter', () => {
       expect(result).to.equal({
         backLink: '/system/return-requirements/61e07498-f309-4829-96a9-72084a54996d/purpose/0',
         licenceId: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d',
-        licencePoints: [
-          'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)',
-          'Between National Grid References SO 524 692 and SO 531 689 (KIRKENEL FARM ASHFORD CARBONEL - RIVER TEME)',
-          'Within the area formed by the straight lines running between National Grid References NZ 892 055 NZ 895 054 NZ 893 053 and NZ 892 053 (AREA D)'
-        ],
+        licencePoints: [{
+          id: '1234',
+          description: 'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'
+        }, {
+          id: '1235',
+          description: 'Between National Grid References SO 524 692 and SO 531 689 (KIRKENEL FARM ASHFORD CARBONEL - RIVER TEME)'
+        }, {
+          id: '1236',
+          description: 'Within the area formed by the straight lines running between National Grid References NZ 892 055 NZ 895 054 NZ 893 053 and NZ 892 053 (AREA D)'
+        }],
         licenceRef: '01/ABC',
         points: '',
         sessionId: '61e07498-f309-4829-96a9-72084a54996d'
@@ -87,7 +92,10 @@ describe('Return Requirements - Points presenter', () => {
       it("returns a 'At National Grid Reference ...' point", () => {
         const result = PointsPresenter.go(session, requirementIndex, pointsData)
 
-        expect(result.licencePoints).to.equal(['At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'])
+        expect(result.licencePoints).to.equal([{
+          id: '1234',
+          description: 'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'
+        }])
       })
     })
 
@@ -99,7 +107,10 @@ describe('Return Requirements - Points presenter', () => {
       it("returns a 'Between National Grid References ...' point", () => {
         const result = PointsPresenter.go(session, requirementIndex, pointsData)
 
-        expect(result.licencePoints).to.equal(['Between National Grid References SO 524 692 and SO 531 689 (KIRKENEL FARM ASHFORD CARBONEL - RIVER TEME)'])
+        expect(result.licencePoints).to.equal([{
+          id: '1235',
+          description: 'Between National Grid References SO 524 692 and SO 531 689 (KIRKENEL FARM ASHFORD CARBONEL - RIVER TEME)'
+        }])
       })
     })
 
@@ -111,7 +122,10 @@ describe('Return Requirements - Points presenter', () => {
       it("returns a 'Within the area formed by the straight lines running between National Grid References ...' point", () => {
         const result = PointsPresenter.go(session, requirementIndex, pointsData)
 
-        expect(result.licencePoints).to.equal(['Within the area formed by the straight lines running between National Grid References NZ 892 055 NZ 895 054 NZ 893 053 and NZ 892 053 (AREA D)'])
+        expect(result.licencePoints).to.equal([{
+          id: '1236',
+          description: 'Within the area formed by the straight lines running between National Grid References NZ 892 055 NZ 895 054 NZ 893 053 and NZ 892 053 (AREA D)'
+        }])
       })
     })
   })
@@ -120,15 +134,15 @@ describe('Return Requirements - Points presenter', () => {
     describe('when the user has previously submitted points', () => {
       beforeEach(() => {
         session.requirements[0].points = [
-          'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)',
-          'Between National Grid References SO 524 692 and SO 531 689 (KIRKENEL FARM ASHFORD CARBONEL - RIVER TEME)'
+          '1234',
+          '1235'
         ]
       })
 
       it('returns a populated points', () => {
         const result = PointsPresenter.go(session, requirementIndex, pointsData)
 
-        expect(result.points).to.equal('At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE),Between National Grid References SO 524 692 and SO 531 689 (KIRKENEL FARM ASHFORD CARBONEL - RIVER TEME)')
+        expect(result.points).to.equal('1234,1235')
       })
     })
 
@@ -145,6 +159,7 @@ describe('Return Requirements - Points presenter', () => {
 function _pointsData () {
   return [
     {
+      ID: '1234',
       NGR1_EAST: '69212',
       NGR2_EAST: 'null',
       NGR3_EAST: 'null',
@@ -160,6 +175,7 @@ function _pointsData () {
       NGR4_SHEET: 'null'
     },
     {
+      ID: '1235',
       NGR1_EAST: '524',
       NGR2_EAST: '531',
       NGR3_EAST: 'null',
@@ -175,6 +191,7 @@ function _pointsData () {
       NGR4_SHEET: 'null'
     },
     {
+      ID: '1236',
       NGR1_EAST: '892',
       NGR2_EAST: '895',
       NGR3_EAST: '893',

diff --git a/test/services/return-requirements/points.service.test.js b/test/services/return-requirements/points.service.test.js
@@ -46,6 +46,7 @@ describe('Return Requirements - Select Points service', () => {
 
     Sinon.stub(FetchPointsService, 'go').resolves([
       {
+        ID: '1234',
         NGR1_EAST: '69212',
         NGR2_EAST: 'null',
         NGR3_EAST: 'null',
@@ -83,7 +84,10 @@ describe('Return Requirements - Select Points service', () => {
         backLink: `/system/return-requirements/${session.id}/purpose/0`,
         licenceId: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d',
         licencePoints: [
-          'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'
+          {
+            id: '1234',
+            description: 'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'
+          }
         ],
         licenceRef: '01/ABC',
         points: ''

diff --git a/test/services/return-requirements/submit-points.service.test.js b/test/services/return-requirements/submit-points.service.test.js
@@ -54,9 +54,7 @@ describe('Return Requirements - Submit Points service', () => {
     describe('with a valid payload', () => {
       beforeEach(() => {
         payload = {
-          points: [
-            'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'
-          ]
+          points: ['1234']
         }
 
         Sinon.stub(FetchPointsService, 'go').resolves(_points())
@@ -68,7 +66,7 @@ describe('Return Requirements - Submit Points service', () => {
         const refreshedSession = await session.$query()
 
         expect(refreshedSession.requirements[0].points).to.equal([
-          'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'
+          '1234'
         ])
       })
 
@@ -97,9 +95,10 @@ describe('Return Requirements - Submit Points service', () => {
         pageTitle: 'Select the points for the requirements for returns',
         backLink: `/system/return-requirements/${session.id}/purpose/0`,
         licenceId: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d',
-        licencePoints: [
-          'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'
-        ],
+        licencePoints: [{
+          id: '1234',
+          description: 'At National Grid Reference TQ 69212 50394 (RIVER MEDWAY AT YALDING INTAKE)'
+        }],
         licenceRef: '01/ABC',
         points: ''
       }, { skip: ['sessionId', 'error'] })
@@ -120,6 +119,7 @@ describe('Return Requirements - Submit Points service', () => {
 function _points () {
   return [
     {
+      ID: '1234',
       NGR1_EAST: '69212',
       NGR2_EAST: 'null',
       NGR3_EAST: 'null',