diff --git a/app/src/main/java/org/mozilla/fenix/ext/HomeFragmentState.kt b/app/src/main/java/org/mozilla/fenix/ext/HomeFragmentState.kt index 6ddef7109..6f47ad34a 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/HomeFragmentState.kt +++ b/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? diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt index e2884e37e..c15cb95e9 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt +++ b/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.filterOut(groupTitle: String?): List { + return when (groupTitle != null) { + true -> filterNot { it.title.equals(groupTitle, true) } + false -> this + } +} diff --git a/app/src/main/java/org/mozilla/fenix/home/recenttabs/RecentTabsListFeature.kt b/app/src/main/java/org/mozilla/fenix/home/recenttabs/RecentTabsListFeature.kt index 44bf33ead..bbf87af61 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recenttabs/RecentTabsListFeature.kt +++ b/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 diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabGroup.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabGroup.kt index d1e6c9f9c..510fcd655 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabGroup.kt +++ b/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, diff --git a/app/src/test/java/org/mozilla/fenix/ext/HomeFragmentStateTest.kt b/app/src/test/java/org/mozilla/fenix/ext/HomeFragmentStateTest.kt index aa197a554..15bea6a68 100644 --- a/app/src/test/java/org/mozilla/fenix/ext/HomeFragmentStateTest.kt +++ b/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( diff --git a/app/src/test/java/org/mozilla/fenix/home/HomeFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/home/HomeFragmentStoreTest.kt index 9e4336989..50bcf3010 100644 --- a/app/src/test/java/org/mozilla/fenix/home/HomeFragmentStoreTest.kt +++ b/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 = 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 = 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 = listOf(mockk(), mockk()) + val historyGroup1 = HistoryMetadataGroup(recentGroup.searchTerm.lowercase()) + val historyGroup2 = HistoryMetadataGroup("differentTitle") + val historyMetadata: List = 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 = 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 = listOf(mockk()) val topSites: List = listOf(mockk(), mockk()) - val recentTabs: List = listOf(mockk(), mockk()) + val recentTabs: List = listOf(mockk(), recentGroup, mockk()) val recentBookmarks: List = listOf(mockk(), mockk()) - val historyMetadata: List = 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 = 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")) + } }