Fix & tidy test DB legacy migrations #725
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://eaflood.atlassian.net/browse/WATER-4365 > For context this came out of us working on re-implementing the SROC annual bill run using what we've learnt and components from our supplementary billing engine. Whilst spiking on getting a working SROC annual billing engine we found that some of the legacy tables the migrations were creating in the test DB didn't match with the real thing. No doubt a result of our original intent being legacy migrations for the test DB did the minimum needed to get tests passing. We've now got the ability to fix legacy migrations without borking everyone's local test DB thanks to [Add wipe test DB function to project](#724). So, in this change we're not just going to fix the 2 issues we found during our annual billing spike. Instead we've reviewed all the original legacy migrations and fixed any inconsistencies with the real tables we found.
Cruikshanks
force-pushed
the
fix-legacy-migrations
branch
from
d3d99ef to
69b493c
Compare
We've add a number of constraints that exist on the real table but which we didn't have in our test DB. The majority are focused on ensuring certain fields are not null if the type is 'x' or the scheme is 'y' for 'reasons'! The key one is that they should all have the start and end date populated which would be based on the charge period. In most cases this aligns with the billing period so we've created a helper that returns the current financial year's start and end dates rather than hard code in values on the basis the assumption for most tests are that we are working in the current financial year.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Came to `water.purposes_uses` and realised we can specify if a column is supposed to be unique when we create the column. There is no need to set a separate constraint. We can reserve those for where the constraint involves more than one column.
diff --git a/db/migrations/legacy/20230818155314_create-crm-v2-invoice-account-addressses.js b/db/migrations/legacy/20230818155314_create-crm-v2-invoice-account-addressses.js
index 4ce0bdda..4fea14a9 100644
--- a/db/migrations/legacy/20230818155314_create-crm-v2-invoice-account-addressses.js
+++ b/db/migrations/legacy/20230818155314_create-crm-v2-invoice-account-addressses.js
@@ -22,6 +22,9 @@ exports.up = function (knex) {
// Legacy timestamps
table.timestamp('date_created').notNullable().defaultTo(knex.fn.now())
table.timestamp('date_updated').notNullable().defaultTo(knex.fn.now())
+
+ // Constraints
+ table.unique(['invoice_account_id', 'start_date'], { useConstraint: true })
})
}
diff --git a/db/migrations/legacy/20230904140845_alter-crm-v2-invoice-account-addresses.js b/db/migrations/legacy/20230904140845_alter-crm-v2-invoice-account-addresses.js
deleted file mode 100644
index c18c20ee..00000000
--- a/db/migrations/legacy/20230904140845_alter-crm-v2-invoice-account-addresses.js
+++ /dev/null
@@ -1,21 +0,0 @@
-'use strict'
-
-const tableName = 'invoice_account_addresses'
-
-exports.up = function (knex) {
- return knex
- .schema
- .withSchema('crm_v2')
- .alterTable(tableName, (table) => {
- table.unique(['invoice_account_id', 'start_date'], { useConstraint: true })
- })
-}
-
-exports.down = function (knex) {
- return knex
- .schema
- .withSchema('crm_v2')
- .alterTable(tableName, (table) => {
- table.dropUnique(['invoice_account_id', 'start_date'])
- })
-}
Cruikshanks
force-pushed
the
fix-legacy-migrations
branch
from
bf73830 to
22ea9be
Compare
For all the migrations the timestamps ensure that as migrations are added their order is retained. This is important as DB changes are made overtime to a system. But our legacy migrations are purely for our test DB. Which means we can alter the timestamps to allow us to control the order of when things happen. Couple this with the fact we now always wipe before running migrations against the test DB means we can also start to group tables by schema. This should make it easier to see what has and hasn't been replicated. SO, this is the first change in a series of tidy ups for the legacy migrations folder.
We know we're gonna need it!
Cruikshanks
changed the title
Cruikshanks
requested review from
Jozzey,
Beckyrose200,
Demwunz,
robertparkinson and
rvsiyad
|
This looks like a massive PR but the changes are relatively minor. I've just added missing details in the migrations and corrected a few others. The extra constraints have meant we need to start populating certain fields hence a few helpers got touched. Also, a couple of tests that created more than one record needed fixing for the same reason. What has really made things look crazy is me taking the opportunity to group what we have by schema, which meant changing the file names. The only person this is going to affect right now is @robertparkinson (more than happy if you want me to jump on your PR and fix what merging this will break Robert). So feel free to 'pass' on reviewing this. Just let me know so I know I can go ahead with abusing my tech-lead privileges. |
robertparkinson
approved these changes
Feb 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://eaflood.atlassian.net/browse/WATER-4365
Whilst spiking on getting a working SROC annual billing engine we found that some of the legacy tables the migrations were creating in the test DB didn't match with the real thing. No doubt a result of our original intent being legacy migrations for the test DB did the minimum needed to get tests passing.
We now can fix legacy migrations without borking everyone's local test DB thanks to Add wipe test DB function to project.
So, in this change, we're not just going to fix the 2 issues we found during our annual billing spike. Instead, we've reviewed all the original legacy migrations and fixed any inconsistencies with the real tables we found.
We've then taken the opportunity to tidy them up; grouping them by schema and ordering them in a way that makes sense.