Conversation
| status == DocumentCaptureSessionAsyncResult::DONE | ||
| end | ||
|
|
||
| alias_method :success?, :done? |
There was a problem hiding this comment.
reason this is needed: the capture doc flow expects to load a DocumentCaptureSessionResult which is basically the same as this one, except that it responds to success?
| result = document_capture_session.load_result || | ||
| document_capture_session.load_doc_auth_async_result |
There was a problem hiding this comment.
reason this is needed: the VerifyDocumentAction stores data via store_doc_auth_result which still contains PII but has slightly different redis keys than the store_result_from_response that the old flow uses
| verify_document_capture_session = if hybrid_flow_mobile? | ||
| document_capture_session | ||
| else | ||
| create_document_capture_session( | ||
| verify_document_capture_session_uuid_key, | ||
| ) | ||
| end |
There was a problem hiding this comment.
reason this is needed: in the hybrid mobile flow, the document_capture_session is the UUID that the desktop sent over, so that's where the desktop is polling for results. In the non-hybrid flow, we can generate a new key like this does
There was a problem hiding this comment.
the key is only used once either way, so we don't need to worry about overwriting?
There was a problem hiding this comment.
I believe, yes
| verify_document: Idv::Actions::VerifyDocumentAction, | ||
| verify_document_status: Idv::Actions::VerifyDocumentStatusAction, |
There was a problem hiding this comment.
reason this is needed: these polling actions for async work were missing from the hybrid flow
| @document_capture_session_result ||= ( | ||
| document_capture_session&.load_result || | ||
| document_capture_session&.load_doc_auth_async_result | ||
| ) |
There was a problem hiding this comment.
allow loading data from the async flow
| document_capture_session_uuid: flow_session[:document_capture_session_uuid], | ||
| endpoint: FeatureManagement.document_capture_async_uploads_enabled? ? | ||
| idv_doc_auth_step_path(step: :verify_document) : | ||
| send(@step_url, step: :verify_document) : |
There was a problem hiding this comment.
reason this is needed:
In the non-hybrid flow, we link to /idv/doc_auth etc endpoints. However, in the hybrid flow, the mobile path hits /idv/capture_doc/ versions of the corresponding endpoints, so this needed to be updated to be dynamic based on the flow
| allow(FeatureManagement).to receive(:document_capture_async_uploads_enabled?). | ||
| and_return(async_uploads_enabled) | ||
|
|
||
| assign(:step_url, :idv_doc_auth_step_url) |
There was a problem hiding this comment.
this is the key change in this file, now that the partial uses @step_url. The rest of the changes in the file were to simplify and minimize stubbing
|
Great work @zachmargolis !!! |
4 participants
Opening this as a draft so I can collect some ideas/feedbackThis is good to go now, I'm going to chase down the various specs that are failing