Skip to content

Commit b2e1a43

Browse files
committed
Bug 2003434 - Fallback to mobile bookmarks root if last saved folder does not exist
1 parent 9dadfbd commit b2e1a43

2 files changed

Lines changed: 45 additions & 6 deletions

File tree

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,16 @@ class MenuDialogMiddleware(
220220
val selectedTab = browserMenuState.selectedTab
221221
val url = selectedTab.getUrl() ?: return@launch
222222

223-
val parentGuid = lastSavedFolderCache.getGuid() ?: BookmarkRoot.Mobile.id
223+
// get the last saved folder id
224+
val targetParentFolderId = lastSavedFolderCache.getGuid() ?: BookmarkRoot.Mobile.id
224225

225-
val parentNode = bookmarksStorage.getBookmark(parentGuid).getOrNull()
226+
// get the corresponding bookmark and fallback to mobile root bookmark node
227+
// this is necessary because it's possible that the last saved folder no longer exists (
228+
// e.g. if the folder is removed through sync)
229+
val parentNode = bookmarksStorage.getBookmark(targetParentFolderId).getOrNull()
230+
?: bookmarksStorage.getBookmark(BookmarkRoot.Mobile.id).getOrNull()
231+
232+
val parentGuid = parentNode?.guid ?: BookmarkRoot.Mobile.id
226233

227234
val guidToEdit = addBookmarkUseCase(
228235
url = url,

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

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,12 @@ class MenuDialogMiddlewareTest {
276276
}
277277

278278
@Test
279-
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`() = runTestOnMain {
279+
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`() = runTestOnMain {
280+
// given that the last saved folder actually exists
281+
val lastSavedFolderId = bookmarksStorage.addFolder(BookmarkRoot.Mobile.id, "last-folder")
282+
.getOrThrow()
283+
`when`(lastSavedFolderCache.getGuid()).thenReturn(lastSavedFolderId)
284+
280285
val url = "https://www.mozilla.org"
281286
val title = "Mozilla"
282287
var dismissWasCalled = false
@@ -297,18 +302,45 @@ class MenuDialogMiddlewareTest {
297302
onDismiss = { dismissWasCalled = true },
298303
)
299304

300-
`when`(lastSavedFolderCache.getGuid()).thenReturn("cached-value")
301-
302305
store.dispatch(MenuAction.AddBookmark)
303306

304-
verify(addBookmarkUseCase).invoke(url = url, title = title, parentGuid = "cached-value")
307+
verify(addBookmarkUseCase).invoke(url = url, title = title, parentGuid = lastSavedFolderId)
305308

306309
captureMiddleware.assertLastAction(BookmarkAction.BookmarkAdded::class) { action: BookmarkAction.BookmarkAdded ->
307310
assertNotNull(action.guidToEdit)
308311
}
309312
assertTrue(dismissWasCalled)
310313
}
311314

315+
@Test
316+
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 {
317+
val url = "https://www.mozilla.org"
318+
val title = "Mozilla"
319+
320+
val browserMenuState = BrowserMenuState(
321+
selectedTab = createTab(
322+
url = url,
323+
title = title,
324+
),
325+
)
326+
val captureMiddleware = CaptureActionsMiddleware<AppState, AppAction>()
327+
val appStore = AppStore(middlewares = listOf(captureMiddleware))
328+
val store = createStore(
329+
appStore = appStore,
330+
menuState = MenuState(
331+
browserMenuState = browserMenuState,
332+
),
333+
onDismiss = { },
334+
)
335+
336+
`when`(lastSavedFolderCache.getGuid()).thenReturn("cached-value")
337+
338+
store.dispatch(MenuAction.AddBookmark)
339+
340+
// we fallback to the mobile root
341+
verify(addBookmarkUseCase).invoke(url = url, title = title, parentGuid = BookmarkRoot.Mobile.id)
342+
}
343+
312344
@Test
313345
fun `GIVEN the last added bookmark does not belong to a folder WHEN bookmark is added THEN bookmark is added to mobile root`() = runTestOnMain {
314346
val url = "https://www.mozilla.org"

0 commit comments

Comments
 (0)