Skip to content
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
merged 10 commits into from
Sep 27, 2024

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Sep 9, 2024

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 to water.points via water.licence_version_purpose_points.

@Cruikshanks Cruikshanks added the housekeeping Refactoring, tidying up or other work which supports the project label Sep 9, 2024
@Cruikshanks Cruikshanks self-assigned this Sep 9, 2024
@Cruikshanks Cruikshanks added the do not merge Used for spikes and experiments label Sep 9, 2024
@Cruikshanks
Copy link
Member Author

I hadn't spotted that the summary page displays "Source of supply". This is taken from the first point in the permit.licence JSONB blob.

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 Cruikshanks force-pushed the update-view-licence-summary-use-new-points branch from 5b6cf23 to fb78256 Compare September 26, 2024 11:54
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 Cruikshanks removed the do not merge Used for spikes and experiments label Sep 26, 2024
@Cruikshanks Cruikshanks marked this pull request as ready for review September 26, 2024 18:49
Copy link
Contributor

@Jozzey Jozzey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Cruikshanks Cruikshanks merged commit f6d4161 into main Sep 27, 2024
5 of 6 checks passed
@Cruikshanks Cruikshanks deleted the update-view-licence-summary-use-new-points branch September 27, 2024 10:21
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
Labels
housekeeping Refactoring, tidying up or other work which supports the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants