From 69434a765f8e9b7dfa18b45ed205c229085d481e Mon Sep 17 00:00:00 2001 From: Yeon Taek Jeong Date: Thu, 8 Aug 2019 16:39:23 -0700 Subject: [PATCH] Fix most issues --- .../fenix/components/BackgroundServices.kt | 4 +- .../account/AccountSettingsFragment.kt | 41 +++++++------------ .../account/AccountSettingsInteractor.kt | 7 +++- .../fenix/settings/account/AccountStore.kt | 4 +- 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt index 716e73e05..6a0c2a7f9 100644 --- a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt +++ b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt @@ -55,7 +55,7 @@ class BackgroundServices( const val REDIRECT_URL = "https://accounts.firefox.com/oauth/success/$CLIENT_ID" } - private val defaultDeviceName = context.getString( + fun defaultDeviceName(context: Context): String = context.getString( R.string.default_device_name, context.getString(R.string.app_name), Build.MANUFACTURER, @@ -64,7 +64,7 @@ class BackgroundServices( private val serverConfig = ServerConfig.release(CLIENT_ID, REDIRECT_URL) private val deviceConfig = DeviceConfig( - name = defaultDeviceName, + name = defaultDeviceName(context), type = DeviceType.MOBILE, // NB: flipping this flag back and worth is currently not well supported and may need hand-holding. 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 cceb06ea7..22d00862a 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 @@ -13,18 +13,15 @@ import androidx.navigation.fragment.findNavController import androidx.preference.EditTextPreference import androidx.preference.Preference import androidx.preference.PreferenceFragmentCompat -import kotlinx.coroutines.Dispatchers.IO +import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.launch import mozilla.components.concept.sync.AccountObserver import mozilla.components.concept.sync.ConstellationState import mozilla.components.concept.sync.DeviceConstellationObserver -import mozilla.components.lib.state.ext.observe -import mozilla.components.service.fxa.FxaException -import mozilla.components.service.fxa.FxaPanicException +import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.service.fxa.sync.getLastSynced -import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.StoreProvider @@ -86,16 +83,14 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { LastSyncTime.Never else LastSyncTime.Success(getLastSynced(requireContext())), - deviceName = "" + deviceName = requireComponents.backgroundServices.defaultDeviceName(requireContext()) ) ) } - accountSettingsStore.observe(this) { - viewLifecycleOwner.lifecycleScope.launch { - updateLastSyncTimePref(it) - updateDeviceName(it) - } + consumeFrom(accountSettingsStore) { + updateLastSyncTimePref(it) + updateDeviceName(it) } accountManager = requireComponents.backgroundServices.accountManager @@ -103,8 +98,8 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { accountSettingsInteractor = AccountSettingsInteractor( findNavController(), - ::onSyncNow, - ::makeSnackbar, + ::syncNow, + ::checkValidName, ::syncDeviceName, accountSettingsStore ) @@ -153,7 +148,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { ) } - private fun onSyncNow() { + private fun syncNow() { lifecycleScope.launch { requireComponents.analytics.metrics.track(Event.SyncAccountSyncNow) // Trigger a sync. @@ -166,7 +161,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { } } - private fun makeSnackbar(newValue: String): Boolean { + private fun checkValidName(newValue: String): Boolean { // The network request requires a nonempty string, so don't persist any changes if the user inputs one. if (newValue.trim().isEmpty()) { FenixSnackbar.make(view!!, FenixSnackbar.LENGTH_LONG) @@ -179,17 +174,11 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { private fun syncDeviceName(newValue: String) { // This may fail, and we'll have a disparity in the UI until `updateDeviceName` is called. - lifecycleScope.launch(IO) { - try { - accountManager.authenticatedAccount() - ?.deviceConstellation() - ?.setDeviceNameAsync(newValue) - ?.await() - } catch (e: FxaPanicException) { - throw e - } catch (e: FxaException) { - Logger.error("Setting device name failed.", e) - } + lifecycleScope.launch(Main) { + accountManager.authenticatedAccount() + ?.deviceConstellation() + ?.setDeviceNameAsync(newValue) + ?.await() } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsInteractor.kt b/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsInteractor.kt index 977c91440..aaa8f9e3d 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsInteractor.kt @@ -45,7 +45,12 @@ class AccountSettingsInteractor( if (!isValidName) { return false } - // Optimistically set the device name to what user requested. + // Our "change the device name on the server" operation may fail. + // Currently, we disregard this failure and pretend we succeeded. + // At the same time, when user changes the device name, we immediately update the UI to display the new name. + // So, in case of a network (or other) failure when talking to the server, + // we'll have a discrepancy - the UI will reflect new value, but actually the value never changed. + // So, when user presses "sync now", we'll fetch the old value, and reset the UI. store.dispatch(AccountSettingsAction.UpdateDeviceName(newDeviceName)) setDeviceName.invoke(newDeviceName) diff --git a/app/src/main/java/org/mozilla/fenix/settings/account/AccountStore.kt b/app/src/main/java/org/mozilla/fenix/settings/account/AccountStore.kt index e0bd246d0..291b5973e 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/account/AccountStore.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/account/AccountStore.kt @@ -28,8 +28,8 @@ sealed class LastSyncTime { * The state for the Account Settings Screen */ data class AccountSettingsState( - val lastSyncedDate: LastSyncTime, - val deviceName: String + val lastSyncedDate: LastSyncTime = LastSyncTime.Never, + val deviceName: String = "" ) : State /**