From 5abd933c36da38ae6c486cd61436d93b75b5f760 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 22:39:23 +0100 Subject: [PATCH 01/10] Fix view licence summary for incomplete licences https://eaflood.atlassian.net/browse/WATER-4322 In testing we have found licences that have incomplete information. For example, `28/39/29/0048C` has no contacts, not even a licence holder, nor any licence version records. The data doesn't appear to exist in the NALD extract we receive so we have nothing we can use to fill in the missing pieces. But we don't want our licence summary to break when a user searches for the licence, then clicks through. So, these changes ensure the page _shouldn't_ error when a user selects one of these incomplete licences. From 4b66fc48bb33a0cb3c15e2167827b8614f0dd821 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 22:56:04 +0100 Subject: [PATCH 02/10] Housekeeping - fix missing .js extensions --- app/services/licences/view-licence-summary.service.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/licences/view-licence-summary.service.js b/app/services/licences/view-licence-summary.service.js index 3913b1233a..51bf14603d 100644 --- a/app/services/licences/view-licence-summary.service.js +++ b/app/services/licences/view-licence-summary.service.js @@ -6,9 +6,9 @@ */ const FetchLicenceAbstractionConditionsService = require('./fetch-licence-abstraction-conditions.service.js') -const FetchLicenceSummaryService = require('./fetch-licence-summary.service') -const ViewLicenceSummaryPresenter = require('../../presenters/licences/view-licence-summary.presenter') -const ViewLicenceService = require('./view-licence.service') +const FetchLicenceSummaryService = require('./fetch-licence-summary.service.js') +const ViewLicenceSummaryPresenter = require('../../presenters/licences/view-licence-summary.presenter.js') +const ViewLicenceService = require('./view-licence.service.js') /** * Orchestrates fetching and presenting the data needed for the licence summary page From eb55681f3684f6c77cfbe0864ad84859d8b1e9bc Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 22:56:35 +0100 Subject: [PATCH 03/10] Housekeeping - fix missing JSDoc param --- app/services/licences/view-licence-summary.service.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/licences/view-licence-summary.service.js b/app/services/licences/view-licence-summary.service.js index 51bf14603d..44e6ea80c4 100644 --- a/app/services/licences/view-licence-summary.service.js +++ b/app/services/licences/view-licence-summary.service.js @@ -14,6 +14,7 @@ const ViewLicenceService = require('./view-licence.service.js') * Orchestrates fetching and presenting the data needed for the licence summary page * * @param {string} licenceId - The UUID of the licence + * @param {object} auth - The auth object taken from `request.auth` containing user details * * @returns {Promise} an object representing the `pageData` needed by the licence summary template. */ From 56f63a2369a71c313bee5b02415c2a92ca18527d Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 22:57:53 +0100 Subject: [PATCH 04/10] Housekeeping - fix missing .js extensions in tests --- test/services/licences/view-licence-summary.service.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/services/licences/view-licence-summary.service.test.js b/test/services/licences/view-licence-summary.service.test.js index c466ea28ab..286befdbe7 100644 --- a/test/services/licences/view-licence-summary.service.test.js +++ b/test/services/licences/view-licence-summary.service.test.js @@ -13,10 +13,10 @@ const LicenceModel = require('../../../app/models/licence.model.js') // Things we need to stub const FetchLicenceAbstractionConditionsService = require('../../../app/services/licences/fetch-licence-abstraction-conditions.service.js') -const FetchLicenceSummaryService = require('../../../app/services/licences/fetch-licence-summary.service') -const ViewLicenceService = require('../../../app/services/licences/view-licence.service') +const FetchLicenceSummaryService = require('../../../app/services/licences/fetch-licence-summary.service.js') +const ViewLicenceService = require('../../../app/services/licences/view-licence.service.js') // Thing under test -const ViewLicenceSummaryService = require('../../../app/services/licences/view-licence-summary.service') +const ViewLicenceSummaryService = require('../../../app/services/licences/view-licence-summary.service.js') describe('View Licence service summary', () => { const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' From ebe4f27c83133efc99ff07a2399ffd5a175dfe7b Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 23:03:03 +0100 Subject: [PATCH 05/10] Housekeeping - Fix missing .js extension --- app/presenters/licences/view-licence-summary.presenter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/presenters/licences/view-licence-summary.presenter.js b/app/presenters/licences/view-licence-summary.presenter.js index 735e8becf4..fc3701f006 100644 --- a/app/presenters/licences/view-licence-summary.presenter.js +++ b/app/presenters/licences/view-licence-summary.presenter.js @@ -5,7 +5,7 @@ * @module ViewLicenceSummaryPresenter */ -const { formatLongDate, formatAbstractionDate } = require('../base.presenter') +const { formatLongDate, formatAbstractionDate } = require('../base.presenter.js') const { generateAbstractionPointDetail } = require('../../lib/general.lib.js') /** From ecfa40d6f9b9876b83a89cc2f9efc26c76103123 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 23:03:34 +0100 Subject: [PATCH 06/10] Housekeeping - Fix typo --- app/presenters/licences/view-licence-summary.presenter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/presenters/licences/view-licence-summary.presenter.js b/app/presenters/licences/view-licence-summary.presenter.js index fc3701f006..af67ff0bc1 100644 --- a/app/presenters/licences/view-licence-summary.presenter.js +++ b/app/presenters/licences/view-licence-summary.presenter.js @@ -120,14 +120,14 @@ function _generateAbstractionPeriods (licenceVersions) { return null } - const formattedAbstactionPeriods = licenceVersions[0].licenceVersionPurposes.map((purpose) => { + const formattedAbstractionPeriods = licenceVersions[0].licenceVersionPurposes.map((purpose) => { const startDate = formatAbstractionDate(purpose.abstractionPeriodStartDay, purpose.abstractionPeriodStartMonth) const endDate = formatAbstractionDate(purpose.abstractionPeriodEndDay, purpose.abstractionPeriodEndMonth) return `${startDate} to ${endDate}` }) - const uniqueAbstractionPeriods = [...new Set(formattedAbstactionPeriods)] + const uniqueAbstractionPeriods = [...new Set(formattedAbstractionPeriods)] return { caption: uniqueAbstractionPeriods.length > 1 ? 'Periods of abstraction' : 'Period of abstraction', From 9bfb0210e8acdd1991decdee3fc770deec7a7dcd Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 23:11:06 +0100 Subject: [PATCH 07/10] Handle licences with no purposes in the permit --- app/presenters/licences/view-licence-summary.presenter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/presenters/licences/view-licence-summary.presenter.js b/app/presenters/licences/view-licence-summary.presenter.js index af67ff0bc1..d172ed3281 100644 --- a/app/presenters/licences/view-licence-summary.presenter.js +++ b/app/presenters/licences/view-licence-summary.presenter.js @@ -175,7 +175,7 @@ function _generatePurposes (licenceVersions) { function _parseAbstractionsAndSourceOfSupply (permitLicence) { if (!permitLicence || - permitLicence?.purposes === undefined || + !permitLicence.purposes || permitLicence.purposes.length === 0 || permitLicence.purposes[0]?.purposePoints === undefined || permitLicence.purposes[0]?.purposePoints.length === 0 From 5b14dbb30d1b0ee47f8698289fdf64a656b18b4c Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 23:11:40 +0100 Subject: [PATCH 08/10] Housekeeping - simplify the variable name --- app/services/licences/view-licence-summary.service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/licences/view-licence-summary.service.js b/app/services/licences/view-licence-summary.service.js index 44e6ea80c4..c0e0f6a19a 100644 --- a/app/services/licences/view-licence-summary.service.js +++ b/app/services/licences/view-licence-summary.service.js @@ -25,9 +25,9 @@ async function go (licenceId, auth) { const currentLicenceVersionId = summaryLicenceData?.licenceVersions[0]?.id - const licenceAbstractionConditions = await FetchLicenceAbstractionConditionsService.go(currentLicenceVersionId) + const abstractionConditions = await FetchLicenceAbstractionConditionsService.go(currentLicenceVersionId) - const pageData = ViewLicenceSummaryPresenter.go(summaryLicenceData, licenceAbstractionConditions) + const pageData = ViewLicenceSummaryPresenter.go(summaryLicenceData, abstractionConditions) return { ...pageData, From ee479cd183ec4281a4105619c98a09f5bcd3b8fb Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 23:12:48 +0100 Subject: [PATCH 09/10] Housekeeping - Fix missing .js extension in tests --- test/presenters/licences/view-licence-summary.presenter.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/presenters/licences/view-licence-summary.presenter.test.js b/test/presenters/licences/view-licence-summary.presenter.test.js index 9c11fba334..0f1540e2e8 100644 --- a/test/presenters/licences/view-licence-summary.presenter.test.js +++ b/test/presenters/licences/view-licence-summary.presenter.test.js @@ -8,7 +8,7 @@ const { describe, it, beforeEach } = exports.lab = Lab.script() const { expect } = Code // Thing under test -const ViewLicenceSummaryPresenter = require('../../../app/presenters/licences/view-licence-summary.presenter') +const ViewLicenceSummaryPresenter = require('../../../app/presenters/licences/view-licence-summary.presenter.js') describe('View Licence Summary presenter', () => { let licenceAbstractionConditions From 4c889043082c4129431e636ea490b8a2d44e4aea Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 19 May 2024 23:17:14 +0100 Subject: [PATCH 10/10] Update FetchAbstractionConditions no versions Update the FetchLicenceAbstractionConditionsService to handle being passed an undefined licence version ID which will happen if the licence is incomplete and missing version records. --- ...ch-licence-abstraction-conditions.service.js | 17 ++++++++++++++--- ...cence-abstraction-conditions.service.test.js | 10 ++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/app/services/licences/fetch-licence-abstraction-conditions.service.js b/app/services/licences/fetch-licence-abstraction-conditions.service.js index f8c13a83d2..57362d8d8c 100644 --- a/app/services/licences/fetch-licence-abstraction-conditions.service.js +++ b/app/services/licences/fetch-licence-abstraction-conditions.service.js @@ -32,7 +32,20 @@ const LicenceVersionPurposeModel = require('../../models/licence-version-purpose * to and the total count of conditions for the licence version. */ async function go (licenceVersionId) { - const results = await LicenceVersionPurposeModel.query() + const results = await _fetch(licenceVersionId) + + return _processResults(results) +} + +function _fetch (licenceVersionId) { + // NOTE: We have found in testing that there are incomplete licences in the DB, for example, with no licence versions. + // If we are dealing with such a licence licenceVersionId will be undefined. As this service knows how to format the + // results for downstream services we handle it here rather than in the calling service. + if (!licenceVersionId) { + return [] + } + + return LicenceVersionPurposeModel.query() .distinct([ 'licenceVersionPurposes.purposeId', 'licenceVersionPurposeConditionTypes.displayTitle' @@ -43,8 +56,6 @@ async function go (licenceVersionId) { .orderBy([ { column: 'licenceVersionPurposeConditionTypes.displayTitle', order: 'asc' } ]) - - return _processResults(results) } function _processResults (results) { diff --git a/test/services/licences/fetch-licence-abstraction-conditions.service.test.js b/test/services/licences/fetch-licence-abstraction-conditions.service.test.js index 479a5754c1..b56f7aafa1 100644 --- a/test/services/licences/fetch-licence-abstraction-conditions.service.test.js +++ b/test/services/licences/fetch-licence-abstraction-conditions.service.test.js @@ -89,4 +89,14 @@ describe('Fetch Licence Abstraction Conditions service', () => { expect(result.numberOfConditions).to.equal(0) }) }) + + describe('when an undefined licence version ID is passed in', () => { + it('returns an empty array of conditions, purpose IDs and 0 for the number of conditions', async () => { + const result = await FetchLicenceAbstractionConditionsService.go(undefined) + + expect(result.conditions).to.be.empty() + expect(result.purposeIds).to.be.empty() + expect(result.numberOfConditions).to.equal(0) + }) + }) })