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
1 change: 1 addition & 0 deletions app/controllers/concerns/idv/ab_test_analytics_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def ab_test_analytics_buckets
buckets = {}
if defined?(idv_session)
buckets[:skip_hybrid_handoff] = idv_session&.skip_hybrid_handoff
buckets[:phone_with_camera] = idv_session&.phone_with_camera
end

buckets.merge(acuant_sdk_ab_test_analytics_args).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ def phone_question_user

def maybe_redirect_for_phone_question_ab_test
return if phone_question_ab_test_bucket != :show_phone_question
return if request.referer == idv_phone_question_url
return if request.referer == idv_link_sent_url
return if request.referer == idv_hybrid_handoff_url
return if request.referer == idv_hybrid_handoff_url(redo: true)

return if !defined?(idv_session)
return if !idv_session.phone_with_camera.nil?

redirect_to idv_phone_question_url
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ def analytics_arguments
step: 'document_capture',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
}.merge(ab_test_analytics_buckets)
}.merge(
ab_test_analytics_buckets,
phone_with_camera,
)
end

def handle_stored_result
Expand All @@ -82,6 +85,10 @@ def redo_document_capture_pending?

document_capture_session.requested_at > stored_result.captured_at
end

def phone_with_camera
{ phone_with_camera: phone_question_ab_test_bucket == :show_phone_question ? true : nil }
end
Comment on lines +89 to +91
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.

  1. I think this should move into phone_question_ab_test_concern
  2. I think that everything we do here in hybrid_mobile/document_capture_controller.rb we also need to do in idv/document_capture_controller.rb ?

Copy link
Copy Markdown
Contributor Author

@amirbey amirbey Oct 30, 2023

Choose a reason for hiding this comment

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

@charleyf - this is present because we don't have idv session here .. this logic is exclusive to the hybrid document capture

end
end
end
12 changes: 4 additions & 8 deletions app/controllers/idv/phone_question_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,16 @@ def show
end

def phone_with_camera
analytics.idv_doc_auth_phone_question_submitted(
**analytics_arguments.
merge(phone_with_camera: true),
)
idv_session.phone_with_camera = true
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.

@amirbey , so basically user is presented(based on bucket) this question, and answered with true or false. If user is not presented with question, this flag will be nil.

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.

correct 👍🏿

analytics.idv_doc_auth_phone_question_submitted(**analytics_arguments)

redirect_to idv_hybrid_handoff_url
end

def phone_without_camera
idv_session.flow_path = 'standard'
analytics.idv_doc_auth_phone_question_submitted(
**analytics_arguments.
merge(phone_with_camera: false),
)
idv_session.phone_with_camera = false
analytics.idv_doc_auth_phone_question_submitted(**analytics_arguments)

redirect_to idv_document_capture_url
end
Expand Down
1 change: 1 addition & 0 deletions app/services/idv/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class Session
mail_only_warning_shown
personal_key
phone_for_mobile_flow
phone_with_camera
pii
pii_from_doc
previous_phone_step_params
Expand Down
10 changes: 9 additions & 1 deletion spec/controllers/concerns/idv/ab_test_analytics_concern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,22 @@

let(:acuant_sdk_args) { { as_bucket: :as_value } }
let(:getting_started_args) { { gs_bucket: :gs_value } }
let(:phone_question_args) { { pq_bucket: :pq_value } }

before do
allow(subject).to receive(:current_user).and_return(user)
expect(subject).to receive(:acuant_sdk_ab_test_analytics_args).
and_return(acuant_sdk_args)
expect(subject).to receive(:getting_started_ab_test_analytics_bucket).
and_return(getting_started_args)
expect(subject).to receive(:phone_question_ab_test_analytics_bucket).
and_return(phone_question_args)
end

context 'idv_session is available' do
before do
sign_in(user)
expect(subject).to receive(:idv_session).and_return(idv_session)
expect(subject).to receive(:idv_session).twice.and_return(idv_session)
end
it 'includes acuant_sdk_ab_test_analytics_args' do
expect(controller.ab_test_analytics_buckets).to include(acuant_sdk_args)
Expand All @@ -39,6 +42,11 @@
idv_session.skip_hybrid_handoff = :shh_value
expect(controller.ab_test_analytics_buckets).to include({ skip_hybrid_handoff: :shh_value })
end

it 'includes phone_with_camera' do
idv_session.phone_with_camera = :the_value
expect(controller.ab_test_analytics_buckets).to include({ phone_with_camera: :the_value })
end
end

context 'idv_session is not available' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,17 @@ def index
before do
sign_in(user)
end

let(:visited) { nil }
context 'A/B test specifies phone question page' do
before do
allow(controller).to receive(:phone_question_ab_test_bucket).
and_return(:show_phone_question)

idv_session = instance_double(Idv::Session)
allow(idv_session).to receive(:method_missing).
with(:phone_with_camera).
and_return(visited)
allow(controller).to receive(:idv_session).and_return(idv_session)
end

it 'redirects to idv_phone_question_url' do
Expand All @@ -82,10 +88,7 @@ def index
end

context 'referred from phone question page' do
let(:referer) { idv_phone_question_url }
before do
request.env['HTTP_REFERER'] = referer
end
let(:visited) { true }
it 'does not redirect users away from hybrid handoff page' do
get :index

Expand Down
5 changes: 2 additions & 3 deletions spec/controllers/idv/hybrid_handoff_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,9 @@
expect(response).to redirect_to(idv_phone_question_url)
end

context 'when refered by phone_question page' do
let(:referer) { idv_phone_question_url }
context 'when user comes from phone_question page' do
before do
request.env['HTTP_REFERER'] = referer
subject.idv_session.phone_with_camera = false
end

it 'does not redirect users away from hybrid handoff page' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
flow_path: 'hybrid',
irs_reproofing: false,
step: 'document_capture',
phone_with_camera: nil,
}.merge(ab_test_args)
end

Expand All @@ -80,6 +81,18 @@
expect(@analytics).to have_logged_event(analytics_name, analytics_args)
end

context 'user visited phone_question_page' do
before do
allow(AbTests::IDV_PHONE_QUESTION).to receive(:bucket).and_return(:show_phone_question)
end
it 'logs user has a phone_with_camera' do
get :show

expect(@analytics).
to have_logged_event(analytics_name, analytics_args.merge(phone_with_camera: true))
end
end

it 'updates DocAuthLog document_capture_view_count' do
doc_auth_log = DocAuthLog.create(user_id: user.id)

Expand Down Expand Up @@ -159,6 +172,7 @@
flow_path: 'hybrid',
irs_reproofing: false,
step: 'document_capture',
phone_with_camera: nil,
}.merge(ab_test_args)
end

Expand Down
20 changes: 18 additions & 2 deletions spec/controllers/idv/phone_question_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@

expect(response).to redirect_to(idv_agreement_url)
end

it 'phone_with_camera not set in idv_session' do
get :phone_with_camera

expect(subject.idv_session.phone_with_camera).to be_nil
end
end

context 'confirm_hybrid_handoff_needed before action' do
Expand Down Expand Up @@ -147,7 +153,12 @@
get :phone_with_camera

expect(@analytics).
to have_logged_event(analytics_name, analytics_args.merge!(phone_with_camera: true))
to have_logged_event(analytics_name, analytics_args)
end

it 'phone_with_camera set in idv_session' do
expect { get :phone_with_camera }.
to change { subject.idv_session.phone_with_camera }.from(nil).to true
end
end

Expand All @@ -164,12 +175,17 @@
get :phone_without_camera

expect(@analytics).
to have_logged_event(analytics_name, analytics_args.merge!(phone_with_camera: false))
to have_logged_event(analytics_name, analytics_args)
end

it 'set idv_session flow path to standard' do
expect { get :phone_without_camera }.
to change { subject.idv_session.flow_path }.from(nil).to 'standard'
end

it 'phone_with_camera set in idv_session' do
expect { get :phone_without_camera }.
to change { subject.idv_session.phone_with_camera }.from(nil).to false
end
end
end
Loading