Skip to content

LG-6927: Omit in-person troubleshooting option when already in flow#6687

Merged
aduth merged 6 commits intomainfrom
aduth-lg-6927-rm-ipp-ln-outage
Aug 5, 2022
Merged

LG-6927: Omit in-person troubleshooting option when already in flow#6687
aduth merged 6 commits intomainfrom
aduth-lg-6927-rm-ipp-ln-outage

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Aug 3, 2022

Why: So that the user doesn't get stuck in a loop of attempting to troubleshoot by verifying in-person.

Testing Instructions:

  1. Navigate to http://localhost:3000
  2. Sign in or create an account
  3. Navigate to http://localhost:3000/verify
  4. Complete proofing flow, optionally choosing to proof in-person
  5. At SSN step, use value "000-00-0000"
  6. Continue to "Verify your information" step
  7. Click "Continue"
  8. Observe that the in-person proofing troubleshooting option is only visible if you are not already in the in-person proofing flow

Screenshots:

Before After
Screen Shot 2022-08-03 at 9 36 11 AM Screen Shot 2022-08-03 at 9 36 00 AM

aduth added 2 commits August 3, 2022 10:52
**Why**: So that the user doesn't get stuck in a loop of attempting to troubleshoot by verifying in-person.

changelog: Upcoming Features, In-person proofing, Hide in-person troubleshooting option while already in flow
@aduth aduth requested review from a team and sheldon-b August 3, 2022 14:58
No longer using request path
@anniehirshman-gsa
Copy link
Contributor

Screenshots look good 👍 Is this also an issue with the remote proofing flow, and if so, does this or a different PR cover that?

@aduth
Copy link
Contributor Author

aduth commented Aug 3, 2022

Screenshots look good 👍 Is this also an issue with the remote proofing flow, and if so, does this or a different PR cover that?

The way the ticket was phrased, my understanding is that we'd want it to continue to show as an option for the verify step in the remote unsupervised flow, and to only hide it in the in-person flow so that the user doesn't get stuck in a loop.

If we want to hide it from all of these exception / "outage" cases, regardless of flow, I can make that revision (it would simplify things, actually).

@aduth
Copy link
Contributor Author

aduth commented Aug 4, 2022

Screenshots look good 👍 Is this also an issue with the remote proofing flow, and if so, does this or a different PR cover that?

The way the ticket was phrased, my understanding is that we'd want it to continue to show as an option for the verify step in the remote unsupervised flow, and to only hide it in the in-person flow so that the user doesn't get stuck in a loop.

If we want to hide it from all of these exception / "outage" cases, regardless of flow, I can make that revision (it would simplify things, actually).

Actually, thinking on this more, I can see that we wouldn't want the option to ever show on this screen, since at least with the way it's set up currently, the location selection can only occur during document capture, and generally it seems like we want to limit entry to coming from document capture.

Aside: I think there's another option implemented in the FSMv2 generic error page that we'd also probably want to remove?

@anniehirshman-gsa
Copy link
Contributor

the way it's set up currently, the location selection can only occur during document capture, and generally it seems like we want to limit entry to coming from document capture.

That's right... the IPP flow is only set up as a path for failing document capture/"Verify your ID". So we wouldn't want the option to appear in any scenario after the user fails "Verify your personal details". Would be great if we could clean that up in this or a follow-up PR, thanks!

@aduth
Copy link
Contributor Author

aduth commented Aug 4, 2022

That's right... the IPP flow is only set up as a path for failing document capture/"Verify your ID". So we wouldn't want the option to appear in any scenario after the user fails "Verify your personal details". Would be great if we could clean that up in this or a follow-up PR, thanks!

👍 Makes sense.

I pushed 76dd34d, which removes the troubleshooting option from everywhere other than the document capture warning page.

This includes:

  • Verify step connectivity error page ("exception"), the original screen in this pull request (but now applying to both in-person and remote unsupervised flows)
  • Document capture throttled error page
    • This one is a bit more debatable, but it lives outside the React application, so this wouldn't work currently, since the prep steps must happen in the React app (location selection, intro instructions). We may want to circle back on this? I expect we might be able to find a way to incorporate the throttle screen into the React app.
  • FSMv2 React crash screen (see LG-6577: Show error page when FSMv2 React application crashes #6509 for context)

* URL to in-person proofing alternative flow, if enabled.
*/
inPersonURL: string | null;
inPersonURL?: string | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this do?

Suggested change
inPersonURL?: string | null;
inPersonURL?: string;

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on usages, I expect not. inPersonURL?: string really means that the key should be absent - i.e. not null nor even set to undefined though that is often still safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll check to see if we're setting it null anywhere currently, and if not, will try to simplify to inPersonURL?: string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed a couple updates, but simpler overall. Updated in b4493c9.

`null` and `undefined` are both used as "empty" values, prefer one. Also simplifies defaulting behavior in document-capture, since it would be assumed to be undefined when absent, previously coerced to null.
@aduth aduth merged commit 1f1b174 into main Aug 5, 2022
@aduth aduth deleted the aduth-lg-6927-rm-ipp-ln-outage branch August 5, 2022 20:09
aduth added a commit that referenced this pull request Aug 8, 2022
aduth added a commit that referenced this pull request Aug 9, 2022
* Verify establishing enrollment for in-person flow

**Why**: Since a user will need to select a location as part of establishing an enrollment before proceeding with the in-person flow.

changelog: Upcoming Features, In-person proofing, Verify establishing enrollment for in-person flow

* Invalidate applicant when assigning PII

* Refactor verify step PII methods as abstract

Avoid ambiguity between the two implementations

* Avoid reinitializing IdV session when blank

Seems to cause issues with replacing an existing session values, leading to session being empty at points-in-time where it's not expected to be, purely by referencing idv_session (e.g. in DocAuthController#redirect_if_session_applicant)

* Update specs after rebase for #6687

* Rename before_action to describe intent, not implementation

See: #6703 (comment)
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.

4 participants