Skip to content

Add #confirm_verify_info_step_complete to the IdV step concern#7941

Merged
jmhooper merged 4 commits intomainfrom
jmhooper-verify-info-complete
Mar 8, 2023
Merged

Add #confirm_verify_info_step_complete to the IdV step concern#7941
jmhooper merged 4 commits intomainfrom
jmhooper-verify-info-complete

Conversation

@jmhooper
Copy link
Contributor

@jmhooper jmhooper commented Mar 7, 2023

With the retirment of the Flow State Machine we will be using the IdV step concern to confirm that steps are completed throughout the proofing flow.

This commit adds a #confirm_verify_info_step_complete which can be used as a before action to confirm verify info is complete. If it is not complete it knows to redirect the user either to the remote verify info or in-person verify info depending on whether the user has a in-person enrollmemnt.

With the retirment of the Flow State Machine we will be using the IdV step concern to confirm that steps are completed throughout the proofing flow.

This commit adds a `#confirm_verify_info_step_complete` which can be used as a before action to confirm verify info is complete. If it is not complete it knows to redirect the user either to the remote verify info or in-person verify info depending on whether the user has a in-person enrollmemnt.

changelog: Improvements, FSM Retirement, A before action that validates that the verify info step was completed was added for use in IdV controllers that render steps that require the verify step to have been completed.
@jmhooper jmhooper marked this pull request as ready for review March 7, 2023 17:30
@jmhooper jmhooper requested a review from soniaconnolly March 7, 2023 17:31
service_provider: nil,
)
idv_session.profile_confirmation = true
idv_session.resolution_successful = 'phone'
Copy link
Contributor

Choose a reason for hiding this comment

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

silly question... does GPO flow not use this controller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, long story here. The GPO flow does use this controller. This value, however, is not related at all to the GPO flow or the phone flow for that matter. It is a value that the Flow State Machine set starting a while back that, as best as I can tell, was a copy/paste error.

We've kept using the phone value for continuity. I've been talking with @soniaconnolly about removing resolution_successful in favor of just using profile_confirmation. That may take some work to make sure things don't break in the 50/50 state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference, here is where this gets set when the profile is verified. This code was copied over from the verify step in the flow state machine. You can also see where the likely typo occurred.

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

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. Pulled down the code, was unable to break.

Did get one unexpected issue with SSN entry "cannot verify your details", which worked fine the next time. Don't think that's specific to this PR, since I didn't get it to happen again.

@jmhooper jmhooper merged commit 2469f2a into main Mar 8, 2023
@jmhooper jmhooper deleted the jmhooper-verify-info-complete branch March 8, 2023 14:45
@jmdembe jmdembe mentioned this pull request Mar 9, 2023
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