From 9e145eb16c9aee44ea30285377562dde22fdb78f Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 4 Jan 2023 15:46:19 +0000 Subject: [PATCH 1/5] Rewrite the migrations to match legacy tables https://github.com/DEFRA/water-abstraction-team/issues/69 > **WARNING** - This will break your test environment. Run `npm run rollback:db:test` before pulling down to prevent this. > Note - Whilst the repo is new, not yet in `production` and we only have a few migrations purely for test purposes we have opted to rewrite rather than 'fix-forward' approach. This change rewrites the existing migrations. We'll be - renaming the files to include which schemas they will be added to - adding timestamp fields that match the original tables - consolidating any changes we've made up to this point This is to ensure our test tables match what we see in the legacy code so we know we're handling the issues we find there and make it clearer what schema a table belongs to. From 689a2654e5b0fc703c538553f474a6677dfb6dd4 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 4 Jan 2023 16:39:57 +0000 Subject: [PATCH 2/5] Consolidate changes --- .../20221108152353_create-charge-versions.js | 3 +++ .../20221111155222_create_licences.js | 1 + .../20221111184905_alter_charge_versions.js | 22 ------------------- .../20221114104700_create_regions.js | 2 ++ .../20221114155444_alter_licences.js | 21 ------------------ .../20221125164859_create_billing_batches.js | 1 + .../20221206084612_alter_charge_versions.js | 21 ------------------ db/migrations/20230102173756_alter_regions.js | 22 ------------------- .../20230102174930_alter_billing_batches.js | 21 ------------------ 9 files changed, 7 insertions(+), 107 deletions(-) delete mode 100644 db/migrations/20221111184905_alter_charge_versions.js delete mode 100644 db/migrations/20221114155444_alter_licences.js delete mode 100644 db/migrations/20221206084612_alter_charge_versions.js delete mode 100755 db/migrations/20230102173756_alter_regions.js delete mode 100755 db/migrations/20230102174930_alter_billing_batches.js diff --git a/db/migrations/20221108152353_create-charge-versions.js b/db/migrations/20221108152353_create-charge-versions.js index 51becef007..5c9808e85c 100644 --- a/db/migrations/20221108152353_create-charge-versions.js +++ b/db/migrations/20221108152353_create-charge-versions.js @@ -13,6 +13,9 @@ exports.up = async function (knex) { // Data table.string('licence_ref') table.string('scheme') + table.uuid('licence_id') + table.date('start_date') + table.date('end_date') // Automatic timestamps table.timestamps(false, true) diff --git a/db/migrations/20221111155222_create_licences.js b/db/migrations/20221111155222_create_licences.js index 404ef3fc8b..bf7edfeb52 100644 --- a/db/migrations/20221111155222_create_licences.js +++ b/db/migrations/20221111155222_create_licences.js @@ -13,6 +13,7 @@ exports.up = async function (knex) { // Data table.string('licence_ref') table.string('include_in_supplementary_billing') + table.uuid('region_id') // Automatic timestamps table.timestamps(false, true) diff --git a/db/migrations/20221111184905_alter_charge_versions.js b/db/migrations/20221111184905_alter_charge_versions.js deleted file mode 100644 index 5c26177c12..0000000000 --- a/db/migrations/20221111184905_alter_charge_versions.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict' - -const tableName = 'charge_versions' - -exports.up = async function (knex) { - await knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.uuid('licence_id') - table.date('end_date') - }) -} - -exports.down = function (knex) { - return knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.dropColumns('licence_id', 'end_date') - }) -} diff --git a/db/migrations/20221114104700_create_regions.js b/db/migrations/20221114104700_create_regions.js index b54c888845..92d09b669a 100644 --- a/db/migrations/20221114104700_create_regions.js +++ b/db/migrations/20221114104700_create_regions.js @@ -13,6 +13,8 @@ exports.up = async function (knex) { // Data table.string('charge_region_id') table.integer('nald_region_id') + table.string('name') + table.string('display_name') // Automatic timestamps table.timestamps(false, true) diff --git a/db/migrations/20221114155444_alter_licences.js b/db/migrations/20221114155444_alter_licences.js deleted file mode 100644 index 85872907b7..0000000000 --- a/db/migrations/20221114155444_alter_licences.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict' - -const tableName = 'licences' - -exports.up = async function (knex) { - await knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.uuid('region_id') - }) -} - -exports.down = function (knex) { - return knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.dropColumns('region_id') - }) -} diff --git a/db/migrations/20221125164859_create_billing_batches.js b/db/migrations/20221125164859_create_billing_batches.js index e12bed89e2..a475a142fb 100644 --- a/db/migrations/20221125164859_create_billing_batches.js +++ b/db/migrations/20221125164859_create_billing_batches.js @@ -25,6 +25,7 @@ exports.up = async function (knex) { table.decimal('credit_note_value') table.string('transaction_file_reference') table.string('scheme') + table.boolean('is_summer').notNullable().defaultTo(false) // Automatic timestamps table.timestamps(false, true) diff --git a/db/migrations/20221206084612_alter_charge_versions.js b/db/migrations/20221206084612_alter_charge_versions.js deleted file mode 100644 index 560c58c5d7..0000000000 --- a/db/migrations/20221206084612_alter_charge_versions.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict' - -const tableName = 'charge_versions' - -exports.up = async function (knex) { - await knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.date('start_date') - }) -} - -exports.down = function (knex) { - return knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.dropColumns('start_date') - }) -} diff --git a/db/migrations/20230102173756_alter_regions.js b/db/migrations/20230102173756_alter_regions.js deleted file mode 100755 index 2efa779764..0000000000 --- a/db/migrations/20230102173756_alter_regions.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict' - -const tableName = 'regions' - -exports.up = async function (knex) { - await knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.string('name') - table.string('display_name') - }) -} - -exports.down = function (knex) { - return knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.dropColumns('name', 'display_name') - }) -} diff --git a/db/migrations/20230102174930_alter_billing_batches.js b/db/migrations/20230102174930_alter_billing_batches.js deleted file mode 100755 index 28e5739cf9..0000000000 --- a/db/migrations/20230102174930_alter_billing_batches.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict' - -const tableName = 'billing_batches' - -exports.up = async function (knex) { - await knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.boolean('is_summer').notNullable().defaultTo(false) - }) -} - -exports.down = function (knex) { - return knex - .schema - .withSchema('water') - .alterTable(tableName, table => { - table.dropColumns('is_summer') - }) -} From ef13c70e1414c99d1f4a34b66404a5cbb6d40a72 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 4 Jan 2023 17:27:44 +0000 Subject: [PATCH 3/5] Replace existing timestamp logic Match the field names and their properties to what we find in the legacy DB. --- .../20221108152353_create-charge-versions.js | 13 +++---------- db/migrations/20221111155222_create_licences.js | 13 +++---------- db/migrations/20221114104700_create_regions.js | 5 +++-- .../20221125164859_create_billing_batches.js | 13 +++---------- db/migrations/20221209193845_create_events.js | 5 +++-- .../20221215174926_create_charge_elements.js | 13 +++---------- ...221216112650_create_billing_charge_categories.js | 5 +++-- .../20221216154722_create_charge_purposes.js | 13 +++---------- 8 files changed, 24 insertions(+), 56 deletions(-) diff --git a/db/migrations/20221108152353_create-charge-versions.js b/db/migrations/20221108152353_create-charge-versions.js index 5c9808e85c..9dc03f4444 100644 --- a/db/migrations/20221108152353_create-charge-versions.js +++ b/db/migrations/20221108152353_create-charge-versions.js @@ -17,17 +17,10 @@ exports.up = async function (knex) { table.date('start_date') table.date('end_date') - // Automatic timestamps - table.timestamps(false, true) + // Legacy timestamps + table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) + table.timestamp('date_updated', { useTz: false }).notNullable().defaultTo(knex.fn.now()) }) - - await knex.raw(` - CREATE TRIGGER update_timestamp - BEFORE UPDATE - ON water.${tableName} - FOR EACH ROW - EXECUTE PROCEDURE update_timestamp(); - `) } exports.down = function (knex) { diff --git a/db/migrations/20221111155222_create_licences.js b/db/migrations/20221111155222_create_licences.js index bf7edfeb52..34176f4cdd 100644 --- a/db/migrations/20221111155222_create_licences.js +++ b/db/migrations/20221111155222_create_licences.js @@ -15,17 +15,10 @@ exports.up = async function (knex) { table.string('include_in_supplementary_billing') table.uuid('region_id') - // Automatic timestamps - table.timestamps(false, true) + // Legacy timestamps + table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) + table.timestamp('date_updated', { useTz: false }).notNullable().defaultTo(knex.fn.now()) }) - - await knex.raw(` - CREATE TRIGGER update_timestamp - BEFORE UPDATE - ON water.${tableName} - FOR EACH ROW - EXECUTE PROCEDURE update_timestamp(); - `) } exports.down = function (knex) { diff --git a/db/migrations/20221114104700_create_regions.js b/db/migrations/20221114104700_create_regions.js index 92d09b669a..d6d3608d06 100644 --- a/db/migrations/20221114104700_create_regions.js +++ b/db/migrations/20221114104700_create_regions.js @@ -16,8 +16,9 @@ exports.up = async function (knex) { table.string('name') table.string('display_name') - // Automatic timestamps - table.timestamps(false, true) + // Legacy timestamps + table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) + table.timestamp('date_updated', { useTz: false }).notNullable().defaultTo(knex.fn.now()) }) await knex.raw(` diff --git a/db/migrations/20221125164859_create_billing_batches.js b/db/migrations/20221125164859_create_billing_batches.js index a475a142fb..df7bbb509f 100644 --- a/db/migrations/20221125164859_create_billing_batches.js +++ b/db/migrations/20221125164859_create_billing_batches.js @@ -27,17 +27,10 @@ exports.up = async function (knex) { table.string('scheme') table.boolean('is_summer').notNullable().defaultTo(false) - // Automatic timestamps - table.timestamps(false, true) + // Legacy timestamps + table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) + table.timestamp('date_updated', { useTz: false }).notNullable().defaultTo(knex.fn.now()) }) - - await knex.raw(` - CREATE TRIGGER update_timestamp - BEFORE UPDATE - ON water.${tableName} - FOR EACH ROW - EXECUTE PROCEDURE update_timestamp(); - `) } exports.down = function (knex) { diff --git a/db/migrations/20221209193845_create_events.js b/db/migrations/20221209193845_create_events.js index a2d2964849..a81b0bbd38 100644 --- a/db/migrations/20221209193845_create_events.js +++ b/db/migrations/20221209193845_create_events.js @@ -17,8 +17,9 @@ exports.up = async function (knex) { table.jsonb('metadata') table.string('status') - // Automatic timestamps - table.timestamps(false, true) + // Legacy timestamps + table.timestamp('created', { precision: 0, useTz: false }).notNullable().defaultTo(knex.fn.now()) + table.timestamp('modified', { precision: 0, useTz: false }).notNullable().defaultTo(knex.fn.now()) }) await knex.raw(` diff --git a/db/migrations/20221215174926_create_charge_elements.js b/db/migrations/20221215174926_create_charge_elements.js index 9f8f3c4ec1..a85557e32d 100755 --- a/db/migrations/20221215174926_create_charge_elements.js +++ b/db/migrations/20221215174926_create_charge_elements.js @@ -25,17 +25,10 @@ exports.up = async function (knex) { table.jsonb('adjustments') table.string('eiuc_region') - // Automatic timestamps - table.timestamps(false, true) + // Legacy timestamps + table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) + table.timestamp('date_updated', { useTz: false }).notNullable().defaultTo(knex.fn.now()) }) - - await knex.raw(` - CREATE TRIGGER update_timestamp - BEFORE UPDATE - ON water.${tableName} - FOR EACH ROW - EXECUTE PROCEDURE update_timestamp(); - `) } exports.down = function (knex) { diff --git a/db/migrations/20221216112650_create_billing_charge_categories.js b/db/migrations/20221216112650_create_billing_charge_categories.js index dd2d1ce69d..68c6aed7e8 100755 --- a/db/migrations/20221216112650_create_billing_charge_categories.js +++ b/db/migrations/20221216112650_create_billing_charge_categories.js @@ -22,8 +22,9 @@ exports.up = async function (knex) { table.bigInteger('min_volume') table.bigInteger('max_volume') - // Automatic timestamps - table.timestamps(false, true) + // Legacy timestamps + table.timestamp('date_created', { useTz: false }).notNullable() + table.timestamp('date_updated', { useTz: false }) }) await knex.raw(` diff --git a/db/migrations/20221216154722_create_charge_purposes.js b/db/migrations/20221216154722_create_charge_purposes.js index 1373af45bb..9f447c395c 100755 --- a/db/migrations/20221216154722_create_charge_purposes.js +++ b/db/migrations/20221216154722_create_charge_purposes.js @@ -28,17 +28,10 @@ exports.up = async function (knex) { table.uuid('purpose_use_id') table.boolean('is_section_127_agreement_enabled') - // Automatic timestamps - table.timestamps(false, true) + // Legacy timestamps + table.timestamp('date_created', { useTz: false }).notNullable().defaultTo(knex.fn.now()) + table.timestamp('date_updated', { useTz: false }).notNullable().defaultTo(knex.fn.now()) }) - - await knex.raw(` - CREATE TRIGGER update_timestamp - BEFORE UPDATE - ON water.${tableName} - FOR EACH ROW - EXECUTE PROCEDURE update_timestamp(); - `) } exports.down = function (knex) { From 0eb660fd9185693a19bdd538dc6fd88b6c6c53f3 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 4 Jan 2023 17:49:53 +0000 Subject: [PATCH 4/5] Rename files Include schema in name (where relevant). Also, be consistent with kebab-case usage (the `20221108143943_*` comes from Knex and is out of our control). --- ...ypto_extension.js => 20221108143017_add-pgcrypto-extension.js} | 0 ...igger.js => 20221108143106_create-update-timestamp-trigger.js} | 0 ...versions.js => 20221108152353_create-water-charge-versions.js} | 0 ...create_licences.js => 20221111155222_create-water-licences.js} | 0 ...0_create_regions.js => 20221114104700_create-water-regions.js} | 0 ..._batches.js => 20221125164859_create-water-billing-batches.js} | 0 ...845_create_events.js => 20221209193845_create-water-events.js} | 0 ...elements.js => 20221215174926_create-water-charge-elements.js} | 0 ...s => 20221216112650_create-water-billing-charge-categories.js} | 0 ...purposes.js => 20221216154722_create-water-charge-purposes.js} | 0 10 files changed, 0 insertions(+), 0 deletions(-) rename db/migrations/{20221108143017_add_pgcrypto_extension.js => 20221108143017_add-pgcrypto-extension.js} (100%) rename db/migrations/{20221108143106_create_update_timestamp_trigger.js => 20221108143106_create-update-timestamp-trigger.js} (100%) rename db/migrations/{20221108152353_create-charge-versions.js => 20221108152353_create-water-charge-versions.js} (100%) rename db/migrations/{20221111155222_create_licences.js => 20221111155222_create-water-licences.js} (100%) rename db/migrations/{20221114104700_create_regions.js => 20221114104700_create-water-regions.js} (100%) rename db/migrations/{20221125164859_create_billing_batches.js => 20221125164859_create-water-billing-batches.js} (100%) rename db/migrations/{20221209193845_create_events.js => 20221209193845_create-water-events.js} (100%) rename db/migrations/{20221215174926_create_charge_elements.js => 20221215174926_create-water-charge-elements.js} (100%) rename db/migrations/{20221216112650_create_billing_charge_categories.js => 20221216112650_create-water-billing-charge-categories.js} (100%) rename db/migrations/{20221216154722_create_charge_purposes.js => 20221216154722_create-water-charge-purposes.js} (100%) diff --git a/db/migrations/20221108143017_add_pgcrypto_extension.js b/db/migrations/20221108143017_add-pgcrypto-extension.js similarity index 100% rename from db/migrations/20221108143017_add_pgcrypto_extension.js rename to db/migrations/20221108143017_add-pgcrypto-extension.js diff --git a/db/migrations/20221108143106_create_update_timestamp_trigger.js b/db/migrations/20221108143106_create-update-timestamp-trigger.js similarity index 100% rename from db/migrations/20221108143106_create_update_timestamp_trigger.js rename to db/migrations/20221108143106_create-update-timestamp-trigger.js diff --git a/db/migrations/20221108152353_create-charge-versions.js b/db/migrations/20221108152353_create-water-charge-versions.js similarity index 100% rename from db/migrations/20221108152353_create-charge-versions.js rename to db/migrations/20221108152353_create-water-charge-versions.js diff --git a/db/migrations/20221111155222_create_licences.js b/db/migrations/20221111155222_create-water-licences.js similarity index 100% rename from db/migrations/20221111155222_create_licences.js rename to db/migrations/20221111155222_create-water-licences.js diff --git a/db/migrations/20221114104700_create_regions.js b/db/migrations/20221114104700_create-water-regions.js similarity index 100% rename from db/migrations/20221114104700_create_regions.js rename to db/migrations/20221114104700_create-water-regions.js diff --git a/db/migrations/20221125164859_create_billing_batches.js b/db/migrations/20221125164859_create-water-billing-batches.js similarity index 100% rename from db/migrations/20221125164859_create_billing_batches.js rename to db/migrations/20221125164859_create-water-billing-batches.js diff --git a/db/migrations/20221209193845_create_events.js b/db/migrations/20221209193845_create-water-events.js similarity index 100% rename from db/migrations/20221209193845_create_events.js rename to db/migrations/20221209193845_create-water-events.js diff --git a/db/migrations/20221215174926_create_charge_elements.js b/db/migrations/20221215174926_create-water-charge-elements.js similarity index 100% rename from db/migrations/20221215174926_create_charge_elements.js rename to db/migrations/20221215174926_create-water-charge-elements.js diff --git a/db/migrations/20221216112650_create_billing_charge_categories.js b/db/migrations/20221216112650_create-water-billing-charge-categories.js similarity index 100% rename from db/migrations/20221216112650_create_billing_charge_categories.js rename to db/migrations/20221216112650_create-water-billing-charge-categories.js diff --git a/db/migrations/20221216154722_create_charge_purposes.js b/db/migrations/20221216154722_create-water-charge-purposes.js similarity index 100% rename from db/migrations/20221216154722_create_charge_purposes.js rename to db/migrations/20221216154722_create-water-charge-purposes.js From 9e2306b868d207872d9f49d53872936ca5b93803 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 5 Jan 2023 08:38:25 +0000 Subject: [PATCH 5/5] Fix broken tests --- .../create-billing-batch-event.presenter.test.js | 4 ++-- .../helpers/water/billing-charge-category.helper.js | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/test/presenters/supplementary-billing/create-billing-batch-event.presenter.test.js b/test/presenters/supplementary-billing/create-billing-batch-event.presenter.test.js index 224938cf2b..7b7a0416e4 100644 --- a/test/presenters/supplementary-billing/create-billing-batch-event.presenter.test.js +++ b/test/presenters/supplementary-billing/create-billing-batch-event.presenter.test.js @@ -44,8 +44,8 @@ describe('Create Billing Batch Event presenter', () => { expect(result.batch.isSummer).to.equal(billingBatch.isSummer) expect(result.batch.netTotal).to.equal(billingBatch.netTotal) - expect(result.batch.dateCreated).to.equal(billingBatch.createdAt) - expect(result.batch.dateUpdated).to.equal(billingBatch.updatedAt) + expect(result.batch.dateCreated).to.equal(billingBatch.dateCreated) + expect(result.batch.dateUpdated).to.equal(billingBatch.dateUpdated) expect(result.batch.invoiceCount).to.equal(billingBatch.invoiceCount) expect(result.batch.invoiceValue).to.equal(billingBatch.invoiceValue) expect(result.batch.creditNoteCount).to.equal(billingBatch.creditNoteCount) diff --git a/test/support/helpers/water/billing-charge-category.helper.js b/test/support/helpers/water/billing-charge-category.helper.js index 5e3089e2c6..dd8067b899 100644 --- a/test/support/helpers/water/billing-charge-category.helper.js +++ b/test/support/helpers/water/billing-charge-category.helper.js @@ -20,7 +20,8 @@ const BillingChargeCategoryModel = require('../../../../app/models/water/billing * - `modelTier` - tier 1 * - `isRestrictedSource` - true * - `minVolume` - 0 - * - `maxVolume` - 5000 + * - `maxVolume` - 5000, + * - `dateCreated` - Date.now() * * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database * @@ -53,7 +54,14 @@ function defaults (data = {}) { modelTier: 'tier 1', isRestrictedSource: true, minVolume: 0, - maxVolume: 5000 + maxVolume: 5000, + // INFO: The billing_charge_categories table does not have a default for the date_created column. But it is set as + // 'not nullable'! So, we need to ensure we set it when creating a new record, something we'll never actually need + // to do because it's a static table. Also, we can't use Date.now() because Javascript returns the time since the + // epoch in milliseconds, whereas a PostgreSQL timestamp field can only hold the seconds since the epoch. Pass it + // an ISO string though ('2023-01-05T08:37:05.575Z') and PostgreSQL can do the conversion + // https://stackoverflow.com/a/61912776/6117745 + dateCreated: new Date().toISOString() } return {