-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update view licence summary to use new points data #1316
Merged
Cruikshanks
merged 10 commits into
main
from
update-view-licence-summary-use-new-points
Sep 27, 2024
Merged
Update view licence summary to use new points data #1316
Cruikshanks
merged 10 commits into
main
from
update-view-licence-summary-use-new-points
Sep 27, 2024
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
Cruikshanks
added
the
housekeeping
Refactoring, tidying up or other work which supports the project
label
Sep 9, 2024
I hadn't spotted that the summary page displays "Source of supply". This is taken from the first point in the We don't have that on the points we're importing, which means we need to do a bit more work around points before we can progress this PR. |
https://eaflood.atlassian.net/browse/WATER-4600 https://eaflood.atlassian.net/browse/WATER-4645 We've been extending and amending the import of return versions from NALD to WRLS as part of our work to switch from NALD to WRLS to manage them. In [Update Return req set up journey to use new points](#1301) we updated the return requirement setup journey to use the new `water.licence_version_purpose_points` table we've created and now populate thanks to [a change to the import](#1301). The one other place we are referencing the `permit.licence` table to retrieve point information is the view licence summary page. This change updates the logic in it to grab the points data from `water.licence_version_purpose_points` instead.
Cruikshanks
force-pushed
the
update-view-licence-summary-use-new-points
branch
from
September 26, 2024 11:54
5b6cf23
to
fb78256
Compare
Update its query to fetch the new point and source data and stop looking in `permit.licence`.
We don't have to perform any matching across purposes to stuff from the JSONB blob any more.
The current test was claiming that the `water` schema was returning no licence versions but `permit.licence` did contain a `current_version` property. So, the test was unrealistic. But sticking with the idea we are dealing with a 'broken' licence (it has no licence versions) now we are not looking at the `permit.licence` record you would expect to see no points or source returned.
Especially as it had no tests!
Cruikshanks
requested review from
robertparkinson,
Jozzey,
jonathangoulding,
Beckyrose200 and
rvsiyad
September 26, 2024 18:49
Jozzey
approved these changes
Sep 27, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cruikshanks
added a commit
that referenced
this pull request
Oct 2, 2024
https://eaflood.atlassian.net/browse/WATER-4600 https://eaflood.atlassian.net/browse/WATER-4645 In [a recent change](#1316) we updated the view licence summary to use our new 'point' tables rather than the legacy `permit.licence` JSON blob. In the change we assumed that we would not get licences with licence version purposes that do not have points. We should have known better when dealing with NALD data! In fairness, its commonly where data has been corrupted from deletions happening in NALD that don't happen on our side. But when it happens it means the page 'errors' rather than falling back to just showing nothing. Specifically, it's when trying to extract the 'Source of supply'. So, in this change we amend the presenter to avoid errors when dealing with 'bad' licence data.
Cruikshanks
added a commit
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 3, 2024
https://eaflood.atlassian.net/browse/WATER-4600 https://eaflood.atlassian.net/browse/WATER-4645 In [a recent change](#1316) we updated the view licence summary to use our new 'point' tables rather than the legacy `permit.licence` JSON blob. In the change, we assumed that we would not get licences with licence version purposes that do not have points. We should have known better when dealing with NALD data! In fairness, it is common for data to be corrupted from deletions happening in NALD that don't occur on our side. But when it happens, it means the page 'errors' rather than falling back to just showing nothing. Specifically, it's when extracting the 'Source of supply'. So, in this change, we amend the presenter to avoid errors when dealing with 'bad' licence data.
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 [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-4600
https://eaflood.atlassian.net/browse/WATER-4645
We've been extending and amending the import of return versions from NALD to WRLS as part of our work to switch from NALD to WRLS to manage them.
In Update Return req set up journey to use new points we updated the return requirement setup journey to use the new
water.licence_version_purpose_points
table we've created and now populate thanks to a change to the import.The one other place we are referencing the
permit.licence
table to retrieve point information is the view licence summary page. This change updates its logic to grab the points data using our new points solution:water.licence_version_purposes
linked towater.points
viawater.licence_version_purpose_points
.