For issue mozilla-mobile#9949 - Inconsistencies between deleting book…

…mark and history items

 - 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

@@ -157,6 +157,7 @@ class HistoryTest {
         }.openHistory {
         }.openThreeDotMenu {
         }.clickDelete {
+            verifyDeleteSnackbarText("Deleted")
             verifyEmptyHistoryView()
         }
     }
@@ -173,6 +174,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

@@ -269,13 +269,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, false)
             }
             is Event.RemoveBookmarkFolder,
             is Event.RemoveBookmark -> {
@@ -296,9 +300,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(
@@ -309,29 +320,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, true)
                     viewLifecycleOwner.lifecycleScope.allowUndo(
                         requireView(),
-                        message,
+                        snackbarMessage,
                         getString(R.string.bookmark_undo_deletion),
                         {
-                            undoPendingDeletion(setOf(selected))
+                            undoPendingDeletion(selected)
                         },
                         operation = getDeleteOperation(Event.RemoveBookmarkFolder)
                     )
@@ -348,14 +368,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

@@ -6,6 +6,7 @@ package org.mozilla.fenix.library.history
 
 import android.content.Context
 import android.text.format.DateUtils
+import android.util.Log
 import android.view.LayoutInflater
 import android.view.ViewGroup
 import androidx.paging.PagedListAdapter
@@ -33,6 +34,9 @@ class HistoryAdapter(
 
     private var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal
     override val selectedItems get() = mode.selectedItems
+    var pendingDeletionIds = emptySet<Int>()
+    private var shouldUpdateVisibility = true
+    val itemsWithHeaders: MutableMap<HistoryItemTimeGroup, Int> = mutableMapOf()
 
     override fun getItemViewType(position: Int): Int = HistoryListItemViewHolder.LAYOUT_ID
 
@@ -48,13 +52,43 @@ class HistoryAdapter(
     }
 
     override fun onBindViewHolder(holder: HistoryListItemViewHolder, position: Int) {
-        val previous = if (position == 0) null else getItem(position - 1)
+        // val previous = if (position == 0) null else getItem(position - 1)
         val current = getItem(position) ?: return
+        val headerForCurrentItem = timeGroupForHistoryItem(current)
+        val isPendingDeletion = pendingDeletionIds.contains(current.id)
+        var timeGroup: HistoryItemTimeGroup? = null
+
+        Log.d("TEST1", "Header MAP IS: $itemsWithHeaders")
+        Log.d("TEST1", "PENDING DELETION ID's IS : $pendingDeletionIds")
+
+        if (itemsWithHeaders.containsKey(headerForCurrentItem)) {
+            if (isPendingDeletion) {
+                itemsWithHeaders.remove(headerForCurrentItem)
+            } else {
+                if (position <= itemsWithHeaders[headerForCurrentItem] as Int) {
+                    itemsWithHeaders[headerForCurrentItem] = position
+                    timeGroup = headerForCurrentItem
+                }
+            }
+        } else if (!pendingDeletionIds.contains(current.id)) {
+            itemsWithHeaders[headerForCurrentItem] = position
+            timeGroup = headerForCurrentItem
+        }
+
+        Log.d(
+            "TEST1",
+            "TIMEGROUP FOR POSITION $position is: $timeGroup, ispendingdeletion = $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, 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<Int>, isFromDelete: Boolean) {
+        this.shouldUpdateVisibility = !isFromDelete
+        this.pendingDeletionIds = pendingDeletionIds
     }
 
     companion object {