diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 036c841944f..8ae8d16106f 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -45,29 +45,31 @@ 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 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 @@ -117,16 +119,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/app/controllers/idv/agreement_controller.rb b/app/controllers/idv/agreement_controller.rb index 9cd06b3a3e8..b83081e0e0c 100644 --- a/app/controllers/idv/agreement_controller.rb +++ b/app/controllers/idv/agreement_controller.rb @@ -5,7 +5,7 @@ class AgreementController < ApplicationController before_action :confirm_not_rate_limited before_action :confirm_welcome_step_complete - before_action :confirm_agreement_needed + before_action :confirm_document_capture_not_complete def show analytics.idv_doc_auth_agreement_visited(**analytics_arguments) @@ -14,6 +14,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/getting_started_controller.rb b/app/controllers/idv/getting_started_controller.rb index 36812b0b31c..d6d52c2b89f 100644 --- a/app/controllers/idv/getting_started_controller.rb +++ b/app/controllers/idv/getting_started_controller.rb @@ -3,7 +3,7 @@ class GettingStartedController < ApplicationController include IdvStepConcern before_action :confirm_not_rate_limited - before_action :confirm_agreement_needed + before_action :confirm_document_capture_not_complete def show analytics.idv_doc_auth_getting_started_visited(**analytics_arguments) @@ -75,11 +75,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/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/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb index 1b515334c08..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_needed + before_action :confirm_document_capture_not_complete before_action :maybe_redirect_for_getting_started_ab_test def show @@ -56,11 +56,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/app/forms/idv/consent_form.rb b/app/forms/idv/consent_form.rb index 933733623bc..9a11113d77e 100644 --- a/app/forms/idv/consent_form.rb +++ b/app/forms/idv/consent_form.rb @@ -2,17 +2,19 @@ module Idv class ConsentForm include ActiveModel::Model - validates :idv_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) + @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 idv_consent_given? - @idv_consent_given - end end end diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index 6115ef4c9f0..e5c348fbd35 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -143,6 +143,14 @@ 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 || has_pii_from_user_in_flow_session || verify_info_step_complete? + end + def verify_info_step_complete? resolution_successful 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/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb index b2c2ff8810a..86e7a90ef47 100644 --- a/spec/controllers/concerns/idv_step_concern_spec.rb +++ b/spec/controllers/concerns/idv_step_concern_spec.rb @@ -34,6 +34,80 @@ def show end end + describe '#confirm_hybrid_handoff_needed' do + controller(idv_step_controller_class) do + before_action :confirm_hybrid_handoff_needed + end + + before(:each) do + sign_in(user) + routes.draw do + get 'show' => 'anonymous#show' + end + end + + context 'redo specified' do + it 'sets flag in idv_session' do + 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: { redo: true } + expect(response).to have_http_status(200) + end + end + + context 'document capture complete' do + before do + idv_session.pii_from_doc = { first_name: 'Susan' } + end + + it 'redirects to ssn screen' do + get :show + expect(response).to redirect_to(idv_ssn_url) + end + + context 'and redo specified' do + it 'does not redirect' do + get :show, params: { redo: true } + expect(response).to have_http_status(200) + end + end + end + + context 'previously skipped hybrid handoff' do + before do + idv_session.skip_hybrid_handoff = true + get :show + 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 + + context 'hybrid flow not available' do + before do + allow(FeatureManagement).to receive(:idv_allow_hybrid_flow?).and_return(false) + get :show + 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 @@ -132,6 +206,65 @@ def show end end + describe '#confirm_document_capture_not_complete' 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' => 'anonymous#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 38068894fae..72ab71711d0 100644 --- a/spec/controllers/idv/agreement_controller_spec.rb +++ b/spec/controllers/idv/agreement_controller_spec.rb @@ -74,12 +74,23 @@ 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 + + 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 diff --git a/spec/controllers/idv/getting_started_controller_spec.rb b/spec/controllers/idv/getting_started_controller_spec.rb index d66c21ea9f1..98581fce871 100644 --- a/spec/controllers/idv/getting_started_controller_spec.rb +++ b/spec/controllers/idv/getting_started_controller_spec.rb @@ -70,13 +70,14 @@ ) 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_hybrid_handoff_url) + expect(response).to redirect_to(idv_ssn_url) end end diff --git a/spec/controllers/idv/hybrid_handoff_controller_spec.rb b/spec/controllers/idv/hybrid_handoff_controller_spec.rb index f345dfd1b7e..83a12d50150 100644 --- a/spec/controllers/idv/hybrid_handoff_controller_spec.rb +++ b/spec/controllers/idv/hybrid_handoff_controller_spec.rb @@ -104,7 +104,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 @@ -112,7 +112,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 946da503ca5..4020738cac0 100644 --- a/spec/controllers/idv/phone_question_controller_spec.rb +++ b/spec/controllers/idv/phone_question_controller_spec.rb @@ -96,20 +96,20 @@ context 'confirm_hybrid_handoff_needed before action' do context 'standard flow_path already defined' do - it 'redirects to document_capture in standard flow' do + it 'does not redirect to document_capture in standard flow' do subject.idv_session.flow_path = 'standard' 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 + it 'does not redirect to link_sent in hybrid flow' do subject.idv_session.flow_path = 'hybrid' get :show - expect(response).to redirect_to(idv_link_sent_url) + expect(response).to render_template :show end end @@ -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 diff --git a/spec/controllers/idv/welcome_controller_spec.rb b/spec/controllers/idv/welcome_controller_spec.rb index 7dfcd41267e..05611132564 100644 --- a/spec/controllers/idv/welcome_controller_spec.rb +++ b/spec/controllers/idv/welcome_controller_spec.rb @@ -69,12 +69,23 @@ 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 + + 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 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 8d0e77909d0..be7cd914035 100644 --- a/spec/features/idv/end_to_end_idv_spec.rb +++ b/spec/features/idv/end_to_end_idv_spec.rb @@ -16,10 +16,12 @@ complete_welcome_step validate_agreement_page + try_to_go_back_from_agreement try_to_skip_ahead_from_agreement 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 @@ -344,16 +346,53 @@ def visit_by_mail_and_return expect(page).to have_current_path(idv_phone_path) end + def try_to_go_back_from_agreement + go_back + expect(current_path).to eq(idv_welcome_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).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) + expect(page).to have_current_path(idv_hybrid_handoff_path) + visit(idv_document_capture_url) end 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_verify_info_path) end def same_phone?(phone1, phone2) 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