From db76b8fe21389cf907a12da172e5d9ca545f5787 Mon Sep 17 00:00:00 2001 From: Elise Richards Date: Wed, 21 Apr 2021 14:13:44 -0500 Subject: [PATCH] For #19114: check state of sync account when navigating from sync sign in menu item (#19118) --- .../java/org/mozilla/fenix/ui/SmokeTest.kt | 5 +- .../fenix/ui/robots/SyncSignInRobot.kt | 6 ++ .../accounts/FenixAccountManager.kt | 33 ++++++ .../toolbar/BrowserToolbarMenuController.kt | 7 +- .../components/toolbar/DefaultToolbarMenu.kt | 12 +-- .../fenix/components/toolbar/ToolbarMenu.kt | 2 +- .../org/mozilla/fenix/home/HomeFragment.kt | 10 +- .../java/org/mozilla/fenix/home/HomeMenu.kt | 67 ++++++------ .../accounts/FenixAccountManagerTest.kt | 101 ++++++++++++++++++ ...DefaultBrowserToolbarMenuControllerTest.kt | 22 ++++ 10 files changed, 216 insertions(+), 49 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/components/accounts/FenixAccountManager.kt create mode 100644 app/src/test/java/org/mozilla/fenix/components/accounts/FenixAccountManagerTest.kt diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt index cf63c78062..473e9a8fe1 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt @@ -227,13 +227,14 @@ class SmokeTest { } @Test - // Verifies the Synced tabs menu opens from a tab's 3 dot menu + // Verifies the Synced tabs menu or Sync Sign In menu opens from a tab's 3 dot menu. + // The test is assuming we are NOT signed in. fun openMainMenuSyncItemTest() { if (FeatureFlags.tabsTrayRewrite) { homeScreen { }.openThreeDotMenu { }.openSyncSignIn { - verifyAccountSettingsMenuHeader() + verifySyncSignInMenuHeader() } } else { homeScreen { diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SyncSignInRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SyncSignInRobot.kt index e355174a5f..ac7132ea77 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SyncSignInRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SyncSignInRobot.kt @@ -22,6 +22,7 @@ import org.mozilla.fenix.helpers.click class SyncSignInRobot { fun verifyAccountSettingsMenuHeader() = assertAccountSettingsMenuHeader() + fun verifySyncSignInMenuHeader() = assertSyncSignInMenuHeader() class Transition { val mDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())!! @@ -44,3 +45,8 @@ private fun assertAccountSettingsMenuHeader() { onView(withText(R.string.preferences_account_settings)) .check((matches(withEffectiveVisibility(Visibility.VISIBLE)))) } + +private fun assertSyncSignInMenuHeader() { + onView(withText(R.string.sign_in_with_camera)) + .check((matches(withEffectiveVisibility(Visibility.VISIBLE)))) +} diff --git a/app/src/main/java/org/mozilla/fenix/components/accounts/FenixAccountManager.kt b/app/src/main/java/org/mozilla/fenix/components/accounts/FenixAccountManager.kt new file mode 100644 index 0000000000..fe5397bd1d --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/components/accounts/FenixAccountManager.kt @@ -0,0 +1,33 @@ +/* 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.components.accounts + +import android.content.Context +import mozilla.components.service.fxa.manager.FxaAccountManager +import org.mozilla.fenix.ext.components + +/** + * Component which holds a reference to [FxaAccountManager]. Manages account authentication, + * profiles, and profile state observers. + */ +open class FenixAccountManager(context: Context) { + val accountManager = context.components.backgroundServices.accountManager + + val authenticatedAccount + get() = accountManager.authenticatedAccount() != null + + val accountProfileEmail + get() = accountManager.accountProfile()?.email + + /** + * Check if the current account is signed in and authenticated. + */ + fun signedInToFxa(): Boolean { + val account = accountManager.authenticatedAccount() + val needsReauth = accountManager.accountNeedsReauth() + + return account != null && !needsReauth + } +} diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt index fee10c3789..8f7fdd0f06 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt @@ -226,10 +226,15 @@ class DefaultBrowserToolbarMenuController( ) } is ToolbarMenu.Item.SyncAccount -> { + val directions = if (item.signedIn) { + BrowserFragmentDirections.actionGlobalAccountSettingsFragment() + } else { + BrowserFragmentDirections.actionGlobalTurnOnSync() + } browserAnimator.captureEngineViewAndDrawStatically { navController.nav( R.id.browserFragment, - BrowserFragmentDirections.actionGlobalAccountSettingsFragment() + directions ) } } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt index 21ab046664..405fbfbf83 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt @@ -39,6 +39,7 @@ import org.mozilla.fenix.FeatureFlags.tabsTrayRewrite import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode +import org.mozilla.fenix.components.accounts.FenixAccountManager import org.mozilla.fenix.experiments.ExperimentBranch import org.mozilla.fenix.experiments.Experiments import org.mozilla.fenix.ext.asActivity @@ -74,12 +75,11 @@ open class DefaultToolbarMenu( private val shouldDeleteDataOnQuit = context.settings().shouldDeleteBrowsingDataOnQuit private val shouldUseBottomToolbar = context.settings().shouldUseBottomToolbar + private val accountManager = FenixAccountManager(context) private val selectedSession: TabSessionState? get() = store.state.selectedTab - private val accountManager = context.components.backgroundServices.accountManager - override val menuBuilder by lazy { WebExtensionBrowserMenuBuilder( items = @@ -534,10 +534,10 @@ open class DefaultToolbarMenu( } private fun getSyncItemTitle(): String { - val authenticatedAccount = accountManager.authenticatedAccount() != null - val email = accountManager.accountProfile()?.email + val authenticatedAccount = accountManager.authenticatedAccount + val email = accountManager.accountProfileEmail - return if (authenticatedAccount && email != null) { + return if (authenticatedAccount && !email.isNullOrEmpty()) { email } else { context.getString(R.string.sync_menu_sign_in) @@ -549,7 +549,7 @@ open class DefaultToolbarMenu( R.drawable.ic_synced_tabs, primaryTextColor() ) { - onItemTapped.invoke(ToolbarMenu.Item.SyncAccount) + onItemTapped.invoke(ToolbarMenu.Item.SyncAccount(accountManager.signedInToFxa())) } @VisibleForTesting(otherwise = PRIVATE) diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt index 7b547e7fe9..ec384c188b 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt @@ -23,7 +23,7 @@ interface ToolbarMenu { object InstallPwaToHomeScreen : Item() object AddToHomeScreen : Item() object SyncedTabs : Item() - object SyncAccount : Item() + data class SyncAccount(val signedIn: Boolean) : Item() object AddonsManager : Item() object Quit : Item() object OpenInApp : Item() 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 9df47af024..a43261f41e 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -89,6 +89,7 @@ import org.mozilla.fenix.GleanMetrics.PerfStartup import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.BrowserAnimator.Companion.getToolbarNavOptions +import org.mozilla.fenix.browser.BrowserFragmentDirections import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.PrivateShortcutCreateManager @@ -792,11 +793,16 @@ class HomeFragment : Fragment() { HomeFragmentDirections.actionGlobalSyncedTabsFragment() ) } - HomeMenu.Item.SyncAccount -> { + is HomeMenu.Item.SyncAccount -> { hideOnboardingIfNeeded() + val directions = if (it.signedIn) { + BrowserFragmentDirections.actionGlobalAccountSettingsFragment() + } else { + BrowserFragmentDirections.actionGlobalTurnOnSync() + } nav( R.id.homeFragment, - HomeFragmentDirections.actionGlobalAccountSettingsFragment() + directions ) } HomeMenu.Item.Bookmarks -> { diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt b/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt index 2f277aa295..4c22f21d8b 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt @@ -26,6 +26,7 @@ import mozilla.components.support.ktx.android.content.getColorFromAttr import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.FeatureFlags.tabsTrayRewrite import org.mozilla.fenix.R +import org.mozilla.fenix.components.accounts.FenixAccountManager import org.mozilla.fenix.experiments.ExperimentBranch import org.mozilla.fenix.experiments.Experiments import org.mozilla.fenix.ext.components @@ -48,7 +49,7 @@ class HomeMenu( object Downloads : Item() object Extensions : Item() object SyncTabs : Item() - object SyncAccount : Item() + data class SyncAccount(val signedIn: Boolean) : Item() object WhatsNew : Item() object Help : Item() object Settings : Item() @@ -64,7 +65,7 @@ class HomeMenu( context.getColorFromAttr(R.attr.syncDisconnectedBackground) private val shouldUseBottomToolbar = context.settings().shouldUseBottomToolbar - private val accountManager = context.components.backgroundServices.accountManager + private val accountManager = FenixAccountManager(context) // 'Reconnect' and 'Quit' items aren't needed most of the time, so we'll only create the if necessary. private val reconnectToSyncItem by lazy { @@ -93,6 +94,33 @@ class HomeMenu( } } + val syncedTabsItem = BrowserMenuImageText( + context.getString(R.string.synced_tabs), + R.drawable.ic_synced_tabs, + primaryTextColor + ) { + onItemTapped.invoke(Item.SyncTabs) + } + + private fun getSyncItemTitle(): String { + val authenticatedAccount = accountManager.authenticatedAccount + val email = accountManager.accountProfileEmail + + return if (authenticatedAccount && !email.isNullOrEmpty()) { + email + } else { + context.getString(R.string.sync_menu_sign_in) + } + } + + val syncSignInMenuItem = BrowserMenuImageText( + getSyncItemTitle(), + R.drawable.ic_synced_tabs, + primaryTextColor + ) { + onItemTapped.invoke(Item.SyncAccount(accountManager.signedInToFxa())) + } + private val oldCoreMenuItems by lazy { val whatsNewItem = BrowserMenuHighlightableItem( context.getString(R.string.browser_menu_whats_new), @@ -159,14 +187,6 @@ class HomeMenu( onItemTapped.invoke(Item.Settings) } - val syncedTabsItem = BrowserMenuImageText( - getSyncItemTitle(), - R.drawable.ic_synced_tabs, - primaryTextColor - ) { - onItemTapped.invoke(Item.SyncTabs) - } - val helpItem = BrowserMenuImageText( context.getString(R.string.browser_menu_help), R.drawable.ic_help, @@ -226,17 +246,6 @@ class HomeMenu( onItemTapped.invoke(Item.DesktopMode(checked)) } - private fun getSyncItemTitle(): String { - val authenticatedAccount = accountManager.authenticatedAccount() != null - val email = accountManager.accountProfile()?.email - - return if (authenticatedAccount && email != null) { - email - } else { - context.getString(R.string.sync_menu_sign_in) - } - } - @Suppress("ComplexMethod") private fun newCoreMenuItems(): List { val experiments = context.components.analytics.experiments @@ -294,22 +303,6 @@ class HomeMenu( onItemTapped.invoke(Item.Extensions) } - val syncedTabsItem = BrowserMenuImageText( - context.getString(R.string.synced_tabs), - R.drawable.ic_synced_tabs, - primaryTextColor - ) { - onItemTapped.invoke(Item.SyncTabs) - } - - val syncSignInMenuItem = BrowserMenuImageText( - getSyncItemTitle(), - R.drawable.ic_synced_tabs, - primaryTextColor - ) { - onItemTapped.invoke(Item.SyncAccount) - } - val whatsNewItem = BrowserMenuHighlightableItem( context.getString(R.string.browser_menu_whats_new), R.drawable.ic_whats_new, diff --git a/app/src/test/java/org/mozilla/fenix/components/accounts/FenixAccountManagerTest.kt b/app/src/test/java/org/mozilla/fenix/components/accounts/FenixAccountManagerTest.kt new file mode 100644 index 0000000000..d93616bc56 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/components/accounts/FenixAccountManagerTest.kt @@ -0,0 +1,101 @@ +/* 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.components.accounts + +import android.content.Context +import io.mockk.every +import io.mockk.mockk +import mozilla.components.concept.sync.OAuthAccount +import mozilla.components.concept.sync.Profile +import mozilla.components.service.fxa.manager.FxaAccountManager +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.ext.components +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class FenixAccountManagerTest { + + private lateinit var fenixFxaManager: FenixAccountManager + private lateinit var accountManagerComponent: FxaAccountManager + private lateinit var context: Context + private lateinit var account: OAuthAccount + private lateinit var profile: Profile + + @Before + fun setUp() { + context = mockk(relaxed = true) + account = mockk(relaxed = true) + profile = mockk(relaxed = true) + accountManagerComponent = mockk(relaxed = true) + } + + @Test + fun `GIVEN an account exists THEN fetch the associated email address`() { + every { accountManagerComponent.authenticatedAccount() } returns account + every { accountManagerComponent.accountProfile() } returns profile + every { context.components.backgroundServices.accountManager } returns accountManagerComponent + + fenixFxaManager = FenixAccountManager(context) + + val emailAddress = "firefoxIsFun@test.com" + every { accountManagerComponent.accountProfile()?.email } returns emailAddress + + val result = fenixFxaManager.accountProfileEmail + assertEquals(emailAddress, result) + } + + @Test + fun `GIVEN an account does not exist THEN return null when fetching the associated email address`() { + every { accountManagerComponent.authenticatedAccount() } returns null + every { accountManagerComponent.accountProfile() } returns null + every { context.components.backgroundServices.accountManager } returns accountManagerComponent + + fenixFxaManager = FenixAccountManager(context) + + val result = fenixFxaManager.accountProfileEmail + assertEquals(null, result) + } + + @Test + fun `GIVEN an account is signed in and authenticated THEN check returns true`() { + every { accountManagerComponent.authenticatedAccount() } returns account + every { accountManagerComponent.accountNeedsReauth() } returns false + every { context.components.backgroundServices.accountManager } returns accountManagerComponent + + fenixFxaManager = FenixAccountManager(context) + + val signedIn = fenixFxaManager.signedInToFxa() + assertTrue(signedIn) + } + + @Test + fun `GIVEN an account is signed in and NOT authenticated THEN check returns false`() { + every { accountManagerComponent.authenticatedAccount() } returns account + every { accountManagerComponent.accountNeedsReauth() } returns true + every { context.components.backgroundServices.accountManager } returns accountManagerComponent + + fenixFxaManager = FenixAccountManager(context) + + val signedIn = fenixFxaManager.signedInToFxa() + assertFalse(signedIn) + } + + @Test + fun `GIVEN an account is not signed in THEN check returns false`() { + every { accountManagerComponent.authenticatedAccount() } returns null + every { accountManagerComponent.accountNeedsReauth() } returns true + every { context.components.backgroundServices.accountManager } returns accountManagerComponent + + fenixFxaManager = FenixAccountManager(context) + + val signedIn = fenixFxaManager.signedInToFxa() + assertFalse(signedIn) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt index 118043de8f..a609774313 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt @@ -572,6 +572,28 @@ class DefaultBrowserToolbarMenuControllerTest { } } + @Test + fun `WHEN sync sign in menu item is pressed AND account is signed out THEN navigate to sync sign in`() = runBlockingTest { + val item = ToolbarMenu.Item.SyncAccount(false) + val directions = BrowserFragmentDirections.actionGlobalTurnOnSync() + + val controller = createController(scope = this, store = browserStore) + controller.handleToolbarItemInteraction(item) + + verify { navController.navigate(directions, null) } + } + + @Test + fun `WHEN sync sign in menu item is pressed AND account is signed in THEN navigate to sync sign in`() = runBlockingTest { + val item = ToolbarMenu.Item.SyncAccount(true) + val directions = BrowserFragmentDirections.actionGlobalAccountSettingsFragment() + + val controller = createController(scope = this, store = browserStore) + controller.handleToolbarItemInteraction(item) + + verify { navController.navigate(directions, null) } + } + private fun createController( scope: CoroutineScope, store: BrowserStore,