What do you want to do with this return? #1570
Conversation
https://eaflood.atlassian.net/browse/WATER-4836 Build a page to enable the user to choose how they would like to edit a return.
After discussion it has been decided that all options will take you down the same route to enter the receipt date. What happens after that will be determined by which radio button was selected.
Jozzey
changed the title
There was a problem hiding this comment.
The main reason I've asked for changes is it is not quite following the pattern we've used in other pages or presenter tests.
I get that it might seem pointless. Why, for example, go to the effort of adding a validator and associated unit test for something so 'simple'?
The reason is the pattern/convention is intended to save us the effort involved in deciding where the line for 'simple' is. Or coming back to something 12 months later and trying to understand why 'x' does it this way, but 'y' doesn't.
For most pages in this journey, there are existing matching or similar pages we can copy and use. They're there, so each time we have to add a journey (IMHO the bulk of a typical digital service), they get easier and quicker because our library of existing pages keeps growing.
Hope this makes sense, and happy to discuss further if needed.
| app: { | ||
| plainOutput: true | ||
| }, |
There was a problem hiding this comment.
This is not an API only endpoint so should not have this config.
| app: { | |
| plainOutput: true | |
| }, |
| app: { | ||
| plainOutput: true | ||
| }, |
There was a problem hiding this comment.
| app: { | |
| plainOutput: true | |
| }, |
| function _validate(whatToDo) { | ||
| if (!whatToDo) { | ||
| return { text: 'Select what you want to do with this return' } | ||
| } | ||
|
|
||
| return null | ||
| } |
There was a problem hiding this comment.
I appreciate this is all that is needed to validate the entry. But it breaks the pattern we have used for all our other pages.
We need a proper Joi-based validator for this, of which there are plenty you can crib from.
| }) | ||
| }) | ||
|
|
||
| describe('when the status is not yet "overdue"', () => { |
There was a problem hiding this comment.
I know 'overdue' is covered by the test above. However, our unit tests can also serve as a form of documentation. So, usually, in the presenter tests, if a property is based on some logic, we try to cover all expected behaviours in the tests, even if the main "correctly presents the data" already covers it.
So, I would add a final scenario to this group to document that the status will be' overdue' when the due date is less than `today()'.
This applies to all the properties. Just noticed they are only testing behaviour not covered by "correctly presents the data".
https://eaflood.atlassian.net/browse/WATER-4836
Build a page to enable the user to choose what they want to do with the return.