Skip to content

Jmax/lg 9107 remove fsm document capture hybrid flow step#8292

Merged
jmax-gsa merged 23 commits intomainfrom
jmax/LG-9107-remove-fsm-document-capture-hybrid-flow-step
May 5, 2023
Merged

Jmax/lg 9107 remove fsm document capture hybrid flow step#8292
jmax-gsa merged 23 commits intomainfrom
jmax/LG-9107-remove-fsm-document-capture-hybrid-flow-step

Conversation

@jmax-gsa
Copy link
Contributor

@jmax-gsa jmax-gsa commented Apr 26, 2023

🎫 Ticket

LG-9107

🛠 Summary of changes

Removed hybrid flow document capture from FSM

Release Plan

Deploy Hybrid Mobile code #8243
Merge and deploy PR to remove 404 action
Test 50/50 state in int
Turn on feature flag doc_auth_hybrid_mobile_controllers_enabled in production
at this point, we're ready to merge and deploy this pr
Merge and deploy delete PR for now-unused FSM capture doc flow.

We put in a separate ticket LG-9611 for removing the async implementation, because there was so much of it.

changelog: internal,dismantle flow state machine,remove old hybrid flow
@jmax-gsa jmax-gsa marked this pull request as ready for review April 27, 2023 14:39
@jmax-gsa jmax-gsa requested review from aduth and jmhooper April 27, 2023 14:40
@soniaconnolly soniaconnolly requested a review from a team April 27, 2023 16:23
Copy link
Contributor

@jskinne3 jskinne3 left a comment

Choose a reason for hiding this comment

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

I don't know much about the FSM (RIP) but from the little I do, this all appears to make sense. I left one minor question.

status_endpoint: FeatureManagement.document_capture_async_uploads_enabled? ?
send(@step_url, step: :verify_document_status) :
nil,
status_endpoint: nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be removed entirely ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also generally confused if we're intending to maintain the async implementation, because it seems like it's partly lingering, but this would remove some parts of the implementation toward a less-complete state.

Copy link
Contributor

Choose a reason for hiding this comment

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

We put in a separate ticket LG-9611 for removing the async implementation, because there was so much of it.

end

def self.analytics_submitted_event
:idv_doc_auth_verify_document_status_submitted
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see AnalyticsEvents#idv_doc_auth_verify_document_status_submitted called anywhere else. Should it be? And if not, should the method be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed this, thanks.

private

def process_async_state(current_async_state)
form = ApiDocumentVerificationStatusForm.new(
Copy link
Contributor

Choose a reason for hiding this comment

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

This form implementation appears to be unused now. Should it be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Added this form name to LG-9611 to make sure we don't leave it behind.

status_endpoint: FeatureManagement.document_capture_async_uploads_enabled? ?
send(@step_url, step: :verify_document_status) :
nil,
status_endpoint: nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also generally confused if we're intending to maintain the async implementation, because it seems like it's partly lingering, but this would remove some parts of the implementation toward a less-complete state.

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jmax-gsa jmax-gsa merged commit 2d53589 into main May 5, 2023
@jmax-gsa jmax-gsa deleted the jmax/LG-9107-remove-fsm-document-capture-hybrid-flow-step branch May 5, 2023 18:34
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.

4 participants