From eab77cf15f44ce9bc6e791c6ca36b32665f3d69f Mon Sep 17 00:00:00 2001 From: Robert Parkinson Date: Tue, 14 May 2024 15:23:28 +0100 Subject: [PATCH 1/3] update points page to use point id as the value --- app/lib/general.lib.js | 38 ++++++++++++++++ .../return-requirements/points.presenter.js | 45 +++++++------------ app/views/return-requirements/points.njk | 6 +-- .../points.service.test.js | 6 ++- .../submit-points.service.test.js | 14 +++--- 5 files changed, 69 insertions(+), 40 deletions(-) diff --git a/app/lib/general.lib.js b/app/lib/general.lib.js index ab6351d644..62590ceb46 100644 --- 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/return-requirements/points.presenter.js b/app/presenters/return-requirements/points.presenter.js index 53b90b1052..e1fabb894f 100644 --- 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 index 1ee0030731..9c384909b6 100644 --- 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/services/return-requirements/points.service.test.js b/test/services/return-requirements/points.service.test.js index a1ba2a1099..9931beeb7a 100644 --- 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 index 51a166c97d..4d55e56e18 100644 --- 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', From 331ccbeb964d05505e93450a09fd39b5d2d5e25b Mon Sep 17 00:00:00 2001 From: Robert Parkinson Date: Tue, 14 May 2024 16:02:23 +0100 Subject: [PATCH 2/3] fix failing unit tests --- .../points.presenter.test.js | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/test/presenters/return-requirements/points.presenter.test.js b/test/presenters/return-requirements/points.presenter.test.js index 7303ef93bb..c2da8e6e18 100644 --- 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', From 599ba019eb7ed451753bb3eb2b29f121054aad09 Mon Sep 17 00:00:00 2001 From: Robert Parkinson Date: Tue, 14 May 2024 16:19:53 +0100 Subject: [PATCH 3/3] remove duplicated code --- .../view-licence-summary.presenter.js | 29 ++----------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/app/presenters/licences/view-licence-summary.presenter.js b/app/presenters/licences/view-licence-summary.presenter.js index 5c4a8fcc65..735e8becf4 100644 --- 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)