From 10923832312e49fe5ec9ccd50ff5ef7f724f0bf7 Mon Sep 17 00:00:00 2001
From: Roger Yang <royang@mozilla.com>
Date: Thu, 30 Sep 2021 19:13:22 -0400
Subject: [PATCH] No issue: Make sure jump back in group always have more than
 one tab

---
 .../org/mozilla/fenix/ext/BrowserState.kt     |  2 +-
 app/src/main/res/values/strings.xml           |  8 +-
 .../org/mozilla/fenix/ext/BrowserStateTest.kt | 53 ++++++++++++-
 .../fenix/home/RecentTabsListFeatureTest.kt   | 78 +++++++++++++++++--
 4 files changed, 125 insertions(+), 16 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt b/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt
index 8b8851172f..0e01cec0c4 100644
--- a/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt
+++ b/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt
@@ -67,7 +67,7 @@ val BrowserState.inProgressMediaTab: TabSessionState?
  */
 val BrowserState.lastSearchGroup: RecentTab.SearchGroup?
     get() {
-        val tabGroup = normalTabs.toSearchGroup().lastOrNull() ?: return null
+        val tabGroup = normalTabs.toSearchGroup().lastOrNull { it.tabs.count() > 1 } ?: return null
         val firstTab = tabGroup.tabs.firstOrNull() ?: return null
 
         return RecentTab.SearchGroup(
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index dbcfd1757d..ac4ef228b5 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -120,10 +120,12 @@
     <string name="recent_tabs_show_all">Show all</string>
     <!-- Content description for the button which navigates the user to show all recent tabs in the tabs tray. -->
     <string name="recent_tabs_show_all_content_description">Show all recent tabs button</string>
-    <!-- Title for showing a group item in the 'Jump back in' section of the new tab -->
+    <!-- Title for showing a group item in the 'Jump back in' section of the new tab
+        The first parameter is the search term that the user used. (for example: your search for "cat")-->
     <string name="recent_tabs_search_term">Your search for \"%1$s\"</string>
-    <!-- Text for the number of tabs in a group in the 'Jump back in' section of the new tab -->
-    <string name="recent_tabs_search_term_count">%1$s sites</string>
+    <!-- Text for the number of tabs in a group in the 'Jump back in' section of the new tab
+        The first parameter is the count for number of sites in the group.  This number will always be more than one. -->
+    <string name="recent_tabs_search_term_count">Sites: %1$s</string>
 
     <!-- History Metadata -->
     <!-- Header text for a section on the home screen that displays grouped highlights from the
diff --git a/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt b/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt
index b7f126ebcf..711cd4ed3f 100644
--- a/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt
@@ -135,7 +135,7 @@ class BrowserStateTest {
     }
 
     @Test
-    fun `GIVEN a tab group exists WHEN recentTabs is called THEN return a tab group`() {
+    fun `GIVEN a tab group with one tab WHEN recentTabs is called THEN return a tab group`() {
         val searchGroupTab = createTab(
             url = "https://www.mozilla.org",
             id = "1",
@@ -152,6 +152,28 @@ class BrowserStateTest {
 
         val result = browserState.asRecentTabs()
 
+        assertEquals(1, result.size)
+        assertEquals(searchGroupTab, (result[0] as RecentTab.Tab).state)
+    }
+
+    @Test
+    fun `GIVEN a tab group exists WHEN recentTabs is called THEN return a tab group`() {
+        val searchGroupTab = createTab(
+            url = "https://www.mozilla.org",
+            id = "1",
+            historyMetadata = HistoryMetadataKey(
+                url = "https://www.mozilla.org",
+                searchTerm = "Test",
+                referrerUrl = "https://www.mozilla.org"
+            )
+        )
+        val browserState = BrowserState(
+            tabs = listOf(searchGroupTab, searchGroupTab),
+            selectedTabId = searchGroupTab.id
+        )
+
+        val result = browserState.asRecentTabs()
+
         assertEquals(2, result.size)
         assertEquals(searchGroupTab, (result[0] as RecentTab.Tab).state)
         assert(result[1] is RecentTab.SearchGroup)
@@ -159,11 +181,11 @@ class BrowserStateTest {
         assertEquals(searchGroupTab.id, (result[1] as RecentTab.SearchGroup).tabId)
         assertEquals(searchGroupTab.content.url, (result[1] as RecentTab.SearchGroup).url)
         assertEquals(searchGroupTab.content.thumbnail, (result[1] as RecentTab.SearchGroup).thumbnail)
-        assertEquals(1, (result[1] as RecentTab.SearchGroup).count)
+        assertEquals(2, (result[1] as RecentTab.SearchGroup).count)
     }
 
     @Test
-    fun `GIVEN the selected tab is a normal tab and tab group exists WHEN asRecentTabs is called THEN return a list of these tabs`() {
+    fun `GIVEN the selected tab is a normal tab and tab group with one tab exists WHEN asRecentTabs is called THEN return only the normal tab`() {
         val selectedTab = createTab(url = "url", id = "3")
         val searchGroupTab = createTab(
             url = "https://www.mozilla.org",
@@ -181,6 +203,29 @@ class BrowserStateTest {
 
         val result = browserState.asRecentTabs()
 
+        assertEquals(2, result.size)
+        assertEquals(selectedTab, (result[0] as RecentTab.Tab).state)
+    }
+
+    @Test
+    fun `GIVEN the selected tab is a normal tab and tab group with two tabs exists WHEN asRecentTabs is called THEN return a list of these tabs`() {
+        val selectedTab = createTab(url = "url", id = "3")
+        val searchGroupTab = createTab(
+            url = "https://www.mozilla.org",
+            id = "4",
+            historyMetadata = HistoryMetadataKey(
+                url = "https://www.mozilla.org",
+                searchTerm = "Test",
+                referrerUrl = "https://www.mozilla.org"
+            )
+        )
+        val browserState = BrowserState(
+            tabs = listOf(mockk(relaxed = true), selectedTab, searchGroupTab, searchGroupTab),
+            selectedTabId = selectedTab.id
+        )
+
+        val result = browserState.asRecentTabs()
+
         assertEquals(3, result.size)
         assertEquals(selectedTab, (result[0] as RecentTab.Tab).state)
         assert(result[2] is RecentTab.SearchGroup)
@@ -188,7 +233,7 @@ class BrowserStateTest {
         assertEquals(searchGroupTab.id, (result[2] as RecentTab.SearchGroup).tabId)
         assertEquals(searchGroupTab.content.url, (result[2] as RecentTab.SearchGroup).url)
         assertEquals(searchGroupTab.content.thumbnail, (result[2] as RecentTab.SearchGroup).thumbnail)
-        assertEquals(1, (result[2] as RecentTab.SearchGroup).count)
+        assertEquals(2, (result[2] as RecentTab.SearchGroup).count)
     }
 
     @Test
diff --git a/app/src/test/java/org/mozilla/fenix/home/RecentTabsListFeatureTest.kt b/app/src/test/java/org/mozilla/fenix/home/RecentTabsListFeatureTest.kt
index 29152e254d..306809b4c5 100644
--- a/app/src/test/java/org/mozilla/fenix/home/RecentTabsListFeatureTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/home/RecentTabsListFeatureTest.kt
@@ -383,7 +383,7 @@ class RecentTabsListFeatureTest {
 
     @Test
     fun `GIVEN a selected tab group WHEN the feature starts THEN dispatch the selected tab group as a recent tab list`() {
-        val tab = createTab(
+        val tab1 = createTab(
             url = "https://www.mozilla.org",
             id = "1",
             historyMetadata = HistoryMetadataKey(
@@ -392,7 +392,17 @@ class RecentTabsListFeatureTest {
                 referrerUrl = "https://www.mozilla.org"
             )
         )
-        val tabs = listOf(tab)
+
+        val tab2 = createTab(
+            url = "https://www.mozilla.org",
+            id = "2",
+            historyMetadata = HistoryMetadataKey(
+                url = "https://www.mozilla.org",
+                searchTerm = "test search term",
+                referrerUrl = "https://www.mozilla.org"
+            )
+        )
+        val tabs = listOf(tab1, tab2)
         val browserStore = BrowserStore(
             BrowserState(
                 tabs = tabs,
@@ -415,11 +425,11 @@ class RecentTabsListFeatureTest {
         assertEquals(searchGroup.tabId, "1")
         assertEquals(searchGroup.url, "https://www.mozilla.org")
         assertEquals(searchGroup.thumbnail, null)
-        assertEquals(searchGroup.count, 1)
+        assertEquals(searchGroup.count, 2)
     }
 
     @Test
-    fun `GIVEN a selected tab group and a selected tab WHEN the feature starts THEN dispatch both the selected tab and the selected tab group as a recent tab list`() {
+    fun `GIVEN a tab group with one tab and a selected tab WHEN the feature starts THEN dispatch selected tab as a recent tab list`() {
         val tab1 = createTab(
             url = "https://www.mozilla.org",
             id = "1"
@@ -449,6 +459,52 @@ class RecentTabsListFeatureTest {
 
         homeStore.waitUntilIdle()
 
+        assertEquals(1, homeStore.state.recentTabs.size)
+        assertTrue(homeStore.state.recentTabs[0] is RecentTab.Tab)
+        assertEquals(tab1, (homeStore.state.recentTabs[0] as RecentTab.Tab).state)
+    }
+
+    @Test
+    fun `GIVEN a tab group with two tabs and a selected tab WHEN the feature starts THEN dispatch both the selected tab and the selected tab group as a recent tab list`() {
+        val tab1 = createTab(
+            url = "https://www.mozilla.org",
+            id = "1"
+        )
+        val tab2 = createTab(
+            url = "https://www.mozilla.org",
+            id = "2",
+            historyMetadata = HistoryMetadataKey(
+                url = "https://www.mozilla.org",
+                searchTerm = "test search term",
+                referrerUrl = "https://www.mozilla.org"
+            )
+        )
+
+        val tab3 = createTab(
+            url = "https://www.mozilla.org",
+            id = "3",
+            historyMetadata = HistoryMetadataKey(
+                url = "https://www.mozilla.org",
+                searchTerm = "test search term",
+                referrerUrl = "https://www.mozilla.org"
+            )
+        )
+        val tabs = listOf(tab1, tab2, tab3)
+        val browserStore = BrowserStore(
+            BrowserState(
+                tabs = tabs,
+                selectedTabId = "1"
+            )
+        )
+        val feature = RecentTabsListFeature(
+            browserStore = browserStore,
+            homeStore = homeStore
+        )
+
+        feature.start()
+
+        homeStore.waitUntilIdle()
+
         assertEquals(2, homeStore.state.recentTabs.size)
         assertTrue(homeStore.state.recentTabs[0] is RecentTab.Tab)
         assertEquals(tab1, (homeStore.state.recentTabs[0] as RecentTab.Tab).state)
@@ -457,7 +513,7 @@ class RecentTabsListFeatureTest {
         assertEquals(searchGroup.tabId, "2")
         assertEquals(searchGroup.url, "https://www.mozilla.org")
         assertEquals(searchGroup.thumbnail, null)
-        assertEquals(searchGroup.count, 1)
+        assertEquals(searchGroup.count, 2)
     }
 
     @Test
@@ -520,15 +576,21 @@ class RecentTabsListFeatureTest {
             referrerUrl = "https://www.mozilla.org"
         )
         val thumbnail = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888)
-        val searchTermTab = createTab(
+        val searchTermTab1 = createTab(
             url = "https://www.mozilla.org",
             id = "44",
             thumbnail = thumbnail,
             historyMetadata = historyMetadataKey
         )
+        val searchTermTab2 = createTab(
+            url = "https://www.mozilla.org",
+            id = "45",
+            thumbnail = thumbnail,
+            historyMetadata = historyMetadataKey
+        )
         val browserStore = BrowserStore(
             BrowserState(
-                tabs = listOf(mediaTab, selectedTab, searchTermTab),
+                tabs = listOf(mediaTab, selectedTab, searchTermTab1, searchTermTab2),
                 selectedTabId = "43"
             )
         )
@@ -550,6 +612,6 @@ class RecentTabsListFeatureTest {
         assertEquals(searchGroup.tabId, "44")
         assertEquals(searchGroup.url, "https://www.mozilla.org")
         assertEquals(searchGroup.thumbnail, thumbnail)
-        assertEquals(searchGroup.count, 1)
+        assertEquals(searchGroup.count, 2)
     }
 }