Skip to content
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.

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

LG-14189: A/B test for allowing F/T setup and authentication on desktop #11347

Open
wants to merge 80 commits into
base: main
Choose a base branch
from

Conversation

jmdembe
Copy link
Contributor

@jmdembe jmdembe commented Oct 15, 2024

🎫 Ticket

Link to the relevant ticket:
LG-14189-Implement: A/B test for allowing F/T setup and authentication on desktop

🛠 Summary of changes

This PR implements an A/B test for setting up F/T unlock on desktops. This test will only run for users who are signing up for authentication methods in English.

📜 Testing Plan

Prerequisites:

  • In application.yml, set desktop_ft_unlock_setup_option_percent_tested config value to 100 in development

  • Optional: set show_unsupported_passkey_platform_authentication_setup to true. This will make it so that when testing by switching to another language, the option does not appear at all.

  • Once on http://localhost:3000, register for a new account

  • Follow steps to create a new account

  • Once on the authentication methods setup page, if you set desktop_ft_unlock_setup_option_percent_tested to 100, you will see the F/T unlock setup option

  • Switch to all other languages, you should not be able to see the setup option

Notes:

  • Set both desktop_ft_unlock_setup_option_percent_tested to 0 and show_unsupported_passkey_platform_authentication_setup to false. When following the testing instructions, you will not see the option at all.
  • If show_unsupported_passkey_platform_authentication_setup is set to true while testing, you will see the F/T unlock option in Spanish, French and Chinese shaded in red. This is not a regression as that config value is set to false in production, so it will not appear as such.

@jmdembe jmdembe marked this pull request as draft October 16, 2024 13:57
@jmdembe jmdembe force-pushed the jd-LG-14189-ft-setup-desktop branch from cdba74a to 6e5027e Compare October 16, 2024 18:34
@jmdembe jmdembe force-pushed the jd-LG-14189-ft-setup-desktop branch from 6e5027e to 8adbf86 Compare October 16, 2024 18:44
@voidlily
Copy link
Contributor

can you rebase this branch to pick up changes to the reviewapp deploy process?

Copy link
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

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

Looks good. Tested locally with prescribed conditions and behaves as expected.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Couple minor suggestions, but works well in my testing otherwise 👍

spec/components/webauthn_input_component_spec.rb Outdated Show resolved Hide resolved
spec/config/initializers/ab_tests_spec.rb Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants