commit a768b8ec470f48159b3b84d287fc82bbb6ae5c03 parent 537c897dbf5950bca1f11ef3ad02bd5027148ced Author: fmasalha <fmasalha@mozilla.com> Date: Mon, 6 Oct 2025 19:23:24 +0000 Bug 1980839 - Added a edit flag for bookmarks to only update lastSavedFolderCache on edits r=android-reviewers,matt-tighe,boek Differential Revision: https://phabricator.services.mozilla.com/D265501 Diffstat:
5 files changed, 62 insertions(+), 2 deletions(-)
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 @@ -246,7 +246,9 @@ internal class BookmarksMiddleware( if (result.isFailure) { reportResultGlobally(BookmarksGlobalResultReport.EditBookmarkFailed) } else { - lastSavedFolderCache.setGuid(it.parentGuid) + if (preReductionState.bookmarksEditBookmarkState.edited) { + lastSavedFolderCache.setGuid(it.parentGuid) + } } } context.store.tryDispatchLoadFor(preReductionState.currentFolder.guid) 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 @@ -128,6 +128,7 @@ private fun BookmarksState.handleEditBookmarkAction(action: EditBookmarkAction): bookmarksEditBookmarkState = bookmarksEditBookmarkState?.let { it.copy( bookmark = it.bookmark.copy(title = action.title), + edited = true, ) }, ) @@ -136,6 +137,7 @@ private fun BookmarksState.handleEditBookmarkAction(action: EditBookmarkAction): bookmarksEditBookmarkState = bookmarksEditBookmarkState?.let { it.copy( bookmark = it.bookmark.copy(url = action.url), + edited = true, ) }, ) @@ -266,7 +268,7 @@ private fun BookmarksState.updateSelectedFolder(folder: SelectFolderItem): Bookm ) } else { alwaysTryUpdate.copy( - bookmarksEditBookmarkState = bookmarksEditBookmarkState.copy(folder = folder.folder), + bookmarksEditBookmarkState = bookmarksEditBookmarkState.copy(folder = folder.folder, edited = true), ) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksState.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksState.kt @@ -212,6 +212,7 @@ internal fun BookmarksSnackbarState.addGuidsToDelete(guids: List<String>) = when internal data class BookmarksEditBookmarkState( val bookmark: BookmarkItem.Bookmark, val folder: BookmarkItem.Folder, + val edited: Boolean = false, ) internal data class BookmarksAddFolderState( 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 @@ -36,6 +36,7 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.anyList +import org.mockito.ArgumentMatchers.anyString import org.mockito.Mockito.never import org.mockito.Mockito.times import org.mockito.Mockito.verify @@ -1696,6 +1697,26 @@ class BookmarksMiddlewareTest { } @Test + fun `GIVEN editing a bookmark WHEN edit fails THAN last saved location does not change`() = runTestOnMain { + val tree = generateBookmarkTree() + `when`(bookmarksStorage.countBookmarksInTrees(listOf(BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id))).thenReturn(0u) + `when`(bookmarksStorage.getTree(BookmarkRoot.Mobile.id)).thenReturn(Result.success(tree)) + val bookmark = tree.children?.first { it.type == BookmarkNodeType.ITEM }!! + val bookmarkItem = BookmarkItem.Bookmark(title = bookmark.title!!, guid = bookmark.guid, url = bookmark.url!!, previewImageUrl = bookmark.url!!, position = bookmark.position) + `when`(bookmarksStorage.updateNode(eq(bookmark.guid), any())).thenReturn(Result.failure(IllegalStateException())) + `when`(lastSavedFolderCache.getGuid()).thenReturn(bookmark.parentGuid) + + val middleware = buildMiddleware() + val store = middleware.makeStore() + + store.dispatch(BookmarksListMenuAction.Bookmark.EditClicked(bookmarkItem)) + store.dispatch(EditBookmarkAction.TitleChanged("")) + store.dispatch(BackClicked) + + verify(lastSavedFolderCache, never()).setGuid(anyString()) + } + + @Test fun `GIVEN editing a bookmark WHEN edit fails THEN result is reported`() = runTestOnMain { val tree = generateBookmarkTree() `when`(bookmarksStorage.countBookmarksInTrees(listOf(BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id))).thenReturn(0u) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/bookmarks/BookmarksReducerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/bookmarks/BookmarksReducerTest.kt @@ -583,6 +583,7 @@ class BookmarksReducerTest { val expected = state.copy( bookmarksEditBookmarkState = state.bookmarksEditBookmarkState?.copy( folder = BookmarkItem.Folder("Nested 0", "guid0", null), + edited = true, ), bookmarksSelectFolderState = state.bookmarksSelectFolderState?.copy( outerSelectionGuid = "guid0", @@ -593,6 +594,39 @@ class BookmarksReducerTest { } @Test + fun `GIVEN a bookmark WHEN a edit is made THEN the edited state is persisted`() { + val bookmarkItem = generateBookmark() + val folderItem = SelectFolderItem(0, BookmarkItem.Folder("Bookmarks", "guid0", null)) + var state = BookmarksState.default.copy( + bookmarkItems = listOf(bookmarkItem), + bookmarksSelectFolderState = BookmarksSelectFolderState( + outerSelectionGuid = "guid0", + folders = listOf( + folderItem, + SelectFolderItem(0, BookmarkItem.Folder("Nested 0", "guid0", null)), + ), + ), + ) + + state = bookmarksReducer(state = state, action = EditBookmarkClicked(bookmarkItem)) + state = bookmarksReducer(state = state, action = EditBookmarkAction.URLChanged("1234")) + + assertEquals(true, state.bookmarksEditBookmarkState?.edited) + + state = bookmarksReducer(state = state, action = EditBookmarkClicked(bookmarkItem)) + assertEquals(false, state.bookmarksEditBookmarkState?.edited) + + state = bookmarksReducer(state = state, action = EditBookmarkAction.TitleChanged("1234")) + assertEquals(true, state.bookmarksEditBookmarkState?.edited) + + state = bookmarksReducer(state = state, action = EditBookmarkClicked(bookmarkItem)) + assertEquals(false, state.bookmarksEditBookmarkState?.edited) + + state = bookmarksReducer(state = state, action = SelectFolderAction.ItemClicked(folderItem)) + assertEquals(true, state.bookmarksEditBookmarkState?.edited) + } + + @Test fun `GIVEN we are on the select folder screen with a selection guid and multiselect state WHEN a folder is clicked THEN update the selection`() { val state = BookmarksState.default.copy( bookmarksMultiselectMoveState = MultiselectMoveState(