Skip to content

Refactor the IdV step concern to no longer check for an applicant#7840

Merged
jmhooper merged 4 commits intomainfrom
jmhooper-refactor-idv-step-concern
Feb 15, 2023
Merged

Refactor the IdV step concern to no longer check for an applicant#7840
jmhooper merged 4 commits intomainfrom
jmhooper-refactor-idv-step-concern

Conversation

@jmhooper
Copy link
Contributor

Prior to the introduction of the flow state machine, the "applicant" value in the IdV session was created when the proofing process was started. After the introduction of the FSM the applicant was created once the FSM flow was complete.

The IdvStepConcern is intended to route users to the correct step based on the state of proofing. Currently it is empty because the Flow State Machine primarily handles this logic. There is a desire to re-introduce the logic to route users to the correct step to this concern.

The IdvStepConcern has a before action to verify that proofing has started. It does that by checking for the applicant which, as described above, does not make sense in the post-FSM world. This commit removes that before action and adds it explicitly to controllers that require an applicant to function.

Prior to the introduction of the flow state machine, the "applicant" value in the IdV session was created when the proofing process was started. After the introduction of the FSM the applicant was created once the FSM flow was complete.

The IdvStepConcern is intended to route users to the correct step based on the state of proofing. Currently it is empty because the Flow State Machine primarily handles this logic. There is a desire to re-introduce the logic to route users to the correct step to this concern.

The IdvStepConcern has a before action to verify that proofing has started. It does that by checking for the applicant which, as described above, does not make sense in the post-FSM world. This commit removes that before action and adds it explicitly to controllers that require an applicant to function.

changelog: Improvements, Rails controller management, A before action that was used to identify if a user has started proofing was removed from a concern and explicitly called in individual controllers that depended on that before action. The logic in that before action was superceded by the flow state machine.
end
end

describe '#confirm_idv_session_started' do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests don't make sense in the post-FSM world as described in the pull request description. I added a test to the phone controller spec to test the equivalent logic. The review controller already had a matching test.

@jmhooper jmhooper marked this pull request as ready for review February 15, 2023 19:56
@jmhooper jmhooper requested a review from a team February 15, 2023 20:24
Copy link
Contributor

@theabrad theabrad 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! I pulled this down and tried jumping to the phone and review controllers from the doc_auth flow and it stayed on the verify_info page as desired.


def confirm_idv_session_started
def confirm_idv_applicant_created
redirect_to idv_verify_info_url if idv_session.applicant.blank?
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we leaving the issue of the hard-coded doc_auth verify redirect for a separate PR? I checked the flow, and it redirects to verify/doc_auth/document_capture. https://cm-jira.usa.gov/browse/LG-8983

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 would like to leave that to a separate PR

@jmhooper jmhooper merged commit 3f6f23d into main Feb 15, 2023
@jmhooper jmhooper deleted the jmhooper-refactor-idv-step-concern branch February 15, 2023 21:27
@solipet solipet mentioned this pull request Feb 21, 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.

3 participants