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

Add feature flag for importing passwords via Google Password Manager #5096

Merged

Conversation

CDRussell
Copy link
Member

@CDRussell CDRussell commented Oct 4, 2024

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

Description

Sets up new remote config handling for autofill subfeature: canImportFromGooglePasswordManager

This dictates whether we offer up the option to import passwords directly from Google Password Manager or not.

Additional config:

Key Type Description
launchUrl String The initial URL to launch for Google Password Manager
javascriptConfig String The config to pass onto the JS layer handling GPM imports

Steps to test this PR

  • QA optional; they'll be tested more easily in the branches above

Optional

  • Apply patch below
  • Fresh install, launch app and allow time for remote config to be downloaded
  • Open Device Explorer and check values look correct in shared_prefs/com.duckduckgo.feature.toggle.autofill.xml

Patch

Index: privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt b/privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt
--- a/privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt	(revision Staged)
+++ b/privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt	(date 1730729845774)
@@ -27,4 +27,5 @@
     TrackingParametersFeatureName("trackingParameters"),
 }
 
-const val PRIVACY_REMOTE_CONFIG_URL = "https://staticcdn.duckduckgo.com/trackerblocking/config/v4/android-config.json"
+// in use for ship review build #2 const val PRIVACY_REMOTE_CONFIG_URL = "https://jsonblob.com/api/1299412335805194240"
+const val PRIVACY_REMOTE_CONFIG_URL = "https://jsonblob.com/api/1301563859532636160"

Copy link
Member Author

CDRussell commented Oct 4, 2024

@CDRussell CDRussell force-pushed the feature/craig/autofill_import_via_gcm_feature_flag branch from e474f86 to 845c86d Compare October 4, 2024 15:26
@CDRussell CDRussell force-pushed the feature/craig/autofill_import_via_gcm_feature_flag branch 2 times, most recently from 893a6a5 to 312493c Compare October 21, 2024 11:34
@CDRussell CDRussell force-pushed the feature/craig/autofill_import_via_gcm_feature_flag branch 8 times, most recently from b76501a to 0aa7a23 Compare October 28, 2024 12:24
@CDRussell CDRussell force-pushed the feature/craig/autofill_import_via_gcm_feature_flag branch 7 times, most recently from 73e0f43 to 33924e6 Compare November 4, 2024 14:15
@CDRussell CDRussell marked this pull request as ready for review November 4, 2024 14:23
@joshliebe joshliebe self-assigned this Nov 4, 2024
override suspend fun getConfig(): AutofillImportPasswordSettings {
return withContext(dispatchers.io()) {
with(autofillFeature.canImportFromGooglePasswordManager()) {
val config = getConfig()
Copy link
Contributor

@joshliebe joshliebe Nov 4, 2024

Choose a reason for hiding this comment

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

nit: you could rename the outer function for clarity, I thought it was recursive 😁

Copy link
Member Author

@CDRussell CDRussell Nov 4, 2024

Choose a reason for hiding this comment

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

Good point; ditched that with as it wasn't making the code any clearer and now there is no ambiguity

Copy link
Contributor

@joshliebe joshliebe left a comment

Choose a reason for hiding this comment

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

Tested, LGTM 👍

@CDRussell CDRussell force-pushed the feature/craig/autofill_import_via_gcm_feature_flag branch 2 times, most recently from f22ca04 to 91d0a37 Compare November 5, 2024 10:50
@CDRussell CDRussell force-pushed the feature/craig/autofill_import_via_gcm_feature_flag branch 3 times, most recently from b52488e to 8c7ae28 Compare November 8, 2024 10:52
@CDRussell CDRussell force-pushed the feature/craig/autofill_import_via_gcm_feature_flag branch from 8c7ae28 to 5256082 Compare November 11, 2024 11:18
@CDRussell CDRussell merged commit 12ebf77 into develop Nov 11, 2024
6 checks passed
@CDRussell CDRussell deleted the feature/craig/autofill_import_via_gcm_feature_flag branch November 11, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants