Skip to content

Commit

Permalink
Fix for view licence abstraction conditions V2 (#853)
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-4332

> @Cruikshanks stab at the problem!

As part of the testing for the new view licence summary page, QA pointed out that the more complicated licences did not match the legacy system. We would see different results for the 'Abstraction conditions' section.

The first thing we'd overlooked was the need to ensure we were only looking at the 'current' licence version. After more investigation, we then discovered that the permit licence JSON blob the legacy code is using is replicated in tables that already exist in the `water` schema. We found we could access the same data using these tables to get a matching result to the legacy view.

This change adds a new service to fetch the abstraction data we need and updates the presenter to generate the distinct list of conditions to display.
  • Loading branch information
Cruikshanks authored Mar 25, 2024
1 parent 8117375 commit 1a0d96a
Show file tree
Hide file tree
Showing 12 changed files with 561 additions and 446 deletions.
35 changes: 14 additions & 21 deletions app/presenters/licences/view-licence.presenter.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
'use strict'

/**
* Formats data for the view `/licences/{id}/` page
* Formats data for the `/licences/{id}` page's summary tab
* @module ViewLicencePresenter
*/

const { formatAbstractionDate } = require('../base.presenter.js')
const { formatLongDate } = require('../base.presenter.js')

/**
* Formats data for the `/licences/{id}/` page
* Formats data for the `/licences/{id}` page's summary tab
*
* @param {module:LicenceModel} licence - The licence where the data will be extracted for from
*
* @returns {Object} The data formatted for the view template
*/
function go (licence, licenceVersionPurposeConditionData) {
function go (licence, licenceAbstractionConditions) {
const {
ends,
expiredDate,
Expand Down Expand Up @@ -45,11 +45,11 @@ function go (licence, licenceVersionPurposeConditionData) {
const pointDetails = _parseAbstractionsAndSourceOfSupply(permitLicence)
const monitoringStationDetails = _generateMonitoringStation(licenceGaugingStations)

const abstractionConditions = _generateAbstractionConditions(licenceVersionPurposeConditionData)
const abstractionConditionDetails = _abstractionConditionDetails(licenceAbstractionConditions)

return {
id,
abstractionConditions,
abstractionConditionDetails,
abstractionPeriods,
abstractionPeriodsAndPurposesLinkText,
abstractionPoints: pointDetails.abstractionPoints,
Expand All @@ -66,8 +66,8 @@ function go (licence, licenceVersionPurposeConditionData) {
purposes,
region: region.displayName,
registeredTo,
startDate: formatLongDate(startDate),
sourceOfSupply: pointDetails.sourceOfSupply,
startDate: formatLongDate(startDate),
warning: _generateWarningMessage(ends)
}
}
Expand All @@ -80,23 +80,16 @@ function _endDate (expiredDate) {
return formatLongDate(expiredDate)
}

function _generateAbstractionConditions (conditionsData) {
if (!conditionsData ||
conditionsData?.abstractionConditions === undefined ||
conditionsData.abstractionConditions.length === 0) {
return {
caption: 'Abstraction condition',
linkText: 'View details of the abstraction condition',
conditions: []
}
}
function _abstractionConditionDetails (licenceAbstractionConditions) {
const { conditions, numberOfConditions } = licenceAbstractionConditions

const conditionText = numberOfConditions === 1 ? 'condition' : 'conditions'

return {
caption: conditionsData.abstractionConditions.length > 1 ? 'Abstraction conditions' : 'Abstraction condition',
linkText: conditionsData.abstractionConditions.length > 1
? 'View details of the abstraction conditions'
: 'View details of the abstraction condition',
conditions: conditionsData.abstractionConditions
caption: `Abstraction ${conditionText}`,
conditions,
linkText: `View details of the abstraction ${conditionText}`,
numberOfConditions
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
'use strict'

/**
* Fetches a licence's abstraction conditions needed for the summary tab on the view '/licences/{id}` page
* @module FetchLicenceAbstractionConditionsService
*/

const LicenceVersionPurposeModel = require('../../models/licence-version-purpose.model.js')

/**
* Fetches a licence's abstraction conditions needed for the summary tab on the view '/licences/{id}` page
*
* Each time a licence is changed in the upstream NALD system a new 'version is generated, the latest of which becomes
* the 'current' version.
*
* Linked to each version could be multiple purposes for the abstraction, for example, spray irrigation and mineral
* washing.
*
* Then each purpose might have one or more conditions attached to it. Where things get complex is the same condition
* can be attached to more than one purpose and we're trying to show a distinct list of abstraction conditions for the
* licence. But we still need a count of them all!
*
* This service is able to determine the purposes for the current licence version and their conditions then extract the
* display title for each one. The total number of results gives us the abstraction condition count we need. As there
* may be duplicates in the titles we process the results to determine the distinct list needed. Finally, just in case
* we also provide a distinct list of the purpose IDs :-)
*
* @param {string} licenceVersionId - The UUID for the 'current' licence version record of the licence being viewed
*
* @returns {Promise<Object>} An object containing the processed results for all the abstraction conditions for the
* current version of the licence. It contains an array of distinct condition titles, the purpose IDs they were linked
* to and the total count of conditions for the licence version.
*/
async function go (licenceVersionId) {
const results = await LicenceVersionPurposeModel.query()
.distinct([
'licenceVersionPurposes.purposeId',
'licenceVersionPurposeConditionTypes.displayTitle'
])
.innerJoin('licenceVersionPurposeConditions', 'licenceVersionPurposes.id', 'licenceVersionPurposeConditions.licenceVersionPurposeId')
.innerJoin('licenceVersionPurposeConditionTypes', 'licenceVersionPurposeConditions.licenceVersionPurposeConditionTypeId', 'licenceVersionPurposeConditionTypes.id')
.where('licenceVersionPurposes.licenceVersionId', licenceVersionId)
.orderBy([
{ column: 'licenceVersionPurposeConditionTypes.displayTitle', order: 'asc' }
])

return _processResults(results)
}

function _processResults (results) {
const allDisplayTitles = []
const allPurposeIds = []

results.forEach((result) => {
allDisplayTitles.push(result.displayTitle)
allPurposeIds.push(result.purposeId)
})

return {
conditions: [...new Set(allDisplayTitles)],
purposeIds: [...new Set(allPurposeIds)],
numberOfConditions: results.length
}
}

module.exports = {
go
}

This file was deleted.

5 changes: 5 additions & 0 deletions app/services/licences/fetch-licence.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ async function _fetchLicence (id) {
'licenceDocumentHeaders.id'
])
})
.withGraphFetched('licenceVersions')
.modifyGraph('licenceVersions', (builder) => {
builder.select(['licenceVersions.id'])
.where('licenceVersions.status', 'current')
})
.withGraphFetched('licenceVersions.[licenceVersionPurposes, purposes]')
.modifyGraph('[licenceVersionPurposes]', (builder) => {
builder.select([
Expand Down
25 changes: 9 additions & 16 deletions app/services/licences/view-licence.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,25 @@
* @module ViewLicenceService
*/

const FetchLicenceAbstractionConditionsService = require('./fetch-licence-abstraction-conditions.service.js')
const FetchLicenceService = require('./fetch-licence.service.js')
const FetchLicenceVersionPurposeConditionService = require('./fetch-licence-version-purpose-condition.service.js')
const ViewLicencePresenter = require('../../presenters/licences/view-licence.presenter.js')

/**
* Orchestrates fetching and presenting the data needed for the licence summary page
*
* @param {string} id The UUID of the licence
* @param {string} licenceId - The UUID of the licence
*
* @returns {Promise<Object>} an object representing the `pageData` needed by the licence summary template.
*/
async function go (id) {
const licenceData = await FetchLicenceService.go(id)

let licenceVersionPurposeConditionData = null

if (!licenceData ||
licenceData?.licenceVersions === undefined ||
licenceData.licenceVersions.length > 0 ||
licenceData.licenceVersions[0]?.licenceVersionPurposes === undefined ||
licenceData.licenceVersions[0]?.licenceVersionPurposes?.length === 0) {
const licenceVersionPurposeId = licenceData?.licenceVersions[0]?.licenceVersionPurposes[0]?.id
licenceVersionPurposeConditionData = await FetchLicenceVersionPurposeConditionService.go(licenceVersionPurposeId)
}
async function go (licenceId) {
const licenceData = await FetchLicenceService.go(licenceId)

const currentLicenceVersionId = licenceData?.licenceVersions[0]?.id

const licenceAbstractionConditions = await FetchLicenceAbstractionConditionsService.go(currentLicenceVersionId)

const pageData = ViewLicencePresenter.go(licenceData, licenceVersionPurposeConditionData)
const pageData = ViewLicencePresenter.go(licenceData, licenceAbstractionConditions)

return {
...pageData
Expand Down
14 changes: 7 additions & 7 deletions app/views/licences/tabs/summary.njk
Original file line number Diff line number Diff line change
Expand Up @@ -111,22 +111,22 @@
</div>

<div class="govuk-summary-list__row">
<dt class="govuk-summary-list__key">{{ abstractionConditions.caption }}</dt>
<dt class="govuk-summary-list__key">{{ abstractionConditionDetails.caption }}</dt>
<dd class="govuk-summary-list__value">
<div>
{% if abstractionConditions.conditions.length > 5 %}
There are {{ abstractionConditions.conditions.length }} abstraction conditions
{% elseif abstractionConditions.conditions.length > 1 %}
{% if abstractionConditionDetails.numberOfConditions > 5 %}
There are {{ abstractionConditionDetails.numberOfConditions }} abstraction conditions
{% elif abstractionConditionDetails.numberOfConditions > 1 %}
<ul class="govuk-list govuk-!-margin-bottom-0">
{% for condition in abstractionConditions.conditions %}
{% for condition in abstractionConditionDetails.conditions %}
<li>{{ condition }}</li>
{% endfor %}
</ul>
{% else %}
{{ abstractionConditions.conditions[0] }}
{{ abstractionConditionDetails.conditions[0] }}
{% endif %}
</div>
<a href="/licences/{{ documentId }}/conditions">{{ abstractionConditions.linkText }}</a>
<a href="/licences/{{ documentId }}/conditions">{{ abstractionConditionDetails.linkText }}</a>
</dd>
</div>

Expand Down
Loading

0 comments on commit 1a0d96a

Please sign in to comment.