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

Integrate with remote config to determine if autofill is on by default #3760

Conversation

CDRussell
Copy link
Member

@CDRussell CDRussell commented Oct 26, 2023

Task/Issue URL: https://app.asana.com/0/0/1205811927559467/f

Description

Integrates with remote config to determine if Autofill should be enabled by default, for new users only.

  • New remote config Autofill subfeature onByDefault will be introduced and incrementally rolled out
  • For now, a user will get Autofill enabled by default only if the incremental rollout enables it for a brand new user (installed today). If on the first day the incremental rollout determines a user:
    • can have it enabled by default, it'll be enabled. And this will be recorded locally so we know they had it enabled by default.
    • can not have it enabled by default, it won't be enabled by default
    • Even if it later becomes available to them through the incremental rollout then they still won't have it on by default if that happens after the initial install day. (i.e., they can have the subfeature onByDefault return true but they still won't get it enabled by default)
  • If we determine a user can have it enabled by default today, we'll save this setting so that it's on by default tomorrow and thereafter (even when they're not a new user)

Steps to test this PR

Need to use play builds to properly test this, as internal builds default to enabled by default.

To make tester simpler, would recommend:

  • changing the following in PrivacyConfigPersister. Change line 63 to be val previousVersion = 0L to bypass version checks from the downloaded config
  • hardcoding URL in PrivacyConfigService as needed for the tests below

When remote config new field not yet available

  • Fresh install play build and let it consume production remote config
  • Visit Logins and verify it is disabled by default

When remote config new field available but not enabled for current user

  • Hardcode privacy config to return onByDefault enabled but rolled out to nobody (example)
  • Fresh install play build and let it consume remote config
  • Visit Logins and verify it is disabled by default

When remote config available to a user on install day

  • Hardcode privacy config to return onByDefault enabled for everybody (example)
  • Fresh install play build and let it consume remote config
  • Visit Logins and verify it is enabled by default
  • Set device date forward some days (tip: adb shell am start -a android.settings.DATE_SETTINGS)
  • Visit Logins and verify it is still enabled by default
  • Disable it, leave Login settings and return. Verify it remains disabled, respecting the manual selection

When remote config available to a user but not on install day

  • Hardcode privacy config to return onByDefault enabled but rolled out to nobody (example)
  • Fresh install play build and let it consume remote config
  • Visit Logins and verify it is disabled by default
  • Set device date forward (tip: adb shell am start -a android.settings.DATE_SETTINGS)
  • Hardcode privacy config to return onByDefault enabled for everybody (example) (might have to do this with a proxy or hardcode JSON if date causes SSL issues). Kill and relaunch app and let it consume remote config.
  • Visit Logins and verify it is still disabled by default

@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch 2 times, most recently from a25ca79 to 5acb164 Compare October 26, 2023 13:14
@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch from 2231905 to 6efe846 Compare October 26, 2023 15:07
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch from 5acb164 to 5bf8a7c Compare October 26, 2023 15:23
@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch from 0b75371 to 41fd748 Compare October 26, 2023 15:27
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch from 5bf8a7c to 11f6bb9 Compare October 26, 2023 15:28
@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch from 41fd748 to 1aae7f9 Compare October 30, 2023 11:30
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch 2 times, most recently from 0df6107 to 9420837 Compare October 30, 2023 14:26
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch from 9420837 to e07ac0e Compare October 31, 2023 09:09
@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch from 1aae7f9 to 9367453 Compare October 31, 2023 12:38
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch from e07ac0e to 73daeb0 Compare October 31, 2023 15:16
@CDRussell CDRussell marked this pull request as ready for review October 31, 2023 16:09
@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch from 43c169b to 41c29de Compare November 1, 2023 09:43
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch from 73daeb0 to fc4eba0 Compare November 1, 2023 09:43
@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch from 41c29de to 4e0486e Compare November 6, 2023 16:04
@CDRussell CDRussell mentioned this pull request Nov 6, 2023
6 tasks
@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch from 4e0486e to 5cd0f2a Compare November 7, 2023 10:02
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch from fc4eba0 to 2f94aa0 Compare November 7, 2023 10:02
Copy link
Collaborator

@aitorvs aitorvs left a comment

Choose a reason for hiding this comment

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

lgtm and PR test steps work as expected

@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch 2 times, most recently from 05a0b02 to 5205cfb Compare November 8, 2023 11:43
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch from 2f94aa0 to 5991aeb Compare November 8, 2023 11:43
@CDRussell CDRussell force-pushed the feature/craig/autofill_enabled_by_default branch from 8d6f977 to 5354e7f Compare November 10, 2023 10:47
@CDRussell CDRussell force-pushed the feature/craig/autofill_on_by_default_from_remote_config branch from 5991aeb to a74078a Compare November 10, 2023 10:51
@CDRussell CDRussell merged commit e3005f1 into feature/craig/autofill_enabled_by_default Nov 10, 2023
@CDRussell CDRussell deleted the feature/craig/autofill_on_by_default_from_remote_config branch November 10, 2023 11:22
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.

2 participants