From 11ad1010a9d845f390ee4c9f4a1077a0be8a120e Mon Sep 17 00:00:00 2001 From: mcarare Date: Tue, 29 Oct 2019 18:02:29 +0200 Subject: [PATCH] For #6323 Creating 1st collection from tab shows Name collection screen Added a check for existence of at least a collection to select from. --- .../fenix/browser/BaseBrowserFragment.kt | 3 +- .../toolbar/BrowserToolbarController.kt | 10 ++++- .../DefaultBrowserToolbarControllerTest.kt | 40 ++++++++++++++++--- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 8382c79f6..ed6574ff0 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -194,7 +194,8 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs flags = Intent.FLAG_ACTIVITY_NEW_TASK }, bottomSheetBehavior = QuickActionSheetBehavior.from(nestedScrollQuickAction), - scope = lifecycleScope + scope = lifecycleScope, + tabCollectionStorage = requireComponents.core.tabCollectionStorage ) browserInteractor = diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt index 2eebccb26..f575225a5 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt @@ -33,6 +33,7 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.collections.SaveCollectionStep +import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.nav @@ -66,7 +67,8 @@ class DefaultBrowserToolbarController( private val getSupportUrl: () -> String, private val openInFenixIntent: Intent, private val bottomSheetBehavior: QuickActionSheetBehavior, - private val scope: LifecycleCoroutineScope + private val scope: LifecycleCoroutineScope, + private val tabCollectionStorage: TabCollectionStorage ) : BrowserToolbarController { private val currentSession @@ -184,7 +186,11 @@ class DefaultBrowserToolbarController( previousFragmentId = R.id.browserFragment, tabIds = arrayOf(currentSession.id), selectedTabIds = arrayOf(currentSession.id), - saveCollectionStep = SaveCollectionStep.SelectCollection + saveCollectionStep = if (tabCollectionStorage.cachedTabCollections.isEmpty()) { + SaveCollectionStep.NameCollection + } else { + SaveCollectionStep.SelectCollection + } ) navController.nav(R.id.browserFragment, directions) } diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt index 8a008485f..d6f411e87 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt @@ -44,6 +44,7 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.collections.SaveCollectionStep import org.mozilla.fenix.components.Analytics import org.mozilla.fenix.components.FenixSnackbar +import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components @@ -79,6 +80,7 @@ class DefaultBrowserToolbarControllerTest { private val scope: LifecycleCoroutineScope = mockk(relaxed = true) private val adjustBackgroundAndNavigate: (NavDirections) -> Unit = mockk(relaxed = true) private val snackbar = mockk(relaxed = true) + private val tabCollectionStorage = mockk(relaxed = true) private lateinit var controller: DefaultBrowserToolbarController @@ -100,7 +102,8 @@ class DefaultBrowserToolbarControllerTest { bottomSheetBehavior = bottomSheetBehavior, scope = scope, browserLayout = browserLayout, - swipeRefresh = swipeRefreshLayout + swipeRefresh = swipeRefreshLayout, + tabCollectionStorage = tabCollectionStorage ) mockkStatic( @@ -402,7 +405,7 @@ class DefaultBrowserToolbarControllerTest { } @Test - fun handleToolbarSaveToCollectionPress() { + fun handleToolbarSaveToCollectionPressWhenAtLeastOneCollectionExists() { val item = ToolbarMenu.Item.SaveToCollection val cachedTabCollections: List = mockk(relaxed = true) every { activity.components.useCases.sessionUseCases } returns sessionUseCases @@ -410,8 +413,10 @@ class DefaultBrowserToolbarControllerTest { controller.handleToolbarItemInteraction(item) - verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.SAVE_TO_COLLECTION)) } - verify { metrics.track(Event.CollectionSaveButtonPressed(DefaultBrowserToolbarController.TELEMETRY_BROWSER_IDENTIFIER)) } + verify { metrics.track( + Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.SAVE_TO_COLLECTION)) } + verify { metrics.track( + Event.CollectionSaveButtonPressed(DefaultBrowserToolbarController.TELEMETRY_BROWSER_IDENTIFIER)) } verify { val directions = BrowserFragmentDirections.actionBrowserFragmentToCreateCollectionFragment( @@ -424,6 +429,30 @@ class DefaultBrowserToolbarControllerTest { } } + @Test + fun handleToolbarSaveToCollectionPressWhenNoCollectionsExists() { + val item = ToolbarMenu.Item.SaveToCollection + val cachedTabCollectionsEmpty: List = emptyList() + every { activity.components.useCases.sessionUseCases } returns sessionUseCases + every { activity.components.core.tabCollectionStorage.cachedTabCollections } returns cachedTabCollectionsEmpty + + controller.handleToolbarItemInteraction(item) + + verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.SAVE_TO_COLLECTION)) } + verify { metrics.track(Event.CollectionSaveButtonPressed( + DefaultBrowserToolbarController.TELEMETRY_BROWSER_IDENTIFIER)) } + verify { + val directions = + BrowserFragmentDirections.actionBrowserFragmentToCreateCollectionFragment( + previousFragmentId = R.id.browserFragment, + saveCollectionStep = SaveCollectionStep.NameCollection, + tabIds = arrayOf(currentSession.id), + selectedTabIds = arrayOf(currentSession.id) + ) + navController.nav(R.id.browserFragment, directions) + } + } + @Test fun handleToolbarOpenInFenixPress() { controller = DefaultBrowserToolbarController( @@ -440,7 +469,8 @@ class DefaultBrowserToolbarControllerTest { bottomSheetBehavior = bottomSheetBehavior, scope = scope, browserLayout = browserLayout, - swipeRefresh = swipeRefreshLayout + swipeRefresh = swipeRefreshLayout, + tabCollectionStorage = tabCollectionStorage ) val sessionManager: SessionManager = mockk(relaxed = true)