Return to branded page consistently when canceling#1511
Conversation
**Why**: Consistency is key. In this case, the cancel link on the forgot password page was always linking to the home page. **How**: Don't use the `shared/cancel` partial because the user will never be signing up or going through IdV if they are accessing the Forgot Password page, so we can directly link to the decorated session's `cancel_link_path` which contains the logic to determine whether it should point to the home page or the branded page.
|
I know it's not ideal but would it be possible to add another condition to the partial to catch this scenario so we can continue having all of the cancel links in one place? |
|
Or we could pass in a |
|
The problem is the partial is going through several conditionals that will always return false. I prefer not to go through conditionals if they're never needed. How about we create a separate shared partial for the scenarios where the user is not signed in and is not submitting a token? |
|
I think what needs to happen ideally is to remove the logic from the partial and instead use the pattern we use with the decorated session where we create different partials for different situations and have the decorator classes decide which partial to use. I'd be happy to work on that in a separate PR. This PR is a quick fix for the cancel link on the forgot password page, and to add the missing view spec. |
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM, seems reasonable to fix the bug now and refactor later
3 participants
Why: Consistency is key. In this case, the cancel link on the
forgot password page was always linking to the home page.
How: Don't use the
shared/cancelpartial because the user willnever be signing up or going through IdV if they are accessing the
Forgot Password page, so we can directly link to the decorated session's
cancel_link_pathwhich contains the logic to determine whether itshould point to the home page or the branded page.