Skip to content

LG-9099 IdvStepConcern updates#8082

Merged
soniaconnolly merged 4 commits intomainfrom
sonia-lg-9099-idv-step-concern-updates
Mar 28, 2023
Merged

LG-9099 IdvStepConcern updates#8082
soniaconnolly merged 4 commits intomainfrom
sonia-lg-9099-idv-step-concern-updates

Conversation

@soniaconnolly
Copy link
Contributor

@soniaconnolly soniaconnolly commented Mar 27, 2023

🎫 Ticket

LG-9099

🛠 Summary of changes

Refactoring PR, app behavior should be unchanged. The behavior change for this ticket will go into confirm_document_capture_step_complete to redirect back from the SSN controller to the DocumentCaptureController for desktop flow when the feature flag is enabled.

This PR might be easier to review by individual commits.

Move and rename two methods from StepUtilitiesConcern to IdvStepConcern, since it is the new place to put before action methods that control ordering of identity verification steps outside the FSM.

  • confirm_pii_from_doc to confirm_document_capture_complete
    • Separate implementation in AddressController also renamed. It should eventually call the one in IdvStepConcern.
  • confirm_profile_not_already_confirmed to confirm_verify_info_step_needed
    • Separate implementation in in_person/verify_info_controller also renamed. This one will stay separate because it redirects inside the in_person flow. Or the method would need to check whether the user is in the in_person flow.

I discussed these changes with @jmhooper before he left on vacation.

📜 Testing Plan

  • create account
  • go through identity verification, checking that you can't jump ahead to verify_info.

In a post-FSM world we want all the logic about redirects between steps to be in IdvStepConcern. Include this method in the pattern, and include IdvStepConcern in SsnController.

Also remove redundant calls to `confirm_two_factor_authenticated` before action since it is included by IdvStepConcern.

changelog: Internal, Identity Verification, Refactor code that moves between steps
…tep_needed

Note that in_person/verify_info_controller has its own implementation which was also renamed.
Note that AddressController has its own slightly different implementation of this method.
@soniaconnolly soniaconnolly requested review from a team and eric-gade March 27, 2023 21:30
Copy link
Contributor

@solipet solipet left a comment

Choose a reason for hiding this comment

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

LGTM!

include StepUtilitiesConcern
include Steps::ThreatMetrixStepHelper

before_action :confirm_two_factor_authenticated
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this method is defined in IdvSession, but auto-included as a before action in IdvStepConcern (which in turn includes IdvSession), should IdvSession be responsible for auto-including this before action?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is worth looking into as a separate cleanup. IdvSession is included in something like 20 controllers, so changing its included actions will take some research and care.

@soniaconnolly soniaconnolly merged commit 8ca4ec6 into main Mar 28, 2023
@soniaconnolly soniaconnolly deleted the sonia-lg-9099-idv-step-concern-updates branch March 28, 2023 18:25
@aduth aduth mentioned this pull request Mar 29, 2023
tomas-nava pushed a commit that referenced this pull request Apr 3, 2023
and remove redundant confirm_two_factor_authenticate (see #8082)
tomas-nava pushed a commit that referenced this pull request Apr 4, 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
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