From cb7701584f3aebf3b9366d107bbc782667362b16 Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Mon, 23 Sep 2019 09:33:55 -0700 Subject: [PATCH] No issue: Extract home fragment mode (#5343) --- .../org/mozilla/fenix/home/HomeFragment.kt | 104 ++++++--------- .../main/java/org/mozilla/fenix/home/Mode.kt | 81 ++++++++++++ .../sessioncontrol/SessionControlAdapter.kt | 1 + .../sessioncontrol/SessionControlComponent.kt | 28 +--- .../sessioncontrol/SessionControlUIView.kt | 2 + .../java/org/mozilla/fenix/home/ModeTest.kt | 123 ++++++++++++++++++ 6 files changed, 245 insertions(+), 94 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/home/Mode.kt create mode 100644 app/src/test/java/org/mozilla/fenix/home/ModeTest.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 cdb536ce9..b760cc1d3 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -5,7 +5,6 @@ package org.mozilla.fenix.home import android.animation.Animator -import android.content.Context import android.content.DialogInterface import android.graphics.drawable.BitmapDrawable import android.os.Bundle @@ -49,7 +48,6 @@ import mozilla.components.browser.session.SessionManager 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 mozilla.components.feature.media.ext.getSession import mozilla.components.feature.media.ext.pauseIfPlaying import mozilla.components.feature.media.ext.playIfPaused @@ -72,17 +70,15 @@ import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.PrivateShortcutCreateManager import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.sessionsOfType import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.toTab import org.mozilla.fenix.home.sessioncontrol.CollectionAction -import org.mozilla.fenix.home.sessioncontrol.Mode import org.mozilla.fenix.home.sessioncontrol.OnboardingAction -import org.mozilla.fenix.home.sessioncontrol.OnboardingState import org.mozilla.fenix.home.sessioncontrol.SessionControlAction import org.mozilla.fenix.home.sessioncontrol.SessionControlChange import org.mozilla.fenix.home.sessioncontrol.SessionControlComponent @@ -103,7 +99,7 @@ import org.mozilla.fenix.utils.allowUndo import org.mozilla.fenix.whatsnew.WhatsNew @SuppressWarnings("TooManyFunctions", "LargeClass") -class HomeFragment : Fragment(), AccountObserver { +class HomeFragment : Fragment() { private val bus = ActionBusFactory.get(this) @@ -141,6 +137,7 @@ class HomeFragment : Fragment(), AccountObserver { private val onboarding by lazy { FenixOnboarding(requireContext()) } private lateinit var sessionControlComponent: SessionControlComponent + private lateinit var currentMode: CurrentMode override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -167,7 +164,12 @@ class HomeFragment : Fragment(), AccountObserver { ): View? { val view = inflater.inflate(R.layout.fragment_home, container, false) - val mode = currentMode(view.context) + currentMode = CurrentMode( + view.context, + onboarding, + browsingModeManager, + getManagedEmitter() + ) sessionControlComponent = SessionControlComponent( view.homeLayout, @@ -178,10 +180,10 @@ class HomeFragment : Fragment(), AccountObserver { ) { SessionControlViewModel( SessionControlState( - listOf(), - setOf(), + emptyList(), + emptySet(), requireComponents.core.tabCollectionStorage.cachedTabCollections, - mode + currentMode.getCurrentMode() ) ) } @@ -304,17 +306,28 @@ class HomeFragment : Fragment(), AccountObserver { getManagedEmitter().onNext( SessionControlChange.Change( tabs = getListOfSessions().toTabs(), - mode = currentMode(context), + mode = currentMode.getCurrentMode(), collections = components.core.tabCollectionStorage.cachedTabCollections ) ) (activity as AppCompatActivity).supportActionBar?.hide() - components.backgroundServices.accountManager.register(this, owner = this) - if (context.settings.showPrivateModeContextualFeatureRecommender && - browsingModeManager.mode.isPrivate - ) { + requireComponents.backgroundServices.accountManager.register(currentMode, owner = this) + requireComponents.backgroundServices.accountManager.register(object : AccountObserver { + override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { + if (authType != AuthType.Existing) { + view?.let { + FenixSnackbar.make(it, Snackbar.LENGTH_SHORT).setText( + it.context.getString(R.string.onboarding_firefox_account_sync_is_on) + ).show() + } + } + } + }, owner = this) + + if (context.settings().showPrivateModeContextualFeatureRecommender && + browsingModeManager.mode.isPrivate) { recommendPrivateBrowsingShortcut() } } @@ -330,14 +343,8 @@ class HomeFragment : Fragment(), AccountObserver { private fun handleOnboardingAction(action: OnboardingAction) { Do exhaustive when (action) { is OnboardingAction.Finish -> { - onboarding.finish() homeLayout?.progress = 0F - val mode = currentMode(requireContext()) - getManagedEmitter().onNext( - SessionControlChange.ModeChange( - mode - ) - ) + hideOnboarding() } } } @@ -346,7 +353,7 @@ class HomeFragment : Fragment(), AccountObserver { private fun handleTabAction(action: TabAction) { Do exhaustive when (action) { is TabAction.SaveTabGroup -> { - if ((activity as HomeActivity).browsingModeManager.mode.isPrivate) return + if (browsingModeManager.mode.isPrivate) return invokePendingDeleteJobs() saveTabToCollection(action.selectedTabSessionId) } @@ -429,7 +436,7 @@ class HomeFragment : Fragment(), AccountObserver { is TabAction.ShareTabs -> { invokePendingDeleteJobs() val shareTabs = sessionManager - .sessionsOfType(private = (activity as HomeActivity).browsingModeManager.mode.isPrivate) + .sessionsOfType(private = browsingModeManager.mode.isPrivate) .map { ShareTab(it.url, it.title) } .toList() share(tabs = shareTabs) @@ -616,10 +623,12 @@ class HomeFragment : Fragment(), AccountObserver { } private fun hideOnboardingIfNeeded() { - if (!onboarding.userHasBeenOnboarded()) { - onboarding.finish() - emitModeChanges() - } + if (!onboarding.userHasBeenOnboarded()) hideOnboarding() + } + + private fun hideOnboarding() { + onboarding.finish() + currentMode.emitModeChanges() } private fun setupHomeMenu() { @@ -811,45 +820,6 @@ class HomeFragment : Fragment(), AccountObserver { nav(R.id.homeFragment, directions) } - private fun currentMode(context: Context): Mode = if (!onboarding.userHasBeenOnboarded()) { - val accountManager = requireComponents.backgroundServices.accountManager - val account = accountManager.authenticatedAccount() - if (account != null) { - Mode.Onboarding(OnboardingState.SignedIn) - } else { - val availableAccounts = accountManager.shareableAccounts(context) - if (availableAccounts.isEmpty()) { - Mode.Onboarding(OnboardingState.SignedOutNoAutoSignIn) - } else { - Mode.Onboarding(OnboardingState.SignedOutCanAutoSignIn(availableAccounts[0])) - } - } - } else { - Mode.fromBrowsingMode(browsingModeManager.mode) - } - - private fun emitModeChanges() { - context?.let { - val mode = currentMode(it) - getManagedEmitter().onNext(SessionControlChange.ModeChange(mode)) - } - } - - override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { - if (authType != AuthType.Existing) { - view?.let { - FenixSnackbar.make(it, Snackbar.LENGTH_SHORT).setText( - it.context.getString(R.string.onboarding_firefox_account_sync_is_on) - ).show() - } - } - emitModeChanges() - } - - override fun onAuthenticationProblems() = emitModeChanges() - override fun onLoggedOut() = emitModeChanges() - override fun onProfileUpdated(profile: Profile) = emitModeChanges() - private fun scrollAndAnimateCollection( tabsAddedToCollectionSize: Int, changedCollection: TabCollection? = null diff --git a/app/src/main/java/org/mozilla/fenix/home/Mode.kt b/app/src/main/java/org/mozilla/fenix/home/Mode.kt new file mode 100644 index 000000000..a20dd82dd --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/home/Mode.kt @@ -0,0 +1,81 @@ +/* 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.home + +import android.content.Context +import io.reactivex.Observer +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 mozilla.components.service.fxa.sharing.ShareableAccount +import org.mozilla.fenix.browser.browsingmode.BrowsingMode +import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager +import org.mozilla.fenix.ext.components +import org.mozilla.fenix.home.sessioncontrol.SessionControlChange +import org.mozilla.fenix.onboarding.FenixOnboarding + +/** + * Describes various states of the home fragment UI. + */ +sealed class Mode { + object Normal : Mode() + object Private : Mode() + data class Onboarding(val state: OnboardingState) : Mode() + + companion object { + fun fromBrowsingMode(browsingMode: BrowsingMode) = when (browsingMode) { + BrowsingMode.Normal -> Normal + BrowsingMode.Private -> Private + } + } +} + +/** + * Describes various onboarding states. + */ +sealed class OnboardingState { + // Signed out, without an option to auto-login using a shared FxA account. + object SignedOutNoAutoSignIn : OnboardingState() + // Signed out, with an option to auto-login into a shared FxA account. + data class SignedOutCanAutoSignIn(val withAccount: ShareableAccount) : OnboardingState() + // Signed in. + object SignedIn : OnboardingState() +} + +class CurrentMode( + private val context: Context, + private val onboarding: FenixOnboarding, + private val browsingModeManager: BrowsingModeManager, + private val emitter: Observer +) : AccountObserver { + + private val accountManager = context.components.backgroundServices.accountManager + + fun getCurrentMode() = if (onboarding.userHasBeenOnboarded()) { + Mode.fromBrowsingMode(browsingModeManager.mode) + } else { + val account = accountManager.authenticatedAccount() + if (account != null) { + Mode.Onboarding(OnboardingState.SignedIn) + } else { + val availableAccounts = accountManager.shareableAccounts(context) + if (availableAccounts.isEmpty()) { + Mode.Onboarding(OnboardingState.SignedOutNoAutoSignIn) + } else { + Mode.Onboarding(OnboardingState.SignedOutCanAutoSignIn(availableAccounts[0])) + } + } + } + + fun emitModeChanges() { + emitter.onNext(SessionControlChange.ModeChange(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/SessionControlAdapter.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt index 03c41f2c5..6cf9999c2 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt @@ -17,6 +17,7 @@ import androidx.recyclerview.widget.RecyclerView import io.reactivex.Observer import kotlinx.android.synthetic.main.tab_list_row.* import mozilla.components.feature.media.state.MediaState +import org.mozilla.fenix.home.OnboardingState import org.mozilla.fenix.home.sessioncontrol.viewholders.CollectionHeaderViewHolder import org.mozilla.fenix.home.sessioncontrol.viewholders.CollectionViewHolder import org.mozilla.fenix.home.sessioncontrol.viewholders.NoContentMessageViewHolder diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlComponent.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlComponent.kt index 070c9c69b..25b563fc9 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlComponent.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlComponent.kt @@ -11,9 +11,8 @@ import androidx.recyclerview.widget.RecyclerView import io.reactivex.Observer import mozilla.components.browser.session.Session import mozilla.components.feature.media.state.MediaState -import mozilla.components.service.fxa.sharing.ShareableAccount -import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.ext.components +import org.mozilla.fenix.home.Mode import org.mozilla.fenix.mvi.Action import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.Change @@ -64,31 +63,6 @@ fun List.toSessionBundle(context: Context): MutableList { return sessionBundle } -/** - * Describes various onboarding states. - */ -sealed class OnboardingState { - // Signed out, without an option to auto-login using a shared FxA account. - object SignedOutNoAutoSignIn : OnboardingState() - // Signed out, with an option to auto-login into a shared FxA account. - data class SignedOutCanAutoSignIn(val withAccount: ShareableAccount) : OnboardingState() - // Signed in. - object SignedIn : OnboardingState() -} - -sealed class Mode { - object Normal : Mode() - object Private : Mode() - data class Onboarding(val state: OnboardingState) : Mode() - - companion object { - fun fromBrowsingMode(browsingMode: BrowsingMode) = when (browsingMode) { - BrowsingMode.Normal -> Normal - BrowsingMode.Private -> Private - } - } -} - data class SessionControlState( val tabs: List, val expandedCollections: Set, diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlUIView.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlUIView.kt index 3493fe3aa..679a5bda6 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlUIView.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlUIView.kt @@ -13,6 +13,8 @@ import io.reactivex.Observable import io.reactivex.Observer import io.reactivex.functions.Consumer import org.mozilla.fenix.R +import org.mozilla.fenix.home.Mode +import org.mozilla.fenix.home.OnboardingState import org.mozilla.fenix.mvi.UIView val noTabMessage = AdapterItem.NoContentMessage( diff --git a/app/src/test/java/org/mozilla/fenix/home/ModeTest.kt b/app/src/test/java/org/mozilla/fenix/home/ModeTest.kt new file mode 100644 index 000000000..2c8d86405 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/home/ModeTest.kt @@ -0,0 +1,123 @@ +/* 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.home + +import android.content.Context +import io.mockk.every +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import io.reactivex.Observer +import mozilla.components.service.fxa.manager.FxaAccountManager +import mozilla.components.service.fxa.sharing.ShareableAccount +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.home.sessioncontrol.SessionControlChange +import org.mozilla.fenix.onboarding.FenixOnboarding + +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 emitter: Observer + private lateinit var currentMode: CurrentMode + + @Before + fun setup() { + context = mockk(relaxed = true) + accountManager = mockk(relaxed = true) + onboarding = mockk(relaxed = true) + browsingModeManager = mockk(relaxed = true) + emitter = mockk(relaxed = true) + + every { context.components.backgroundServices.accountManager } returns accountManager + + currentMode = CurrentMode( + context, + onboarding, + browsingModeManager, + emitter + ) + } + + @Test + fun `get current mode after onboarding`() { + every { onboarding.userHasBeenOnboarded() } returns true + every { browsingModeManager.mode } returns BrowsingMode.Normal + + assertEquals(Mode.Normal, currentMode.getCurrentMode()) + } + + @Test + fun `get current private mode after onboarding`() { + every { onboarding.userHasBeenOnboarded() } returns true + every { browsingModeManager.mode } returns BrowsingMode.Private + + assertEquals(Mode.Private, 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), currentMode.getCurrentMode()) + } + + @Test + fun `get current onboarding mode when signed out`() { + every { onboarding.userHasBeenOnboarded() } returns false + every { accountManager.authenticatedAccount() } returns null + every { accountManager.shareableAccounts(context) } returns emptyList() + + assertEquals(Mode.Onboarding(OnboardingState.SignedOutNoAutoSignIn), currentMode.getCurrentMode()) + } + + @Test + fun `get current onboarding mode when can auto sign in`() { + val shareableAccount: ShareableAccount = mockk() + every { onboarding.userHasBeenOnboarded() } returns false + every { accountManager.authenticatedAccount() } returns null + every { accountManager.shareableAccounts(context) } returns listOf(shareableAccount) + + assertEquals( + Mode.Onboarding(OnboardingState.SignedOutCanAutoSignIn(shareableAccount)), + currentMode.getCurrentMode() + ) + } + + @Test + fun `emit mode change`() { + every { onboarding.userHasBeenOnboarded() } returns true + every { browsingModeManager.mode } returns BrowsingMode.Normal + + currentMode.emitModeChanges() + + verify { emitter.onNext(SessionControlChange.ModeChange(Mode.Normal)) } + } + + @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() } + } +}