From 053e5b8065ba58ee0b26ac8ab618a23528a87670 Mon Sep 17 00:00:00 2001
From: ekager <emilykager@gmail.com>
Date: Thu, 16 Jan 2020 15:10:51 -0800
Subject: [PATCH] For #6396 For #6553 - Update sync engine checkboxes and send
 telemetry once

---
 .../fenix/settings/SettingsFragment.kt        | 10 +++++--
 .../account/AccountSettingsFragment.kt        | 28 +++++++++++--------
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt
index 81317ef34..3476a32e5 100644
--- a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt
@@ -72,7 +72,8 @@ class SettingsFragment : PreferenceFragmentCompat() {
             lifecycleScope.launch {
                 updateAccountUIState(
                     context = context,
-                    profile = profile ?: context.components.backgroundServices.accountManager.accountProfile()
+                    profile = profile
+                        ?: context.components.backgroundServices.accountManager.accountProfile()
                 )
             }
         }
@@ -107,7 +108,11 @@ class SettingsFragment : PreferenceFragmentCompat() {
                 try {
                     context?.let { context ->
                         context.components.analytics.metrics.track(
-                            Event.PreferenceToggled(key, sharedPreferences.getBoolean(key, false), context)
+                            Event.PreferenceToggled(
+                                key,
+                                sharedPreferences.getBoolean(key, false),
+                                context
+                            )
                         )
                     }
                 } catch (e: IllegalArgumentException) {
@@ -185,7 +190,6 @@ class SettingsFragment : PreferenceFragmentCompat() {
         findPreference<Preference>(getPreferenceKey(pref_key_passwords))?.apply {
             isVisible = FeatureFlags.logins
         }
-
         findPreference<PreferenceCategory>(getPreferenceKey(R.string.pref_key_advanced))?.apply {
             isVisible = FeatureFlags.fenixLanguagePicker
         }
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 0de6b84c2..02bc1875d 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
@@ -166,12 +166,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() {
         val historyNameKey = getPreferenceKey(R.string.pref_key_sync_history)
         findPreference<CheckBoxPreference>(historyNameKey)?.apply {
             setOnPreferenceChangeListener { _, newValue ->
-                requireComponents.analytics.metrics.track(Event.PreferenceToggled(
-                    preferenceKey = historyNameKey,
-                    enabled = newValue as Boolean,
-                    context = context
-                ))
-                SyncEnginesStorage(context).setStatus(SyncEngine.History, newValue)
+                SyncEnginesStorage(context).setStatus(SyncEngine.History, newValue as Boolean)
                 @Suppress("DeferredResultUnused")
                 context.components.backgroundServices.accountManager.syncNowAsync(SyncReason.EngineChange)
                 true
@@ -181,12 +176,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() {
         val bookmarksNameKey = getPreferenceKey(R.string.pref_key_sync_bookmarks)
         findPreference<CheckBoxPreference>(bookmarksNameKey)?.apply {
             setOnPreferenceChangeListener { _, newValue ->
-                requireComponents.analytics.metrics.track(Event.PreferenceToggled(
-                    preferenceKey = bookmarksNameKey,
-                    enabled = newValue as Boolean,
-                    context = context
-                ))
-                SyncEnginesStorage(context).setStatus(SyncEngine.Bookmarks, newValue)
+                SyncEnginesStorage(context).setStatus(SyncEngine.Bookmarks, newValue as Boolean)
                 @Suppress("DeferredResultUnused")
                 context.components.backgroundServices.accountManager.syncNowAsync(SyncReason.EngineChange)
                 true
@@ -327,6 +317,17 @@ class AccountSettingsFragment : PreferenceFragmentCompat() {
         }
     }
 
+    private fun setEnginesDisabledWhileSyncing(isSyncing: Boolean) {
+        listOf(
+            R.string.pref_key_sync_bookmarks,
+            R.string.pref_key_sync_history,
+            R.string.pref_key_sync_logins
+        )
+            .map { getPreferenceKey(it) }
+            .map { findPreference<CheckBoxPreference>(it) }
+            .forEach { it?.isEnabled = !isSyncing }
+    }
+
     private val syncStatusObserver = object : SyncStatusObserver {
         override fun onStarted() {
             lifecycleScope.launch {
@@ -334,6 +335,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() {
                 view?.announceForAccessibility(getString(R.string.sync_syncing_in_progress))
                 pref?.title = getString(R.string.sync_syncing_in_progress)
                 pref?.isEnabled = false
+                setEnginesDisabledWhileSyncing(true)
             }
         }
 
@@ -350,6 +352,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() {
                 }
                 // Make sure out sync engine checkboxes are up-to-date.
                 updateSyncEngineStates()
+                setEnginesDisabledWhileSyncing(false)
             }
         }
 
@@ -359,6 +362,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() {
                 val pref = findPreference<Preference>(getPreferenceKey(R.string.pref_key_sync_now))
                 pref?.let {
                     pref.title = getString(R.string.preferences_sync_now)
+                    // We want to only enable the sync button, and not the checkboxes here
                     pref.isEnabled = true
 
                     val failedTime = getLastSynced(requireContext())