commit 7ab29a4da1daff2217e16b5aa946398abd8e11ca parent 93b226dfcba38e5b1d7319410f25ab4f894f32df Author: Sandor Molnar <smolnar@mozilla.com> Date: Thu, 11 Dec 2025 02:15:42 +0200 Revert "Bug 2002304 - Added safeguards around stale bookmarks multi select move state r=android-reviewers,matt-tighe" for causing fenix debug failures @ BookmarksMiddlewareTest This reverts commit 49bd0e4c2ece121d164a9d53cbf9c8290488ba3e. Diffstat:
6 files changed, 13 insertions(+), 52 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksAction.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksAction.kt @@ -149,5 +149,4 @@ internal sealed class DeletionDialogAction : BookmarksAction { internal sealed class SnackbarAction : BookmarksAction { data object Undo : SnackbarAction() data object Dismissed : SnackbarAction() - data object SelectFolderFailed : SnackbarAction() } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksMiddleware.kt @@ -208,15 +208,11 @@ internal class BookmarksMiddleware( return@also } scope.launch { - val successes = preReductionState.createMovePairs() - ?.mapNotNull { item -> - bookmarksStorage.updateNode(item.first, item.second) - .takeIf { result -> - result.isSuccess - } - } - if (successes.isNullOrEmpty()) { - context.store.dispatch(SnackbarAction.SelectFolderFailed) + preReductionState.createMovePairs()?.forEach { + val result = bookmarksStorage.updateNode(it.first, it.second) + if (result.isFailure) { + reportResultGlobally(BookmarksGlobalResultReport.SelectFolderFailed) + } } context.store.tryDispatchLoadFor(preReductionState.currentFolder.guid) } @@ -381,7 +377,6 @@ internal class BookmarksMiddleware( } is InitEditLoaded, SnackbarAction.Undo, - SnackbarAction.SelectFolderFailed, is OpenTabsConfirmationDialogAction.Present, OpenTabsConfirmationDialogAction.CancelTapped, DeletionDialogAction.CancelTapped, @@ -696,10 +691,7 @@ private suspend fun BookmarksStorage.hasDesktopBookmarks(): Boolean { private fun BookmarksState.createMovePairs() = bookmarksMultiselectMoveState?.let { moveState -> moveState.guidsToMove.map { guid -> - val bookmarkItem = bookmarkItems.firstOrNull { it.guid == guid } - if (bookmarkItem == null) { - return null - } + val bookmarkItem = bookmarkItems.first { it.guid == guid } guid to BookmarkInfo( moveState.destination, // Setting position to 'null' is treated as a 'move to the end' by the storage API. diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksReducer.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksReducer.kt @@ -217,10 +217,6 @@ private fun BookmarksState.handleSnackbarAction(action: SnackbarAction): Bookmar ) } } - - SnackbarAction.SelectFolderFailed -> { - this.copy(bookmarksSnackbarState = BookmarksSnackbarState.SelectFolderFailed) - } } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksScreen.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksScreen.kt @@ -273,9 +273,6 @@ private fun BookmarksList( val (id, titleOrCount) = state.undoSnackbarText() stringResource(id, titleOrCount) } - BookmarksSnackbarState.SelectFolderFailed -> { - stringResource(R.string.bookmark_error_select_folder) - } is BookmarksSnackbarState.BookmarkMoved -> { stringResource( R.string.bookmark_moved_single_item, @@ -285,6 +282,9 @@ private fun BookmarksList( ), ) } + BookmarksSnackbarState.SelectFolderFailed -> { + stringResource(R.string.bookmark_error_select_folder) + } else -> "" } @@ -316,13 +316,13 @@ private fun BookmarksList( onDismissPerformed = { store.dispatch(SnackbarAction.Dismissed) }, ) } - BookmarksSnackbarState.SelectFolderFailed -> scope.launch { + is BookmarksSnackbarState.BookmarkMoved -> scope.launch { snackbarHostState.displaySnackbar( message = snackbarMessage, onDismissPerformed = { store.dispatch(SnackbarAction.Dismissed) }, ) } - is BookmarksSnackbarState.BookmarkMoved -> scope.launch { + BookmarksSnackbarState.SelectFolderFailed -> scope.launch { snackbarHostState.displaySnackbar( message = snackbarMessage, onDismissPerformed = { store.dispatch(SnackbarAction.Dismissed) }, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksTelemetryMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksTelemetryMiddleware.kt @@ -99,9 +99,8 @@ internal class BookmarksTelemetryMiddleware : Middleware<BookmarksState, Bookmar MetricsUtils.recordBookmarkMetrics(MetricsUtils.BookmarkAction.DELETE, source) } } - SnackbarAction.SelectFolderFailed, - SnackbarAction.Undo, - -> Unit + + SnackbarAction.Undo -> Unit } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/bookmarks/BookmarksMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/bookmarks/BookmarksMiddlewareTest.kt @@ -1101,31 +1101,6 @@ class BookmarksMiddlewareTest { } @Test - fun `GIVEN we are in the select folder screen and we are moving multiple bookmarks and bookmark items is empty WHEN the folder is selected and back button is clicked THEN we show a snackbar describing the error`() = runTestOnMain { - val tree = generateBookmarkTree() - val middleware = buildMiddleware() - val store = middleware.makeStore( - initialState = BookmarksState.default.copy( - bookmarkItems = listOf(), - bookmarksSelectFolderState = BookmarksSelectFolderState( - outerSelectionGuid = tree.guid, - innerSelectionGuid = tree.children!!.first { it.type == BookmarkNodeType.FOLDER }.guid, - ), - bookmarksMultiselectMoveState = MultiselectMoveState( - guidsToMove = listOf("item guid 1", "item guid 2"), - destination = "Some other location", - ), - ), - ) - - assertEquals(BookmarksSnackbarState.None, store.state.bookmarksSnackbarState) - - store.dispatch(BackClicked) - - assertEquals(BookmarksSnackbarState.SelectFolderFailed, store.state.bookmarksSnackbarState) - } - - @Test fun `GIVEN a user is on the edit screen with nothing on the backstack WHEN delete is clicked THEN pop the backstack, delete the bookmark and exit bookmarks`() = runTestOnMain { var exited = false exitBookmarks = { exited = true }