[fenix] For https://github.com/mozilla-mobile/fenix/issues/24214 - Move bookmark removal events to BookmarkController

app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt

@@ -29,9 +29,6 @@ sealed class Event {
     object DismissedOnboarding : Event()
     object ClearedPrivateData : Event()
     object AddBookmark : Event()
-    object RemoveBookmark : Event()
-    object RemoveBookmarkFolder : Event()
-    object RemoveBookmarks : Event()
     object CustomTabsClosed : Event()
     object CustomTabsActionTapped : Event()
     object CustomTabsMenuOpened : Event()

app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt

@@ -778,7 +778,6 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.AddonInstalled -> null
         is Event.SearchWidgetInstalled -> null
         is Event.SyncAuthFromSharedReuse, Event.SyncAuthFromSharedCopy -> null
-        else -> null
     }
 
 /**

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt

@@ -21,7 +21,6 @@ import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.bookmarkStorage
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.nav
@@ -44,14 +43,27 @@ interface BookmarkController {
     fun handleCopyUrl(item: BookmarkNode)
     fun handleBookmarkSharing(item: BookmarkNode)
     fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
-    fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event)
+
+    /**
+     * Handle bookmark nodes deletion
+     * @param nodes The set of nodes to be deleted.
+     * @param removeType Type of removal.
+     */
+    fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, removeType: BookmarkRemoveType)
     fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>)
     fun handleRequestSync()
     fun handleBackPressed()
     fun handleSearch()
 }
 
-@Suppress("TooManyFunctions")
+/**
+ * Type of bookmark nodes deleted.
+ */
+enum class BookmarkRemoveType {
+    SINGLE, MULTIPLE, FOLDER
+}
+
+@Suppress("TooManyFunctions", "LongParameterList")
 class DefaultBookmarkController(
     private val activity: HomeActivity,
     private val navController: NavController,
@@ -62,7 +74,7 @@ class DefaultBookmarkController(
     private val tabsUseCases: TabsUseCases?,
     private val loadBookmarkNode: suspend (String) -> BookmarkNode?,
     private val showSnackbar: (String) -> Unit,
-    private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit,
+    private val deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit,
     private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
     private val invokePendingDeletion: () -> Unit,
     private val showTabTray: () -> Unit
@@ -146,8 +158,8 @@ class DefaultBookmarkController(
         showTabTray()
     }
 
-    override fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event) {
-        deleteBookmarkNodes(nodes, eventType)
+    override fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, removeType: BookmarkRemoveType) {
+        deleteBookmarkNodes(nodes, removeType)
     }
 
     override fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>) {

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt

@@ -43,7 +43,6 @@ import org.mozilla.fenix.NavHostActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.FenixSnackbar
 import org.mozilla.fenix.components.StoreProvider
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.databinding.FragmentBookmarkBinding
 import org.mozilla.fenix.ext.bookmarkStorage
 import org.mozilla.fenix.ext.components
@@ -306,7 +305,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         }
     }
 
-    private fun deleteMulti(selected: Set<BookmarkNode>, eventType: Event = Event.RemoveBookmarks) {
+    private fun deleteMulti(selected: Set<BookmarkNode>, eventType: BookmarkRemoveType = BookmarkRemoveType.MULTIPLE) {
         selected.iterator().forEach {
             if (it.type == BookmarkNodeType.FOLDER) {
                 showRemoveFolderDialog(selected)
@@ -318,18 +317,17 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         pendingBookmarkDeletionJob = getDeleteOperation(eventType)
 
         val message = when (eventType) {
-            is Event.RemoveBookmarks -> {
+            BookmarkRemoveType.MULTIPLE -> {
                 getRemoveBookmarksSnackBarMessage(selected, containsFolders = false)
             }
-            is Event.RemoveBookmarkFolder,
-            is Event.RemoveBookmark -> {
+            BookmarkRemoveType.FOLDER,
+            BookmarkRemoveType.SINGLE -> {
                 val bookmarkNode = selected.first()
                 getString(
                     R.string.bookmark_deletion_snackbar_message,
                     bookmarkNode.url?.toShortUrl(requireContext().components.publicSuffixList) ?: bookmarkNode.title
                 )
             }
-            else -> throw IllegalStateException("Illegal event type in onDeleteSome")
         }
 
         viewLifecycleOwner.lifecycleScope.allowUndo(
@@ -386,7 +384,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
                 }
                 setPositiveButton(R.string.delete_browsing_data_prompt_allow) { dialog: DialogInterface, _ ->
                     updatePendingBookmarksToDelete(selected)
-                    pendingBookmarkDeletionJob = getDeleteOperation(Event.RemoveBookmarkFolder)
+                    pendingBookmarkDeletionJob = getDeleteOperation(BookmarkRemoveType.FOLDER)
                     dialog.dismiss()
                     val snackbarMessage = getRemoveBookmarksSnackBarMessage(selected, containsFolders = true)
                     // Use fragment's lifecycle; the view may be gone by the time dialog is interacted with.
@@ -397,7 +395,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
                         {
                             undoPendingDeletion(selected)
                         },
-                        operation = getDeleteOperation(Event.RemoveBookmarkFolder)
+                        operation = getDeleteOperation(BookmarkRemoveType.FOLDER)
                     )
                 }
                 create()
@@ -418,20 +416,17 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         refreshBookmarks()
     }
 
-    private fun getDeleteOperation(event: Event): (suspend () -> Unit) {
+    private fun getDeleteOperation(event: BookmarkRemoveType): (suspend () -> Unit) {
         return {
             deleteSelectedBookmarks(pendingBookmarksToDelete)
             pendingBookmarkDeletionJob = null
             when (event) {
-                is Event.RemoveBookmarkFolder ->
+                BookmarkRemoveType.FOLDER ->
                     BookmarksManagement.folderRemove.record(NoExtras())
-                is Event.RemoveBookmarks ->
+                BookmarkRemoveType.MULTIPLE ->
                     BookmarksManagement.multiRemoved.record(NoExtras())
-                is Event.RemoveBookmark ->
+                BookmarkRemoveType.SINGLE ->
                     BookmarksManagement.removed.record(NoExtras())
-                else -> {
-                    throw IllegalArgumentException("Illegal event type in getDeleteOperation")
-                }
             }
             refreshBookmarks()
         }

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt

@@ -9,7 +9,6 @@ import mozilla.components.concept.storage.BookmarkNodeType
 import mozilla.telemetry.glean.private.NoExtras
 import org.mozilla.fenix.GleanMetrics.BookmarksManagement
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.utils.Do
 
@@ -89,11 +88,11 @@ class BookmarkFragmentInteractor(
         }
         val eventType = when (nodes.singleOrNull()?.type) {
             BookmarkNodeType.ITEM,
-            BookmarkNodeType.SEPARATOR -> Event.RemoveBookmark
-            BookmarkNodeType.FOLDER -> Event.RemoveBookmarkFolder
-            null -> Event.RemoveBookmarks
+            BookmarkNodeType.SEPARATOR -> BookmarkRemoveType.SINGLE
+            BookmarkNodeType.FOLDER -> BookmarkRemoveType.FOLDER
+            null -> BookmarkRemoveType.MULTIPLE
         }
-        if (eventType == Event.RemoveBookmarkFolder) {
+        if (eventType == BookmarkRemoveType.FOLDER) {
             bookmarksController.handleBookmarkFolderDeletion(nodes)
         } else {
             bookmarksController.handleBookmarkDeletion(nodes, eventType)

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt

@@ -38,7 +38,6 @@ import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
 import org.mozilla.fenix.components.Services
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.bookmarkStorage
 import org.mozilla.fenix.ext.components
 
@@ -396,12 +395,12 @@ class BookmarkControllerTest {
     fun `handleBookmarkDeletion for an item should properly call a passed in delegate`() {
         var deleteBookmarkNodesInvoked = false
         createController(
-            deleteBookmarkNodes = { nodes, event ->
+            deleteBookmarkNodes = { nodes, removeEvent ->
                 assertEquals(setOf(item), nodes)
-                assertEquals(Event.RemoveBookmark, event)
+                assertEquals(BookmarkRemoveType.SINGLE, removeEvent)
                 deleteBookmarkNodesInvoked = true
             }
-        ).handleBookmarkDeletion(setOf(item), Event.RemoveBookmark)
+        ).handleBookmarkDeletion(setOf(item), BookmarkRemoveType.SINGLE)
 
         assertTrue(deleteBookmarkNodesInvoked)
     }
@@ -410,12 +409,12 @@ class BookmarkControllerTest {
     fun `handleBookmarkDeletion for multiple bookmarks should properly call a passed in delegate`() {
         var deleteBookmarkNodesInvoked = false
         createController(
-            deleteBookmarkNodes = { nodes, event ->
+            deleteBookmarkNodes = { nodes, removeEvent ->
                 assertEquals(setOf(item, subfolder), nodes)
-                assertEquals(Event.RemoveBookmarks, event)
+                assertEquals(BookmarkRemoveType.MULTIPLE, removeEvent)
                 deleteBookmarkNodesInvoked = true
             }
-        ).handleBookmarkDeletion(setOf(item, subfolder), Event.RemoveBookmarks)
+        ).handleBookmarkDeletion(setOf(item, subfolder), BookmarkRemoveType.MULTIPLE)
 
         assertTrue(deleteBookmarkNodesInvoked)
     }
@@ -481,7 +480,7 @@ class BookmarkControllerTest {
     private fun createController(
         loadBookmarkNode: suspend (String) -> BookmarkNode? = { _ -> null },
         showSnackbar: (String) -> Unit = { _ -> },
-        deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = { _, _ -> },
+        deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit = { _, _ -> },
         deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = { _ -> },
         invokePendingDeletion: () -> Unit = { },
         showTabTray: () -> Unit = { }

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt

@@ -19,7 +19,6 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.mozilla.fenix.GleanMetrics.BookmarksManagement
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 
@@ -174,7 +173,7 @@ class BookmarkFragmentInteractorTest {
         interactor.onDelete(setOf(item))
 
         verify {
-            bookmarkController.handleBookmarkDeletion(setOf(item), Event.RemoveBookmark)
+            bookmarkController.handleBookmarkDeletion(setOf(item), BookmarkRemoveType.SINGLE)
         }
     }
 
@@ -197,7 +196,7 @@ class BookmarkFragmentInteractorTest {
         interactor.onDelete(setOf(item, subfolder))
 
         verify {
-            bookmarkController.handleBookmarkDeletion(setOf(item, subfolder), Event.RemoveBookmarks)
+            bookmarkController.handleBookmarkDeletion(setOf(item, subfolder), BookmarkRemoveType.MULTIPLE)
         }
     }