From 6459a859b95dcce839bdf71c020a2c6c25437944 Mon Sep 17 00:00:00 2001 From: Elise Richards Date: Thu, 13 May 2021 14:08:46 -0500 Subject: [PATCH] For #17537: Add preferences for syncing credit cards and addresses (#19471) * Add preferences for enable/disable sync for cc and addresses * Set pref visibility based on feature flags * Helper function for pin warning preferences and set default values for cc and addresses to false. * Kdocs for relevant sync functions in account settings * Default visibility to false for credit cards and addresses in account settings --- .../account/AccountSettingsFragment.kt | 124 ++++++++++++++---- .../creditcards/CreditCardsSettingFragment.kt | 1 - app/src/main/res/values/preference_keys.xml | 4 + app/src/main/res/values/strings.xml | 4 + .../res/xml/account_settings_preferences.xml | 13 ++ 5 files changed, 116 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt index 655d76e659..9122e47cbb 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt @@ -35,6 +35,7 @@ import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.service.fxa.sync.getLastSynced import mozilla.components.support.ktx.android.content.getColorFromAttr +import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.StoreProvider @@ -164,22 +165,22 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { updateSyncEngineStates() setDisabledWhileSyncing(accountManager.isSyncActive()) - fun updateSyncEngineState(context: Context, engine: SyncEngine, newState: Boolean) { - SyncEnginesStorage(context).setStatus(engine, newState) - viewLifecycleOwner.lifecycleScope.launch { - context.components.backgroundServices.accountManager.syncNow(SyncReason.EngineChange) - } - } - fun SyncEngine.prefId(): Int = when (this) { SyncEngine.History -> R.string.pref_key_sync_history SyncEngine.Bookmarks -> R.string.pref_key_sync_bookmarks SyncEngine.Passwords -> R.string.pref_key_sync_logins SyncEngine.Tabs -> R.string.pref_key_sync_tabs + SyncEngine.CreditCards -> R.string.pref_key_sync_credit_cards + SyncEngine.Addresses -> R.string.pref_key_sync_address else -> throw IllegalStateException("Accessing internal sync engines") } - listOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Tabs).forEach { + listOf( + SyncEngine.History, + SyncEngine.Bookmarks, + SyncEngine.Tabs, + SyncEngine.Addresses + ).forEach { requirePreference(it.prefId()).apply { setOnPreferenceChangeListener { _, newValue -> updateSyncEngineState(context, it, newValue as Boolean) @@ -188,19 +189,17 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { } } - // 'Passwords' listener is special, since we also display a pin protection warning. + // 'Passwords' and 'Credit card' listeners are special, since we also display a pin protection warning. requirePreference(SyncEngine.Passwords.prefId()).apply { setOnPreferenceChangeListener { _, newValue -> - val manager = - activity?.getSystemService(Context.KEYGUARD_SERVICE) as KeyguardManager - if (manager.isKeyguardSecure || - newValue == false || - !context.settings().shouldShowSecurityPinWarningSync - ) { - updateSyncEngineState(context, SyncEngine.Passwords, newValue as Boolean) - } else { - showPinDialogWarning(newValue as Boolean) - } + updateSyncEngineStateWithPinWarning(SyncEngine.Passwords, newValue as Boolean) + true + } + } + + requirePreference(SyncEngine.CreditCards.prefId()).apply { + setOnPreferenceChangeListener { _, newValue -> + updateSyncEngineStateWithPinWarning(SyncEngine.CreditCards, newValue as Boolean) true } } @@ -218,7 +217,57 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { ) } - private fun showPinDialogWarning(newValue: Boolean) { + /** + * Prompts the user if they do not have a password/pin set up to secure their device, and + * updates the state of the sync engine with the new checkbox value. + * + * Currently used for logins and credit cards. + * + * @param engine the sync engine whose preference has changed. + * @param newValue the value denoting whether or not to sync the specified preference. + */ + private fun CheckBoxPreference.updateSyncEngineStateWithPinWarning( + syncEngine: SyncEngine, + newValue: Boolean + ) { + val manager = activity?.getSystemService(Context.KEYGUARD_SERVICE) as KeyguardManager + + if (manager.isKeyguardSecure || + !newValue || + !requireContext().settings().shouldShowSecurityPinWarningSync + ) { + updateSyncEngineState(context, syncEngine, newValue) + } else { + showPinDialogWarning(syncEngine, newValue) + } + } + + /** + * Updates the sync engine status with the new state of the preference and triggers a sync + * event. + * + * @param engine the sync engine whose preference has changed. + * @param newValue the new value of the sync preference, where true indicates sync for that + * preference and false indicates not synced. + */ + private fun updateSyncEngineState(context: Context, engine: SyncEngine, newValue: Boolean) { + SyncEnginesStorage(context).setStatus(engine, newValue) + viewLifecycleOwner.lifecycleScope.launch { + context.components.backgroundServices.accountManager.syncNow(SyncReason.EngineChange) + } + } + + /** + * Creates and shows a warning dialog that prompts the user to create a pin/password to + * secure their device when none is detected. The user has the option to continue with + * updating their sync preferences (updates the [SyncEngine] state) or navigating to + * device security settings to create a pin/password. + * + * @param syncEngine the sync engine whose preference has changed. + * @param newValue the new value of the sync preference, where true indicates sync for that + * preference and false indicates not synced. + */ + private fun showPinDialogWarning(syncEngine: SyncEngine, newValue: Boolean) { context?.let { AlertDialog.Builder(it).apply { setTitle(getString(R.string.logins_warning_dialog_title)) @@ -227,11 +276,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { ) setNegativeButton(getString(R.string.logins_warning_dialog_later)) { _: DialogInterface, _ -> - SyncEnginesStorage(context).setStatus(SyncEngine.Passwords, newValue) - // Use fragment's lifecycle; the view may be gone by the time dialog is interacted with. - lifecycleScope.launch { - context.components.backgroundServices.accountManager.syncNow(SyncReason.EngineChange) - } + updateSyncEngineState(context, syncEngine, newValue) } setPositiveButton(getString(R.string.logins_warning_dialog_set_up_now)) { it: DialogInterface, _ -> @@ -247,12 +292,20 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { } } + /** + * Updates the status of all [SyncEngine] states. + */ private fun updateSyncEngineStates() { val syncEnginesStatus = SyncEnginesStorage(requireContext()).getStatus() requirePreference(R.string.pref_key_sync_bookmarks).apply { isEnabled = syncEnginesStatus.containsKey(SyncEngine.Bookmarks) isChecked = syncEnginesStatus.getOrElse(SyncEngine.Bookmarks) { true } } + requirePreference(R.string.pref_key_sync_credit_cards).apply { + isVisible = FeatureFlags.creditCardsFeature + isEnabled = syncEnginesStatus.containsKey(SyncEngine.CreditCards) + isChecked = syncEnginesStatus.getOrElse(SyncEngine.CreditCards) { true } + } requirePreference(R.string.pref_key_sync_history).apply { isEnabled = syncEnginesStatus.containsKey(SyncEngine.History) isChecked = syncEnginesStatus.getOrElse(SyncEngine.History) { true } @@ -265,8 +318,17 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { isEnabled = syncEnginesStatus.containsKey(SyncEngine.Tabs) isChecked = syncEnginesStatus.getOrElse(SyncEngine.Tabs) { true } } + requirePreference(R.string.pref_key_sync_address).apply { + isVisible = FeatureFlags.addressesFeature + isEnabled = syncEnginesStatus.containsKey(SyncEngine.Addresses) + isChecked = syncEnginesStatus.getOrElse(SyncEngine.Addresses) { true } + } } + /** + * Manual sync triggered by the user. This also checks account authentication and refreshes the + * device list. + */ private fun syncNow() { viewLifecycleOwner.lifecycleScope.launch { requireComponents.analytics.metrics.track(Event.SyncAccountSyncNow) @@ -281,8 +343,13 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { } } - private fun syncDeviceName(newValue: String): Boolean { - if (newValue.trim().isEmpty()) { + /** + * Takes a non-empty value and sets the device name. May fail due to authentication. + * + * @param newDeviceName the new name of the device. Cannot be an empty string. + */ + private fun syncDeviceName(newDeviceName: String): Boolean { + if (newDeviceName.trim().isEmpty()) { return false } // This may fail, and we'll have a disparity in the UI until `updateDeviceName` is called. @@ -290,7 +357,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { context?.let { accountManager.authenticatedAccount() ?.deviceConstellation() - ?.setDeviceName(newValue, it) + ?.setDeviceName(newDeviceName, it) } } return true @@ -409,6 +476,5 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { companion object { private const val DEVICE_NAME_MAX_LENGTH = 128 - private const val DEVICE_NAME_EDIT_TEXT_MIN_HEIGHT_DP = 48 } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt index 42c3364f02..6c927d8762 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt @@ -98,7 +98,6 @@ class CreditCardsSettingFragment : PreferenceFragmentCompat() { ) } - @Suppress("MaxLineLength") override fun onPreferenceTreeClick(preference: Preference): Boolean { when (preference.key) { getPreferenceKey(R.string.pref_key_credit_cards_add_credit_card) -> { diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 6409596a5c..3a7ae1d35c 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -91,6 +91,10 @@ pref_key_fxa_has_synced_items pref_key_search_widget_installed pref_key_saved_logins_sorting_strategy + + pref_key_sync_credit_cards + + pref_key_sync_address pref_key_search_engine_list diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 4afbc624e3..79ec8d0803 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -402,6 +402,10 @@ The first parameter is the application name, the second is the device manufacturer name and the third is the device model. --> %1$s on %2$s %3$s + + Credit cards + + Addresses diff --git a/app/src/main/res/xml/account_settings_preferences.xml b/app/src/main/res/xml/account_settings_preferences.xml index 0e409fbc46..5aa6e74e16 100644 --- a/app/src/main/res/xml/account_settings_preferences.xml +++ b/app/src/main/res/xml/account_settings_preferences.xml @@ -29,6 +29,13 @@ android:layout="@layout/checkbox_left_preference" android:title="@string/preferences_sync_bookmarks" /> + + +