From 2a8b39a12773e0bc1eb8d512598fe48f6b63f5ed Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Thu, 9 Mar 2023 00:26:18 -0500 Subject: [PATCH] Bug 1809998 - Part 1: Refactor CurrentMode to OnboardingAccountObserver --- .../org/mozilla/fenix/home/HomeFragment.kt | 23 +---- .../main/java/org/mozilla/fenix/home/Mode.kt | 42 --------- .../home/sessioncontrol/SessionControlView.kt | 2 +- .../onboarding/OnboardingAccountObserver.kt | 48 ++++++++++ .../mozilla/fenix/components/AppStoreTest.kt | 10 +- .../java/org/mozilla/fenix/home/ModeTest.kt | 92 +------------------ .../OnboardingAccountObserverTest.kt | 70 ++++++++++++++ 7 files changed, 126 insertions(+), 161 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/onboarding/OnboardingAccountObserver.kt create mode 100644 app/src/test/java/org/mozilla/fenix/onboarding/OnboardingAccountObserverTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index dd4acffea4..cfeb037f38 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -188,8 +188,6 @@ class HomeFragment : Fragment() { @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) internal var homeMenuView: HomeMenuView? = null - private lateinit var currentMode: CurrentMode - private var lastAppliedWallpaperName: String = Wallpaper.defaultName private val topSitesFeature = ViewBoundFeatureWrapper() @@ -233,14 +231,7 @@ class HomeFragment : Fragment() { val currentWallpaperName = requireContext().settings().currentWallpaperName applyWallpaper(wallpaperName = currentWallpaperName, orientationChange = false) - currentMode = CurrentMode( - requireContext(), - requireComponents.fenixOnboarding, - browsingModeManager, - ::dispatchModeChanges, - ) - - components.appStore.dispatch(AppAction.ModeChange(currentMode.getCurrentMode())) + components.appStore.dispatch(AppAction.ModeChange(Mode.fromBrowsingMode(browsingModeManager.mode))) lifecycleScope.launch(IO) { if (requireContext().settings().showPocketRecommendationsFeature) { @@ -723,10 +714,6 @@ class HomeFragment : Fragment() { return@runIfReadyOrQueue } - requireComponents.backgroundServices.accountManager.register( - currentMode, - owner = this@HomeFragment.viewLifecycleOwner, - ) requireComponents.backgroundServices.accountManager.register( object : AccountObserver { override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { @@ -764,12 +751,6 @@ class HomeFragment : Fragment() { } } - private fun dispatchModeChanges(mode: Mode) { - if (mode != Mode.fromBrowsingMode(browsingModeManager.mode)) { - requireContext().components.appStore.dispatch(AppAction.ModeChange(mode)) - } - } - @VisibleForTesting internal fun removeCollectionWithUndo(tabCollection: TabCollection) { val snackbarMessage = getString(R.string.snackbar_collection_deleted) @@ -877,7 +858,7 @@ class HomeFragment : Fragment() { requireComponents.fenixOnboarding.finish() requireContext().components.appStore.dispatch( AppAction.ModeChange( - mode = currentMode.getCurrentMode(), + mode = Mode.fromBrowsingMode(browsingModeManager.mode), ), ) } diff --git a/app/src/main/java/org/mozilla/fenix/home/Mode.kt b/app/src/main/java/org/mozilla/fenix/home/Mode.kt index bce9afde67..4674b0ff77 100644 --- a/app/src/main/java/org/mozilla/fenix/home/Mode.kt +++ b/app/src/main/java/org/mozilla/fenix/home/Mode.kt @@ -4,17 +4,7 @@ package org.mozilla.fenix.home -import android.content.Context -import mozilla.components.concept.sync.AccountObserver -import mozilla.components.concept.sync.AuthType -import mozilla.components.concept.sync.OAuthAccount -import mozilla.components.concept.sync.Profile import org.mozilla.fenix.browser.browsingmode.BrowsingMode -import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager -import org.mozilla.fenix.ext.components -import org.mozilla.fenix.onboarding.FenixOnboarding -import org.mozilla.fenix.onboarding.OnboardingState -import org.mozilla.fenix.nimbus.Onboarding as OnboardingConfig /** * Describes various states of the home fragment UI. @@ -22,7 +12,6 @@ import org.mozilla.fenix.nimbus.Onboarding as OnboardingConfig sealed class Mode { object Normal : Mode() object Private : Mode() - data class Onboarding(val state: OnboardingState, val config: OnboardingConfig) : Mode() companion object { fun fromBrowsingMode(browsingMode: BrowsingMode) = when (browsingMode) { @@ -31,34 +20,3 @@ sealed class Mode { } } } - -class CurrentMode( - private val context: Context, - private val onboarding: FenixOnboarding, - private val browsingModeManager: BrowsingModeManager, - private val dispatchModeChanges: (mode: Mode) -> Unit, -) : AccountObserver { - - private val accountManager by lazy { context.components.backgroundServices.accountManager } - private val settings by lazy { context.components.settings } - - fun getCurrentMode() = if (onboarding.userHasBeenOnboarded() || settings.junoOnboardingEnabled) { - Mode.fromBrowsingMode(browsingModeManager.mode) - } else { - val account = accountManager.authenticatedAccount() - if (account != null) { - Mode.Onboarding(OnboardingState.SignedIn, onboarding.config) - } else { - Mode.Onboarding(OnboardingState.SignedOutNoAutoSignIn, onboarding.config) - } - } - - fun emitModeChanges() { - dispatchModeChanges(getCurrentMode()) - } - - override fun onAuthenticated(account: OAuthAccount, authType: AuthType) = emitModeChanges() - override fun onAuthenticationProblems() = emitModeChanges() - override fun onLoggedOut() = emitModeChanges() - override fun onProfileUpdated(profile: Profile) = emitModeChanges() -} diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt index dacc571501..fc7cd66b22 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt @@ -129,6 +129,7 @@ private fun showCollections( private fun privateModeAdapterItems() = listOf(AdapterItem.PrivateBrowsingDescription) +@Suppress("UnusedPrivateMember") private fun onboardingAdapterItems( onboardingState: OnboardingState, onboardingConfig: OnboardingConfig, @@ -173,7 +174,6 @@ private fun AppState.toAdapterList(settings: Settings): List = when firstFrameDrawn, ) is Mode.Private -> privateModeAdapterItems() - is Mode.Onboarding -> onboardingAdapterItems(mode.state, mode.config) } private fun collectionTabItems(collection: TabCollection) = diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingAccountObserver.kt b/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingAccountObserver.kt new file mode 100644 index 0000000000..7ef6200b41 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingAccountObserver.kt @@ -0,0 +1,48 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.onboarding + +import android.content.Context +import androidx.annotation.VisibleForTesting +import mozilla.components.concept.sync.AccountObserver +import mozilla.components.concept.sync.AuthType +import mozilla.components.concept.sync.OAuthAccount +import mozilla.components.concept.sync.Profile +import org.mozilla.fenix.ext.components + +/** + * Observes various account-related events and dispatches the [OnboardingState] based on whether + * or not the account is authenticated. + */ +class OnboardingAccountObserver( + private val context: Context, + private val dispatchChanges: (state: OnboardingState) -> Unit, +) : AccountObserver { + + private val accountManager by lazy { context.components.backgroundServices.accountManager } + + /** + * Returns the current [OnboardingState] based on the account state. + */ + fun getOnboardingState(): OnboardingState { + val account = accountManager.authenticatedAccount() + + return if (account != null) { + OnboardingState.SignedIn + } else { + OnboardingState.SignedOutNoAutoSignIn + } + } + + @VisibleForTesting + internal fun emitChanges() { + dispatchChanges(getOnboardingState()) + } + + override fun onAuthenticated(account: OAuthAccount, authType: AuthType) = emitChanges() + override fun onAuthenticationProblems() = emitChanges() + override fun onLoggedOut() = emitChanges() + override fun onProfileUpdated(profile: Profile) = emitChanges() +} diff --git a/app/src/test/java/org/mozilla/fenix/components/AppStoreTest.kt b/app/src/test/java/org/mozilla/fenix/components/AppStoreTest.kt index 4af64d522b..f92ebe4163 100644 --- a/app/src/test/java/org/mozilla/fenix/components/AppStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/AppStoreTest.kt @@ -34,7 +34,6 @@ import org.mozilla.fenix.components.appstate.AppState import org.mozilla.fenix.components.appstate.filterOut import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getFilteredStories -import org.mozilla.fenix.home.CurrentMode import org.mozilla.fenix.home.Mode import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesSelectedCategory @@ -53,7 +52,6 @@ class AppStoreTest { private lateinit var accountManager: FxaAccountManager private lateinit var onboarding: FenixOnboarding private lateinit var browsingModeManager: BrowsingModeManager - private lateinit var currentMode: CurrentMode private lateinit var appState: AppState private lateinit var appStore: AppStore private lateinit var recentSyncedTabsList: List @@ -78,16 +76,10 @@ class AppStoreTest { every { onboarding.userHasBeenOnboarded() } returns true every { browsingModeManager.mode } returns BrowsingMode.Normal - currentMode = CurrentMode( - context, - onboarding, - browsingModeManager, - ) {} - appState = AppState( collections = emptyList(), expandedCollections = emptySet(), - mode = currentMode.getCurrentMode(), + mode = Mode.fromBrowsingMode(browsingModeManager.mode), topSites = emptyList(), showCollectionPlaceholder = true, recentTabs = emptyList(), diff --git a/app/src/test/java/org/mozilla/fenix/home/ModeTest.kt b/app/src/test/java/org/mozilla/fenix/home/ModeTest.kt index a14b40f65a..d169e038da 100644 --- a/app/src/test/java/org/mozilla/fenix/home/ModeTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/ModeTest.kt @@ -4,118 +4,34 @@ package org.mozilla.fenix.home -import android.content.Context import io.mockk.every import io.mockk.mockk -import io.mockk.spyk -import io.mockk.verify -import mozilla.components.service.fxa.manager.FxaAccountManager import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager -import org.mozilla.fenix.ext.components -import org.mozilla.fenix.onboarding.FenixOnboarding -import org.mozilla.fenix.onboarding.OnboardingState class ModeTest { - private lateinit var context: Context - private lateinit var accountManager: FxaAccountManager - private lateinit var onboarding: FenixOnboarding private lateinit var browsingModeManager: BrowsingModeManager - private lateinit var currentMode: CurrentMode - private lateinit var dispatchModeChanges: (mode: Mode) -> Unit - private var dispatchModeChangesResult: Mode? = null @Before fun setup() { - context = mockk(relaxed = true) - accountManager = mockk(relaxed = true) - onboarding = mockk(relaxed = true) browsingModeManager = mockk(relaxed = true) - - dispatchModeChangesResult = null - dispatchModeChanges = { - dispatchModeChangesResult = it - } - - every { context.components.backgroundServices.accountManager } returns accountManager - - currentMode = CurrentMode( - context, - onboarding, - browsingModeManager, - dispatchModeChanges, - ) } @Test - fun `get current mode after onboarding`() { - every { onboarding.userHasBeenOnboarded() } returns true + fun `WHEN browsing mode is normal THEN return normal mode`() { every { browsingModeManager.mode } returns BrowsingMode.Normal - assertEquals(Mode.Normal, currentMode.getCurrentMode()) + assertEquals(Mode.Normal, Mode.fromBrowsingMode(browsingModeManager.mode)) } @Test - fun `get current private mode after onboarding`() { - every { onboarding.userHasBeenOnboarded() } returns true + fun `WHEN browsing mode is private THEN return private mode`() { every { browsingModeManager.mode } returns BrowsingMode.Private - assertEquals(Mode.Private, currentMode.getCurrentMode()) - } - - @Test - fun `get current browsing mode when userHasBeenOnboarded is false and juno onboarding is enabled`() { - every { onboarding.userHasBeenOnboarded() } returns false - every { context.components.settings.junoOnboardingEnabled } returns true - every { browsingModeManager.mode } returns BrowsingMode.Normal - - assertEquals(Mode.Normal, currentMode.getCurrentMode()) - } - - @Test - fun `get current onboarding mode when signed in`() { - every { onboarding.userHasBeenOnboarded() } returns false - every { accountManager.authenticatedAccount() } returns mockk() - - assertEquals(Mode.Onboarding(OnboardingState.SignedIn, onboarding.config), currentMode.getCurrentMode()) - } - - @Test - fun `get current onboarding mode when signed out`() { - every { onboarding.userHasBeenOnboarded() } returns false - every { accountManager.authenticatedAccount() } returns null - - assertEquals(Mode.Onboarding(OnboardingState.SignedOutNoAutoSignIn, onboarding.config), currentMode.getCurrentMode()) - } - - @Test - fun `emit mode change`() { - every { onboarding.userHasBeenOnboarded() } returns true - every { browsingModeManager.mode } returns BrowsingMode.Normal - - currentMode.emitModeChanges() - - assertEquals(Mode.Normal, dispatchModeChangesResult) - } - - @Test - fun `account observer calls emitModeChanges`() { - val spy = spyk(currentMode) - - spy.onAuthenticated(mockk(), mockk()) - verify { spy.emitModeChanges() } - - spy.onAuthenticationProblems() - verify { spy.emitModeChanges() } - - spy.onLoggedOut() - verify { spy.emitModeChanges() } - - spy.onProfileUpdated(mockk()) - verify { spy.emitModeChanges() } + assertEquals(Mode.Private, Mode.fromBrowsingMode(browsingModeManager.mode)) } } diff --git a/app/src/test/java/org/mozilla/fenix/onboarding/OnboardingAccountObserverTest.kt b/app/src/test/java/org/mozilla/fenix/onboarding/OnboardingAccountObserverTest.kt new file mode 100644 index 0000000000..dd20c25a28 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/onboarding/OnboardingAccountObserverTest.kt @@ -0,0 +1,70 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.onboarding + +import android.content.Context +import io.mockk.every +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import mozilla.components.service.fxa.manager.FxaAccountManager +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.mozilla.fenix.ext.components + +class OnboardingAccountObserverTest { + + private lateinit var context: Context + private lateinit var accountManager: FxaAccountManager + private lateinit var observer: OnboardingAccountObserver + private lateinit var dispatchChanges: (mode: OnboardingState) -> Unit + + private var dispatchChangesResult: OnboardingState? = null + + @Before + fun setup() { + context = mockk(relaxed = true) + accountManager = mockk(relaxed = true) + + dispatchChangesResult = null + dispatchChanges = { + dispatchChangesResult = it + } + + every { context.components.backgroundServices.accountManager } returns accountManager + + observer = spyk( + OnboardingAccountObserver( + context = context, + dispatchChanges = dispatchChanges, + ), + ) + } + + @Test + fun `WHEN account is authenticated THEN return signed in state`() { + every { accountManager.authenticatedAccount() } returns mockk() + + assertEquals(OnboardingState.SignedIn, observer.getOnboardingState()) + } + + @Test + fun `WHEN account is not authenticated THEN return signed out state`() { + every { accountManager.authenticatedAccount() } returns null + + assertEquals(OnboardingState.SignedOutNoAutoSignIn, observer.getOnboardingState()) + } + + @Test + fun `WHEN account observer receives a state change THEN emit state changes`() { + observer.onAuthenticated(mockk(), mockk()) + observer.onAuthenticationProblems() + observer.onLoggedOut() + observer.onProfileUpdated(mockk()) + + verify(exactly = 4) { observer.emitChanges() } + } +}