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

Add controls & validation to rtn reqs reason page #688

Conversation

Demwunz
Copy link
Contributor

@Demwunz Demwunz commented Jan 25, 2024

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.

…f 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.
@Demwunz Demwunz self-assigned this Jan 25, 2024
@Demwunz Demwunz changed the title Returns required journey - Select return reason page iteration 2 of 7 Returns required journey - Select return reason page iteration 2 (2/7) Jan 25, 2024
@Demwunz Demwunz changed the title Returns required journey - Select return reason page iteration 2 (2/7) Returns required journey - Select return reason page iteration 2 (2 of 7) Jan 25, 2024
…quired-journey-select-return-reason-page-iteration-2-2-of-7
…quired-journey-select-return-reason-page-iteration-2-2-of-7
@Cruikshanks Cruikshanks added the enhancement New feature or request label Jan 30, 2024
…quired-journey-select-return-reason-page-iteration-2-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
@Demwunz Demwunz marked this pull request as ready for review February 1, 2024 13:01
@Cruikshanks Cruikshanks force-pushed the WATER-4265-returns-required-journey-select-return-reason-page-iteration-2-2-of-7 branch from 71620bd to 91a63da Compare February 5, 2024 00:11
…quired-journey-select-return-reason-page-iteration-2-2-of-7
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.
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.
Now handled by the submit service.
We'll never pass an error in now because we'll never call this in the POST handler.
Also bring it inline with no returns required and start date.
We changed the control to match the page and everything else so we need to update the validator to use the new reference.
As our unit test confirmed it was checking for a value but would accept any value. Now it will only accept certain values.
Now it is consistent with what we just did in the `ReasonValidator`.
@Cruikshanks Cruikshanks changed the title Returns required journey - Select return reason page iteration 2 (2 of 7) Add controls & validation to rtn reqs reason page Feb 5, 2024
@Cruikshanks
Copy link
Member

Adjusted the title to match our standards for PRs and what the final commit title will be.

For reference we try to follow How to Write a Git Commit Message

@Cruikshanks
Copy link
Member

With these changes in place @Demwunz I'd be happy to approve. But double-check what I've done in case you have any questions or concerns.

@Demwunz
Copy link
Contributor Author

Demwunz commented Feb 5, 2024

Thanks for going through these @Cruikshanks

@Demwunz Demwunz merged commit 530cdd7 into main Feb 5, 2024
6 checks passed
@Demwunz Demwunz deleted the WATER-4265-returns-required-journey-select-return-reason-page-iteration-2-2-of-7 branch February 5, 2024 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants