From 645674c9bd19668c8b4cd3f8d4a8c60440ae1280 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Mon, 19 Aug 2019 18:34:57 +0300 Subject: [PATCH] Closes #4396 - Add a Bookmarks Controller (#4593) * For #4396 - Rename BookmarkInteractor methods Following the naming model used in other Interactors this too will use reactive method names in the form of "on..." instead of the previous imperative model. Kept the imperative naming model for the methods from `SelectionInteractor` as they are a new addition and I'm not sure about the future direction. * For #4396 - Add a BookmarkController It abstracts the Fragment behavior in a contract through which various Interactors can inform about the specific View changes and can ask for modifications in their container Fragment. This contract and it's implementation - `DefaultBookmarkController` are the result of extracting the container Fragment's business logic from `BookmarkFragmentInteractor` in it's own standalone component. * For #4396 - Refactored Bookmark related tests Added a new `BookmarkControllerTest` tests class which complements the new `BookmarkController` to ensure that it properly operates on `BookmarkFragment` Also refactored the existing `BookmarkFragmentInteractorTest` to accommodate `BookmarkFragmentInteractor`'s now more specialized behavior. --- .../library/bookmarks/BookmarkController.kt | 118 ++++++++++ .../library/bookmarks/BookmarkFragment.kt | 38 +-- .../bookmarks/BookmarkFragmentInteractor.kt | 173 ++++++-------- .../fenix/library/bookmarks/BookmarkView.kt | 28 +-- .../fenix/library/bookmarks/SignInView.kt | 8 +- .../SelectBookmarkFolderFragment.kt | 8 +- .../SelectBookmarkFolderInteractor.kt | 6 +- .../viewholders/BookmarkNodeViewHolder.kt | 12 +- .../bookmarks/BookmarkControllerTest.kt | 217 ++++++++++++++++++ .../BookmarkFragmentInteractorTest.kt | 146 ++++-------- 10 files changed, 493 insertions(+), 261 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt create mode 100644 app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt new file mode 100644 index 000000000..9b631e147 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt @@ -0,0 +1,118 @@ +/* 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.bookmarks + +import android.content.Context +import android.content.res.Resources +import androidx.navigation.NavController +import androidx.navigation.NavDirections +import mozilla.components.concept.storage.BookmarkNode +import org.mozilla.fenix.BrowserDirection +import org.mozilla.fenix.BrowsingMode +import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.R +import org.mozilla.fenix.components.FenixSnackbarPresenter +import org.mozilla.fenix.components.Services +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.copyUrl +import org.mozilla.fenix.ext.nav + +/** + * [BookmarkFragment] controller. + * Delegated by View Interactors, handles container business logic and operates changes on it. + */ +@SuppressWarnings("TooManyFunctions") +interface BookmarkController { + fun handleBookmarkTapped(item: BookmarkNode) + fun handleBookmarkExpand(folder: BookmarkNode) + fun handleSelectionModeSwitch() + fun handleBookmarkEdit(node: BookmarkNode) + fun handleBookmarkSelected(node: BookmarkNode) + fun handleCopyUrl(item: BookmarkNode) + fun handleBookmarkSharing(item: BookmarkNode) + fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) + fun handleBookmarkDeletion(nodes: Set, eventType: Event) + fun handleBackPressed() + fun handleSigningIn() +} + +@SuppressWarnings("TooManyFunctions") +class DefaultBookmarkController( + private val context: Context, + private val navController: NavController, + private val snackbarPresenter: FenixSnackbarPresenter, + private val deleteBookmarkNodes: (Set, Event) -> Unit +) : BookmarkController { + + private val activity: HomeActivity = context as HomeActivity + private val resources: Resources = context.resources + private val services: Services = activity.components.services + + override fun handleBookmarkTapped(item: BookmarkNode) { + openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, BrowsingMode.Normal) + } + + override fun handleBookmarkExpand(folder: BookmarkNode) { + navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(folder.guid)) + } + + override fun handleSelectionModeSwitch() { + activity.invalidateOptionsMenu() + } + + override fun handleBookmarkEdit(node: BookmarkNode) { + navigate(BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(node.guid)) + } + + override fun handleBookmarkSelected(node: BookmarkNode) { + snackbarPresenter.present(resources.getString(R.string.bookmark_cannot_edit_root)) + } + + override fun handleCopyUrl(item: BookmarkNode) { + item.copyUrl(context) + snackbarPresenter.present(resources.getString(R.string.url_copied)) + } + + override fun handleBookmarkSharing(item: BookmarkNode) { + navigate(BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment( + url = item.url!!, + title = item.title + ) + ) + } + + override fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) { + openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, mode) + } + + override fun handleBookmarkDeletion(nodes: Set, eventType: Event) { + deleteBookmarkNodes(nodes, eventType) + } + + override fun handleBackPressed() { + navController.popBackStack() + } + + override fun handleSigningIn() { + services.launchPairingSignIn(context, navController) + } + + private fun openInNewTab( + searchTermOrURL: String, + newTab: Boolean, + from: BrowserDirection, + mode: BrowsingMode + ) { + with(activity) { + browsingModeManager.mode = mode + openToBrowserAndLoad(searchTermOrURL, newTab, from) + } + } + + private fun navigate(directions: NavDirections) { + navController.nav(R.id.bookmarkFragment, directions) + } +} 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 b65b35bab..d6425039c 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 @@ -64,9 +64,10 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou private val onDestinationChangedListener = NavController.OnDestinationChangedListener { _, destination, args -> if (destination.id != R.id.bookmarkFragment || - args != null && BookmarkFragmentArgs.fromBundle(args).currentRoot != currentRoot?.guid - ) - bookmarkInteractor.deselectAll() + args != null && BookmarkFragmentArgs.fromBundle(args).currentRoot != currentRoot?.guid) { + + bookmarkInteractor.onAllBookmarksDeselected() + } } lateinit var initialJob: Job private var pendingBookmarkDeletionJob: (suspend () -> Unit)? = null @@ -84,12 +85,15 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou BookmarkStore(BookmarkState(null)) } bookmarkInteractor = BookmarkFragmentInteractor( - context!!, - findNavController(), - bookmarkStore, - sharedViewModel, - FenixSnackbarPresenter(view), - ::deleteMulti + bookmarkStore = bookmarkStore, + viewModel = sharedViewModel, + bookmarksController = DefaultBookmarkController( + context = context!!, + navController = findNavController(), + snackbarPresenter = FenixSnackbarPresenter(view), + deleteBookmarkNodes = ::deleteMulti + ), + metrics = metrics!! ) bookmarkView = BookmarkView(view.bookmarkLayout, bookmarkInteractor) @@ -137,7 +141,7 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou if (!isActive) return@launch launch(Main) { - bookmarkInteractor.change(currentRoot!!) + bookmarkInteractor.onBookmarksChanged(currentRoot!!) sharedViewModel.selectedFolder = currentRoot } } @@ -146,8 +150,8 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou private fun checkIfSignedIn() { context?.components?.backgroundServices?.accountManager?.let { it.register(this, owner = this) - it.authenticatedAccount()?.let { bookmarkInteractor.signedIn() } - ?: bookmarkInteractor.signedOut() + it.authenticatedAccount()?.let { bookmarkInteractor.onSignedIn() } + ?: bookmarkInteractor.onSignedOut() } } @@ -226,14 +230,14 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou override fun onBackPressed(): Boolean = bookmarkView.onBackPressed() override fun onAuthenticated(account: OAuthAccount, newAccount: Boolean) { - bookmarkInteractor.signedIn() + bookmarkInteractor.onSignedIn() lifecycleScope.launch { refreshBookmarks() } } override fun onLoggedOut() { - bookmarkInteractor.signedOut() + bookmarkInteractor.onSignedOut() } private suspend fun refreshBookmarks() { @@ -247,7 +251,7 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou pendingBookmarksToDelete.forEach { rootNode -= it.guid } - bookmarkInteractor.change(rootNode) + bookmarkInteractor.onBookmarksChanged(rootNode) } } @@ -269,7 +273,7 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou pendingBookmarksToDelete.forEach { bookmarkTree -= it.guid } - bookmarkInteractor.change(bookmarkTree!!) + bookmarkInteractor.onBookmarksChanged(bookmarkTree!!) val deleteOperation: (suspend () -> Unit) = { deleteSelectedBookmarks(selected) @@ -293,7 +297,7 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou bookmarkNode.url?.urlToTrimmedHost(context!!) ?: bookmarkNode.title ) } - else -> throw IllegalStateException("Illegal event type in deleteMulti") + else -> throw IllegalStateException("Illegal event type in onDeleteSome") } lifecycleScope.allowUndo( diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt index 3f7c40c42..ec9850cfd 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt @@ -4,142 +4,79 @@ package org.mozilla.fenix.library.bookmarks -import android.content.Context -import androidx.navigation.NavController import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType -import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.BrowsingMode -import org.mozilla.fenix.HomeActivity -import org.mozilla.fenix.R -import org.mozilla.fenix.components.FenixSnackbarPresenter import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController -import org.mozilla.fenix.ext.asActivity -import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.copyUrl -import org.mozilla.fenix.ext.nav +import org.mozilla.fenix.lib.Do /** * Interactor for the Bookmarks screen. * Provides implementations for the BookmarkViewInteractor. * - * @property context The current Android Context - * @property navController The Android Navigation NavController - * @property bookmarkStore The BookmarkStore - * @property sharedViewModel The shared ViewModel used between the Bookmarks screens - * @property snackbarPresenter A presenter for the FenixSnackBar - * @property deleteBookmarkNodes A lambda function for deleting bookmark nodes with undo + * @property bookmarkStore bookmarks state + * @property viewModel view state + * @property bookmarksController view controller + * @property metrics telemetry controller */ @SuppressWarnings("TooManyFunctions") class BookmarkFragmentInteractor( - private val context: Context, - private val navController: NavController, private val bookmarkStore: BookmarkStore, - private val sharedViewModel: BookmarksSharedViewModel, - private val snackbarPresenter: FenixSnackbarPresenter, - private val deleteBookmarkNodes: (Set, Event) -> Unit + private val viewModel: BookmarksSharedViewModel, + private val bookmarksController: BookmarkController, + private val metrics: MetricController ) : BookmarkViewInteractor, SignInInteractor { - val activity: HomeActivity? - get() = context.asActivity() as? HomeActivity - val metrics: MetricController - get() = context.components.analytics.metrics - - override fun change(node: BookmarkNode) { + override fun onBookmarksChanged(node: BookmarkNode) { bookmarkStore.dispatch(BookmarkAction.Change(node)) } - override fun open(item: BookmarkNode) { - when (item.type) { - BookmarkNodeType.ITEM -> openItem(item) - BookmarkNodeType.FOLDER -> { - navController.nav( - R.id.bookmarkFragment, - BookmarkFragmentDirections.actionBookmarkFragmentSelf(item.guid) - ) - } - BookmarkNodeType.SEPARATOR -> throw IllegalStateException("Cannot open separators") - } + override fun onSelectionModeSwitch(mode: BookmarkState.Mode) { + bookmarksController.handleSelectionModeSwitch() } - override fun switchMode(mode: BookmarkState.Mode) { - activity?.invalidateOptionsMenu() + override fun onEditPressed(node: BookmarkNode) { + bookmarksController.handleBookmarkEdit(node) } - override fun edit(node: BookmarkNode) { - navController.nav( - R.id.bookmarkFragment, - BookmarkFragmentDirections - .actionBookmarkFragmentToBookmarkEditFragment(node.guid) - ) - } - - override fun select(item: BookmarkNode) { - if (item.inRoots()) { - snackbarPresenter.present(context.getString(R.string.bookmark_cannot_edit_root)) - return - } - bookmarkStore.dispatch(BookmarkAction.Select(item)) - } - - override fun deselect(item: BookmarkNode) { - bookmarkStore.dispatch(BookmarkAction.Deselect(item)) - } - - override fun deselectAll() { + override fun onAllBookmarksDeselected() { bookmarkStore.dispatch(BookmarkAction.DeselectAll) } - override fun copy(item: BookmarkNode) { + override fun onCopyPressed(item: BookmarkNode) { require(item.type == BookmarkNodeType.ITEM) - item.copyUrl(activity!!) - snackbarPresenter.present(context.getString(R.string.url_copied)) - metrics.track(Event.CopyBookmark) + item.url?.let { + bookmarksController.handleCopyUrl(item) + metrics.track(Event.CopyBookmark) + } } - override fun share(item: BookmarkNode) { + override fun onSharePressed(item: BookmarkNode) { require(item.type == BookmarkNodeType.ITEM) - item.url?.apply { - navController.nav( - R.id.bookmarkFragment, - BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment( - url = this, - title = item.title - ) - ) + item.url?.let { + bookmarksController.handleBookmarkSharing(item) metrics.track(Event.ShareBookmark) } } - override fun openInNewTab(item: BookmarkNode) { - openItem(item, BrowsingMode.Normal) - } - - override fun openInPrivateTab(item: BookmarkNode) { - openItem(item, BrowsingMode.Private) - } - - private fun openItem(item: BookmarkNode, tabMode: BrowsingMode? = null) { + override fun onOpenInNormalTab(item: BookmarkNode) { require(item.type == BookmarkNodeType.ITEM) - item.url?.let { url -> - tabMode?.let { activity?.browsingModeManager?.mode = it } - activity?.openToBrowserAndLoad( - searchTermOrURL = url, - newTab = true, - from = BrowserDirection.FromBookmarks - ) - metrics.track( - when (tabMode) { - BrowsingMode.Private -> Event.OpenedBookmarkInPrivateTab - BrowsingMode.Normal -> Event.OpenedBookmarkInNewTab - null -> Event.OpenedBookmark - } - ) + item.url?.let { + bookmarksController.handleOpeningBookmark(item, BrowsingMode.Normal) + metrics.track(Event.OpenedBookmarkInNewTab) } } - override fun delete(nodes: Set) { + override fun onOpenInPrivateTab(item: BookmarkNode) { + require(item.type == BookmarkNodeType.ITEM) + item.url?.let { + bookmarksController.handleOpeningBookmark(item, BrowsingMode.Private) + metrics.track(Event.OpenedBookmarkInPrivateTab) + } + } + + override fun onDelete(nodes: Set) { if (nodes.find { it.type == BookmarkNodeType.SEPARATOR } != null) { throw IllegalStateException("Cannot delete separators") } @@ -149,22 +86,44 @@ class BookmarkFragmentInteractor( BookmarkNodeType.FOLDER -> Event.RemoveBookmarkFolder null -> Event.RemoveBookmarks } - deleteBookmarkNodes(nodes, eventType) + bookmarksController.handleBookmarkDeletion(nodes, eventType) } - override fun backPressed() { - navController.popBackStack() + override fun onBackPressed() { + bookmarksController.handleBackPressed() } - override fun clickedSignIn() { - context.components.services.launchPairingSignIn(context, navController) + override fun onSignInPressed() { + bookmarksController.handleSigningIn() } - override fun signedIn() { - sharedViewModel.signedIn.postValue(true) + override fun onSignedIn() { + viewModel.signedIn.postValue(true) } - override fun signedOut() { - sharedViewModel.signedIn.postValue(false) + override fun onSignedOut() { + viewModel.signedIn.postValue(false) + } + + override fun open(item: BookmarkNode) { + Do exhaustive when (item.type) { + BookmarkNodeType.ITEM -> { + bookmarksController.handleBookmarkTapped(item) + metrics.track(Event.OpenedBookmark) + } + BookmarkNodeType.FOLDER -> bookmarksController.handleBookmarkExpand(item) + BookmarkNodeType.SEPARATOR -> throw IllegalStateException("Cannot open separators") + } + } + + override fun select(item: BookmarkNode) { + when (item.inRoots()) { + true -> bookmarksController.handleBookmarkSelected(item) + false -> bookmarkStore.dispatch(BookmarkAction.Select(item)) + } + } + + override fun deselect(item: BookmarkNode) { + bookmarkStore.dispatch(BookmarkAction.Deselect(item)) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt index 9c8f993ad..248e9fc83 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt @@ -27,68 +27,68 @@ interface BookmarkViewInteractor : SelectionInteractor { * * @param node the head node of the new bookmarks tree */ - fun change(node: BookmarkNode) + fun onBookmarksChanged(node: BookmarkNode) /** * Switches the current bookmark multi-selection mode. * * @param mode the multi-select mode to switch to */ - fun switchMode(mode: BookmarkState.Mode) + fun onSelectionModeSwitch(mode: BookmarkState.Mode) /** * Opens up an interface to edit a bookmark node. * * @param node the bookmark node to edit */ - fun edit(node: BookmarkNode) + fun onEditPressed(node: BookmarkNode) /** * De-selects all bookmark nodes, clearing the multi-selection mode. * */ - fun deselectAll() + fun onAllBookmarksDeselected() /** * Copies the URL of a bookmark item to the copy-paste buffer. * * @param item the bookmark item to copy the URL from */ - fun copy(item: BookmarkNode) + fun onCopyPressed(item: BookmarkNode) /** * Opens the share sheet for a bookmark item. * * @param item the bookmark item to share */ - fun share(item: BookmarkNode) + fun onSharePressed(item: BookmarkNode) /** * Opens a bookmark item in a new tab. * * @param item the bookmark item to open in a new tab */ - fun openInNewTab(item: BookmarkNode) + fun onOpenInNormalTab(item: BookmarkNode) /** * Opens a bookmark item in a private tab. * * @param item the bookmark item to open in a private tab */ - fun openInPrivateTab(item: BookmarkNode) + fun onOpenInPrivateTab(item: BookmarkNode) /** - * Deletes a set of bookmark node. + * Deletes a set of bookmark nodes. * * @param nodes the bookmark nodes to delete */ - fun delete(nodes: Set) + fun onDelete(nodes: Set) /** * Handles back presses for the bookmark screen, so navigation up the tree is possible. * */ - fun backPressed() + fun onBackPressed() } class BookmarkView( @@ -117,7 +117,7 @@ class BookmarkView( tree = state.tree if (state.mode != mode) { mode = state.mode - interactor.switchMode(mode) + interactor.onSelectionModeSwitch(mode) } bookmarkAdapter.updateData(state.tree, mode) @@ -132,11 +132,11 @@ class BookmarkView( override fun onBackPressed(): Boolean { return when { mode is BookmarkState.Mode.Selecting -> { - interactor.deselectAll() + interactor.onAllBookmarksDeselected() true } canGoBack -> { - interactor.backPressed() + interactor.onBackPressed() true } else -> false diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/SignInView.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/SignInView.kt index fcc8e4250..80693a1f7 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/SignInView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/SignInView.kt @@ -12,9 +12,9 @@ import kotlinx.android.extensions.LayoutContainer import org.mozilla.fenix.R interface SignInInteractor { - fun clickedSignIn() - fun signedIn() - fun signedOut() + fun onSignInPressed() + fun onSignedIn() + fun onSignedOut() } class SignInView( @@ -31,7 +31,7 @@ class SignInView( init { view.setOnClickListener { - interactor.clickedSignIn() + interactor.onSignInPressed() } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt index 4d97a0b4e..a662358c8 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt @@ -96,8 +96,8 @@ class SelectBookmarkFolderFragment : Fragment(), AccountObserver { private fun checkIfSignedIn() { val accountManager = requireComponents.backgroundServices.accountManager accountManager.register(this, owner = this) - accountManager.authenticatedAccount()?.let { bookmarkInteractor.signedIn() } - ?: bookmarkInteractor.signedOut() + accountManager.authenticatedAccount()?.let { bookmarkInteractor.onSignedIn() } + ?: bookmarkInteractor.onSignedOut() } override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) { @@ -125,10 +125,10 @@ class SelectBookmarkFolderFragment : Fragment(), AccountObserver { } } override fun onAuthenticated(account: OAuthAccount, newAccount: Boolean) { - bookmarkInteractor.signedIn() + bookmarkInteractor.onSignedIn() } override fun onLoggedOut() { - bookmarkInteractor.signedOut() + bookmarkInteractor.onSignedOut() } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderInteractor.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderInteractor.kt index 2d198cacc..fecec4567 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderInteractor.kt @@ -16,15 +16,15 @@ class SelectBookmarkFolderInteractor( private val sharedViewModel: BookmarksSharedViewModel ) : SignInInteractor { - override fun clickedSignIn() { + override fun onSignInPressed() { context.components.services.launchPairingSignIn(context, navController) } - override fun signedIn() { + override fun onSignedIn() { sharedViewModel.signedIn.postValue(true) } - override fun signedOut() { + override fun onSignedOut() { sharedViewModel.signedIn.postValue(false) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt index 3780a50cf..a22fc264b 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt @@ -30,13 +30,13 @@ abstract class BookmarkNodeViewHolder( protected fun setupMenu(item: BookmarkNode) { val bookmarkItemMenu = BookmarkItemMenu(containerView.context, item) { when (it) { - BookmarkItemMenu.Item.Edit -> interactor.edit(item) + BookmarkItemMenu.Item.Edit -> interactor.onEditPressed(item) BookmarkItemMenu.Item.Select -> interactor.select(item) - BookmarkItemMenu.Item.Copy -> interactor.copy(item) - BookmarkItemMenu.Item.Share -> interactor.share(item) - BookmarkItemMenu.Item.OpenInNewTab -> interactor.openInNewTab(item) - BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.openInPrivateTab(item) - BookmarkItemMenu.Item.Delete -> interactor.delete(setOf(item)) + BookmarkItemMenu.Item.Copy -> interactor.onCopyPressed(item) + BookmarkItemMenu.Item.Share -> interactor.onSharePressed(item) + BookmarkItemMenu.Item.OpenInNewTab -> interactor.onOpenInNormalTab(item) + BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.onOpenInPrivateTab(item) + BookmarkItemMenu.Item.Delete -> interactor.onDelete(setOf(item)) } } diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt new file mode 100644 index 000000000..120da5a75 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt @@ -0,0 +1,217 @@ +/* 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.bookmarks + +import android.content.ClipData +import android.content.ClipboardManager +import android.content.Context +import androidx.core.content.getSystemService +import androidx.navigation.NavController +import androidx.navigation.NavDestination +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.verify +import io.mockk.verifyOrder +import mozilla.appservices.places.BookmarkRoot +import mozilla.components.concept.storage.BookmarkNode +import mozilla.components.concept.storage.BookmarkNodeType +import org.junit.Before +import org.junit.Test +import org.mozilla.fenix.BrowserDirection +import org.mozilla.fenix.BrowsingMode +import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.R +import org.mozilla.fenix.components.FenixSnackbarPresenter +import org.mozilla.fenix.components.Services +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.components + +@SuppressWarnings("TooManyFunctions", "LargeClass") +class BookmarkControllerTest { + + private lateinit var controller: BookmarkController + + private val context: Context = mockk(relaxed = true) + private val navController: NavController = mockk(relaxed = true) + private val snackbarPresenter: FenixSnackbarPresenter = mockk(relaxed = true) + private val deleteBookmarkNodes: (Set, Event) -> Unit = mockk(relaxed = true) + + private val homeActivity: HomeActivity = mockk(relaxed = true) + private val services: Services = mockk(relaxed = true) + + private val item = BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0, "Mozilla", "http://mozilla.org", null) + private val subfolder = BookmarkNode(BookmarkNodeType.FOLDER, "987", "123", 0, "Subfolder", null, listOf()) + private val childItem = BookmarkNode( + BookmarkNodeType.ITEM, "987", "123", 2, "Firefox", "https://www.mozilla.org/en-US/firefox/", null + ) + private val tree = BookmarkNode( + BookmarkNodeType.FOLDER, "123", null, 0, "Mobile", null, listOf(item, item, childItem, subfolder) + ) + private val root = BookmarkNode( + BookmarkNodeType.FOLDER, BookmarkRoot.Root.id, null, 0, BookmarkRoot.Root.name, null, null + ) + + @Before + fun setup() { + // needed for mocking 'getSystemService()' + mockkStatic( + "androidx.core.content.ContextCompat", + "android.content.ClipData" + ) + + every { homeActivity.components.services } returns services + every { navController.currentDestination } returns NavDestination("").apply { id = R.id.bookmarkFragment } + + controller = DefaultBookmarkController( + context = homeActivity, + navController = navController, + snackbarPresenter = snackbarPresenter, + deleteBookmarkNodes = deleteBookmarkNodes + ) + } + + @Test + fun `handleBookmarkTapped should load the bookmark in a new tab`() { + controller.handleBookmarkTapped(item) + + verifyOrder { + homeActivity.browsingModeManager.mode = BrowsingMode.Normal + homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks) + } + } + + @Test + fun `handleBookmarkExpand should navigate to the 'Bookmark' fragment`() { + controller.handleBookmarkExpand(tree) + + verify { + navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(tree.guid)) + } + } + + @Test + fun `handleSelectionModeSwitch should invalidateOptionsMenu`() { + controller.handleSelectionModeSwitch() + + verify { + homeActivity.invalidateOptionsMenu() + } + } + + @Test + fun `handleBookmarkEdit should navigate to the 'Edit' fragment`() { + controller.handleBookmarkEdit(item) + + verify { + navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(item.guid)) + } + } + + @Test + fun `handleBookmarkSelected should show a toast when selecting a folder`() { + val errorMessage = context.getString(R.string.bookmark_cannot_edit_root) + + controller.handleBookmarkSelected(root) + + verify { + snackbarPresenter.present(errorMessage, any(), any(), any()) + } + } + + @Test + fun `handleCopyUrl should copy bookmark url to clipboard and show a toast`() { + val clipboardManager: ClipboardManager = mockk(relaxed = true) + val urlCopiedMessage = context.getString(R.string.url_copied) + every { any().getSystemService() } returns clipboardManager + every { ClipData.newPlainText(any(), any()) } returns mockk(relaxed = true) + + controller.handleCopyUrl(item) + + verifyOrder { + ClipData.newPlainText(item.url, item.url) + snackbarPresenter.present(urlCopiedMessage, any(), any(), any()) + } + } + + @Test + fun `handleBookmarkSharing should navigate to the 'Share' fragment`() { + controller.handleBookmarkSharing(item) + + verify { + navController.navigate( + BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment( + item.url, + item.title + ) + ) + } + } + + @Test + fun `handleOpeningBookmark should open the bookmark a new 'Normal' tab`() { + controller.handleOpeningBookmark(item, BrowsingMode.Normal) + + verifyOrder { + homeActivity.browsingModeManager.mode = BrowsingMode.Normal + homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks) + } + } + + @Test + fun `handleOpeningBookmark should open the bookmark a new 'Private' tab`() { + controller.handleOpeningBookmark(item, BrowsingMode.Private) + + verifyOrder { + homeActivity.browsingModeManager.mode = BrowsingMode.Private + homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks) + } + } + + @Test + fun `handleBookmarkDeletion for an item should properly call a passed in delegate`() { + controller.handleBookmarkDeletion(setOf(item), Event.RemoveBookmark) + + verify { + deleteBookmarkNodes(setOf(item), Event.RemoveBookmark) + } + } + + @Test + fun `handleBookmarkDeletion for multiple bookmarks should properly call a passed in delegate`() { + controller.handleBookmarkDeletion(setOf(item, subfolder), Event.RemoveBookmarks) + + verify { + deleteBookmarkNodes(setOf(item, subfolder), Event.RemoveBookmarks) + } + } + + @Test + fun `handleBookmarkDeletion for a folder should properly call a passed in delegate`() { + controller.handleBookmarkDeletion(setOf(subfolder), Event.RemoveBookmarkFolder) + + verify { + deleteBookmarkNodes(setOf(subfolder), Event.RemoveBookmarkFolder) + } + } + + @Test + fun `handleBackPressed should trigger handleBackPressed in NavController`() { + controller.handleBackPressed() + + verify { + navController.popBackStack() + } + } + + @Test + fun `handleSigningIn should trigger 'PairingSignIn`() { + controller.handleSigningIn() + + verify { + services.launchPairingSignIn(homeActivity, navController) + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt index ca5b3da57..382154ddd 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt @@ -4,16 +4,9 @@ package org.mozilla.fenix.library.bookmarks -import android.content.ClipData -import android.content.ClipboardManager -import android.content.Context -import androidx.core.content.getSystemService -import androidx.navigation.NavController -import androidx.navigation.NavDestination import io.mockk.called import io.mockk.every import io.mockk.mockk -import io.mockk.mockkStatic import io.mockk.spyk import io.mockk.verify import io.mockk.verifyOrder @@ -22,46 +15,25 @@ import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import org.junit.Before import org.junit.Test -import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.FenixApplication -import org.mozilla.fenix.HomeActivity -import org.mozilla.fenix.R -import org.mozilla.fenix.components.FenixSnackbarPresenter -import org.mozilla.fenix.components.Services +import org.mozilla.fenix.BrowsingMode import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController -import org.mozilla.fenix.ext.asActivity -import org.mozilla.fenix.ext.components +@SuppressWarnings("TooManyFunctions", "LargeClass") class BookmarkFragmentInteractorTest { private lateinit var interactor: BookmarkFragmentInteractor - private val context: Context = mockk(relaxed = true) - private val navController: NavController = mockk(relaxed = true) private val bookmarkStore = spyk(BookmarkStore(BookmarkState(null))) private val sharedViewModel: BookmarksSharedViewModel = mockk(relaxed = true) - private val snackbarPresenter: FenixSnackbarPresenter = mockk(relaxed = true) - private val deleteBookmarkNodes: (Set, Event) -> Unit = mockk(relaxed = true) - - private val applicationContext: FenixApplication = mockk(relaxed = true) - private val homeActivity: HomeActivity = mockk(relaxed = true) + private val bookmarkController: DefaultBookmarkController = mockk(relaxed = true) private val metrics: MetricController = mockk(relaxed = true) private val item = BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0, "Mozilla", "http://mozilla.org", null) private val separator = BookmarkNode(BookmarkNodeType.SEPARATOR, "789", "123", 1, null, null, null) private val subfolder = BookmarkNode(BookmarkNodeType.FOLDER, "987", "123", 0, "Subfolder", null, listOf()) - private val childItem = BookmarkNode( - BookmarkNodeType.ITEM, - "987", - "123", - 2, - "Firefox", - "https://www.mozilla.org/en-US/firefox/", - null - ) - private val tree = BookmarkNode( - BookmarkNodeType.FOLDER, "123", null, 0, "Mobile", null, listOf(item, separator, childItem, subfolder) + private val tree: BookmarkNode = BookmarkNode( + BookmarkNodeType.FOLDER, "123", null, 0, "Mobile", null, listOf(item, separator, item, subfolder) ) private val root = BookmarkNode( BookmarkNodeType.FOLDER, BookmarkRoot.Root.id, null, 0, BookmarkRoot.Root.name, null, null @@ -69,31 +41,20 @@ class BookmarkFragmentInteractorTest { @Before fun setup() { - mockkStatic( - "org.mozilla.fenix.ext.ContextKt", - "androidx.core.content.ContextCompat", - "android.content.ClipData" - ) - every { any().asActivity() } returns homeActivity - every { context.applicationContext } returns applicationContext - every { applicationContext.components.analytics.metrics } returns metrics - every { navController.currentDestination } returns NavDestination("").apply { id = R.id.bookmarkFragment } every { bookmarkStore.dispatch(any()) } returns mockk() interactor = BookmarkFragmentInteractor( - context, - navController, - bookmarkStore, - sharedViewModel, - snackbarPresenter, - deleteBookmarkNodes + bookmarkStore = bookmarkStore, + viewModel = sharedViewModel, + bookmarksController = bookmarkController, + metrics = metrics ) } @Test fun `update bookmarks tree`() { - interactor.change(tree) + interactor.onBookmarksChanged(tree) verify { bookmarkStore.dispatch(BookmarkAction.Change(tree)) @@ -104,15 +65,10 @@ class BookmarkFragmentInteractorTest { fun `open a bookmark item`() { interactor.open(item) - val url = item.url!! verifyOrder { - homeActivity.openToBrowserAndLoad( - searchTermOrURL = url, - newTab = true, - from = BrowserDirection.FromBookmarks - ) + bookmarkController.handleBookmarkTapped(item) + metrics.track(Event.OpenedBookmark) } - metrics.track(Event.OpenedBookmark) } @Test(expected = IllegalStateException::class) @@ -125,25 +81,25 @@ class BookmarkFragmentInteractorTest { interactor.open(tree) verify { - navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(tree.guid)) + bookmarkController.handleBookmarkExpand(tree) } } @Test fun `switch between bookmark selection modes`() { - interactor.switchMode(BookmarkState.Mode.Normal) + interactor.onSelectionModeSwitch(BookmarkState.Mode.Normal) verify { - homeActivity.invalidateOptionsMenu() + bookmarkController.handleSelectionModeSwitch() } } @Test fun `press the edit bookmark button`() { - interactor.edit(item) + interactor.onEditPressed(item) verify { - navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(item.guid)) + bookmarkController.handleBookmarkEdit(item) } } @@ -167,7 +123,7 @@ class BookmarkFragmentInteractorTest { @Test fun `deselectAll bookmark items`() { - interactor.deselectAll() + interactor.onAllBookmarksDeselected() verify { bookmarkStore.dispatch(BookmarkAction.DeselectAll) @@ -183,119 +139,97 @@ class BookmarkFragmentInteractorTest { @Test fun `copy a bookmark item`() { - val clipboardManager: ClipboardManager = mockk(relaxed = true) - every { any().getSystemService() } returns clipboardManager - every { ClipData.newPlainText(any(), any()) } returns mockk(relaxed = true) + interactor.onCopyPressed(item) - interactor.copy(item) - - verify { + verifyOrder { + bookmarkController.handleCopyUrl(item) metrics.track(Event.CopyBookmark) } } @Test fun `share a bookmark item`() { - interactor.share(item) + interactor.onSharePressed(item) verifyOrder { - navController.navigate( - BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment( - item.url, - item.title - ) - ) + bookmarkController.handleBookmarkSharing(item) metrics.track(Event.ShareBookmark) } } @Test fun `open a bookmark item in a new tab`() { - interactor.openInNewTab(item) + interactor.onOpenInNormalTab(item) - val url = item.url!! verifyOrder { - homeActivity.openToBrowserAndLoad( - searchTermOrURL = url, - newTab = true, - from = BrowserDirection.FromBookmarks - ) + bookmarkController.handleOpeningBookmark(item, BrowsingMode.Normal) metrics.track(Event.OpenedBookmarkInNewTab) } } @Test fun `open a bookmark item in a private tab`() { - interactor.openInPrivateTab(item) + interactor.onOpenInPrivateTab(item) - val url = item.url!! verifyOrder { - homeActivity.openToBrowserAndLoad( - searchTermOrURL = url, - newTab = true, - from = BrowserDirection.FromBookmarks - ) + bookmarkController.handleOpeningBookmark(item, BrowsingMode.Private) metrics.track(Event.OpenedBookmarkInPrivateTab) } } @Test fun `delete a bookmark item`() { - interactor.delete(setOf(item)) + interactor.onDelete(setOf(item)) verify { - deleteBookmarkNodes(setOf(item), Event.RemoveBookmark) + bookmarkController.handleBookmarkDeletion(setOf(item), Event.RemoveBookmark) } } @Test(expected = IllegalStateException::class) fun `delete a separator`() { - interactor.delete(setOf(item, item.copy(type = BookmarkNodeType.SEPARATOR))) + interactor.onDelete(setOf(item, item.copy(type = BookmarkNodeType.SEPARATOR))) } @Test fun `delete a bookmark folder`() { - interactor.delete(setOf(subfolder)) + interactor.onDelete(setOf(subfolder)) verify { - deleteBookmarkNodes(setOf(subfolder), Event.RemoveBookmarkFolder) + bookmarkController.handleBookmarkDeletion(setOf(subfolder), Event.RemoveBookmarkFolder) } } @Test fun `delete multiple bookmarks`() { - interactor.delete(setOf(item, subfolder)) + interactor.onDelete(setOf(item, subfolder)) verify { - deleteBookmarkNodes(setOf(item, subfolder), Event.RemoveBookmarks) + bookmarkController.handleBookmarkDeletion(setOf(item, subfolder), Event.RemoveBookmarks) } } @Test fun `press the back button`() { - interactor.backPressed() + interactor.onBackPressed() verify { - navController.popBackStack() + bookmarkController.handleBackPressed() } } @Test fun `clicked sign in on bookmarks screen`() { - val services: Services = mockk(relaxed = true) - every { context.components.services } returns services - - interactor.clickedSignIn() + interactor.onSignInPressed() verify { - context.components.services - services.launchPairingSignIn(context, navController) + bookmarkController.handleSigningIn() } } @Test fun `got signed in signal on bookmarks screen`() { - interactor.signedIn() + interactor.onSignedIn() verify { sharedViewModel.signedIn.postValue(true) @@ -304,7 +238,7 @@ class BookmarkFragmentInteractorTest { @Test fun `got signed out signal on bookmarks screen`() { - interactor.signedOut() + interactor.onSignedOut() verify { sharedViewModel.signedIn.postValue(false)