[fenix] For https://github.com/mozilla-mobile/fenix/issues/28264 - Refactor synced tabs handling out of `NavigationInteractor`

1 year ago · 721316f905
parent 6cb2f9da59
commit 721316f905
10 changed files with 131 additions and 67 deletions
--- a/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt
@ -15,11 +15,9 @@ import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.prompt.ShareData
 import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.service.glean.private.NoExtras
-import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.GleanMetrics.Collections
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.TabsTray
-import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.collections.CollectionsDialog
 import org.mozilla.fenix.collections.show
 import org.mozilla.fenix.components.TabCollectionStorage
@ -28,7 +26,6 @@ import org.mozilla.fenix.home.HomeFragment
 import org.mozilla.fenix.tabstray.ext.getTabSessionState
 import org.mozilla.fenix.tabstray.ext.isActiveDownload
 import kotlin.coroutines.CoroutineContext
-import mozilla.components.browser.storage.sync.Tab as SyncTab

 /**
 * An interactor that helps with navigating to different parts of the app from the tabs tray.
@ -85,11 +82,6 @@ interface NavigationInteractor {
     * Used when adding [TabSessionState]s as bookmarks.
     */
    fun onSaveToBookmarks(tabs: Collection<TabSessionState>)
-
-    /**
-     * Called when clicking on a SyncedTab item.
-     */
-    fun onSyncedTabClicked(tab: SyncTab)
 }

 /**
@ -98,7 +90,6 @@ interface NavigationInteractor {
@Suppress("LongParameterList", "TooManyFunctions")
 class DefaultNavigationInteractor(
    private val context: Context,
-    private val activity: HomeActivity,
    private val browserStore: BrowserStore,
    private val navController: NavController,
    private val dismissTabTray: () -> Unit,
@ -244,15 +235,4 @@ class DefaultNavigationInteractor(

        showBookmarkSnackbar(tabs.size)
    }
-
-    override fun onSyncedTabClicked(tab: SyncTab) {
-        Events.syncedTabOpened.record(NoExtras())
-
-        dismissTabTray()
-        activity.openToBrowserAndLoad(
-            searchTermOrURL = tab.active().url,
-            newTab = true,
-            from = BrowserDirection.FromTabsTray,
-        )
-    }
 }
--- a/app/src/main/java/org/mozilla/fenix/tabstray/SyncedTabsController.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/SyncedTabsController.kt
@ -0,0 +1,19 @@
+/* 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.tabstray
+
+import mozilla.components.browser.storage.sync.Tab
+
+/**
+ * Controller for handling any actions on synced tabs in the tabs tray.
+ */
+interface SyncedTabsController {
+    /**
+     * Handles a synced tab item click.
+     *
+     * @param tab The synced [Tab] that was clicked.
+     */
+    fun handleSyncedTabClicked(tab: Tab)
+}
--- a/app/src/main/java/org/mozilla/fenix/tabstray/SyncedTabsInteractor.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/SyncedTabsInteractor.kt
@ -0,0 +1,19 @@
+/* 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.tabstray
+
+import mozilla.components.browser.storage.sync.Tab
+
+/**
+ * Interactor for responding to any actions on synced tabs in the tabs tray.
+ */
+interface SyncedTabsInteractor {
+    /**
+     * Invoked when the user clicks on a synced [Tab].
+     *
+     * @param tab The synced [Tab] that was clicked.
+     */
+    fun onSyncedTabClicked(tab: Tab)
+}
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt
@ -10,16 +10,20 @@ import mozilla.components.browser.state.action.DebugAction
 import mozilla.components.browser.state.action.LastAccessAction
 import mozilla.components.browser.state.selector.findTab
 import mozilla.components.browser.state.selector.getNormalOrPrivateTabs
+import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.SessionState
 import mozilla.components.browser.state.state.TabSessionState
 import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.browser.storage.sync.Tab
 import mozilla.components.concept.base.profiler.Profiler
 import mozilla.components.concept.engine.mediasession.MediaSession.PlaybackState
 import mozilla.components.feature.tabs.TabsUseCases
 import mozilla.components.lib.state.DelicateAction
 import mozilla.telemetry.glean.private.NoExtras
-import org.mozilla.fenix.GleanMetrics.Tab
+import org.mozilla.fenix.BrowserDirection
+import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.TabsTray
+import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
 import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager
@ -28,8 +32,12 @@ import org.mozilla.fenix.ext.DEFAULT_ACTIVE_DAYS
 import org.mozilla.fenix.home.HomeFragment
 import org.mozilla.fenix.tabstray.ext.isActiveDownload
 import java.util.concurrent.TimeUnit
+import org.mozilla.fenix.GleanMetrics.Tab as GleanTab

-interface TabsTrayController {
+/**
+ * Controller for handling any actions in the tabs tray.
+ */
+interface TabsTrayController : SyncedTabsController {

    /**
     * Called to open a new tab.
@ -107,8 +115,25 @@ interface TabsTrayController {
    fun handleMediaClicked(tab: SessionState)
 }

+/**
+ * Default implementation of [TabsTrayController].
+ *
+ * @property activity [HomeActivity] used to perform top-level app actions.
+ * @property trayStore [TabsTrayStore] used to read/update the [TabsTrayState].
+ * @property browserStore [BrowserStore] used to read/update the current [BrowserState].
+ * @property browsingModeManager [BrowsingModeManager] used to read/update the current [BrowsingMode].
+ * @property navController [NavController] used to navigate away from the tabs tray.
+ * @property navigateToHomeAndDeleteSession Lambda used to return to the Homescreen and delete the current session.
+ * @property navigationInteractor [NavigationInteractor] used to perform navigation actions with side effects.
+ * @property tabsUseCases Use case wrapper for interacting with tabs.
+ * @property selectTabPosition Lambda used to scroll the tabs tray to the desired position.
+ * @property dismissTray Lambda used to dismiss/minimize the tabs tray.
+ * @property showUndoSnackbarForTab Lambda used to display an UNDO Snackbar.
+ * @property showCancelledDownloadWarning Lambda used to display a cancelled download warning.
+ */
@Suppress("TooManyFunctions", "LongParameterList")
 class DefaultTabsTrayController(
+    private val activity: HomeActivity,
    private val trayStore: TabsTrayStore,
    private val browserStore: BrowserStore,
    private val browsingModeManager: BrowsingModeManager,
@ -121,7 +146,6 @@ class DefaultTabsTrayController(
    private val dismissTray: () -> Unit,
    private val showUndoSnackbarForTab: (Boolean) -> Unit,
    internal val showCancelledDownloadWarning: (downloadCount: Int, tabId: String?, source: String?) -> Unit,
-
 ) : TabsTrayController {

    override fun handleOpeningNewTab(isPrivate: Boolean) {
@ -279,12 +303,12 @@ class DefaultTabsTrayController(
    override fun handleMediaClicked(tab: SessionState) {
        when (tab.mediaSessionState?.playbackState) {
            PlaybackState.PLAYING -> {
-                Tab.mediaPause.record(NoExtras())
+                GleanTab.mediaPause.record(NoExtras())
                tab.mediaSessionState?.controller?.pause()
            }

            PlaybackState.PAUSED -> {
-                Tab.mediaPlay.record(NoExtras())
+                GleanTab.mediaPlay.record(NoExtras())
                tab.mediaSessionState?.controller?.play()
            }
            else -> throw AssertionError(
@ -292,4 +316,15 @@ class DefaultTabsTrayController(
            )
        }
    }
+
+    override fun handleSyncedTabClicked(tab: Tab) {
+        Events.syncedTabOpened.record(NoExtras())
+
+        dismissTray()
+        activity.openToBrowserAndLoad(
+            searchTermOrURL = tab.active().url,
+            newTab = true,
+            from = BrowserDirection.FromTabsTray,
+        )
+    }
 }
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
@ -197,7 +197,6 @@ class TabsTrayFragment : AppCompatDialogFragment() {
        navigationInteractor =
            DefaultNavigationInteractor(
                context = requireContext(),
-                activity = activity,
                tabsTrayStore = tabsTrayStore,
                browserStore = requireComponents.core.store,
                navController = findNavController(),
@ -213,6 +212,7 @@ class TabsTrayFragment : AppCompatDialogFragment() {
            )

        tabsTrayController = DefaultTabsTrayController(
+            activity = activity,
            trayStore = tabsTrayStore,
            browserStore = requireComponents.core.store,
            browsingModeManager = activity.browsingModeManager,
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt
@ -5,8 +5,12 @@
 package org.mozilla.fenix.tabstray

 import mozilla.components.browser.state.state.TabSessionState
+import mozilla.components.browser.storage.sync.Tab

-interface TabsTrayInteractor {
+/**
+ * Interactor for responding to all user actions in the tabs tray.
+ */
+interface TabsTrayInteractor : SyncedTabsInteractor {
    /**
     * Set the current tray item to the clamped [position].
     *
@ -53,7 +57,7 @@ interface TabsTrayInteractor {
 }

 /**
- * Interactor to be called for any tabs tray user actions.
+ * Default implementation of [TabsTrayInteractor].
 *
 * @property controller [TabsTrayController] to which user actions can be delegated for actual app update.
 */
@ -91,4 +95,8 @@ class DefaultTabsTrayInteractor(
    override fun onInactiveDebugClicked(tabs: Collection<TabSessionState>) {
        controller.forceTabsAsInactive(tabs)
    }
+
+    override fun onSyncedTabClicked(tab: Tab) {
+        controller.handleSyncedTabClicked(tab)
+    }
 }
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt
@ -91,7 +91,7 @@ class TrayPagerAdapter(
                        )
                    },
                    tabsTrayStore = tabsTrayStore,
-                    navigationInteractor = navInteractor,
+                    interactor = tabsTrayInteractor,
                )
            }
            else -> throw IllegalStateException("Unknown viewType.")
--- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/SyncedTabsPageViewHolder.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/SyncedTabsPageViewHolder.kt
@ -9,7 +9,7 @@ import androidx.compose.ui.platform.ComposeView
 import androidx.recyclerview.widget.RecyclerView
 import mozilla.components.lib.state.ext.observeAsComposableState
 import org.mozilla.fenix.ext.settings
-import org.mozilla.fenix.tabstray.NavigationInteractor
+import org.mozilla.fenix.tabstray.SyncedTabsInteractor
 import org.mozilla.fenix.tabstray.TabsTrayState
 import org.mozilla.fenix.tabstray.TabsTrayStore
 import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsList
@ -21,12 +21,12 @@ import org.mozilla.fenix.theme.Theme
 *
 * @param composeView Root ComposeView passed-in from TrayPagerAdapter.
 * @param tabsTrayStore Store used as a Composable State to listen for changes to [TabsTrayState.syncedTabs].
- * @param navigationInteractor The lambda for handling clicks on synced tabs.
+ * @param interactor [SyncedTabsInteractor] used to respond to interactions with synced tabs.
 */
 class SyncedTabsPageViewHolder(
    private val composeView: ComposeView,
    private val tabsTrayStore: TabsTrayStore,
-    private val navigationInteractor: NavigationInteractor,
+    private val interactor: SyncedTabsInteractor,
 ) : AbstractPageViewHolder(composeView) {

    fun bind() {
@ -36,7 +36,7 @@ class SyncedTabsPageViewHolder(
                SyncedTabsList(
                    syncedTabs = tabs ?: emptyList(),
                    taskContinuityEnabled = composeView.context.settings().enableTaskContinuityEnhancements,
-                    onTabClick = navigationInteractor::onSyncedTabClicked,
+                    onTabClick = interactor::onSyncedTabClicked,
                )
            }
        }
--- a/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt
@ -22,6 +22,8 @@ import mozilla.components.browser.state.state.TabSessionState
 import mozilla.components.browser.state.state.content.DownloadState
 import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.browser.storage.sync.Tab
+import mozilla.components.browser.storage.sync.TabEntry
 import mozilla.components.concept.base.profiler.Profiler
 import mozilla.components.feature.tabs.TabsUseCases
 import mozilla.components.service.glean.testing.GleanTestRule
@ -35,7 +37,10 @@ import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mozilla.fenix.BrowserDirection
+import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.TabsTray
+import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@ -64,6 +69,9 @@ class DefaultTabsTrayControllerTest {
    @MockK(relaxed = true)
    private lateinit var tabsUseCases: TabsUseCases

+    @MockK(relaxed = true)
+    private lateinit var activity: HomeActivity
+
    @get:Rule
    val gleanTestRule = GleanTestRule(testContext)

@ -475,6 +483,34 @@ class DefaultTabsTrayControllerTest {
        assertTrue(navigateToHomeAndDeleteSessionInvoked)
    }

+    @Test
+    fun `WHEN a synced tab is clicked THEN the metrics are reported and the tab is opened`() {
+        val tab = mockk<Tab>()
+        val entry = mockk<TabEntry>()
+        assertNull(Events.syncedTabOpened.testGetValue())
+
+        every { tab.active() }.answers { entry }
+        every { entry.url }.answers { "https://mozilla.org" }
+
+        var dismissTabTrayInvoked = false
+        createController(
+            dismissTray = {
+                dismissTabTrayInvoked = true
+            },
+        ).handleSyncedTabClicked(tab)
+
+        assertTrue(dismissTabTrayInvoked)
+        assertNotNull(Events.syncedTabOpened.testGetValue())
+
+        verify {
+            activity.openToBrowserAndLoad(
+                searchTermOrURL = "https://mozilla.org",
+                newTab = true,
+                from = BrowserDirection.FromTabsTray,
+            )
+        }
+    }
+
    private fun createController(
        navigateToHomeAndDeleteSession: (String) -> Unit = { },
        selectTabPosition: (Int, Boolean) -> Unit = { _, _ -> },
@ -483,6 +519,7 @@ class DefaultTabsTrayControllerTest {
        showCancelledDownloadWarning: (Int, String?, String?) -> Unit = { _, _, _ -> },
    ): DefaultTabsTrayController {
        return DefaultTabsTrayController(
+            activity,
            trayStore,
            browserStore,
            browsingModeManager,
--- a/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt
@ -20,7 +20,6 @@ import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.TabSessionState
 import mozilla.components.browser.state.state.content.DownloadState
 import mozilla.components.browser.state.store.BrowserStore
-import mozilla.components.browser.storage.sync.TabEntry
 import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.service.glean.testing.GleanTestRule
 import mozilla.components.support.test.robolectric.testContext
@ -35,17 +34,14 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.RuleChain
 import org.junit.runner.RunWith
-import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.TabsTray
-import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.collections.CollectionsDialog
 import org.mozilla.fenix.collections.show
 import org.mozilla.fenix.components.TabCollectionStorage
 import org.mozilla.fenix.components.bookmarks.BookmarksUseCase
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import mozilla.components.browser.state.state.createTab as createStateTab
-import mozilla.components.browser.storage.sync.Tab as SyncTab

@RunWith(FenixRobolectricTestRunner::class) // for gleanTestRule
 class NavigationInteractorTest {
@ -57,7 +53,6 @@ class NavigationInteractorTest {
    private val context: Context = mockk(relaxed = true)
    private val collectionStorage: TabCollectionStorage = mockk(relaxed = true)
    private val accountManager: FxaAccountManager = mockk(relaxed = true)
-    private val activity: HomeActivity = mockk(relaxed = true)

    val coroutinesTestRule: MainCoroutineRule = MainCoroutineRule()
    val gleanTestRule = GleanTestRule(testContext)
@ -220,34 +215,6 @@ class NavigationInteractorTest {
        Assert.assertEquals("1", snapshot.single().extra?.getValue("tab_count"))
    }

-    @Test
-    fun `onSyncedTabsClicked sets metrics and opens browser`() {
-        val tab = mockk<SyncTab>()
-        val entry = mockk<TabEntry>()
-        assertNull(Events.syncedTabOpened.testGetValue())
-
-        every { tab.active() }.answers { entry }
-        every { entry.url }.answers { "https://mozilla.org" }
-
-        var dismissTabTrayInvoked = false
-        createInteractor(
-            dismissTabTray = {
-                dismissTabTrayInvoked = true
-            },
-        ).onSyncedTabClicked(tab)
-
-        assertTrue(dismissTabTrayInvoked)
-        assertNotNull(Events.syncedTabOpened.testGetValue())
-
-        verify {
-            activity.openToBrowserAndLoad(
-                searchTermOrURL = "https://mozilla.org",
-                newTab = true,
-                from = BrowserDirection.FromTabsTray,
-            )
-        }
-    }
-
    @Suppress("LongParameterList")
    private fun createInteractor(
        browserStore: BrowserStore = store,
@ -259,7 +226,6 @@ class NavigationInteractorTest {
    ): NavigationInteractor {
        return DefaultNavigationInteractor(
            context,
-            activity,
            browserStore,
            navController,
            dismissTabTray,