Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor purposes page to use purpose ids as values #1025

Merged
merged 11 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/presenters/return-requirements/purpose.presenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ function _backLink (session) {

function _licencePurposes (purposesData) {
return purposesData.map((purpose) => {
return purpose.description
return {
id: purpose.id,
description: purpose.description
}
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ async function go (licenceId) {

async function _fetch (licenceId) {
return PurposeModel.query()
.distinct('description')
.select([
'purposes.id',
'purposes.description'
])
.innerJoin('licenceVersionPurposes', 'purposes.id', 'licenceVersionPurposes.purposeId')
.innerJoin('licenceVersions', 'licenceVersionPurposes.licenceVersionId', 'licenceVersions.id')
.where('licenceVersions.licenceId', licenceId)
Expand Down
4 changes: 2 additions & 2 deletions app/services/return-requirements/purpose.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @module PurposeService
*/

const FetchPurposesService = require('../../services/return-requirements/fetch-purposes.service.js')
const FetchLicencePurposesService = require('./fetch-licence-purposes.service.js')
const SelectPurposePresenter = require('../../presenters/return-requirements/purpose.presenter.js')
const SessionModel = require('../../models/session.model.js')

Expand All @@ -22,7 +22,7 @@ const SessionModel = require('../../models/session.model.js')
*/
async function go (sessionId, requirementIndex) {
const session = await SessionModel.query().findById(sessionId)
const purposesData = await FetchPurposesService.go(session.licence.id)
const purposesData = await FetchLicencePurposesService.go(session.licence.id)

const formattedData = SelectPurposePresenter.go(session, requirementIndex, purposesData)

Expand Down
15 changes: 10 additions & 5 deletions app/services/return-requirements/submit-purpose.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @module SubmitPurposeService
*/

const FetchPurposesService = require('../../services/return-requirements/fetch-purposes.service.js')
const FetchLicencePurposesService = require('./fetch-licence-purposes.service.js')
const PurposeValidation = require('../../validators/return-requirements/purpose.validator.js')
const PurposePresenter = require('../../presenters/return-requirements/purpose.presenter.js')
const SessionModel = require('../../models/session.model.js')
Expand All @@ -28,10 +28,11 @@ const SessionModel = require('../../models/session.model.js')
*/
async function go (sessionId, requirementIndex, payload) {
const session = await SessionModel.query().findById(sessionId)
const purposesData = await FetchLicencePurposesService.go(session.licence.id)

_handleOneOptionSelected(payload)

const validationResult = _validate(payload)
const validationResult = await _validate(payload, purposesData)

if (!validationResult) {
await _save(session, requirementIndex, payload)
Expand All @@ -41,7 +42,6 @@ async function go (sessionId, requirementIndex, payload) {
}
}

const purposesData = await FetchPurposesService.go(session.licence.id)
const formattedData = PurposePresenter.go(session, requirementIndex, purposesData)

return {
Expand Down Expand Up @@ -69,8 +69,13 @@ async function _save (session, requirementIndex, payload) {
return session.$update()
}

function _validate (payload) {
const validation = PurposeValidation.go(payload)
async function _validate (payload, purposesData) {
const purposeIds = []
purposesData.forEach((purpose) => {
purposeIds.push(purpose.id)
})
robertparkinson marked this conversation as resolved.
Show resolved Hide resolved

const validation = PurposeValidation.go(payload, purposeIds)

if (!validation.error) {
return null
Expand Down
65 changes: 2 additions & 63 deletions app/validators/return-requirements/purpose.validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,67 +7,6 @@

const Joi = require('joi')

const VALID_VALUES = [
'Animal Watering & General Use In Non Farming Situations',
'Boiler Feed',
'Conveying Materials',
'Drinking, Cooking, Sanitary, Washing, (Small Garden) - Commercial/Industrial/Public Services',
'Drinking, Cooking, Sanitary, Washing, (Small Garden) - Household',
'Dust Suppression',
'Effluent/Slurry Dilution',
'Evaporative Cooling',
'Fish Farm/Cress Pond Throughflow',
'Fish Pass/Canoe Pass',
'Gas Suppression/Scrubbing',
'General Cooling (Existing Licences Only) (High Loss)',
'General Cooling (Existing Licences Only) (Low Loss)',
'General Farming & Domestic',
'General Use Relating To Secondary Category (High Loss)',
'General Use Relating To Secondary Category (Medium Loss)',
'General Use Relating To Secondary Category (Low Loss)',
'General Use Relating To Secondary Category (Very Low Loss)',
'General Washing/Process Washing',
'Heat Pump',
'Horticultural Watering',
'Hydraulic Rams',
'Hydraulic Testing',
'Hydroelectric Power Generation',
'Lake & Pond Throughflow',
'Large Garden Watering',
'Laundry Use',
'Make-Up Or Top Up Water',
'Milling & Water Power Other Than Electricity Generation',
'Mineral Washing',
'Non-Evaporative Cooling',
'Pollution Remediation',
'Potable Water Supply - Direct',
'Potable Water Supply - Storage',
'Process Water',
'Raw Water Supply',
'River Recirculation',
'Spray Irrigation - Anti Frost',
'Spray Irrigation - Anti Frost Storage',
'Spray Irrigation - Direct',
'Spray Irrigation - Spray Irrigation Definition Order',
'Spray Irrigation - Storage',
'Supply To A Canal For Throughflow',
'Supply To A Leat For Throughflow',
'Transfer Between Sources (Pre Water Act 2003)',
'Vegetable Washing',
'Water Bottling',
'Water Wheels Not Used For Power',
'Impounding (for any purpose excluding impounding for HEP)',
'Trickle Irrigation - Direct',
'Trickle Irrigation - Under Cover/Containers',
'Trickle Irrigation - Storage',
'Flood Irrigation, Including Water Meadows, Warping And Pest Control',
'Wet Fencing And Nature Conservation',
'Transfer Between Sources (Post Water Act 2003)',
'Dewatering',
'Hydraulic Fracturing (Fracking)',
'Wet Fencing and Agriculture'
]

/**
* Validates data submitted for the `/return-requirements/{sessionId}/purpose` page
*
Expand All @@ -80,14 +19,14 @@ const VALID_VALUES = [
* @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) {
function go (payload, purposeIds) {
const purposes = payload.purposes

const errorMessage = 'Select any purpose for the requirements for returns'

const schema = Joi.object({
purposes: Joi.array()
.items(Joi.string().valid(...VALID_VALUES))
.items(Joi.string().valid(...purposeIds))
.required()
.messages({
'any.required': errorMessage,
Expand Down
8 changes: 4 additions & 4 deletions app/views/return-requirements/purpose.njk
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
titleText: "There is a problem",
errorList: [
{
text: "Select any uses for the return requirement",
text: error.text,
href: "#purposes"
}
]
Expand All @@ -44,9 +44,9 @@
{% for purpose in licencePurposes %}

{% set checkBoxItem = {
value: purpose,
text: purpose,
checked: purpose in purposes
value: purpose.id,
text: purpose.description,
checked: purpose.id in purposes
} %}

{% set checkBoxItems = (checkBoxItems.push(checkBoxItem), checkBoxItems) %}
Expand Down
24 changes: 17 additions & 7 deletions test/presenters/return-requirements/purpose.presenter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ describe('Return Requirements - Purpose presenter', () => {

beforeEach(() => {
purposesData = [
{ description: 'Heat Pump' },
{ description: 'Horticultural Watering' },
{ description: 'Hydraulic Rams' },
{ description: 'Hydraulic Testing' }
{ id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f', description: 'Heat Pump' },
{ id: '49088608-ee9f-491a-8070-6831240945ac', description: 'Horticultural Watering' },
{ id: '1d03c79b-da97-4838-a68c-ccb613d54367', description: 'Hydraulic Rams' },
{ id: '02036782-81d2-43be-b6af-bf20898653e1', description: 'Hydraulic Testing' }
]

session = {
Expand Down Expand Up @@ -49,7 +49,12 @@ describe('Return Requirements - Purpose presenter', () => {
expect(result).to.equal({
backLink: '/system/return-requirements/61e07498-f309-4829-96a9-72084a54996d/setup',
licenceId: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d',
licencePurposes: ['Heat Pump', 'Horticultural Watering', 'Hydraulic Rams', 'Hydraulic Testing'],
licencePurposes: [
{ id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f', description: 'Heat Pump' },
{ id: '49088608-ee9f-491a-8070-6831240945ac', description: 'Horticultural Watering' },
{ id: '1d03c79b-da97-4838-a68c-ccb613d54367', description: 'Hydraulic Rams' },
{ id: '02036782-81d2-43be-b6af-bf20898653e1', description: 'Hydraulic Testing' }
],
licenceRef: '01/ABC',
purposes: '',
sessionId: '61e07498-f309-4829-96a9-72084a54996d'
Expand Down Expand Up @@ -80,10 +85,15 @@ describe('Return Requirements - Purpose presenter', () => {
})

describe("the 'licencePurposes' property", () => {
it("returns the description from each 'purpose' passed in", () => {
it("returns the id and description from each 'purpose' passed in", () => {
const result = PurposePresenter.go(session, requirementIndex, purposesData)

expect(result.licencePurposes).to.equal(['Heat Pump', 'Horticultural Watering', 'Hydraulic Rams', 'Hydraulic Testing'])
expect(result.licencePurposes).to.equal([
{ id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f', description: 'Heat Pump' },
{ id: '49088608-ee9f-491a-8070-6831240945ac', description: 'Horticultural Watering' },
{ id: '1d03c79b-da97-4838-a68c-ccb613d54367', description: 'Hydraulic Rams' },
{ id: '02036782-81d2-43be-b6af-bf20898653e1', description: 'Hydraulic Testing' }
])
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ const LicenceVersionPurposeHelper = require('../../support/helpers/licence-versi
const PurposeHelper = require('../../support/helpers/purpose.helper.js')

// Thing under test
const FetchPurposesService = require('../../../app/services/return-requirements/fetch-purposes.service.js')
const FetchPurposesService = require('../../../app/services/return-requirements/fetch-licence-purposes.service.js')
robertparkinson marked this conversation as resolved.
Show resolved Hide resolved

describe('Return Requirements - Fetch Purposes service', () => {
describe('Return Requirements - Fetch Purposes for a given licence service', () => {
robertparkinson marked this conversation as resolved.
Show resolved Hide resolved
let licenceVersion
let purposes

Expand All @@ -28,9 +28,9 @@ describe('Return Requirements - Fetch Purposes service', () => {

// Create 3 descriptions for the purposes
purposes = await Promise.all([
await PurposeHelper.add({ description: 'Heat Pump' }),
await PurposeHelper.add({ description: 'Horticultural Watering' }),
await PurposeHelper.add({ description: 'Large Garden Watering' })
await PurposeHelper.add({ id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f', description: 'Heat Pump' }),
await PurposeHelper.add({ id: '49088608-ee9f-491a-8070-6831240945ac', description: 'Horticultural Watering' }),
await PurposeHelper.add({ id: '8290bb6a-4265-4cc8-b9bb-37cde1357d5d', description: 'Large Garden Watering' })
])

// Create the licenceVersionPurposes with the purposes and licenceVersion
Expand All @@ -46,11 +46,16 @@ describe('Return Requirements - Fetch Purposes service', () => {
it('fetches the data', async () => {
const result = await FetchPurposesService.go(licenceVersion.licenceId)

expect(result).to.equal([
{ description: 'Heat Pump' },
{ description: 'Horticultural Watering' },
{ description: 'Large Garden Watering' }
])
expect(result).to.equal([{
id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f',
description: 'Heat Pump'
}, {
id: '49088608-ee9f-491a-8070-6831240945ac',
description: 'Horticultural Watering'
}, {
id: '8290bb6a-4265-4cc8-b9bb-37cde1357d5d',
description: 'Large Garden Watering'
}])
})
})

Expand Down
41 changes: 28 additions & 13 deletions test/services/return-requirements/purpose.service.test.js
robertparkinson marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,46 @@ const { expect } = Code

// Test helpers
const DatabaseSupport = require('../../support/database.js')
const LicenceVersionHelper = require('../../support/helpers/licence-version.helper.js')
const LicenceVersionPurposeHelper = require('../../support/helpers/licence-version-purpose.helper.js')
const PurposeHelper = require('../../support/helpers/purpose.helper.js')
const SessionHelper = require('../../support/helpers/session.helper.js')

// Things we need to stub
const FetchPurposesService = require('../../../app/services/return-requirements/fetch-purposes.service.js')

// Thing under test
const PurposeService = require('../../../app/services/return-requirements/purpose.service.js')

describe('Return Requirements - Purpose service', () => {
const requirementIndex = 0

let licenceVersion
let purposes
let session

beforeEach(async () => {
await DatabaseSupport.clean()

// Create the initial licenceVersion
licenceVersion = await LicenceVersionHelper.add()

// Create 3 descriptions for the purposes
purposes = await Promise.all([
await PurposeHelper.add({ id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f', description: 'Heat Pump' }),
await PurposeHelper.add({ id: '49088608-ee9f-491a-8070-6831240945ac', description: 'Horticultural Watering' })
])

// Create the licenceVersionPurposes with the purposes and licenceVersion
for (const purpose of purposes) {
await LicenceVersionPurposeHelper.add({
licenceVersionId: licenceVersion.id,
purposeId: purpose.id
})
}

session = await SessionHelper.add({
data: {
checkPageVisited: false,
licence: {
id: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d',
id: licenceVersion.licenceId,
currentVersionStartDate: '2023-01-01T00:00:00.000Z',
endDate: null,
licenceRef: '01/ABC',
Expand All @@ -43,11 +62,6 @@ describe('Return Requirements - Purpose service', () => {
reason: 'major-change'
}
})

Sinon.stub(FetchPurposesService, 'go').resolves([
{ description: 'Transfer Between Sources (Pre Water Act 2003)' },
{ description: 'Potable Water Supply - Direct' }
])
})

afterEach(() => {
Expand All @@ -68,13 +82,14 @@ describe('Return Requirements - Purpose service', () => {
activeNavBar: 'search',
pageTitle: 'Select the purpose for the requirements for returns',
backLink: `/system/return-requirements/${session.id}/setup`,
licenceId: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d',
licenceId: licenceVersion.licenceId,
licencePurposes: [
'Transfer Between Sources (Pre Water Act 2003)',
'Potable Water Supply - Direct'
{ id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f', description: 'Heat Pump' },
{ id: '49088608-ee9f-491a-8070-6831240945ac', description: 'Horticultural Watering' }
],
licenceRef: '01/ABC',
purposes: ''
purposes: '',
sessionId: session.id
}, { skip: ['sessionId'] })
})
})
Expand Down
Loading
Loading