From d03cea5aa5028430a19b078f69839fa5a10d5f31 Mon Sep 17 00:00:00 2001 From: Segun Famisa Date: Wed, 21 Jan 2026 11:27:08 +0100 Subject: [PATCH] Bug 2003434 - Fallback to mobile bookmarks root if last saved folder does not exist --- .../menu/middleware/MenuDialogMiddleware.kt | 11 ++++- .../menu/MenuDialogMiddlewareTest.kt | 44 +++++++++++++++++-- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/menu/middleware/MenuDialogMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/menu/middleware/MenuDialogMiddleware.kt index eb7e1df43f979..af7375dd7e8d1 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/menu/middleware/MenuDialogMiddleware.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/menu/middleware/MenuDialogMiddleware.kt @@ -216,9 +216,16 @@ class MenuDialogMiddleware( val selectedTab = browserMenuState.selectedTab val url = selectedTab.getUrl() ?: return@launch - val parentGuid = lastSavedFolderCache.getGuid() ?: BookmarkRoot.Mobile.id + // get the last saved folder id + val targetParentFolderId = lastSavedFolderCache.getGuid() ?: BookmarkRoot.Mobile.id - val parentNode = bookmarksStorage.getBookmark(parentGuid).getOrNull() + // get the corresponding bookmark and fallback to mobile root bookmark node + // this is necessary because it's possible that the last saved folder no longer exists ( + // e.g. if the folder is removed through sync) + val parentNode = bookmarksStorage.getBookmark(targetParentFolderId).getOrNull() + ?: bookmarksStorage.getBookmark(BookmarkRoot.Mobile.id).getOrNull() + + val parentGuid = parentNode?.guid ?: BookmarkRoot.Mobile.id val guidToEdit = addBookmarkUseCase( url = url, diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/menu/MenuDialogMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/menu/MenuDialogMiddlewareTest.kt index 583f09f3e403c..02e8aa072e09c 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/menu/MenuDialogMiddlewareTest.kt +++ b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/menu/MenuDialogMiddlewareTest.kt @@ -10,6 +10,7 @@ import androidx.appcompat.app.AlertDialog import com.google.android.material.dialog.MaterialAlertDialogBuilder import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.state.state.ReaderState @@ -271,7 +272,11 @@ class MenuDialogMiddlewareTest { } @Test - fun `GIVEN last save folder cache has a value WHEN add bookmark action is dispatched for a selected tab THEN bookmark is added with the caches value as its parent`() = runTest(testDispatcher) { + fun `GIVEN last save folder cache has a value WHEN add bookmark action is dispatched for a selected tab THEN bookmark is added with the cached value as its parent`() = runTest(testDispatcher) { + // given that the last saved folder actually exists + val lastSavedFolderId = bookmarksStorage.addFolder(BookmarkRoot.Mobile.id, "last-folder") + .getOrThrow() + `when`(lastSavedFolderCache.getGuid()).thenReturn(lastSavedFolderId) val url = "https://www.mozilla.org" val title = "Mozilla" var dismissWasCalled = false @@ -293,12 +298,10 @@ class MenuDialogMiddlewareTest { ) testScheduler.advanceUntilIdle() - `when`(lastSavedFolderCache.getGuid()).thenReturn("cached-value") - store.dispatch(MenuAction.AddBookmark) testScheduler.advanceUntilIdle() - verify(addBookmarkUseCase).invoke(url = url, title = title, parentGuid = "cached-value") + verify(addBookmarkUseCase).invoke(url = url, title = title, parentGuid = lastSavedFolderId) captureMiddleware.assertLastAction(BookmarkAction.BookmarkAdded::class) { action: BookmarkAction.BookmarkAdded -> assertNotNull(action.guidToEdit) @@ -306,6 +309,39 @@ class MenuDialogMiddlewareTest { assertTrue(dismissWasCalled) } + @Test + fun `GIVEN last save folder cache has a value that is no longer available THEN a new bookmark is added to the mobile root`() = + runTest(testDispatcher) { + val url = "https://www.mozilla.org" + val title = "Mozilla" + + val browserMenuState = BrowserMenuState( + selectedTab = createTab( + url = url, + title = title, + ), + ) + val captureMiddleware = CaptureActionsMiddleware() + val appStore = AppStore(middlewares = listOf(captureMiddleware)) + val store = createStore( + appStore = appStore, + menuState = MenuState( + browserMenuState = browserMenuState, + ), + onDismiss = { }, + ) + testScheduler.advanceUntilIdle() + + `when`(lastSavedFolderCache.getGuid()).thenReturn("cached-value") + + store.dispatch(MenuAction.AddBookmark) + + testScheduler.advanceUntilIdle() + + // we fallback to the mobile root + verify(addBookmarkUseCase).invoke(url = url, title = title, parentGuid = BookmarkRoot.Mobile.id) + } + @Test fun `GIVEN the last added bookmark does not belong to a folder WHEN bookmark is added THEN bookmark is added to mobile root`() = runTest(testDispatcher) { val url = "https://www.mozilla.org"