Skip to content

LG-5258: Allow users to start over IAL2 verification after clicking Cancel#6542

Merged
aduth merged 10 commits intomainfrom
aduth-lg-5258-cancel
Jul 6, 2022
Merged

LG-5258: Allow users to start over IAL2 verification after clicking Cancel#6542
aduth merged 10 commits intomainfrom
aduth-lg-5258-cancel

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jun 30, 2022

Why: As an end user, I expect that when I click Cancel, I will see an option to start over the identity verification process (IAL2), so that I can try to verify my identity again if I do not want to abandon the process completely.

(Review note: I may plan to split out some of this work to reduce the size of the pull request)

Screenshots:

Screen Before After
Step before_step after_step
Confirmation (without SP) before_confirm after_confirm
Confirmation (with SP) before_confirm_sp after_confirm_sp
Confirmation (hybrid) before_confirm_hybrid after_confirm_hybrid
Document capture step (hybrid) before_docauth_hybrid after_docauth_hybrid
After confirmation before_post_cancel N/A
After confirmation (hybrid) before_post_cancel_hybrid after_post_cancel_hybrid

@anniehirshman-gsa
Copy link
Contributor

Screenshots LGTM at first glance!

@Kamal-Munshi I'll be OOO - would you be able to take a closer look compared to the Figma mockups, and test this out in a sandbox? Also CC @kellular if you are interested in taking a look at Andrew's implementation of the new Cancel/Start Over functionality in the remote proofing flow. Thanks y'all!

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I was worried about this PR based on the number of files changed, but this looks like a straightforward rename

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😭 I forgot we had this code that parsed full HTML pages for things we didn't have an API for

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'd actually meant to leave a note here about this as well, since I think now after removing no-JS support for identity verification, it would be a good time to revisit these "wait" steps and have the back-end endpoint return a JSON response structured in the same way I've implemented here for this experience.

Comment on lines 55 to 57
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ love to see more bullet lists as arrays in i18n

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ love to see more bullet lists as arrays in i18n

I was also (pleasantly) surprised to find out that _html suffixing with an array of locale strings just works, which is nice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT of a name like click_spinner_button_and_wait?

Copy link
Contributor Author

@aduth aduth Jul 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT of a name like click_spinner_button_and_wait?

I like that it's a bit more explicit, but not so much that it's quite verbose.

I suppose it's not entirely clear what's involved in clicking a spinner button by the name alone, so I can update this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially I wanted this to just be a plain redirect, but since this is submitted as a form, the redirect would be blocked due to CSP. Another option could be to have this be a GET request, but I think it makes sense the way it's implemented now, since it is technically a destructive action (cancelling = "destroying" the session).

aduth and others added 6 commits July 5, 2022 08:53
…ancel

**Why**: As an end user, I expect that when I click Cancel, I will see an option to start over the identity verification process (IAL2), so that I can try to verify my identity again if I do not want to abandon the process completely.

changelog: Improvements, Identity Verification, Revise cancellation screen to allow user to start over
No longer managed by the React applications, since the choice to start over is moved into the cancellation experience.
See: #6542 (comment)
Co-Authored-By: Zach Margolis <zbmargolis@gmail.com>
If it turns out to be necessary to pull into viewport, might be good to have in base behavior anyways

Also unsure if we still have multiple instances of "Continue" on the same screen in IdV

Alternatively, update click_spinner_button_and_wait to handle argument as either a locator or as a node
@aduth aduth force-pushed the aduth-lg-5258-cancel branch from 013add5 to 9164861 Compare July 5, 2022 12:55
aduth added 4 commits July 5, 2022 10:48
Consolidate cancellation assertions to features/idv/cancel_spec.rb
Brakeman reports params used in render path. While technically true that it's referenced, it's only used in forwarding "step" parameter for URL creation, used for analytics logging

Next best workaround would be to filter param with an allowlist of steps. This is simple enough, but difficult to maintain, as it would require a developer to remember to add the step to the allowlist. Without automated enforcement checks, this would be far too easy to forget.
@aduth
Copy link
Contributor Author

aduth commented Jul 6, 2022

@aduth aduth merged commit b13f593 into main Jul 6, 2022
@aduth aduth deleted the aduth-lg-5258-cancel branch July 6, 2022 13:03
@jmdembe jmdembe mentioned this pull request Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants