Bug 2003434 - Fallback to mobile bookmarks root if last saved folder does not exist

Author: segunfamisa

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/menu/middleware/MenuDialogMiddleware.kt

@@ -223,9 +223,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,

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/menu/MenuDialogMiddlewareTest.kt

@@ -282,7 +282,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
@@ -304,19 +308,46 @@ 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)
         }
         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`() = runTestOnMain {
+        val url = "https://www.mozilla.org"
+        val title = "Mozilla"
+
+        val browserMenuState = BrowserMenuState(
+            selectedTab = createTab(
+                url = url,
+                title = title,
+            ),
+        )
+        val captureMiddleware = CaptureActionsMiddleware<AppState, AppAction>()
+        val appStore = AppStore(middlewares = listOf(captureMiddleware))
+        val store = createStore(
+            appStore = appStore,
+            menuState = MenuState(
+                browserMenuState = browserMenuState,
+            ),
+            onDismiss = { },
+        )
+
+        `when`(lastSavedFolderCache.getGuid()).thenReturn("cached-value")
+
+        store.dispatch(MenuAction.AddBookmark)
+
+        // 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"