Fix use of new points in setup ret. req. journey #1314
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-4600 https://eaflood.atlassian.net/browse/WATER-4645 > Part of the work to migrate return versions from NALD to WRLS 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). This change updates the logic behind the 'Approve' step in the journey to use the new points rather than those pulled from the `permit.licence` record.
Cruikshanks
added
the
housekeeping
Working on [Update rtn requirement fixtures to use new points](DEFRA/water-abstraction-acceptance-tests#134) exposed that copy from an existing return requirement was not actually working. We tracked the issue down to the use of IDs. The changes we had made to the journey to support reading points from `water.licence_version_purpose_points` were the cause. We were storing the licence point ID in the session and using it as the value when displaying the points on the manual journey. But when you copy from existing, the source point comes from `water.return_requirement_points`. When it got to the `/check` page its presenter was trying to match the ID we'd stored in the session (from return points) with the ID of a licence point. No matching was being found causing the error. Fortunately, the one value both tables share is the NALD point ID. Like with our journey, when you create the return requirement in NALD you select from the same licence points as we do. This means there will _always_ be a match between licence points and return points. You just have to match on NALD Point ID instead.
Cruikshanks
changed the title
Cruikshanks
deleted the
updateapprove-ret-requirements-for-new-points
branch
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_pointstable we've created and now populate thanks to a change to the import.We then began updating our acceptance tests to update the return requirement fixtures to use new points but found the 'Copy from existing journey' was failing.
We tracked the issue down to the use of IDs. The changes we had made to the journey to support reading points from
water.licence_version_purpose_pointswere the cause.Starting with the manual journey, we updated the fetch points service to retrieve points information from
water.licence_version_purpose_points. The ID we provided in the presenter to the view was theLicenceVersionPurposePoint.id. This is then what gets stored in the session when a user makes a selection.Then, we did something similar in the 'Use abstraction data' journey. Again, we retrieve the licence version purpose points and store their IDs in the session.
In both journeys, when you get to the
/checkscreen, it will use theFetchPointsServiceto fetch the licence version purpose points again. The presenter for the check page can match the selected points in the session to the results of the fetch points service.The journey we broke was 'Copy from existing'. In that scenario, the points are sourced from
water.return_requirement_points, which means they have a different ID than what is inwater.licence_version_purpose_pointsthat we are storing in the session. This means that when you then get passed to the/checkpage, it is unable to find a match in the results fromFetchPointsService,so an error is thrown.Fortunately, the one value both tables share is the NALD point ID. Like with our journey, when you create the return requirement in NALD, you select from the same licence points as ours, which also have a NALD point ID. This means there will always be a match between licence points and return points. You just have to match on NALD Point ID instead of their IDs.