Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions app/controllers/idv/in_person/address_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ class AddressController < ApplicationController
include IdvStepConcern

before_action :confirm_step_allowed
before_action :confirm_in_person_address_step_needed, only: :show
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed confirm_in_person_address_step_needed in order to conform with the flow policy documentation.

before_action :set_usps_form_presenter

def show
Expand Down Expand Up @@ -50,7 +49,9 @@ def self.step_info
key: :ipp_address,
controller: self,
next_steps: [:ipp_ssn],
preconditions: ->(idv_session:, user:) { idv_session.ipp_state_id_complete? },
preconditions: ->(idv_session:, user:) {
idv_session.ipp_state_id_complete?
},
undo_step: ->(idv_session:, user:) do
idv_session.invalidate_in_person_address_step!
end,
Expand Down Expand Up @@ -96,13 +97,6 @@ def redirect_to_next_page
end
end

def confirm_in_person_address_step_needed
return if pii_from_user&.dig(:same_address_as_id) == 'false' &&
!pii_from_user.has_key?(:address1)
return if request.referer == idv_in_person_verify_info_url
redirect_to idv_in_person_ssn_url
end

def set_usps_form_presenter
@presenter = Idv::InPerson::UspsFormPresenter.new
end
Expand Down
26 changes: 7 additions & 19 deletions app/controllers/idv/in_person/ssn_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ class SsnController < ApplicationController
include Steps::ThreatMetrixStepHelper
include ThreatMetrixConcern

before_action :confirm_step_allowed
before_action :confirm_not_rate_limited_after_doc_auth
before_action :confirm_in_person_address_step_complete
before_action :confirm_repeat_ssn, only: :show
before_action :override_csp_for_threat_metrix,
if: -> { FeatureManagement.proofing_device_profiling_collecting_enabled? }

Expand Down Expand Up @@ -66,23 +65,17 @@ def self.step_info
key: :ipp_ssn,
controller: self,
next_steps: [:ipp_verify_info],
preconditions: ->(idv_session:, user:) { idv_session.ipp_document_capture_complete? },
undo_step: ->(idv_session:, user:) { idv_session.ssn = nil },
preconditions: ->(idv_session:, user:) {
idv_session.ipp_document_capture_complete?
},
undo_step: ->(idv_session:, user:) {
idv_session.invalidate_ssn_step!
},
)
end

private

def flow_session
user_session.fetch('idv/in_person', {})
end

def confirm_repeat_ssn
return if !idv_session.ssn
return if request.referer == idv_in_person_verify_info_url
redirect_to idv_in_person_verify_info_url
end

def next_url
idv_in_person_verify_info_url
end
Expand All @@ -96,11 +89,6 @@ def analytics_arguments
}.merge(ab_test_analytics_buckets)
.merge(**extra_analytics_properties)
end

def confirm_in_person_address_step_complete
return if flow_session[:pii_from_user] && flow_session[:pii_from_user][:address1].present?
redirect_to idv_in_person_address_url
end
end
end
end
17 changes: 3 additions & 14 deletions app/controllers/idv/in_person/verify_info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ class VerifyInfoController < ApplicationController
include VerifyInfoConcern

before_action :confirm_not_rate_limited_after_doc_auth, except: [:show]
before_action :confirm_pii_data_present
before_action :confirm_ssn_step_complete
before_action :confirm_step_allowed

def show
@step_indicator_steps = step_indicator_steps
Expand Down Expand Up @@ -40,7 +39,8 @@ def self.step_info
controller: self,
next_steps: [:phone],
preconditions: ->(idv_session:, user:) do
idv_session.ssn && idv_session.ipp_document_capture_complete?
idv_session.ssn && idv_session.ipp_document_capture_complete? &&
threatmetrix_session_id_present_or_not_required?(idv_session:)
end,
undo_step: ->(idv_session:, user:) do
idv_session.residential_resolution_vendor = nil
Expand Down Expand Up @@ -89,17 +89,6 @@ def analytics_arguments
}.merge(ab_test_analytics_buckets)
.merge(**extra_analytics_properties)
end

def confirm_ssn_step_complete
return if pii.present? && idv_session.ssn.present?
redirect_to prev_url
end

def confirm_pii_data_present
unless user_session.dig('idv/in_person').present?
redirect_to idv_path
end
end
end
end
end
4 changes: 3 additions & 1 deletion app/controllers/idv/ssn_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ def self.step_info
controller: self,
next_steps: [:verify_info],
preconditions: ->(idv_session:, user:) { idv_session.remote_document_capture_complete? },
undo_step: ->(idv_session:, user:) { idv_session.ssn = nil },
undo_step: ->(idv_session:, user:) {
idv_session.invalidate_ssn_step!
},
)
end

Expand Down
6 changes: 6 additions & 0 deletions app/services/idv/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,12 @@ def ssn_step_complete?
ssn.present?
end

def invalidate_ssn_step!
if user_session[:idv].has_key?(:ssn)
user_session[:idv].delete(:ssn)
end
end

def verify_info_step_complete?
resolution_successful
end
Expand Down
21 changes: 8 additions & 13 deletions spec/controllers/idv/in_person/address_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@
allow(IdentityConfig.store).to receive(:usps_ipp_transliteration_enabled)
.and_return(true)
stub_sign_in(user)
stub_up_to(:hybrid_handoff, idv_session: subject.idv_session)
stub_up_to(:ipp_state_id, idv_session: subject.idv_session)
allow(user).to receive(:establishing_in_person_enrollment).and_return(enrollment)
subject.user_session['idv/in_person'] = {
pii_from_user: pii_from_user,
}
subject.idv_session.ssn = nil
stub_analytics
end

Expand Down Expand Up @@ -97,17 +96,6 @@
expect(response).to render_template :show
end

context 'when address1 present' do
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since the flow policy stipulates that we should rely on the before_action confirm_step_allowed and not use other before_actions, I removed the before_action confirm_in_person_address_step_needed . As a result, this test is no longer relevant.

@jennyverdeyen I'd be curious to hear any thoughts you have on this!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes sense! Since we know this was a duplicate check for the redirect behavior that was already being handled in the state id controller, I went to see if there were tests for this in the state id controller specs. I think there maybe aren't... Is this something we should add there?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jennyverdeyen Good point! I added a new test to the address_controller_spec in 2b4aa47. Let me know if that provides enough coverage or if you want me to add another test.

before do
subject.user_session['idv/in_person'][:pii_from_user][:address1] = '123 Main St'
end
it 'redirects to ssn page' do
get :show

expect(response).to redirect_to idv_in_person_ssn_url
end
end

it 'logs idv_in_person_proofing_address_visited' do
get :show

Expand Down Expand Up @@ -170,6 +158,12 @@
)
end

it 'enables the user to navigate to the ssn page after entering their residential address' do
put :update, params: params

expect(response).to redirect_to(idv_in_person_ssn_url)
end

it 'logs idv_in_person_proofing_address_submitted with 5-digit zipcode' do
put :update, params: params

Expand All @@ -178,6 +172,7 @@

context 'when updating the residential address' do
before do
stub_up_to(:ipp_verify_info, idv_session: subject.idv_session)
subject.user_session['idv/in_person'][:pii_from_user][:address1] =
'123 New Residential Ave'
end
Expand Down
62 changes: 30 additions & 32 deletions spec/controllers/idv/in_person/ssn_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require 'rails_helper'

RSpec.describe Idv::InPerson::SsnController do
include FlowPolicyHelper

let(:pii_from_user) { Idp::Constants::MOCK_IDV_APPLICANT_SAME_ADDRESS_AS_ID_WITH_NO_SSN.dup }

let(:flow_session) do
Expand All @@ -25,13 +27,22 @@
end

describe 'before_actions' do
context '#confirm_in_person_address_step_complete' do
it 'redirects if address page not completed' do
subject.user_session['idv/in_person'][:pii_from_user].delete(:address1)
get :show
before do
stub_up_to(:ipp_state_id, idv_session: subject.idv_session)
subject.user_session['idv/in_person'][:pii_from_user].delete(:address1)
allow(user).to receive(:has_establishing_in_person_enrollment?).and_return(true)
end
it 'redirects if address page not completed' do
get :show

expect(response).to redirect_to idv_in_person_address_url
end
expect(response).to redirect_to idv_in_person_address_url
end

it 'checks that step is allowed' do
expect(subject).to have_actions(
:before,
:confirm_step_allowed,
)
end
end

Expand Down Expand Up @@ -67,37 +78,24 @@
)
end

it 'adds a threatmetrix session id to idv_session' do
expect { get :show }.to change { controller.idv_session.threatmetrix_session_id }.from(nil)
end

it 'does not change threatmetrix_session_id when updating ssn' do
controller.idv_session.ssn = ssn
expect { get :show }.not_to change { controller.idv_session.threatmetrix_session_id }
end
context 'threatmetrix_session_id is nil' do
it 'adds a threatmetrix session id to idv_session' do
expect { get :show }.to change { controller.idv_session.threatmetrix_session_id }.from(nil)
end

context 'with an ssn in idv_session' do
let(:referer) { idv_in_person_address_url }
before do
it 'sets a threatmetrix_session_id when updating ssn' do
controller.idv_session.ssn = ssn
request.env['HTTP_REFERER'] = referer
expect { get :show }.to change { controller.idv_session.threatmetrix_session_id }.from(nil)
end
end

context 'referer is not verify_info' do
it 'redirects to verify_info' do
get :show

expect(response).to redirect_to(idv_in_person_verify_info_url)
end
context 'threatmetrix_session_id is not nil' do
before do
stub_up_to(:ipp_ssn, idv_session: controller.idv_session)
end

context 'referer is verify_info' do
let(:referer) { idv_in_person_verify_info_url }
it 'does not redirect' do
get :show

expect(response).to render_template 'idv/shared/ssn'
end
it 'does not change threatmetrix_session_id when updating ssn' do
controller.idv_session.ssn = ssn
expect { get :show }.not_to change { controller.idv_session.threatmetrix_session_id }
end
end

Expand Down
Loading