Show offence ID on check your answers and full referral pages #665
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
libuk
force-pushed
the
change/1595-show-index-offence-id-in-referral
branch
from
e7ac526 to
428fed9
Compare
HPTs currently have no way of knowing which index offence a referral belongs to and are having to ask the CPP, therefore, we want to show the offence ID on the referral page under the 'sentence information' section. To achieve this we are adding the offence ID to the response of the first page of the 'sentence information' task. As there is no way to change the offence ID without starting a new referral, we hide the "change" action on the check your answers page. This isn't an ideal solution as it doesn't follow the conventions of the other form pages, but here is some rationale: * Offence ID is fetched by the server without the need for user input. We only ask the user to choose an offence ID if more than one offence is retrieved from the API. This means making a TaskListPage for offence ID non-trivial and would require a different approach to what we have now. Given there are limited dev days left on CAS3, taking on this work isn't feasible. * I thought about adding a new type of TaskListPage which wouldn't be visible to the user, but changing the existing domain model for this one instance felt a bit premature, and would still be a good chunk of work. * The solution in this commit is a hack, but is fairly non-invasive and easy to change in the future.
libuk
force-pushed
the
change/1595-show-index-offence-id-in-referral
branch
from
428fed9 to
15e8c13
Compare
|
@libuk sorry I thought this was for a rollback. You might want some CAS3 eyes on this! |
|
Thanks @edavey. @richpjames approved it initially, but it failed on CI due to an e2e test. This PR just adds a fix to stop it failing. Given it was approved and merged before, I'm going to go ahead and merge, but I've updated the PR description for clarity. |
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.
Context
HPTs currently have no way of knowing which index offence a referral belongs to and are having to ask the CPP, therefore, we want to show the offence ID on the referral page under the 'sentence information' section.
To achieve this we are adding the offence ID to the response of the first page of the 'sentence information' task. As there is no way to change the offence ID without starting a new referral, we hide the "change" action on the check your answers page.
This isn't an ideal solution as it doesn't follow the conventions of the other form pages, but here is some rationale:
Offence ID is fetched by the server without the need for user input. We only ask the user to choose an offence ID if more than one offence is retrieved from the API. This means making a TaskListPage for offence ID non-trivial and would require a different approach to what we have now. Given there are limited dev days left on CAS3, taking on this work isn't feasible.
I thought about adding a new type of TaskListPage which wouldn't be visible to the user, but changing the existing domain model for this one instance felt a bit premature, and would still be a good chunk of work.
The solution in this commit is a hack but is fairly non-invasive and easy to change in the future.
Let me know if this is something you've encountered in CAS1/CAS2. Alternative solutions are welcome!
Changes in this PR
Screenshots of UI changes
Before
After
Release checklist
Release process documentation.
Pre-merge checklist
(eg. CAS API)
Post-merge checklist
release to test
release to preprod
release to prod