Replace rtn vers mod log import with all versions #998
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-4565 > Part of the work to display a licence's history to users (mod log) In [Include mod log info in return versions import](#990) we extended the existing return versions import (part of 'charging-import') to extract the matching return log for a return version and populate its new `mod_log` field. However, we've since learned that a version in NALD can have multiple mod log records. Each my have different information and our users want to see this in WRLS. This change amends what we did to extract _all_ mod logs linked to a return version and store them in its now named `mod_logs` field.
Cruikshanks
changed the title
This never went live into production.
It felt prudent to keep them distinct from the other queries and follow the existing pattern.
Cruikshanks
added a commit
that referenced
this pull request
Aug 19, 2024
https://eaflood.atlassian.net/browse/WATER-4636 > Part of the work to display a licence's history to users (mod log) Now that we are [importing the NALD mod logs](#998) and linking them to their WRLS version records (charge, licence and returns) we can look to update the WRLS records with their missing reasons. We've done some work with the Billing & Data team to map NALD reasons to those in WRLS. There are a couple that don't map, and for those [water-abstraction-system](https://github.com/DEFRA/water-abstraction-system) will fall back to whatever reason description we have in the mod log. For the rest though, we can fill in their missing `reason` field with the mapped WRLS reason. We can then start displaying this to users in the out new UI screens. This change adds another query to the charging data import, that will do the mapping for those WRLS return versions that - have an empty `reason`` - have mod log records - the first mod log record has a reason code - the reason code is mapped to WRLS
Cruikshanks
added a commit
that referenced
this pull request
Aug 20, 2024
https://eaflood.atlassian.net/browse/WATER-4636 > Part of the work to display a licence's history to users (mod log) Now that we are [importing the NALD mod logs](#998) and linking them to their WRLS version records (charge, licence and returns) we can look to update the WRLS records with their missing reasons. We've worked with the Billing & Data team to map NALD reasons to those in WRLS. There are a couple that don't map, and for those, [water-abstraction-system](https://github.com/DEFRA/water-abstraction-system) will fall back to whatever reason description we have in the mod log. For the rest, though, we can fill in their missing `reason` field with the mapped WRLS reason. We can then start displaying this to users on our new UI screens. This change adds another query to the charging data import that will do the mapping for those WRLS return versions. - that have an empty `reason` - that have mod log records - the first mod log record has a reason code - the reason code is mapped to WRLS
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
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 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 has created the table we can now populate using queries we'll add in this change.