Alter name & default for versions mod_log field
#2612
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-4563 https://eaflood.atlassian.net/browse/WATER-4564 https://eaflood.atlassian.net/browse/WATER-4565 > Part of the work to display a licence's history to users (mod log) In [Add mod_log fields 2 return, licence & charge ver.](#2598) we added a data migration to add a new `mod_log` field to the charge, licence and return version tables in the `water` schema. This was intended to hold details from the linked NALD mod log record, such as who, when and why the version was created. However, we've since learnt that it is possible for a version in NALD to have multiple mod log records. Therefore, we want to change the default to avoid complexity in the code. Now we want it to default to an empty JSONB array rather than an empty object. This change is the data migration for that.
Cruikshanks
added
the
housekeeping
Cruikshanks
changed the title
mod_log fieldmod_log field
Cruikshanks
added a commit
that referenced
this pull request
Aug 18, 2024
https://eaflood.atlassian.net/browse/WATER-4563 https://eaflood.atlassian.net/browse/WATER-4564 https://eaflood.atlassian.net/browse/WATER-4565 > Part of the work to display a licence's history to users (mod log) **It's a trap!** We have fallen into making the same mistake as the previous team. When we believed that a version (charge, licence or return) had a one-to-one relationship with NALD mod log records a JSONB field in each was a suitable solution to capturing just the information we need. It's only the historic records we have to worry about after all. But when we learned that a version record could be linked to multiple mod logs we should have taken a different approach. Instead, we ploughed on with our JSON column column [altering its name and default](#2612) to match. 🤦😬 Once you've started down the JSONB road, it is easy to get trapped! Thankfully, we've come to our senses before this saw the light of day in production. The mod log data we are importing needs to go into its own table and we link our version records accordingly. A 'standard' relationship database solution. This change drops the columns and adds a new table instead.
This was referenced Aug 18, 2024
Cruikshanks
added a commit
that referenced
this pull request
Aug 18, 2024
https://eaflood.atlassian.net/browse/WATER-4563 https://eaflood.atlassian.net/browse/WATER-4564 https://eaflood.atlassian.net/browse/WATER-4565 > Part of the work to display a licence's history to users (mod log) **It's a trap!** We have made the same mistake as the previous team. When we believed that a version (charge, licence, or return) had a one-to-one relationship with NALD mod log records, a JSONB field in each was a suitable solution to capturing just the information we needed. After all, it's only the historical records we have to worry about. But when we learned that a version record could be linked to multiple mod logs, we should have taken a different approach. Instead, we ploughed on with our JSON column [altering its name and default](#2612) to match. 🤦😬 Once you've started down the JSONB road, it is easy to get trapped! Thankfully, we've come to our senses before this saw the light of day in production. The mod log data we are importing needs to go into its own table, and we will link our version records accordingly. A 'standard' relationship database solution. We could just add a migration that drops the columns and adds the new table. But because of the interaction between the views [water-abstraction-system](https://github.com/DEFRA/water-abstraction-system) creates, it's no longer possible. Because the mod log column migrations have not been run in production yet, we're just going to delete them. This might be a bit messy for our local environments, but it avoids leaving a 'dead' field in the database. So, in this change, we delete the old migrations and then add our new table migration.
Cruikshanks
added a commit
to DEFRA/water-abstraction-system
that referenced
this pull request
Aug 18, 2024
https://eaflood.atlassian.net/browse/WATER-4565 > Part of the work to display a licence's history to users (mod log) In [Add mod_log field to return versions view](#1252), we added a migration to add the new `mod_log` field to the return version view. This field was intended to hold details from the linked NALD mod log record, such as who, when, and why the version was created. However, we've since learned that a version in NALD can have multiple mod log records. So, let's rename the field `mod_logs` and change its default to an array. 😜 **It's a trap!** We have made the same mistake as the previous team. When we believed that a version (charge, licence, or return) had a one-to-one relationship with NALD mod log records, a JSONB field in each was a suitable solution to capturing just the information we needed. After all, it's only the historical records we have to worry about. But when we learned that a version record could be linked to multiple mod logs, we should have taken a different approach. Instead, we ploughed on with our JSON column [altering its name and default](DEFRA/water-abstraction-service#2612) to match. 🤦😬 Once you've started down the JSONB road, it is easy to get trapped! Thankfully, we've come to our senses before this saw the light of day in production. The mod log data we are importing needs to go into its own table, and we will link our version records accordingly. A 'standard' relationship database solution. We've made a change to [water-abstraction-service to delete the column migrations and add a new table one instead](DEFRA/water-abstraction-service#2613). This is because of the interaction between our views and the tables: you cannot drop a column referenced by a view. That means we need to do the same thing here: delete the migration we added and pretend it never happened!
Cruikshanks
added a commit
to DEFRA/water-abstraction-import
that referenced
this pull request
Aug 18, 2024
https://eaflood.atlassian.net/browse/WATER-4563 https://eaflood.atlassian.net/browse/WATER-4564 https://eaflood.atlassian.net/browse/WATER-4565 > Part of the work to display a licence's history to users (mod log) **It's a trap!** We have made the same mistake as the previous team. When we believed that a version (charge, licence, or return) had a one-to-one relationship with NALD mod log records, a JSONB field in each was a suitable solution to capturing just the information we needed. After all, it's only the historical records we have to worry about. But when we learned that a version record could be linked to multiple mod logs, we should have taken a different approach. Instead, we ploughed on with our JSON column [altering its name and default](DEFRA/water-abstraction-service#2612) to match. 🤦😬 Once you've started down the JSONB road, it is easy to get trapped! Thankfully, we've come to our senses before this saw the light of day in production. The mod log data we are importing needs to go into its own table, and we will link our version records accordingly. A 'standard' relationship database solution. So, in this change, we replace the import we did just for return mod logs into the return versions table with one that can handle importing all mod logs for all version types. The change [Replace mod_logs columns with a table](DEFRA/water-abstraction-service#2613) has created the table we can now populate using queries we'll add in this change.
Cruikshanks
added a commit
that referenced
this pull request
Aug 19, 2024
https://eaflood.atlassian.net/browse/WATER-4563 https://eaflood.atlassian.net/browse/WATER-4564 https://eaflood.atlassian.net/browse/WATER-4565 > We foolishly merged [Replace mod_logs columns with a table](#2613) as an empty commit hence doing it for real this time! > Part of the work to display a licence's history to users (mod log) **It's a trap!** We have made the same mistake as the previous team. When we believed that a version (charge, licence, or return) had a one-to-one relationship with NALD mod log records, a JSONB field in each was a suitable solution to capturing just the information we needed. After all, it's only the historical records we have to worry about. But when we learned that a version record could be linked to multiple mod logs, we should have taken a different approach. Instead, we ploughed on with our JSON column [altering its name and default](#2612) to match. 🤦😬 Once you've started down the JSONB road, it is easy to get trapped! Thankfully, we've come to our senses before this saw the light of day in production. The mod log data we are importing needs to go into its own table, and we will link our version records accordingly. A 'standard' relationship database solution. We could just add a migration that drops the columns and adds the new table. But because of the interaction between the views [water-abstraction-system](https://github.com/DEFRA/water-abstraction-system) creates, it's no longer possible. Because the mod log column migrations have not been run in production yet, we're just going to delete them. This might be a bit messy for our local environments, but it avoids leaving a 'dead' field in the database. So, in this change, we delete the old migrations and then add our new table migration.
Cruikshanks
added a commit
that referenced
this pull request
Aug 19, 2024
https://eaflood.atlassian.net/browse/WATER-4563 https://eaflood.atlassian.net/browse/WATER-4564 https://eaflood.atlassian.net/browse/WATER-4565 > We foolishly merged [Replace mod_logs columns with a table](#2613) as an empty commit hence doing it for real this time! > Part of the work to display a licence's history to users (mod log) **It's a trap!** We have made the same mistake as the previous team. When we believed that a version (charge, licence, or return) had a one-to-one relationship with NALD mod log records, a JSONB field in each was a suitable solution to capturing just the information we needed. After all, it's only the historical records we have to worry about. But when we learned that a version record could be linked to multiple mod logs, we should have taken a different approach. Instead, we ploughed on with our JSON column [altering its name and default](#2612) to match. 🤦😬 Once you've started down the JSONB road, it is easy to get trapped! Thankfully, we've come to our senses before this saw the light of day in production. The mod log data we are importing needs to go into its own table, and we will link our version records accordingly. A 'standard' relationship database solution. We could just add a migration that drops the columns and adds the new table. But because of the interaction between the views [water-abstraction-system](https://github.com/DEFRA/water-abstraction-system) creates, it's no longer possible. Because the mod log column migrations have not been run in production yet, we're just going to delete them. This might be a bit messy for our local environments, but it avoids leaving a 'dead' field in the database. So, in this change, we delete the old migrations and then add our new table migration.
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-4563
https://eaflood.atlassian.net/browse/WATER-4564
https://eaflood.atlassian.net/browse/WATER-4565
In Add mod_log fields 2 return, licence & charge ver., we added a data migration to add a new
mod_logfield to the charge, licence, and return version tables in thewaterschema. This field was intended to hold details from the linked NALD mod log record, such as who, when, and why the version was created.However, we've since learned that a version in NALD can have multiple mod log records. Therefore, we want to change the name to make that clear. We also change the default to avoid complexity in the code. Now, it will default to an empty JSONB array rather than an empty object.
This change is the data migration for that.