From 6c27f4932874db1c4335e54febbedc15ad487801 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 18 Jun 2024 14:54:24 +0100 Subject: [PATCH 01/22] Add requirements for returns view page https://eaflood.atlassian.net/browse/WATER-4422 The requirements for return journey has been built and the check page has been finished. This showed a page with requirements for returns allowing users to make changes to requirements for returns. This piece of work will replicate the requirements for returns check page but will not allow users to make changes (the card and information is identical, apart from the title and a return reference). There will be a link "View" on the licence set up page that links to this page. That is added as part of this change. Work for this was done here - https://github.com/DEFRA/water-abstraction-system/pull/1054 --- .../return-requirements/view.service.js | 27 +++ app/views/return-requirements/view.njk | 172 ++++++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 app/services/return-requirements/view.service.js create mode 100644 app/views/return-requirements/view.njk diff --git a/app/services/return-requirements/view.service.js b/app/services/return-requirements/view.service.js new file mode 100644 index 0000000000..5551834ce6 --- /dev/null +++ b/app/services/return-requirements/view.service.js @@ -0,0 +1,27 @@ +'use strict' + +/** + * Orchestrates fetching and presenting the data for `/return-requirements/{sessionId}/view` page + * @module ViewService + */ + +const FetchRequirementsForReturnsService = require('./fetch-requirements-for-returns.service.js') + +/** + * Orchestrates fetching and presenting the data for `/return-requirements/{sessionId}/view` page + * + * @param {string} returnVersionId - The UUID for return requirement version + * + * @returns {Promise} page data needed by the view template + */ +async function go (returnVersionId) { + // const requirementsForReturns = await FetchRequirementsForReturnsService.go(returnVersionId) + + return { + activeNavBar: 'search' + } +} + +module.exports = { + go +} diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk new file mode 100644 index 0000000000..9621966e48 --- /dev/null +++ b/app/views/return-requirements/view.njk @@ -0,0 +1,172 @@ +{% extends 'layout.njk' %} +{% from "govuk/components/back-link/macro.njk" import govukBackLink %} +{% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} +{% from "macros/charge-version-status-tag.njk" import statusTag %} + +{% macro newLine(array) %} + {% for item in array %} +

{{ item }}

+ {% endfor %} +{% endmacro %} + +{% block breadcrumbs %} + {# Back link #} + {{ govukBackLink({ + text: 'Back', + href: '/system/licences/' + licenceId + '/set-up' + }) }} +{% endblock %} + +{% block content %} + {# Main heading #} +
+

+ Licence {{ licenceRef }} + {{ pageTitle }} +

+ {{ statusTag(status) }} +
+ +
+
+ {{ govukSummaryList({ + classes: 'govuk-!-margin-bottom-2', + rows: [ + { + classes: 'govuk-summary-list govuk-summary-list__row--no-border', + key: { + text: "Reason" + }, + value: { + html: '' + reason + '' + } + }, + { + classes: 'govuk-summary-list govuk-summary-list__row--no-border', + key: { + text: "Start date" + }, + value: { + html: '' + startDate + '' + } + } + ] + }) }} +
+
+ +
+

Requirements for returns

+ {% for requirement in requirements %} + {% set rowIndex = loop.index0 %} +
+ {{ govukSummaryList({ + card: { + title: { + html: "

Return reference " + requirement.returnReference + "

" + requirement.title + } + }, + attributes: { + 'data-test': 'requirement-' + rowIndex + }, + classes: "govuk-summary-list--no-border", + rows: [ + { + key: { + text: "Purpose" + }, + value: { + html: '' + newLine(requirement.purposes) + '' + } + }, + { + key: { + text: "Points" + }, + value: { + html: '' + newLine(requirement.points) + '' + } + }, + { + key: { + text: "Abstraction period" + }, + value: { + html: '

' + requirement.abstractionPeriod + '

' + } + }, + { + key: { + text: "Returns cycle" + }, + value: { + html: '

' + requirement.returnsCycle + '

' + } + }, + { + key: { + text: "Site description" + }, + value: { + html: '

' + requirement.siteDescription + '

' + } + }, + { + key: { + text: "Collection" + }, + value: { + html: '

' + requirement.frequencyCollected | capitalize + '

' + } + }, + { + key: { + text: "Reporting" + }, + value: { + html: '

' + requirement.frequencyReported | capitalize + '

' + } + }, + { + key: { + text: "Agreements exceptions" + }, + value: { + html: '

' + requirement.agreementsExceptions + '

' + } + } + ] + }) }} +
+ {% endfor %} +
+ +
+ {{ govukSummaryList({ + classes: 'govuk-!-margin-bottom-2', + rows: [ + { + classes: 'govuk-summary-list', + key: { + text: 'Additional submission options', + classes: "govuk-heading-m govuk-!-width-one-half" + }, + value: { + text: '' + } + }, + { + classes: 'govuk-summary-list govuk-summary-list__row--no-border', + key: { + text: 'Multiple upload', + classes: "govuk-body " + }, + value: { + text: additionalSubmissionOptions.multipleUpload + } + } + ] + }) }} +
+ +{% endblock %} From f0469b4b79805364df288363e7c0fcf0886f315a Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 18 Jun 2024 14:54:34 +0100 Subject: [PATCH 02/22] Add requirements for returns view page https://eaflood.atlassian.net/browse/WATER-4422 The requirements for return journey has been built and the check page has been finished. This showed a page with requirements for returns allowing users to make changes to requirements for returns. This piece of work will replicate the requirements for returns check page but will not allow users to make changes (the card and information is identical, apart from the title and a return reference). There will be a link "View" on the licence set up page that links to this page. That is added as part of this change. Work for this was done here - https://github.com/DEFRA/water-abstraction-system/pull/1054 --- .../return-requirements.controller.js | 13 ++++++++- app/routes/return-requirement.routes.js | 12 ++++++++ .../return-requirements/view.service.js | 2 +- .../return-requirements.controller.test.js | 29 +++++++++++++++++++ 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/app/controllers/return-requirements.controller.js b/app/controllers/return-requirements.controller.js index 1325b352dc..a3c9f98834 100644 --- a/app/controllers/return-requirements.controller.js +++ b/app/controllers/return-requirements.controller.js @@ -43,6 +43,7 @@ const SubmitReturnsCycleService = require('../services/return-requirements/submi const SubmitSetupService = require('../services/return-requirements/setup/submit-setup.service.js') const SubmitSiteDescriptionService = require('../services/return-requirements/submit-site-description.service.js') const SubmitStartDateService = require('../services/return-requirements/submit-start-date.service.js') +const ViewService = require('../services/return-requirements/view.service.js') async function abstractionPeriod (request, h) { const { requirementIndex, sessionId } = request.params @@ -494,6 +495,15 @@ async function submitStartDate (request, h) { return h.redirect(`/system/return-requirements/${sessionId}/no-returns-required`) } +async function view (request, h) { + const { returnVersionId } = request.params + const pageData = await ViewService.go(returnVersionId) + + return h.view('return-requirements/view.njk', { + ...pageData + }) +} + module.exports = { abstractionPeriod, add, @@ -533,5 +543,6 @@ module.exports = { submitReturnsCycle, submitSetup, submitSiteDescription, - submitStartDate + submitStartDate, + view } diff --git a/app/routes/return-requirement.routes.js b/app/routes/return-requirement.routes.js index dc28622818..775f8053a4 100644 --- a/app/routes/return-requirement.routes.js +++ b/app/routes/return-requirement.routes.js @@ -470,6 +470,18 @@ const routes = [ } } } + }, + { + method: 'GET', + path: '/return-requirements/{returnVersionId}/view', + handler: ReturnRequirementsController.view, + options: { + auth: { + access: { + scope: ['billing'] + } + } + } } ] diff --git a/app/services/return-requirements/view.service.js b/app/services/return-requirements/view.service.js index 5551834ce6..bd9fd3813e 100644 --- a/app/services/return-requirements/view.service.js +++ b/app/services/return-requirements/view.service.js @@ -5,7 +5,7 @@ * @module ViewService */ -const FetchRequirementsForReturnsService = require('./fetch-requirements-for-returns.service.js') +// const FetchRequirementsForReturnsService = require('./fetch-requirements-for-returns.service.js') /** * Orchestrates fetching and presenting the data for `/return-requirements/{sessionId}/view` page diff --git a/test/controllers/return-requirements.controller.test.js b/test/controllers/return-requirements.controller.test.js index 510f9c4747..2c130fbf92 100644 --- a/test/controllers/return-requirements.controller.test.js +++ b/test/controllers/return-requirements.controller.test.js @@ -41,6 +41,7 @@ const SubmitReturnsCycleService = require('../../app/services/return-requirement const SubmitSetupService = require('../../app/services/return-requirements/setup/submit-setup.service.js') const SubmitSiteDescriptionService = require('../../app/services/return-requirements/submit-site-description.service.js') const SubmitStartDateService = require('../../app/services/return-requirements/submit-start-date.service.js') +const ViewService = require('../../app/services/return-requirements/view.service.js') // For running our service const { init } = require('../../app/server.js') @@ -1004,6 +1005,34 @@ describe('Return requirements controller', () => { }) }) }) + + describe('/return-requirements/{sessionId}/view', () => { + const path = 'view' + const returnVersionId = '2a075724-b66c-410e-9fc8-b964077204f2' + + describe('GET', () => { + beforeEach(async () => { + Sinon.stub(ViewService, 'go').resolves({ + pageTitle: 'Requirements for returns valid from' + }) + }) + + describe('when the request succeeds', () => { + it('returns the page successfully', async () => { + const response = await server.inject({ + method: 'GET', + url: `/return-requirements/${returnVersionId}/${path}`, + auth: { + strategy: 'session', + credentials: { scope: ['billing'] } + } + }) + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Requirements for returns valid from') + }) + }) + }) + }) }) function _getOptions (path, index = -1) { From 3ccc670afd4086d4162e856b0e3b4f3d64e26cad Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 19 Jun 2024 13:27:52 +0100 Subject: [PATCH 03/22] feat: add logic to display view --- app/lib/general.lib.js | 31 ++++ app/presenters/licences/set-up.presenter.js | 2 +- .../return-requirements/view.presenter.js | 138 ++++++++++++++++++ .../fetch-requirements-for-returns.service.js | 83 +++++++++++ .../return-requirements/view.service.js | 10 +- app/views/return-requirements/view.njk | 14 +- .../licences/set-up.presenter.test.js | 4 +- 7 files changed, 269 insertions(+), 13 deletions(-) create mode 100644 app/presenters/return-requirements/view.presenter.js create mode 100644 app/services/return-requirements/fetch-requirements-for-returns.service.js diff --git a/app/lib/general.lib.js b/app/lib/general.lib.js index bb1785a201..0fc1663364 100644 --- a/app/lib/general.lib.js +++ b/app/lib/general.lib.js @@ -152,6 +152,36 @@ function generateAbstractionPointDetail (pointDetail) { return abstractionPoint } +/** + * Generate a string that represents an abstraction point based on the new data (obs change this in PR) + * + */ +function generatePointDetail (point) { + let abstractionPoint = null + + if (point.ngr4) { + const point1 = point.ngr1 + const point2 = point.ngr2 + const point3 = point.ngr3 + const point4 = point.ngr4 + + abstractionPoint = `Within the area formed by the straight lines running between National Grid References ${point1} ${point2} ${point3} and ${point4}` + } else if (point.ngr2) { + const point1 = point.ngr1 + const point2 = point.ngr2 + + abstractionPoint = `Between National Grid References ${point1} and ${point2}` + } else { + const point1 = point.ngr1 + + abstractionPoint = `At National Grid Reference ${point1}` + } + + abstractionPoint += point.description !== undefined ? ` (${point.description})` : '' + + return abstractionPoint +} + /** * Generate a Universally Unique Identifier (UUID) * @@ -295,6 +325,7 @@ module.exports = { determineCurrentFinancialYear, flashNotification, generateAbstractionPointDetail, + generatePointDetail, generateUUID, periodsOverlap, timestampForPostgres, diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index 76f179fd0f..c9c1a1a498 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -175,7 +175,7 @@ function _returnVersions (returnVersions = [{}]) { return { action: [{ text: 'View', - link: '' + link: `/system/return-requirements/${returnVersion.id}/view` }], endDate: returnVersion.endDate ? formatLongDate(returnVersion.endDate) : '', reason: returnVersion.reason ? returnRequirementReasons[returnVersion.reason] : '', diff --git a/app/presenters/return-requirements/view.presenter.js b/app/presenters/return-requirements/view.presenter.js new file mode 100644 index 0000000000..f1aee353fb --- /dev/null +++ b/app/presenters/return-requirements/view.presenter.js @@ -0,0 +1,138 @@ +'use strict' + +/** + * Formats return requirements data for the `/return-requirements/{sessionId}/view` page + * @module ViewPresenter + */ + +const { formatAbstractionDate } = require('../base.presenter.js') +const { formatLongDate } = require('../base.presenter.js') +const { generatePointDetail } = require('../../lib/general.lib.js') +const { returnRequirementReasons, returnRequirementFrequencies } = require('../../lib/static-lookups.lib.js') + +/** + * Formats requirements for returns data for the `/return-requirements/{sessionId}/view` page + * + * @param {ReturnVersionModel[]} requirementsForReturns - The return version inc licence and requirements + * + * @returns {Object} returns requirement data needed by the view template + */ + +function go (requirementsForReturns) { + const { returnRequirements, licence, reason, startDate, status } = requirementsForReturns + + return { + additionalSubmissionOptions: { + multipleUpload: requirementsForReturns.multipleUpload === true ? 'Yes' : 'No' + }, + licenceId: licence.id, + licenceRef: licence.licenceRef, + pageTitle: `Check the requirements for returns for ${licence.$licenceHolder()}`, + reason: returnRequirementReasons[reason] || '', + requirements: _requirements(returnRequirements), + startDate: formatLongDate(startDate), + status: _status(status) + } +} + +function _abstractionPeriod (requirement) { + const { + abstractionPeriodStartDay, + abstractionPeriodStartMonth, + abstractionPeriodEndDay, + abstractionPeriodEndMonth + } = requirement + + const startDate = formatAbstractionDate(abstractionPeriodStartDay, abstractionPeriodStartMonth) + const endDate = formatAbstractionDate(abstractionPeriodEndDay, abstractionPeriodEndMonth) + + return `From ${startDate} to ${endDate}` +} + +function _agreementsExceptions (returnRequirement) { + const agreementsExceptions = _buildAgreementExceptions(returnRequirement) + + if (agreementsExceptions.length === 1) { + return agreementsExceptions[0] + } + + if (agreementsExceptions.length === 2) { + return agreementsExceptions.join(' and ') + } + + return agreementsExceptions.slice(0, agreementsExceptions.length - 1) + .join(', ') + ', and ' + agreementsExceptions[agreementsExceptions.length - 1] +} + +function _buildAgreementExceptions (returnRequirement) { + const { fiftySixException, gravityFill, reabstraction, twoPartTariff } = returnRequirement + const agreementsExceptions = [] + + if (gravityFill) { + agreementsExceptions.push('Gravity fill') + } + + if (reabstraction) { + agreementsExceptions.push('Transfer re-abstraction scheme') + } + + if (twoPartTariff) { + agreementsExceptions.push('Two-part tariff') + } + + if (fiftySixException) { + agreementsExceptions.push('56 returns exception') + } + + if (agreementsExceptions.length === 0) { + agreementsExceptions.push('None') + } + + return agreementsExceptions +} + +function _mapRequirement (requirement) { + return { + abstractionPeriod: _abstractionPeriod(requirement), + agreementsExceptions: _agreementsExceptions(requirement), + frequencyCollected: returnRequirementFrequencies[requirement.collectionFrequency], + frequencyReported: returnRequirementFrequencies[requirement.reportingFrequency], + points: _points(requirement.points), + purposes: _purposes(requirement.purposes), + returnsCycle: requirement.returnsCycle === 'summer' ? 'Summer' : 'Winter and all year', + siteDescription: requirement.siteDescription, + returnReference: requirement.legacyId, + title: requirement.siteDescription + } +} + +function _purposes (returnRequirementPurposes) { + return returnRequirementPurposes.map((returnRequirementPurpose) => { + return returnRequirementPurpose.purposeDetails.description + }) +} + +function _points (returnRequirementPoints) { + return returnRequirementPoints.map((returnRequirementPoint) => { + return generatePointDetail(returnRequirementPoint) + }) +} + +function _requirements (requirements, points) { + return requirements.map((requirement) => { + return _mapRequirement(requirement, points) + }) +} + +function _status (status) { + const statuses = { + current: 'approved', + superseded: 'replaced' + } + + return statuses[status] +} + +module.exports = { + go +} diff --git a/app/services/return-requirements/fetch-requirements-for-returns.service.js b/app/services/return-requirements/fetch-requirements-for-returns.service.js new file mode 100644 index 0000000000..762b6e7bbe --- /dev/null +++ b/app/services/return-requirements/fetch-requirements-for-returns.service.js @@ -0,0 +1,83 @@ +'use strict' + +/** + * Fetches requirements for returns by the return version id + * @module FetchRequirementsForReturnsService + */ + +const ReturnVersionModel = require('../../models/return-version.model.js') + +/** + * Fetches requirements for returns by the return version id + * + * Includes the licence + * + * Formats the requirements + * + * @param {string} returnVersionId - The UUID of the selected return version to get requirements for + * + * @returns {Promise} The return version and its linked return requirements, plus their points and purposes + */ +async function go (returnVersionId) { + const returnVersion = await _fetch(returnVersionId) + + return returnVersion +} + +async function _fetch (returnVersionId) { + return ReturnVersionModel.query() + .findById(returnVersionId) + .select([ + 'id', + 'reason', + 'startDate', + 'status', + 'multiple_upload' + ]) + .withGraphFetched('licence') + .modifyGraph('licence', (builder) => { + builder.select([ + 'id' + ]).modify('licenceHolder') + }) + .withGraphFetched('returnRequirements') + .modifyGraph('returnRequirements', (builder) => { + builder.select([ + 'id', + 'abstractionPeriodEndDay', + 'abstractionPeriodEndMonth', + 'abstractionPeriodStartDay', + 'abstractionPeriodStartMonth', + 'collectionFrequency', + 'fiftySixException', + 'gravityFill', + 'reabstraction', + 'reportingFrequency', + 'siteDescription', + 'summer', + 'twoPartTariff', + 'legacyId' + ]) + }) + .withGraphFetched('returnRequirements.[returnRequirementPoints as points]') + .modifyGraph('returnRequirements.[returnRequirementPoints as points]', (builder) => { + builder.select([ + 'description', + 'ngr1', + 'ngr2', + 'ngr3', + 'ngr4' + ]) + }) + .withGraphFetched('returnRequirements.[returnRequirementPurposes as purposes.[purpose as purposeDetails]]') + .modifyGraph('returnRequirements.[returnRequirementPurposes as purposes]', (builder) => { + builder.select(['id']) + }) + .modifyGraph('returnRequirements.[returnRequirementPurposes as purposes.[purpose as purposeDetails]]', (builder) => { + builder.select(['description']) + }) +} + +module.exports = { + go +} diff --git a/app/services/return-requirements/view.service.js b/app/services/return-requirements/view.service.js index bd9fd3813e..c1a0c248ea 100644 --- a/app/services/return-requirements/view.service.js +++ b/app/services/return-requirements/view.service.js @@ -5,7 +5,8 @@ * @module ViewService */ -// const FetchRequirementsForReturnsService = require('./fetch-requirements-for-returns.service.js') +const FetchRequirementsForReturnsService = require('./fetch-requirements-for-returns.service.js') +const ViewPresenter = require('../../presenters/return-requirements/view.presenter.js') /** * Orchestrates fetching and presenting the data for `/return-requirements/{sessionId}/view` page @@ -15,10 +16,13 @@ * @returns {Promise} page data needed by the view template */ async function go (returnVersionId) { - // const requirementsForReturns = await FetchRequirementsForReturnsService.go(returnVersionId) + const requirementsForReturns = await FetchRequirementsForReturnsService.go(returnVersionId) + + const data = ViewPresenter.go(requirementsForReturns) return { - activeNavBar: 'search' + activeNavBar: 'search', + ...data } } diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk index 9621966e48..44ec67793e 100644 --- a/app/views/return-requirements/view.njk +++ b/app/views/return-requirements/view.njk @@ -35,20 +35,20 @@ { classes: 'govuk-summary-list govuk-summary-list__row--no-border', key: { - text: "Reason" + text: "Start date" }, - value: { - html: '' + reason + '' + value: { + html: '' + startDate + '' } }, { classes: 'govuk-summary-list govuk-summary-list__row--no-border', key: { - text: "Start date" - }, + text: "Reason" + }, value: { - html: '' + startDate + '' - } + html: '' + reason + '' + } } ] }) }} diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index d7fd9efc3b..fa2b3f2c69 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -505,7 +505,7 @@ describe('Licences - Set Up presenter', () => { { action: [ { - link: '', + link: '/system/return-requirements/0312e5eb-67ae-44fb-922c-b1a0b81bc08d/view', text: 'View' } ], @@ -529,7 +529,7 @@ describe('Licences - Set Up presenter', () => { { action: [ { - link: '', + link: '/system/return-requirements/0312e5eb-67ae-44fb-922c-b1a0b81bc08d/view', text: 'View' } ], From 5724f2aed17f9a9d76c9fefada19a8546632c0be Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 19 Jun 2024 15:00:11 +0100 Subject: [PATCH 04/22] feat: add note --- .../return-requirements/view.presenter.js | 3 ++- .../fetch-requirements-for-returns.service.js | 11 +++++----- app/views/return-requirements/view.njk | 22 +++++++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/app/presenters/return-requirements/view.presenter.js b/app/presenters/return-requirements/view.presenter.js index f1aee353fb..2e3a21b7ff 100644 --- a/app/presenters/return-requirements/view.presenter.js +++ b/app/presenters/return-requirements/view.presenter.js @@ -19,7 +19,7 @@ const { returnRequirementReasons, returnRequirementFrequencies } = require('../. */ function go (requirementsForReturns) { - const { returnRequirements, licence, reason, startDate, status } = requirementsForReturns + const { returnRequirements, licence, reason, startDate, status, notes } = requirementsForReturns return { additionalSubmissionOptions: { @@ -27,6 +27,7 @@ function go (requirementsForReturns) { }, licenceId: licence.id, licenceRef: licence.licenceRef, + notes, pageTitle: `Check the requirements for returns for ${licence.$licenceHolder()}`, reason: returnRequirementReasons[reason] || '', requirements: _requirements(returnRequirements), diff --git a/app/services/return-requirements/fetch-requirements-for-returns.service.js b/app/services/return-requirements/fetch-requirements-for-returns.service.js index 762b6e7bbe..639f964091 100644 --- a/app/services/return-requirements/fetch-requirements-for-returns.service.js +++ b/app/services/return-requirements/fetch-requirements-for-returns.service.js @@ -29,10 +29,11 @@ async function _fetch (returnVersionId) { .findById(returnVersionId) .select([ 'id', + 'multiple_upload', + 'notes', 'reason', 'startDate', - 'status', - 'multiple_upload' + 'status' ]) .withGraphFetched('licence') .modifyGraph('licence', (builder) => { @@ -43,7 +44,6 @@ async function _fetch (returnVersionId) { .withGraphFetched('returnRequirements') .modifyGraph('returnRequirements', (builder) => { builder.select([ - 'id', 'abstractionPeriodEndDay', 'abstractionPeriodEndMonth', 'abstractionPeriodStartDay', @@ -51,12 +51,13 @@ async function _fetch (returnVersionId) { 'collectionFrequency', 'fiftySixException', 'gravityFill', + 'id', + 'legacyId', 'reabstraction', 'reportingFrequency', 'siteDescription', 'summer', - 'twoPartTariff', - 'legacyId' + 'twoPartTariff' ]) }) .withGraphFetched('returnRequirements.[returnRequirementPoints as points]') diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk index 44ec67793e..d05aa0be95 100644 --- a/app/views/return-requirements/view.njk +++ b/app/views/return-requirements/view.njk @@ -55,6 +55,28 @@
+{% if notes %} +
+

Notes

+
+ {{ govukSummaryList({ + classes: 'govuk-!-margin-bottom-2', + rows: [ + { + classes: 'govuk-summary-list govuk-summary-list__row--no-border', + key: { + text: userEmail, + classes: "govuk-body govuk-!-font-weight-regular" + }, + value: { + text: notes | escape | nl2br + } + } + ] + }) }} +
+
+{% endif %}

Requirements for returns

{% for requirement in requirements %} From 8fe036beb992894791ca283b9b6ba6473b2f7623 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 19 Jun 2024 15:31:57 +0100 Subject: [PATCH 05/22] test: view presenter and fetch service --- .../return-requirements/view.presenter.js | 2 +- .../view.presenter.test.js | 296 ++++++++++++++++++ .../view-licence-set-up.service.test.js | 2 +- ...h-requirements-for-returns.service.test.js | 118 +++++++ .../requirements-for-returns.seeder.js | 5 + 5 files changed, 421 insertions(+), 2 deletions(-) create mode 100644 test/presenters/return-requirements/view.presenter.test.js create mode 100644 test/services/return-requirements/fetch-requirements-for-returns.service.test.js diff --git a/app/presenters/return-requirements/view.presenter.js b/app/presenters/return-requirements/view.presenter.js index 2e3a21b7ff..9c9e254b28 100644 --- a/app/presenters/return-requirements/view.presenter.js +++ b/app/presenters/return-requirements/view.presenter.js @@ -100,7 +100,7 @@ function _mapRequirement (requirement) { frequencyReported: returnRequirementFrequencies[requirement.reportingFrequency], points: _points(requirement.points), purposes: _purposes(requirement.purposes), - returnsCycle: requirement.returnsCycle === 'summer' ? 'Summer' : 'Winter and all year', + returnsCycle: requirement.summer === true ? 'Summer' : 'Winter and all year', siteDescription: requirement.siteDescription, returnReference: requirement.legacyId, title: requirement.siteDescription diff --git a/test/presenters/return-requirements/view.presenter.test.js b/test/presenters/return-requirements/view.presenter.test.js new file mode 100644 index 0000000000..2822e93bad --- /dev/null +++ b/test/presenters/return-requirements/view.presenter.test.js @@ -0,0 +1,296 @@ +'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 ViewPresenter = require('../../../app/presenters/return-requirements/view.presenter.js') + +describe('Return Requirements - View presenter', () => { + let requirement + let requirementsForReturns + let points + let purposes + + beforeEach(() => { + points = [ + { + description: 'Midway River', + ngr1: 'TQ 69212 50394', + ngr2: null, + ngr3: null, + ngr4: null + } + ] + + purposes = [ + { + id: '3675988f-076a-42af-8bcd-a7d53e78461c', + purposeDetails: { + description: 'Transfer Between Sources (Pre Water Act 2003)' + } + } + ] + + requirement = { + abstractionPeriodEndDay: 31, + abstractionPeriodEndMonth: 3, + abstractionPeriodStartDay: 1, + abstractionPeriodStartMonth: 4, + collectionFrequency: 'day', + fiftySixException: false, + gravityFill: true, + id: '49a0aff3-268d-4b1d-870d-67acf3e3b0b5', + legacyId: 12345, + reabstraction: false, + reportingFrequency: 'day', + siteDescription: 'A place in the sun', + summer: false, + twoPartTariff: false, + points, + purposes + } + + requirementsForReturns = { + id: '0d6f0f07-7d5c-48bf-90f0-d441680c3653', + licence: { + id: 'c32ab7c6-e342-47b2-9c2e-d178ca89c5e5', + licenceRef: '02/01', + $licenceHolder: () => { return 'Martha Stuart' }, + licenceDocument: { + id: '4391032d-7165-49ab-98cd-245ae8b8e10d', + licenceDocumentRoles: [] + } + }, + multipleUpload: false, + notes: 'A special note', + reason: 'major-change', + returnRequirements: [{ ...requirement }], + startDate: new Date('2023-04-21'), + status: 'current' + } + }) + + describe('when provided with requirements for returns', () => { + it('correctly presents the data', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result).to.equal({ + additionalSubmissionOptions: { + multipleUpload: 'No' + }, + licenceId: 'c32ab7c6-e342-47b2-9c2e-d178ca89c5e5', + licenceRef: '02/01', + notes: 'A special note', + pageTitle: 'Check the requirements for returns for Martha Stuart', + reason: 'Major change', + requirements: [ + { + abstractionPeriod: 'From 1 April to 31 March', + agreementsExceptions: 'Gravity fill', + frequencyCollected: 'daily', + frequencyReported: 'daily', + points: [ + 'At National Grid Reference TQ 69212 50394 (Midway River)' + ], + purposes: [ + 'Transfer Between Sources (Pre Water Act 2003)' + ], + returnReference: 12345, + returnsCycle: 'Winter and all year', + siteDescription: 'A place in the sun', + title: 'A place in the sun' + } + ], + startDate: '21 April 2023', + status: 'approved' + }) + }) + + describe('the "notes" property', () => { + it('returns notes', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.notes).to.equal('A special note') + }) + }) + + describe('the "reason" property', () => { + describe('and a reason', () => { + it('returns the formatted reason', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.reason).to.equal('Major change') + }) + }) + + describe('and no reason', () => { + beforeEach(() => { + requirementsForReturns.reason = null + }) + + it('correctly defaults reason to an empty string', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.reason).to.equal('') + }) + }) + }) + + describe('the "additionalSubmissionOptions" property', () => { + describe('when multipleUpload is true', () => { + beforeEach(() => { + requirementsForReturns.multipleUpload = true + }) + + it('returns "Yes"', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.additionalSubmissionOptions.multipleUpload).to.equal('Yes') + }) + }) + + describe('when multipleUpload is false', () => { + it('returns "No"', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.additionalSubmissionOptions.multipleUpload).to.equal('No') + }) + }) + }) + + describe('the \'requirements\' property', () => { + it('correctly returns the requirement', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements).to.equal([{ + abstractionPeriod: 'From 1 April to 31 March', + agreementsExceptions: 'Gravity fill', + frequencyCollected: 'daily', + frequencyReported: 'daily', + points: [ + 'At National Grid Reference TQ 69212 50394 (Midway River)' + ], + purposes: [ + 'Transfer Between Sources (Pre Water Act 2003)' + ], + returnsCycle: 'Winter and all year', + returnReference: 12345, + siteDescription: 'A place in the sun', + title: 'A place in the sun' + }]) + }) + + it('returns the purpose descriptions', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements[0].purposes).to.equal(['Transfer Between Sources (Pre Water Act 2003)']) + }) + + it('maps the selected points to the abstraction point details format', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements[0].points).to.equal(['At National Grid Reference TQ 69212 50394 (Midway River)']) + }) + + describe('and the return cycle is', () => { + describe('Summer', () => { + beforeEach(() => { + requirementsForReturns.returnRequirements = [{ ...requirement, summer: true }] + }) + + it('should return the text for a summer return cycle', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements[0].returnsCycle).to.equal('Summer') + }) + }) + + describe('Winter and all year', () => { + it('should return the text for a Winter and all year return cycle', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements[0].returnsCycle).to.equal('Winter and all year') + }) + }) + }) + + describe('and the agreement exceptions', () => { + describe('is "none"', () => { + beforeEach(() => { + requirementsForReturns.returnRequirements = [{ + ...requirement, + fiftySixException: false, + gravityFill: false, + reabstraction: false, + twoPartTariff: false + }] + }) + + it('should return "None"', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements[0].agreementsExceptions).to.equal('None') + }) + }) + + describe('has one exception', () => { + it('should return the exception as "Gravity fill"', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements[0].agreementsExceptions).to.equal('Gravity fill') + }) + }) + + describe('has two exceptions', () => { + beforeEach(() => { + requirementsForReturns.returnRequirements = [{ + ...requirement, + fiftySixException: false, + gravityFill: true, + reabstraction: true, + twoPartTariff: false + }] + }) + + it('should return the exceptions as "Gravity fill and Transfer re-abstraction scheme"', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements[0].agreementsExceptions).to.equal('Gravity fill and Transfer re-abstraction scheme') + }) + }) + + describe('has more than two exceptions', () => { + beforeEach(() => { + requirementsForReturns.returnRequirements = [ + { + ...requirement, + fiftySixException: true, + gravityFill: true, + reabstraction: true, + twoPartTariff: true + }] + }) + + it('should return the exceptions as "Gravity fill, Transfer re-abstraction scheme, Two-part tariff, and 56 returns exception"', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements[0].agreementsExceptions).to.equal('Gravity fill, Transfer re-abstraction scheme, Two-part tariff, and 56 returns exception') + }) + }) + }) + + describe('the "pageTitle" property', () => { + it('correctly formats the licence holder name in the title', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.pageTitle).to.equal('Check the requirements for returns for Martha Stuart') + }) + }) + }) + }) +}) diff --git a/test/services/licences/view-licence-set-up.service.test.js b/test/services/licences/view-licence-set-up.service.test.js index bdbb38d8bb..f8157baf09 100644 --- a/test/services/licences/view-licence-set-up.service.test.js +++ b/test/services/licences/view-licence-set-up.service.test.js @@ -141,7 +141,7 @@ describe('View Licence Set Up service', () => { { action: [ { - link: '', + link: '/system/return-requirements/0312e5eb-67ae-44fb-922c-b1a0b81bc08d/view', text: 'View' } ], diff --git a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js new file mode 100644 index 0000000000..a55bf09b07 --- /dev/null +++ b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js @@ -0,0 +1,118 @@ +'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 + +// Test helpers +const DatabaseSupport = require('../../support/database.js') +const LicenceHelper = require('../../support/helpers/licence.helper.js') +const RequirementsForReturnsSeeder = require('../../support/seeders/requirements-for-returns.seeder.js') + +// Thing under test +const FetchRequirementsForReturnsService = require('../../../app/services/return-requirements/fetch-requirements-for-returns.service.js') + +describe('Return Requirements - Fetch Requirements for returns service', () => { + let returnVersion + let licence + + beforeEach(async () => { + await DatabaseSupport.clean() + }) + + describe('when a matching return version exists', () => { + beforeEach(async () => { + licence = await LicenceHelper.add() + + returnVersion = await RequirementsForReturnsSeeder.seed(licence.id) + }) + + it('returns the details of the requirements for returns transformed for use in the view page', async () => { + const result = await FetchRequirementsForReturnsService.go(returnVersion.id) + const [returnRequirementsOne, returnRequirementsTwo] = result.returnRequirements + expect(result).to.equal({ + id: returnVersion.id, + licence: { + id: licence.id, + licenceDocument: null + }, + multipleUpload: false, + notes: null, + reason: 'new-licence', + returnRequirements: [ + { + abstractionPeriodEndDay: 31, + abstractionPeriodEndMonth: 3, + abstractionPeriodStartDay: 1, + abstractionPeriodStartMonth: 4, + collectionFrequency: 'week', + fiftySixException: false, + gravityFill: false, + id: returnRequirementsOne.id, + legacyId: returnRequirementsOne.legacyId, + points: [ + { + description: null, + ngr1: returnRequirementsOne.points[0].ngr1, + ngr2: null, + ngr3: null, + ngr4: null + } + ], + purposes: [ + { + id: returnRequirementsOne.purposes[0].id, + purposeDetails: { + description: 'Spray Irrigation - Storage' + } + } + ], + reabstraction: false, + reportingFrequency: 'week', + siteDescription: 'FIRST BOREHOLE AT AVALON', + summer: false, + twoPartTariff: false + }, + { + abstractionPeriodEndDay: 31, + abstractionPeriodEndMonth: 3, + abstractionPeriodStartDay: 1, + abstractionPeriodStartMonth: 4, + collectionFrequency: 'week', + fiftySixException: true, + gravityFill: true, + id: returnRequirementsTwo.id, + legacyId: returnRequirementsTwo.legacyId, + points: [ + { + description: null, + ngr1: returnRequirementsTwo.points[0].ngr1, + ngr2: null, + ngr3: null, + ngr4: null + } + ], + purposes: [ + { + id: returnRequirementsTwo.purposes[0].id, + purposeDetails: { + description: 'Spray Irrigation - Storage' + } + } + ], + reabstraction: true, + reportingFrequency: 'month', + siteDescription: 'SECOND BOREHOLE AT AVALON', + summer: true, + twoPartTariff: true + } + ], + startDate: new Date('2022-04-01'), + status: 'current' + }) + }) + }) +}) diff --git a/test/support/seeders/requirements-for-returns.seeder.js b/test/support/seeders/requirements-for-returns.seeder.js index 9e753b8622..08b7136066 100644 --- a/test/support/seeders/requirements-for-returns.seeder.js +++ b/test/support/seeders/requirements-for-returns.seeder.js @@ -4,6 +4,7 @@ * @module RequirementsForReturnsSeeder */ +const PurposeHelper = require('../helpers/purpose.helper.js') const ReturnRequirementPointHelper = require('../helpers/return-requirement-point.helper.js') const ReturnRequirementPurposeHelper = require('../helpers/return-requirement-purpose.helper.js') const ReturnRequirementHelper = require('../helpers/return-requirement.helper.js') @@ -78,6 +79,10 @@ async function _returnRequirement (returnVersionId, reportingFrequency, summer, const purpose = await ReturnRequirementPurposeHelper.add({ purposeId, returnRequirementId }) returnRequirement.returnRequirementPurposes = [purpose] + await PurposeHelper.add({ + id: purposeId + }) + return returnRequirement } From 3b8d1db58fb7201b6c42ea5d69692668d85f495e Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 19 Jun 2024 15:39:36 +0100 Subject: [PATCH 06/22] test: services --- ...h-requirements-for-returns.service.test.js | 5 +- .../return-requirements/view.service.test.js | 85 +++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 test/services/return-requirements/view.service.test.js diff --git a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js index a55bf09b07..90d0bbbe68 100644 --- a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js +++ b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js @@ -16,8 +16,8 @@ const RequirementsForReturnsSeeder = require('../../support/seeders/requirements const FetchRequirementsForReturnsService = require('../../../app/services/return-requirements/fetch-requirements-for-returns.service.js') describe('Return Requirements - Fetch Requirements for returns service', () => { - let returnVersion let licence + let returnVersion beforeEach(async () => { await DatabaseSupport.clean() @@ -25,8 +25,9 @@ describe('Return Requirements - Fetch Requirements for returns service', () => { describe('when a matching return version exists', () => { beforeEach(async () => { - licence = await LicenceHelper.add() + await DatabaseSupport.clean() + licence = await LicenceHelper.add() returnVersion = await RequirementsForReturnsSeeder.seed(licence.id) }) diff --git a/test/services/return-requirements/view.service.test.js b/test/services/return-requirements/view.service.test.js new file mode 100644 index 0000000000..e1afe791ba --- /dev/null +++ b/test/services/return-requirements/view.service.test.js @@ -0,0 +1,85 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') + +const { describe, it, afterEach, beforeEach } = exports.lab = Lab.script() +const { expect } = Code +const Sinon = require('sinon') + +// Test helpers +const DatabaseSupport = require('../../support/database.js') +const LicenceHelper = require('../../support/helpers/licence.helper.js') +const RequirementsForReturnsSeeder = require('../../support/seeders/requirements-for-returns.seeder.js') + +// Thing under test +const ViewService = require('../../../app/services/return-requirements/view.service.js') + +describe('Return Requirements - View service', () => { + let licence + let returnVersion + + beforeEach(async () => { + await DatabaseSupport.clean() + + licence = await LicenceHelper.add() + returnVersion = await RequirementsForReturnsSeeder.seed(licence.id) + }) + + afterEach(() => { + Sinon.restore() + }) + + describe('when called', () => { + it('returns page data for the view', async () => { + const result = await ViewService.go(returnVersion.id) + const [requirementOne, requirementTwo] = result.requirements + + expect(result).to.equal({ + activeNavBar: 'search', + additionalSubmissionOptions: { + multipleUpload: 'No' + }, + licenceId: licence.id, + licenceRef: licence.ref, + notes: null, + pageTitle: 'Check the requirements for returns for null', + reason: 'New licence', + requirements: [ + { + abstractionPeriod: 'From 1 April to 31 March', + agreementsExceptions: 'None', + frequencyCollected: 'weekly', + frequencyReported: 'weekly', + points: requirementOne.points, + purposes: [ + 'Spray Irrigation - Storage' + ], + returnReference: requirementOne.returnReference, + returnsCycle: 'Winter and all year', + siteDescription: 'FIRST BOREHOLE AT AVALON', + title: 'FIRST BOREHOLE AT AVALON' + }, + { + abstractionPeriod: 'From 1 April to 31 March', + agreementsExceptions: 'Gravity fill, Transfer re-abstraction scheme, Two-part tariff, and 56 returns exception', + frequencyCollected: 'weekly', + frequencyReported: 'monthly', + points: requirementTwo.points, + purposes: [ + 'Spray Irrigation - Storage' + ], + returnReference: requirementTwo.returnReference, + returnsCycle: 'Summer', + siteDescription: 'SECOND BOREHOLE AT AVALON', + title: 'SECOND BOREHOLE AT AVALON' + } + ], + startDate: '1 April 2022', + status: 'approved' + } + ) + }) + }) +}) From 1130418655ba55beb7a4da5a6734352508e707ce Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 19 Jun 2024 15:51:59 +0100 Subject: [PATCH 07/22] test: licence id and ref --- .../fetch-requirements-for-returns.service.js | 3 ++- .../fetch-requirements-for-returns.service.test.js | 1 + test/services/return-requirements/view.service.test.js | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/services/return-requirements/fetch-requirements-for-returns.service.js b/app/services/return-requirements/fetch-requirements-for-returns.service.js index 639f964091..0c839da985 100644 --- a/app/services/return-requirements/fetch-requirements-for-returns.service.js +++ b/app/services/return-requirements/fetch-requirements-for-returns.service.js @@ -38,7 +38,8 @@ async function _fetch (returnVersionId) { .withGraphFetched('licence') .modifyGraph('licence', (builder) => { builder.select([ - 'id' + 'id', + 'licenceRef' ]).modify('licenceHolder') }) .withGraphFetched('returnRequirements') diff --git a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js index 90d0bbbe68..9fc6dfc757 100644 --- a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js +++ b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js @@ -38,6 +38,7 @@ describe('Return Requirements - Fetch Requirements for returns service', () => { id: returnVersion.id, licence: { id: licence.id, + licenceRef: licence.licenceRef, licenceDocument: null }, multipleUpload: false, diff --git a/test/services/return-requirements/view.service.test.js b/test/services/return-requirements/view.service.test.js index e1afe791ba..eba17a463d 100644 --- a/test/services/return-requirements/view.service.test.js +++ b/test/services/return-requirements/view.service.test.js @@ -42,7 +42,7 @@ describe('Return Requirements - View service', () => { multipleUpload: 'No' }, licenceId: licence.id, - licenceRef: licence.ref, + licenceRef: result.licenceRef, notes: null, pageTitle: 'Check the requirements for returns for null', reason: 'New licence', From cc7e3a9ec8519692e194e211ec145bf058dfa1c9 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 19 Jun 2024 16:01:34 +0100 Subject: [PATCH 08/22] fix: unused arg --- app/presenters/return-requirements/view.presenter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/presenters/return-requirements/view.presenter.js b/app/presenters/return-requirements/view.presenter.js index 9c9e254b28..f5191b21a8 100644 --- a/app/presenters/return-requirements/view.presenter.js +++ b/app/presenters/return-requirements/view.presenter.js @@ -119,9 +119,9 @@ function _points (returnRequirementPoints) { }) } -function _requirements (requirements, points) { +function _requirements (requirements) { return requirements.map((requirement) => { - return _mapRequirement(requirement, points) + return _mapRequirement(requirement) }) } From a4c5fc1b2aaaf1b442d3bdb4e18ec5ee4e456646 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 20 Jun 2024 09:55:44 +0100 Subject: [PATCH 09/22] chore: update generatePointDetail jsdocs --- app/lib/general.lib.js | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/app/lib/general.lib.js b/app/lib/general.lib.js index 0fc1663364..fc8be5364f 100644 --- a/app/lib/general.lib.js +++ b/app/lib/general.lib.js @@ -153,31 +153,40 @@ function generateAbstractionPointDetail (pointDetail) { } /** - * Generate a string that represents an abstraction point based on the new data (obs change this in PR) + * Generate a string that represents an abstraction point based on the assumption the points have already been merged * + * 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. + * + * This follows the same out put as generateAbstractionPointDetail but the points have already been merged + * + * @param {Object} pointDetail - Object containing all the details for the point + * + * @returns {String} a description of the abstraction point */ -function generatePointDetail (point) { +function generatePointDetail (pointDetail) { let abstractionPoint = null - if (point.ngr4) { - const point1 = point.ngr1 - const point2 = point.ngr2 - const point3 = point.ngr3 - const point4 = point.ngr4 + if (pointDetail.ngr4) { + const point1 = pointDetail.ngr1 + const point2 = pointDetail.ngr2 + const point3 = pointDetail.ngr3 + const point4 = pointDetail.ngr4 abstractionPoint = `Within the area formed by the straight lines running between National Grid References ${point1} ${point2} ${point3} and ${point4}` - } else if (point.ngr2) { - const point1 = point.ngr1 - const point2 = point.ngr2 + } else if (pointDetail.ngr2) { + const point1 = pointDetail.ngr1 + const point2 = pointDetail.ngr2 abstractionPoint = `Between National Grid References ${point1} and ${point2}` } else { - const point1 = point.ngr1 + const point1 = pointDetail.ngr1 abstractionPoint = `At National Grid Reference ${point1}` } - abstractionPoint += point.description !== undefined ? ` (${point.description})` : '' + abstractionPoint += pointDetail.description !== undefined ? ` (${pointDetail.description})` : '' return abstractionPoint } From 5d9dd92f03feb096e5df6f8268ab76c1db90b320 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 20 Jun 2024 11:25:38 +0100 Subject: [PATCH 10/22] chore: pre pr check --- .../return-requirements/view.presenter.js | 13 +++++++------ .../fetch-requirements-for-returns.service.js | 8 ++++---- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app/presenters/return-requirements/view.presenter.js b/app/presenters/return-requirements/view.presenter.js index f5191b21a8..cefe3d7f68 100644 --- a/app/presenters/return-requirements/view.presenter.js +++ b/app/presenters/return-requirements/view.presenter.js @@ -1,7 +1,7 @@ 'use strict' /** - * Formats return requirements data for the `/return-requirements/{sessionId}/view` page + * Formats requirements for returns data for the `/return-requirements/{sessionId}/view` page * @module ViewPresenter */ @@ -13,17 +13,18 @@ const { returnRequirementReasons, returnRequirementFrequencies } = require('../. /** * Formats requirements for returns data for the `/return-requirements/{sessionId}/view` page * - * @param {ReturnVersionModel[]} requirementsForReturns - The return version inc licence and requirements + * @param {ReturnVersionModel[]} requirementsForReturns + * return version, licence, return requirements (requirement, points, purposes) * - * @returns {Object} returns requirement data needed by the view template + * @returns {Object} requirements for returns data needed by the view template */ function go (requirementsForReturns) { - const { returnRequirements, licence, reason, startDate, status, notes } = requirementsForReturns + const { licence, reason, notes, multipleUpload, returnRequirements, startDate, status } = requirementsForReturns return { additionalSubmissionOptions: { - multipleUpload: requirementsForReturns.multipleUpload === true ? 'Yes' : 'No' + multipleUpload: multipleUpload === true ? 'Yes' : 'No' }, licenceId: licence.id, licenceRef: licence.licenceRef, @@ -100,9 +101,9 @@ function _mapRequirement (requirement) { frequencyReported: returnRequirementFrequencies[requirement.reportingFrequency], points: _points(requirement.points), purposes: _purposes(requirement.purposes), + returnReference: requirement.legacyId, returnsCycle: requirement.summer === true ? 'Summer' : 'Winter and all year', siteDescription: requirement.siteDescription, - returnReference: requirement.legacyId, title: requirement.siteDescription } } diff --git a/app/services/return-requirements/fetch-requirements-for-returns.service.js b/app/services/return-requirements/fetch-requirements-for-returns.service.js index 0c839da985..25accb228b 100644 --- a/app/services/return-requirements/fetch-requirements-for-returns.service.js +++ b/app/services/return-requirements/fetch-requirements-for-returns.service.js @@ -10,13 +10,13 @@ const ReturnVersionModel = require('../../models/return-version.model.js') /** * Fetches requirements for returns by the return version id * - * Includes the licence - * - * Formats the requirements + * Includes the licence, return requirements (requirement, points, purposes) * * @param {string} returnVersionId - The UUID of the selected return version to get requirements for * - * @returns {Promise} The return version and its linked return requirements, plus their points and purposes + * @returns {Promise} + * The return version, licence, return requirements (requirement, points, purposes) + * */ async function go (returnVersionId) { const returnVersion = await _fetch(returnVersionId) From f4e757a355b7edfb3bbef7bbc1d4468f44717eba Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 20 Jun 2024 11:27:15 +0100 Subject: [PATCH 11/22] chore: pre pr check --- app/views/return-requirements/view.njk | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk index d05aa0be95..94b9995aee 100644 --- a/app/views/return-requirements/view.njk +++ b/app/views/return-requirements/view.njk @@ -10,7 +10,6 @@ {% endmacro %} {% block breadcrumbs %} - {# Back link #} {{ govukBackLink({ text: 'Back', href: '/system/licences/' + licenceId + '/set-up' @@ -18,7 +17,6 @@ {% endblock %} {% block content %} - {# Main heading #}

Licence {{ licenceRef }} @@ -190,5 +188,4 @@ ] }) }}

- {% endblock %} From 4b1ea51d0bcaa381664d6a2cfa655845f43f7efa Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 20 Jun 2024 11:30:49 +0100 Subject: [PATCH 12/22] chore: pre pr check --- .../return-requirements/view.presenter.test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/presenters/return-requirements/view.presenter.test.js b/test/presenters/return-requirements/view.presenter.test.js index 2822e93bad..bfb41e1bdb 100644 --- a/test/presenters/return-requirements/view.presenter.test.js +++ b/test/presenters/return-requirements/view.presenter.test.js @@ -46,25 +46,25 @@ describe('Return Requirements - View presenter', () => { gravityFill: true, id: '49a0aff3-268d-4b1d-870d-67acf3e3b0b5', legacyId: 12345, + points, + purposes, reabstraction: false, reportingFrequency: 'day', siteDescription: 'A place in the sun', summer: false, - twoPartTariff: false, - points, - purposes + twoPartTariff: false } requirementsForReturns = { id: '0d6f0f07-7d5c-48bf-90f0-d441680c3653', licence: { - id: 'c32ab7c6-e342-47b2-9c2e-d178ca89c5e5', - licenceRef: '02/01', $licenceHolder: () => { return 'Martha Stuart' }, + id: 'c32ab7c6-e342-47b2-9c2e-d178ca89c5e5', licenceDocument: { id: '4391032d-7165-49ab-98cd-245ae8b8e10d', licenceDocumentRoles: [] - } + }, + licenceRef: '02/01' }, multipleUpload: false, notes: 'A special note', @@ -163,7 +163,7 @@ describe('Return Requirements - View presenter', () => { }) }) - describe('the \'requirements\' property', () => { + describe('the "requirements" property', () => { it('correctly returns the requirement', () => { const result = ViewPresenter.go(requirementsForReturns) @@ -185,7 +185,7 @@ describe('Return Requirements - View presenter', () => { }]) }) - it('returns the purpose descriptions', () => { + it('returns the purposes descriptions', () => { const result = ViewPresenter.go(requirementsForReturns) expect(result.requirements[0].purposes).to.equal(['Transfer Between Sources (Pre Water Act 2003)']) From 04d3395629a5f4ccf102861d69395a91c197e11c Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 20 Jun 2024 11:32:36 +0100 Subject: [PATCH 13/22] chore: pre pr check --- .../fetch-requirements-for-returns.service.test.js | 4 +++- test/services/return-requirements/view.service.test.js | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js index 9fc6dfc757..721372b46b 100644 --- a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js +++ b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js @@ -31,9 +31,11 @@ describe('Return Requirements - Fetch Requirements for returns service', () => { returnVersion = await RequirementsForReturnsSeeder.seed(licence.id) }) - it('returns the details of the requirements for returns transformed for use in the view page', async () => { + it('returns the details of the requirements for returns', async () => { const result = await FetchRequirementsForReturnsService.go(returnVersion.id) + const [returnRequirementsOne, returnRequirementsTwo] = result.returnRequirements + expect(result).to.equal({ id: returnVersion.id, licence: { diff --git a/test/services/return-requirements/view.service.test.js b/test/services/return-requirements/view.service.test.js index eba17a463d..20c73fe499 100644 --- a/test/services/return-requirements/view.service.test.js +++ b/test/services/return-requirements/view.service.test.js @@ -78,8 +78,7 @@ describe('Return Requirements - View service', () => { ], startDate: '1 April 2022', status: 'approved' - } - ) + }) }) }) }) From 377f9216c4cebc00b7e7aaee52bd200381403ba8 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 10:43:11 +0100 Subject: [PATCH 14/22] refactor: notes to new design --- app/views/return-requirements/view.njk | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk index 94b9995aee..0bc6fc4162 100644 --- a/app/views/return-requirements/view.njk +++ b/app/views/return-requirements/view.njk @@ -57,21 +57,9 @@

Notes


- {{ govukSummaryList({ - classes: 'govuk-!-margin-bottom-2', - rows: [ - { - classes: 'govuk-summary-list govuk-summary-list__row--no-border', - key: { - text: userEmail, - classes: "govuk-body govuk-!-font-weight-regular" - }, - value: { - text: notes | escape | nl2br - } - } - ] - }) }} +

+ {{ notes | escape | nl2br }} +


{% endif %} From 5519d13a77fd1fd70fbd44b8ca6ea4c81e90a8c6 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 11:35:28 +0100 Subject: [PATCH 15/22] refactor: update check note section to new design --- .../return-requirements/check.presenter.js | 21 +++++- app/views/return-requirements/check.njk | 42 ++++------- .../check.presenter.test.js | 70 +++++++++---------- .../return-requirements/check.service.test.js | 13 +++- 4 files changed, 77 insertions(+), 69 deletions(-) diff --git a/app/presenters/return-requirements/check.presenter.js b/app/presenters/return-requirements/check.presenter.js index 3bd73cb155..2b091eb803 100644 --- a/app/presenters/return-requirements/check.presenter.js +++ b/app/presenters/return-requirements/check.presenter.js @@ -16,13 +16,28 @@ function go (session) { return { additionalSubmissionOptions: additionalSubmissionOptions ?? [], licenceRef: licence.licenceRef, - note: note ? note.content : null, + note: { + actions: _noteActions(note), + text: note ? note.content : 'No notes added' + }, pageTitle: `Check the requirements for returns for ${licence.licenceHolder}`, reason: returnRequirementReasons[reason], reasonLink: _reasonLink(sessionId, returnsRequired), sessionId, - startDate: _startDate(session), - userEmail: note ? note.userEmail : 'No notes added' + startDate: _startDate(session) + } +} + +function _noteActions (note) { + if (note && note.content) { + return [ + { text: 'Change', href: 'note' }, + { text: 'Delete', href: 'delete-note' } + ] + } else { + return [ + { text: 'Add a note', href: 'note' } + ] } } diff --git a/app/views/return-requirements/check.njk b/app/views/return-requirements/check.njk index cb24e4d706..42094e3bc6 100644 --- a/app/views/return-requirements/check.njk +++ b/app/views/return-requirements/check.njk @@ -75,34 +75,20 @@

Notes


- {{ govukSummaryList({ - classes: 'govuk-!-margin-bottom-2', - rows: [ - { - classes: 'govuk-summary-list govuk-summary-list__row--no-border', - key: { - text: userEmail, - classes: "govuk-body govuk-!-font-weight-regular" - }, - value: { - text: note | escape | nl2br - }, - actions: { - items: [ - { - attributes: { 'data-test': 'note' }, - href: "note", - text: "Change" if note else "Add a note" - }, - { - href: "delete-note", - text: "Delete" - } if note - ] - } - } - ] - }) }} +
+

{{ note.text | escape }}

+ + + +

diff --git a/test/presenters/return-requirements/check.presenter.test.js b/test/presenters/return-requirements/check.presenter.test.js index 1b4fcfddd5..d8cbd03b67 100644 --- a/test/presenters/return-requirements/check.presenter.test.js +++ b/test/presenters/return-requirements/check.presenter.test.js @@ -38,13 +38,20 @@ describe('Return Requirements - Check presenter', () => { expect(result).to.equal({ additionalSubmissionOptions: [], licenceRef: '01/ABC', - note: null, + note: { + actions: [ + { + href: 'note', + text: 'Add a note' + } + ], + text: 'No notes added' + }, pageTitle: 'Check the requirements for returns for Turbo Kid', reason: 'Major change', reasonLink: '/system/return-requirements/61e07498-f309-4829-96a9-72084a54996d/reason', sessionId: '61e07498-f309-4829-96a9-72084a54996d', - startDate: '1 January 2023', - userEmail: 'No notes added' + startDate: '1 January 2023' }) }) }) @@ -75,23 +82,42 @@ describe('Return Requirements - Check presenter', () => { describe('when the user has added a note', () => { beforeEach(() => { session.note = { - content: 'Note attached to requirement', - userEmail: 'carol.shaw@atari.com' + content: 'Note attached to requirement' } }) - it('returns a populated note', () => { + it('returns text with the note content and the change and delete a note action', () => { const result = CheckPresenter.go(session) - expect(result.note).to.equal('Note attached to requirement') + expect(result.note).to.equal({ + actions: [ + { + href: 'note', + text: 'Change' + }, + { + href: 'delete-note', + text: 'Delete' + } + ], + text: 'Note attached to requirement' + }) }) }) describe('when the user has not added a note', () => { - it('returns an empty note', () => { + it('returns text with "No notes added" and the add a note action', () => { const result = CheckPresenter.go(session) - expect(result.note).to.be.null() + expect(result.note).to.equal({ + actions: [ + { + href: 'note', + text: 'Add a note' + } + ], + text: 'No notes added' + }) }) }) }) @@ -158,30 +184,4 @@ describe('Return Requirements - Check presenter', () => { }) }) }) - - describe('the "userEmail" property', () => { - describe('when the user has added a note', () => { - beforeEach(() => { - session.note = { - content: 'Note attached to requirement', - status: 'Added', - userEmail: 'carol.shaw@atari.com' - } - }) - - it("returns a the user's email address", () => { - const result = CheckPresenter.go(session) - - expect(result.userEmail).to.equal('carol.shaw@atari.com') - }) - }) - - describe('when the user has not added a note', () => { - it('returns the message "no notes added"', () => { - const result = CheckPresenter.go(session) - - expect(result.userEmail).to.equal('No notes added') - }) - }) - }) }) diff --git a/test/services/return-requirements/check.service.test.js b/test/services/return-requirements/check.service.test.js index a565fac62d..f7f9231d18 100644 --- a/test/services/return-requirements/check.service.test.js +++ b/test/services/return-requirements/check.service.test.js @@ -67,15 +67,22 @@ describe('Return Requirements - Check service', () => { activeNavBar: 'search', additionalSubmissionOptions: [], licenceRef: '01/ABC', - note: null, + note: { + actions: [ + { + href: 'note', + text: 'Add a note' + } + ], + text: 'No notes added' + }, notification: undefined, pageTitle: 'Check the requirements for returns for Turbo Kid', reason: 'Major change', reasonLink: `/system/return-requirements/${session.id}/reason`, requirements: [], returnsRequired: true, - startDate: '1 January 2023', - userEmail: 'No notes added' + startDate: '1 January 2023' }, { skip: ['sessionId'] }) }) From ed0e2471e18d168dda2a686e704045d8be272296 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 11:40:53 +0100 Subject: [PATCH 16/22] feat: add created by and at --- .../return-requirements/view.presenter.js | 7 +++++-- .../fetch-requirements-for-returns.service.js | 2 ++ app/views/return-requirements/view.njk | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/app/presenters/return-requirements/view.presenter.js b/app/presenters/return-requirements/view.presenter.js index cefe3d7f68..f98bf8c533 100644 --- a/app/presenters/return-requirements/view.presenter.js +++ b/app/presenters/return-requirements/view.presenter.js @@ -20,7 +20,8 @@ const { returnRequirementReasons, returnRequirementFrequencies } = require('../. */ function go (requirementsForReturns) { - const { licence, reason, notes, multipleUpload, returnRequirements, startDate, status } = requirementsForReturns + const { createdAt, createdBy, licence, reason, notes, multipleUpload, returnRequirements, startDate, status } = + requirementsForReturns return { additionalSubmissionOptions: { @@ -33,7 +34,9 @@ function go (requirementsForReturns) { reason: returnRequirementReasons[reason] || '', requirements: _requirements(returnRequirements), startDate: formatLongDate(startDate), - status: _status(status) + status: _status(status), + createdDate: formatLongDate(createdAt), + createdBy: createdBy || '' } } diff --git a/app/services/return-requirements/fetch-requirements-for-returns.service.js b/app/services/return-requirements/fetch-requirements-for-returns.service.js index 25accb228b..f732add2ee 100644 --- a/app/services/return-requirements/fetch-requirements-for-returns.service.js +++ b/app/services/return-requirements/fetch-requirements-for-returns.service.js @@ -28,6 +28,8 @@ async function _fetch (returnVersionId) { return ReturnVersionModel.query() .findById(returnVersionId) .select([ + 'createdAt', + 'createdBy', 'id', 'multiple_upload', 'notes', diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk index 0bc6fc4162..0a2a3dbdbe 100644 --- a/app/views/return-requirements/view.njk +++ b/app/views/return-requirements/view.njk @@ -47,6 +47,24 @@ value: { html: '' + reason + '' } + }, + { + classes: 'govuk-summary-list govuk-summary-list__row--no-border', + key: { + text: "Created date" + }, + value: { + html: '' + createdDate + '' + } + }, + { + classes: 'govuk-summary-list govuk-summary-list__row--no-border', + key: { + text: "Created by" + }, + value: { + html: '' + createdBy + '' + } } ] }) }} From 334894289b7fdb3b1f507b85538760557d4cd097 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 12:08:37 +0100 Subject: [PATCH 17/22] feat: add created by and at --- .../return-requirements/view.presenter.js | 4 +-- .../fetch-requirements-for-returns.service.js | 7 ++++- .../view.presenter.test.js | 31 +++++++++++++++++++ ...h-requirements-for-returns.service.test.js | 4 ++- .../return-requirements/view.service.test.js | 2 ++ 5 files changed, 44 insertions(+), 4 deletions(-) diff --git a/app/presenters/return-requirements/view.presenter.js b/app/presenters/return-requirements/view.presenter.js index f98bf8c533..83479b9829 100644 --- a/app/presenters/return-requirements/view.presenter.js +++ b/app/presenters/return-requirements/view.presenter.js @@ -20,7 +20,7 @@ const { returnRequirementReasons, returnRequirementFrequencies } = require('../. */ function go (requirementsForReturns) { - const { createdAt, createdBy, licence, reason, notes, multipleUpload, returnRequirements, startDate, status } = + const { createdAt, licence, reason, notes, multipleUpload, returnRequirements, startDate, status, user } = requirementsForReturns return { @@ -36,7 +36,7 @@ function go (requirementsForReturns) { startDate: formatLongDate(startDate), status: _status(status), createdDate: formatLongDate(createdAt), - createdBy: createdBy || '' + createdBy: user ? user.username : '' } } diff --git a/app/services/return-requirements/fetch-requirements-for-returns.service.js b/app/services/return-requirements/fetch-requirements-for-returns.service.js index f732add2ee..748ed56e58 100644 --- a/app/services/return-requirements/fetch-requirements-for-returns.service.js +++ b/app/services/return-requirements/fetch-requirements-for-returns.service.js @@ -29,7 +29,6 @@ async function _fetch (returnVersionId) { .findById(returnVersionId) .select([ 'createdAt', - 'createdBy', 'id', 'multiple_upload', 'notes', @@ -37,6 +36,12 @@ async function _fetch (returnVersionId) { 'startDate', 'status' ]) + .withGraphFetched('user') + .modifyGraph('user', (builder) => { + builder.select([ + 'username' + ]) + }) .withGraphFetched('licence') .modifyGraph('licence', (builder) => { builder.select([ diff --git a/test/presenters/return-requirements/view.presenter.test.js b/test/presenters/return-requirements/view.presenter.test.js index bfb41e1bdb..069c421854 100644 --- a/test/presenters/return-requirements/view.presenter.test.js +++ b/test/presenters/return-requirements/view.presenter.test.js @@ -56,6 +56,7 @@ describe('Return Requirements - View presenter', () => { } requirementsForReturns = { + createdAt: new Date('2020-12-01'), id: '0d6f0f07-7d5c-48bf-90f0-d441680c3653', licence: { $licenceHolder: () => { return 'Martha Stuart' }, @@ -83,6 +84,8 @@ describe('Return Requirements - View presenter', () => { additionalSubmissionOptions: { multipleUpload: 'No' }, + createdBy: '', + createdDate: '1 December 2020', licenceId: 'c32ab7c6-e342-47b2-9c2e-d178ca89c5e5', licenceRef: '02/01', notes: 'A special note', @@ -111,6 +114,34 @@ describe('Return Requirements - View presenter', () => { }) }) + describe('the "createdBy" property', () => { + describe('and there is no user linked to the return', () => { + it('returns an empty string', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.createdBy).to.equal('') + }) + }) + describe('and there is a user linked to the return', () => { + beforeEach(() => { + requirementsForReturns.user = { username: 'iron@man.net' } + }) + it('returns the users username', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.createdBy).to.equal('iron@man.net') + }) + }) + }) + + describe('the "createdDate" property', () => { + it('returns created date', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.createdDate).to.equal('1 December 2020') + }) + }) + describe('the "notes" property', () => { it('returns notes', () => { const result = ViewPresenter.go(requirementsForReturns) diff --git a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js index 721372b46b..82996ed33e 100644 --- a/test/services/return-requirements/fetch-requirements-for-returns.service.test.js +++ b/test/services/return-requirements/fetch-requirements-for-returns.service.test.js @@ -37,6 +37,7 @@ describe('Return Requirements - Fetch Requirements for returns service', () => { const [returnRequirementsOne, returnRequirementsTwo] = result.returnRequirements expect(result).to.equal({ + createdAt: returnVersion.createdAt, id: returnVersion.id, licence: { id: licence.id, @@ -115,7 +116,8 @@ describe('Return Requirements - Fetch Requirements for returns service', () => { } ], startDate: new Date('2022-04-01'), - status: 'current' + status: 'current', + user: null }) }) }) diff --git a/test/services/return-requirements/view.service.test.js b/test/services/return-requirements/view.service.test.js index 20c73fe499..941bf3214b 100644 --- a/test/services/return-requirements/view.service.test.js +++ b/test/services/return-requirements/view.service.test.js @@ -41,6 +41,8 @@ describe('Return Requirements - View service', () => { additionalSubmissionOptions: { multipleUpload: 'No' }, + createdBy: '', + createdDate: returnVersion.createdAt.toLocaleDateString('en-GB', { year: 'numeric', month: 'long', day: 'numeric' }), licenceId: licence.id, licenceRef: result.licenceRef, notes: null, From 557614de1007fe133847a1ac2472bdc45ae7c92b Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 14:12:03 +0100 Subject: [PATCH 18/22] fix: sonar cloud optional chaining --- app/presenters/return-requirements/check.presenter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/presenters/return-requirements/check.presenter.js b/app/presenters/return-requirements/check.presenter.js index 2b091eb803..a4d00884b1 100644 --- a/app/presenters/return-requirements/check.presenter.js +++ b/app/presenters/return-requirements/check.presenter.js @@ -29,7 +29,7 @@ function go (session) { } function _noteActions (note) { - if (note && note.content) { + if (note?.content) { return [ { text: 'Change', href: 'note' }, { text: 'Delete', href: 'delete-note' } From 8182deb0c7e25a253934dd8aa97b3ce1ee17a2d7 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 14:19:39 +0100 Subject: [PATCH 19/22] fix: use nlbr --- app/views/return-requirements/check.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/return-requirements/check.njk b/app/views/return-requirements/check.njk index 42094e3bc6..b05ca5ebb2 100644 --- a/app/views/return-requirements/check.njk +++ b/app/views/return-requirements/check.njk @@ -76,7 +76,7 @@

Notes


-

{{ note.text | escape }}

+

{{ note.text | escape | nl2br }}

    {% for action in note.actions %} From 2cf2f620bc71c0ec570ac3afdd9084c6a512b68e Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 14:26:29 +0100 Subject: [PATCH 20/22] feat: handle empty returnRequirements --- app/views/return-requirements/view.njk | 5 +++++ .../return-requirements/view.presenter.test.js | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk index 0a2a3dbdbe..8c801f537a 100644 --- a/app/views/return-requirements/view.njk +++ b/app/views/return-requirements/view.njk @@ -81,6 +81,7 @@
{% endif %} +{% if requirements.length > 0 %}

Requirements for returns

{% for requirement in requirements %} @@ -194,4 +195,8 @@ ] }) }}
+ {% else %} +

Returns are not required for this licence

+
+ {% endif %} {% endblock %} diff --git a/test/presenters/return-requirements/view.presenter.test.js b/test/presenters/return-requirements/view.presenter.test.js index 069c421854..568022f063 100644 --- a/test/presenters/return-requirements/view.presenter.test.js +++ b/test/presenters/return-requirements/view.presenter.test.js @@ -322,6 +322,18 @@ describe('Return Requirements - View presenter', () => { expect(result.pageTitle).to.equal('Check the requirements for returns for Martha Stuart') }) }) + + describe('and there are no return requirements', () => { + beforeEach(() => { + requirementsForReturns.returnRequirements = [] + }) + + it('should return an empty array', () => { + const result = ViewPresenter.go(requirementsForReturns) + + expect(result.requirements).to.equal([]) + }) + }) }) }) }) From cce406c2b6eda89933a416e02fbcbc5c59215453 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 14:30:14 +0100 Subject: [PATCH 21/22] fix: double not content check --- .../return-requirements/check.presenter.js | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/app/presenters/return-requirements/check.presenter.js b/app/presenters/return-requirements/check.presenter.js index a4d00884b1..d1479f3191 100644 --- a/app/presenters/return-requirements/check.presenter.js +++ b/app/presenters/return-requirements/check.presenter.js @@ -16,10 +16,7 @@ function go (session) { return { additionalSubmissionOptions: additionalSubmissionOptions ?? [], licenceRef: licence.licenceRef, - note: { - actions: _noteActions(note), - text: note ? note.content : 'No notes added' - }, + note: _note(note), pageTitle: `Check the requirements for returns for ${licence.licenceHolder}`, reason: returnRequirementReasons[reason], reasonLink: _reasonLink(sessionId, returnsRequired), @@ -28,16 +25,22 @@ function go (session) { } } -function _noteActions (note) { +function _note (note) { if (note?.content) { - return [ - { text: 'Change', href: 'note' }, - { text: 'Delete', href: 'delete-note' } - ] + return { + actions: [ + { text: 'Change', href: 'note' }, + { text: 'Delete', href: 'delete-note' } + ], + text: note.content + } } else { - return [ - { text: 'Add a note', href: 'note' } - ] + return { + actions: [ + { text: 'Add a note', href: 'note' } + ], + text: 'No notes added' + } } } From d52f5b84d481e820c0ce3c8552bd2f82e27c6f13 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 21 Jun 2024 14:31:45 +0100 Subject: [PATCH 22/22] fix: pre pr check --- app/views/return-requirements/view.njk | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk index 8c801f537a..458ce50972 100644 --- a/app/views/return-requirements/view.njk +++ b/app/views/return-requirements/view.njk @@ -81,6 +81,7 @@
{% endif %} + {% if requirements.length > 0 %}

Requirements for returns