diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index 2922a6dffc..e62a99cfab 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -64,11 +64,6 @@ object FeatureFlags { */ const val unifiedSearchSettings = true - /** - * Enables the lib-state HistoryFragment refactor - */ - val historyFragmentLibStateRefactor = Config.channel.isNightlyOrDebug - /** * Allows users to enable translations. * Preference to fully enable translations is pref_key_enable_translations. diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt index 86c6686bca..89e364d08c 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt @@ -15,7 +15,6 @@ import org.mozilla.fenix.selection.SelectionHolder * Adapter for the list of visited pages, that uses Paging 3 versions of the Paging library. */ class HistoryAdapter( - private val historyInteractor: HistoryInteractor, private val store: HistoryFragmentStore, private val onEmptyStateChanged: (Boolean) -> Unit, ) : PagingDataAdapter(historyDiffCallback), @@ -41,7 +40,6 @@ class HistoryAdapter( return HistoryListItemViewHolder( view = view, - historyInteractor = historyInteractor, selectionHolder = this, store = store, ) diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt deleted file mode 100644 index 2464d3004c..0000000000 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt +++ /dev/null @@ -1,209 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.library.history - -import android.content.Context -import androidx.navigation.NavController -import androidx.navigation.NavOptions -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch -import mozilla.components.browser.state.action.EngineAction -import mozilla.components.browser.state.action.HistoryMetadataAction -import mozilla.components.browser.state.action.RecentlyClosedAction -import mozilla.components.browser.state.store.BrowserStore -import mozilla.components.browser.storage.sync.PlacesHistoryStorage -import mozilla.components.service.glean.private.NoExtras -import org.mozilla.fenix.GleanMetrics.Events -import org.mozilla.fenix.R -import org.mozilla.fenix.components.AppStore -import org.mozilla.fenix.components.appstate.AppAction -import org.mozilla.fenix.components.history.DefaultPagedHistoryProvider -import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.navigateSafe -import org.mozilla.fenix.library.history.HistoryFragment.DeleteConfirmationDialogFragment -import org.mozilla.fenix.GleanMetrics.History as GleanHistory - -@Suppress("TooManyFunctions") -interface HistoryController { - fun handleOpen(item: History) - fun handleSelect(item: History) - fun handleDeselect(item: History) - fun handleBackPressed(): Boolean - fun handleModeSwitched() - fun handleSearch() - - /** - * Displays a [DeleteConfirmationDialogFragment]. - */ - fun handleDeleteTimeRange() - fun handleDeleteSome(items: Set) - - /** - * Deletes history items inside the time frame. - * - * @param timeFrame Selected time frame by the user. If `null`, removes all history. - */ - fun handleDeleteTimeRangeConfirmed(timeFrame: RemoveTimeFrame?) - fun handleRequestSync() - fun handleEnterRecentlyClosed() -} - -@Suppress("TooManyFunctions", "LongParameterList") -class DefaultHistoryController( - private val store: HistoryFragmentStore, - private val appStore: AppStore, - private val browserStore: BrowserStore, - private val historyStorage: PlacesHistoryStorage, - private var historyProvider: DefaultPagedHistoryProvider, - private val navController: NavController, - private val scope: CoroutineScope, - private val openToBrowser: (item: History.Regular) -> Unit, - private val displayDeleteTimeRange: () -> Unit, - private val onTimeFrameDeleted: () -> Unit, - private val invalidateOptionsMenu: () -> Unit, - private val deleteSnackbar: ( - items: Set, - undo: suspend (Set) -> Unit, - delete: (Set) -> suspend (context: Context) -> Unit, - ) -> Unit, - private val syncHistory: suspend () -> Unit, -) : HistoryController { - - override fun handleOpen(item: History) { - when (item) { - is History.Regular -> openToBrowser(item) - is History.Group -> { - GleanHistory.searchTermGroupTapped.record(NoExtras()) - navController.navigate( - HistoryFragmentDirections.actionGlobalHistoryMetadataGroup( - title = item.title, - historyMetadataItems = item.items.toTypedArray(), - ), - NavOptions.Builder().setPopUpTo(R.id.historyMetadataGroupFragment, true).build(), - ) - } - else -> { /* noop */ } - } - } - - override fun handleSelect(item: History) { - if (store.state.mode === HistoryFragmentState.Mode.Syncing) { - return - } - - store.dispatch(HistoryFragmentAction.AddItemForRemoval(item)) - } - - override fun handleDeselect(item: History) { - store.dispatch(HistoryFragmentAction.RemoveItemForRemoval(item)) - } - - override fun handleBackPressed(): Boolean { - return if (store.state.mode is HistoryFragmentState.Mode.Editing) { - store.dispatch(HistoryFragmentAction.ExitEditMode) - true - } else { - false - } - } - - override fun handleModeSwitched() { - invalidateOptionsMenu.invoke() - } - - override fun handleSearch() { - navController.navigateSafe( - R.id.historyFragment, - HistoryFragmentDirections.actionGlobalSearchDialog(sessionId = null), - ) - } - - override fun handleDeleteTimeRange() { - displayDeleteTimeRange.invoke() - } - - override fun handleDeleteSome(items: Set) { - val pendingDeletionItems = items.map { it.toPendingDeletionHistory() }.toSet() - appStore.dispatch(AppAction.AddPendingDeletionSet(pendingDeletionItems)) - deleteSnackbar.invoke(items, ::undo, ::delete) - } - - override fun handleDeleteTimeRangeConfirmed(timeFrame: RemoveTimeFrame?) { - scope.launch { - store.dispatch(HistoryFragmentAction.EnterDeletionMode) - if (timeFrame == null) { - historyStorage.deleteEverything() - } else { - val longRange = timeFrame.toLongRange() - historyStorage.deleteVisitsBetween( - startTime = longRange.first, - endTime = longRange.last, - ) - } - when (timeFrame) { - RemoveTimeFrame.LastHour -> GleanHistory.removedLastHour.record(NoExtras()) - RemoveTimeFrame.TodayAndYesterday -> GleanHistory.removedTodayAndYesterday.record(NoExtras()) - null -> GleanHistory.removedAll.record(NoExtras()) - } - // We introduced more deleting options, but are keeping these actions for all options. - // The approach could be improved: https://github.com/mozilla-mobile/fenix/issues/26102 - browserStore.dispatch(RecentlyClosedAction.RemoveAllClosedTabAction) - browserStore.dispatch(EngineAction.PurgeHistoryAction).join() - - store.dispatch(HistoryFragmentAction.ExitDeletionMode) - - launch(Dispatchers.Main) { - onTimeFrameDeleted.invoke() - } - } - } - - private fun undo(items: Set) { - val pendingDeletionItems = items.map { it.toPendingDeletionHistory() }.toSet() - appStore.dispatch(AppAction.UndoPendingDeletionSet(pendingDeletionItems)) - } - - private fun delete(items: Set): suspend (context: Context) -> Unit { - return { context -> - CoroutineScope(Dispatchers.IO).launch { - store.dispatch(HistoryFragmentAction.EnterDeletionMode) - for (item in items) { - GleanHistory.removed.record(NoExtras()) - - when (item) { - is History.Regular -> context.components.core.historyStorage.deleteVisitsFor(item.url) - is History.Group -> { - // NB: If we have non-search groups, this logic needs to be updated. - historyProvider.deleteMetadataSearchGroup(item) - context.components.core.store.dispatch( - HistoryMetadataAction.DisbandSearchGroupAction(searchTerm = item.title), - ) - } - // We won't encounter individual metadata entries outside of groups. - is History.Metadata -> {} - } - } - store.dispatch(HistoryFragmentAction.ExitDeletionMode) - } - } - } - - override fun handleRequestSync() { - scope.launch { - store.dispatch(HistoryFragmentAction.StartSync) - syncHistory.invoke() - store.dispatch(HistoryFragmentAction.FinishSync) - } - } - - override fun handleEnterRecentlyClosed() { - navController.navigate( - HistoryFragmentDirections.actionGlobalRecentlyClosed(), - NavOptions.Builder().setPopUpTo(R.id.recentlyClosedFragment, true).build(), - ) - Events.recentlyClosedTabsOpened.record(NoExtras()) - } -} 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 59f74471ce..10031171c9 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 @@ -5,7 +5,6 @@ package org.mozilla.fenix.library.history import android.app.Dialog -import android.content.Context import android.content.DialogInterface import android.os.Bundle import android.text.SpannableString @@ -42,7 +41,6 @@ import mozilla.components.support.ktx.kotlin.toShortUrl import mozilla.components.ui.widgets.withCenterAlignedButtons 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 @@ -72,7 +70,6 @@ import org.mozilla.fenix.GleanMetrics.History as GleanHistory @SuppressWarnings("TooManyFunctions", "LargeClass") class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, MenuProvider { private lateinit var historyStore: HistoryFragmentStore - private lateinit var historyInteractor: HistoryInteractor private lateinit var historyProvider: DefaultPagedHistoryProvider private var deleteHistory: MenuItem? = null @@ -113,56 +110,33 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, historyStore = StoreProvider.get(this) { HistoryFragmentStore( initialState = HistoryFragmentState.initial, - middleware = if (FeatureFlags.historyFragmentLibStateRefactor) { - listOf( - HistoryNavigationMiddleware( - navController = findNavController(), - openToBrowser = ::openItem, - onBackPressed = requireActivity().onBackPressedDispatcher::onBackPressed, - ), - HistoryTelemetryMiddleware( - isInPrivateMode = requireComponents.appStore.state.mode == Mode.Private, - ), - HistorySyncMiddleware( - accountManager = requireContext().components.backgroundServices.accountManager, - refreshView = { historyView.historyAdapter.refresh() }, - scope = lifecycleScope, - ), - HistoryStorageMiddleware( - appStore = requireContext().components.appStore, - browserStore = requireContext().components.core.store, - historyProvider = historyProvider, - historyStorage = requireContext().components.core.historyStorage, - undoDeleteSnackbar = ::showDeleteSnackbar, - onTimeFrameDeleted = ::onTimeFrameDeleted, - ), - ) - } else { - listOf() - }, + middleware = listOf( + HistoryNavigationMiddleware( + navController = findNavController(), + openToBrowser = ::openItem, + onBackPressed = requireActivity().onBackPressedDispatcher::onBackPressed, + ), + HistoryTelemetryMiddleware( + isInPrivateMode = requireComponents.appStore.state.mode == Mode.Private, + ), + HistorySyncMiddleware( + accountManager = requireContext().components.backgroundServices.accountManager, + refreshView = { historyView.historyAdapter.refresh() }, + scope = lifecycleScope, + ), + HistoryStorageMiddleware( + appStore = requireContext().components.appStore, + browserStore = requireContext().components.core.store, + historyProvider = historyProvider, + historyStorage = requireContext().components.core.historyStorage, + undoDeleteSnackbar = ::showDeleteSnackbar, + onTimeFrameDeleted = ::onTimeFrameDeleted, + ), + ), ) } - val historyController: HistoryController = DefaultHistoryController( - store = historyStore, - appStore = requireContext().components.appStore, - browserStore = requireComponents.core.store, - historyStorage = requireComponents.core.historyStorage, - historyProvider = historyProvider, - navController = findNavController(), - scope = lifecycleScope, - openToBrowser = ::openItem, - displayDeleteTimeRange = ::displayDeleteTimeRange, - invalidateOptionsMenu = ::invalidateOptionsMenu, - deleteSnackbar = ::deleteSnackbar, - onTimeFrameDeleted = ::onTimeFrameDeleted, - syncHistory = ::syncHistory, - ) - historyInteractor = DefaultHistoryInteractor( - historyController, - ) _historyView = HistoryView( binding.historyLayout, - historyInteractor, onZeroItemsLoaded = { historyStore.dispatch( HistoryFragmentAction.ChangeEmptyState(isEmpty = true), @@ -203,22 +177,6 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, GleanHistory.opened.record(NoExtras()) } - private fun deleteSnackbar( - items: Set, - undo: suspend (items: Set) -> Unit, - delete: (Set) -> suspend (context: Context) -> Unit, - ) { - CoroutineScope(IO).allowUndo( - requireActivity().getRootView()!!, - getMultiSelectSnackBarMessage(items), - getString(R.string.snackbar_deleted_undo), - { - undo(items) - }, - delete(items), - ) - } - private fun showDeleteSnackbar( items: Set, undo: suspend (Set) -> Unit, @@ -310,7 +268,6 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, } } - @Suppress("LongMethod") override fun onMenuItemSelected(item: MenuItem): Boolean = when (item.itemId) { R.id.share_history_multi_select -> { val selectedHistory = historyStore.state.mode.selectedItems @@ -339,14 +296,9 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, true } R.id.delete_history_multi_select -> { - if (FeatureFlags.historyFragmentLibStateRefactor) { - with(historyStore) { - dispatch(HistoryFragmentAction.DeleteItems(state.mode.selectedItems)) - dispatch(HistoryFragmentAction.ExitEditMode) - } - } else { - historyInteractor.onDeleteSome(historyStore.state.mode.selectedItems) - historyStore.dispatch(HistoryFragmentAction.ExitEditMode) + with(historyStore) { + dispatch(HistoryFragmentAction.DeleteItems(state.mode.selectedItems)) + dispatch(HistoryFragmentAction.ExitEditMode) } true } @@ -376,18 +328,13 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, true } R.id.history_search -> { - historyInteractor.onSearch() + historyStore.dispatch(HistoryFragmentAction.SearchClicked) true } R.id.history_delete -> { - if (FeatureFlags.historyFragmentLibStateRefactor) { - DeleteConfirmationDialogFragment( - store = historyStore, - historyInteractor = historyInteractor, - ).show(childFragmentManager, null) - } else { - historyInteractor.onDeleteTimeRange() - } + DeleteConfirmationDialogFragment( + store = historyStore, + ).show(childFragmentManager, null) true } // other options are not handled by this menu provider @@ -424,12 +371,10 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, } } - override fun onBackPressed() = if (FeatureFlags.historyFragmentLibStateRefactor) { + override fun onBackPressed(): Boolean { // The state needs to be updated accordingly if Edit mode is active historyStore.dispatch(HistoryFragmentAction.BackPressed) - true - } else { - historyView.onBackPressed() + return true } override fun onDestroyView() { @@ -455,13 +400,6 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, ) } - private fun displayDeleteTimeRange() { - DeleteConfirmationDialogFragment( - store = historyStore, - historyInteractor = historyInteractor, - ).show(childFragmentManager, null) - } - private fun share(data: List) { GleanHistory.shared.record(NoExtras()) val directions = HistoryFragmentDirections.actionGlobalShareFragment( @@ -490,7 +428,6 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, internal class DeleteConfirmationDialogFragment( private val store: HistoryFragmentStore, - private val historyInteractor: HistoryInteractor, ) : DialogFragment() { override fun onCreateDialog(savedInstanceState: Bundle?): Dialog = AlertDialog.Builder(requireContext()).apply { @@ -511,11 +448,7 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, R.id.everything_button -> null else -> throw IllegalStateException("Unexpected radioButtonId") } - if (FeatureFlags.historyFragmentLibStateRefactor) { - store.dispatch(HistoryFragmentAction.DeleteTimeRange(selectedTimeFrame)) - } else { - historyInteractor.onDeleteTimeRangeConfirmed(selectedTimeFrame) - } + store.dispatch(HistoryFragmentAction.DeleteTimeRange(selectedTimeFrame)) dialog.dismiss() } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryInteractor.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryInteractor.kt deleted file mode 100644 index d9395a010b..0000000000 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryInteractor.kt +++ /dev/null @@ -1,114 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.library.history - -import mozilla.components.service.glean.private.NoExtras -import org.mozilla.fenix.selection.SelectionInteractor -import org.mozilla.fenix.GleanMetrics.History as GleanHistory - -/** - * Interface for the HistoryInteractor. This interface is implemented by objects that want - * to respond to user interaction on the HistoryView - */ -interface HistoryInteractor : SelectionInteractor { - - /** - * Called on backpressed to exit edit mode - */ - fun onBackPressed(): Boolean - - /** - * Called when the mode is switched so we can invalidate the menu - */ - fun onModeSwitched() - - /** - * Called when search is tapped - */ - fun onSearch() - - /** - * Called when the delete menu button is tapped. - */ - fun onDeleteTimeRange() - - /** - * Called when multiple history items are deleted - * @param items the history items to delete - */ - fun onDeleteSome(items: Set) - - /** - * Called when the user has confirmed deletion of a time range. - * - * @param timeFrame The selected timeframe. `null` means no specific time frame has been - * selected; should remove everything. - */ - fun onDeleteTimeRangeConfirmed(timeFrame: RemoveTimeFrame?) - - /** - * Called when the user requests a sync of the history - */ - fun onRequestSync() - - /** - * Called when the user clicks on recently closed tab button. - */ - fun onRecentlyClosedClicked() -} - -/** - * Interactor for the history screen - * Provides implementations for the HistoryInteractor - */ -@SuppressWarnings("TooManyFunctions") -class DefaultHistoryInteractor( - private val historyController: HistoryController, -) : HistoryInteractor { - override fun open(item: History) { - historyController.handleOpen(item) - } - - override fun select(item: History) { - historyController.handleSelect(item) - } - - override fun deselect(item: History) { - historyController.handleDeselect(item) - } - - override fun onBackPressed(): Boolean { - return historyController.handleBackPressed() - } - - override fun onModeSwitched() { - historyController.handleModeSwitched() - } - - override fun onSearch() { - GleanHistory.searchIconTapped.record(NoExtras()) - historyController.handleSearch() - } - - override fun onDeleteTimeRange() { - historyController.handleDeleteTimeRange() - } - - override fun onDeleteSome(items: Set) { - historyController.handleDeleteSome(items) - } - - override fun onDeleteTimeRangeConfirmed(timeFrame: RemoveTimeFrame?) { - historyController.handleDeleteTimeRangeConfirmed(timeFrame) - } - - override fun onRequestSync() { - historyController.handleRequestSync() - } - - override fun onRecentlyClosedClicked() { - historyController.handleEnterRecentlyClosed() - } -} diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt index 1a958fd57e..d47430bcbc 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt @@ -11,8 +11,6 @@ import androidx.core.view.isVisible import androidx.paging.LoadState import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.SimpleItemAnimator -import mozilla.components.support.base.feature.UserInteractionHandler -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R import org.mozilla.fenix.components.appstate.AppState import org.mozilla.fenix.databinding.ComponentHistoryBinding @@ -25,11 +23,10 @@ import org.mozilla.fenix.theme.ThemeManager */ class HistoryView( container: ViewGroup, - val interactor: HistoryInteractor, val store: HistoryFragmentStore, val onZeroItemsLoaded: () -> Unit, val onEmptyStateChanged: (Boolean) -> Unit, -) : LibraryPageView(container), UserInteractionHandler { +) : LibraryPageView(container) { val binding = ComponentHistoryBinding.inflate( LayoutInflater.from(container.context), @@ -40,7 +37,7 @@ class HistoryView( var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal private set - val historyAdapter = HistoryAdapter(interactor, store) { isEmpty -> + val historyAdapter = HistoryAdapter(store) { isEmpty -> onEmptyStateChanged(isEmpty) }.apply { addLoadStateListener { @@ -71,17 +68,11 @@ class HistoryView( val primaryTextColor = ThemeManager.resolveAttribute(R.attr.textPrimary, context) binding.swipeRefresh.setColorSchemeColors(primaryTextColor) binding.swipeRefresh.setOnRefreshListener { - if (FeatureFlags.historyFragmentLibStateRefactor) { - store.dispatch(HistoryFragmentAction.StartSync) - } else { - interactor.onRequestSync() - } + store.dispatch(HistoryFragmentAction.StartSync) } } fun update(state: HistoryFragmentState) { - val oldMode = mode - binding.progressBar.isVisible = state.isDeletingItems binding.swipeRefresh.isRefreshing = state.mode === HistoryFragmentState.Mode.Syncing binding.swipeRefresh.isEnabled = @@ -99,10 +90,6 @@ class HistoryView( val last = layoutManager.findLastVisibleItemPosition() + 1 historyAdapter.notifyItemRangeChanged(first, last - first) - if (state.mode::class != oldMode::class && !FeatureFlags.historyFragmentLibStateRefactor) { - interactor.onModeSwitched() - } - when (val mode = state.mode) { is HistoryFragmentState.Mode.Normal -> { setUiForNormalMode( @@ -135,11 +122,7 @@ class HistoryView( with(binding.recentlyClosedNavEmpty) { recentlyClosedNav.setOnClickListener { - if (FeatureFlags.historyFragmentLibStateRefactor) { - store.dispatch(HistoryFragmentAction.EnterRecentlyClosed) - } else { - interactor.onRecentlyClosedClicked() - } + store.dispatch(HistoryFragmentAction.EnterRecentlyClosed) } val numRecentTabs = recentlyClosedNav.context.components.core.store.state.closedTabs.size recentlyClosedTabsDescription.text = String.format( @@ -158,8 +141,4 @@ class HistoryView( binding.historyEmptyView.announceForAccessibility(context.getString(R.string.history_empty_message)) } } - - override fun onBackPressed(): Boolean { - return interactor.onBackPressed() - } } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt index 06b65e45f7..154cee6361 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt @@ -7,7 +7,6 @@ package org.mozilla.fenix.library.history.viewholders import android.view.View import androidx.core.view.isVisible import androidx.recyclerview.widget.RecyclerView -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R import org.mozilla.fenix.databinding.HistoryListItemBinding import org.mozilla.fenix.ext.components @@ -17,13 +16,11 @@ import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.history.HistoryFragmentAction import org.mozilla.fenix.library.history.HistoryFragmentState import org.mozilla.fenix.library.history.HistoryFragmentStore -import org.mozilla.fenix.library.history.HistoryInteractor import org.mozilla.fenix.library.history.HistoryItemTimeGroup import org.mozilla.fenix.selection.SelectionHolder class HistoryListItemViewHolder( view: View, - private val historyInteractor: HistoryInteractor, private val selectionHolder: SelectionHolder, private val store: HistoryFragmentStore, ) : RecyclerView.ViewHolder(view) { @@ -33,11 +30,7 @@ class HistoryListItemViewHolder( init { binding.recentlyClosedNavEmpty.recentlyClosedNav.setOnClickListener { - if (FeatureFlags.historyFragmentLibStateRefactor) { - store.dispatch(HistoryFragmentAction.EnterRecentlyClosed) - } else { - historyInteractor.onRecentlyClosedClicked() - } + store.dispatch(HistoryFragmentAction.EnterRecentlyClosed) } binding.historyLayout.overflowView.apply { @@ -45,11 +38,7 @@ class HistoryListItemViewHolder( contentDescription = view.context.getString(R.string.history_delete_item) setOnClickListener { val item = item ?: return@setOnClickListener - if (FeatureFlags.historyFragmentLibStateRefactor) { - store.dispatch(HistoryFragmentAction.DeleteItems(setOf(item))) - } else { - historyInteractor.onDeleteSome(setOf(item)) - } + store.dispatch(HistoryFragmentAction.DeleteItems(setOf(item))) } } } @@ -97,16 +86,12 @@ class HistoryListItemViewHolder( val headerText = timeGroup?.humanReadable(itemView.context) toggleHeader(headerText) - if (FeatureFlags.historyFragmentLibStateRefactor) { - binding.historyLayout.setOnClickListener { - store.dispatch(HistoryFragmentAction.HistoryItemClicked(item)) - } - binding.historyLayout.setOnLongClickListener { - store.dispatch(HistoryFragmentAction.HistoryItemLongClicked(item)) - true - } - } else { - binding.historyLayout.setSelectionInteractor(item, selectionHolder, historyInteractor) + binding.historyLayout.setOnClickListener { + store.dispatch(HistoryFragmentAction.HistoryItemClicked(item)) + } + binding.historyLayout.setOnLongClickListener { + store.dispatch(HistoryFragmentAction.HistoryItemLongClicked(item)) + true } binding.historyLayout.changeSelected(item in selectionHolder.selectedItems) diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt deleted file mode 100644 index f75f22b0e6..0000000000 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt +++ /dev/null @@ -1,272 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.library.history - -import androidx.navigation.NavController -import io.mockk.coVerifyOrder -import io.mockk.every -import io.mockk.mockk -import io.mockk.verify -import mozilla.components.browser.state.action.EngineAction -import mozilla.components.browser.state.action.RecentlyClosedAction -import mozilla.components.browser.state.store.BrowserStore -import mozilla.components.browser.storage.sync.PlacesHistoryStorage -import mozilla.components.service.glean.testing.GleanTestRule -import mozilla.components.support.test.robolectric.testContext -import mozilla.components.support.test.rule.MainCoroutineRule -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.R -import org.mozilla.fenix.components.AppStore -import org.mozilla.fenix.components.history.DefaultPagedHistoryProvider -import org.mozilla.fenix.ext.navigateSafe -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner - -@RunWith(FenixRobolectricTestRunner::class) -class HistoryControllerTest { - private val historyItem = History.Regular( - 0, - "title", - "url", - 0.toLong(), - HistoryItemTimeGroup.timeGroupForTimestamp(0), - ) - - @get:Rule - val gleanTestRule = GleanTestRule(testContext) - - @get:Rule - val coroutinesTestRule = MainCoroutineRule() - private val scope = coroutinesTestRule.scope - - private val store: HistoryFragmentStore = mockk(relaxed = true) - private val appStore: AppStore = mockk(relaxed = true) - private val browserStore: BrowserStore = mockk(relaxed = true) - private val historyStorage: PlacesHistoryStorage = mockk(relaxed = true) - private val state: HistoryFragmentState = mockk(relaxed = true) - private val navController: NavController = mockk(relaxed = true) - private val historyProvider: DefaultPagedHistoryProvider = mockk(relaxed = true) - - @Before - fun setUp() { - every { store.state } returns state - } - - @Test - fun onPressHistoryItemInNormalMode() { - var actualHistoryItem: History? = null - val controller = createController( - openInBrowser = { - actualHistoryItem = it - }, - ) - controller.handleOpen(historyItem) - assertEquals(historyItem, actualHistoryItem) - } - - @Test - fun onPressHistoryItemInEditMode() { - every { state.mode } returns HistoryFragmentState.Mode.Editing(setOf()) - - createController().handleSelect(historyItem) - - verify { - store.dispatch(HistoryFragmentAction.AddItemForRemoval(historyItem)) - } - } - - @Test - fun onPressSelectedHistoryItemInEditMode() { - every { state.mode } returns HistoryFragmentState.Mode.Editing(setOf(historyItem)) - - createController().handleDeselect(historyItem) - - verify { - store.dispatch(HistoryFragmentAction.RemoveItemForRemoval(historyItem)) - } - } - - @Test - fun onSelectHistoryItemDuringSync() { - every { state.mode } returns HistoryFragmentState.Mode.Syncing - - createController().handleSelect(historyItem) - - verify(exactly = 0) { - store.dispatch(HistoryFragmentAction.AddItemForRemoval(historyItem)) - } - } - - @Test - fun onBackPressedInNormalMode() { - every { state.mode } returns HistoryFragmentState.Mode.Normal - - assertFalse(createController().handleBackPressed()) - } - - @Test - fun onBackPressedInEditMode() { - every { state.mode } returns HistoryFragmentState.Mode.Editing(setOf()) - - assertTrue(createController().handleBackPressed()) - verify { - store.dispatch(HistoryFragmentAction.ExitEditMode) - } - } - - @Test - fun onModeSwitched() { - var invalidateOptionsMenuInvoked = false - val controller = createController( - invalidateOptionsMenu = { - invalidateOptionsMenuInvoked = true - }, - ) - - controller.handleModeSwitched() - assertTrue(invalidateOptionsMenuInvoked) - } - - @Test - fun onSearch() { - val controller = createController() - - controller.handleSearch() - verify { - navController.navigateSafe( - R.id.historyFragment, - HistoryFragmentDirections.actionGlobalSearchDialog(sessionId = null), - ) - } - } - - @Test - fun onDeleteTimeRange() { - var displayDeleteTimeRangeInvoked = false - val controller = createController( - displayDeleteTimeRange = { - displayDeleteTimeRangeInvoked = true - }, - ) - - controller.handleDeleteTimeRange() - assertTrue(displayDeleteTimeRangeInvoked) - } - - @Test - fun `WHEN user confirms history deletion GIVEN timeFrame is null THEN delete all history, log the event, purge history and remove recently closed items`() { - val controller = createController() - assertNull(org.mozilla.fenix.GleanMetrics.History.removedAll.testGetValue()) - - controller.handleDeleteTimeRangeConfirmed(null) - coVerifyOrder { - store.dispatch(HistoryFragmentAction.EnterDeletionMode) - historyStorage.deleteEverything() - browserStore.dispatch(RecentlyClosedAction.RemoveAllClosedTabAction) - browserStore.dispatch(EngineAction.PurgeHistoryAction) - store.dispatch(HistoryFragmentAction.ExitDeletionMode) - } - - assertNotNull(org.mozilla.fenix.GleanMetrics.History.removedAll.testGetValue()) - } - - @Test - fun `WHEN user confirms history deletion GIVEN timeFrame is lastHour THEN delete visits between the time frame, log the event, purge history and remove recently closed items`() { - val controller = createController() - assertNull(org.mozilla.fenix.GleanMetrics.History.removedLastHour.testGetValue()) - - controller.handleDeleteTimeRangeConfirmed(RemoveTimeFrame.LastHour) - coVerifyOrder { - store.dispatch(HistoryFragmentAction.EnterDeletionMode) - historyStorage.deleteVisitsBetween(any(), any()) - browserStore.dispatch(RecentlyClosedAction.RemoveAllClosedTabAction) - browserStore.dispatch(EngineAction.PurgeHistoryAction) - store.dispatch(HistoryFragmentAction.ExitDeletionMode) - } - - assertNotNull(org.mozilla.fenix.GleanMetrics.History.removedLastHour.testGetValue()) - } - - @Test - fun `WHEN user confirms history deletion GIVEN timeFrame is todayAndYesterday THEN delete visits between the time frame, log the event, purge history and remove recently closed items`() { - val controller = createController() - assertNull(org.mozilla.fenix.GleanMetrics.History.removedTodayAndYesterday.testGetValue()) - - controller.handleDeleteTimeRangeConfirmed(RemoveTimeFrame.TodayAndYesterday) - coVerifyOrder { - store.dispatch(HistoryFragmentAction.EnterDeletionMode) - historyStorage.deleteVisitsBetween(any(), any()) - browserStore.dispatch(RecentlyClosedAction.RemoveAllClosedTabAction) - browserStore.dispatch(EngineAction.PurgeHistoryAction) - store.dispatch(HistoryFragmentAction.ExitDeletionMode) - } - - assertNotNull(org.mozilla.fenix.GleanMetrics.History.removedTodayAndYesterday.testGetValue()) - } - - @Test - fun onDeleteSome() { - val itemsToDelete = setOf(historyItem) - var actualItems: Set? = null - val controller = createController( - deleteHistoryItems = { items -> - actualItems = items - }, - ) - - controller.handleDeleteSome(itemsToDelete) - assertEquals(itemsToDelete, actualItems) - } - - @Test - fun onRequestSync() { - var syncHistoryInvoked = false - createController( - syncHistory = { - syncHistoryInvoked = true - }, - ).handleRequestSync() - - coVerifyOrder { - store.dispatch(HistoryFragmentAction.StartSync) - store.dispatch(HistoryFragmentAction.FinishSync) - } - - assertTrue(syncHistoryInvoked) - } - - @Suppress("LongParameterList") - private fun createController( - openInBrowser: (History) -> Unit = { _ -> }, - displayDeleteTimeRange: () -> Unit = {}, - onTimeFrameDeleted: () -> Unit = {}, - invalidateOptionsMenu: () -> Unit = {}, - deleteHistoryItems: (Set) -> Unit = { _ -> }, - syncHistory: suspend () -> Unit = {}, - ): HistoryController { - return DefaultHistoryController( - store, - appStore, - browserStore, - historyStorage, - historyProvider, - navController, - scope, - openInBrowser, - displayDeleteTimeRange, - onTimeFrameDeleted, - invalidateOptionsMenu, - { items, _, _ -> deleteHistoryItems.invoke(items) }, - syncHistory, - ) - } -} diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt deleted file mode 100644 index 58790ee73e..0000000000 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt +++ /dev/null @@ -1,128 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.library.history - -import io.mockk.every -import io.mockk.mockk -import io.mockk.verifyAll -import mozilla.components.service.glean.testing.GleanTestRule -import mozilla.components.support.test.robolectric.testContext -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull -import org.junit.Assert.assertTrue -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner -import org.mozilla.fenix.GleanMetrics.History as GleanHistory - -@RunWith(FenixRobolectricTestRunner::class) // For GleanTestRule -class HistoryInteractorTest { - private val historyItem = History.Regular(0, "title", "url", 0.toLong(), HistoryItemTimeGroup.timeGroupForTimestamp(0)) - val controller: HistoryController = mockk(relaxed = true) - val interactor = DefaultHistoryInteractor(controller) - - @get:Rule - val gleanTestRule = GleanTestRule(testContext) - - @Test - fun onOpen() { - interactor.open(historyItem) - - verifyAll { - controller.handleOpen(historyItem) - } - } - - @Test - fun onSelect() { - interactor.select(historyItem) - - verifyAll { - controller.handleSelect(historyItem) - } - } - - @Test - fun onDeselect() { - interactor.deselect(historyItem) - - verifyAll { - controller.handleDeselect(historyItem) - } - } - - @Test - fun onBackPressed() { - every { - controller.handleBackPressed() - } returns true - - val backpressHandled = interactor.onBackPressed() - - verifyAll { - controller.handleBackPressed() - } - assertTrue(backpressHandled) - } - - @Test - fun onModeSwitched() { - interactor.onModeSwitched() - - verifyAll { - controller.handleModeSwitched() - } - } - - @Test - fun onSearch() { - assertNull(GleanHistory.searchIconTapped.testGetValue()) - interactor.onSearch() - - verifyAll { - controller.handleSearch() - } - assertNotNull(GleanHistory.searchIconTapped.testGetValue()) - } - - @Test - fun onDeleteTimeRange() { - interactor.onDeleteTimeRange() - - verifyAll { - controller.handleDeleteTimeRange() - } - } - - @Test - fun onDeleteTimeRangeConfirmed() { - interactor.onDeleteTimeRangeConfirmed(null) - - verifyAll { - controller.handleDeleteTimeRangeConfirmed(null) - } - } - - @Test - fun onDeleteSome() { - val items = setOf(historyItem) - - interactor.onDeleteSome(items) - - verifyAll { - controller.handleDeleteSome(items) - } - } - - @Test - fun onRequestSync() { - interactor.onRequestSync() - - verifyAll { - controller.handleRequestSync() - } - } -}