Skip to content

LG-7355 GettingStarted A/B analytics #8822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
soniaconnolly merged 21 commits into from
Jul 21, 2023
Merged

LG-7355 GettingStarted A/B analytics #8822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4f19d8a
GettingStarted A/B test uses user uuid, not session id
soniaconnolly Jul 20, 2023
8dff6ca
Prepare to remove StepUtilitiesConcern
soniaconnolly Jul 20, 2023
511914b
Move StepUtilitiesConcern methods into IdvSession concern
soniaconnolly Jul 20, 2023
b7c5423
Replace acuant_sdk_ab_test_analytics_args with ab_test_analytics_args
soniaconnolly Jul 20, 2023
6b91063
Remove unneeded ** before merge of ab_test_analytics_args
soniaconnolly Jul 20, 2023
6ada06f
Remove unneeded native_camera_ab_testing_variables method from hybrid…
soniaconnolly Jul 20, 2023
fd2ff9f
Add specs for AbTestAnalyticsConcern and then mock in controller specs
soniaconnolly Jul 20, 2023
cfc9bed
Update analytics_spec with getting started analytics bucket
soniaconnolly Jul 20, 2023
6d4ae6d
Rename getting_started_a_b_test_bucket to getting_started_ab_test_buc…
soniaconnolly Jul 20, 2023
18a5cd3
Collect ab test analytics methods in AbTestAnalyticsConcern rather th…
soniaconnolly Jul 20, 2023
b767285
lint
soniaconnolly Jul 20, 2023
1794b96
Revert "Collect ab test analytics methods in AbTestAnalyticsConcern r…
soniaconnolly Jul 20, 2023
e83e961
Rename ab_test_analytics_args to ab_test_analytics_buckets
soniaconnolly Jul 20, 2023
545fd57
Get uuid from document_capture_user in hybrid flow
soniaconnolly Jul 20, 2023
37ac56c
Add a/b test bucket info to analytics for GettingStarted, Welcome, Ag…
soniaconnolly Jul 20, 2023
e1cbc3d
Add A/B test buckets to verify proofing results analytics
soniaconnolly Jul 20, 2023
61ad822
Clean up getting_started_ab_test_concern_spec
soniaconnolly Jul 21, 2023
a44cc61
Extract method to choose document_capture_user or current_user to mak…
soniaconnolly Jul 21, 2023
64fd4f1
Add ab test analytics to review_controller visited and submitted events
soniaconnolly Jul 21, 2023
4c329c8
Fix analytics_spec for review events
soniaconnolly Jul 21, 2023
7d2323f
Merge remote-tracking branch 'origin/main' into sonia-lg-7355-getting…
soniaconnolly Jul 21, 2023
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
11 changes: 11 additions & 0 deletions app/controllers/concerns/idv/ab_test_analytics_concern.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Idv
module AbTestAnalyticsConcern
include AcuantConcern
include Idv::GettingStartedAbTestConcern
Copy link
Copy Markdown
Contributor

@jmhooper jmhooper Jul 20, 2023

Choose a reason for hiding this comment

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

I start getting a little nervous when I see a concern the includes a concern that includes a concern.

I'm thinking through whether there is some way for us to flatten this out into an A/B test concern that includes methods for each of the A/B tests we are performing and the tooling for logging them 🤔. That helps to make the rabbit a little more shallow.

Copy link
Copy Markdown
Contributor Author

@soniaconnolly soniaconnolly Jul 20, 2023

Choose a reason for hiding this comment

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

We could put just the analytics methods in here and not include the rest of the concern. I started that way and then thought this might be neater.

Copy link
Copy Markdown
Contributor Author

@soniaconnolly soniaconnolly Jul 20, 2023

Choose a reason for hiding this comment

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

That didn't work out at all! I think we'd have to combine all the code for the A/B tests in one Concern, like you said, and that doesn't seem great either.


def ab_test_analytics_buckets
acuant_sdk_ab_test_analytics_args.
merge(getting_started_ab_test_analytics_bucket)
end
end
end
21 changes: 18 additions & 3 deletions app/controllers/concerns/idv/getting_started_ab_test_concern.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,28 @@
module Idv
module GettingStartedAbTestConcern
def getting_started_a_b_test_bucket
AbTests::IDV_GETTING_STARTED.bucket(sp_session[:request_id] || session.id)
def getting_started_ab_test_bucket
AbTests::IDV_GETTING_STARTED.bucket(getting_started_user.uuid)
end

def getting_started_user
if defined?(document_capture_user) # hybrid flow
document_capture_user
else
current_user
end
end

def maybe_redirect_for_getting_started_ab_test
return if getting_started_a_b_test_bucket != :getting_started
return if getting_started_ab_test_bucket != :getting_started

redirect_to idv_getting_started_url
end

def getting_started_ab_test_analytics_bucket
{
getting_started_ab_test_bucket:
getting_started_ab_test_bucket,
}
end
end
end
1 change: 1 addition & 0 deletions app/controllers/concerns/idv/hybrid_mobile/hybrid_mobile_concern.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module HybridMobileConcern
extend ActiveSupport::Concern

include AcuantConcern
include Idv::AbTestAnalyticsConcern

def check_valid_document_capture_session
if !document_capture_user
Expand Down
23 changes: 0 additions & 23 deletions app/controllers/concerns/idv/step_utilities_concern.rb
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion app/controllers/concerns/idv/verify_info_concern.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ def async_state_done(current_async_state)
address_line2_present: !pii[:address2].blank?,
pii_like_keypaths: [[:errors, :ssn], [:response_body, :first_name],
[:state_id, :state_id_jurisdiction]],
},
}.merge(ab_test_analytics_buckets),
)
log_idv_verification_submitted_event(
success: form_response.success?,
Expand Down
17 changes: 17 additions & 0 deletions app/controllers/concerns/idv_session.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,23 @@ def flow_session
user_session['idv/doc_auth'] ||= {}
end

def irs_reproofing?
current_user&.reproof_for_irs?(
service_provider: current_sp,
).present?
end

def document_capture_session_uuid
flow_session[:document_capture_session_uuid]
end

def document_capture_session
return @document_capture_session if defined?(@document_capture_session)
@document_capture_session = DocumentCaptureSession.find_by(
uuid: document_capture_session_uuid,
)
end

def redirect_unless_idv_session_user
redirect_to root_url if !idv_session_user
end
Expand Down
1 change: 1 addition & 0 deletions app/controllers/concerns/idv_step_concern.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module IdvStepConcern
include RateLimitConcern
include FraudReviewConcern
include Idv::OutageConcern
include Idv::AbTestAnalyticsConcern

included do
before_action :confirm_two_factor_authenticated
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/idv/agreement_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ module Idv
class AgreementController < ApplicationController
include IdvStepConcern
include StepIndicatorConcern
include StepUtilitiesConcern

before_action :confirm_welcome_step_complete
before_action :confirm_agreement_needed
Expand Down Expand Up @@ -43,7 +42,7 @@ def analytics_arguments
step: 'agreement',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
}
}.merge(ab_test_analytics_buckets)
end

def skip_to_capture
Expand Down
4 changes: 1 addition & 3 deletions app/controllers/idv/document_capture_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ class DocumentCaptureController < ApplicationController
include DocumentCaptureConcern
include IdvStepConcern
include StepIndicatorConcern
include StepUtilitiesConcern
include RateLimitConcern

before_action :confirm_hybrid_handoff_complete
before_action :confirm_document_capture_needed
Expand Down Expand Up @@ -77,7 +75,7 @@ def analytics_arguments
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
redo_document_capture: flow_session[:redo_document_capture],
}.compact.merge(**acuant_sdk_ab_test_analytics_args)
}.compact.merge(ab_test_analytics_buckets)
end

def handle_stored_result
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/idv/getting_started_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module Idv
class GettingStartedController < ApplicationController
include IdvStepConcern
include StepUtilitiesConcern

before_action :confirm_agreement_needed

Expand Down Expand Up @@ -49,7 +48,7 @@ def analytics_arguments
step: 'getting_started',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
}
}.merge(ab_test_analytics_buckets)
end

def create_document_capture_session
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/idv/hybrid_handoff_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ class HybridHandoffController < ApplicationController
include ActionView::Helpers::DateHelper
include IdvStepConcern
include StepIndicatorConcern
include StepUtilitiesConcern

before_action :confirm_agreement_step_complete
before_action :confirm_hybrid_handoff_needed, only: :show
Expand Down Expand Up @@ -157,7 +156,7 @@ def analytics_arguments
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
redo_document_capture: params[:redo] ? true : nil,
}.compact.merge(**acuant_sdk_ab_test_analytics_args)
}.compact.merge(ab_test_analytics_buckets)
end

def form_response(destination:)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/idv/hybrid_mobile/capture_complete_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def analytics_arguments
step: 'capture_complete',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
}.merge(**acuant_sdk_ab_test_analytics_args)
}.merge(ab_test_analytics_buckets)
end
end
end
Expand Down
10 changes: 1 addition & 9 deletions app/controllers/idv/hybrid_mobile/document_capture_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ def extra_view_variables
document_capture_session_uuid: document_capture_session_uuid,
failure_to_proof_url: return_to_sp_failure_to_proof_url(step: 'document_capture'),
}.merge(
native_camera_ab_testing_variables,
acuant_sdk_upgrade_a_b_testing_variables,
)
end
Expand All @@ -57,7 +56,7 @@ def analytics_arguments
step: 'document_capture',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
}.merge(**acuant_sdk_ab_test_analytics_args)
}.merge(ab_test_analytics_buckets)
end

def handle_stored_result
Expand All @@ -70,13 +69,6 @@ def handle_stored_result
failure(I18n.t('doc_auth.errors.general.network_error'), extra)
end
end

def native_camera_ab_testing_variables
{
acuant_sdk_upgrade_ab_test_bucket:
AbTests::ACUANT_SDK.bucket(document_capture_session_uuid),
}
end
end
end
end
3 changes: 1 addition & 2 deletions app/controllers/idv/in_person/ssn_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ module InPerson
class SsnController < ApplicationController
include IdvStepConcern
include StepIndicatorConcern
include StepUtilitiesConcern
include Steps::ThreatMetrixStepHelper
include ThreatMetrixConcern

Expand Down Expand Up @@ -84,7 +83,7 @@ def analytics_arguments
step: 'ssn',
analytics_id: 'In Person Proofing',
irs_reproofing: irs_reproofing?,
}.merge(**acuant_sdk_ab_test_analytics_args)
}.merge(ab_test_analytics_buckets)
end

def updating_ssn?
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/idv/in_person/verify_info_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ module InPerson
class VerifyInfoController < ApplicationController
include IdvStepConcern
include StepIndicatorConcern
include StepUtilitiesConcern
include Steps::ThreatMetrixStepHelper
include VerifyInfoConcern

Expand Down Expand Up @@ -63,7 +62,7 @@ def pii
@pii = flow_session[:pii_from_user]
end

# override StepUtilitiesConcern
# override IdvSession concern
def flow_session
user_session.fetch('idv/in_person', {})
end
Expand All @@ -74,7 +73,7 @@ def analytics_arguments
step: 'verify',
analytics_id: 'In Person Proofing',
irs_reproofing: irs_reproofing?,
}.merge(**acuant_sdk_ab_test_analytics_args).
}.merge(ab_test_analytics_buckets).
merge(**extra_analytics_properties)
end

Expand Down
3 changes: 1 addition & 2 deletions app/controllers/idv/link_sent_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ class LinkSentController < ApplicationController
include DocumentCaptureConcern
include IdvStepConcern
include StepIndicatorConcern
include StepUtilitiesConcern

before_action :confirm_hybrid_handoff_complete
before_action :confirm_document_capture_needed
Expand Down Expand Up @@ -63,7 +62,7 @@ def analytics_arguments
analytics_id: 'Doc Auth',
flow_path: 'hybrid',
irs_reproofing: irs_reproofing?,
}.merge(**acuant_sdk_ab_test_analytics_args)
}.merge(ab_test_analytics_buckets)
end

def handle_document_verification_success(get_results_response)
Expand Down
8 changes: 7 additions & 1 deletion app/controllers/idv/review_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def confirm_current_password
gpo_verification_pending: current_user.gpo_verification_pending_profile?,
fraud_review_pending: fraud_review_pending?,
fraud_rejection: fraud_rejection?,
**ab_test_analytics_buckets,
)
irs_attempts_api_tracker.idv_password_entered(success: false)

Expand All @@ -34,7 +35,10 @@ def confirm_current_password
def new
Funnel::DocAuth::RegisterStep.new(current_user.id, current_sp&.issuer).
call(:encrypt, :view, true)
analytics.idv_review_info_visited(address_verification_method: address_verification_method)
analytics.idv_review_info_visited(
address_verification_method: address_verification_method,
**ab_test_analytics_buckets,
)

gpo_mail_service = Idv::GpoMail.new(current_user)
flash_now = flash.now
Expand Down Expand Up @@ -67,6 +71,7 @@ def create
fraud_rejection: idv_session.profile.fraud_rejection?,
gpo_verification_pending: idv_session.profile.gpo_verification_pending?,
deactivation_reason: idv_session.profile.deactivation_reason,
**ab_test_analytics_buckets,
)
Funnel::DocAuth::RegisterStep.new(current_user.id, current_sp&.issuer).
call(:verified, :view, true)
Expand All @@ -76,6 +81,7 @@ def create
fraud_rejection: idv_session.profile.fraud_rejection?,
gpo_verification_pending: idv_session.profile.gpo_verification_pending?,
deactivation_reason: idv_session.profile.deactivation_reason,
**ab_test_analytics_buckets,
)

return unless FeatureManagement.reveal_gpo_code?
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/idv/ssn_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ module Idv
class SsnController < ApplicationController
include IdvStepConcern
include StepIndicatorConcern
include StepUtilitiesConcern
include Steps::ThreatMetrixStepHelper
include ThreatMetrixConcern

Expand Down Expand Up @@ -90,7 +89,7 @@ def analytics_arguments
step: 'ssn',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
}.merge(**acuant_sdk_ab_test_analytics_args)
}.merge(ab_test_analytics_buckets)
end

def updating_ssn?
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/idv/verify_info_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module Idv
class VerifyInfoController < ApplicationController
include IdvStepConcern
include StepUtilitiesConcern
include StepIndicatorConcern
include VerifyInfoConcern
include Steps::ThreatMetrixStepHelper
Expand Down Expand Up @@ -52,7 +51,7 @@ def analytics_arguments
step: 'verify',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
}.merge(**acuant_sdk_ab_test_analytics_args)
}.merge(ab_test_analytics_buckets)
end

# copied from verify_step
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/idv/welcome_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ module Idv
class WelcomeController < ApplicationController
include IdvStepConcern
include StepIndicatorConcern
include StepUtilitiesConcern
include GettingStartedAbTestConcern

before_action :confirm_welcome_needed
Expand Down Expand Up @@ -37,7 +36,7 @@ def analytics_arguments
step: 'welcome',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
}
}.merge(ab_test_analytics_buckets)
end

def create_document_capture_session
Expand Down
Loading