diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index c17efab613..b1cfa8e71f 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -92,11 +92,6 @@ object FeatureFlags { */ const val showHomeOnboarding = true - /** - * Enables history improvement features. - */ - const val historyImprovementFeatures = true - /** * Enables the Task Continuity enhancements. */ diff --git a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt index d95a8c8ff2..00cf6002b2 100644 --- a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt @@ -11,7 +11,6 @@ import mozilla.components.concept.storage.HistoryMetadataKey import mozilla.components.concept.storage.VisitInfo import mozilla.components.concept.storage.VisitType import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.history.HistoryItemTimeGroup import org.mozilla.fenix.utils.Settings.Companion.SEARCH_GROUP_MINIMUM_SITES @@ -86,7 +85,6 @@ interface PagedHistoryProvider { */ class DefaultPagedHistoryProvider( private val historyStorage: PlacesHistoryStorage, - private val historyImprovementFeatures: Boolean = FeatureFlags.historyImprovementFeatures, ) : PagedHistoryProvider { /** @@ -132,11 +130,7 @@ class DefaultPagedHistoryProvider( ) } .filter { - if (historyImprovementFeatures) { - it.items.size >= SEARCH_GROUP_MINIMUM_SITES - } else { - true - } + it.items.size >= SEARCH_GROUP_MINIMUM_SITES } .toList() } @@ -207,10 +201,7 @@ class DefaultPagedHistoryProvider( emptyList() } val historyMetadata = historyGroupsInOffset.flatMap { it.items } - - if (historyImprovementFeatures) { - history = history.distinctBy { Pair(it.historyTimeGroup, it.url) } - } + history = history.distinctBy { Pair(it.historyTimeGroup, it.url) } // Add all history items that are not in a group filtering out any matches with a history // metadata item. @@ -227,11 +218,7 @@ class DefaultPagedHistoryProvider( }, ) - return if (historyImprovementFeatures) { - result.sortedByDescending { it.visitedAt } - } else { - result.removeConsecutiveDuplicates().sortedByDescending { it.visitedAt } - } + return result.sortedByDescending { it.visitedAt } } private fun transformVisitInfoToHistoryItem(visit: VisitInfo): HistoryDB.Regular { diff --git a/app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt b/app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt index af35ce52cf..f1bdcd0086 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt @@ -17,7 +17,6 @@ import mozilla.components.concept.storage.HistoryHighlightWeights import mozilla.components.concept.storage.HistoryMetadata import mozilla.components.concept.storage.HistoryMetadataStorage import mozilla.components.support.base.feature.LifecycleAwareFeature -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.home.HomeFragment @@ -52,7 +51,6 @@ class RecentVisitsFeature( private val historyHighlightsStorage: Lazy, private val scope: CoroutineScope, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, - private val historyImprovementFeatures: Boolean = FeatureFlags.historyImprovementFeatures, ) : LifecycleAwareFeature { private var job: Job? = null @@ -214,11 +212,7 @@ class RecentVisitsFeature( ) } .filter { - if (historyImprovementFeatures) { - it.groupItems.size >= SEARCH_GROUP_MINIMUM_SITES - } else { - true - } + it.groupItems.size >= SEARCH_GROUP_MINIMUM_SITES } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 1f9ff22ce3..316228583e 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -40,7 +40,6 @@ import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.telemetry.glean.private.NoExtras -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.GleanMetrics.BookmarksManagement import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.NavHostActivity @@ -183,10 +182,6 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan if (mode.showMenu) { inflater.inflate(R.menu.bookmarks_menu, menu) } - - if (!FeatureFlags.historyImprovementFeatures) { - menu.findItem(R.id.bookmark_search)?.isVisible = false - } } is BookmarkFragmentState.Mode.Selecting -> { if (mode.selectedItems.any { it.type != BookmarkNodeType.ITEM }) { diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index ecdb548681..5df4e92cd0 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -38,7 +38,6 @@ import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.NavHostActivity import org.mozilla.fenix.R @@ -224,10 +223,6 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, } else { inflater.inflate(R.menu.history_menu, menu) } - - if (!FeatureFlags.historyImprovementFeatures) { - menu.findItem(R.id.history_search)?.isVisible = false - } } override fun onMenuItemSelected(item: MenuItem): Boolean = when (item.itemId) { diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index 675d626c01..7f76ccbba5 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -31,7 +31,6 @@ import mozilla.components.support.locale.LocaleManager import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.Config import org.mozilla.fenix.FeatureFlags -import org.mozilla.fenix.FeatureFlags.historyImprovementFeatures import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.metrics.MozillaProductDetector @@ -82,9 +81,9 @@ class Settings(private val appContext: Context) : PreferencesHolder { /** * The minimum number a search groups should contain. - * Filtering is applied depending on the [historyImprovementFeatures] flag value. */ - const val SEARCH_GROUP_MINIMUM_SITES: Int = 2 + @VisibleForTesting + internal var SEARCH_GROUP_MINIMUM_SITES: Int = 2 // The maximum number of top sites to display. const val TOP_SITES_MAX_COUNT = 16 diff --git a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt index 2c6bff52f2..ccdccba8ea 100644 --- a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt @@ -17,6 +17,7 @@ import mozilla.components.concept.storage.VisitType import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test +import org.mozilla.fenix.utils.Settings class PagedHistoryProviderTest { @@ -25,13 +26,13 @@ class PagedHistoryProviderTest { @Before fun setup() { storage = mockk() + Settings.SEARCH_GROUP_MINIMUM_SITES = 1 } @Test fun `getHistory uses getVisitsPaginated`() = runTest { val provider = DefaultPagedHistoryProvider( historyStorage = storage, - historyImprovementFeatures = false, ) val visitInfo1 = VisitInfo( @@ -148,7 +149,6 @@ class PagedHistoryProviderTest { fun `history metadata matching lower bound`() = runTest { val provider = DefaultPagedHistoryProvider( historyStorage = storage, - historyImprovementFeatures = false, ) // Oldest history visit on the page is 15 seconds (buffer time) newer than matching // metadata record. @@ -218,7 +218,6 @@ class PagedHistoryProviderTest { fun `history metadata matching upper bound`() = runTest { val provider = DefaultPagedHistoryProvider( historyStorage = storage, - historyImprovementFeatures = false, ) // Newest history visit on the page is 15 seconds (buffer time) older than matching // metadata record. @@ -288,7 +287,6 @@ class PagedHistoryProviderTest { fun `redirects are filtered out from history metadata groups`() = runTest { val provider = DefaultPagedHistoryProvider( historyStorage = storage, - historyImprovementFeatures = false, ) val visitInfo1 = VisitInfo( diff --git a/app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt b/app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt index 92a60da536..4b0933b7ae 100644 --- a/app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt @@ -35,6 +35,7 @@ import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryGrou import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryHighlight import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItemInternal.HistoryGroupInternal import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItemInternal.HistoryHighlightInternal +import org.mozilla.fenix.utils.Settings import kotlin.random.Random @OptIn(ExperimentalCoroutinesApi::class) @@ -55,6 +56,7 @@ class RecentVisitsFeatureTest { fun setup() { historyHightlightsStorage = mockk(relaxed = true) historyMetadataStorage = mockk(relaxed = true) + Settings.SEARCH_GROUP_MINIMUM_SITES = 1 } @Test @@ -391,7 +393,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN a list of history highlights and groups WHEN updateState is called THEN emit RecentHistoryChange`() { - val feature = spyk(RecentVisitsFeature(appStore, mockk(), mockk(), mockk(), mockk(), false)) + val feature = spyk(RecentVisitsFeature(appStore, mockk(), mockk(), mockk(), mockk())) val expected = List(1) { mockk() } every { feature.getCombinedHistory(any(), any()) } returns expected @@ -405,7 +407,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN highlights visits exist in search groups WHEN getCombined is called THEN remove the highlights already in groups`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(4) val directVisits = getDirectVisitsHistoryMetadataItems(4) val directDupeVisits = getSearchFromHistoryMetadataItems(2).map { @@ -429,7 +431,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN fewer than needed highlights and search groups WHEN getCombined is called THEN the result is sorted by date`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(4) val directVisits = getDirectVisitsHistoryMetadataItems(4) val expected = directVisits.reversed().toRecentHistoryHighlights() @@ -448,7 +450,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN more highlights are newer than search groups WHEN getCombined is called THEN then return an even split then sorted by date`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(5) val directVisits = getDirectVisitsHistoryMetadataItems(14) val expected = directVisits.takeLast(5).reversed().toRecentHistoryHighlights() + @@ -464,7 +466,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN more search groups are newer than highlights WHEN getCombined is called THEN then return an even split then sorted by date`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(14) val directVisits = getDirectVisitsHistoryMetadataItems(5) val expected = visitsFromSearch.takeLast(4).toIndividualRecentHistoryGroups() + @@ -480,7 +482,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN all highlights have metadata WHEN getHistoryHighlights is called THEN return a list of highlights with an inferred last access time`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) @@ -497,7 +499,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN not all highlights have metadata WHEN getHistoryHighlights is called THEN set 0 for the highlights with not found last access time`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) val highlightsWithUnknownAccessTime = directVisits.toHistoryHighlightsInternal().take(5).map { @@ -518,7 +520,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN multiple metadata records for the same highlight WHEN getHistoryHighlights is called THEN set the latest access time from multiple available`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) val newerDirectVisits = directVisits.mapIndexed { index, item -> @@ -540,7 +542,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN multiple metadata entries only for direct accessed pages WHEN getHistorySearchGroups is called THEN return an empty list`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val directVisits = getDirectVisitsHistoryMetadataItems(10) val result = feature.getHistorySearchGroups(directVisits) @@ -550,7 +552,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN multiple metadata entries WHEN getHistorySearchGroups is called THEN group all entries by their search term`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) @@ -563,7 +565,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN multiple metadata entries for the same url WHEN getHistorySearchGroups is called THEN entries are deduped`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val newerVisitsFromSearch = visitsFromSearch.map { it.copy(updatedAt = it.updatedAt * 2) } val directVisits = getDirectVisitsHistoryMetadataItems(10) @@ -582,7 +584,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN highlights and search groups WHEN getSortedHistory is called THEN sort descending all items based on the last access time`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) val expected = directVisits.reversed().toRecentHistoryHighlights() @@ -601,7 +603,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN highlights don't have a valid title WHEN getSortedHistory is called THEN the url is set as title`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10).mapIndexed { index, item -> when (index % 3) { @@ -653,7 +655,6 @@ class RecentVisitsFeatureTest { lazy { historyHightlightsStorage }, scope, testDispatcher, - false, ) assertEquals(emptyList(), appStore.state.recentHistory)