Skip to content

Commit

Permalink
Add optional alias to return requirement purpose (#1177)
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-4573

> Part of the return requirements set up work

When setting up new return requirements, you are required to select one or more purposes. By default, we use the purpose description when referring to the return requirement purpose. But because the returns are intended to reflect what is on the licence document, sometimes there is a discrepancy between how the system describes the purpose and how it is recorded in the document.

To deal with this, NALD has the concept of a 'purpose alias'. This is an optional alternate description that should be used when referring to the purpose of a return requirement.

This change updates the 'Select the purpose' page in the return requirements set up journey to add a [conditional reveal to the GOV.UK checkbox](https://design-system.service.gov.uk/components/checkboxes/). This will reveal a textbox that users can use to enter a description, though it is not required.

We will add some validation, but only to limit the description to 100 characters, which is the same as site description.

---

As part of this, we were able to do some refactoring. The key change was to store the description from the purpose in the session to avoid having to fetch it again when we get to the `/check` page. That simplified what we were doing in that area to the extent that we could drop a service completely.

But adding the optional alias with a text limit meant the validation became more complex. Plus, we now have to deal with the scenario on the purpose page where the submission is invalid, but we have to replay the user's choices. Previously, as long as they selected something, they were good to go!
  • Loading branch information
Cruikshanks authored Jul 10, 2024
1 parent 3f60b65 commit 0acfe21
Show file tree
Hide file tree
Showing 14 changed files with 612 additions and 525 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,16 @@ const agreementsExceptionsText = {
* Formats return requirements data for the `/return-requirements/{sessionId}/check` page
*
* @param {Object[]} requirements - The existing return requirements in the current session
* @param {module:PurposeModel[]} purposes - All purposes that match those selected across the return requirements
* @param {Object[]} points - The points related to the licence
* @param {string} journey - Whether the setup journey is 'no-returns-required' or 'returns-required'
*
* @returns {Object} returns requirement data needed by the view template
*/

function go (requirements, purposes, points, journey) {
function go (requirements, points, journey) {
return {
returnsRequired: journey === 'returns-required',
requirements: _requirements(requirements, purposes, points)
requirements: _requirements(requirements, points)
}
}

Expand All @@ -48,10 +47,6 @@ function _abstractionPeriod (abstractionPeriod) {
}

function _agreementsExceptions (agreementsExceptions) {
if (agreementsExceptions[0] === agreementsExceptionsText.none) {
return 'None'
}

const formattedExceptions = agreementsExceptions.map((exception) => {
return agreementsExceptionsText[exception]
})
Expand All @@ -68,40 +63,41 @@ function _agreementsExceptions (agreementsExceptions) {
.join(', ') + ', and ' + formattedExceptions[formattedExceptions.length - 1]
}

function _requirements (requirements, purposes, points) {
function _requirements (requirements, points) {
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, purposes, points))
completedRequirements.push(_mapRequirement(requirement, index, points))
}
}

return completedRequirements
}

function _mapPurposes (requirementPurposes, purposes) {
return requirementPurposes.map((requirementPurpose) => {
const matchedPurpose = purposes.find((purpose) => {
return purpose.id === requirementPurpose
})
function _mapPurposes (purposes) {
return purposes.map((purpose) => {
if (purpose.alias) {
return `${purpose.description} (${purpose.alias})`
}

return matchedPurpose.description
return purpose.description
})
}

function _mapRequirement (requirement, index, purposes, points) {
function _mapRequirement (requirement, index, points) {
return {
abstractionPeriod: _abstractionPeriod(requirement.abstractionPeriod),
agreementsExceptions: _agreementsExceptions(requirement.agreementsExceptions),
frequencyCollected: requirement.frequencyCollected,
frequencyReported: requirement.frequencyReported,
index,
points: _mapPoints(requirement.points, points),
purposes: _mapPurposes(requirement.purposes, purposes),
purposes: _mapPurposes(requirement.purposes),
returnsCycle: requirement.returnsCycle === 'summer' ? 'Summer' : 'Winter and all year',
siteDescription: requirement.siteDescription
}
Expand Down
21 changes: 13 additions & 8 deletions app/presenters/return-requirements/purpose.presenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,19 @@
*
* @param {module:SessionModel} session - The returns requirements session instance
* @param {string} requirementIndex - The index of the requirement being added or changed
* @param {module:PurposeModel[]} purposesData - The purposes for the licence
* @param {module:PurposeModel[]} licencePurposes - All the purposes for the licence
*
* @returns {Object} - The data formatted for the view template
*/
function go (session, requirementIndex, purposesData) {
function go (session, requirementIndex, licencePurposes) {
const { id: sessionId, licence, requirements } = session
const requirement = requirements[requirementIndex]

return {
backLink: _backLink(session),
licenceId: licence.id,
licencePurposes: _licencePurposes(purposesData),
licenceRef: licence.licenceRef,
purposes: requirement?.purposes ? requirement.purposes.join(',') : '',
purposes: _purposes(licencePurposes, requirement.purposes),
sessionId
}
}
Expand All @@ -47,11 +46,17 @@ function _backLink (session) {
return `/system/return-requirements/${id}/setup`
}

function _licencePurposes (purposesData) {
return purposesData.map((purpose) => {
function _purposes (licencePurposes, requirementPurposes) {
return licencePurposes.map((licencePurpose) => {
const matchedRequirementPurpose = requirementPurposes?.find((requirementPurpose) => {
return requirementPurpose.id === licencePurpose.id
})

return {
id: purpose.id,
description: purpose.description
alias: matchedRequirementPurpose?.alias ? matchedRequirementPurpose.alias : '',
checked: !!matchedRequirementPurpose,
description: licencePurpose.description,
id: licencePurpose.id
}
})
}
Expand Down
13 changes: 11 additions & 2 deletions app/services/return-requirements/check.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
*/

const CheckPresenter = require('../../presenters/return-requirements/check.presenter.js')
const ReturnRequirementsService = require('./check/returns-requirements.service.js')
const FetchPointsService = require('./fetch-points.service.js')
const ReturnRequirementsPresenter = require('../../presenters/return-requirements/check/returns-requirements.presenter.js')
const SessionModel = require('../../models/session.model.js')

/**
Expand All @@ -22,7 +23,7 @@ async function go (sessionId, yar) {

await _markCheckPageVisited(session)

const returnRequirements = await ReturnRequirementsService.go(session)
const returnRequirements = await _returnRequirements(session)

const formattedData = CheckPresenter.go(session)

Expand All @@ -42,6 +43,14 @@ async function _markCheckPageVisited (session) {
return session.$update()
}

async function _returnRequirements (session) {
const { licence, requirements, journey } = session

const points = await FetchPointsService.go(licence.id)

return ReturnRequirementsPresenter.go(requirements, points, journey)
}

module.exports = {
go
}

This file was deleted.

48 changes: 41 additions & 7 deletions app/services/return-requirements/submit-purpose.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,16 @@ const SessionModel = require('../../models/session.model.js')
*/
async function go (sessionId, requirementIndex, payload, yar) {
const session = await SessionModel.query().findById(sessionId)
const purposesData = await FetchLicencePurposesService.go(session.licence.id)
const licencePurposes = await FetchLicencePurposesService.go(session.licence.id)

_handleOneOptionSelected(payload)

const validationResult = await _validate(payload, purposesData)
const purposes = _combinePurposeDetails(payload, licencePurposes)

const validationResult = await _validate(purposes, licencePurposes)

if (!validationResult) {
await _save(session, requirementIndex, payload)
await _save(session, requirementIndex, purposes)

if (session.checkPageVisited) {
GeneralLib.flashNotification(yar)
Expand All @@ -48,33 +50,65 @@ async function go (sessionId, requirementIndex, payload, yar) {
}
}

const formattedData = PurposePresenter.go(session, requirementIndex, purposesData)
const submittedSessionData = _submittedSessionData(session, requirementIndex, purposes, licencePurposes)

return {
activeNavBar: 'search',
error: validationResult,
pageTitle: 'Select the purpose for the requirements for returns',
...formattedData
...submittedSessionData
}
}

function _combinePurposeDetails (payload, licencePurposes) {
const combinedValues = []

for (const purpose of payload.purposes) {
const alias = payload[`alias-${purpose}`]
const matchedLicencePurpose = licencePurposes.find((licencePurpose) => {
return licencePurpose.id === purpose
})

combinedValues.push({
id: purpose,
alias: alias || '',
description: matchedLicencePurpose.description
})
}

return combinedValues
}

/**
* When a single purpose is checked by the user, it returns as a string. When multiple purposes are checked, the
* 'purposes' is returned as an array. This function works to make those single selected string 'purposes' into an array
* for uniformity.
*/
function _handleOneOptionSelected (payload) {
if (!payload.purposes) {
payload.purposes = []
}

if (!Array.isArray(payload.purposes)) {
payload.purposes = [payload.purposes]
}
}

async function _save (session, requirementIndex, payload) {
session.requirements[requirementIndex].purposes = payload.purposes
async function _save (session, requirementIndex, purposes) {
session.requirements[requirementIndex].purposes = purposes

return session.$update()
}

/**
* Combines the existing session data with the submitted payload formatted by the presenter
*/
function _submittedSessionData (session, requirementIndex, purposes, licencePurposes) {
session.requirements[requirementIndex].purposes = purposes

return PurposePresenter.go(session, requirementIndex, licencePurposes)
}

async function _validate (payload, purposesData) {
const purposeIds = purposesData.map((purpose) => {
return purpose.id
Expand Down
42 changes: 31 additions & 11 deletions app/validators/return-requirements/purpose.validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,50 @@ const Joi = require('joi')
* Users must select one or more purposes linked to the licence. If these requirements are not met
* the validation will return an error.
*
* @param {Object} payload - The payload from the request to be validated
* Users also have the option to add an alias (purpose description) to the purpose. This is not required but if added
* we check to ensure it is no more than 100 characters.
*
* @param {Object[]} purposes - The selected purposes and aliases (if entered) from the payload
*
* @returns {Object} The result from calling Joi's schema.validate(). If any errors are found the
* `error:` property will also exist detailing what the issue is.
*/
function go (payload, purposeIds) {
const purposes = payload.purposes

function go (purposes, purposeIds) {
const errorMessage = 'Select any purpose for the requirements for returns'

const schema = Joi.object({
purposes: Joi.array()
.items(Joi.string().valid(...purposeIds))
.items({
id: Joi
.string()
.valid(...purposeIds)
.required()
.messages({
'any.required': errorMessage,
'any.only': errorMessage
}),
alias: Joi
.string()
.max(100)
.optional()
.allow('')
.messages({
'string.max': 'Purpose description must be 100 characters or less'
}),
// Description will not be persisted. It is simply to avoid having to fetch the purpose description again in
// the /check page. But if we didn't match a selected ID to a description in the SubmitPurposeService then
// something has gone wrong!
description: Joi
.string()
})
.min(1)
.required()
.messages({
'any.required': errorMessage,
'any.only': errorMessage,
'array.includesOne': errorMessage,
'array.includes': errorMessage,
'array.sparse': errorMessage
'array.min': errorMessage
})
})

return schema.validate({ purposes }, { abortEarly: false })
return schema.validate({ purposes }, { abortEarly: true })
}

module.exports = {
Expand Down
Loading

0 comments on commit 0acfe21

Please sign in to comment.