Skip to content

LG-8908: Add USPS document check to ProofingComponent when enrollment is created#8113

Merged
tomas-nava merged 6 commits intomainfrom
tomas/lg-8908-document-check-refactor
Apr 4, 2023
Merged

LG-8908: Add USPS document check to ProofingComponent when enrollment is created#8113
tomas-nava merged 6 commits intomainfrom
tomas/lg-8908-document-check-refactor

Conversation

@tomas-nava
Copy link
Contributor

🎫 Ticket

Related to LG-8908, follow-up to #7854

🛠 Summary of changes

Add the USPS document check to the ProofingComponent when the enrollment is created in the Idv::InPerson::UspsLocationsController instead of when the verify page is submitted.

This value in the ProofingComponent serves for record keeping and sometimes to determine whether the user is in the process of enrolling in in-person proofing.

The immediate reason for moving it is so that the new Idv::InPerson::VerifyInfoController shows the correct steps in its step indicator.

📜 Testing Plan

  • set the in_person_verify_info_controller_enabled feature flag to true
  • create an account
  • navigate through the in-person proofing flow to the verify page
  • confirm that the page's URL is /verify/in_person/verify_info
  • confirm that the in-person proofing step indicator is displayed

changelog: Internal, refactor, in-person VerifyInfoController outside Flow State Machine
@tomas-nava tomas-nava requested review from a team, gina-yamada and soniaconnolly March 31, 2023 19:58
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

Tried this out locally, ran into issues. I did see the correct Step Indicator!

  1. Probably unrelated to this PR: I used a yaml file to get an image resolution error to get into In Person proofing. Then I entered a Puerto Rico address, and said my residential address is different from my ID. I was curious if I would see any Puerto Rico hints, which I didn't on the address entry page. I went to Update Address from VerifyInfo, and saw a New York address that isn't in the yaml file I used. Not sure where that came from! If I click on Back, I still see the Puerto Rico address I entered on the VerifyInfo page.
  2. Possibly related to whatever went wrong above, when I clicked Continue on the VerifyInfo page, I got an exception. undefined method `log_irs_tmx_fraud_check_event' for #Idv::InPerson::VerifyInfoController:0x0000000003ff48 on verify_info_concern.rb:224

@tomas-nava
Copy link
Contributor Author

  1. Probably unrelated to this PR: I used a yaml file to get an image resolution error to get into In Person proofing. Then I entered a Puerto Rico address, and said my residential address is different from my ID. I was curious if I would see any Puerto Rico hints, which I didn't on the address entry page. I went to Update Address from VerifyInfo, and saw a New York address that isn't in the yaml file I used. Not sure where that came from! If I click on Back, I still see the Puerto Rico address I entered on the VerifyInfo page.

This is fixed in 51db501 (although I couldn't reproduce the error where you saw a different address than the one you entered)

  1. Possibly related to whatever went wrong above, when I clicked Continue on the VerifyInfo page, I got an exception. undefined method `log_irs_tmx_fraud_check_event' for #Idv::InPerson::VerifyInfoController:0x0000000003ff48 on verify_info_concern.rb:224

This is fixed in c2e02b7

Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

LGTM, with a comment on possible code cleanup. I ran this and it worked as expected with the correct step indicator on the VerifyInfo step.

Also noting possible follow-on work to bring in threatmetrix specs from remote VerifyInfoController feature specs, or possibly create a separate spec/example file for the shared VerifyInfoConcern.

analytics.idv_proofing_resolution_result_missing
flash.now[:error] = I18n.t('idv.failure.timeout')
render 'idv/verify_info/show'
render 'idv/in_person/verify_info/show'
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to change this because I was using the shared view, but you might be able to use render :show here and on line 146, and then you might be able to use the shared process_async_state in VerifyInfoConcern if they haven't diverged anywhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohh, great catch, thank you. I moved the method into the concern in 7ddff6d

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good! Tried out the flow and everything works.

@tomas-nava tomas-nava merged commit 783fdfa into main Apr 4, 2023
@tomas-nava tomas-nava deleted the tomas/lg-8908-document-check-refactor branch April 4, 2023 17:14
jc-gsa pushed a commit that referenced this pull request Apr 19, 2023
… is created (#8113)

* add usps doc check to proofing component earlier

changelog: Internal, refactor, in-person VerifyInfoController outside Flow State Machine

* include Steps::ThreadMetricStepHelper (see #7924)

* replace IdvSession with IdvStepConcern (see #7840)

and remove redundant confirm_two_factor_authenticate (see #8082)

* give in-person verify info controller its own view

and fix links to update pages

* move process_async_state into the concern
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.

2 participants