From d201240f480a0c101f4e5f7b4627fae3e2786a63 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 13:33:41 +0100 Subject: [PATCH 01/10] Fix model migrations since last release > Warning! This will break local environments unless you manually remove existing tables and views in the `public` schema. We've been doing work for both return requirements and two-part tariff that has resulted in a number new migrations being added and existing ones being edited. However, because these 2 features are large, complex and constantly in flux we've often needed to make changes. Or, we simply got it wrong and the original migrations was incorrect. We've just spotted that a recent attempt to fix a migration actually ended up reverting an earlier fix! We've always had a policy that migrations are not 'fixed' until they are shipped to production. So, we're taking a pause and tidying up the currently 'unshipped' migrations. This means for example, where we have created a migration, then altered it a number of times that those alterations will be added to the original `create` then deleted. We'll also take the opportunity to get the files names back on track with what they should be. From 75d6678a7abff6a1b97735b743073ec2d9b2e5dc Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 14:02:18 +0100 Subject: [PATCH 02/10] Fix licence version purposes --- ...08_water-licence-version-purposes-table.js | 7 ++- ...717_alter-licence-version-purposes-view.js | 26 +++++++- ...226_alter-licence-version-purposes-view.js | 59 ------------------- .../helpers/licence-version-purpose.helper.js | 24 ++++---- 4 files changed, 41 insertions(+), 75 deletions(-) delete mode 100644 db/migrations/public/20240611200226_alter-licence-version-purposes-view.js diff --git a/db/migrations/legacy/20221108007008_water-licence-version-purposes-table.js b/db/migrations/legacy/20221108007008_water-licence-version-purposes-table.js index ed230d332b..09e14ee522 100644 --- a/db/migrations/legacy/20221108007008_water-licence-version-purposes-table.js +++ b/db/migrations/legacy/20221108007008_water-licence-version-purposes-table.js @@ -26,13 +26,16 @@ exports.up = function (knex) { table.decimal('daily_quantity') table.decimal('hourly_quantity') table.decimal('annual_quantity') - table.string('external_id').unique() - table.boolean('is_test') + table.string('external_id').notNullable() + table.boolean('is_test').notNullable().default(false) // Legacy timestamps // NOTE: They are not automatically set table.dateTime('date_created').notNullable() table.dateTime('date_updated').notNullable() + + // Constraints + table.unique(['external_id'], { useConstraint: true }) }) } diff --git a/db/migrations/public/20240426123717_alter-licence-version-purposes-view.js b/db/migrations/public/20240426123717_alter-licence-version-purposes-view.js index f1e475eec9..ed595b11ee 100644 --- a/db/migrations/public/20240426123717_alter-licence-version-purposes-view.js +++ b/db/migrations/public/20240426123717_alter-licence-version-purposes-view.js @@ -11,8 +11,8 @@ exports.up = function (knex) { view.as(knex('licence_version_purposes').withSchema('water').select([ 'licence_version_purpose_id AS id', 'licence_version_id', - 'purpose_primary_id', - 'purpose_secondary_id', + 'purpose_primary_id AS primary_purpose_id', + 'purpose_secondary_id AS secondary_purpose_id', 'purpose_use_id AS purpose_id', 'abstraction_period_start_day', 'abstraction_period_start_month', @@ -37,4 +37,26 @@ exports.down = function (knex) { return knex .schema .dropViewIfExists(viewName) + .createView(viewName, (view) => { + // NOTE: We have commented out unused columns from the source table + view.as(knex('licence_version_purposes').withSchema('water').select([ + 'licence_version_purpose_id AS id', + 'licence_version_id', + 'purpose_primary_id', + 'purpose_secondary_id', + 'purpose_use_id AS purpose_id', + 'abstraction_period_start_day', + 'abstraction_period_start_month', + 'abstraction_period_end_day', + 'abstraction_period_end_month', + 'time_limited_start_date', + 'time_limited_end_date', + 'notes', + 'annual_quantity', + 'external_id', + // 'is_test ', + 'date_created AS created_at', + 'date_updated AS updated_at' + ])) + }) } diff --git a/db/migrations/public/20240611200226_alter-licence-version-purposes-view.js b/db/migrations/public/20240611200226_alter-licence-version-purposes-view.js deleted file mode 100644 index 609cecd754..0000000000 --- a/db/migrations/public/20240611200226_alter-licence-version-purposes-view.js +++ /dev/null @@ -1,59 +0,0 @@ -'use strict' - -const viewName = 'licence_version_purposes' - -exports.up = function (knex) { - return knex - .schema - .dropViewIfExists(viewName) - .createView(viewName, (view) => { - // NOTE: We have commented out unused columns from the source table - view.as(knex('licence_version_purposes').withSchema('water').select([ - 'licence_version_purpose_id AS id', - 'licence_version_id', - 'purpose_primary_id AS primary_purpose_id', - 'purpose_secondary_id AS secondary_purpose_id', - 'purpose_use_id AS purpose_id', - 'abstraction_period_start_day', - 'abstraction_period_start_month', - 'abstraction_period_end_day', - 'abstraction_period_end_month', - 'time_limited_start_date', - 'time_limited_end_date', - 'notes', - 'annual_quantity', - 'external_id', - // 'is_test ', - 'date_created AS created_at', - 'date_updated AS updated_at' - ])) - }) -} - -exports.down = function (knex) { - return knex - .schema - .dropViewIfExists(viewName) - .createView(viewName, (view) => { - // NOTE: We have commented out unused columns from the source table - view.as(knex('licence_version_purposes').withSchema('water').select([ - 'licence_version_purpose_id AS id', - 'licence_version_id', - 'purpose_primary_id', - 'purpose_secondary_id', - 'purpose_use_id AS purpose_id', - 'abstraction_period_start_day', - 'abstraction_period_start_month', - 'abstraction_period_end_day', - 'abstraction_period_end_month', - 'time_limited_start_date', - 'time_limited_end_date', - 'notes', - 'annual_quantity', - 'external_id', - // 'is_test ', - 'date_created AS created_at', - 'date_updated AS updated_at' - ])) - }) -} diff --git a/test/support/helpers/licence-version-purpose.helper.js b/test/support/helpers/licence-version-purpose.helper.js index 244a768838..b31317375f 100644 --- a/test/support/helpers/licence-version-purpose.helper.js +++ b/test/support/helpers/licence-version-purpose.helper.js @@ -13,17 +13,17 @@ const { randomInteger } = require('../general.js') * * If no `data` is provided, default values will be used. These are * - * - `licenceVersionPurposeId` - [random UUID] - * - `licenceVersionId` - [random UUID] - * - `primaryPurposeId` - [random UUID] - * - `secondaryPurposeId` - [random UUID] - * - `purposeUseId` - [random UUID] * - `abstractionPeriodStartDay` - [1] * - `abstractionPeriodStartMonth` - [1] * - `abstractionPeriodEndDay` - [31] * - `abstractionPeriodEndMonth` - [3] - * - `dateCreated` - new Date() - * - `dateUpdated` - new Date() + * - `externalId` - [randomly generated - 9:99999] + * - `licenceVersionId` - [random UUID] + * - `primaryPurposeId` - [random UUID] + * - `purposeId` - [random UUID] + * - `secondaryPurposeId` - [random UUID] + * - `created` - new Date() + * - `updated` - new Date() * * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database * @@ -53,14 +53,14 @@ function defaults (data = {}) { abstractionPeriodStartMonth: 1, abstractionPeriodEndDay: 31, abstractionPeriodEndMonth: 3, - annualQuantity: 1000, - createdAt: timestamp, - updatedAt: timestamp, - externalId: `9:${randomInteger(10000, 99999)}:1:0`, + externalId: `9:${randomInteger(10000, 99999)}`, licenceVersionId: generateUUID(), primaryPurposeId: generateUUID(), + purposeId: generateUUID(), secondaryPurposeId: generateUUID(), - purposeId: generateUUID() + // INFO: The table does not have a default for the date columns + createdAt: timestamp, + updatedAt: timestamp } return { From 029d68cd593e5a2c4a8ec11463eb26143612ab69 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 14:09:33 +0100 Subject: [PATCH 03/10] Fix alter licences view file name --- ...pt-billing-column.js => 20240429145100_alter-licences-view.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename db/migrations/public/{20240429145100_alter-licences-view-add-tpt-billing-column.js => 20240429145100_alter-licences-view.js} (100%) diff --git a/db/migrations/public/20240429145100_alter-licences-view-add-tpt-billing-column.js b/db/migrations/public/20240429145100_alter-licences-view.js similarity index 100% rename from db/migrations/public/20240429145100_alter-licences-view-add-tpt-billing-column.js rename to db/migrations/public/20240429145100_alter-licences-view.js From 9b26e149749d4623a4289d02bd02a61703fe164f Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 14:23:14 +0100 Subject: [PATCH 04/10] Fix financial agreements --- app/models/financial-agreement.model.js | 2 +- app/presenters/licences/set-up.presenter.js | 2 +- .../licences/fetch-agreements.service.js | 2 +- .../fetch-licence-agreements.service.js | 2 +- ...8007030_water-financial-agreement-types.js | 8 +++---- ...25140_create-financial-agreements-view.js} | 3 ++- test/models/financial-agreement.model.test.js | 23 +++++++++++-------- .../licences/set-up.presenter.test.js | 12 +++++----- .../licences/fetch-agreements.service.test.js | 2 +- .../view-licence-set-up.service.test.js | 2 +- .../helpers/financial-agreement.helper.js | 23 +++++++++++-------- 11 files changed, 44 insertions(+), 37 deletions(-) rename db/migrations/public/{20240430125140_water-add-financial-agreements-view.js => 20240430125140_create-financial-agreements-view.js} (81%) diff --git a/app/models/financial-agreement.model.js b/app/models/financial-agreement.model.js index 43a6de29a6..dada0cd83e 100644 --- a/app/models/financial-agreement.model.js +++ b/app/models/financial-agreement.model.js @@ -16,7 +16,7 @@ class FinancialAgreementModel extends BaseModel { static get relationMappings () { return { - licenceAgreementTypes: { + licenceAgreements: { relation: Model.HasManyRelation, modelClass: 'licence-agreement.model', join: { diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index 229c6b4e7a..0ef8578f76 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -167,7 +167,7 @@ function _endsSixYearsAgo (endDate) { } function _financialAgreementCode (agreement) { - return agreement.financialAgreements[0].financialAgreementCode + return agreement.financialAgreements[0].code } function _returnVersions (returnVersions = [{}]) { diff --git a/app/services/licences/fetch-agreements.service.js b/app/services/licences/fetch-agreements.service.js index 4ac2e49205..0d3a1b4aea 100644 --- a/app/services/licences/fetch-agreements.service.js +++ b/app/services/licences/fetch-agreements.service.js @@ -30,7 +30,7 @@ async function _fetch (licenceRef) { .withGraphFetched('financialAgreements') .modifyGraph('financialAgreements', (builder) => { builder.select([ - 'financialAgreementCode' + 'code' ]) }) .orderBy([ diff --git a/app/services/return-requirements/fetch-licence-agreements.service.js b/app/services/return-requirements/fetch-licence-agreements.service.js index fdd86295ac..eda1d88b4a 100644 --- a/app/services/return-requirements/fetch-licence-agreements.service.js +++ b/app/services/return-requirements/fetch-licence-agreements.service.js @@ -28,7 +28,7 @@ async function _fetch (licenceRef) { builder.select([ 'id' ]) - .where('financialAgreementCode', 'S127') + .where('code', 'S127') }) } diff --git a/db/migrations/legacy/20221108007030_water-financial-agreement-types.js b/db/migrations/legacy/20221108007030_water-financial-agreement-types.js index aa9eeab44e..8956466175 100644 --- a/db/migrations/legacy/20221108007030_water-financial-agreement-types.js +++ b/db/migrations/legacy/20221108007030_water-financial-agreement-types.js @@ -11,10 +11,10 @@ exports.up = function (knex) { table.uuid('financial_agreement_type_id').primary().defaultTo(knex.raw('gen_random_uuid()')) // Data - table.string('financial_agreement_code') - table.string('description') - table.boolean('disabled') - table.boolean('is_test') + table.string('financial_agreement_code').notNullable() + table.string('description').notNullable() + table.boolean('disabled').default(false) + table.boolean('is_test').notNullable().default(false) // Legacy timestamps table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) diff --git a/db/migrations/public/20240430125140_water-add-financial-agreements-view.js b/db/migrations/public/20240430125140_create-financial-agreements-view.js similarity index 81% rename from db/migrations/public/20240430125140_water-add-financial-agreements-view.js rename to db/migrations/public/20240430125140_create-financial-agreements-view.js index 2c785b8179..c503c4e9bb 100644 --- a/db/migrations/public/20240430125140_water-add-financial-agreements-view.js +++ b/db/migrations/public/20240430125140_create-financial-agreements-view.js @@ -6,9 +6,10 @@ exports.up = function (knex) { return knex .schema .createView(viewName, (view) => { + // NOTE: We have commented out unused columns from the source table view.as(knex('financial_agreement_types').withSchema('water').select([ 'financial_agreement_type_id AS id', - 'financial_agreement_code', + 'financial_agreement_code AS code', 'description', 'disabled', // 'is_test', diff --git a/test/models/financial-agreement.model.test.js b/test/models/financial-agreement.model.test.js index a1dd612a77..d5edcd9dc1 100644 --- a/test/models/financial-agreement.model.test.js +++ b/test/models/financial-agreement.model.test.js @@ -37,35 +37,38 @@ describe('Financial Agreement model', () => { }) describe('Relationships', () => { - describe('when linking to licence entity roles', () => { + describe('when linking to licence agreements', () => { let testLicenceAgreements beforeEach(async () => { testRecord = await FinancialAgreementHelper.add() - const { id: financialAgreementId } = testRecord - - testLicenceAgreements = await LicenceAgreementHelper.add({ financialAgreementId }) + testLicenceAgreements = [] + for (let i = 0; i < 2; i++) { + const licenceAgreement = await LicenceAgreementHelper.add({ financialAgreementId: testRecord.id }) + testLicenceAgreements.push(licenceAgreement) + } }) it('can successfully run a related query', async () => { const query = await FinancialAgreementModel.query() - .innerJoinRelated('licenceAgreementTypes') + .innerJoinRelated('licenceAgreements') expect(query).to.exist() }) - it('can eager load the licence entity roles', async () => { + it('can eager load the licence agreements', async () => { const result = await FinancialAgreementModel.query() .findById(testRecord.id) - .withGraphFetched('licenceAgreementTypes') + .withGraphFetched('licenceAgreements') expect(result).to.be.instanceOf(FinancialAgreementModel) expect(result.id).to.equal(testRecord.id) - expect(result.licenceAgreementTypes).to.be.an.array() - expect(result.licenceAgreementTypes[0]).to.be.an.instanceOf(LicenceAgreementModel) - expect(result.licenceAgreementTypes).to.include(testLicenceAgreements) + expect(result.licenceAgreements).to.be.an.array() + expect(result.licenceAgreements[0]).to.be.an.instanceOf(LicenceAgreementModel) + expect(result.licenceAgreements).to.include(testLicenceAgreements[0]) + expect(result.licenceAgreements).to.include(testLicenceAgreements[1]) }) }) }) diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index 14fc7c0a8b..b9eb4422ad 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -20,7 +20,7 @@ describe('Licence Set Up presenter', () => { startDate: new Date('2020-01-01'), endDate: null, dateSigned: null, - financialAgreements: [{ financialAgreementCode: 'S127' }] + financialAgreements: [{ code: 'S127' }] } const chargeVersion = { @@ -226,7 +226,7 @@ describe('Licence Set Up presenter', () => { describe('when the financial agreement code ', () => { describe('is for Two-part tariff ', () => { beforeEach(() => { - agreement.financialAgreements[0].financialAgreementCode = 'S127' + agreement.financialAgreements[0].code = 'S127' }) it('correctly maps the code to the description', () => { const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) @@ -237,7 +237,7 @@ describe('Licence Set Up presenter', () => { describe('is for Canal and Rivers Trust, supported source (S130S) ', () => { beforeEach(() => { - agreement.financialAgreements[0].financialAgreementCode = 'S130S' + agreement.financialAgreements[0].code = 'S130S' }) it('correctly maps the code to the description', () => { const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) @@ -248,7 +248,7 @@ describe('Licence Set Up presenter', () => { describe('is for Canal and Rivers Trust, unsupported source (S130U)', () => { beforeEach(() => { - agreement.financialAgreements[0].financialAgreementCode = 'S130U' + agreement.financialAgreements[0].code = 'S130U' }) it('correctly maps the code to the description', () => { const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) @@ -259,7 +259,7 @@ describe('Licence Set Up presenter', () => { describe('is for Abatement', () => { beforeEach(() => { - agreement.financialAgreements[0].financialAgreementCode = 'S126' + agreement.financialAgreements[0].code = 'S126' }) it('correctly maps the code to the description', () => { const result = SetUpPresenter @@ -272,7 +272,7 @@ describe('Licence Set Up presenter', () => { describe('when the licence is less than 6 years old and all the actions are available for an agreement', () => { beforeEach(() => { - agreement.financialAgreements[0].financialAgreementCode = 'S127' + agreement.financialAgreements[0].code = 'S127' }) it('shows delete, end and recalculate bills actions', () => { diff --git a/test/services/licences/fetch-agreements.service.test.js b/test/services/licences/fetch-agreements.service.test.js index 54e4a64fc9..58e1073353 100644 --- a/test/services/licences/fetch-agreements.service.test.js +++ b/test/services/licences/fetch-agreements.service.test.js @@ -38,7 +38,7 @@ describe('Fetch Agreements service', () => { { dateSigned: result[0].dateSigned, endDate: null, - financialAgreements: [{ financialAgreementCode: 'S127' }], + financialAgreements: [{ code: 'S127' }], id: result[0].id, startDate: result[0].startDate } diff --git a/test/services/licences/view-licence-set-up.service.test.js b/test/services/licences/view-licence-set-up.service.test.js index 2f411f83e8..55db8207b8 100644 --- a/test/services/licences/view-licence-set-up.service.test.js +++ b/test/services/licences/view-licence-set-up.service.test.js @@ -33,7 +33,7 @@ describe('View Licence Set Up service', () => { startDate: new Date('2020-01-01'), endDate: null, dateSigned: null, - financialAgreements: [{ financialAgreementCode: 'S127' }] + financialAgreements: [{ code: 'S127' }] } ]) diff --git a/test/support/helpers/financial-agreement.helper.js b/test/support/helpers/financial-agreement.helper.js index ec7d8eff60..1216a34a8d 100644 --- a/test/support/helpers/financial-agreement.helper.js +++ b/test/support/helpers/financial-agreement.helper.js @@ -4,18 +4,16 @@ * @module FinancialAgreementHelper */ -const { generateUUID } = require('../../../app/lib/general.lib.js') const FinancialAgreementModel = require('../../../app/models/financial-agreement.model.js') +const { randomInteger } = require('../general.js') /** * Add a new financial agreement * * If no `data` is provided, default values will be used. These are * - * - `financialAgreementId` - [random UUID] - * - `financialAgreementCode` - INST - * - `description` - Installment - * - `disabled` - false + * - `code` - [randomly generated - S127] + * - `description` - [randomly generated - Section S127] * * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database * @@ -38,11 +36,11 @@ async function add (data = {}) { * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database */ function defaults (data = {}) { + const code = data.code ? data.code : generateFinancialAgreementCode() + const defaults = { - id: generateUUID(), - financialAgreementCode: 'S127', - description: 'Section 127 (Two Part Tariff)', - disabled: false + code, + description: `Section ${code}` } return { @@ -51,7 +49,12 @@ function defaults (data = {}) { } } +function generateFinancialAgreementCode () { + return `S${randomInteger(100, 199)}` +} + module.exports = { add, - defaults + defaults, + generateFinancialAgreementCode } From 5430b62c562e16ba86b56fea12cb2271311fcce5 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 14:30:11 +0100 Subject: [PATCH 05/10] Fix licence agreements --- app/models/licence-agreement.model.js | 4 +-- .../licences/fetch-agreements.service.js | 9 ++--- .../fetch-licence-agreements.service.js | 4 +-- ...20221108007031_water-licence-agreements.js | 10 +++--- ...0133702_create-licence-agreements-view.js} | 3 +- test/models/licence-agreement.model.test.js | 20 +++++------ .../licences/fetch-agreements.service.test.js | 35 +++++++++++-------- .../fetch-licence-agreements.service.test.js | 18 ++++------ .../helpers/licence-agreement.helper.js | 10 ++---- 9 files changed, 55 insertions(+), 58 deletions(-) rename db/migrations/public/{20240430133702_water-add-licence-agreements-view.js => 20240430133702_create-licence-agreements-view.js} (85%) diff --git a/app/models/licence-agreement.model.js b/app/models/licence-agreement.model.js index 4805dd5c88..c73c54123e 100644 --- a/app/models/licence-agreement.model.js +++ b/app/models/licence-agreement.model.js @@ -16,8 +16,8 @@ class LicenceAgreementModel extends BaseModel { static get relationMappings () { return { - financialAgreements: { - relation: Model.HasManyRelation, + financialAgreement: { + relation: Model.HasOneRelation, modelClass: 'financial-agreement.model', join: { from: 'licenceAgreements.financialAgreementId', diff --git a/app/services/licences/fetch-agreements.service.js b/app/services/licences/fetch-agreements.service.js index 0d3a1b4aea..890fdb03bf 100644 --- a/app/services/licences/fetch-agreements.service.js +++ b/app/services/licences/fetch-agreements.service.js @@ -8,7 +8,7 @@ const LicenceAgreementModel = require('../../models/licence-agreement.model.js') /** - * Fetches charge version data needed for the view '/licences/{id}/set-up` page + * Fetches licence agreements data needed for the view '/licences/{id}/set-up` page * * @param {string} licenceRef - The licence ref for the licence to fetch licence agreements for * @@ -25,11 +25,12 @@ async function _fetch (licenceRef) { 'id', 'startDate', 'endDate', - 'dateSigned' + 'signedOn' ]) - .withGraphFetched('financialAgreements') - .modifyGraph('financialAgreements', (builder) => { + .withGraphFetched('financialAgreement') + .modifyGraph('financialAgreement', (builder) => { builder.select([ + 'id', 'code' ]) }) diff --git a/app/services/return-requirements/fetch-licence-agreements.service.js b/app/services/return-requirements/fetch-licence-agreements.service.js index eda1d88b4a..80d5359161 100644 --- a/app/services/return-requirements/fetch-licence-agreements.service.js +++ b/app/services/return-requirements/fetch-licence-agreements.service.js @@ -23,8 +23,8 @@ async function _fetch (licenceRef) { .where('licenceRef', licenceRef) .whereNull('endDate') .orWhere('endDate', '>=', new Date()) - .withGraphFetched('financialAgreements') - .modifyGraph('financialAgreements', (builder) => { + .withGraphFetched('financialAgreement') + .modifyGraph('financialAgreement', (builder) => { builder.select([ 'id' ]) diff --git a/db/migrations/legacy/20221108007031_water-licence-agreements.js b/db/migrations/legacy/20221108007031_water-licence-agreements.js index ab2c45f7a4..80525f9b20 100644 --- a/db/migrations/legacy/20221108007031_water-licence-agreements.js +++ b/db/migrations/legacy/20221108007031_water-licence-agreements.js @@ -11,14 +11,14 @@ exports.up = function (knex) { table.uuid('licence_agreement_id').primary().defaultTo(knex.raw('gen_random_uuid()')) // Data - table.uuid('financial_agreement_type_id') - table.string('licence_ref') - table.date('start_date') + table.uuid('financial_agreement_type_id').notNullable() + table.string('licence_ref').notNullable() + table.date('start_date').notNullable() table.date('end_date') table.date('date_signed') table.date('date_deleted') - table.string('source') - table.boolean('is_test') + table.string('source').notNullable().default('wrls') + table.boolean('is_test').notNullable().default(false) // Legacy timestamps table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) diff --git a/db/migrations/public/20240430133702_water-add-licence-agreements-view.js b/db/migrations/public/20240430133702_create-licence-agreements-view.js similarity index 85% rename from db/migrations/public/20240430133702_water-add-licence-agreements-view.js rename to db/migrations/public/20240430133702_create-licence-agreements-view.js index 2e2b5b745d..fc24a2760b 100644 --- a/db/migrations/public/20240430133702_water-add-licence-agreements-view.js +++ b/db/migrations/public/20240430133702_create-licence-agreements-view.js @@ -6,13 +6,14 @@ exports.up = function (knex) { return knex .schema .createView(viewName, (view) => { + // NOTE: We have commented out unused columns from the source table view.as(knex('licence_agreements').withSchema('water').select([ 'licence_agreement_id AS id', 'financial_agreement_type_id AS financial_agreement_id', 'licence_ref', 'start_date', 'end_date', - 'date_signed', + 'date_signed AS signed_on', 'date_deleted AS deleted_at', 'source', // 'is_test', diff --git a/test/models/licence-agreement.model.test.js b/test/models/licence-agreement.model.test.js index 14134264eb..92c7d79fb5 100644 --- a/test/models/licence-agreement.model.test.js +++ b/test/models/licence-agreement.model.test.js @@ -39,35 +39,33 @@ describe('Licence Agreement model', () => { }) describe('Relationships', () => { - describe('when linking to financial agreements', () => { - let testFinancialAgreements + describe('when linking to financial agreement', () => { + let testFinancialAgreement beforeEach(async () => { - testFinancialAgreements = await FinancialAgreementHelper.add() - - const { id: financialAgreementId } = testFinancialAgreements + testFinancialAgreement = await FinancialAgreementHelper.add() + const { id: financialAgreementId } = testFinancialAgreement testRecord = await LicenceAgreementHelper.add({ financialAgreementId }) }) it('can successfully run a related query', async () => { const query = await LicenceAgreementModel.query() - .innerJoinRelated('financialAgreements') + .innerJoinRelated('financialAgreement') expect(query).to.exist() }) - it('can eager load the financial agreements', async () => { + it('can eager load the financial agreement', async () => { const result = await LicenceAgreementModel.query() .findById(testRecord.id) - .withGraphFetched('financialAgreements') + .withGraphFetched('financialAgreement') expect(result).to.be.instanceOf(LicenceAgreementModel) expect(result.id).to.equal(testRecord.id) - expect(result.financialAgreements).to.be.an.array() - expect(result.financialAgreements[0]).to.be.an.instanceOf(FinancialAgreementModel) - expect(result.financialAgreements).to.include(testFinancialAgreements) + expect(result.financialAgreement).to.be.an.instanceOf(FinancialAgreementModel) + expect(result.financialAgreement).to.equal(testFinancialAgreement) }) }) diff --git a/test/services/licences/fetch-agreements.service.test.js b/test/services/licences/fetch-agreements.service.test.js index 58e1073353..75255d48e6 100644 --- a/test/services/licences/fetch-agreements.service.test.js +++ b/test/services/licences/fetch-agreements.service.test.js @@ -17,7 +17,7 @@ const FetchAgreementsService = require('../../../app/services/licences/fetch-agreements.service.js') describe('Fetch Agreements service', () => { - let testRecord + const licenceRef = '01/12/34/1000' beforeEach(async () => { await DatabaseSupport.clean() @@ -25,24 +25,29 @@ describe('Fetch Agreements service', () => { describe('when the licence has agreements data', () => { beforeEach(async () => { - const financialAgreement = await FinancialAgreementHelper.add() + const financialAgreement = await FinancialAgreementHelper.add({ + id: '970168ce-06c3-4823-b84d-9da30b742bb8', + code: 'S127' + }) - testRecord = await LicenceAgreementHelper.add({ - financial_agreement_id: financialAgreement.id + await LicenceAgreementHelper.add({ + endDate: new Date('2040-05-01'), + financialAgreementId: financialAgreement.id, + licenceRef, + startDate: new Date('2022-04-01'), + signedOn: new Date('2022-04-01') }) }) + it('returns the matching agreements data', async () => { - const result = await FetchAgreementsService.go(testRecord.licenceRef) - - expect(result).to.equal([ - { - dateSigned: result[0].dateSigned, - endDate: null, - financialAgreements: [{ code: 'S127' }], - id: result[0].id, - startDate: result[0].startDate - } - ]) + const results = await FetchAgreementsService.go(licenceRef) + + expect(results[0]).to.equal({ + startDate: new Date('2022-04-01'), + signedOn: new Date('2022-04-01'), + endDate: new Date('2040-05-01'), + financialAgreement: { id: '970168ce-06c3-4823-b84d-9da30b742bb8', code: 'S127' } + }, { skip: ['id'] }) }) }) }) diff --git a/test/services/return-requirements/fetch-licence-agreements.service.test.js b/test/services/return-requirements/fetch-licence-agreements.service.test.js index 8ad420515d..86e08153a0 100644 --- a/test/services/return-requirements/fetch-licence-agreements.service.test.js +++ b/test/services/return-requirements/fetch-licence-agreements.service.test.js @@ -22,7 +22,7 @@ describe('Return Requirements - Fetch Licence Agreements service', () => { beforeEach(async () => { await DatabaseSupport.clean() - const financialAgreement = await FinancialAgreementHelper.add() + const financialAgreement = await FinancialAgreementHelper.add({ code: 'S127' }) financialAgreementId = financialAgreement.id }) @@ -41,14 +41,12 @@ describe('Return Requirements - Fetch Licence Agreements service', () => { licenceRef: licenceAgreement.licenceRef, startDate: licenceAgreement.startDate, endDate: null, - dateSigned: licenceAgreement.dateSigned, + signedOn: licenceAgreement.signedOn, deletedAt: null, - source: 'nald', + source: 'wrls', createdAt: licenceAgreement.createdAt, updatedAt: licenceAgreement.updatedAt, - financialAgreements: [{ - id: financialAgreementId - }] + financialAgreement: { id: financialAgreementId } }]) }) }) @@ -67,14 +65,12 @@ describe('Return Requirements - Fetch Licence Agreements service', () => { licenceRef: licenceAgreement.licenceRef, startDate: licenceAgreement.startDate, endDate: new Date('2099-01-01'), - dateSigned: licenceAgreement.dateSigned, + signedOn: licenceAgreement.signedOn, deletedAt: null, - source: 'nald', + source: 'wrls', createdAt: licenceAgreement.createdAt, updatedAt: licenceAgreement.updatedAt, - financialAgreements: [{ - id: financialAgreementId - }] + financialAgreement: { id: financialAgreementId } }]) }) }) diff --git a/test/support/helpers/licence-agreement.helper.js b/test/support/helpers/licence-agreement.helper.js index 841a093ed8..4d557d4117 100644 --- a/test/support/helpers/licence-agreement.helper.js +++ b/test/support/helpers/licence-agreement.helper.js @@ -13,11 +13,9 @@ const LicenceHelper = require('./licence.helper.js') * * If no `data` is provided, default values will be used. These are * - * - `licenceAgreementId` - [random UUID] + * - `financialAgreementId` - [random UUID] * - `licenceRef` - [randomly generated - 01/123] * - `startDate` - 2023-01-01 - * - `dateSigned` - 2022-12-31 - * - `source` - 'nald' * * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database * @@ -41,11 +39,9 @@ async function add (data = {}) { */ function defaults (data = {}) { const defaults = { - id: generateUUID(), + financialAgreementId: generateUUID(), licenceRef: LicenceHelper.generateLicenceRef(), - startDate: new Date('2023-01-01'), - dateSigned: new Date('2022-01-01'), - source: 'nald' + startDate: new Date('2023-01-01') } return { From ac0a6b9c2b77b0fdfdc2a606cf8672933491dda7 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 14:31:23 +0100 Subject: [PATCH 06/10] Fix file name --- ...umn-type.js => 20240503155022_alter-review-charge-elements.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename db/migrations/public/{20240503155022_alter-review-charge-elements-column-type.js => 20240503155022_alter-review-charge-elements.js} (100%) diff --git a/db/migrations/public/20240503155022_alter-review-charge-elements-column-type.js b/db/migrations/public/20240503155022_alter-review-charge-elements.js similarity index 100% rename from db/migrations/public/20240503155022_alter-review-charge-elements-column-type.js rename to db/migrations/public/20240503155022_alter-review-charge-elements.js From f12b51bdd62364afc04fe4cb2cbb740db21731d1 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 14:43:29 +0100 Subject: [PATCH 07/10] Fix company contacts --- .../20221108003009_crm-v2-company-contacts.js | 27 +++++++-------- ...0513134550_create-company-contacts-view.js | 33 ++++++++----------- .../support/helpers/company-contact.helper.js | 4 ++- 3 files changed, 27 insertions(+), 37 deletions(-) diff --git a/db/migrations/legacy/20221108003009_crm-v2-company-contacts.js b/db/migrations/legacy/20221108003009_crm-v2-company-contacts.js index 16b499bf6f..57b37d1698 100644 --- a/db/migrations/legacy/20221108003009_crm-v2-company-contacts.js +++ b/db/migrations/legacy/20221108003009_crm-v2-company-contacts.js @@ -2,10 +2,6 @@ const tableName = 'company_contacts' -/** - * @param { import("knex").Knex } knex - * @returns { Promise } - */ exports.up = function (knex) { return knex .schema @@ -15,26 +11,25 @@ exports.up = function (knex) { table.uuid('company_contact_id').primary().defaultTo(knex.raw('gen_random_uuid()')) // Data - table.boolean('is_default') - table.boolean('is_test') - table.boolean('water_abstraction_alerts_enabled') - table.date('end_date') - table.date('start_date') + table.uuid('company_id').notNullable() + table.uuid('contact_id').notNullable() + table.uuid('role_id').notNullable() + table.boolean('is_default').notNullable().defaultTo(false) table.string('email_address') - table.uuid('company_id') - table.uuid('contact_id') - table.uuid('role_id') + table.date('start_date').notNullable() + table.date('end_date') + table.boolean('is_test').notNullable().defaultTo(false) + table.boolean('water_abstraction_alerts_enabled').defaultTo(false) // Legacy timestamps table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) table.timestamp('date_updated', { useTz: false }).notNullable().defaultTo(knex.fn.now()) + + // Constraints + table.unique(['company_id', 'contact_id', 'role_id', 'start_date'], { useConstraint: true }) }) } -/** - * @param { import("knex").Knex } knex - * @returns { Promise } - */ exports.down = function (knex) { return knex .schema diff --git a/db/migrations/public/20240513134550_create-company-contacts-view.js b/db/migrations/public/20240513134550_create-company-contacts-view.js index bad1d2f0e4..f26199459b 100644 --- a/db/migrations/public/20240513134550_create-company-contacts-view.js +++ b/db/migrations/public/20240513134550_create-company-contacts-view.js @@ -1,36 +1,29 @@ 'use strict' const viewName = 'company_contacts' -/** - * @param { import("knex").Knex } knex - * @returns { Promise } - */ + exports.up = function (knex) { return knex .schema .createView(viewName, (view) => { // NOTE: We have commented out unused columns from the source table view.as(knex('company_contacts').withSchema('crm_v2').select([ - 'company_contacts.company_contact_id AS id', - 'company_contacts.company_id', - 'company_contacts.contact_id', - 'company_contacts.role_id', - 'company_contacts.date_created AS created_at', - 'company_contacts.date_updated AS updated_at' - // company_contacts.is_default - // company_contacts.email_address - // company_contacts.start_date - // company_contacts.end_date - // company_contacts.is_test - // company_contacts.water_abstraction_alerts_enabled + 'company_contact_id AS id', + 'company_id', + 'contact_id', + 'role_id', + 'is_default AS default', + 'start_date', + 'water_abstraction_alerts_enabled AS abstraction_alerts', + // email_address + // end_date + // is_test + 'date_created AS created_at', + 'date_updated AS updated_at' ])) }) } -/** - * @param { import("knex").Knex } knex - * @returns { Promise } - */ exports.down = function (knex) { return knex .schema diff --git a/test/support/helpers/company-contact.helper.js b/test/support/helpers/company-contact.helper.js index ccf8ab43f4..c1df62b991 100644 --- a/test/support/helpers/company-contact.helper.js +++ b/test/support/helpers/company-contact.helper.js @@ -15,6 +15,7 @@ const { generateUUID } = require('../../../app/lib/general.lib.js') * - `companyId` - [random UUID] * - `contactId` - [random UUID] * - `roleId` - [random UUID] + * - `startDate` - 2022-04-01 * * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database * @@ -40,7 +41,8 @@ function defaults (data = {}) { const defaults = { companyId: generateUUID(), contactId: generateUUID(), - roleId: generateUUID() + roleId: generateUUID(), + startDate: new Date('2022-04-01') } return { From 304c823dbfbc57789d9258a479b61f2c404ba7ef Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 17:32:55 +0100 Subject: [PATCH 08/10] Fix scheduled notifications --- app/models/scheduled-notification.model.js | 9 +-- .../licences/fetch-communications.service.js | 8 +-- ...108007033_water-scheduled-notification.js} | 43 ++++++--------- ...450_create-scheduled-notifications-view.js | 55 ++++++++----------- ...led-notification-add-message-ref-column.js | 43 --------------- .../scheduled-notification.model.test.js | 17 +++--- .../fetch-communications.service.test.js | 37 +++++++++---- .../helpers/scheduled-notification.helper.js | 12 ++-- 8 files changed, 88 insertions(+), 136 deletions(-) rename db/migrations/legacy/{20221108007033_water-scheduled-notifications.js => 20221108007033_water-scheduled-notification.js} (66%) delete mode 100644 db/migrations/public/20240516044126_alter-scheduled-notification-add-message-ref-column.js diff --git a/app/models/scheduled-notification.model.js b/app/models/scheduled-notification.model.js index 9829fd3655..7136be9f77 100644 --- a/app/models/scheduled-notification.model.js +++ b/app/models/scheduled-notification.model.js @@ -1,16 +1,17 @@ 'use strict' /** - * Model for scheduled_notification (water.scheduled_notification) + * Model for scheduled_notifications (water.scheduled_notification) * @module ScheduledNotificationModel */ -const BaseModel = require('./base.model.js') const { Model } = require('objection') +const BaseModel = require('./base.model.js') + class ScheduledNotificationModel extends BaseModel { static get tableName () { - return 'scheduled_notification' + return 'scheduledNotifications' } static get relationMappings () { @@ -19,7 +20,7 @@ class ScheduledNotificationModel extends BaseModel { relation: Model.HasOneRelation, modelClass: 'event.model', join: { - from: 'scheduled_notification.eventId', + from: 'scheduledNotifications.eventId', to: 'events.id' } } diff --git a/app/services/licences/fetch-communications.service.js b/app/services/licences/fetch-communications.service.js index 12335e4630..8b91b0c565 100644 --- a/app/services/licences/fetch-communications.service.js +++ b/app/services/licences/fetch-communications.service.js @@ -32,17 +32,17 @@ async function _fetch (licenceRef, page) { 'messageRef' ]) .where('licences', '@>', `["${licenceRef}"]`) - .andWhere('notify_status', 'in', ['delivered', 'received']) + .andWhere('notifyStatus', 'in', ['delivered', 'received']) .andWhere('eventId', 'is not', null) .withGraphFetched('event') .modifyGraph('event', (builder) => { builder.select([ 'createdAt', + 'issuer', 'metadata', - 'type', - 'subtype', 'status', - 'issuer' + 'subtype', + 'type' ]) }) .page(page - 1, DatabaseConfig.defaultPageSize) diff --git a/db/migrations/legacy/20221108007033_water-scheduled-notifications.js b/db/migrations/legacy/20221108007033_water-scheduled-notification.js similarity index 66% rename from db/migrations/legacy/20221108007033_water-scheduled-notifications.js rename to db/migrations/legacy/20221108007033_water-scheduled-notification.js index 8015297a31..85d9cc0b98 100644 --- a/db/migrations/legacy/20221108007033_water-scheduled-notifications.js +++ b/db/migrations/legacy/20221108007033_water-scheduled-notification.js @@ -2,10 +2,6 @@ const tableName = 'scheduled_notification' -/** - * @param { import("knex").Knex } knex - * @returns { Promise } - */ exports.up = function (knex) { return knex .schema @@ -15,40 +11,35 @@ exports.up = function (knex) { table.uuid('id').primary().defaultTo(knex.raw('gen_random_uuid()')) // Data - table.bigint('status_checks') - table.date('send_after') - table.integer('notification_type') - table.jsonb('licences') - table.jsonb('metadata') + table.string('recipient') + table.string('message_type') + table.string('message_ref') table.jsonb('personalisation') - table.string('company_entity_id') - table.string('individual_entity_id') - table.string('job_id').unique() + table.timestamp('send_after') + table.string('status') table.string('log') + table.jsonb('licences') + table.string('individual_entity_id') + table.string('company_entity_id') table.string('medium') - table.string('message_ref') - table.string('message_type') table.string('notify_id') table.string('notify_status') table.string('plaintext') - table.string('recipient') - table.string('status') - table.timestamp('next_status_check') table.uuid('event_id') + table.jsonb('metadata') + table.bigint('status_checks') + table.timestamp('next_status_check') + table.decimal('notification_type') + table.string('job_id') // Legacy timestamps - table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) + table.timestamp('date_created', { useTz: false }).notNullable() + + // Constraints + table.unique(['job_id'], { useConstraint: true }) }) - .raw(` - CREATE INDEX idx_scheduled_notification_statuses ON water.scheduled_notification USING btree (status, notify_status); - CREATE INDEX scheduled_notification_idx_send_after ON water.scheduled_notification USING btree (send_after); - `) } -/** - * @param { import("knex").Knex } knex - * @returns { Promise } - */ exports.down = function (knex) { return knex .schema diff --git a/db/migrations/public/20240515082450_create-scheduled-notifications-view.js b/db/migrations/public/20240515082450_create-scheduled-notifications-view.js index 471acf0e14..d737f7a057 100644 --- a/db/migrations/public/20240515082450_create-scheduled-notifications-view.js +++ b/db/migrations/public/20240515082450_create-scheduled-notifications-view.js @@ -1,48 +1,39 @@ 'use strict' -const viewName = 'scheduled_notification' +const viewName = 'scheduled_notifications' -/** - * @param { import("knex").Knex } knex - * @returns { Promise } - */ exports.up = function (knex) { return knex .schema .createView(viewName, (view) => { // NOTE: We have commented out unused columns from the source table view.as(knex('scheduled_notification').withSchema('water').select([ - 'scheduled_notification.date_created AS created_at', - 'scheduled_notification.event_id', - 'scheduled_notification.id', - 'scheduled_notification.licences', - 'scheduled_notification.message_type', - 'scheduled_notification.notify_status', - 'scheduled_notification.send_after', - 'scheduled_notification.status' - - // 'scheduled_notification.company_entity_id', - // 'scheduled_notification.individual_entity_id', - // 'scheduled_notification.job_id', - // 'scheduled_notification.log', - // 'scheduled_notification.medium', - // 'scheduled_notification.message_ref', - // 'scheduled_notification.metadata', - // 'scheduled_notification.next_status_check', - // 'scheduled_notification.notification_type', - // 'scheduled_notification.notify_id', - // 'scheduled_notification.personalisation', - // 'scheduled_notification.plaintext', - // 'scheduled_notification.recipient', - // 'scheduled_notification.status_checks' + 'id', + 'recipient', + 'message_type', + 'message_ref', + 'personalisation', + 'send_after', + 'status', + 'log', + 'licences', + 'individual_entity_id AS individual_id', + 'company_entity_id AS company_id', + // 'medium', + 'notify_id', + 'notify_status', + 'plaintext', + 'event_id', + 'metadata', + 'status_checks', + 'next_status_check', + 'notification_type', + 'job_id', + 'date_created AS created_at' ])) }) } -/** - * @param { import("knex").Knex } knex - * @returns { Promise } - */ exports.down = function (knex) { return knex .schema diff --git a/db/migrations/public/20240516044126_alter-scheduled-notification-add-message-ref-column.js b/db/migrations/public/20240516044126_alter-scheduled-notification-add-message-ref-column.js deleted file mode 100644 index 7cc71bd16c..0000000000 --- a/db/migrations/public/20240516044126_alter-scheduled-notification-add-message-ref-column.js +++ /dev/null @@ -1,43 +0,0 @@ -'use strict' - -const viewName = 'scheduled_notification' - -exports.up = async function (knex) { - return knex - .schema - .dropViewIfExists(viewName) - .createView(viewName, (view) => { - // NOTE: We have commented out unused columns from the source table - view.as(knex('scheduled_notification').withSchema('water').select([ - 'scheduled_notification.date_created AS created_at', - 'scheduled_notification.event_id', - 'scheduled_notification.id', - 'scheduled_notification.licences', - 'scheduled_notification.message_type', - 'scheduled_notification.message_ref', - 'scheduled_notification.notify_status', - 'scheduled_notification.send_after', - 'scheduled_notification.status' - - // 'scheduled_notification.company_entity_id', - // 'scheduled_notification.individual_entity_id', - // 'scheduled_notification.job_id', - // 'scheduled_notification.log', - // 'scheduled_notification.medium', - // 'scheduled_notification.metadata', - // 'scheduled_notification.next_status_check', - // 'scheduled_notification.notification_type', - // 'scheduled_notification.notify_id', - // 'scheduled_notification.personalisation', - // 'scheduled_notification.plaintext', - // 'scheduled_notification.recipient', - // 'scheduled_notification.status_checks' - ])) - }) -} - -exports.down = async function (knex) { - return knex - .schema - .dropViewIfExists(viewName) -} diff --git a/test/models/scheduled-notification.model.test.js b/test/models/scheduled-notification.model.test.js index c4e5d648be..09ded167f2 100644 --- a/test/models/scheduled-notification.model.test.js +++ b/test/models/scheduled-notification.model.test.js @@ -21,11 +21,13 @@ describe('Scheduled Notification model', () => { beforeEach(async () => { await DatabaseSupport.clean() - - testRecord = await ScheduledNotificationHelper.add() }) describe('Basic query', () => { + beforeEach(async () => { + testRecord = await ScheduledNotificationHelper.add() + }) + it('can successfully run a basic query', async () => { const result = await ScheduledNotificationModel.query().findById(testRecord.id) @@ -35,14 +37,13 @@ describe('Scheduled Notification model', () => { }) describe('Relationships', () => { - describe('when linking to events', () => { + describe('when linking to event', () => { let testEvent + beforeEach(async () => { - testRecord = await ScheduledNotificationHelper.add() + testEvent = await EventHelper.add() - testEvent = await EventHelper.add({ - id: testRecord.eventId - }) + testRecord = await ScheduledNotificationHelper.add({ eventId: testEvent.id }) }) it('can successfully run a related query', async () => { @@ -52,7 +53,7 @@ describe('Scheduled Notification model', () => { expect(query).to.exist() }) - it('can eager load the events', async () => { + it('can eager load the event', async () => { const result = await ScheduledNotificationModel.query() .findById(testRecord.id) .withGraphFetched('event') diff --git a/test/services/licences/fetch-communications.service.test.js b/test/services/licences/fetch-communications.service.test.js index 943e95ca60..5da0e2acc3 100644 --- a/test/services/licences/fetch-communications.service.test.js +++ b/test/services/licences/fetch-communications.service.test.js @@ -19,24 +19,31 @@ const FetchCommunicationsService = describe('Fetch Communications service', () => { const licenceRef = '01/01' - let testRecord + let event + let scheduledNotification beforeEach(async () => { await DatabaseSupport.clean() - testRecord = await ScheduledNotificationModel.add({ - licences: JSON.stringify([licenceRef]) + event = await EventHelper.add({ + createdAt: new Date('2024-06-01'), + licences: JSON.stringify([licenceRef]), + metadata: null, + status: 'sent', + subtype: 'renewal', + type: 'notification' }) - await EventHelper.add({ - licences: JSON.stringify([licenceRef]) + scheduledNotification = await ScheduledNotificationModel.add({ + eventId: event.id, + licences: JSON.stringify([licenceRef]), + notifyStatus: 'delivered' }) }) describe('when the licence has communications', () => { - it('returns the matching communication', async () => { - const result = await FetchCommunicationsService.go(licenceRef) + const result = await FetchCommunicationsService.go(licenceRef, 1) expect(result.pagination).to.equal({ total: 1 @@ -44,8 +51,15 @@ describe('Fetch Communications service', () => { expect(result.communications).to.equal( [{ - event: null, - id: testRecord.id, + event: { + createdAt: new Date('2024-06-01'), + issuer: 'test.user@defra.gov.uk', + metadata: null, + status: 'sent', + subtype: 'renewal', + type: 'notification' + }, + id: scheduledNotification.id, messageRef: null, messageType: null }] @@ -53,10 +67,9 @@ describe('Fetch Communications service', () => { }) }) - describe('when the licence has no communications', () => { - + describe.skip('when the licence has no communications', () => { it('returns no communications', async () => { - const result = await FetchCommunicationsService.go('01/02') + const result = await FetchCommunicationsService.go('01/02', 1) expect(result.pagination).to.equal({ total: 0 diff --git a/test/support/helpers/scheduled-notification.helper.js b/test/support/helpers/scheduled-notification.helper.js index 26dd3110cc..1dcd56ebb5 100644 --- a/test/support/helpers/scheduled-notification.helper.js +++ b/test/support/helpers/scheduled-notification.helper.js @@ -4,17 +4,15 @@ * @module ScheduledNotificationHelper */ +const { timestampForPostgres } = require('../../../app/lib/general.lib.js') const ScheduledNotificationModel = require('../../../app/models/scheduled-notification.model.js') -const { generateUUID } = require('../../../app/lib/general.lib.js') /** - * Add a new company contact + * Add a new scheduled notification * * If no `data` is provided, default values will be used. These are * - * - `id` - [random UUID] - * - `event_id` - [random UUID] - * - 'notify_status': 'delivered' + * - `createdAt` - new Date() * * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database * @@ -38,8 +36,8 @@ function add (data = {}) { */ function defaults (data = {}) { const defaults = { - eventId: generateUUID(), - notifyStatus: 'delivered' + // INFO: The table does not have a default for the createdAt column. + createdAt: timestampForPostgres() } return { From 48ed4e5b0cdd0108f51e7d8fca9220a8795fa63f Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 17:38:36 +0100 Subject: [PATCH 09/10] Fix return requirement purposes --- ...create-return-requirement-purposes-view.js | 4 +- ..._alter-return-requirement-purposes-view.js | 41 ------------------- 2 files changed, 2 insertions(+), 43 deletions(-) delete mode 100644 db/migrations/public/20240611215502_alter-return-requirement-purposes-view.js diff --git a/db/migrations/public/20240606104927_create-return-requirement-purposes-view.js b/db/migrations/public/20240606104927_create-return-requirement-purposes-view.js index 0659ef52dd..a4277064c1 100644 --- a/db/migrations/public/20240606104927_create-return-requirement-purposes-view.js +++ b/db/migrations/public/20240606104927_create-return-requirement-purposes-view.js @@ -9,8 +9,8 @@ exports.up = function (knex) { view.as(knex('return_requirement_purposes').withSchema('water').select([ 'return_requirement_purpose_id AS id', 'return_requirement_id', - 'purpose_primary_id', - 'purpose_secondary_id', + 'purpose_primary_id AS primary_purpose_id', + 'purpose_secondary_id AS secondary_purpose_id', 'purpose_use_id AS purpose_id', 'purpose_alias AS alias', 'external_id', diff --git a/db/migrations/public/20240611215502_alter-return-requirement-purposes-view.js b/db/migrations/public/20240611215502_alter-return-requirement-purposes-view.js deleted file mode 100644 index 36d578e0c6..0000000000 --- a/db/migrations/public/20240611215502_alter-return-requirement-purposes-view.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict' - -const viewName = 'return_requirement_purposes' - -exports.up = function (knex) { - return knex - .schema - .dropViewIfExists(viewName) - .createView(viewName, (view) => { - view.as(knex('return_requirement_purposes').withSchema('water').select([ - 'return_requirement_purpose_id AS id', - 'return_requirement_id', - 'purpose_primary_id AS primary_purpose_id', - 'purpose_secondary_id AS secondary_purpose_id', - 'purpose_use_id AS purpose_id', - 'purpose_alias AS alias', - 'external_id', - 'date_created AS created_at', - 'date_updated AS updated_at' - ])) - }) -} - -exports.down = function (knex) { - return knex - .schema - .dropViewIfExists(viewName) - .createView(viewName, (view) => { - view.as(knex('return_requirement_purposes').withSchema('water').select([ - 'return_requirement_purpose_id AS id', - 'return_requirement_id', - 'purpose_primary_id', - 'purpose_secondary_id', - 'purpose_use_id AS purpose_id', - 'purpose_alias AS alias', - 'external_id', - 'date_created AS created_at', - 'date_updated AS updated_at' - ])) - }) -} From f4c8ed128522aeb6b0cd7ac2110b6824d3ab0817 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sat, 15 Jun 2024 23:48:03 +0100 Subject: [PATCH 10/10] Fix broken tests --- app/presenters/licences/set-up.presenter.js | 4 +-- app/views/licences/tabs/set-up.njk | 2 +- .../licences/set-up.presenter.test.js | 26 ++++++++----------- .../fetch-customer-contacts.service.test.js | 8 +++--- .../view-licence-set-up.service.test.js | 18 ++++++------- 5 files changed, 27 insertions(+), 31 deletions(-) diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index 0ef8578f76..76f179fd0f 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -57,7 +57,7 @@ function _agreements (commonData, agreements, auth) { startDate: formatLongDate(agreement.startDate), endDate: agreement.endDate ? formatLongDate(agreement.endDate) : '', description: agreementDescriptions[_financialAgreementCode(agreement)], - dateSigned: agreement.dateSigned ? formatLongDate(agreement.dateSigned) : '', + signedOn: agreement.signedOn ? formatLongDate(agreement.signedOn) : '', action: _agreementActionLinks(commonData, agreement, auth) } }) @@ -167,7 +167,7 @@ function _endsSixYearsAgo (endDate) { } function _financialAgreementCode (agreement) { - return agreement.financialAgreements[0].code + return agreement.financialAgreement.code } function _returnVersions (returnVersions = [{}]) { diff --git a/app/views/licences/tabs/set-up.njk b/app/views/licences/tabs/set-up.njk index 17b641ef97..e7440dae96 100644 --- a/app/views/licences/tabs/set-up.njk +++ b/app/views/licences/tabs/set-up.njk @@ -172,7 +172,7 @@ text: agreement.description }, { - text: agreement.signedDate + text: agreement.signedOn }, { html: createLink(agreement) diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index b9eb4422ad..d7fd9efc3b 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -14,13 +14,13 @@ const FeatureFlagsConfig = require('../../../config/feature-flags.config.js') // Thing under test const SetUpPresenter = require('../../../app/presenters/licences/set-up.presenter.js') -describe('Licence Set Up presenter', () => { +describe('Licences - Set Up presenter', () => { const agreement = { id: '123', startDate: new Date('2020-01-01'), endDate: null, - dateSigned: null, - financialAgreements: [{ code: 'S127' }] + signedOn: null, + financialAgreement: { id: '970168ce-06c3-4823-b84d-9da30b742bb8', code: 'S127' } } const chargeVersion = { @@ -69,12 +69,10 @@ describe('Licence Set Up presenter', () => { } } - const lessThanSixYearsAgo = new Date() - commonData = { licenceId: 'f91bf145-ce8e-481c-a842-4da90348062b', ends: { - date: lessThanSixYearsAgo + date: new Date() } } @@ -91,7 +89,6 @@ describe('Licence Set Up presenter', () => { describe('when provided with populated licence set up data', () => { describe('that includes licence agreements', () => { beforeEach(() => { - agreement.endDate = null agreements = [{ ...agreement }] chargeVersions = [] workflows = [] @@ -121,7 +118,7 @@ describe('Licence Set Up presenter', () => { text: 'Recalculate bills' } ], - dateSigned: '', + signedOn: '', description: 'Two-part tariff', endDate: '', startDate: '1 January 2020' @@ -225,9 +222,6 @@ describe('Licence Set Up presenter', () => { describe('when the financial agreement code ', () => { describe('is for Two-part tariff ', () => { - beforeEach(() => { - agreement.financialAgreements[0].code = 'S127' - }) it('correctly maps the code to the description', () => { const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) @@ -237,8 +231,9 @@ describe('Licence Set Up presenter', () => { describe('is for Canal and Rivers Trust, supported source (S130S) ', () => { beforeEach(() => { - agreement.financialAgreements[0].code = 'S130S' + agreement.financialAgreement.code = 'S130S' }) + it('correctly maps the code to the description', () => { const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) @@ -248,8 +243,9 @@ describe('Licence Set Up presenter', () => { describe('is for Canal and Rivers Trust, unsupported source (S130U)', () => { beforeEach(() => { - agreement.financialAgreements[0].code = 'S130U' + agreement.financialAgreement.code = 'S130U' }) + it('correctly maps the code to the description', () => { const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) @@ -259,7 +255,7 @@ describe('Licence Set Up presenter', () => { describe('is for Abatement', () => { beforeEach(() => { - agreement.financialAgreements[0].code = 'S126' + agreement.financialAgreement.code = 'S126' }) it('correctly maps the code to the description', () => { const result = SetUpPresenter @@ -272,7 +268,7 @@ describe('Licence Set Up presenter', () => { describe('when the licence is less than 6 years old and all the actions are available for an agreement', () => { beforeEach(() => { - agreement.financialAgreements[0].code = 'S127' + agreement.financialAgreement.code = 'S127' }) it('shows delete, end and recalculate bills actions', () => { diff --git a/test/services/licences/fetch-customer-contacts.service.test.js b/test/services/licences/fetch-customer-contacts.service.test.js index 01a5602a0e..3b35fe1721 100644 --- a/test/services/licences/fetch-customer-contacts.service.test.js +++ b/test/services/licences/fetch-customer-contacts.service.test.js @@ -8,12 +8,12 @@ const { describe, it, beforeEach } = exports.lab = Lab.script() const { expect } = Code // Test helpers -const CompanyContactsHelper = require('../../support/helpers/company-contact.helper.js') +const CompanyContactHelper = require('../../support/helpers/company-contact.helper.js') const CompanyHelper = require('../../support/helpers/company.helper.js') const ContactHelper = require('../../support/helpers/contact.helper.js') const DatabaseSupport = require('../../support/database.js') const LicenceDocumentHelper = require('../../support/helpers/licence-document.helper.js') -const LicenceDocumentRolesHelper = require('../../support/helpers/licence-document-role.helper.js') +const LicenceDocumentRoleHelper = require('../../support/helpers/licence-document-role.helper.js') const LicenceHelper = require('../../support/helpers/licence.helper.js') const LicenceRoleHelper = require('../../support/helpers/licence-role.helper.js') @@ -44,13 +44,13 @@ describe('Fetch Customer Contacts service', () => { const { id: licenceDocumentId } = await LicenceDocumentHelper.add({ licenceRef: licence.licenceRef }) const { id: licenceRoleId } = await LicenceRoleHelper.add() - await CompanyContactsHelper.add({ + await CompanyContactHelper.add({ companyId, contactId, roleId: licenceRoleId }) - await LicenceDocumentRolesHelper.add({ + await LicenceDocumentRoleHelper.add({ companyId, contactId, endDate: null, diff --git a/test/services/licences/view-licence-set-up.service.test.js b/test/services/licences/view-licence-set-up.service.test.js index 55db8207b8..bdbb38d8bb 100644 --- a/test/services/licences/view-licence-set-up.service.test.js +++ b/test/services/licences/view-licence-set-up.service.test.js @@ -29,11 +29,11 @@ describe('View Licence Set Up service', () => { Sinon.stub(FetchAgreementsService, 'go').returns([ { - id: '123', + id: 'ca41d547-2cb6-4995-8af4-90117839bf86', startDate: new Date('2020-01-01'), endDate: null, - dateSigned: null, - financialAgreements: [{ code: 'S127' }] + signedOn: null, + financialAgreement: { id: 'f766a058-99e0-4cb3-8b63-53856dd60cf9', code: 'S127' } } ]) @@ -41,7 +41,7 @@ describe('View Licence Set Up service', () => { { changeReason: { description: 'Missing thing' }, endDate: new Date('2020-09-01'), - id: '123', + id: 'c0601335-b6ad-4651-b54b-c586f8d22ac3', licenceId: '456', startDate: new Date('2020-01-01'), status: 'current' @@ -60,7 +60,7 @@ describe('View Licence Set Up service', () => { Sinon.stub(FetchWorkflowsService, 'go').returns([ { - id: '123', + id: 'f3fe1275-50ff-4f69-98cb-5a35c17654f3', createdAt: new Date('2020-01-01'), status: 'review', data: { chargeVersion: { changeReason: { description: 'changed something' } } }, @@ -98,7 +98,7 @@ describe('View Licence Set Up service', () => { agreements: [ { action: [], - dateSigned: '', + signedOn: '', description: 'Two-part tariff', endDate: '', startDate: '1 January 2020' @@ -108,7 +108,7 @@ describe('View Licence Set Up service', () => { { action: [], endDate: '-', - id: '123', + id: 'f3fe1275-50ff-4f69-98cb-5a35c17654f3', reason: 'changed something', startDate: '1 January 2020', status: 'review' @@ -116,12 +116,12 @@ describe('View Licence Set Up service', () => { { action: [ { - link: '/licences/456/charge-information/123/view', + link: '/licences/456/charge-information/c0601335-b6ad-4651-b54b-c586f8d22ac3/view', text: 'View' } ], endDate: '1 September 2020', - id: '123', + id: 'c0601335-b6ad-4651-b54b-c586f8d22ac3', reason: 'Missing thing', startDate: '1 January 2020', status: 'approved'