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

Adds Feature Flag for Autofill Service #5567

Open
wants to merge 1 commit into
base: feature/karl/autofill/mapping
Choose a base branch
from

Conversation

cmonfortep
Copy link
Contributor

@cmonfortep cmonfortep commented Jan 31, 2025

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

Description

Adds FFs around Autofill Service. See task for details.

Steps to test this PR

  • checkout the branch
  • go to AutofillServiceFeature, and remove all InternalAlwaysEnabled (otherwise is not possible to test)

Feature 1

  • install the app
  • Go to autofill system settings, ensure DuckDuckGo is not listed there
  • open the app, go to feature flags internal setting, enable Autofill Service
  • restart the app
  • Go to autofill system settings, ensure DuckDuckGo is listed there
  • Selected DDG

Feature 2

  • Perform one test from https://app.asana.com/0/1149059203486286/1209279712661812/f on a native app
  • It should not suggest you anything
  • open the app, go to feature flags internal setting, enable canUpdateAppToDomainDataset and canMapAppToDomain
  • restart the app
  • Repeat the test from step 1
  • Now you should see some suggestions (since we have enabled mapping between apps and domains)

UI changes

Before After
!(Upload before screenshot) (Upload after screenshot)

@cmonfortep
Copy link
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@cmonfortep cmonfortep requested a review from CDRussell January 31, 2025 10:46
@cmonfortep cmonfortep force-pushed the feature/cristian/autofill/ff_autofill_service branch 2 times, most recently from 1552747 to a5e0e7e Compare January 31, 2025 14:21
Comment on lines 92 to 93
COMPONENT_ENABLED_STATE_DISABLED_USER -> true
COMPONENT_ENABLED_STATE_DISABLED_UNTIL_USED -> true
Copy link
Member

Choose a reason for hiding this comment

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

these should return false ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed this offline. For those 2 values we've decided to take no action. They seem to be external to us, one even disabled by the user, so we should honor those disabled states and don't enable our service by our remote config.

New approach returns null for those states, and we don't update the value.

@cmonfortep cmonfortep force-pushed the feature/karl/autofill/mapping branch from 5d63aa5 to 810b9c2 Compare February 4, 2025 13:52
@cmonfortep cmonfortep force-pushed the feature/cristian/autofill/ff_autofill_service branch from a5e0e7e to dec51cb Compare February 4, 2025 13:53
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