Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
segunfamisa wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions ...ix/app/src/main/java/org/mozilla/fenix/components/menu/middleware/MenuDialogMiddleware.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
44 changes: 40 additions & 4 deletions ...oid/fenix/app/src/test/java/org/mozilla/fenix/components/menu/MenuDialogMiddlewareTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -293,19 +298,50 @@ 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`() =
runTest(testDispatcher) {
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 = { },
)
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"
Expand Down