Bug 1835525 - Bookmark saves to the last select folder in TabsTray

app/src/main/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCase.kt

@@ -29,13 +29,18 @@ class BookmarksUseCase(
          * one with the identical [url] already exists.
          */
         @WorkerThread
-        suspend operator fun invoke(url: String, title: String, position: UInt? = null): Boolean {
+        suspend operator fun invoke(
+            url: String,
+            title: String,
+            position: UInt? = null,
+            parentGuid: String? = null,
+        ): Boolean {
             return try {
                 val canAdd = storage.getBookmarksWithUrl(url).firstOrNull { it.url == url } == null
 
                 if (canAdd) {
                     storage.addItem(
-                        BookmarkRoot.Mobile.id,
+                        parentGuid ?: BookmarkRoot.Mobile.id,
                         url = url,
                         title = title,
                         position = position,

app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt

@@ -42,6 +42,7 @@ import org.mozilla.fenix.components.bookmarks.BookmarksUseCase
 import org.mozilla.fenix.ext.DEFAULT_ACTIVE_DAYS
 import org.mozilla.fenix.ext.potentialInactiveTabs
 import org.mozilla.fenix.home.HomeFragment
+import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
 import org.mozilla.fenix.tabstray.browser.InactiveTabsController
 import org.mozilla.fenix.tabstray.browser.TabsTrayFabController
 import org.mozilla.fenix.tabstray.ext.getTabSessionState
@@ -193,6 +194,7 @@ interface TabsTrayController : SyncedTabsController, InactiveTabsController, Tab
  * @param showBookmarkSnackbar Lambda used to display a snackbar upon saving tabs as bookmarks.
  * @param showCollectionSnackbar Lambda used to display a snackbar upon successfully saving tabs
  * to a collection.
+ * @param bookmarksSharedViewModel [BookmarksSharedViewModel] used to get currently selected bookmark root.
  */
 @Suppress("TooManyFunctions", "LongParameterList")
 class DefaultTabsTrayController(
@@ -219,6 +221,7 @@ class DefaultTabsTrayController(
         tabSize: Int,
         isNewCollection: Boolean,
     ) -> Unit,
+    private val bookmarksSharedViewModel: BookmarksSharedViewModel,
 ) : TabsTrayController {
 
     override fun handleNormalTabsFabClick() {
@@ -405,7 +408,11 @@ class DefaultTabsTrayController(
             // if we leave the fragment, i.e. we still want the bookmarks to be added if the
             // tabs tray closes before the job is done.
             CoroutineScope(ioDispatcher).launch {
-                bookmarksUseCase.addBookmark(tab.content.url, tab.content.title)
+                bookmarksUseCase.addBookmark(
+                    tab.content.url,
+                    tab.content.title,
+                    parentGuid = bookmarksSharedViewModel.selectedFolder?.guid,
+                )
             }
         }
 

app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt

@@ -53,6 +53,7 @@ import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.ext.runIfFragmentIsAttached
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.home.HomeScreenViewModel
+import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
 import org.mozilla.fenix.share.ShareFragment
 import org.mozilla.fenix.tabstray.browser.SelectionBannerBinding
 import org.mozilla.fenix.tabstray.browser.SelectionBannerBinding.VisibilityModifier
@@ -99,6 +100,7 @@ class TabsTrayFragment : AppCompatDialogFragment() {
     private val tabsFeature = ViewBoundFeatureWrapper<TabsFeature>()
     private val tabsTrayInactiveTabsOnboardingBinding = ViewBoundFeatureWrapper<TabsTrayInactiveTabsOnboardingBinding>()
     private val syncedTabsIntegration = ViewBoundFeatureWrapper<SyncedTabsIntegration>()
+    private val bookmarksSharedViewModel: BookmarksSharedViewModel by activityViewModels()
 
     @VisibleForTesting
     @Suppress("VariableNaming")
@@ -188,6 +190,7 @@ class TabsTrayFragment : AppCompatDialogFragment() {
             showCancelledDownloadWarning = ::showCancelledDownloadWarning,
             showCollectionSnackbar = ::showCollectionSnackbar,
             showBookmarkSnackbar = ::showBookmarkSnackbar,
+            bookmarksSharedViewModel = bookmarksSharedViewModel,
         )
 
         tabsTrayInteractor = DefaultTabsTrayInteractor(

app/src/test/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCaseTest.kt

@@ -57,6 +57,23 @@ class BookmarksUseCaseTest {
         coVerify { bookmarksStorage.addItem(BookmarkRoot.Mobile.id, "https://mozilla.org", "Mozilla", null) }
     }
 
+    @Test
+    fun `WHEN adding bookmark THEN new item is stored in folder`() = runTest {
+        val bookmarksStorage = mockk<BookmarksStorage>(relaxed = true)
+        val historyStorage = mockk<HistoryStorage>(relaxed = true)
+        val bookmarkNode = mockk<BookmarkNode>()
+        val useCase = BookmarksUseCase(bookmarksStorage, historyStorage)
+
+        every { bookmarkNode.url }.answers { "https://firefox.com" }
+        coEvery { bookmarksStorage.getBookmarksWithUrl(any()) }.coAnswers { listOf(bookmarkNode) }
+
+        val result = useCase.addBookmark("https://mozilla.org", "Mozilla", parentGuid = "parentGuid")
+
+        assertTrue(result)
+
+        coVerify { bookmarksStorage.addItem("parentGuid", "https://mozilla.org", "Mozilla", null) }
+    }
+
     @Test
     fun `WHEN recently saved bookmarks exist THEN retrieve the list from storage`() = runTest {
         val bookmarksStorage = mockk<BookmarksStorage>(relaxed = true)

app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt

@@ -68,6 +68,7 @@ import org.mozilla.fenix.ext.maxActiveTime
 import org.mozilla.fenix.ext.potentialInactiveTabs
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.home.HomeFragment
+import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
 import org.mozilla.fenix.utils.Settings
 import java.util.concurrent.TimeUnit
 
@@ -103,6 +104,8 @@ class DefaultTabsTrayControllerTest {
     private val bookmarksUseCase: BookmarksUseCase = mockk(relaxed = true)
     private val collectionStorage: TabCollectionStorage = mockk(relaxed = true)
 
+    private val bookmarksSharedViewModel: BookmarksSharedViewModel = mockk(relaxed = true)
+
     private val coroutinesTestRule: MainCoroutineRule = MainCoroutineRule()
     private val testDispatcher = coroutinesTestRule.testDispatcher
 
@@ -1089,7 +1092,7 @@ class DefaultTabsTrayControllerTest {
             },
         ).handleBookmarkSelectedTabsClicked()
 
-        coVerify(exactly = 1) { bookmarksUseCase.addBookmark(any(), any(), any()) }
+        coVerify(exactly = 1) { bookmarksUseCase.addBookmark(any(), any(), any(), any()) }
         assertTrue(showBookmarkSnackbarInvoked)
 
         assertNotNull(TabsTray.bookmarkSelectedTabs.testGetValue())
@@ -1155,6 +1158,7 @@ class DefaultTabsTrayControllerTest {
             showCancelledDownloadWarning = showCancelledDownloadWarning,
             showCollectionSnackbar = showCollectionSnackbar,
             showBookmarkSnackbar = showBookmarkSnackbar,
+            bookmarksSharedViewModel = bookmarksSharedViewModel,
         )
     }
 }