From 51700a1935d7467a00b6505eb0f6132124b849d8 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 16:35:22 +0000 Subject: [PATCH 01/12] Add registered user modifier to LicenceModel https://eaflood.atlassian.net/browse/WATER-4324 > This is part of a series of changes to allow us to replace the legacy view licence page When viewing a licence using the [legacy water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui) the page will display information on who the registered user is. The registered is an external user who has declared that they manage the licence via the external UI. They are not the licence holder, just the person declared to be responsible for managing it. Unfortunately, this functionality dates from the very start of the service and the data is in one of the oldest legacy schemas; `crm`. It seems this was one of the things which the previous team never managed to migrate to `crm_v2`, their intended better CRM structure. We have to go through a number of tables using a specific query to identify the registered user and whether they have set a custom name for the licence. This change adds a [modifier](https://vincit.github.io/objection.js/recipes/modifiers.html#modifiers) to the `LicenceModel` so we only have to do this once! From c1d50266ad2d06647c016b0969c158036e0de6fd Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 16:44:48 +0000 Subject: [PATCH 02/12] DO NOT MERGE - licence holder changes These are some of the changes made in https://github.com/DEFRA/water-abstraction-system/pull/679 to support displaying the licence holder in the view page. We will not retain them at the end of this PR. But `FetchLicenceService` is where we intend to first make use of our new registered user modifier. To make sure it works and doesn't break anything we're copying what will soon be merged before we then start playing around! --- .../licences/view-licence.presenter.js | 17 ++++++++++----- .../licences/fetch-licence.service.js | 21 +++++++++++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/app/presenters/licences/view-licence.presenter.js b/app/presenters/licences/view-licence.presenter.js index 67c7abad74..6e5d76d83e 100644 --- a/app/presenters/licences/view-licence.presenter.js +++ b/app/presenters/licences/view-licence.presenter.js @@ -15,16 +15,17 @@ const { formatLongDate } = require('../base.presenter.js') * @returns {Object} The data formatted for the view template */ function go (licence) { - const { expiredDate, id, licenceRef, region, startDate } = licence - const warning = _generateWarningMessage(licence) + const { ends, expiredDate, id, licenceDocumentHeader, licenceHolder, licenceRef, region, startDate } = licence return { id, + documentId: licenceDocumentHeader.id, endDate: _endDate(expiredDate), + licenceHolder: _generateLicenceHolder(licenceHolder), licenceRef, region: region.displayName, startDate: formatLongDate(startDate), - warning + warning: _generateWarningMessage(ends) } } @@ -36,9 +37,15 @@ function _endDate (expiredDate) { return formatLongDate(expiredDate) } -function _generateWarningMessage (licence) { - const ends = licence.$ends() +function _generateLicenceHolder (licenceHolder) { + if (!licenceHolder) { + return 'Unregistered licence' + } + + return licenceHolder +} +function _generateWarningMessage (ends) { if (!ends) { return null } diff --git a/app/services/licences/fetch-licence.service.js b/app/services/licences/fetch-licence.service.js index c16cd79df0..8d7c48a2fd 100644 --- a/app/services/licences/fetch-licence.service.js +++ b/app/services/licences/fetch-licence.service.js @@ -19,11 +19,21 @@ const LicenceModel = require('../../models/licence.model.js') async function go (id) { const licence = await _fetchLicence(id) - return licence + const data = await _data(licence) + + return data +} + +async function _data (licence) { + return { + ...licence, + ends: licence.$ends(), + licenceHolder: licence.$licenceHolder() + } } async function _fetchLicence (id) { - const result = LicenceModel.query() + const result = await LicenceModel.query() .findById(id) .select([ 'expiredDate', @@ -40,6 +50,13 @@ async function _fetchLicence (id) { 'displayName' ]) }) + .withGraphFetched('licenceDocumentHeader') + .modifyGraph('licenceDocumentHeader', (builder) => { + builder.select([ + 'id' + ]) + }) + .modify('licenceHolder') return result } From af2236d43f096d3cb2171daac15547a12f39c1e4 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 17:57:06 +0000 Subject: [PATCH 03/12] Alter LicenceDocumentHeader view migration There were fields which didn't appear to be used when we create the migration. No doubt we didn't check pre-prod properly because if we had we would have seen that a number of these fields are populated. Specifically, we need `companyEntityId` in order to link through to the new tables we've added to get the primary user. And we need `document_name` as this is where **Licence name** comes from (shown as a field in the external summary and above the licence ref in the internal summary). --- ...27170505_alter-licence-document-headers.js | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 db/migrations/public/20240127170505_alter-licence-document-headers.js diff --git a/db/migrations/public/20240127170505_alter-licence-document-headers.js b/db/migrations/public/20240127170505_alter-licence-document-headers.js new file mode 100644 index 0000000000..2b844de3fc --- /dev/null +++ b/db/migrations/public/20240127170505_alter-licence-document-headers.js @@ -0,0 +1,54 @@ +'use strict' + +const viewName = 'licence_document_headers' + +exports.up = function (knex) { + return knex + .schema + .dropView(viewName) + .createView(viewName, (view) => { + view.as(knex('document_header').withSchema('crm').select([ + 'document_id AS id', + // This could be ignored as it is always set to the same ID. But that id comes from a single record in the + // crm.entity table which has the `entity_type` regime. So, for the purposes of testing we just have to live + // with always populating it even though we don't care about it. + 'regime_entity_id', + // 'system_id', + 'system_internal_id AS nald_id', + 'system_external_id AS licence_ref', + 'metadata', + 'company_entity_id', + // 'verification_id', + 'document_name AS licence_name', + 'date_created AS created_at', + 'date_updated AS updated_at', + 'date_deleted AS deleted_at' + ])) + }) +} + +exports.down = function (knex) { + return knex + .schema + .dropView(viewName) + .createView(viewName, (view) => { + // NOTE: We have commented out unused columns from the source table + view.as(knex('document_header').withSchema('crm').select([ + 'document_id AS id', + // This could be ignored as it is always set to the same ID. But that id comes from a single record in the + // crm.entity table which has the `entity_type` regime. So, for the purposes of testing we just have to live + // with always populating it even though we don't care about it. + 'regime_entity_id', + // 'system_id', + 'system_internal_id AS nald_id', + 'system_external_id AS licence_ref', + 'metadata', + // 'company_entity_id', + // 'verification_id', + // 'document_name', + 'date_created AS created_at', + 'date_updated AS updated_at', + 'date_deleted AS deleted_at' + ])) + }) +} From a23226ff7b30206b5ecdae9f06bb57cb45b3321b Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 17:59:38 +0000 Subject: [PATCH 04/12] Get primary user name and licence name Here is a working example of the query we need to add to get a licence's registered username and if set the licence name they assigned. We're having to use an standard join rather than a `withGraphFetched` because the link between `LicenceDocumentHeader` and `LicenceEntityRole` is via the `companyEntityId` which just seems _too_ tenuous. So, we haven't defined a relationship for it in the model hence the need for the stand INNER JOIN. We're also having to drop down into Knex for part of it in order to add an extra clause to the JOIN. We only want the record that is set as the `primary_user`. Because we are using standard joins we can't deal with this in the WHERE clause. We could have left it out but then it'd be prudent to add extra logic to ensure the `LicenceEntityRole` definitely has the role `primary_user`. This means you can use the result without extra checking. --- .../licences/fetch-licence.service.js | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/app/services/licences/fetch-licence.service.js b/app/services/licences/fetch-licence.service.js index 8d7c48a2fd..c00d9465fc 100644 --- a/app/services/licences/fetch-licence.service.js +++ b/app/services/licences/fetch-licence.service.js @@ -5,6 +5,7 @@ * @module FetchLicenceService */ +const { db } = require('../../../db/db.js') const LicenceModel = require('../../models/licence.model.js') /** @@ -36,12 +37,12 @@ async function _fetchLicence (id) { const result = await LicenceModel.query() .findById(id) .select([ - 'expiredDate', - 'id', - 'lapsedDate', - 'licenceRef', - 'revokedDate', - 'startDate' + 'licences.expiredDate', + 'licences.id', + 'licences.lapsedDate', + 'licences.licenceRef', + 'licences.revokedDate', + 'licences.startDate' ]) .withGraphFetched('region') .modifyGraph('region', (builder) => { @@ -53,8 +54,17 @@ async function _fetchLicence (id) { .withGraphFetched('licenceDocumentHeader') .modifyGraph('licenceDocumentHeader', (builder) => { builder.select([ - 'id' + 'licenceDocumentHeaders.id', + 'licenceDocumentHeaders.licenceName', + 'licenceEntityRoles.role', + 'licenceEntities.name' ]) + .leftJoin('licenceEntityRoles', function () { + this + .on('licenceEntityRoles.companyEntityId', '=', 'licenceDocumentHeaders.companyEntityId') + .andOn('licenceEntityRoles.role', '=', db.raw('?', ['primary_user'])) + }) + .leftJoin('licenceEntities', 'licenceEntities.id', 'licenceEntityRoles.licenceEntityId') }) .modify('licenceHolder') From 14e2b7c7aba71126c704a868b48ed4c54fcca70c Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 18:11:01 +0000 Subject: [PATCH 05/12] Implement modifier This proves we can migrate the part of the query we've enhanced to a modifier and it still works (we console.Log() the result and confirmed it doesn't blow up for those with and without a registered user). --- app/models/licence.model.js | 18 ++++++++++++++++++ app/services/licences/fetch-licence.service.js | 17 +---------------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/app/models/licence.model.js b/app/models/licence.model.js index c68b12cb0a..d3f3ce0a39 100644 --- a/app/models/licence.model.js +++ b/app/models/licence.model.js @@ -130,6 +130,24 @@ class LicenceModel extends BaseModel { 'suffix' ]) }) + }, + registeredUserAndLicenceName (query) { + query + .withGraphFetched('licenceDocumentHeader') + .modifyGraph('licenceDocumentHeader', (builder) => { + builder.select([ + 'licenceDocumentHeaders.id', + 'licenceDocumentHeaders.licenceName', + 'licenceEntityRoles.role', + 'licenceEntities.name' + ]) + .leftJoin('licenceEntityRoles', function () { + this + .on('licenceEntityRoles.companyEntityId', '=', 'licenceDocumentHeaders.companyEntityId') + .andOn('licenceEntityRoles.role', '=', Model.raw('?', ['primary_user'])) + }) + .leftJoin('licenceEntities', 'licenceEntities.id', 'licenceEntityRoles.licenceEntityId') + }) } } } diff --git a/app/services/licences/fetch-licence.service.js b/app/services/licences/fetch-licence.service.js index c00d9465fc..5f79670294 100644 --- a/app/services/licences/fetch-licence.service.js +++ b/app/services/licences/fetch-licence.service.js @@ -5,7 +5,6 @@ * @module FetchLicenceService */ -const { db } = require('../../../db/db.js') const LicenceModel = require('../../models/licence.model.js') /** @@ -51,22 +50,8 @@ async function _fetchLicence (id) { 'displayName' ]) }) - .withGraphFetched('licenceDocumentHeader') - .modifyGraph('licenceDocumentHeader', (builder) => { - builder.select([ - 'licenceDocumentHeaders.id', - 'licenceDocumentHeaders.licenceName', - 'licenceEntityRoles.role', - 'licenceEntities.name' - ]) - .leftJoin('licenceEntityRoles', function () { - this - .on('licenceEntityRoles.companyEntityId', '=', 'licenceDocumentHeaders.companyEntityId') - .andOn('licenceEntityRoles.role', '=', db.raw('?', ['primary_user'])) - }) - .leftJoin('licenceEntities', 'licenceEntities.id', 'licenceEntityRoles.licenceEntityId') - }) .modify('licenceHolder') + .modify('registeredUserAndLicenceName') return result } From 132f8f25643dd0612cc640fde73e6db34f173235 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 18:22:08 +0000 Subject: [PATCH 06/12] Rename modifier and add customer instance methods The new name better matches how the value is referred to in the UI. The instance methods make it easier to access the values and give us something to build our tests against. --- app/models/licence.model.js | 16 ++++++++++++++-- app/services/licences/fetch-licence.service.js | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/models/licence.model.js b/app/models/licence.model.js index d3f3ce0a39..849f5c4b90 100644 --- a/app/models/licence.model.js +++ b/app/models/licence.model.js @@ -131,7 +131,7 @@ class LicenceModel extends BaseModel { ]) }) }, - registeredUserAndLicenceName (query) { + registeredToAndLicenceName (query) { query .withGraphFetched('licenceDocumentHeader') .modifyGraph('licenceDocumentHeader', (builder) => { @@ -139,7 +139,7 @@ class LicenceModel extends BaseModel { 'licenceDocumentHeaders.id', 'licenceDocumentHeaders.licenceName', 'licenceEntityRoles.role', - 'licenceEntities.name' + 'licenceEntities.name AS registeredTo' ]) .leftJoin('licenceEntityRoles', function () { this @@ -248,6 +248,18 @@ class LicenceModel extends BaseModel { return company.name } + + $licenceName () { + const licenceName = this?.licenceDocumentHeader?.licenceName + + return licenceName || null + } + + $registeredTo () { + const registeredUserName = this?.licenceDocumentHeader?.registeredTo + + return registeredUserName || null + } } module.exports = LicenceModel diff --git a/app/services/licences/fetch-licence.service.js b/app/services/licences/fetch-licence.service.js index 5f79670294..8c0d450d34 100644 --- a/app/services/licences/fetch-licence.service.js +++ b/app/services/licences/fetch-licence.service.js @@ -51,7 +51,7 @@ async function _fetchLicence (id) { ]) }) .modify('licenceHolder') - .modify('registeredUserAndLicenceName') + .modify('registeredToAndLicenceName') return result } From f0bb4cdbc21afd03cef4beff214a3508f691ff56 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 18:34:48 +0000 Subject: [PATCH 07/12] Add unit tests for $licenceName() --- test/models/licence.model.test.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/models/licence.model.test.js b/test/models/licence.model.test.js index 7a0df1e6f8..e6a06258be 100644 --- a/test/models/licence.model.test.js +++ b/test/models/licence.model.test.js @@ -562,4 +562,33 @@ describe('Licence model', () => { }) }) }) + + describe('$licenceName', () => { + describe('when instance has not been set with the additional properties needed', () => { + it('returns null', () => { + const result = testRecord.$licenceName() + + expect(result).to.be.null() + }) + }) + + describe('when the instance has been set with the additional properties needed', () => { + beforeEach(async () => { + const { id: licenceId, licenceRef } = await LicenceHelper.add() + + await LicenceDocumentHeaderHelper.add({ + licenceName: 'Between Two Ferns', + licenceRef + }) + + testRecord = await LicenceModel.query().findById(licenceId).modify('registeredToAndLicenceName') + }) + + it('returns the licence name', async () => { + const result = testRecord.$licenceName() + + expect(result).to.equal('Between Two Ferns') + }) + }) + }) }) From dafde79130e0198478a7d153e9a9969eb52f6ef4 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 18:48:28 +0000 Subject: [PATCH 08/12] Correct the LicenceEntityHelper First the type default didn't match the comments or reflect what we intended to set it to. Also, after checking the real data when the type is `individual` we expect the name to be there email address. So, this change corrects these mistakes. --- test/support/helpers/licence-entity.helper.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/support/helpers/licence-entity.helper.js b/test/support/helpers/licence-entity.helper.js index 99356f4ca4..a832c285cc 100644 --- a/test/support/helpers/licence-entity.helper.js +++ b/test/support/helpers/licence-entity.helper.js @@ -13,7 +13,7 @@ const LicenceEntityModel = require('../../../app/models/licence-entity.model.js' * If no `data` is provided, default values will be used. These are * * - `id` - [random UUID] - * - `name` - Grace Hopper + * - `name` - grace.hopper@example.com * - `type` - individual * * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database @@ -39,8 +39,8 @@ async function add (data = {}) { function defaults (data = {}) { const defaults = { id: generateUUID(), - name: 'Grace Hopper', - type: 'Licence Holder' + name: 'grace.hopper@example.com', + type: 'individual' } return { From 8e7e88c4abe511a639355dd34f4e6bbdbb567b7e Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 18:50:31 +0000 Subject: [PATCH 09/12] Add unit tests for $registeredTo() --- test/models/licence.model.test.js | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/models/licence.model.test.js b/test/models/licence.model.test.js index e6a06258be..76b5218197 100644 --- a/test/models/licence.model.test.js +++ b/test/models/licence.model.test.js @@ -21,6 +21,8 @@ const LicenceDocumentModel = require('../../app/models/licence-document.model.js const LicenceDocumentHeaderHelper = require('../support/helpers/licence-document-header.helper.js') const LicenceDocumentHeaderModel = require('../../app/models/licence-document-header.model.js') const LicenceDocumentRoleHelper = require('../support/helpers/licence-document-role.helper.js') +const LicenceEntityRoleHelper = require('../support/helpers/licence-entity-role.helper.js') +const LicenceEntityHelper = require('../support/helpers/licence-entity.helper.js') const LicenceRoleHelper = require('../support/helpers/licence-role.helper.js') const LicenceVersionHelper = require('../support/helpers/licence-version.helper.js') const LicenceVersionModel = require('../../app/models/licence-version.model.js') @@ -591,4 +593,38 @@ describe('Licence model', () => { }) }) }) + + describe('$registeredTo', () => { + describe('when instance has not been set with the additional properties needed', () => { + it('returns null', () => { + const result = testRecord.$registeredTo() + + expect(result).to.be.null() + }) + }) + + describe('when the instance has been set with the additional properties needed', () => { + beforeEach(async () => { + const { id: licenceId, licenceRef } = await LicenceHelper.add() + const companyEntityId = 'c960a4a1-94f9-4c05-9db1-a70ce5d08738' + + await LicenceDocumentHeaderHelper.add({ + companyEntityId, + licenceName: 'Between Two Ferns', + licenceRef + }) + + const { id: licenceEntityId } = await LicenceEntityHelper.add() + await LicenceEntityRoleHelper.add({ companyEntityId, licenceEntityId }) + + testRecord = await LicenceModel.query().findById(licenceId).modify('registeredToAndLicenceName') + }) + + it('returns who the licence is registered to', async () => { + const result = testRecord.$registeredTo() + + expect(result).to.equal('grace.hopper@example.com') + }) + }) + }) }) From 34266f09ced5b3174b0c400a35e304add1895df0 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 19:13:47 +0000 Subject: [PATCH 10/12] Update tests to use new seeder --- test/models/licence.model.test.js | 28 ++++------- .../registered-to-and-licence-name.seeder.js | 47 +++++++++++++++++++ 2 files changed, 55 insertions(+), 20 deletions(-) create mode 100644 test/support/seeders/registered-to-and-licence-name.seeder.js diff --git a/test/models/licence.model.test.js b/test/models/licence.model.test.js index 76b5218197..7d45878ed6 100644 --- a/test/models/licence.model.test.js +++ b/test/models/licence.model.test.js @@ -21,13 +21,12 @@ const LicenceDocumentModel = require('../../app/models/licence-document.model.js const LicenceDocumentHeaderHelper = require('../support/helpers/licence-document-header.helper.js') const LicenceDocumentHeaderModel = require('../../app/models/licence-document-header.model.js') const LicenceDocumentRoleHelper = require('../support/helpers/licence-document-role.helper.js') -const LicenceEntityRoleHelper = require('../support/helpers/licence-entity-role.helper.js') -const LicenceEntityHelper = require('../support/helpers/licence-entity.helper.js') const LicenceRoleHelper = require('../support/helpers/licence-role.helper.js') const LicenceVersionHelper = require('../support/helpers/licence-version.helper.js') const LicenceVersionModel = require('../../app/models/licence-version.model.js') const RegionHelper = require('../support/helpers/region.helper.js') const RegionModel = require('../../app/models/region.model.js') +const RegisteredToAndLicenceNameSeeder = require('../support/seeders/registered-to-and-licence-name.seeder.js') const WorkflowHelper = require('../support/helpers/workflow.helper.js') const WorkflowModel = require('../../app/models/workflow.model.js') @@ -576,20 +575,17 @@ describe('Licence model', () => { describe('when the instance has been set with the additional properties needed', () => { beforeEach(async () => { - const { id: licenceId, licenceRef } = await LicenceHelper.add() + const licence = await LicenceHelper.add() - await LicenceDocumentHeaderHelper.add({ - licenceName: 'Between Two Ferns', - licenceRef - }) + await RegisteredToAndLicenceNameSeeder.seed(licence) - testRecord = await LicenceModel.query().findById(licenceId).modify('registeredToAndLicenceName') + testRecord = await LicenceModel.query().findById(licence.id).modify('registeredToAndLicenceName') }) it('returns the licence name', async () => { const result = testRecord.$licenceName() - expect(result).to.equal('Between Two Ferns') + expect(result).to.equal('My custom licence name') }) }) }) @@ -605,19 +601,11 @@ describe('Licence model', () => { describe('when the instance has been set with the additional properties needed', () => { beforeEach(async () => { - const { id: licenceId, licenceRef } = await LicenceHelper.add() - const companyEntityId = 'c960a4a1-94f9-4c05-9db1-a70ce5d08738' - - await LicenceDocumentHeaderHelper.add({ - companyEntityId, - licenceName: 'Between Two Ferns', - licenceRef - }) + const licence = await LicenceHelper.add() - const { id: licenceEntityId } = await LicenceEntityHelper.add() - await LicenceEntityRoleHelper.add({ companyEntityId, licenceEntityId }) + await RegisteredToAndLicenceNameSeeder.seed(licence) - testRecord = await LicenceModel.query().findById(licenceId).modify('registeredToAndLicenceName') + testRecord = await LicenceModel.query().findById(licence.id).modify('registeredToAndLicenceName') }) it('returns who the licence is registered to', async () => { diff --git a/test/support/seeders/registered-to-and-licence-name.seeder.js b/test/support/seeders/registered-to-and-licence-name.seeder.js new file mode 100644 index 0000000000..da1913ea08 --- /dev/null +++ b/test/support/seeders/registered-to-and-licence-name.seeder.js @@ -0,0 +1,47 @@ +'use strict' + +/** + * Seeder to setup registered to and licence name details for a LicenceModel + * @module RegisteredToAndLicenceNameSeeder + */ + +const LicenceDocumentHeaderHelper = require('../helpers/licence-document-header.helper.js') +const LicenceEntityRoleHelper = require('../helpers/licence-entity-role.helper.js') +const LicenceEntityHelper = require('../helpers/licence-entity.helper.js') + +/** + * Sets up the additional records needed for `$licenceName()` and `%registeredTo()` on the licence to return values + * + * The registered to and licence name details are seen on the view licence page (if a licence has them). Unfortunately, + * we have to link through a number of the legacy tables to extract the data. + * + * This seeder ensures the records needed are created and specifically support the `registeredToAndLicenceName()` + * modifier on the `LicenceModel`. + * + * @param {module:LicenceModel} licence - The licence instance we are setting up the records for + * @param {String} [licenceName] The custom licence name to use + */ +async function seed (licence, licenceName = 'My custom licence name') { + const { licenceRef } = licence + + // We get from the licence to the registered user via LicenceDocumentHeader and LicenceEntityRole which are linked + // by the same companyEntityId + const companyEntityId = 'c960a4a1-94f9-4c05-9db1-a70ce5d08738' + + // Create a licence document header record + await LicenceDocumentHeaderHelper.add({ + companyEntityId, + licenceName, + licenceRef + }) + + // Create the licence entity record. It's `name` field holds the user email that the licence is registered to + const { id: licenceEntityId } = await LicenceEntityHelper.add() + + // Create the licence entity role record that is the part of the link between the licence and the user email + await LicenceEntityRoleHelper.add({ companyEntityId, licenceEntityId }) +} + +module.exports = { + seed +} From 7a64a4eda758bc82bb0b78bd167dc7697178fe2f Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 19:36:03 +0000 Subject: [PATCH 11/12] Add JSDoc comments --- app/models/licence.model.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/app/models/licence.model.js b/app/models/licence.model.js index 849f5c4b90..4144fd0bd5 100644 --- a/app/models/licence.model.js +++ b/app/models/licence.model.js @@ -131,6 +131,10 @@ class LicenceModel extends BaseModel { ]) }) }, + /** + * registeredToAndLicenceName modifier fetches the linked `licenceDocumentHeader` which holds the licence name and + * adds to it the registered user's email address if one is set. + */ registeredToAndLicenceName (query) { query .withGraphFetched('licenceDocumentHeader') @@ -249,12 +253,42 @@ class LicenceModel extends BaseModel { return company.name } + /** + * Determine the licence name for the licence + * + * > We recommend adding the `registeredToAndLicenceName` modifier to your query to ensure the joined records are + * > available to determine this + * + * If set this is visible on the view licence page above the licence reference and on the legacy external view as a + * field in the summary tab. + * + * The licence name is a custom name the registered user of the licence can set. So, you will only see a licence name + * if the licence is registered to a user and they have chosen to set a name for it via the external UI. + * + * @returns {(string|null)} `null` if this instance does not have the additional properties needed to determine the + * licence name else the licence's custom name + */ $licenceName () { const licenceName = this?.licenceDocumentHeader?.licenceName return licenceName || null } + /** + * Determine who the licence is registered to + * + * > We recommend adding the `registeredToAndLicenceName` modifier to your query to ensure the joined records are + * > available to determine this + * + * If set this is visible on the view licence page below the licence reference. + * + * When an external user has an account they can add a licence via the external UI. We'll generate a letter with a + * code which gets sent to the licence's address. Once they receive it they can enter the code in the UI and they + * then become the registered user for it. + * + * @returns {(string|null)} `null` if this instance does not have the additional properties needed to determine who + * the licence is registered to else the email of the user + */ $registeredTo () { const registeredUserName = this?.licenceDocumentHeader?.registeredTo From bac7773ca301c94381886e28add6ed553c88c2d8 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 27 Jan 2024 19:37:50 +0000 Subject: [PATCH 12/12] Revert changes to view licence code As stated in the first commit, these will come through in a different PR. But hopefully we have demonstrated how the modifier and custom instance methods can be used to provide the registered user and licence name details. --- .../licences/view-licence.presenter.js | 17 ++++------- .../licences/fetch-licence.service.js | 28 ++++++------------- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/app/presenters/licences/view-licence.presenter.js b/app/presenters/licences/view-licence.presenter.js index 6e5d76d83e..67c7abad74 100644 --- a/app/presenters/licences/view-licence.presenter.js +++ b/app/presenters/licences/view-licence.presenter.js @@ -15,17 +15,16 @@ const { formatLongDate } = require('../base.presenter.js') * @returns {Object} The data formatted for the view template */ function go (licence) { - const { ends, expiredDate, id, licenceDocumentHeader, licenceHolder, licenceRef, region, startDate } = licence + const { expiredDate, id, licenceRef, region, startDate } = licence + const warning = _generateWarningMessage(licence) return { id, - documentId: licenceDocumentHeader.id, endDate: _endDate(expiredDate), - licenceHolder: _generateLicenceHolder(licenceHolder), licenceRef, region: region.displayName, startDate: formatLongDate(startDate), - warning: _generateWarningMessage(ends) + warning } } @@ -37,15 +36,9 @@ function _endDate (expiredDate) { return formatLongDate(expiredDate) } -function _generateLicenceHolder (licenceHolder) { - if (!licenceHolder) { - return 'Unregistered licence' - } - - return licenceHolder -} +function _generateWarningMessage (licence) { + const ends = licence.$ends() -function _generateWarningMessage (ends) { if (!ends) { return null } diff --git a/app/services/licences/fetch-licence.service.js b/app/services/licences/fetch-licence.service.js index 8c0d450d34..c16cd79df0 100644 --- a/app/services/licences/fetch-licence.service.js +++ b/app/services/licences/fetch-licence.service.js @@ -19,29 +19,19 @@ const LicenceModel = require('../../models/licence.model.js') async function go (id) { const licence = await _fetchLicence(id) - const data = await _data(licence) - - return data -} - -async function _data (licence) { - return { - ...licence, - ends: licence.$ends(), - licenceHolder: licence.$licenceHolder() - } + return licence } async function _fetchLicence (id) { - const result = await LicenceModel.query() + const result = LicenceModel.query() .findById(id) .select([ - 'licences.expiredDate', - 'licences.id', - 'licences.lapsedDate', - 'licences.licenceRef', - 'licences.revokedDate', - 'licences.startDate' + 'expiredDate', + 'id', + 'lapsedDate', + 'licenceRef', + 'revokedDate', + 'startDate' ]) .withGraphFetched('region') .modifyGraph('region', (builder) => { @@ -50,8 +40,6 @@ async function _fetchLicence (id) { 'displayName' ]) }) - .modify('licenceHolder') - .modify('registeredToAndLicenceName') return result }