Skip to content

Comments

Refactor verify info update#8595

Merged
soniaconnolly merged 5 commits intomainfrom
sonia-refactor-verify-info-update
Jun 14, 2023
Merged

Refactor verify info update#8595
soniaconnolly merged 5 commits intomainfrom
sonia-refactor-verify-info-update

Conversation

@soniaconnolly
Copy link
Contributor

For the deployment of InPerson::VerifyInfoController, in PR #8585 the InPerson::VerifyStep was marked complete at the end of VerifyInfoConcern#update. This method is shared with doc_auth VerifyInfoController. To eliminate the possibility of unintended effects, and to clarify #update for both controllers, give them each their own update method and rename the method in VerifyInfoConcern to shared_update.

In #8576 we recently added code to the shared update method that prevents redoing document capture after VerifyInfo is submitted. This does not apply to InPerson::VerifyInfoController, so this code was moved to the doc_auth VerifyInfoController#update.

📜 Testing Plan

  • Create account, start IdV
  • Continue past submitting VerifyInfo
  • Cancel and Start Over
  • In DocumentCapture submit yaml file with an error
  • Choose In Person verification
  • Continue past submitting VerifyInfo
  • Expect unchanged functionality, no errors

soniaconnolly and others added 4 commits June 13, 2023 14:35
Keep shared code in VerifyInfoConcern#shared_update.

[skip changelog]

Co-authored-by: Gina Yamada <gina.yamada@gsa.gov>
The in_person flow does not use redo_document_capture
@soniaconnolly soniaconnolly requested review from a team and gina-yamada June 13, 2023 21:57
@gina-yamada
Copy link
Contributor

✅ Run through testing plan locally
Create account, start IdV
Continue past submitting VerifyInfo
Cancel and Start Over
In DocumentCapture submit yaml file with an error
Choose In Person verification
Continue past submitting VerifyInfo
Expect unchanged functionality, no errors
✅ confirmed initial url /verify updated to /verify/doc_auth/welcome (same behavior on dev)
✅ made it to barcode page for in person flow 2048-7021-9880-4353
✅ cancel link in the in person flow take me back to get started verifying your identity, what I'd expect
✅ confirmed I was able to verify my identify without going through the in person flow (doc upload was success)

@gina-yamada gina-yamada requested a review from sheldon-b June 14, 2023 15:17

if success
# Don't allow the user to go back to document capture after verifying
if flow_session['redo_document_capture']
Copy link
Contributor

Choose a reason for hiding this comment

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

@soniaconnolly I'm pretty sure this isn't an issue for IPP, but how did your team encounter this behavior? I just want to make sure we don't also need this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See LG-9996 and slack thread. VerifyInfo shows the user the option to redo document capture if they had a barcode read error. The code that lets the user redo is in the in_person verify_info show template, but I think it can be removed, since people can't get to the in person flow unless their ID couldn't be scanned.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I agree that that code can be deleted from the in_person verify_info show template. The @had_barcode_read_failure is not set in our controller. I can open a separate ticket for that work.

Thanks for flagging this!

Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

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

LGTM!


if success
# Don't allow the user to go back to document capture after verifying
if flow_session['redo_document_capture']
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I agree that that code can be deleted from the in_person verify_info show template. The @had_barcode_read_failure is not set in our controller. I can open a separate ticket for that work.

Thanks for flagging this!

@soniaconnolly soniaconnolly merged commit d699db6 into main Jun 14, 2023
@soniaconnolly soniaconnolly deleted the sonia-refactor-verify-info-update branch June 14, 2023 18:38
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