Add new points and sources tables & amend existing #2638
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 > Part of the work to migrate return versions from NALD to WRLS We recently added a new `water.licence_version_purpose_points` table to support our work migrating management of return versions to WRLS from NALD. Users need to be able to select from a licence's points when setting up a new return version and we found the source the legacy code was using (`permit.licence`) was compromised because it is not always populated during the import process. That solved our problems when setting up a new return version. We then intended to remove _all_ references to `permit.licence` when retrieving points information, for example, when viewing the licence summary. But we became blocked because we are also extracting the source from the first point in `permit.licence`. We didn't factor that into our new points tables (this includes `water.return_requirement_points`). We also knew NALD works with the concept of a distinct list of points, that are then linked to licence version purposes and return version requirements. We were hoping to avoid working in this way. But not only are the source details linked to the distinct points, there is other information we are missing. So, this change covers two aspects - add new tables to store sources and points - update our existing tables to link to the new `water.points` Then, we'll make changes to [water-abstraction-import](https://github.com/DEFRA/water-abstraction-import) to ensure the `water.points` table is updated during import, and the existing tables are correctly linked to it.
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 We recently [Added new points and sources tables & amend existing](#2638) to the service. Not only does it support our work on return versions, it means we can move away from abstracting point information from the licence JSON blob in `permit.licence`. We're looking to also migrate the pages linked to from the view licence summary, which are currently in the legacy [water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui). One of them is information on points. 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 there, for example, "Surface Mounted Pump (Fixed)". We need to capture this information as we import the point information from NALD. So, the first step is to add the table and update `water.points` so that it can be linked to a method. > We have to name our table and entities in code and the DB. `means` doesn't really mean anything, and `means_of_abstraction` causes problems because it is not plural. Hence `methods`!
Cruikshanks
added a commit
to DEFRA/water-abstraction-system
that referenced
this pull request
Oct 3, 2024
https://eaflood.atlassian.net/browse/WATER-4645 > Part of the work to migrate return versions from NALD to WRLS We recently [Updated view licence summary to use new points data](#1316). This is part of a series of changes to help us move away from extracting point information from the licence JSON blob in `permit.licence`. We're also looking to migrate the pages linked to from the view licence summary, which are currently in the legacy [water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui). One of them is information on 'points'. We thought we'd captured everything 'point' related in [the changes we made to the points schema](DEFRA/water-abstraction-service#2638) but have since spotted 'Means of abstraction'. This is linked to an abstraction purpose point in NALD and gives how water will be abstracted there, for example, "Surface Mounted Pump (Fixed)". We'll [populate this field](DEFRA/water-abstraction-import#1029) as we import a licence's point information from NALD. This change updates the view so we can access the new field.
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 We recently [Added new points and sources tables & amend existing](#2638) to the service. This not only supports our work on return versions, it means we can move away from abstracting point information from the licence JSON blob in `permit.licence`. We're also looking to migrate the pages linked to from the view licence summary, which are currently in the legacy [water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui). One of them is information on points. 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 point in NALD and gives how water will be abstracted there, for example, "Surface Mounted Pump (Fixed)". We must capture this information as we import a licence's point information from NALD. So, the first step is to update the `water.licence_version_purpose_points` table to add a new `method` field that will allow us to store this information.
Cruikshanks
added a commit
to DEFRA/water-abstraction-system
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 We recently [Updated view licence summary to use new points data](#1316). This is part of a series of changes to help us move away from extracting point information from the licence JSON blob in `permit.licence`. We're also looking to migrate the pages linked to from the view licence summary, which are currently in the legacy [water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui). One of them is information on 'points'. We thought we'd captured everything 'point' related in [the changes we made to the points schema](DEFRA/water-abstraction-service#2638) but have since spotted 'Means of abstraction'. This is linked to an abstraction purpose point in NALD and gives how water will be abstracted there, for example, "Surface Mounted Pump (Fixed)". We'll [populate this field](DEFRA/water-abstraction-import#1029) as we import a licence's point information from NALD. This change updates the view so we can access the new field.
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
We recently added a new
water.licence_version_purpose_pointstable to support our work migrating management of return versions to WRLS from NALD. Users need to be able to select from a licence's points when setting up a new return version and we found the source the legacy code was using (permit.licence) was compromised because it is not always populated during the import process.That solved our problems when setting up a new return version. We then intended to remove all references to
permit.licencewhen retrieving points information, for example, when viewing the licence summary. But we became blocked because we are also extracting the source from the first point inpermit.licence.We didn't factor that into our new points tables (this includes
water.return_requirement_points). We also knew NALD works with the concept of a distinct list of points, that are then linked to licence version purposes and return version requirements. We were hoping to avoid working in this way. But not only are the source details linked to the distinct points, there is other information we are missing.So, this change covers two aspects
water.pointsThen, we'll make changes to water-abstraction-import to ensure the
water.pointstable is updated during import, and the existing tables are correctly linked to it.