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 optional alias to return requirement purpose #1177

Merged
merged 11 commits into from
Jul 10, 2024

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Jul 9, 2024

https://eaflood.atlassian.net/browse/WATER-4573

Part of the return requirements set up work

When setting up new return requirements, you are required to select one or more purposes. By default, we use the purpose description when referring to the return requirement purpose. But because the returns are intended to reflect what is on the licence document, sometimes there is a discrepancy between how the system describes the purpose and how it is recorded in the document.

To deal with this, NALD has the concept of a 'purpose alias'. This is an optional alternate description that should be used when referring to the purpose of a return requirement.

This change updates the 'Select the purpose' page in the return requirements set up journey to add a conditional reveal to the GOV.UK checkbox. This will reveal a textbox that users can use to enter a description, though it is not required.

We will add some validation, but only to limit the description to 100 characters, which is the same as site description.


As part of this, we were able to do some refactoring. The key change was to store the description from the purpose in the session to avoid having to fetch it again when we get to the /check page. That simplified what we were doing in that area to the extent that we could drop a service completely.

But adding the optional alias with a text limit meant the validation became more complex. Plus, we now have to deal with the scenario on the purpose page where the submission is invalid, but we have to replay the user's choices. Previously, as long as they selected something, they were good to go!

https://eaflood.atlassian.net/browse/WATER-4573

> Part of the return requirements set up work

When setting up new return requirements, you are required to select one or more purposes. By default, we use the purpose description when referring to the return requirement purpose. But because the returns are intended to reflect what is on the licence document, sometimes there is a discrepancy between how the system describes the purpose and how it was recorded in the document.

To deal with this, NALD has the concept of a 'purpose alias'. This is an optional alternate description that should be used when referring to the purpose on a return requirement.

This change updates the 'Select the purpose' page in the return requirements set up journey to add a [conditional reveal to the GOV.UK checkbox](https://design-system.service.gov.uk/components/checkboxes/). This will reveal a textbox that users can use to enter a description, though it is not required.

We will add some validation, but only to limit the description to 100 characters, which is the same as site description.
@Cruikshanks Cruikshanks added the enhancement New feature or request label Jul 9, 2024
@Cruikshanks Cruikshanks self-assigned this Jul 9, 2024
We also include description as we aim to simplify the check page by retaining the description now thus saving having to fetch it again in the check page.
If we combine the licence purpose information with that in the return requirement currently being setup we can simplify the view and test that it will get the data it expects.
With adding in this extra element the view has become more complex. Hence the push to move some of the logic that was there into the presenter.
This is the core of 2 changes. Needing to combine the aliases and the selected purposes from the request. But also to start persisting the description to save us having to fetch it again in the check page.

We also need to handle the edge case of a user entering a description that is too long. Before the only validation issue was that they hadn't selected anything so there was nothing to play back.

Now with the alias they may have selected some purposes but also caused a validation error. We need to ensure what they submitted is played back to them.
With us no longer needing to fetch purposes and map them to the selected purposes in the requirements the little work that is left can be moved to the `CheckService`.
@Cruikshanks Cruikshanks force-pushed the add-optional-alias-to-purpose branch from e16e787 to 9356c1b Compare July 10, 2024 12:01
@Cruikshanks Cruikshanks marked this pull request as ready for review July 10, 2024 12:45
@Cruikshanks Cruikshanks merged commit 0acfe21 into main Jul 10, 2024
6 checks passed
@Cruikshanks Cruikshanks deleted the add-optional-alias-to-purpose branch July 10, 2024 12:52
Cruikshanks added a commit that referenced this pull request Jul 10, 2024
https://eaflood.atlassian.net/browse/WATER-4573

> Part of the return requirements set up work

In [Add optional alias to return requirement purpose](#1177) we added the ability to assign a purpose description (alias) to the purposes selected when setting up a new return requirement.

As part of that we made a change to how we store the purposes in the session. No longer do we just store the ID, we now store an object.

```javascript
{
  purposes: [{
    alias: 'spray all over the place',
    description: 'Spray Irrigation - Direct',
    id: '42d5f68d-0b7a-4d29-b04e-943362746f59'
  }]
}
```

The 'Select a purpose' now expects the purposes inside the session to be in this format, and will persist them in this manner when you submit your selection. But the 'Start with abstraction data' and 'Copy existing requirement' journeys are still persisting the purposes in the session using the old `{ purposes: ['42d5f68d-0b7a-4d29-b04e-943362746f59'] }` format.

This change updates them and gets all 3 journeys working again.
Cruikshanks added a commit that referenced this pull request Jul 10, 2024
https://eaflood.atlassian.net/browse/WATER-4573

> Part of the return requirements set up work

In [Add optional alias to return requirement purpose](#1177) we added the ability to assign a purpose description (alias) to the purposes selected when setting up a new return requirement.

As part of that, we made a change to how we store the purposes in the session. No longer do we just store the ID; we now store an object.

```javascript
{
  purposes: [{
    alias: 'spray all over the place',
    description: 'Spray Irrigation - Direct',
    id: '42d5f68d-0b7a-4d29-b04e-943362746f59'
  }]
}
```

The 'Select a purpose' now expects the purposes inside the session to be in this format and will persist in this manner when you submit your selection. But the 'Start with abstraction data' and 'Copy existing requirement' journeys are still persisting the purposes in the session using the old `{ purposes: ['42d5f68d-0b7a-4d29-b04e-943362746f59'] }` format.

This change updates them and gets all 3 journeys working again.
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.

1 participant