For #22442 - Don't display history group if already in Jump back in

app/src/main/java/org/mozilla/fenix/ext/HomeFragmentState.kt

@@ -9,6 +9,7 @@ import mozilla.components.service.pocket.PocketRecommendedStory
 import org.mozilla.fenix.home.HomeFragmentState
 import org.mozilla.fenix.home.pocket.POCKET_STORIES_DEFAULT_CATEGORY_NAME
 import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory
+import org.mozilla.fenix.home.recenttabs.RecentTab.SearchGroup
 
 /**
  * Get the list of stories to be displayed based on the user selected categories.
@@ -92,3 +93,10 @@ internal fun getFilteredStoriesCount(
 
     return emptyMap()
 }
+
+/**
+ * Get the [SearchGroup] shown in the "Jump back in" section.
+ * May be null if no search group is shown.
+ */
+internal val HomeFragmentState.recentSearchGroup: SearchGroup?
+    get() = recentTabs.find { it is SearchGroup } as SearchGroup?

app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt

@@ -5,6 +5,7 @@
 package org.mozilla.fenix.home
 
 import android.graphics.Bitmap
+import androidx.annotation.VisibleForTesting
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.feature.tab.collections.TabCollection
 import mozilla.components.feature.top.sites.TopSite
@@ -15,11 +16,13 @@ import mozilla.components.lib.state.Store
 import mozilla.components.service.pocket.PocketRecommendedStory
 import org.mozilla.fenix.components.tips.Tip
 import org.mozilla.fenix.ext.getFilteredStories
+import org.mozilla.fenix.ext.recentSearchGroup
 import org.mozilla.fenix.historymetadata.HistoryMetadataGroup
 import org.mozilla.fenix.home.recenttabs.RecentTab
 import org.mozilla.fenix.home.pocket.POCKET_STORIES_TO_SHOW_COUNT
 import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory
 import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesSelectedCategory
+import org.mozilla.fenix.home.recenttabs.RecentTab.SearchGroup
 
 /**
  * The [Store] for holding the [HomeFragmentState] and applying [HomeFragmentAction]s.
@@ -125,7 +128,12 @@ private fun homeFragmentStateReducer(
             tip = action.tip,
             recentBookmarks = action.recentBookmarks,
             recentTabs = action.recentTabs,
-            historyMetadata = action.historyMetadata
+            historyMetadata = if (action.historyMetadata.isNotEmpty() && action.recentTabs.isNotEmpty()) {
+                val recentSearchGroup = action.recentTabs.find { it is SearchGroup } as SearchGroup?
+                action.historyMetadata.filterOut(recentSearchGroup?.searchTerm)
+            } else {
+                action.historyMetadata
+            }
         )
         is HomeFragmentAction.CollectionExpanded -> {
             val newExpandedCollection = state.expandedCollections.toMutableSet()
@@ -148,11 +156,23 @@ private fun homeFragmentStateReducer(
             state.copy(showCollectionPlaceholder = false)
         }
         is HomeFragmentAction.RemoveSetDefaultBrowserCard -> state.copy(showSetAsDefaultBrowserCard = false)
-        is HomeFragmentAction.RecentTabsChange -> state.copy(recentTabs = action.recentTabs)
+        is HomeFragmentAction.RecentTabsChange -> {
+            val recentSearchGroup = action.recentTabs.find { it is SearchGroup } as SearchGroup?
+            state.copy(
+                recentTabs = action.recentTabs,
+                historyMetadata = state.historyMetadata.filterOut(recentSearchGroup?.searchTerm)
+            )
+        }
         is HomeFragmentAction.RecentBookmarksChange -> state.copy(recentBookmarks = action.recentBookmarks)
-        is HomeFragmentAction.HistoryMetadataChange -> state.copy(historyMetadata = action.historyMetadata)
+        is HomeFragmentAction.HistoryMetadataChange -> state.copy(
+            historyMetadata = action.historyMetadata.filterOut(state.recentSearchGroup?.searchTerm)
+        )
         is HomeFragmentAction.DisbandSearchGroupAction -> state.copy(
-            historyMetadata = state.historyMetadata.filter { it.title.lowercase() != action.searchTerm.lowercase() }
+            historyMetadata = state.historyMetadata
+                .filter {
+                    it.title.lowercase() != action.searchTerm.lowercase() &&
+                        it.title.lowercase() != state.recentSearchGroup?.searchTerm?.lowercase()
+                }
         )
         is HomeFragmentAction.SelectPocketStoriesCategory -> {
             val updatedCategoriesState = state.copy(
@@ -221,3 +241,16 @@ private fun homeFragmentStateReducer(
         }
     }
 }
+
+/**
+ * Removes a [HistoryMetadataGroup] identified by [groupTitle] if it exists in the current list.
+ *
+ * @param groupTitle [HistoryMetadataGroup.title] of the item that should be removed.
+ */
+@VisibleForTesting
+internal fun List<HistoryMetadataGroup>.filterOut(groupTitle: String?): List<HistoryMetadataGroup> {
+    return when (groupTitle != null) {
+        true -> filterNot { it.title.equals(groupTitle, true) }
+        false -> this
+    }
+}

app/src/main/java/org/mozilla/fenix/home/recenttabs/RecentTabsListFeature.kt

@@ -51,7 +51,7 @@ sealed class RecentTab {
     /**
      * A search term group that was recently viewed
      *
-     * @param searchTerm The search term that was recently viewed
+     * @param searchTerm The search term that was recently viewed. Forced to start with uppercase.
      * @param tabId The id of the tab that was recently viewed
      * @param url The url that was recently viewed
      * @param thumbnail The thumbnail of the search term that was recently viewed

app/src/main/java/org/mozilla/fenix/tabstray/browser/TabGroup.kt

@@ -9,6 +9,8 @@ import mozilla.components.browser.state.state.TabSessionState
 data class TabGroup(
     /**
      * The search term used for the tab group.
+     * Not case dependant - searches with difference letter cases will be part of the same group.
+     * This property's value is then forced to start with an uppercase character.
      */
     val searchTerm: String,
 

app/src/test/java/org/mozilla/fenix/ext/HomeFragmentStateTest.kt

@@ -4,6 +4,7 @@
 
 package org.mozilla.fenix.ext
 
+import io.mockk.mockk
 import mozilla.components.service.pocket.PocketRecommendedStory
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
@@ -14,6 +15,7 @@ import org.mozilla.fenix.home.HomeFragmentState
 import org.mozilla.fenix.home.pocket.POCKET_STORIES_DEFAULT_CATEGORY_NAME
 import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory
 import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesSelectedCategory
+import org.mozilla.fenix.home.recenttabs.RecentTab
 import kotlin.random.Random
 
 class HomeFragmentStateTest {
@@ -274,6 +276,24 @@ class HomeFragmentStateTest {
         assertEquals(3, result.size)
         assertNull(result.firstOrNull { it.category != anotherStoriesCategory.name })
     }
+
+    @Test
+    fun `GIVEN recentTabs contains a SearchGroup WHEN recentSearchGroup is called THEN return the group`() {
+        val searchGroup: RecentTab.SearchGroup = mockk()
+        val normalTab: RecentTab.Tab = mockk()
+        val state = HomeFragmentState(recentTabs = listOf(normalTab, searchGroup))
+
+        assertEquals(searchGroup, state.recentSearchGroup)
+    }
+
+    @Test
+    fun `GIVEN recentTabs does not contains SearchGroup WHEN recentSearchGroup is called THEN return null`() {
+        val normalTab1: RecentTab.Tab = mockk()
+        val normalTab2: RecentTab.Tab = mockk()
+        val state = HomeFragmentState(recentTabs = listOf(normalTab1, normalTab2))
+
+        assertNull(state.recentSearchGroup)
+    }
 }
 
 private fun getFakePocketStories(

app/src/test/java/org/mozilla/fenix/home/HomeFragmentStoreTest.kt

@@ -109,34 +109,58 @@ class HomeFragmentStoreTest {
 
     @Test
     fun `Test changing the recent tabs in HomeFragmentStore`() = runBlocking {
+        val historyGroup1 = HistoryMetadataGroup(title = "historyGroup1")
+        val historyGroup2 = HistoryMetadataGroup(title = "historyGroup2")
+        val historyGroup3 = HistoryMetadataGroup(title = "historyGroup3")
+        homeFragmentStore = HomeFragmentStore(
+            HomeFragmentState(
+                historyMetadata = listOf(historyGroup1, historyGroup2, historyGroup3)
+            )
+        )
         assertEquals(0, homeFragmentStore.state.recentTabs.size)
 
-        // Add 2 TabSessionState to the HomeFragmentStore.
-        val recentTabs: List<RecentTab> = listOf(mockk(), mockk())
+        // Add 2 RecentTabs to the HomeFragmentStore
+        // A new SearchGroup already shown in history should hide the HistoryGroup.
+        val recentTab1: RecentTab.Tab = mockk()
+        val recentTab2 = RecentTab.SearchGroup(historyGroup2.title, "tabId", "url", null, 2)
+        val recentTabs: List<RecentTab> = listOf(recentTab1, recentTab2)
         homeFragmentStore.dispatch(HomeFragmentAction.RecentTabsChange(recentTabs)).join()
 
         assertEquals(recentTabs, homeFragmentStore.state.recentTabs)
+        assertEquals(listOf(historyGroup1, historyGroup3), homeFragmentStore.state.historyMetadata)
     }
 
     @Test
     fun `Test changing the history metadata in HomeFragmentStore`() = runBlocking {
+        val recentGroup = RecentTab.SearchGroup("testSearchTerm", "id", "url", null, 3)
+        homeFragmentStore = HomeFragmentStore(
+            HomeFragmentState(recentTabs = listOf(recentGroup))
+        )
         assertEquals(0, homeFragmentStore.state.historyMetadata.size)
 
-        val historyMetadata: List<HistoryMetadataGroup> = listOf(mockk(), mockk())
+        val historyGroup1 = HistoryMetadataGroup(recentGroup.searchTerm.lowercase())
+        val historyGroup2 = HistoryMetadataGroup("differentTitle")
+        val historyMetadata: List<HistoryMetadataGroup> = listOf(historyGroup1, historyGroup2)
         homeFragmentStore.dispatch(HomeFragmentAction.HistoryMetadataChange(historyMetadata)).join()
 
-        assertEquals(historyMetadata, homeFragmentStore.state.historyMetadata)
+        assertEquals(listOf(historyGroup2), homeFragmentStore.state.historyMetadata)
     }
 
     @Test
     fun `Test disbanding search group in HomeFragmentStore`() = runBlocking {
+        val recentGroup = RecentTab.SearchGroup("testSearchTerm", "id", "url", null, 3)
         val g1 = HistoryMetadataGroup(title = "test One")
         val g2 = HistoryMetadataGroup(title = "test two")
-        val historyMetadata: List<HistoryMetadataGroup> = listOf(g1, g2)
-        homeFragmentStore.dispatch(HomeFragmentAction.HistoryMetadataChange(historyMetadata)).join()
-        assertEquals(historyMetadata, homeFragmentStore.state.historyMetadata)
+        val g3 = HistoryMetadataGroup(title = recentGroup.searchTerm.lowercase())
+        homeFragmentStore = HomeFragmentStore(
+            HomeFragmentState(
+                recentTabs = listOf(recentGroup),
+                historyMetadata = listOf(g1, g2, g3)
+            )
+        )
 
         homeFragmentStore.dispatch(HomeFragmentAction.DisbandSearchGroupAction("Test one")).join()
+
         assertEquals(listOf(g2), homeFragmentStore.state.historyMetadata)
     }
 
@@ -174,11 +198,15 @@ class HomeFragmentStoreTest {
             assertEquals(0, homeFragmentStore.state.historyMetadata.size)
             assertEquals(Mode.Normal, homeFragmentStore.state.mode)
 
+            val recentGroup = RecentTab.SearchGroup("testSearchTerm", "id", "url", null, 3)
             val collections: List<TabCollection> = listOf(mockk())
             val topSites: List<TopSite> = listOf(mockk(), mockk())
-            val recentTabs: List<RecentTab> = listOf(mockk(), mockk())
+            val recentTabs: List<RecentTab> = listOf(mockk(), recentGroup, mockk())
             val recentBookmarks: List<BookmarkNode> = listOf(mockk(), mockk())
-            val historyMetadata: List<HistoryMetadataGroup> = listOf(mockk(), mockk())
+            val g1 = HistoryMetadataGroup(title = "test One")
+            val g2 = HistoryMetadataGroup(title = recentGroup.searchTerm.lowercase())
+            val g3 = HistoryMetadataGroup(title = "test two")
+            val historyMetadata: List<HistoryMetadataGroup> = listOf(g1, g2, g3)
 
             homeFragmentStore.dispatch(
                 HomeFragmentAction.Change(
@@ -192,11 +220,11 @@ class HomeFragmentStoreTest {
                 )
             ).join()
 
-            assertEquals(1, homeFragmentStore.state.collections.size)
-            assertEquals(2, homeFragmentStore.state.topSites.size)
-            assertEquals(2, homeFragmentStore.state.recentTabs.size)
-            assertEquals(2, homeFragmentStore.state.recentBookmarks.size)
-            assertEquals(2, homeFragmentStore.state.historyMetadata.size)
+            assertEquals(collections, homeFragmentStore.state.collections)
+            assertEquals(topSites, homeFragmentStore.state.topSites)
+            assertEquals(recentTabs, homeFragmentStore.state.recentTabs)
+            assertEquals(recentBookmarks, homeFragmentStore.state.recentBookmarks)
+            assertEquals(listOf(g1, g3), homeFragmentStore.state.historyMetadata)
             assertEquals(Mode.Private, homeFragmentStore.state.mode)
         }
 
@@ -336,4 +364,18 @@ class HomeFragmentStoreTest {
             assertSame(firstFilteredStories, homeFragmentStore.state.pocketStories)
         }
     }
+
+    @Test
+    fun `Test filtering out search groups`() {
+        val group1 = HistoryMetadataGroup("group1")
+        val group2 = HistoryMetadataGroup("group2")
+        val group3 = HistoryMetadataGroup("group3")
+        val groups = listOf(group1, group2, group3)
+
+        assertEquals(groups, groups.filterOut(null))
+        assertEquals(groups, groups.filterOut(""))
+        assertEquals(groups, groups.filterOut(" "))
+        assertEquals(groups - group2, groups.filterOut("Group2"))
+        assertEquals(groups - group3, groups.filterOut("group3"))
+    }
 }