Implement new 'points' job #1028
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-4645 In [Start importing licence version points to 'water'](#1009) we added a step to the **licence-import** job to start importing licence points to a new table we'd created: `water.licence_version_purpose_points`. However, we've since encountered two issues with that original approach. - We've found the legacy UI also display details about sources, which are connected to points. Our original solution didn't cater for this which means we cannot switch wholly to it - Updating the `licence_version_purpose_points` table happens as the last step of the **licence-import** job. But we've found it is often failing because of bad licence data, which means our new step is never getting called So, in this change we are creating a whole new job to handle points. Part of the change is NALD points will now sit in their own table. `return_requirement_points` and `licence_version_purpose_points` will link to it rather than holding their own copies of the licence. The job job will be responsible for updating all three.
The query is outdated anyway.
This replaces what was happening in return-versions.
Cruikshanks
added a commit
to DEFRA/water-abstraction-service
that referenced
this pull request
Sep 24, 2024
This will create the new sources table then populate it. Then it will create the new `points` table and update the existing points tables ready for linking to it. We don't drop any redundant columns in this migration because there will be a period where we have to wait for the [changes we've made to the **water-abstraction-import**](DEFRA/water-abstraction-import#1028) to take affect.
Cruikshanks
added a commit
to DEFRA/water-abstraction-system
that referenced
this pull request
Sep 24, 2024
https://eaflood.atlassian.net/browse/WATER-4600 https://eaflood.atlassian.net/browse/WATER-4645 > Part of the work to migrate return versions from NALD to WRLS In [Add Licence Version Purpose Point model](#1288) we added our second model that represented a table in the `water` schema that hold point information. When we added `ReturnRequirementPointModel` it was to represent points linked to a return requirement. In the setup journey for new return requirements you select the points to associate from those against the licence. We were extracting this information from the `permit.licence` table until we realised that [water-abstraction-import](https://github.com/DEFRA/water-abstraction-import) does not populate it when a licence is 'dead'. We need to be able to create new return requirements even for dead licences in order to correct historic information. So, this source being defunct we made changes to the import to populate a new `water.licence_version_purpose_points` from which we could then derive a licence's points. That worked great for returns setup, so we moved to updating the view licence summary page to use the new source. It also was effected by the issue that point information was no longer available if the licence was dead. The page includes information about the water source for a licence. It was then we realised that the water source is derived from a licence's points! What we'd built for return requirements and licence version purposes was missing this information. We investigated the NALD data properly and came away with some realisations - NALD maintains a central list of 'points' (we knew this, but like addresses felt this was unnecessary) - Each point holds a lot more data than just the National Grid References for its location - Each NALD point is linked to a NALD source, and these too hold extra information There was extra information users were maintaining against each point that we were missing. This information explained why points were centrally managed and other records were linked to them. This realisation meant we needed to completely re-think how we should maintain points in WRLS going forward. So, in [the water-abstraction-service](DEFRA/water-abstraction-service#2638) we've created a migration to add new `sources` and `points` tables. We'also amended the existing `licence_version_purpose_points` and `return_requirement_points` to make it possible to link them to `points`. (Their `ngr_*` fields plus things like `description` are now defunct but to avoid breaking changes we'll need to drop them in a future change.) In [water-abstraction-import](DEFRA/water-abstraction-import#1028) we've added a new job to ensure `points` gets populated as part of the import. We've also incorporated populating and linking `licence_version_purpose_points` and `return_requirement_points` as part of the new job. With those changes in place, this change updates everything around the existing point models, as well as adding the new ones. The water sources are reference data, so as part of adding `SourceModel` we include a seeder. The existing models and their tests are updated to represent the fact they are now just `many-to-many` models. This means we make `points` a top level property of `ReturnRequirementModel` and `LicenceVersionPurposeModel`. The logic for describing a point is copied to `PointModel`. We'll delete `BasePointModel` in the next change, where we update the existing functionality to use the new structure.
Cruikshanks
added a commit
to DEFRA/water-abstraction-system
that referenced
this pull request
Sep 25, 2024
https://eaflood.atlassian.net/browse/WATER-4600 https://eaflood.atlassian.net/browse/WATER-4645 > Part of the work to migrate return versions from NALD to WRLS In [Add Licence Version Purpose Point model](#1288), we added our second model, which represented a table in the `water` schema that holds point information. When we added `ReturnRequirementPointModel,` it was to represent points linked to a return requirement. In the setup journey for new return requirements, you select the points to associate with those against the licence. We were extracting this information from the `permit.licence` table until we realised that [water-abstraction-import](https://github.com/DEFRA/water-abstraction-import) does not populate it when a licence is 'dead'. We need to be able to create new return requirements, even for dead licences, to correct historical information. So, this source being defunct, we made changes to the import to populate a new `water.licence_version_purpose_points` from which we could then derive a licence's points. That worked great for returns setup, so we also updated the view licence summary page to use `water.licence_version_purpose_points`. The issue that point information was not available if the licence was dead affected it. The page also includes information about a licence's water source. It was then that we realised that the water source is derived from a licence's points! What we'd built for return requirements and licence version purposes was missing this information. We investigated the NALD data properly and came up with some new realisations. - NALD maintains a central list of 'points' (we knew this, but like addresses felt this was unnecessary) - Each point holds a lot more data than just the National Grid References for its location - Each NALD point is linked to a NALD source, and these, too, hold extra information There was extra information users were maintaining against each point that we were missing. This information explained why points were centrally managed and linked to other records. This realisation meant we needed to completely re-think how we should maintain points in WRLS going forward. So, in [the water-abstraction-service](DEFRA/water-abstraction-service#2638), we've created a migration to add new `sources` and `points` tables. We also amended the existing `licence_version_purpose_points` and `return_requirement_points` to make it possible to link them to `points`. (Their `ngr_*` fields plus things like `description` are now defunct, but to avoid breaking changes, we'll need to drop them in a future change.) In [water-abstraction-import](DEFRA/water-abstraction-import#1028), we've added a new job to ensure `points` get populated as part of the import. We've also incorporated populating and linking `licence_version_purpose_points` and `return_requirement_points` into the new job. With those changes in place, this change updates everything around the existing point models and adds new ones. The water sources are reference data, so we include a seeder when adding `SourceModel`. We update the existing models and their tests to reflect the fact that they are now just `many-to-many` models. This means we make `points` a top-level property of `ReturnRequirementModel` and `LicenceVersionPurposeModel`. We then go through _all_ existing logic and update it to use the new points data structure.
Cruikshanks
added a commit
that referenced
this pull request
Oct 2, 2024
https://eaflood.atlassian.net/browse/WATER-4645 > Part of the work to migrate return versions from NALD to WRLS In [Implement new 'points' job](#1028) we amended the import service to include a new 'points' job. Having realised that the way were intending to store point information was insufficient, we made enough changes to our points schema to warrant its own job to manage importing the NALD data. We thought we'd captured everything 'point' related in the change but have since spotted 'Means of abstraction'. This is linked to a point in NALD and gives how water will be abstracted, for example, "Surface Mounted Pump (Fixed)". We are required to display this in the UI. Having [added the new column](DEFRA/water-abstraction-service#2642), this ensures it gets populated.
Cruikshanks
added a commit
that referenced
this pull request
Oct 7, 2024
https://eaflood.atlassian.net/browse/WATER-4645 > Part of the work to migrate return versions from NALD to WRLS In [Implement new 'points' job](#1028) we amended the import service to include a new 'points' job. Having realised that the way we intended to store point information was insufficient, we made enough changes to our points schema to warrant its own job to manage importing the NALD data. We thought we'd captured everything 'point' related in the change but have since spotted 'Means of abstraction'. This is linked to an abstraction purpose in NALD and gives how water will be abstracted, for example, "Surface Mounted Pump (Fixed)". We are required to display this in the UI. Having [added the new column](DEFRA/water-abstraction-service#2642), this ensures it gets populated.
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-4645
In Start importing licence version points to 'water', we added a step to the licence-import job to start importing licence points to a new table we'd created:
water.licence_version_purpose_points.However, we've since encountered two issues with that original approach.
licence_version_purpose_pointstable happens as the last step of the licence-import job. But we've found it is often failing because of bad licence data, which means our new step is never getting calledSo, in this change, we are creating a whole new job to handle points. Part of the change is NALD points will now sit in their own table.
return_requirement_pointsandlicence_version_purpose_pointswill link to it rather than holding their own copies of the licence. The job will be responsible for updating all three.