Skip to content

Commit e02a3c8

Browse files
segunfamisaMatthewTighe
authored andcommitted
Bug 2003434 - Fallback to mobile bookmarks root if last saved folder does not exist r=matt-tighe
Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=2003434 Try: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=174618 Pull request: #38
1 parent dd0aab5 commit e02a3c8

2 files changed

Lines changed: 49 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
@@ -216,9 +216,16 @@ class MenuDialogMiddleware(
216216
val selectedTab = browserMenuState.selectedTab
217217
val url = selectedTab.getUrl() ?: return@launch
218218

219-
val parentGuid = lastSavedFolderCache.getGuid() ?: BookmarkRoot.Mobile.id
219+
// get the last saved folder id
220+
val targetParentFolderId = lastSavedFolderCache.getGuid() ?: BookmarkRoot.Mobile.id
220221

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

223230
val guidToEdit = addBookmarkUseCase(
224231
url = url,

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

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import androidx.appcompat.app.AlertDialog
1010
import com.google.android.material.dialog.MaterialAlertDialogBuilder
1111
import kotlinx.coroutines.runBlocking
1212
import kotlinx.coroutines.test.StandardTestDispatcher
13+
import kotlinx.coroutines.test.advanceUntilIdle
1314
import kotlinx.coroutines.test.runTest
1415
import mozilla.appservices.places.BookmarkRoot
1516
import mozilla.components.browser.state.state.ReaderState
@@ -271,7 +272,11 @@ class MenuDialogMiddlewareTest {
271272
}
272273

273274
@Test
274-
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) {
275+
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) {
276+
// given that the last saved folder actually exists
277+
val lastSavedFolderId = bookmarksStorage.addFolder(BookmarkRoot.Mobile.id, "last-folder")
278+
.getOrThrow()
279+
`when`(lastSavedFolderCache.getGuid()).thenReturn(lastSavedFolderId)
275280
val url = "https://www.mozilla.org"
276281
val title = "Mozilla"
277282
var dismissWasCalled = false
@@ -293,19 +298,50 @@ class MenuDialogMiddlewareTest {
293298
)
294299
testScheduler.advanceUntilIdle()
295300

296-
`when`(lastSavedFolderCache.getGuid()).thenReturn("cached-value")
297-
298301
store.dispatch(MenuAction.AddBookmark)
299302
testScheduler.advanceUntilIdle()
300303

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

303306
captureMiddleware.assertLastAction(BookmarkAction.BookmarkAdded::class) { action: BookmarkAction.BookmarkAdded ->
304307
assertNotNull(action.guidToEdit)
305308
}
306309
assertTrue(dismissWasCalled)
307310
}
308311

312+
@Test
313+
fun `GIVEN last save folder cache has a value that is no longer available THEN a new bookmark is added to the mobile root`() =
314+
runTest(testDispatcher) {
315+
val url = "https://www.mozilla.org"
316+
val title = "Mozilla"
317+
318+
val browserMenuState = BrowserMenuState(
319+
selectedTab = createTab(
320+
url = url,
321+
title = title,
322+
),
323+
)
324+
val captureMiddleware = CaptureActionsMiddleware<AppState, AppAction>()
325+
val appStore = AppStore(middlewares = listOf(captureMiddleware))
326+
val store = createStore(
327+
appStore = appStore,
328+
menuState = MenuState(
329+
browserMenuState = browserMenuState,
330+
),
331+
onDismiss = { },
332+
)
333+
testScheduler.advanceUntilIdle()
334+
335+
`when`(lastSavedFolderCache.getGuid()).thenReturn("cached-value")
336+
337+
store.dispatch(MenuAction.AddBookmark)
338+
339+
testScheduler.advanceUntilIdle()
340+
341+
// we fallback to the mobile root
342+
verify(addBookmarkUseCase).invoke(url = url, title = title, parentGuid = BookmarkRoot.Mobile.id)
343+
}
344+
309345
@Test
310346
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) {
311347
val url = "https://www.mozilla.org"

0 commit comments

Comments
 (0)