From d27c048c3b0dc636cf33480cf5fcb53d5f3a2622 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 31 May 2024 11:50:29 +0100 Subject: [PATCH] Check your requirements - purposes Due to the estimated complexity of adding fetch services for purposes, points and other display items. We have broken down the complex display items into their own branches of work. https://eaflood.atlassian.net/browse/WATER-4386 This branch will add purposes to the return requirement summary card Previous work done here - https://github.com/DEFRA/water-abstraction-system/pull/1019 --- .../return-requirements/check.presenter.js | 48 +-- .../return-requirements/check.service.js | 5 + .../check/fetch-purposes.service.js | 32 ++ .../check/returns-requirements.presenter.js | 65 +++ .../check/returns-requirements.service.js | 44 ++ app/views/return-requirements/check.njk | 407 +++++++++--------- .../check.presenter.test.js | 64 +-- .../returns-requirements.service.test.js | 102 +++++ 8 files changed, 462 insertions(+), 305 deletions(-) create mode 100644 app/services/return-requirements/check/fetch-purposes.service.js create mode 100644 app/services/return-requirements/check/returns-requirements.presenter.js create mode 100644 app/services/return-requirements/check/returns-requirements.service.js create mode 100644 test/presenters/return-requirements/check/returns-requirements.service.test.js diff --git a/app/presenters/return-requirements/check.presenter.js b/app/presenters/return-requirements/check.presenter.js index c8f62ab9ac..d56536da75 100644 --- a/app/presenters/return-requirements/check.presenter.js +++ b/app/presenters/return-requirements/check.presenter.js @@ -5,71 +5,35 @@ * @module CheckPresenter */ -const { formatAbstractionDate, formatLongDate } = require('../base.presenter.js') +const { formatLongDate } = require('../base.presenter.js') const { returnRequirementReasons } = require('../../lib/static-lookups.lib.js') function go (session) { const { additionalSubmissionOptions, id: sessionId, journey, licence, note, reason } = session + const returnsRequired = journey === 'returns-required' + return { additionalSubmissionOptions: additionalSubmissionOptions ?? [], - journey, licenceRef: licence.licenceRef, note: note ? note.content : null, pageTitle: `Check the return requirements for ${licence.licenceHolder}`, reason: returnRequirementReasons[reason], - reasonLink: _reasonLink(sessionId, journey), - requirements: _requirements(session), + reasonLink: _reasonLink(sessionId, returnsRequired), sessionId, startDate: _startDate(session), userEmail: note ? note.userEmail : 'No notes added' } } -function _abstractionPeriod (abstractionPeriod) { - const { 'start-abstraction-period-day': startDay, 'start-abstraction-period-month': startMonth, 'end-abstraction-period-day': endDay, 'end-abstraction-period-month': endMonth } = abstractionPeriod - const startDate = formatAbstractionDate(startDay, startMonth) - const endDate = formatAbstractionDate(endDay, endMonth) - - return `From ${startDate} to ${endDate}` -} - -function _reasonLink (sessionId, journey) { - if (journey === 'returns-required') { +function _reasonLink (sessionId, returnsRequired) { + if (returnsRequired) { return `/system/return-requirements/${sessionId}/reason` } return `/system/return-requirements/${sessionId}/no-returns-required` } -function _requirements (session) { - const { requirements } = session - - const completedRequirements = [] - - for (const [index, requirement] of requirements.entries()) { - const { agreementsExceptions } = requirement - // NOTE: We determine a requirement is complete because agreement exceptions is populated and it is the last step in - // the journey - if (agreementsExceptions) { - completedRequirements.push(_mapRequirement(requirement, index)) - } - } - - return completedRequirements -} - -function _mapRequirement (requirement, index) { - return { - abstractionPeriod: _abstractionPeriod(requirement.abstractionPeriod), - frequencyCollected: requirement.frequencyCollected, - frequencyReported: requirement.frequencyReported, - index, - purposes: 'purpose', - siteDescription: requirement.siteDescription - } -} - function _startDate (session) { const { licence, startDateOptions, startDateDay, startDateMonth, startDateYear } = session diff --git a/app/services/return-requirements/check.service.js b/app/services/return-requirements/check.service.js index 49846c79ea..848e140081 100644 --- a/app/services/return-requirements/check.service.js +++ b/app/services/return-requirements/check.service.js @@ -8,6 +8,8 @@ const CheckPresenter = require('../../presenters/return-requirements/check.presenter.js') const SessionModel = require('../../models/session.model.js') +const RequirementsService = require('./check/returns-requirements.service.js') + /** * Orchestrates fetching and presenting the data for `/return-requirements/{sessionId}/check` page * @@ -21,6 +23,8 @@ async function go (sessionId, yar) { await _markCheckPageVisited(session) + const requirements = await RequirementsService.go(sessionId) + const formattedData = CheckPresenter.go(session) const notification = yar.flash('notification')[0] @@ -28,6 +32,7 @@ async function go (sessionId, yar) { return { activeNavBar: 'search', notification, + ...requirements, ...formattedData } } diff --git a/app/services/return-requirements/check/fetch-purposes.service.js b/app/services/return-requirements/check/fetch-purposes.service.js new file mode 100644 index 0000000000..3c58c2a235 --- /dev/null +++ b/app/services/return-requirements/check/fetch-purposes.service.js @@ -0,0 +1,32 @@ +'use strict' + +/** + * Fetches a licence's purposes needed for `/return-requirements/{sessionId}/purpose` page + * @module FetchPurposesByIdsService + */ + +const PurposeModel = require('../../../models/purpose.model.js') + +/** + * Fetches a licence's purposes needed for `/return-requirements/{sessionId}/purpose` page + * + * @param {[string]} purposeIds - An array of ids to fetch + * + * @returns {Promise} The purposes matching the array of id's + */ +async function go (purposeIds) { + return _fetch(purposeIds) +} + +async function _fetch (purposeIds) { + return PurposeModel.query() + .select([ + 'purposes.id', + 'purposes.description' + ]) + .findByIds(purposeIds) +} + +module.exports = { + go +} diff --git a/app/services/return-requirements/check/returns-requirements.presenter.js b/app/services/return-requirements/check/returns-requirements.presenter.js new file mode 100644 index 0000000000..3dcb1fe371 --- /dev/null +++ b/app/services/return-requirements/check/returns-requirements.presenter.js @@ -0,0 +1,65 @@ +'use strict' + +const { formatAbstractionDate } = require('../../../presenters/base.presenter.js') + +/** + * Formats data for the `/return-requirements/{sessionId}/check` page + * @module ReturnRequirementsPresenter + */ + +function go (requirements, purposeIds, journey) { + // Not clear of this can be an if else + const returnsRequired = journey === 'returns-required' + const noReturnsRequired = journey === 'no-returns-required' + + return { + returnsRequired, + noReturnsRequired, + requirements: _requirements(requirements, purposeIds) + } +} + +function _abstractionPeriod (abstractionPeriod) { + const { 'start-abstraction-period-day': startDay, 'start-abstraction-period-month': startMonth, 'end-abstraction-period-day': endDay, 'end-abstraction-period-month': endMonth } = abstractionPeriod + const startDate = formatAbstractionDate(startDay, startMonth) + const endDate = formatAbstractionDate(endDay, endMonth) + + return `From ${startDate} to ${endDate}` +} +function _requirements (requirements, purposeIds) { + const completedRequirements = [] + + for (const [index, requirement] of requirements.entries()) { + const { agreementsExceptions } = requirement + // NOTE: We determine a requirement is complete because agreement exceptions is populated and it is the last step in + // the journey + if (agreementsExceptions) { + completedRequirements.push(_mapRequirement(requirement, index, purposeIds)) + } + } + + return completedRequirements +} + +function _mapRequirement (requirement, index, purposeIds) { + return { + abstractionPeriod: _abstractionPeriod(requirement.abstractionPeriod), + frequencyCollected: requirement.frequencyCollected, + frequencyReported: requirement.frequencyReported, + index, + purposes: _mapPurpoes(requirement.purposes, purposeIds), + siteDescription: requirement.siteDescription + } +} + +function _mapPurpoes (purposes, purposeIds) { + const uniquePurposes = [...new Set(purposes)] + + return uniquePurposes.map((purpose) => { + return purposeIds.find((pid) => { return pid.id === purpose }).description + }) +} + +module.exports = { + go +} diff --git a/app/services/return-requirements/check/returns-requirements.service.js b/app/services/return-requirements/check/returns-requirements.service.js new file mode 100644 index 0000000000..da75da0fa9 --- /dev/null +++ b/app/services/return-requirements/check/returns-requirements.service.js @@ -0,0 +1,44 @@ +'use strict' + +/** + * Orchestrates fetching and presenting the requirements for the check page + * @module RequirementsService + */ + +const FetchPurposesByIdsService = require('./fetch-purposes.service.js') +const ReturnRequirementsPresenter = require('./returns-requirements.presenter.js') +const SessionModel = require('../../../models/session.model.js') + +/** + * Orchestrates fetching and presenting the requirements for `/return-requirements/{sessionId}/check` page + * + * @param {string} sessionId - The UUID for return requirement setup session record + * + * @returns {Promise} page data needed by the view template + */ +async function go (sessionId) { + const session = await SessionModel.query().findById(sessionId) + + const { requirements, journey } = session + const purposeIds = _purposeIds(requirements) + const purposes = await FetchPurposesByIdsService.go(purposeIds) + + return ReturnRequirementsPresenter.go(requirements, purposes, journey) +} + +function _purposeIds (requirements) { + const requirementPurposes = requirements.flatMap((requirement) => { + if (requirement.purposes) { + return requirement.purposes + } + + return [] + }) + + // Duplicate is a bug this is temp + return [...new Set(requirementPurposes)] +} + +module.exports = { + go +} diff --git a/app/views/return-requirements/check.njk b/app/views/return-requirements/check.njk index 05d6c7e037..4210e29237 100644 --- a/app/views/return-requirements/check.njk +++ b/app/views/return-requirements/check.njk @@ -3,6 +3,13 @@ {% from "govuk/components/notification-banner/macro.njk" import govukNotificationBanner %} {% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} + +{% macro newLine(array) %} + {% for item in array %} +

{{ item }}

+ {% endfor %} +{% endmacro %} + {% block content %} {% if notification %} {{ govukNotificationBanner({ @@ -27,38 +34,38 @@ { classes: 'govuk-summary-list govuk-summary-list__row--no-border', key: { - text: "Start date" - }, + text: "Start date" + }, value: { - text: startDate - }, + text: startDate + }, actions: { - items: [ - { - href: "/system/return-requirements/" + sessionId + "/start-date", - text: "Change", - visuallyHiddenText: "the start date for the return requirement" - } - ] - } + items: [ + { + href: "/system/return-requirements/" + sessionId + "/start-date", + text: "Change", + visuallyHiddenText: "the start date for the return requirement" + } + ] + } }, { classes: 'govuk-summary-list govuk-summary-list__row--no-border', key: { - text: "Reason" - }, + text: "Reason" + }, value: { - text: reason - }, + text: reason + }, actions: { - items: [ - { - href: reasonLink, - text: "Change", - visuallyHiddenText: "the reason for the return requirement" - } - ] - } + items: [ + { + href: reasonLink, + text: "Change", + visuallyHiddenText: "the reason for the return requirement" + } + ] + } } ] }) }} @@ -66,7 +73,7 @@
-

Notes

+

Notes

{{ govukSummaryList({ classes: 'govuk-!-margin-bottom-2', @@ -74,34 +81,34 @@ { classes: 'govuk-summary-list govuk-summary-list__row--no-border', key: { - text: userEmail, - classes: "govuk-body govuk-!-font-weight-regular" - }, + text: userEmail, + classes: "govuk-body govuk-!-font-weight-regular" + }, value: { - text: note | escape | nl2br - }, + text: note | escape | nl2br + }, actions: { - items: [ - { - href: "note", - text: "Change" if note else "Add a note" - }, - { - href: "delete-note", - text: "Delete" - } if note - ] - } + items: [ + { + href: "note", + text: "Change" if note else "Add a note" + }, + { + href: "delete-note", + text: "Delete" + } if note + ] + } } ] }) }}
- {% if journey == 'returns-required' %} + {% if returnsRequired %}
-

Requirements for returns

-
+

Requirements for returns

+ {{ govukButton({ text: "Add another requirement", classes: "govuk-button--secondary", @@ -110,152 +117,152 @@
{% for requirement in requirements %}
- {{ govukSummaryList({ - card: { - title: { - text: requirement.siteDescription - } + {{ govukSummaryList({ + card: { + title: { + text: requirement.siteDescription + } + }, + classes: "govuk-summary-list--no-border", + rows: [ + { + key: { + text: "Purpose" }, - classes: "govuk-summary-list--no-border", - rows: [ - { - key: { - text: "Purpose" - }, - value: { - html: "requirement.purposes" - }, - actions: { - items: [ - { - href: "purpose/" + requirement.index , - text: "Change", - visuallyHiddenText: "Purpose" - } - ] + value: { + html: newLine(requirement.purposes) + }, + actions: { + items: [ + { + href: "purpose/" + requirement.index , + text: "Change", + visuallyHiddenText: "Purpose" } - }, - { - key: { - text: "Point" - }, - value: { - html: "requirement.points" - }, - actions: { - items: [ - { - href: "points/" + requirement.index , - text: "Change", - visuallyHiddenText: "Point" - } - ] + ] + } + }, + { + key: { + text: "Point" + }, + value: { + html: "requirement.points" + }, + actions: { + items: [ + { + href: "points/" + requirement.index , + text: "Change", + visuallyHiddenText: "Point" } - }, - { - key: { - text: "Abstraction period" - }, - value: { - html: requirement.abstractionPeriod - }, - actions: { - items: [ - { - href: "abstraction-period/" + requirement.index , - text: "Change", - visuallyHiddenText: "Abstraction period" - } - ] + ] + } + }, + { + key: { + text: "Abstraction period" + }, + value: { + html: requirement.abstractionPeriod + }, + actions: { + items: [ + { + href: "abstraction-period/" + requirement.index , + text: "Change", + visuallyHiddenText: "Abstraction period" } - }, - { - key: { - text: "Returns cycle" - }, - value: { - html: "requirement.returnsCycle" - }, - actions: { - items: [ - { - href: "returns-cycle/" + requirement.index , - text: "Change", - visuallyHiddenText: "Returns cycle" - } - ] + ] + } + }, + { + key: { + text: "Returns cycle" + }, + value: { + html: "requirement.returnsCycle" + }, + actions: { + items: [ + { + href: "returns-cycle/" + requirement.index , + text: "Change", + visuallyHiddenText: "Returns cycle" } - }, - { - key: { - text: "Site description" - }, - value: { - html: requirement.siteDescription - }, - actions: { - items: [ - { - href: "site-description/" + requirement.index , - text: "Change", - visuallyHiddenText: "Site description" - } - ] + ] + } + }, + { + key: { + text: "Site description" + }, + value: { + html: requirement.siteDescription + }, + actions: { + items: [ + { + href: "site-description/" + requirement.index , + text: "Change", + visuallyHiddenText: "Site description" } - }, - { - key: { - text: "Collection" - }, - value: { - html: requirement.frequencyCollected | capitalize - }, - actions: { - items: [ - { - href: "frequency-collected/" + requirement.index , - text: "Change", - visuallyHiddenText: "Collection" - } - ] + ] + } + }, + { + key: { + text: "Collection" + }, + value: { + html: requirement.frequencyCollected | capitalize + }, + actions: { + items: [ + { + href: "frequency-collected/" + requirement.index , + text: "Change", + visuallyHiddenText: "Collection" } - }, - { - key: { - text: "Reporting" - }, - value: { - html: requirement.frequencyReported | capitalize - }, - actions: { - items: [ - { - href: "frequency-reported/" + requirement.index , - text: "Change", - visuallyHiddenText: "Reporting" - } - ] + ] + } + }, + { + key: { + text: "Reporting" + }, + value: { + html: requirement.frequencyReported | capitalize + }, + actions: { + items: [ + { + href: "frequency-reported/" + requirement.index , + text: "Change", + visuallyHiddenText: "Reporting" } - }, - { - key: { - text: "Agreements exceptions" - }, - value: { - html: "requirement.agreementsExceptions" - }, - actions: { - items: [ - { - href: "agreements-exceptions/" + requirement.index , - text: "Change", - visuallyHiddenText: "Agreements exception" - } - ] + ] + } + }, + { + key: { + text: "Agreements exceptions" + }, + value: { + html: "requirement.agreementsExceptions" + }, + actions: { + items: [ + { + href: "agreements-exceptions/" + requirement.index , + text: "Change", + visuallyHiddenText: "Agreements exception" } - } - ] - }) }} + ] + } + } + ] + }) }} {% if requirements.length >= 2 %} {{ govukButton({ text: "Remove requirement", @@ -271,41 +278,41 @@
- {% if journey == 'no-returns-required' %} + {% if noReturnsRequired %}

Returns are not required for this licence

{% endif %} - {% if journey == 'returns-required' %} + {% if returnsRequired %} {{ govukSummaryList({ classes: 'govuk-!-margin-bottom-2', rows: [ { classes: 'govuk-summary-list', key: { - text: 'Additional submission options', - classes: "govuk-heading-m govuk-!-width-one-half" - }, + text: 'Additional submission options', + classes: "govuk-heading-m govuk-!-width-one-half" + }, value: { - text: '' - }, + text: '' + }, actions: { - items: [ - { - href: "additional-submission-options", - text: "Change" - } - ] - } + items: [ + { + href: "additional-submission-options", + text: "Change" + } + ] + } }, { classes: 'govuk-summary-list govuk-summary-list__row--no-border', key: { - text: 'Multiple upload', - classes: "govuk-body " - }, + text: 'Multiple upload', + classes: "govuk-body " + }, value: { - text: 'Yes' if additionalSubmissionOptions.includes('multiple-upload') else "No" - } + text: 'Yes' if additionalSubmissionOptions.includes('multiple-upload') else "No" + } } ] }) }} diff --git a/test/presenters/return-requirements/check.presenter.test.js b/test/presenters/return-requirements/check.presenter.test.js index 8dc5ff9b3c..9b7e29432f 100644 --- a/test/presenters/return-requirements/check.presenter.test.js +++ b/test/presenters/return-requirements/check.presenter.test.js @@ -12,34 +12,12 @@ const CheckPresenter = require('../../../app/presenters/return-requirements/chec describe('Return Requirements - Check presenter', () => { let session - let requirement beforeEach(() => { - requirement = { - points: [ - '286' - ], - purposes: [ - '772136d1-9184-417b-90cd-91053287d1df' - ], - returnsCycle: 'summer', - siteDescription: 'A place in the sun', - abstractionPeriod: { - 'end-abstraction-period-day': '01', - 'end-abstraction-period-month': '03', - 'start-abstraction-period-day': '01', - 'start-abstraction-period-month': '06' - }, - frequencyReported: 'daily', - frequencyCollected: 'daily', - agreementsExceptions: [ - 'gravity-fill' - ] - } - session = { id: '61e07498-f309-4829-96a9-72084a54996d', checkPageVisited: false, + journey: 'returns-required', licence: { id: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d', currentVersionStartDate: '2023-01-01T00:00:00.000Z', @@ -48,8 +26,6 @@ describe('Return Requirements - Check presenter', () => { licenceHolder: 'Turbo Kid', startDate: '2022-04-01T00:00:00.000Z' }, - journey: 'returns-required', - requirements: [{ ...requirement }], startDateOptions: 'licenceStartDate', reason: 'major-change' } @@ -61,20 +37,11 @@ describe('Return Requirements - Check presenter', () => { expect(result).to.equal({ additionalSubmissionOptions: [], - journey: 'returns-required', licenceRef: '01/ABC', note: null, pageTitle: 'Check the return requirements for Turbo Kid', reason: 'Major change', reasonLink: '/system/return-requirements/61e07498-f309-4829-96a9-72084a54996d/reason', - requirements: [{ - abstractionPeriod: 'From 1 June to 1 March', - frequencyCollected: 'daily', - frequencyReported: 'daily', - index: 0, - purposes: 'purpose', - siteDescription: 'A place in the sun' - }], sessionId: '61e07498-f309-4829-96a9-72084a54996d', startDate: '1 January 2023', userEmail: 'No notes added' @@ -217,33 +184,4 @@ describe('Return Requirements - Check presenter', () => { }) }) }) - - describe("the 'requirements' property", () => { - describe('when the requirement has agreements exceptions', () => { - it('correctly returns and requirement with agreements exceptions', () => { - const result = CheckPresenter.go(session) - - expect(result.requirements).to.equal([{ - abstractionPeriod: 'From 1 June to 1 March', - frequencyCollected: 'daily', - frequencyReported: 'daily', - index: 0, - purposes: 'purpose', - siteDescription: 'A place in the sun' - } - ]) - }) - }) - describe('when the requirement does not have any agreements exceptions', () => { - beforeEach(() => { - delete session.requirements[0].agreementsExceptions - }) - - it('correctly does not return the requirement', () => { - const result = CheckPresenter.go(session) - - expect(result.requirements).to.equal([]) - }) - }) - }) }) diff --git a/test/presenters/return-requirements/check/returns-requirements.service.test.js b/test/presenters/return-requirements/check/returns-requirements.service.test.js new file mode 100644 index 0000000000..0ebc9c2d20 --- /dev/null +++ b/test/presenters/return-requirements/check/returns-requirements.service.test.js @@ -0,0 +1,102 @@ +'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 ReturnRequirementsPresenter = require('../../../../app/services/return-requirements/check/returns-requirements.presenter.js') + +describe('Return Requirements presenter', () => { + let requirement + let requirements = [] + let purposeIds = [] + let journey + + beforeEach(() => { + requirement = { + points: [ + '286' + ], + purposes: [ + '772136d1-9184-417b-90cd-91053287d1df' + ], + returnsCycle: 'summer', + siteDescription: 'A place in the sun', + abstractionPeriod: { + 'end-abstraction-period-day': '01', + 'end-abstraction-period-month': '03', + 'start-abstraction-period-day': '01', + 'start-abstraction-period-month': '06' + }, + frequencyReported: 'daily', + frequencyCollected: 'daily', + agreementsExceptions: [ + 'gravity-fill' + ] + } + journey = {} + + purposeIds = [{ + id: '772136d1-9184-417b-90cd-91053287d1df', + description: 'A singular purpose' + }] + + requirements = [{ ...requirement }] + }) + + describe('when provided requirements and purposes', () => { + it('correctly presents the data', () => { + const result = ReturnRequirementsPresenter.go(requirements, purposeIds, journey) + + expect(result).to.equal({ + noReturnsRequired: false, + returnsRequired: false, + requirements: [{ + abstractionPeriod: 'From 1 June to 1 March', + frequencyCollected: 'daily', + frequencyReported: 'daily', + index: 0, + purposes: [ + 'A singular purpose' + ], + siteDescription: 'A place in the sun' + }] + }) + }) + }) + + describe("the 'requirements' property", () => { + describe('when the requirement has agreements exceptions', () => { + it('correctly returns and requirement with agreements exceptions', () => { + const result = ReturnRequirementsPresenter.go(requirements, purposeIds, journey) + + expect(result.requirements).to.equal([{ + abstractionPeriod: 'From 1 June to 1 March', + frequencyCollected: 'daily', + frequencyReported: 'daily', + index: 0, + purposes: [ + 'A singular purpose' + ], + siteDescription: 'A place in the sun' + } + ]) + }) + }) + describe('when the requirement does not have any agreements exceptions', () => { + beforeEach(() => { + delete requirements[0].agreementsExceptions + }) + + it('correctly does not return the requirement', () => { + const result = ReturnRequirementsPresenter.go(requirements, purposeIds, journey) + + expect(result.requirements).to.equal([]) + }) + }) + }) +})