-
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
Fix for view licence abstraction conditions V2 #853
Merged
Cruikshanks
merged 11 commits into
main
from
alans-crack-at-abstraction-conditions-bugs
Mar 25, 2024
Merged
Fix for view licence abstraction conditions V2 #853
Cruikshanks
merged 11 commits into
main
from
alans-crack-at-abstraction-conditions-bugs
Mar 25, 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
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.
robertparkinson
requested changes
Mar 25, 2024
app/services/licences/fetch-licence-abstraction-conditions.service.js
Outdated
Show resolved
Hide resolved
robertparkinson
approved these changes
Mar 25, 2024
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-4332
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.