From 04f88fcd83662d4a0ef82283d5dea62ea60732e0 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 31 May 2023 15:37:24 -0700 Subject: [PATCH 01/11] Use :js only where needed in hybrid_handoff feature specs --- spec/features/idv/doc_auth/hybrid_handoff_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/features/idv/doc_auth/hybrid_handoff_spec.rb b/spec/features/idv/doc_auth/hybrid_handoff_spec.rb index a15d437ba52..8d9ec50d1b9 100644 --- a/spec/features/idv/doc_auth/hybrid_handoff_spec.rb +++ b/spec/features/idv/doc_auth/hybrid_handoff_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -feature 'doc auth upload step' do +feature 'doc auth hybrid_handoff step' do include IdvStepHelper include DocAuthHelper include ActionView::Helpers::DateHelper @@ -19,14 +19,14 @@ allow(IdentityConfig.store).to receive(:doc_auth_hybrid_handoff_controller_enabled). and_return(new_controller_enabled) allow_any_instance_of(Idv::HybridHandoffController).to receive(:mobile_device?).and_return(true) - complete_doc_auth_steps_before_upload_step allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics) allow_any_instance_of(ApplicationController).to receive(:irs_attempts_api_tracker). and_return(fake_attempts_tracker) end - context 'on a desktop device', js: true do + context 'on a desktop device' do before do + complete_doc_auth_steps_before_upload_step allow_any_instance_of( Idv::HybridHandoffController, ).to receive( @@ -56,7 +56,7 @@ ) end - it "defaults phone to user's 2fa phone number" do + it "defaults phone to user's 2fa phone number", :js do field = page.find_field(t('two_factor_authentication.phone_label')) expect(field.value).to eq('(202) 555-1212') end @@ -75,7 +75,7 @@ ) end - it 'proceeds to the next page with valid info' do + it 'proceeds to the next page with valid info', :js do expect(fake_attempts_tracker).to receive( :idv_phone_upload_link_sent, ).with( @@ -96,7 +96,7 @@ expect(page).to have_current_path(idv_link_sent_path) end - it 'does not proceed to the next page with invalid info' do + it 'does not proceed to the next page with invalid info', :js do fill_in :doc_auth_phone, with: '' click_send_link From b97c56d6ed8082f269a4094b5b9e416eae9ee56b Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 31 May 2023 15:58:08 -0700 Subject: [PATCH 02/11] Add failing specs --- .../features/idv/doc_auth/hybrid_handoff_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/features/idv/doc_auth/hybrid_handoff_spec.rb b/spec/features/idv/doc_auth/hybrid_handoff_spec.rb index 8d9ec50d1b9..75fdb5b736b 100644 --- a/spec/features/idv/doc_auth/hybrid_handoff_spec.rb +++ b/spec/features/idv/doc_auth/hybrid_handoff_spec.rb @@ -24,6 +24,16 @@ and_return(fake_attempts_tracker) end + it 'does not skip ahead in standard desktop flow' do + visit(idv_hybrid_handoff_url) + expect(page).to have_current_path(idv_doc_auth_welcome_step) + complete_welcome_step + visit(idv_hybrid_handoff_url) + expect(page).to have_current_path(idv_doc_auth_agreement_step) + complete_agreement_step + expect(page).to have_current_path(idv_hybrid_handoff_step) + end + context 'on a desktop device' do before do complete_doc_auth_steps_before_upload_step @@ -54,6 +64,9 @@ 'IdV: doc auth upload submitted', hash_including(step: 'upload', destination: :document_capture), ) + + visit(idv_hybrid_handoff_url) + expect(page).to have_current_path(idv_document_capture_path) end it "defaults phone to user's 2fa phone number", :js do @@ -73,6 +86,9 @@ 'IdV: doc auth upload submitted', hash_including(step: 'upload', destination: :link_sent), ) + + visit(idv_hybrid_handoff_url) + expect(page).to have_current_path(idv_link_sent_path) end it 'proceeds to the next page with valid info', :js do From b2fb0253da4758cf39512575aeea2a8f4a7670d7 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 31 May 2023 15:59:05 -0700 Subject: [PATCH 03/11] Remove unused methods from hybrid_handoff_controller --- app/controllers/idv/hybrid_handoff_controller.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/controllers/idv/hybrid_handoff_controller.rb b/app/controllers/idv/hybrid_handoff_controller.rb index 09459ebd55e..d677111caea 100644 --- a/app/controllers/idv/hybrid_handoff_controller.rb +++ b/app/controllers/idv/hybrid_handoff_controller.rb @@ -162,14 +162,6 @@ def analytics_arguments }.merge(**acuant_sdk_ab_test_analytics_args) end - def mark_link_sent_step_complete - flow_session['Idv::Steps::LinkSentStep'] = true - end - - def mark_upload_step_complete - flow_session['Idv::Steps::UploadStep'] = true - end - def form_response(destination:) FormResponse.new( success: true, From 18919e0b78d62d02954896b52ce949a2122e5650 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 31 May 2023 16:04:35 -0700 Subject: [PATCH 04/11] Add before_action that checks if flow_path is set --- app/controllers/idv/hybrid_handoff_controller.rb | 11 +++++++++++ spec/features/idv/doc_auth/hybrid_handoff_spec.rb | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/controllers/idv/hybrid_handoff_controller.rb b/app/controllers/idv/hybrid_handoff_controller.rb index d677111caea..dd34dc8a8ce 100644 --- a/app/controllers/idv/hybrid_handoff_controller.rb +++ b/app/controllers/idv/hybrid_handoff_controller.rb @@ -9,6 +9,7 @@ class HybridHandoffController < ApplicationController before_action :confirm_two_factor_authenticated before_action :render_404_if_hybrid_handoff_controller_disabled before_action :confirm_agreement_step_complete + before_action :confirm_hybrid_handoff_needed, only: :show def show flow_session[:flow_path] = 'standard' @@ -212,6 +213,16 @@ def confirm_agreement_step_complete redirect_to idv_doc_auth_url end + def confirm_hybrid_handoff_needed + return if !flow_session[:flow_path] + + if flow_session[:flow_path] == 'standard' + redirect_to idv_document_capture_url + elsif flow_session[:flow_path] == 'hybrid' + redirect_to idv_link_sent_url + end + end + def formatted_destination_phone raw_phone = params.require(:doc_auth).permit(:phone) PhoneFormatter.format(raw_phone, country_code: 'US') diff --git a/spec/features/idv/doc_auth/hybrid_handoff_spec.rb b/spec/features/idv/doc_auth/hybrid_handoff_spec.rb index 75fdb5b736b..92583e99ed4 100644 --- a/spec/features/idv/doc_auth/hybrid_handoff_spec.rb +++ b/spec/features/idv/doc_auth/hybrid_handoff_spec.rb @@ -31,7 +31,7 @@ visit(idv_hybrid_handoff_url) expect(page).to have_current_path(idv_doc_auth_agreement_step) complete_agreement_step - expect(page).to have_current_path(idv_hybrid_handoff_step) + expect(page).to have_current_path(idv_hybrid_handoff_path) end context 'on a desktop device' do From 9621991824add363d6b89184dfc70faf13fdaccb Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 31 May 2023 16:09:31 -0700 Subject: [PATCH 05/11] Add controller specs for before_action --- .../idv/hybrid_handoff_controller_spec.rb | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/spec/controllers/idv/hybrid_handoff_controller_spec.rb b/spec/controllers/idv/hybrid_handoff_controller_spec.rb index 62fa39f37d2..618b9dcc84f 100644 --- a/spec/controllers/idv/hybrid_handoff_controller_spec.rb +++ b/spec/controllers/idv/hybrid_handoff_controller_spec.rb @@ -28,6 +28,13 @@ :confirm_agreement_step_complete, ) end + + it 'checks that hybrid_handoff is needed' do + expect(subject).to have_actions( + :before, + :confirm_hybrid_handoff_needed, + ) + end end describe '#show' do @@ -68,6 +75,24 @@ expect(response).to redirect_to(idv_doc_auth_url) end end + + context 'hybrid_handoff already visited' do + it 'redirects to document_capture in standard flow' do + subject.user_session['idv/doc_auth'][:flow_path] = 'standard' + + get :show + + expect(response).to redirect_to(idv_document_capture_url) + end + + it 'redirects to link_sent in hybrid flow' do + subject.user_session['idv/doc_auth'][:flow_path] = 'hybrid' + + get :show + + expect(response).to redirect_to(idv_link_sent_url) + end + end end describe '#update' do From 0376423bf478328d33fa9be3b22a949d90bab97f Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 31 May 2023 16:28:47 -0700 Subject: [PATCH 06/11] Set flow_path to nil when redirecting to hybrid_handoff --- app/controllers/idv/hybrid_handoff_controller.rb | 2 ++ app/controllers/idv/link_sent_controller.rb | 1 + app/services/idv/actions/cancel_link_sent_action.rb | 1 + app/services/idv/actions/redo_document_capture_action.rb | 1 + app/services/idv/steps/agreement_step.rb | 4 +--- 5 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/controllers/idv/hybrid_handoff_controller.rb b/app/controllers/idv/hybrid_handoff_controller.rb index dd34dc8a8ce..e241de6e2fc 100644 --- a/app/controllers/idv/hybrid_handoff_controller.rb +++ b/app/controllers/idv/hybrid_handoff_controller.rb @@ -65,6 +65,7 @@ def handle_phone_submission redirect_to idv_link_sent_url else redirect_to idv_hybrid_handoff_url + flow_session[:flow_path] = nil end analytics.idv_doc_auth_upload_submitted( @@ -193,6 +194,7 @@ def throttled_failure failure(message) redirect_to idv_hybrid_handoff_url + flow_session[:flow_path] = nil end # copied from Flow::Failure module diff --git a/app/controllers/idv/link_sent_controller.rb b/app/controllers/idv/link_sent_controller.rb index ab7f362fbd1..a225ce44cd1 100644 --- a/app/controllers/idv/link_sent_controller.rb +++ b/app/controllers/idv/link_sent_controller.rb @@ -80,6 +80,7 @@ def handle_document_verification_success(get_results_response) def render_document_capture_cancelled if IdentityConfig.store.doc_auth_hybrid_handoff_controller_enabled redirect_to idv_hybrid_handoff_url + flow_session[:flow_path] = nil else mark_upload_step_incomplete redirect_to idv_doc_auth_url # was idv_url, why? diff --git a/app/services/idv/actions/cancel_link_sent_action.rb b/app/services/idv/actions/cancel_link_sent_action.rb index 66dc4ba49ae..60393229121 100644 --- a/app/services/idv/actions/cancel_link_sent_action.rb +++ b/app/services/idv/actions/cancel_link_sent_action.rb @@ -8,6 +8,7 @@ def self.analytics_submitted_event def call if IdentityConfig.store.doc_auth_hybrid_handoff_controller_enabled redirect_to idv_hybrid_handoff_url + flow_session[:flow_path] = nil else mark_step_incomplete(:upload) end diff --git a/app/services/idv/actions/redo_document_capture_action.rb b/app/services/idv/actions/redo_document_capture_action.rb index 2054bc349ce..40ffbe4aabe 100644 --- a/app/services/idv/actions/redo_document_capture_action.rb +++ b/app/services/idv/actions/redo_document_capture_action.rb @@ -11,6 +11,7 @@ def call redirect_to idv_document_capture_url elsif IdentityConfig.store.doc_auth_hybrid_handoff_controller_enabled redirect_to idv_hybrid_handoff_url + flow_session[:flow_path] = nil else mark_step_incomplete(:upload) end diff --git a/app/services/idv/steps/agreement_step.rb b/app/services/idv/steps/agreement_step.rb index e5eb9076bfc..21942060dd9 100644 --- a/app/services/idv/steps/agreement_step.rb +++ b/app/services/idv/steps/agreement_step.rb @@ -16,6 +16,7 @@ def call if flow_session[:skip_upload_step] redirect_to idv_document_capture_url + flow_session[:flow_path] = 'standard' else redirect_to idv_hybrid_handoff_url end @@ -30,9 +31,6 @@ def form_submit def skip_to_capture # See: Idv::DocAuthController#update_if_skipping_upload flow_session[:skip_upload_step] = true - if IdentityConfig.store.doc_auth_hybrid_handoff_controller_enabled - flow_session[:flow_path] = 'standard' - end end def consent_form_params From fa1ecdd500e860b76ebf3040c445549e8b2c453d Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 31 May 2023 16:39:25 -0700 Subject: [PATCH 07/11] changelog [skip changelog] From 2c3eb50f0dfcfd4a11915105f8f706b54bd3f4bd Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 1 Jun 2023 08:25:59 -0700 Subject: [PATCH 08/11] Fix prev PR: Redirect to document_capture instead of capture_complete After trying to go to link_sent --- app/controllers/idv/hybrid_handoff_controller.rb | 10 +++------- spec/controllers/idv/hybrid_handoff_controller_spec.rb | 3 +-- spec/features/idv/hybrid_mobile/hybrid_mobile_spec.rb | 3 +-- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/app/controllers/idv/hybrid_handoff_controller.rb b/app/controllers/idv/hybrid_handoff_controller.rb index e241de6e2fc..572fd03cfa6 100644 --- a/app/controllers/idv/hybrid_handoff_controller.rb +++ b/app/controllers/idv/hybrid_handoff_controller.rb @@ -12,7 +12,6 @@ class HybridHandoffController < ApplicationController before_action :confirm_hybrid_handoff_needed, only: :show def show - flow_session[:flow_path] = 'standard' analytics.idv_doc_auth_upload_visited(**analytics_arguments) Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]).call( @@ -65,12 +64,11 @@ def handle_phone_submission redirect_to idv_link_sent_url else redirect_to idv_hybrid_handoff_url - flow_session[:flow_path] = nil end - analytics.idv_doc_auth_upload_submitted( - **analytics_arguments.merge(form_response(destination: :link_sent).to_h), - ) + analytics_args = analytics_arguments.merge(form_response(destination: :link_sent).to_h) + analytics_args[:flow_path] = flow_session[:flow_path] + analytics.idv_doc_auth_upload_submitted(**analytics_args) end def send_link @@ -160,7 +158,6 @@ def analytics_arguments step: 'upload', analytics_id: 'Doc Auth', irs_reproofing: irs_reproofing?, - flow_path: flow_session[:flow_path], }.merge(**acuant_sdk_ab_test_analytics_args) end @@ -194,7 +191,6 @@ def throttled_failure failure(message) redirect_to idv_hybrid_handoff_url - flow_session[:flow_path] = nil end # copied from Flow::Failure module diff --git a/spec/controllers/idv/hybrid_handoff_controller_spec.rb b/spec/controllers/idv/hybrid_handoff_controller_spec.rb index 618b9dcc84f..a1ae8933685 100644 --- a/spec/controllers/idv/hybrid_handoff_controller_spec.rb +++ b/spec/controllers/idv/hybrid_handoff_controller_spec.rb @@ -40,8 +40,7 @@ describe '#show' do let(:analytics_name) { 'IdV: doc auth upload visited' } let(:analytics_args) do - { flow_path: 'standard', - step: 'upload', + { step: 'upload', analytics_id: 'Doc Auth', irs_reproofing: false } end diff --git a/spec/features/idv/hybrid_mobile/hybrid_mobile_spec.rb b/spec/features/idv/hybrid_mobile/hybrid_mobile_spec.rb index 1318d633099..c7ad3927831 100644 --- a/spec/features/idv/hybrid_mobile/hybrid_mobile_spec.rb +++ b/spec/features/idv/hybrid_mobile/hybrid_mobile_spec.rb @@ -45,8 +45,7 @@ # Confirm that jumping to LinkSent page does not cause errors visit idv_link_sent_url expect(page).to have_current_path(root_url) - - visit idv_hybrid_mobile_capture_complete_url + visit idv_hybrid_mobile_document_capture_url attach_and_submit_images From e674bd7d94333f8e7c55d967372901058946a930 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 1 Jun 2023 09:05:01 -0700 Subject: [PATCH 09/11] Check flag in document_capture redirect --- app/controllers/idv/document_capture_controller.rb | 6 +++++- spec/controllers/idv/document_capture_controller_spec.rb | 5 +---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/controllers/idv/document_capture_controller.rb b/app/controllers/idv/document_capture_controller.rb index 9175f873079..044de86df5e 100644 --- a/app/controllers/idv/document_capture_controller.rb +++ b/app/controllers/idv/document_capture_controller.rb @@ -54,7 +54,11 @@ def extra_view_variables def confirm_upload_step_complete return if flow_session[:flow_path].present? - redirect_to idv_doc_auth_url + if IdentityConfig.store.doc_auth_hybrid_handoff_controller_enabled + redirect_to idv_hybrid_handoff_url + else + redirect_to idv_doc_auth_url + end end def confirm_document_capture_needed diff --git a/spec/controllers/idv/document_capture_controller_spec.rb b/spec/controllers/idv/document_capture_controller_spec.rb index 7224ba5d670..d156974a724 100644 --- a/spec/controllers/idv/document_capture_controller_spec.rb +++ b/spec/controllers/idv/document_capture_controller_spec.rb @@ -7,7 +7,7 @@ { 'document_capture_session_uuid' => 'fd14e181-6fb1-4cdc-92e0-ef66dad0df4e', :threatmetrix_session_id => 'c90ae7a5-6629-4e77-b97c-f1987c2df7d0', :flow_path => 'standard', - 'Idv::Steps::UploadStep' => true } + } end let(:user) { create(:user) } @@ -19,9 +19,6 @@ ) end - let(:default_sdk_version) { IdentityConfig.store.idv_acuant_sdk_version_default } - let(:alternate_sdk_version) { IdentityConfig.store.idv_acuant_sdk_version_alternate } - before do allow(subject).to receive(:flow_session).and_return(flow_session) stub_sign_in(user) From 328fd7f983bd32df8d33ce82d19f7f307adb140a Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 1 Jun 2023 11:28:33 -0700 Subject: [PATCH 10/11] lint --- spec/controllers/idv/document_capture_controller_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/controllers/idv/document_capture_controller_spec.rb b/spec/controllers/idv/document_capture_controller_spec.rb index d156974a724..da2f0cb1287 100644 --- a/spec/controllers/idv/document_capture_controller_spec.rb +++ b/spec/controllers/idv/document_capture_controller_spec.rb @@ -6,8 +6,7 @@ let(:flow_session) do { 'document_capture_session_uuid' => 'fd14e181-6fb1-4cdc-92e0-ef66dad0df4e', :threatmetrix_session_id => 'c90ae7a5-6629-4e77-b97c-f1987c2df7d0', - :flow_path => 'standard', - } + :flow_path => 'standard' } end let(:user) { create(:user) } From 914ada0126ccf60f85487afa75e4ca0f2a2c6550 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 1 Jun 2023 12:20:34 -0700 Subject: [PATCH 11/11] With feature flag on, redirect from UploadStep back to DocumentCapture When UploadStep or HybridHandoff have already been completed. This is for the 50/50 state. --- app/controllers/idv/hybrid_handoff_controller.rb | 7 ++++++- spec/features/idv/doc_auth/document_capture_spec.rb | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/controllers/idv/hybrid_handoff_controller.rb b/app/controllers/idv/hybrid_handoff_controller.rb index 572fd03cfa6..39594ce4c05 100644 --- a/app/controllers/idv/hybrid_handoff_controller.rb +++ b/app/controllers/idv/hybrid_handoff_controller.rb @@ -62,6 +62,9 @@ def handle_phone_submission if !failure_reason flow_session[:flow_path] = 'hybrid' redirect_to idv_link_sent_url + + # for the 50/50 state + flow_session['Idv::Steps::UploadStep'] = true else redirect_to idv_hybrid_handoff_url end @@ -118,11 +121,13 @@ def bypass_send_link_steps flow_session[:flow_path] = 'standard' redirect_to idv_document_capture_url + # for the 50/50 state + flow_session['Idv::Steps::UploadStep'] = true + response = form_response(destination: :document_capture) analytics.idv_doc_auth_upload_submitted( **analytics_arguments.merge(response.to_h), ) - response end def extra_view_variables diff --git a/spec/features/idv/doc_auth/document_capture_spec.rb b/spec/features/idv/doc_auth/document_capture_spec.rb index bfc16edd4fb..87c3d502eea 100644 --- a/spec/features/idv/doc_auth/document_capture_spec.rb +++ b/spec/features/idv/doc_auth/document_capture_spec.rb @@ -35,7 +35,7 @@ end it 'shows the new DocumentCapture page for desktop standard flow' do - expect(page).to have_current_path(idv_document_capture_url) + expect(page).to have_current_path(idv_document_capture_path) expect(page).to have_content(t('doc_auth.headings.document_capture').tr(' ', ' ')) expect(page).to have_content(t('step_indicator.flows.idv.verify_id')) @@ -48,6 +48,12 @@ irs_reproofing: false, acuant_sdk_upgrade_ab_test_bucket: :default, ) + + # it redirects here if trying to move earlier in the flow + visit(idv_doc_auth_agreement_step) + expect(page).to have_current_path(idv_document_capture_path) + visit(idv_doc_auth_upload_step) + expect(page).to have_current_path(idv_document_capture_path) end it 'logs return to sp link click' do