LG-14655: A/B test to recommend platform authenticator to SMS users#11402
LG-14655: A/B test to recommend platform authenticator to SMS users#11402
Conversation
There was a problem hiding this comment.
Removed since:
contextis for a scenario which no longer exists (a feature flag that's been removed)- The written test description is the opposite of what it asserts (describes being sent for second MFA, asserts being sent to account page)
- There's already a test case above that asserts the expected behavior for a single selection in the multi-MFA setup flow (source)
|
Should there be a test to support the creation of the |
|
@jmdembe Ah, yeah, there is some precedent for that in |
Added in 7851915. |
There was a problem hiding this comment.
For my understanding: does this differentiate between users in the bucket vs. not?
There was a problem hiding this comment.
ab_test_bucket returns either one of the buckets they've been assigned to if they're in the X% group, otherwise it returns :default. So this checks that they're part of the group.
config/initializers/ab_tests.rb
Outdated
There was a problem hiding this comment.
Request from product to be able to support different configured percentage for recommendation during sign-in vs. account creation.
Could probably handle this by creating separate buckets:
| buckets: { | |
| recommend: IdentityConfig.store.recommend_webauthn_platform_for_sms_ab_test_percent, | |
| }, | |
| buckets: { | |
| recommend_for_account_creation: IdentityConfig.store.recommend_webauthn_platform_for_sms_ab_test_account_creation_percent, | |
| recommend_for_authentication: IdentityConfig.store.recommend_webauthn_platform_for_sms_ab_test_authentication_percent, | |
| }, |
7851915 to
eaa8fad
Compare
|
Looks like theres a merge conflict @aduth |
mdiarra3
left a comment
There was a problem hiding this comment.
Everything LGTM, tested and worked as expected locally. Do we have a followup ticket to ensure set this test up to work for non english users once we get translations.
There was a problem hiding this comment.
Nice like having this split out
That's a good question. I'm assuming the current plan is to wait and see how the test performs to decide whether we want to make this permanent, and only then get the translations. But the immediate follow-on is LG-14191. |
changelog: Internal, Upcoming Features, Create A/B test to recommend platform authenticator to SMS users
Mistakenly removed in rebase
Standardize on "recommend" terminology, also broaden to cover account creation flow
eaa8fad to
4cb2340
Compare
| recommend_for_account_creation: | ||
| IdentityConfig.store.recommend_webauthn_platform_for_sms_ab_test_account_creation_percent, | ||
| recommend_for_authentication: | ||
| IdentityConfig.store.recommend_webauthn_platform_for_sms_ab_test_authentication_percent, |
There was a problem hiding this comment.
Noting that having separate configurations per buckets makes local testing a little cumbersome, since they can't both be maxed out at the same time, otherwise initialization fails with "buckets exceeding 100". The solution is to set one or the other to 100 at a time and test each individually. In production, we'd have a smaller percentage, and using buckets this way allows for test candidates which are mutually exclusive from each other, which isn't strictly a requirement, but also not an issue.
🎫 Ticket
LG-14655
🛠 Summary of changes
Implements new A/B test to recommend Face or Touch Unlock as an authenticator option after either...
📜 Testing Plan
Force A/B test by adding configuration to
config/application.yml:Verify you are presented recommendation when creating account with only SMS:
Verify you are presented recommendation when creating account with only SMS:
mainbranch, repeat Steps 1-5 aboveaduth-lg-14655-sms-ft-recommend-abbranchAlso consider testing with some edge-case scenarios, where users should not be able to opt in...
👀 Screenshots