For issue mozilla-mobile#9949 - Bookmarks/History deletion inconsiste…

…ncies

 - Added the undo action for deleting individual history items by creating a new field to the history state containing the id's of the history items that are pending for deletion; This field is used inside the update function from the view to show/hide the items.

 - Added a new check inside the "deleteMulti" method from BookmarkFragment that calls the showRemoveFoldersDialog to prevent the user from being able to delete one or more bookmark folders without being asked for confirmation, as in mozilla-mobile#8648.

app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt

@@ -158,6 +158,7 @@ class HistoryTest {
         }.openHistory {
         }.openThreeDotMenu {
         }.clickDelete {
+            verifyDeleteSnackbarText("Deleted")
             verifyEmptyHistoryView()
         }
     }
@@ -174,6 +175,7 @@ class HistoryTest {
             clickDeleteHistoryButton()
             verifyDeleteConfirmationMessage()
             confirmDeleteAllHistory()
+            verifyDeleteSnackbarText("Browsing data deleted")
             verifyEmptyHistoryView()
         }
     }

app/src/androidTest/java/org/mozilla/fenix/ui/robots/HistoryRobot.kt

@@ -83,6 +83,8 @@ class HistoryRobot {
             .click()
     }
 
+    fun verifyDeleteSnackbarText(text: String) = assertSnackBarText(text)
+
     class Transition {
         fun goBack(interact: HistoryRobot.() -> Unit): Transition {
             goBackButton().click()
@@ -152,3 +154,6 @@ private fun assertDeleteConfirmationMessage() =
         .check(matches(isDisplayed()))
 
 private fun assertCopySnackBarText() = snackBarText().check(matches(withText("URL copied")))
+
+private fun assertSnackBarText(text: String) =
+    snackBarText().check(matches(withText(Matchers.containsString(text))))

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

@@ -35,7 +35,7 @@ interface BookmarkController {
     fun handleBookmarkSharing(item: BookmarkNode)
     fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
     fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event)
-    fun handleBookmarkFolderDeletion(node: BookmarkNode)
+    fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>)
     fun handleBackPressed()
 }
 
@@ -45,7 +45,7 @@ class DefaultBookmarkController(
     private val navController: NavController,
     private val showSnackbar: (String) -> Unit,
     private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit,
-    private val deleteBookmarkFolder: (BookmarkNode) -> Unit,
+    private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
     private val invokePendingDeletion: () -> Unit
 ) : BookmarkController {
 
@@ -94,8 +94,8 @@ class DefaultBookmarkController(
         deleteBookmarkNodes(nodes, eventType)
     }
 
-    override fun handleBookmarkFolderDeletion(node: BookmarkNode) {
-        deleteBookmarkFolder(node)
+    override fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>) {
+        deleteBookmarkFolder(nodes)
     }
 
     override fun handleBackPressed() {

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

@@ -274,13 +274,17 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
     }
 
     private fun deleteMulti(selected: Set<BookmarkNode>, eventType: Event = Event.RemoveBookmarks) {
+        selected.forEach { if (it.type == BookmarkNodeType.FOLDER) {
+            showRemoveFolderDialog(selected)
+            return
+        } }
         updatePendingBookmarksToDelete(selected)
 
         pendingBookmarkDeletionJob = getDeleteOperation(eventType)
 
         val message = when (eventType) {
             is Event.RemoveBookmarks -> {
-                getRemoveBookmarksSnackBarMessage(selected)
+                getRemoveBookmarksSnackBarMessage(selected, containsFolders = false)
             }
             is Event.RemoveBookmarkFolder,
             is Event.RemoveBookmark -> {
@@ -301,9 +305,16 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         )
     }
 
-    private fun getRemoveBookmarksSnackBarMessage(selected: Set<BookmarkNode>): String {
+    private fun getRemoveBookmarksSnackBarMessage(
+        selected: Set<BookmarkNode>,
+        containsFolders: Boolean
+    ): String {
         return if (selected.size > 1) {
-            getString(R.string.bookmark_deletion_multiple_snackbar_message_2)
+            return if (containsFolders) {
+                getString(R.string.bookmark_deletion_multiple_snackbar_message_3)
+            } else {
+                getString(R.string.bookmark_deletion_multiple_snackbar_message_2)
+            }
         } else {
             val bookmarkNode = selected.first()
             getString(
@@ -314,29 +325,38 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         }
     }
 
+    private fun getDialogConfirmationMessage(selected: Set<BookmarkNode>): String {
+        return if (selected.size > 1) {
+            getString(R.string.bookmark_delete_multiple_folders_confirmation_dialog, getString(R.string.app_name))
+        } else {
+            getString(R.string.bookmark_delete_folder_confirmation_dialog)
+        }
+    }
+
     override fun onDestroyView() {
         super.onDestroyView()
         _bookmarkInteractor = null
     }
 
-    private fun showRemoveFolderDialog(selected: BookmarkNode) {
+    private fun showRemoveFolderDialog(selected: Set<BookmarkNode>) {
         activity?.let { activity ->
             AlertDialog.Builder(activity).apply {
-                setMessage(R.string.bookmark_delete_folder_confirmation_dialog)
+                val dialogConfirmationMessage = getDialogConfirmationMessage(selected)
+                setMessage(dialogConfirmationMessage)
                 setNegativeButton(R.string.delete_browsing_data_prompt_cancel) { dialog: DialogInterface, _ ->
                     dialog.cancel()
                 }
                 setPositiveButton(R.string.delete_browsing_data_prompt_allow) { dialog: DialogInterface, _ ->
-                    updatePendingBookmarksToDelete(setOf(selected))
+                    updatePendingBookmarksToDelete(selected)
                     pendingBookmarkDeletionJob = getDeleteOperation(Event.RemoveBookmarkFolder)
                     dialog.dismiss()
-                    val message = getDeleteDialogString(selected)
+                    val snackbarMessage = getRemoveBookmarksSnackBarMessage(selected, containsFolders = true)
                     viewLifecycleOwner.lifecycleScope.allowUndo(
                         requireView(),
-                        message,
+                        snackbarMessage,
                         getString(R.string.bookmark_undo_deletion),
                         {
-                            undoPendingDeletion(setOf(selected))
+                            undoPendingDeletion(selected)
                         },
                         operation = getDeleteOperation(Event.RemoveBookmarkFolder)
                     )
@@ -353,14 +373,6 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         bookmarkInteractor.onBookmarksChanged(bookmarkTree)
     }
 
-    private fun getDeleteDialogString(selected: BookmarkNode): String {
-        return getString(
-            R.string.bookmark_deletion_snackbar_message,
-            context?.components?.publicSuffixList?.let { selected.url?.toShortUrl(it) }
-                ?: selected.title
-        )
-    }
-
     private suspend fun undoPendingDeletion(selected: Set<BookmarkNode>) {
         pendingBookmarksToDelete.removeAll(selected)
         pendingBookmarkDeletionJob = null

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

@@ -90,7 +90,7 @@ class BookmarkFragmentInteractor(
             null -> Event.RemoveBookmarks
         }
         if (eventType == Event.RemoveBookmarkFolder) {
-            bookmarksController.handleBookmarkFolderDeletion(nodes.first())
+            bookmarksController.handleBookmarkFolderDeletion(nodes)
         } else {
             bookmarksController.handleBookmarkDeletion(nodes, eventType)
         }

app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt

@@ -33,6 +33,8 @@ class HistoryAdapter(
 
     private var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal
     override val selectedItems get() = mode.selectedItems
+    var pendingDeletionIds = emptySet<Long>()
+    private val itemsWithHeaders: MutableMap<HistoryItemTimeGroup, Int> = mutableMapOf()
 
     override fun getItemViewType(position: Int): Int = HistoryListItemViewHolder.LAYOUT_ID
 
@@ -48,13 +50,33 @@ class HistoryAdapter(
     }
 
     override fun onBindViewHolder(holder: HistoryListItemViewHolder, position: Int) {
-        val previous = if (position == 0) null else getItem(position - 1)
         val current = getItem(position) ?: return
+        val headerForCurrentItem = timeGroupForHistoryItem(current)
+        val isPendingDeletion = pendingDeletionIds.contains(current.visitedAt)
+        var timeGroup: HistoryItemTimeGroup? = null
+
+        // Add or remove the header and position to the map depending on it's deletion status
+        if (itemsWithHeaders.containsKey(headerForCurrentItem)) {
+            if (isPendingDeletion && itemsWithHeaders[headerForCurrentItem] == position) {
+                itemsWithHeaders.remove(headerForCurrentItem)
+            } else if (isPendingDeletion && itemsWithHeaders[headerForCurrentItem] != position) {
+                // do nothing
+            } else {
+                if (position <= itemsWithHeaders[headerForCurrentItem] as Int) {
+                    itemsWithHeaders[headerForCurrentItem] = position
+                    timeGroup = headerForCurrentItem
+                }
+            }
+        } else if (!isPendingDeletion) {
+            itemsWithHeaders[headerForCurrentItem] = position
+            timeGroup = headerForCurrentItem
+        }
+
+        holder.bind(current, timeGroup, position == 0, mode, isPendingDeletion)
+    }
 
-        val previousHeader = previous?.let(::timeGroupForHistoryItem)
-        val currentHeader = timeGroupForHistoryItem(current)
-        val timeGroup = if (currentHeader != previousHeader) currentHeader else null
-        holder.bind(current, timeGroup, position == 0, mode)
+    fun updatePendingDeletionIds(pendingDeletionIds: Set<Long>) {
+        this.pendingDeletionIds = pendingDeletionIds
     }
 
     companion object {