Update no-returns-required to use submit service #708
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-4262 When we added validation and controls to the 'no returns required' page of the returns requirements setup journey it was the first page that we did so. It just has 3 radio buttons and no business logic so validation of the submitted form was quite simple. Therefore, so was our solution. The [Select a start date page](#646) on the overhand was a different story. Not only did we have to cover date validation for the first time, we had to combine the date entry with radio buttons and business logic. It was at this point we decided a whole new `Submit[Page]Service` was needed to manage handling the validation. In both cases we will also need to handle persisting valid results into the session so a service separate from what we use during the `GET` request starts to make a lot of sense. This change revisits the 'no returns required' page to update it to use a `SubmitNoReturnsRequiredService`. Hopefully, this will make clear our patterns and conventions going forward.
Cruikshanks
added
the
housekeeping
The submit service is doing this for us now.
We'll never pass an error in now because we'll never call this in the POST handler.
We adding an error property to the page data to be consistent across the pages. As we learnt on the 'Select a start date page' we might need something more complex than just the text message. Having an `error:` object gives some consistency to the page data and the flexibility to add whatever need (or not).
We're trying to keep a clean separation of concerns. The view templates manage everything to do with the layout and how things are presented. The 'presenters' are responsible for formatting the data the views need, whether it is to directly to display or make decisions on what to display.
And remove unused journey property from page data.
As our unit test confirmed it was checking for a value but would accept any value. Now it will only accept certain values.
Cruikshanks
deleted the
update-no-returns-required-to-use-submit-service
branch
Cruikshanks
added a commit
that referenced
this pull request
Feb 5, 2024
Having completed the [Returns required journey - Select start date page iteration 2 (1 of 7)](#646) I was expecting the pattern implemented to follow that more than what we'd originally done for 'no-returns-required'. But with so few pages implemented with controls it's hard to see a pattern! So, that wasn't made clear enough. To help I've completed [Update no-returns-required to use submit service](#708) which updates the 'no-returns-required' page to follow the template laid out for 'start-date'. Essentially, validation and error handling will now be done in a `Submit[Page]Service` rather than reuse `[Page]Service`. And so again, I'll go through and contribute to this PR to highlight the changes and when possible, expand on the reasons why.
Demwunz
added a commit
that referenced
this pull request
Feb 5, 2024
* Returns required journey - Select return reason page iteration 2 (2 of 7) https://eaflood.atlassian.net/browse/WATER-4265 This is the second page in the returns required journey. This page will have radio control options. This page will have validation to ensure that the user has selected a radio option. * feat(app): WATER-4265 Added controls and validation for step 2 of 7 This commit adds controls for the 2nd step in the no returns required journey. It also has validation which checks to ensure a value is selected on the form * fix(app): WATER-4265 fixed broken test test had wrong data inside. fixed with correct data * fix: fix broken test * fix: amend sonarcloud issues * fix: amend another sonarcloud issue * Thoughts and opinions Having completed the [Returns required journey - Select start date page iteration 2 (1 of 7)](#646) I was expecting the pattern implemented to follow that more than what we'd originally done for 'no-returns-required'. But with so few pages implemented with controls it's hard to see a pattern! So, that wasn't made clear enough. To help I've completed [Update no-returns-required to use submit service](#708) which updates the 'no-returns-required' page to follow the template laid out for 'start-date'. Essentially, validation and error handling will now be done in a `Submit[Page]Service` rather than reuse `[Page]Service`. And so again, I'll go through and contribute to this PR to highlight the changes and when possible, expand on the reasons why. * Housekeeping - clean up after merging in `main` * Correction - The module name does not match file * Correction - wrong page in comments That was me! Doh! * Consistency - add new submit service This is slightly pre-emptive but we know we will need to handle persisting valid payloads to the sessions record soon. Having a service separate from what we call on the GET request gives us an appropriate place to put that. It also allows us to simplify the controller logic in the meantime. * Remove error handling from the presenter Now handled by the submit service. * Remove optional error from the GET service We'll never pass an error in now because we'll never call this in the POST handler. * Update view to reference error Also bring it inline with no returns required and start date. * Update controller to use new service * Update reference in validator We changed the control to match the page and everything else so we need to update the validator to use the new reference. * Fix the tests * Remove unused static lookup * Make the validator more robust As our unit test confirmed it was checking for a value but would accept any value. Now it will only accept certain values. * Update no-rtns-req validator Now it is consistent with what we just did in the `ReasonValidator`. * Add missing JSDoc comments * Correct test title * Add test for new submit service --------- Co-authored-by: Alan Cruikshanks <[email protected]>
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-4262
When we added validation and controls to the 'no returns required' page of the returns requirements setup journey it was the first page that we did so. It just has 3 radio buttons and no business logic so validation of the submitted form was quite simple. Therefore, so was our solution.
The Select a start date page on the overhand was a different story. Not only did we have to cover date validation for the first time, but we also had to combine the date entry with radio buttons and business logic. It was at this point we decided a whole new
Submit[Page]Servicewas needed to manage handling the validation.In both cases, we will also need to handle persisting valid results into the session so a service separate from what we use during the
GETrequest starts to make a lot of sense.This change revisits the 'no returns required' page to update it to use a
SubmitNoReturnsRequiredService. Hopefully, this will make clear our patterns and conventions going forward.