Fix the bookmarked sites counting logic

Author: 0nko

app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt

@@ -464,7 +464,11 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
     }
 
     private fun showBookmarkToast(numBookmarks: Int) {
-        val message = resources.getQuantityString(R.plurals.tabSwitcherBookmarkToast, numBookmarks, numBookmarks)
+        val message = if (numBookmarks == 0) {
+            getString(R.string.tabSwitcherBookmarkToastZero)
+        } else {
+            resources.getQuantityString(R.plurals.tabSwitcherBookmarkToast, numBookmarks, numBookmarks)
+        }
         Toast.makeText(this, message, Toast.LENGTH_SHORT).show()
     }
 

app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt

@@ -44,6 +44,8 @@ import com.duckduckgo.di.scopes.ActivityScope
 import com.duckduckgo.savedsites.api.SavedSitesRepository
 import com.duckduckgo.savedsites.api.models.SavedSite.Bookmark
 import javax.inject.Inject
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.combine
@@ -246,54 +248,48 @@ class TabSwitcherViewModel @Inject constructor(
     }
 
     fun onCloseSelectedTabs() {
-        (selectionViewState.value.mode as? SelectionViewState.Mode.Selection)?.selectedTabs?.size?.let { numTabs ->
-            if (numTabs > 0) {
-                command.value = BookmarkTabs(numTabs)
-            }
-        }
     }
 
     fun onCloseOtherTabs() {
     }
 
     fun onBookmarkTabsConfirmed(numTabs: Int) {
-        val numBookmarkedTabs = when (val mode = selectionViewState.value.mode) {
-            is SelectionViewState.Mode.Selection -> {
-                // bookmark selected tabs (or all tabs if none selected)
-                if (mode.selectedTabs.isNotEmpty()) {
-                    bookmarkTabs(mode.selectedTabs)
-                } else {
-                    bookmarkAllTabs()
+        viewModelScope.launch {
+            val numBookmarkedTabs = when (val mode = selectionViewState.value.mode) {
+                is SelectionViewState.Mode.Selection -> {
+                    // bookmark selected tabs (or all tabs if none selected)
+                    if (mode.selectedTabs.isNotEmpty()) {
+                        bookmarkTabs(mode.selectedTabs)
+                    } else {
+                        bookmarkAllTabs()
+                    }
                 }
-            }
 
-            SelectionViewState.Mode.Normal -> {
-                if (numTabs == 1) {
-                    activeTab.value?.tabId?.let { bookmarkTabs(listOf(it)) } ?: 0
-                } else {
-                    bookmarkAllTabs()
+                SelectionViewState.Mode.Normal -> {
+                    if (numTabs == 1) {
+                        activeTab.value?.tabId?.let { bookmarkTabs(listOf(it)) } ?: 0
+                    } else {
+                        bookmarkAllTabs()
+                    }
                 }
             }
+            command.value = ShowBookmarkToast(numBookmarkedTabs)
         }
-        command.value = ShowBookmarkToast(numBookmarkedTabs)
     }
 
-    private fun bookmarkAllTabs(): Int {
+    private suspend fun bookmarkAllTabs(): Int {
         return tabSwitcherItems.value?.filterIsInstance<TabSwitcherItem.Tab>()?.let { tabIds ->
             bookmarkTabs(tabIds.map { it.id })
         } ?: 0
     }
 
-    private fun bookmarkTabs(tabIds: List<String>): Int {
-        var bookmarkedSites = 0
-        tabIds.forEach {
-            viewModelScope.launch {
-                if (saveSiteBookmark(it) != null) {
-                    bookmarkedSites++
-                }
+    private suspend fun bookmarkTabs(tabIds: List<String>): Int {
+        val results = tabIds.map { tabId ->
+            viewModelScope.async {
+                saveSiteBookmark(tabId)
             }
         }
-        return bookmarkedSites
+        return results.awaitAll().count { it != null }
     }
 
     fun onCloseAllTabsConfirmed() {

app/src/main/res/values/donottranslate.xml

@@ -95,8 +95,8 @@
         <item quantity="one">Bookmark Selected Tab?</item>
         <item quantity="other">Bookmark %1$d Tabs?</item>
     </plurals>
+    <string name="tabSwitcherBookmarkToastZero">No unique bookmark was added.</string>
     <plurals name="tabSwitcherBookmarkToast" tools:ignore="ImpliedQuantity,MissingInstruction">
-        <item quantity="zero">No new bookmark was added.</item>
         <item quantity="one">One new bookmark was added.</item>
         <item quantity="other">%1$d new bookmarks were added.</item>
     </plurals>