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

Fix for view licence abstraction conditions V2 #853

Merged
merged 11 commits into from
Mar 25, 2024

Conversation

Cruikshanks
Copy link
Member

https://eaflood.atlassian.net/browse/WATER-4332

@Cruikshanks stab at the problem!

As part of the testing for the new view licence summary page, QA pointed out that the more complicated licences did not match the legacy system. We would see different results for the 'Abstraction conditions' section.

The first thing we'd overlooked was the need to ensure we were only looking at the 'current' licence version. After more investigation, we then discovered that the permit licence JSON blob the legacy code is using is replicated in tables that already exist in the water schema. We found we could access the same data using these tables to get a matching result to the legacy view.

This change adds a new service to fetch the abstraction data we need and updates the presenter to generate the distinct list of conditions to display.

https://eaflood.atlassian.net/browse/WATER-4332

As part of the testing of for the new view licence summary page QA pointed out that the more complicated licences did not match the legacy system. We would see different results for the 'Abstraction conditions' section.

First thing we'd overlooked was the need to ensure we were only looking at the 'current' licence version. After more investigation we then discovered that the permit licence JSON blob the legacy code is using is replicated in tables that already exist in the `water` schema. We found we could access the same data using these tables to get a matching result to the legacy view.

This change adds a new service to fetch the abstraction data we need and updates the presenter to generate the distinct list of conditions to display.
We should be using camelCase rather than snake case for the properties. Also ensure the comments reflect what we are doing.
We were able to build on the fantastic insight that the information we need is in tables so we can avoid the JSON blob. We were able to put together a query that will extract the abstraction conditions within the query rather than having to bring the data back and manipulate the results.

We have done something we don't normally do; process the results within the service. After building the service and passing the raw results to the presenter it felt a better fit to move the processing to the service. That way the knowledge that we have to deal with the situation of the same condition being assigned to different purposes stays in one place.

Also makes testing a lot easier! :-)
From what I can tell FetchLicenceAbstractionConditionsService replaces what this service and the presenter were doing so we can park it for now.
This is exactly the same change as made in the original PR.
This also includes a few corrections, some re-ordering of things into alphabetical order etc.
@Cruikshanks Cruikshanks self-assigned this Mar 23, 2024
@Cruikshanks Cruikshanks marked this pull request as ready for review March 25, 2024 09:51
@Cruikshanks Cruikshanks merged commit 1a0d96a into main Mar 25, 2024
6 checks passed
@Cruikshanks Cruikshanks deleted the alans-crack-at-abstraction-conditions-bugs branch March 25, 2024 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants