From 660afdf216977b855c8506bfebb6d71293be29b5 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Thu, 28 Sep 2023 14:18:42 -0700 Subject: [PATCH 01/26] Allow going back to the welcome step changelog: User-Facing Improvements, Identity verification, Enable browser back button for early idv steps --- app/controllers/idv/welcome_controller.rb | 7 ------- spec/features/idv/end_to_end_idv_spec.rb | 13 +++++++++++++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index 1b515334c08..f1c2402e451 100644 --- a/app/controllers/idv/welcome_controller.rb +++ b/app/controllers/idv/welcome_controller.rb @@ -5,7 +5,6 @@ class WelcomeController < ApplicationController include GettingStartedAbTestConcern before_action :confirm_not_rate_limited - before_action :confirm_welcome_needed before_action :maybe_redirect_for_getting_started_ab_test def show @@ -56,11 +55,5 @@ def cancel_previous_in_person_enrollments UspsInPersonProofing::EnrollmentHelper. cancel_stale_establishing_enrollments_for_user(current_user) end - - def confirm_welcome_needed - return unless idv_session.welcome_visited - - redirect_to idv_agreement_url - end end end diff --git a/spec/features/idv/end_to_end_idv_spec.rb b/spec/features/idv/end_to_end_idv_spec.rb index 8d0e77909d0..387e0899055 100644 --- a/spec/features/idv/end_to_end_idv_spec.rb +++ b/spec/features/idv/end_to_end_idv_spec.rb @@ -16,6 +16,7 @@ complete_welcome_step validate_agreement_page + try_to_go_back_from_agreement try_to_skip_ahead_from_agreement complete_agreement_step @@ -344,6 +345,13 @@ def visit_by_mail_and_return expect(page).to have_current_path(idv_phone_path) end + def try_to_go_back_from_agreement + # The back button should work from the agreement step + go_back + expect(current_path).to eq(idv_welcome_path) + visit(idv_agreement_path) + end + def try_to_go_back_from_document_capture visit(idv_agreement_path) expect(page).to have_current_path(idv_document_capture_path) @@ -354,6 +362,11 @@ def try_to_go_back_from_document_capture def try_to_go_back_from_verify_info visit(idv_document_capture_url) expect(page).to have_current_path(idv_verify_info_path) + + visit(idv_welcome_path) + expect(page).to have_current_path(idv_welcome_path) + complete_welcome_step + expect(page).to have_current_path(idv_verify_info_path) end def same_phone?(phone1, phone2) From 9069a21afdf1a862ead761f45d97bb74b2ada0f1 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Fri, 29 Sep 2023 16:26:38 -0700 Subject: [PATCH 02/26] Allow going back to agreement step - Keep consent checkbox checked on back - Clear consent checkbox after re-submitting welcome --- app/controllers/idv/agreement_controller.rb | 5 ++- app/controllers/idv/welcome_controller.rb | 1 + app/forms/idv/consent_form.rb | 10 ++++- app/views/idv/agreement/show.html.erb | 3 +- spec/features/idv/end_to_end_idv_spec.rb | 43 +++++++++++++++++++-- 5 files changed, 55 insertions(+), 7 deletions(-) diff --git a/app/controllers/idv/agreement_controller.rb b/app/controllers/idv/agreement_controller.rb index 9cd06b3a3e8..2b602cd35b2 100644 --- a/app/controllers/idv/agreement_controller.rb +++ b/app/controllers/idv/agreement_controller.rb @@ -5,7 +5,6 @@ class AgreementController < ApplicationController before_action :confirm_not_rate_limited before_action :confirm_welcome_step_complete - before_action :confirm_agreement_needed def show analytics.idv_doc_auth_agreement_visited(**analytics_arguments) @@ -14,6 +13,10 @@ def show 'agreement', :view, true ) + + @consent_form = Idv::ConsentForm.new( + idv_consent_given: idv_session.idv_consent_given, + ) end def update diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index f1c2402e451..f658e8e2011 100644 --- a/app/controllers/idv/welcome_controller.rb +++ b/app/controllers/idv/welcome_controller.rb @@ -28,6 +28,7 @@ def update cancel_previous_in_person_enrollments idv_session.welcome_visited = true + idv_session.idv_consent_given = false redirect_to idv_agreement_url end diff --git a/app/forms/idv/consent_form.rb b/app/forms/idv/consent_form.rb index 933733623bc..922feb20a24 100644 --- a/app/forms/idv/consent_form.rb +++ b/app/forms/idv/consent_form.rb @@ -2,15 +2,23 @@ module Idv class ConsentForm include ActiveModel::Model - validates :idv_consent_given?, + validates :ial2_consent_given, acceptance: { message: proc { I18n.t('errors.doc_auth.consent_form') } } + def initialize(idv_consent_given: false) + @idv_consent_given = idv_consent_given + end + def submit(params) @idv_consent_given = params[:idv_consent_given] == '1' FormResponse.new(success: valid?, errors: errors) end + def ial2_consent_given + idv_consent_given? + end + def idv_consent_given? @idv_consent_given end diff --git a/app/views/idv/agreement/show.html.erb b/app/views/idv/agreement/show.html.erb index e39fbc04049..cbd0f9476e5 100644 --- a/app/views/idv/agreement/show.html.erb +++ b/app/views/idv/agreement/show.html.erb @@ -17,7 +17,8 @@

<%= t('doc_auth.info.secure_account') %>

<%= simple_form_for( - :doc_auth, + @consent_form, + as: :doc_auth, url: url_for, method: 'put', html: { autocomplete: 'off', class: 'margin-top-2 margin-bottom-5 js-consent-continue-form' }, diff --git a/spec/features/idv/end_to_end_idv_spec.rb b/spec/features/idv/end_to_end_idv_spec.rb index 387e0899055..c6ca9f2bd6f 100644 --- a/spec/features/idv/end_to_end_idv_spec.rb +++ b/spec/features/idv/end_to_end_idv_spec.rb @@ -21,6 +21,7 @@ complete_agreement_step validate_hybrid_handoff_page + try_to_go_back_from_hybrid_handoff try_to_skip_ahead_from_hybrid_handoff complete_hybrid_handoff_step # upload photos @@ -346,15 +347,42 @@ def visit_by_mail_and_return end def try_to_go_back_from_agreement - # The back button should work from the agreement step go_back expect(current_path).to eq(idv_welcome_path) - visit(idv_agreement_path) + complete_welcome_step + expect(current_path).to eq(idv_agreement_path) + expect(page).not_to have_checked_field( + t('doc_auth.instructions.consent', app_name: APP_NAME), + visible: :all, + ) + end + + def try_to_go_back_from_hybrid_handoff + go_back + expect(current_path).to eql(idv_agreement_path) + expect(page).to have_checked_field( + t('doc_auth.instructions.consent', app_name: APP_NAME), + visible: :all, + ) + visit idv_welcome_path + expect(current_path).to eql(idv_welcome_path) + complete_welcome_step + expect(page).to have_current_path(idv_agreement_path) + expect(page).not_to have_checked_field( + t('doc_auth.instructions.consent', app_name: APP_NAME), + visible: :all, + ) + complete_agreement_step end def try_to_go_back_from_document_capture visit(idv_agreement_path) - expect(page).to have_current_path(idv_document_capture_path) + expect(page).to have_current_path(idv_agreement_path) + expect(page).to have_checked_field( + t('doc_auth.instructions.consent', app_name: APP_NAME), + visible: :all, + ) + visit(idv_hybrid_handoff_url) expect(page).to have_current_path(idv_document_capture_path) end @@ -362,10 +390,17 @@ def try_to_go_back_from_document_capture def try_to_go_back_from_verify_info visit(idv_document_capture_url) expect(page).to have_current_path(idv_verify_info_path) - visit(idv_welcome_path) expect(page).to have_current_path(idv_welcome_path) complete_welcome_step + + expect(page).to have_current_path(idv_agreement_path) + expect(page).not_to have_checked_field( + t('doc_auth.instructions.consent', app_name: APP_NAME), + visible: :all, + ) + complete_agreement_step + expect(page).to have_current_path(idv_verify_info_path) end From 3d65d9ed9848af4cd52693ff6ca2102ae95178e7 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 2 Oct 2023 12:23:17 -0700 Subject: [PATCH 03/26] Update WelcomeController spec --- spec/controllers/idv/welcome_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/idv/welcome_controller_spec.rb b/spec/controllers/idv/welcome_controller_spec.rb index 7dfcd41267e..ad306ab3f2a 100644 --- a/spec/controllers/idv/welcome_controller_spec.rb +++ b/spec/controllers/idv/welcome_controller_spec.rb @@ -69,12 +69,12 @@ end context 'welcome already visited' do - it 'redirects to agreement' do + it 'does not redirect to agreement' do subject.idv_session.welcome_visited = true get :show - expect(response).to redirect_to(idv_agreement_url) + expect(response).to render_template('idv/welcome/show') end end From c5ee3f85480d834b8eb56acd9d34fa376b93b04b Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 2 Oct 2023 12:27:51 -0700 Subject: [PATCH 04/26] Update AgreementController spec --- spec/controllers/idv/agreement_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/idv/agreement_controller_spec.rb b/spec/controllers/idv/agreement_controller_spec.rb index 38068894fae..53dcf8a3ad8 100644 --- a/spec/controllers/idv/agreement_controller_spec.rb +++ b/spec/controllers/idv/agreement_controller_spec.rb @@ -74,12 +74,12 @@ end context 'agreement already visited' do - it 'redirects to hybrid_handoff' do + it 'does not redirect to hybrid_handoff' do allow(subject.idv_session).to receive(:idv_consent_given).and_return(true) get :show - expect(response).to redirect_to(idv_hybrid_handoff_url) + expect(response).to render_template('idv/agreement/show') end end end From 3b8533ef53eda2e0199fb338c614a7ff4800d8dd Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 2 Oct 2023 14:57:57 -0700 Subject: [PATCH 05/26] Update agreement view spec --- spec/views/idv/agreement/show.html.erb_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/views/idv/agreement/show.html.erb_spec.rb b/spec/views/idv/agreement/show.html.erb_spec.rb index 25ba56fd5c8..78a43dae781 100644 --- a/spec/views/idv/agreement/show.html.erb_spec.rb +++ b/spec/views/idv/agreement/show.html.erb_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' RSpec.describe 'idv/agreement/show' do + let(:consent_form) do + Idv::ConsentForm.new + end + before do allow(view).to receive(:user_signing_up?).and_return(false) allow(view).to receive(:url_for).and_wrap_original do |method, *args, &block| @@ -8,6 +12,7 @@ rescue '' end + assign :consent_form, consent_form render end From cb88b04068c61bbcf700cd9cdc95e2a4a240ddbd Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 2 Oct 2023 15:07:58 -0700 Subject: [PATCH 06/26] Update redo document capture feature spec --- spec/features/idv/doc_auth/redo_document_capture_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/idv/doc_auth/redo_document_capture_spec.rb b/spec/features/idv/doc_auth/redo_document_capture_spec.rb index 7e3eea50122..1a80ebc66d6 100644 --- a/spec/features/idv/doc_auth/redo_document_capture_spec.rb +++ b/spec/features/idv/doc_auth/redo_document_capture_spec.rb @@ -88,7 +88,7 @@ expect(page).to have_content(t('idv.failure.phone.warning.heading')) visit idv_url - expect(current_path).to eq(idv_phone_path) + expect(current_path).to eq(idv_welcome_path) visit idv_hybrid_handoff_url expect(current_path).to eq(idv_phone_path) From 2010e770587d45bc7d018c39550199536b6ed0c0 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Tue, 3 Oct 2023 11:21:37 -0700 Subject: [PATCH 07/26] Don't reset consent flag when proceeding from welcome step Reviewed this with the team and there was a desire to keep this box checked after the user had checked it. --- app/controllers/idv/welcome_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index f658e8e2011..f1c2402e451 100644 --- a/app/controllers/idv/welcome_controller.rb +++ b/app/controllers/idv/welcome_controller.rb @@ -28,7 +28,6 @@ def update cancel_previous_in_person_enrollments idv_session.welcome_visited = true - idv_session.idv_consent_given = false redirect_to idv_agreement_url end From 0227ac8fb3692c27497aaf134ca3ee89b406f6a8 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Tue, 3 Oct 2023 15:50:47 -0700 Subject: [PATCH 08/26] Prevent accessing welcome step after completing document capture --- app/controllers/idv/welcome_controller.rb | 6 ++++++ spec/controllers/idv/welcome_controller_spec.rb | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index f1c2402e451..0e80f08c74a 100644 --- a/app/controllers/idv/welcome_controller.rb +++ b/app/controllers/idv/welcome_controller.rb @@ -5,6 +5,7 @@ class WelcomeController < ApplicationController include GettingStartedAbTestConcern before_action :confirm_not_rate_limited + before_action :confirm_welcome_step_available before_action :maybe_redirect_for_getting_started_ab_test def show @@ -42,6 +43,11 @@ def analytics_arguments }.merge(ab_test_analytics_buckets) end + def confirm_welcome_step_available + # Don't allow access to the welcome step after the user has completed document capture. + redirect_to idv_phone_url if idv_session.pii_from_doc + end + def create_document_capture_session document_capture_session = DocumentCaptureSession.create( user_id: current_user.id, diff --git a/spec/controllers/idv/welcome_controller_spec.rb b/spec/controllers/idv/welcome_controller_spec.rb index ad306ab3f2a..11c0c048559 100644 --- a/spec/controllers/idv/welcome_controller_spec.rb +++ b/spec/controllers/idv/welcome_controller_spec.rb @@ -76,6 +76,16 @@ expect(response).to render_template('idv/welcome/show') end + + context 'and document capture already completed' do + before do + subject.idv_session.pii_from_doc = { first_name: 'Susan' } + end + it 'redirects to phone step' do + get :show + expect(response).to redirect_to(idv_phone_url) + end + end end it 'redirects to please call page if fraud review is pending' do From e419c25c3156dbd5b45f863c7a3ee58bfeee7755 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Tue, 3 Oct 2023 16:16:59 -0700 Subject: [PATCH 09/26] Update idv end-to-end spec --- spec/features/idv/end_to_end_idv_spec.rb | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/spec/features/idv/end_to_end_idv_spec.rb b/spec/features/idv/end_to_end_idv_spec.rb index c6ca9f2bd6f..d3b3c8c63b7 100644 --- a/spec/features/idv/end_to_end_idv_spec.rb +++ b/spec/features/idv/end_to_end_idv_spec.rb @@ -368,7 +368,7 @@ def try_to_go_back_from_hybrid_handoff expect(current_path).to eql(idv_welcome_path) complete_welcome_step expect(page).to have_current_path(idv_agreement_path) - expect(page).not_to have_checked_field( + expect(page).to have_checked_field( t('doc_auth.instructions.consent', app_name: APP_NAME), visible: :all, ) @@ -391,16 +391,6 @@ def try_to_go_back_from_verify_info visit(idv_document_capture_url) expect(page).to have_current_path(idv_verify_info_path) visit(idv_welcome_path) - expect(page).to have_current_path(idv_welcome_path) - complete_welcome_step - - expect(page).to have_current_path(idv_agreement_path) - expect(page).not_to have_checked_field( - t('doc_auth.instructions.consent', app_name: APP_NAME), - visible: :all, - ) - complete_agreement_step - expect(page).to have_current_path(idv_verify_info_path) end From 6cd44b828209395c056ae164d078ba18e70e8e15 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 4 Oct 2023 12:28:28 -0700 Subject: [PATCH 10/26] Clean up ial2_consent_given vs idv_consent_given --- app/forms/idv/consent_form.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/app/forms/idv/consent_form.rb b/app/forms/idv/consent_form.rb index 922feb20a24..9a11113d77e 100644 --- a/app/forms/idv/consent_form.rb +++ b/app/forms/idv/consent_form.rb @@ -2,7 +2,9 @@ module Idv class ConsentForm include ActiveModel::Model - validates :ial2_consent_given, + attr_reader :idv_consent_given + + validates :idv_consent_given, acceptance: { message: proc { I18n.t('errors.doc_auth.consent_form') } } def initialize(idv_consent_given: false) @@ -14,13 +16,5 @@ def submit(params) FormResponse.new(success: valid?, errors: errors) end - - def ial2_consent_given - idv_consent_given? - end - - def idv_consent_given? - @idv_consent_given - end end end From efd55ca0c2d4bbb058a2bc9a58384c7e7ce509a9 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 5 Oct 2023 11:12:53 -0700 Subject: [PATCH 11/26] Add document_capture_complete? method in idv_session Co-authored-by Matt Hinz --- app/controllers/idv/welcome_controller.rb | 2 +- app/services/idv/session.rb | 4 ++++ spec/controllers/idv/welcome_controller_spec.rb | 5 +++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index 0e80f08c74a..8614d7177c6 100644 --- a/app/controllers/idv/welcome_controller.rb +++ b/app/controllers/idv/welcome_controller.rb @@ -45,7 +45,7 @@ def analytics_arguments def confirm_welcome_step_available # Don't allow access to the welcome step after the user has completed document capture. - redirect_to idv_phone_url if idv_session.pii_from_doc + redirect_to idv_ssn_url if idv_session.document_capture_complete? end def create_document_capture_session diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index 6115ef4c9f0..f6a2edba032 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -143,6 +143,10 @@ def add_failed_phone_step_number(phone) failed_phone_step_numbers << phone_e164 if !failed_phone_step_numbers.include?(phone_e164) end + def document_capture_complete? + pii_from_doc || verify_info_step_complete? + end + def verify_info_step_complete? resolution_successful end diff --git a/spec/controllers/idv/welcome_controller_spec.rb b/spec/controllers/idv/welcome_controller_spec.rb index 11c0c048559..05611132564 100644 --- a/spec/controllers/idv/welcome_controller_spec.rb +++ b/spec/controllers/idv/welcome_controller_spec.rb @@ -81,9 +81,10 @@ before do subject.idv_session.pii_from_doc = { first_name: 'Susan' } end - it 'redirects to phone step' do + + it 'redirects to ssn step' do get :show - expect(response).to redirect_to(idv_phone_url) + expect(response).to redirect_to(idv_ssn_url) end end end From bf2a947de76cc87132cb2deeb12edcd052faefa1 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 5 Oct 2023 11:18:00 -0700 Subject: [PATCH 12/26] Move and rename before_action that confirms that document capture is not complete --- app/controllers/concerns/idv_step_concern.rb | 6 ++++++ app/controllers/idv/welcome_controller.rb | 7 +------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 036c841944f..bd199705e07 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -68,6 +68,12 @@ def confirm_hybrid_handoff_needed private + def confirm_document_capture_not_complete + return unless idv_session.document_capture_complete? + + redirect_to idv_ssn_url + end + def confirm_ssn_step_complete return if pii.present? && idv_session.ssn.present? redirect_to prev_url diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index 8614d7177c6..5eaa1537da2 100644 --- a/app/controllers/idv/welcome_controller.rb +++ b/app/controllers/idv/welcome_controller.rb @@ -5,7 +5,7 @@ class WelcomeController < ApplicationController include GettingStartedAbTestConcern before_action :confirm_not_rate_limited - before_action :confirm_welcome_step_available + before_action :confirm_document_capture_not_complete before_action :maybe_redirect_for_getting_started_ab_test def show @@ -43,11 +43,6 @@ def analytics_arguments }.merge(ab_test_analytics_buckets) end - def confirm_welcome_step_available - # Don't allow access to the welcome step after the user has completed document capture. - redirect_to idv_ssn_url if idv_session.document_capture_complete? - end - def create_document_capture_session document_capture_session = DocumentCaptureSession.create( user_id: current_user.id, From d71df45c22ab63ae2f346a8141e121861f00de86 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 5 Oct 2023 11:44:56 -0700 Subject: [PATCH 13/26] Add before action to agreement_controller, add specs, add pii_from_user --- app/controllers/idv/agreement_controller.rb | 1 + app/services/idv/session.rb | 6 +- .../concerns/idv_step_concern_spec.rb | 59 +++++++++++++++++++ .../idv/agreement_controller_spec.rb | 11 ++++ 4 files changed, 76 insertions(+), 1 deletion(-) diff --git a/app/controllers/idv/agreement_controller.rb b/app/controllers/idv/agreement_controller.rb index 2b602cd35b2..b83081e0e0c 100644 --- a/app/controllers/idv/agreement_controller.rb +++ b/app/controllers/idv/agreement_controller.rb @@ -5,6 +5,7 @@ class AgreementController < ApplicationController before_action :confirm_not_rate_limited before_action :confirm_welcome_step_complete + before_action :confirm_document_capture_not_complete def show analytics.idv_doc_auth_agreement_visited(**analytics_arguments) diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index f6a2edba032..e5c348fbd35 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -143,8 +143,12 @@ def add_failed_phone_step_number(phone) failed_phone_step_numbers << phone_e164 if !failed_phone_step_numbers.include?(phone_e164) end + def has_pii_from_user_in_flow_session + user_session.dig('idv/in_person', :pii_from_user) + end + def document_capture_complete? - pii_from_doc || verify_info_step_complete? + pii_from_doc || has_pii_from_user_in_flow_session || verify_info_step_complete? end def verify_info_step_complete? diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index b2c2ff8810a..635d75bd79a 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -132,6 +132,65 @@ def show end end + describe '#confirm_document_capture_not_complete' do + controller Idv::StepController do + before_action :confirm_document_capture_not_complete + end + + before(:each) do + sign_in(user) + routes.draw do + get 'show' => 'idv/step#show' + end + end + + context 'the user has not completed document capture' do + it 'does not redirect and renders the view' do + idv_session.pii_from_doc = nil + idv_session.resolution_successful = nil + + get :show + + expect(response.body).to eq('Hello') + expect(response.status).to eq(200) + end + end + + context 'the user has completed remote document capture but not verify_info' do + it 'redirects to the ssn step' do + idv_session.pii_from_doc = { first_name: 'Susan' } + idv_session.resolution_successful = false + + get :show + + expect(response).to redirect_to(idv_ssn_url) + end + end + + context 'the user has completed in person document capture but not verify_info' do + it 'redirects to the ssn step' do + subject.user_session['idv/in_person'] = {} + subject.user_session['idv/in_person'][:pii_from_user] = { first_name: 'Susan' } + idv_session.resolution_successful = false + + get :show + + expect(response).to redirect_to(idv_ssn_url) + end + end + + context 'the user has completed document capture and verify_info' do + it 'redirects to the ssn step' do + idv_session.pii_from_doc = nil + idv_session.resolution_successful = true + + get :show + + expect(response).to redirect_to(idv_ssn_url) + end + end + end + describe '#confirm_verify_info_step_complete' do controller(idv_step_controller_class) do before_action :confirm_verify_info_step_complete diff --git a/spec/controllers/idv/agreement_controller_spec.rb b/spec/controllers/idv/agreement_controller_spec.rb index 53dcf8a3ad8..72ab71711d0 100644 --- a/spec/controllers/idv/agreement_controller_spec.rb +++ b/spec/controllers/idv/agreement_controller_spec.rb @@ -82,6 +82,17 @@ expect(response).to render_template('idv/agreement/show') end end + + context 'and document capture already completed' do + before do + subject.idv_session.pii_from_doc = { first_name: 'Susan' } + end + + it 'redirects to ssn step' do + get :show + expect(response).to redirect_to(idv_ssn_url) + end + end end describe '#update' do From 3a45aabaea383ad3702e69223e0a59d6aef5ecea Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Fri, 6 Oct 2023 09:05:29 -0700 Subject: [PATCH 14/26] Update redo_document_capture_spec.rb Once verify info is completed, all steps before and including it should be inaccessible --- spec/features/idv/doc_auth/redo_document_capture_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/idv/doc_auth/redo_document_capture_spec.rb b/spec/features/idv/doc_auth/redo_document_capture_spec.rb index 1a80ebc66d6..7e3eea50122 100644 --- a/spec/features/idv/doc_auth/redo_document_capture_spec.rb +++ b/spec/features/idv/doc_auth/redo_document_capture_spec.rb @@ -88,7 +88,7 @@ expect(page).to have_content(t('idv.failure.phone.warning.heading')) visit idv_url - expect(current_path).to eq(idv_welcome_path) + expect(current_path).to eq(idv_phone_path) visit idv_hybrid_handoff_url expect(current_path).to eq(idv_phone_path) From f265ad258b33bc48abacd97a1435d5dd8f711dd4 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 6 Oct 2023 14:43:34 -0700 Subject: [PATCH 15/26] Check for document_capture started as well as completed Since we don't have back button for hybrid_handoff worked out yet, don't allow users to go back to agreement or welcome once they submit hybrid_handoff. --- app/controllers/concerns/idv_step_concern.rb | 6 +++ app/controllers/idv/agreement_controller.rb | 1 + app/controllers/idv/welcome_controller.rb | 1 + .../concerns/idv_step_concern_spec.rb | 44 +++++++++++++++++++ .../idv/agreement_controller_spec.rb | 13 +++++- .../idv/welcome_controller_spec.rb | 13 +++++- 6 files changed, 76 insertions(+), 2 deletions(-) diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index bd199705e07..c467985ccef 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -74,6 +74,12 @@ def confirm_document_capture_not_complete redirect_to idv_ssn_url end + def confirm_document_capture_not_started + return unless idv_session.flow_path.present? + + redirect_to idv_hybrid_handoff_url + end + def confirm_ssn_step_complete return if pii.present? && idv_session.ssn.present? redirect_to prev_url diff --git a/app/controllers/idv/agreement_controller.rb b/app/controllers/idv/agreement_controller.rb index b83081e0e0c..8bdeb9367c1 100644 --- a/app/controllers/idv/agreement_controller.rb +++ b/app/controllers/idv/agreement_controller.rb @@ -6,6 +6,7 @@ class AgreementController < ApplicationController before_action :confirm_not_rate_limited before_action :confirm_welcome_step_complete before_action :confirm_document_capture_not_complete + before_action :confirm_document_capture_not_started def show analytics.idv_doc_auth_agreement_visited(**analytics_arguments) diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index 5eaa1537da2..de872df1af4 100644 --- a/app/controllers/idv/welcome_controller.rb +++ b/app/controllers/idv/welcome_controller.rb @@ -6,6 +6,7 @@ class WelcomeController < ApplicationController before_action :confirm_not_rate_limited before_action :confirm_document_capture_not_complete + before_action :confirm_document_capture_not_started before_action :maybe_redirect_for_getting_started_ab_test def show diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index 635d75bd79a..636aca99912 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -191,6 +191,50 @@ def show end end + describe '#confirm_document_capture_not_started' do + controller Idv::StepController do + before_action :confirm_document_capture_not_started + end + + before(:each) do + sign_in(user) + routes.draw do + get 'show' => 'idv/step#show' + end + end + + context 'the user has not started document capture' do + it 'does not redirect and renders the view' do + idv_session.flow_path = nil + + get :show + + expect(response.body).to eq('Hello') + expect(response.status).to eq(200) + end + end + + context 'the user chose standard flow' do + it 'redirects to hybrid handoff' do + idv_session.flow_path = 'standard' + + get :show + + expect(response).to redirect_to(idv_hybrid_handoff_url) + end + end + + context 'the user chose hybrid flow' do + it 'redirects to hybrid handoff' do + idv_session.flow_path = 'hybrid' + + get :show + + expect(response).to redirect_to(idv_hybrid_handoff_url) + end + end + end + describe '#confirm_verify_info_step_complete' do controller(idv_step_controller_class) do before_action :confirm_verify_info_step_complete diff --git a/spec/controllers/idv/agreement_controller_spec.rb b/spec/controllers/idv/agreement_controller_spec.rb index 72ab71711d0..3c5b8b88ab3 100644 --- a/spec/controllers/idv/agreement_controller_spec.rb +++ b/spec/controllers/idv/agreement_controller_spec.rb @@ -83,7 +83,7 @@ end end - context 'and document capture already completed' do + context 'document capture already completed' do before do subject.idv_session.pii_from_doc = { first_name: 'Susan' } end @@ -93,6 +93,17 @@ expect(response).to redirect_to(idv_ssn_url) end end + + context 'document capture started' do + before do + subject.idv_session.flow_path = 'standard' + end + + it 'redirects to hybrid_handoff step' do + get :show + expect(response).to redirect_to(idv_hybrid_handoff_url) + end + end end describe '#update' do diff --git a/spec/controllers/idv/welcome_controller_spec.rb b/spec/controllers/idv/welcome_controller_spec.rb index 05611132564..48fd6a3e5f3 100644 --- a/spec/controllers/idv/welcome_controller_spec.rb +++ b/spec/controllers/idv/welcome_controller_spec.rb @@ -77,7 +77,7 @@ expect(response).to render_template('idv/welcome/show') end - context 'and document capture already completed' do + context 'document capture already completed' do before do subject.idv_session.pii_from_doc = { first_name: 'Susan' } end @@ -87,6 +87,17 @@ expect(response).to redirect_to(idv_ssn_url) end end + + context 'document capture started' do + before do + subject.idv_session.flow_path = 'standard' + end + + it 'redirects to hybrid_handoff step' do + get :show + expect(response).to redirect_to(idv_hybrid_handoff_url) + end + end end it 'redirects to please call page if fraud review is pending' do From 40b6af7111c4ee16b8b75307bdabfae5f02934a2 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 6 Oct 2023 14:55:51 -0700 Subject: [PATCH 16/26] Add changes to GettingStarted --- .../idv/getting_started_controller.rb | 9 ++------- .../idv/getting_started_controller_spec.rb | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/app/controllers/idv/getting_started_controller.rb b/app/controllers/idv/getting_started_controller.rb index 36812b0b31c..126259a79c2 100644 --- a/app/controllers/idv/getting_started_controller.rb +++ b/app/controllers/idv/getting_started_controller.rb @@ -3,7 +3,8 @@ class GettingStartedController < ApplicationController include IdvStepConcern before_action :confirm_not_rate_limited - before_action :confirm_agreement_needed + before_action :confirm_document_capture_not_complete + before_action :confirm_document_capture_not_started def show analytics.idv_doc_auth_getting_started_visited(**analytics_arguments) @@ -75,11 +76,5 @@ def skip_to_capture def consent_form_params params.require(:doc_auth).permit(:idv_consent_given) end - - def confirm_agreement_needed - return unless idv_session.idv_consent_given - - redirect_to idv_hybrid_handoff_url - end end end diff --git a/spec/controllers/idv/getting_started_controller_spec.rb b/spec/controllers/idv/getting_started_controller_spec.rb index d66c21ea9f1..eb6d2df3c73 100644 --- a/spec/controllers/idv/getting_started_controller_spec.rb +++ b/spec/controllers/idv/getting_started_controller_spec.rb @@ -70,12 +70,24 @@ ) end - context 'getting_started already visited' do - it 'redirects to hybrid_handoff' do - subject.idv_session.idv_consent_given = true + context 'document capture already completed' do + before do + subject.idv_session.pii_from_doc = { first_name: 'Susan' } + end + it 'redirects to ssn step' do get :show + expect(response).to redirect_to(idv_ssn_url) + end + end + context 'document capture started' do + before do + subject.idv_session.flow_path = 'standard' + end + + it 'redirects to hybrid_handoff step' do + get :show expect(response).to redirect_to(idv_hybrid_handoff_url) end end From 9b3e861404d2d3c65697d29fff66d00fa0d435cf Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 10 Oct 2023 16:45:28 -0700 Subject: [PATCH 17/26] Revert "Check for document_capture started as well as completed" This reverts commit 6ee321689ea3d555c6f60b9424ef810b9974c3cb. --- app/controllers/concerns/idv_step_concern.rb | 6 --- app/controllers/idv/agreement_controller.rb | 1 - app/controllers/idv/welcome_controller.rb | 1 - .../concerns/idv_step_concern_spec.rb | 44 ------------------- .../idv/agreement_controller_spec.rb | 13 +----- .../idv/welcome_controller_spec.rb | 13 +----- 6 files changed, 2 insertions(+), 76 deletions(-) diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index c467985ccef..bd199705e07 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -74,12 +74,6 @@ def confirm_document_capture_not_complete redirect_to idv_ssn_url end - def confirm_document_capture_not_started - return unless idv_session.flow_path.present? - - redirect_to idv_hybrid_handoff_url - end - def confirm_ssn_step_complete return if pii.present? && idv_session.ssn.present? redirect_to prev_url diff --git a/app/controllers/idv/agreement_controller.rb b/app/controllers/idv/agreement_controller.rb index 8bdeb9367c1..b83081e0e0c 100644 --- a/app/controllers/idv/agreement_controller.rb +++ b/app/controllers/idv/agreement_controller.rb @@ -6,7 +6,6 @@ class AgreementController < ApplicationController before_action :confirm_not_rate_limited before_action :confirm_welcome_step_complete before_action :confirm_document_capture_not_complete - before_action :confirm_document_capture_not_started def show analytics.idv_doc_auth_agreement_visited(**analytics_arguments) diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index de872df1af4..5eaa1537da2 100644 --- a/app/controllers/idv/welcome_controller.rb +++ b/app/controllers/idv/welcome_controller.rb @@ -6,7 +6,6 @@ class WelcomeController < ApplicationController before_action :confirm_not_rate_limited before_action :confirm_document_capture_not_complete - before_action :confirm_document_capture_not_started before_action :maybe_redirect_for_getting_started_ab_test def show diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index 636aca99912..635d75bd79a 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -191,50 +191,6 @@ def show end end - describe '#confirm_document_capture_not_started' do - controller Idv::StepController do - before_action :confirm_document_capture_not_started - end - - before(:each) do - sign_in(user) - routes.draw do - get 'show' => 'idv/step#show' - end - end - - context 'the user has not started document capture' do - it 'does not redirect and renders the view' do - idv_session.flow_path = nil - - get :show - - expect(response.body).to eq('Hello') - expect(response.status).to eq(200) - end - end - - context 'the user chose standard flow' do - it 'redirects to hybrid handoff' do - idv_session.flow_path = 'standard' - - get :show - - expect(response).to redirect_to(idv_hybrid_handoff_url) - end - end - - context 'the user chose hybrid flow' do - it 'redirects to hybrid handoff' do - idv_session.flow_path = 'hybrid' - - get :show - - expect(response).to redirect_to(idv_hybrid_handoff_url) - end - end - end - describe '#confirm_verify_info_step_complete' do controller(idv_step_controller_class) do before_action :confirm_verify_info_step_complete diff --git a/spec/controllers/idv/agreement_controller_spec.rb b/spec/controllers/idv/agreement_controller_spec.rb index 3c5b8b88ab3..72ab71711d0 100644 --- a/spec/controllers/idv/agreement_controller_spec.rb +++ b/spec/controllers/idv/agreement_controller_spec.rb @@ -83,7 +83,7 @@ end end - context 'document capture already completed' do + context 'and document capture already completed' do before do subject.idv_session.pii_from_doc = { first_name: 'Susan' } end @@ -93,17 +93,6 @@ expect(response).to redirect_to(idv_ssn_url) end end - - context 'document capture started' do - before do - subject.idv_session.flow_path = 'standard' - end - - it 'redirects to hybrid_handoff step' do - get :show - expect(response).to redirect_to(idv_hybrid_handoff_url) - end - end end describe '#update' do diff --git a/spec/controllers/idv/welcome_controller_spec.rb b/spec/controllers/idv/welcome_controller_spec.rb index 48fd6a3e5f3..05611132564 100644 --- a/spec/controllers/idv/welcome_controller_spec.rb +++ b/spec/controllers/idv/welcome_controller_spec.rb @@ -77,7 +77,7 @@ expect(response).to render_template('idv/welcome/show') end - context 'document capture already completed' do + context 'and document capture already completed' do before do subject.idv_session.pii_from_doc = { first_name: 'Susan' } end @@ -87,17 +87,6 @@ expect(response).to redirect_to(idv_ssn_url) end end - - context 'document capture started' do - before do - subject.idv_session.flow_path = 'standard' - end - - it 'redirects to hybrid_handoff step' do - get :show - expect(response).to redirect_to(idv_hybrid_handoff_url) - end - end end it 'redirects to please call page if fraud review is pending' do From 8abd90aa19a3029106d48b7d4da8c57dface7056 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 10 Oct 2023 16:46:53 -0700 Subject: [PATCH 18/26] Revert changes to getting_started for document capture not started --- app/controllers/idv/getting_started_controller.rb | 1 - .../idv/getting_started_controller_spec.rb | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/app/controllers/idv/getting_started_controller.rb b/app/controllers/idv/getting_started_controller.rb index 126259a79c2..d6d52c2b89f 100644 --- a/app/controllers/idv/getting_started_controller.rb +++ b/app/controllers/idv/getting_started_controller.rb @@ -4,7 +4,6 @@ class GettingStartedController < ApplicationController before_action :confirm_not_rate_limited before_action :confirm_document_capture_not_complete - before_action :confirm_document_capture_not_started def show analytics.idv_doc_auth_getting_started_visited(**analytics_arguments) diff --git a/spec/controllers/idv/getting_started_controller_spec.rb b/spec/controllers/idv/getting_started_controller_spec.rb index eb6d2df3c73..98581fce871 100644 --- a/spec/controllers/idv/getting_started_controller_spec.rb +++ b/spec/controllers/idv/getting_started_controller_spec.rb @@ -81,17 +81,6 @@ end end - context 'document capture started' do - before do - subject.idv_session.flow_path = 'standard' - end - - it 'redirects to hybrid_handoff step' do - get :show - expect(response).to redirect_to(idv_hybrid_handoff_url) - end - end - it 'redirects to please call page if fraud review is pending' do profile = create(:profile, :fraud_review_pending) From 4e8e344c1432ab39f69a8da6eba254ebdb4d165c Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 10 Oct 2023 16:48:08 -0700 Subject: [PATCH 19/26] Enable back button for hybrid handoff It turns out that polling continues if you go back from LinkSent and return, so it's safe to enable the back button for hybrid handoff. Co-authored-by: Doug Price --- app/controllers/concerns/idv_step_concern.rb | 35 +++++-------------- .../idv/hybrid_handoff_controller_spec.rb | 4 +-- .../idv/phone_question_controller_spec.rb | 18 ---------- .../idv/doc_auth/hybrid_handoff_spec.rb | 3 -- spec/features/idv/end_to_end_idv_spec.rb | 3 +- 5 files changed, 13 insertions(+), 50 deletions(-) diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index bd199705e07..546036cac10 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -45,24 +45,19 @@ def flow_path end def confirm_hybrid_handoff_needed - setup_for_redo if params[:redo] - - if idv_session.skip_hybrid_handoff? - # We previously skipped hybrid handoff. Keep doing that. - idv_session.flow_path = 'standard' + if params[:redo] + idv_session.redo_document_capture = true + elsif idv_session.document_capture_complete? + redirect_to idv_ssn_url && return end - if !FeatureManagement.idv_allow_hybrid_flow? - # When hybrid flow is unavailable, skip it. - # But don't store that we skipped it in idv_session, in case it is back to - # available when the user tries to redo document capture. + # If we previously skipped hybrid handoff, keep doing that. + # If hybrid flow is unavailable, skip it. + # But don't store that we skipped it in idv_session, in case it is back to + # available when the user tries to redo document capture. + if idv_session.skip_hybrid_handoff? || !FeatureManagement.idv_allow_hybrid_flow? idv_session.flow_path = 'standard' - end - - if idv_session.flow_path == 'standard' redirect_to idv_document_capture_url - elsif idv_session.flow_path == 'hybrid' - redirect_to idv_link_sent_url end end @@ -123,16 +118,4 @@ def extra_analytics_properties end extra end - - def setup_for_redo - idv_session.redo_document_capture = true - - # If we previously skipped hybrid handoff for the user (because they're on a mobile - # device with a camera), skip it _again_ here. - if idv_session.skip_hybrid_handoff? - idv_session.flow_path = 'standard' - else - idv_session.flow_path = nil - end - end end diff --git a/spec/controllers/idv/hybrid_handoff_controller_spec.rb b/spec/controllers/idv/hybrid_handoff_controller_spec.rb index 673aac94439..0d9f9727a7b 100644 --- a/spec/controllers/idv/hybrid_handoff_controller_spec.rb +++ b/spec/controllers/idv/hybrid_handoff_controller_spec.rb @@ -97,7 +97,7 @@ get :show - expect(response).to redirect_to(idv_document_capture_url) + expect(response).to render_template :show end it 'redirects to link_sent in hybrid flow' do @@ -105,7 +105,7 @@ get :show - expect(response).to redirect_to(idv_link_sent_url) + expect(response).to render_template :show end end diff --git a/spec/controllers/idv/phone_question_controller_spec.rb b/spec/controllers/idv/phone_question_controller_spec.rb index d07a2da4d95..4a794185462 100644 --- a/spec/controllers/idv/phone_question_controller_spec.rb +++ b/spec/controllers/idv/phone_question_controller_spec.rb @@ -62,24 +62,6 @@ end end - context 'hybrid_handoff already visited' do - it 'redirects to document_capture in standard flow' do - subject.idv_session.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.idv_session.flow_path = 'hybrid' - - get :show - - expect(response).to redirect_to(idv_link_sent_url) - end - end - context 'on mobile device' do it 'redirects to hybrid_handoff controller' do subject.idv_session.skip_hybrid_handoff = true diff --git a/spec/features/idv/doc_auth/hybrid_handoff_spec.rb b/spec/features/idv/doc_auth/hybrid_handoff_spec.rb index 1432aff561b..36870f2e2e1 100644 --- a/spec/features/idv/doc_auth/hybrid_handoff_spec.rb +++ b/spec/features/idv/doc_auth/hybrid_handoff_spec.rb @@ -36,9 +36,6 @@ 'IdV: doc auth hybrid handoff submitted', hash_including(step: 'hybrid_handoff', 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 diff --git a/spec/features/idv/end_to_end_idv_spec.rb b/spec/features/idv/end_to_end_idv_spec.rb index d3b3c8c63b7..be7cd914035 100644 --- a/spec/features/idv/end_to_end_idv_spec.rb +++ b/spec/features/idv/end_to_end_idv_spec.rb @@ -384,7 +384,8 @@ def try_to_go_back_from_document_capture ) visit(idv_hybrid_handoff_url) - expect(page).to have_current_path(idv_document_capture_path) + expect(page).to have_current_path(idv_hybrid_handoff_path) + visit(idv_document_capture_url) end def try_to_go_back_from_verify_info From 6ed0aaf885ab28afa0ce9cc3b5a1e8aba2aec13b Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 11 Oct 2023 08:35:10 -0700 Subject: [PATCH 20/26] Make test controller anonymous in new idv_step_concern specs --- spec/controllers/concerns/idv_step_concern_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index 635d75bd79a..d1856aa0002 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -133,14 +133,14 @@ def show end describe '#confirm_document_capture_not_complete' do - controller Idv::StepController do + controller (idv_step_controller_class) do before_action :confirm_document_capture_not_complete end before(:each) do sign_in(user) routes.draw do - get 'show' => 'idv/step#show' + get 'show' => 'anonymous#show' end end From 32a8b58fa8936fe2fa2bf84bb109042d6c629e38 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 11 Oct 2023 08:47:57 -0700 Subject: [PATCH 21/26] lint --- spec/controllers/concerns/idv_step_concern_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index d1856aa0002..a13a938390c 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -133,7 +133,7 @@ def show end describe '#confirm_document_capture_not_complete' do - controller (idv_step_controller_class) do + controller(idv_step_controller_class) do before_action :confirm_document_capture_not_complete end From b8e2d7ce09838740890e15a24f9460938b1fe3cb Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Mon, 16 Oct 2023 11:20:25 -0700 Subject: [PATCH 22/26] Redirect from PhoneQuestion directly to DocumentCapture on 'no phone' --- app/controllers/idv/phone_question_controller.rb | 2 +- spec/controllers/idv/phone_question_controller_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/idv/phone_question_controller.rb b/app/controllers/idv/phone_question_controller.rb index a41ce91142a..1c3e59cc82a 100644 --- a/app/controllers/idv/phone_question_controller.rb +++ b/app/controllers/idv/phone_question_controller.rb @@ -31,7 +31,7 @@ def phone_without_camera merge(phone_with_camera: false), ) - redirect_to idv_hybrid_handoff_url + redirect_to idv_document_capture_url end private diff --git a/spec/controllers/idv/phone_question_controller_spec.rb b/spec/controllers/idv/phone_question_controller_spec.rb index cb9ed1023c9..4020738cac0 100644 --- a/spec/controllers/idv/phone_question_controller_spec.rb +++ b/spec/controllers/idv/phone_question_controller_spec.rb @@ -156,10 +156,10 @@ describe '#phone_without_camera' do let(:analytics_name) { :idv_doc_auth_phone_question_submitted } - it 'redirects to hybrid handoff' do + it 'redirects to document_capture' do get :phone_without_camera - expect(response).to redirect_to(idv_hybrid_handoff_url) + expect(response).to redirect_to(idv_document_capture_url) end it 'sends analytics submitted event' do From a5c4f32ccc9bf76409043cdf875b1ff9290906ec Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 18 Oct 2023 13:29:57 -0700 Subject: [PATCH 23/26] Add (failing) spec for confirm_hybrid_handoff needed Add spec to cover the issue Zach identified with `redirect_to && return` --- .../concerns/idv_step_concern_spec.rb | 77 ++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index a13a938390c..4a6253cb5ed 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -3,7 +3,11 @@ RSpec.describe 'IdvStepConcern' do let(:user) { create(:user, :fully_registered, email: 'old_email@example.com') } let(:idv_session) do - Idv::Session.new(user_session: subject.user_session, current_user: user, service_provider: nil) + Idv::Session.new( + user_session: subject.user_session, + current_user: user, + service_provider: nil, + ) end idv_step_controller_class = Class.new(ApplicationController) do @@ -34,6 +38,77 @@ def show end end + describe '#confirm_hybrid_handoff_needed' do + controller(idv_step_controller_class) do + before_action :confirm_hybrid_handoff_needed + end + + let(:params) do + {} + end + + let(:pii_from_doc) { nil } + + let(:skip_hybrid_handoff) { nil } + + before(:each) do + sign_in(user) + routes.draw do + get 'show' => 'anonymous#show' + end + idv_session.pii_from_doc = pii_from_doc + idv_session.skip_hybrid_handoff = skip_hybrid_handoff + end + + context 'redo specified' do + let(:params) { { redo: true } } + + it 'sets flag in idv_session' do + expect { get :show, params: params }.to change { + idv_session.redo_document_capture + }.from(nil).to(true) + end + + it 'does not redirect' do + get :show, params: params + expect(response).to have_http_status(200) + end + end + + context 'document capture complete' do + let(:pii_from_doc) { { first_name: 'Susan' } } + + it 'redirects to ssn screen' do + get :show, params: params + expect(response).to redirect_to(idv_ssn_url) + end + + context 'and redo specified' do + let(:params) { { redo: true } } + it 'does not redirect' do + get :show, params: params + expect(response).to have_http_status(200) + end + end + end + + context 'previously skipped hybrid handoff' do + let(:skip_hybrid_handoff) { true } + + before do + get :show, params: params + end + + it 'sets flow_path to standard' do + expect(idv_session.flow_path).to eql('standard') + end + + it 'redirects to document capture' do + expect(response).to redirect_to(idv_document_capture_url) + end + end + end + describe '#confirm_idv_needed' do controller(idv_step_controller_class) do before_action :confirm_idv_needed From 20b2411d9f7fb6ca633a49d327b7b89bb5e7f504 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 18 Oct 2023 17:47:55 +0000 Subject: [PATCH 24/26] Update app/controllers/concerns/idv_step_concern.rb Co-authored-by: Zach Margolis --- app/controllers/concerns/idv_step_concern.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 546036cac10..8ae8d16106f 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -48,7 +48,8 @@ def confirm_hybrid_handoff_needed if params[:redo] idv_session.redo_document_capture = true elsif idv_session.document_capture_complete? - redirect_to idv_ssn_url && return + redirect_to idv_ssn_url + return end # If we previously skipped hybrid handoff, keep doing that. From 03315ae45a4d016cd7ac8c7eddcd0875e7ba5cee Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 18 Oct 2023 13:44:26 -0700 Subject: [PATCH 25/26] Also cover the "hybrid flow unavailable" case --- .../controllers/concerns/idv_step_concern_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index 4a6253cb5ed..f863234c6c2 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -107,6 +107,21 @@ def show expect(response).to redirect_to(idv_document_capture_url) end end + + context 'hybrid flow not available' do + before do + allow(FeatureManagement).to receive(:idv_allow_hybrid_flow?).and_return(false) + get :show, params: params + end + + it 'sets flow_path to standard' do + expect(idv_session.flow_path).to eql('standard') + end + + it 'redirects to document capture' do + expect(response).to redirect_to(idv_document_capture_url) + end + end end describe '#confirm_idv_needed' do From 0978b0bb4c91317c61e03fb63b237e671cb49263 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 18 Oct 2023 14:50:34 -0700 Subject: [PATCH 26/26] Clean up spec a little bit Fewer lets, less indirection, all that good stuff. --- .../concerns/idv_step_concern_spec.rb | 42 ++++++------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index f863234c6c2..86e7a90ef47 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -3,11 +3,7 @@ RSpec.describe 'IdvStepConcern' do let(:user) { create(:user, :fully_registered, email: 'old_email@example.com') } let(:idv_session) do - Idv::Session.new( - user_session: subject.user_session, - current_user: user, - service_provider: nil, - ) + Idv::Session.new(user_session: subject.user_session, current_user: user, service_provider: nil) end idv_step_controller_class = Class.new(ApplicationController) do @@ -43,60 +39,48 @@ def show before_action :confirm_hybrid_handoff_needed end - let(:params) do - {} - end - - let(:pii_from_doc) { nil } - - let(:skip_hybrid_handoff) { nil } - before(:each) do sign_in(user) routes.draw do get 'show' => 'anonymous#show' end - idv_session.pii_from_doc = pii_from_doc - idv_session.skip_hybrid_handoff = skip_hybrid_handoff end context 'redo specified' do - let(:params) { { redo: true } } - it 'sets flag in idv_session' do - expect { get :show, params: params }.to change { - idv_session.redo_document_capture - }.from(nil).to(true) + expect { get :show, params: { redo: true } }.to change { + idv_session.redo_document_capture + }.from(nil).to(true) end it 'does not redirect' do - get :show, params: params + get :show, params: { redo: true } expect(response).to have_http_status(200) end end context 'document capture complete' do - let(:pii_from_doc) { { first_name: 'Susan' } } + before do + idv_session.pii_from_doc = { first_name: 'Susan' } + end it 'redirects to ssn screen' do - get :show, params: params + get :show expect(response).to redirect_to(idv_ssn_url) end context 'and redo specified' do - let(:params) { { redo: true } } it 'does not redirect' do - get :show, params: params + get :show, params: { redo: true } expect(response).to have_http_status(200) end end end context 'previously skipped hybrid handoff' do - let(:skip_hybrid_handoff) { true } - before do - get :show, params: params + idv_session.skip_hybrid_handoff = true + get :show end it 'sets flow_path to standard' do @@ -111,7 +95,7 @@ def show context 'hybrid flow not available' do before do allow(FeatureManagement).to receive(:idv_allow_hybrid_flow?).and_return(false) - get :show, params: params + get :show end it 'sets flow_path to standard' do