Skip to content

Commit

Permalink
Fix duplicate purposes in return reqs. set up (#1180)
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-4592

> Part of the return requirements set up work

During the return requirements set up journey, users need to select at least one purpose for each return requirement they are setting up. The purposes they can choose from are only those linked to the current version of the licence.

The issue found is that if a licence is linked to a purpose and then to a point, then the same purpose again but to a different point (perhaps because it has condition or different abstraction properties), it causes our logic to return the same purpose twice. More if things are really complicated!

This change fixes it so that the user only sees a distinct list of applicable purposes on the 'Select the purpose for the requirements for returns' page.
  • Loading branch information
Cruikshanks authored Jul 10, 2024
1 parent 1fe0a7a commit 0ed4882
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const PurposeModel = require('../../models/purpose.model.js')
*
* @param {string} licenceId - The UUID for the licence to fetch
*
* @returns {Promise<Object>} The purposes for the matching licenceId
* @returns {Promise<Object>} The distinct purposes for the matching licence's current version
*/
async function go (licenceId) {
return _fetch(licenceId)
Expand All @@ -24,10 +24,12 @@ async function _fetch (licenceId) {
'purposes.id',
'purposes.description'
])
.innerJoin('licenceVersionPurposes', 'purposes.id', 'licenceVersionPurposes.purposeId')
.innerJoin('licenceVersions', 'licenceVersionPurposes.licenceVersionId', 'licenceVersions.id')
.where('licenceVersions.licenceId', licenceId)
.where('licenceVersions.status', 'current')
.whereExists(
PurposeModel.relatedQuery('licenceVersionPurposes')
.innerJoin('licenceVersions', 'licenceVersions.id', 'licenceVersionPurposes.licenceVersionId')
.where('licenceVersions.licenceId', licenceId)
.where('licenceVersions.status', 'current')
)
.orderBy([
{ column: 'purposes.description', order: 'asc' }
])
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 FetchLicencePurposesService = require('./fetch-licence-purposes.service.js')
const FetchPurposesService = require('./fetch-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 FetchLicencePurposesService.go(session.licence.id)
const purposesData = await FetchPurposesService.go(session.licence.id)

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

Expand Down
4 changes: 2 additions & 2 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 FetchLicencePurposesService = require('./fetch-licence-purposes.service.js')
const FetchPurposesService = require('./fetch-purposes.service.js')
const GeneralLib = require('../../lib/general.lib.js')
const PurposePresenter = require('../../presenters/return-requirements/purpose.presenter.js')
const PurposeValidation = require('../../validators/return-requirements/purpose.validator.js')
Expand All @@ -30,7 +30,7 @@ const SessionModel = require('../../models/session.model.js')
*/
async function go (sessionId, requirementIndex, payload, yar) {
const session = await SessionModel.query().findById(sessionId)
const licencePurposes = await FetchLicencePurposesService.go(session.licence.id)
const licencePurposes = await FetchPurposesService.go(session.licence.id)

_handleOneOptionSelected(payload)

Expand Down

This file was deleted.

74 changes: 74 additions & 0 deletions test/services/return-requirements/fetch-purposes.service.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
'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 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')

// Thing under test
const FetchPurposesService = require('../../../app/services/return-requirements/fetch-purposes.service.js')

describe('Return Requirements - Fetch Purposes service', () => {
let licenceVersion
let purposes

beforeEach(async () => {
// Create the initial licenceVersion
licenceVersion = await LicenceVersionHelper.add()

// Create 3 purposes. Note - we purposefully don't add them in alphabetical order so we can test they get sorted
// by the service
purposes = await Promise.all([
PurposeHelper.add({ description: 'Large Garden Watering' }),
PurposeHelper.add({ description: 'Heat Pump' }),
PurposeHelper.add({ description: 'Horticultural Watering' })
])

// Create the licenceVersionPurposes. Note - two of them are for the same purpose. This is common in the service
// where, for example, a licence might abstract water from 2 different points for the same purpose, but they were
// set up separately (rather than the licence version purpose being linked to multiple points) because the details
// and/or conditions are different at the 2 points.
const licenceVersionId = licenceVersion.id

await Promise.all([
LicenceVersionPurposeHelper.add({ licenceVersionId, purposeId: purposes[0].id }),
LicenceVersionPurposeHelper.add({ licenceVersionId, purposeId: purposes[1].id }),
LicenceVersionPurposeHelper.add({ licenceVersionId, purposeId: purposes[1].id }),
LicenceVersionPurposeHelper.add({ licenceVersionId, purposeId: purposes[2].id })
])
})

describe('when called with a valid licenceId', () => {
it('fetches the data', async () => {
const result = await FetchPurposesService.go(licenceVersion.licenceId)

expect(result[0]).to.equal({
id: purposes[1].id,
description: 'Heat Pump'
})
expect(result[1]).to.equal({
id: purposes[2].id,
description: 'Horticultural Watering'
})
expect(result[2]).to.equal({
id: purposes[0].id,
description: 'Large Garden Watering'
})
})
})

describe('when called with an invalid licenceId', () => {
it('returns an empty result', async () => {
const result = await FetchPurposesService.go('5505ca34-270a-4dfb-894c-168c8a4d6e23')

expect(result).to.be.empty()
})
})
})
4 changes: 2 additions & 2 deletions test/services/return-requirements/purpose.service.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const DatabaseSupport = require('../../support/database.js')
const SessionHelper = require('../../support/helpers/session.helper.js')

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

// Thing under test
const PurposeService = require('../../../app/services/return-requirements/purpose.service.js')
Expand All @@ -26,7 +26,7 @@ describe('Return Requirements - Purpose service', () => {
beforeEach(async () => {
await DatabaseSupport.clean()

Sinon.stub(FetchLicencePurposesService, 'go').resolves([
Sinon.stub(FetchPurposesService, 'go').resolves([
{ id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f', description: 'Heat Pump' },
{ id: '49088608-ee9f-491a-8070-6831240945ac', description: 'Horticultural Watering' }
])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const { expect } = Code
const SessionHelper = require('../../support/helpers/session.helper.js')

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

// Thing under test
const SubmitPurposeService = require('../../../app/services/return-requirements/submit-purpose.service.js')
Expand Down Expand Up @@ -48,7 +48,7 @@ describe('Return Requirements - Submit Purpose service', () => {

yarStub = { flash: Sinon.stub() }

Sinon.stub(FetchLicencePurposesService, 'go').resolves([
Sinon.stub(FetchPurposesService, 'go').resolves([
{ id: '14794d57-1acf-4c91-8b48-4b1ec68bfd6f', description: 'Heat Pump' },
{ id: '49088608-ee9f-491a-8070-6831240945ac', description: 'Horticultural Watering' }
])
Expand Down

0 comments on commit 0ed4882

Please sign in to comment.