refactor: move deck operations business logic to DeckPickerViewModel

sanjaysargam · mikehardy · commit fa83782d721b · 2026-01-17T17:56:54.000Z
- Move 8 deck-related functions from DeckPicker to DeckPickerViewModel
  - Add reactive flows for UI communication (export, shortcuts, subdeck creation)
  - Update DeckPicker to use ViewModel methods instead of direct collection access
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
@@ -113,6 +113,7 @@ import com.ichi2.anki.deckpicker.DeckPickerViewModel.FlattenedDeckList
 import com.ichi2.anki.deckpicker.DeckPickerViewModel.StartupResponse
 import com.ichi2.anki.deckpicker.EmptyCardsResult
 import com.ichi2.anki.deckpicker.OptionsMenuState
+import com.ichi2.anki.deckpicker.ShortcutData
 import com.ichi2.anki.deckpicker.SyncIconState
 import com.ichi2.anki.dialogs.AsyncDialogFragment
 import com.ichi2.anki.dialogs.BackupPromptDialog
@@ -144,7 +145,6 @@ import com.ichi2.anki.export.ExportDialogFragment
 import com.ichi2.anki.introduction.CollectionPermissionScreenLauncher
 import com.ichi2.anki.introduction.hasCollectionStoragePermissions
 import com.ichi2.anki.libanki.DeckId
-import com.ichi2.anki.libanki.Decks
 import com.ichi2.anki.libanki.exception.ConfirmModSchemaException
 import com.ichi2.anki.libanki.sched.DeckNode
 import com.ichi2.anki.libanki.undoAvailable
@@ -678,6 +678,10 @@ open class DeckPicker :
             startActivity(destination.toIntent(this))
         }
 
+        fun onExportDeck(deckId: DeckId) {
+            ExportDialogFragment.newInstance(deckId).show(supportFragmentManager, "exportOptions")
+        }
+
         fun onPromptUserToUpdateScheduler(op: Unit) {
             SchedulerUpgradeDialog(
                 activity = this,
@@ -776,7 +780,7 @@ open class DeckPicker :
         }
 
         fun onFocusedDeckChanged(deckId: DeckId?) {
-            val position = deckId?.let { findDeckPosition(it) } ?: 0
+            val position = deckId?.let { viewModel.findDeckPosition(it) } ?: 0
             // HACK: a small delay is required before scrolling works
             deckPickerBinding.decks.postDelayed({
                 decksLayoutManager.scrollToPositionWithOffset(position, deckPickerBinding.decks.height / 2)
@@ -828,6 +832,9 @@ open class DeckPicker :
         viewModel.emptyCardsNotification.launchCollectionInLifecycleScope(::onCardsEmptied)
         viewModel.flowOfDeckCountsChanged.launchCollectionInLifecycleScope(::onDeckCountsChanged)
         viewModel.flowOfDestination.launchCollectionInLifecycleScope(::onDestinationChanged)
+        viewModel.flowOfExportDeck.launchCollectionInLifecycleScope(::onExportDeck)
+        viewModel.flowOfCreateShortcut.launchCollectionInLifecycleScope(::createIcon)
+        viewModel.flowOfDisableShortcuts.launchCollectionInLifecycleScope(::disableDeckAndChildrenShortcuts)
         viewModel.onError.launchCollectionInLifecycleScope(::onError)
         viewModel.flowOfPromptUserToUpdateScheduler.launchCollectionInLifecycleScope(::onPromptUserToUpdateScheduler)
         viewModel.flowOfUndoUpdated.launchCollectionInLifecycleScope(::onUndoUpdated)
@@ -879,7 +886,7 @@ open class DeckPicker :
                  * if fixed or given free hand to delete the shortcut with the help of API update this method and use the new one
                  */
                 // TODO: it feels buggy that this is not called on all deck deletion paths
-                disableDeckAndChildrenShortcuts(deckId)
+                viewModel.disableDeckAndChildrenShortcuts(deckId)
                 dismissAllDialogFragments()
                 deleteDeck(deckId)
             }
@@ -894,7 +901,7 @@ open class DeckPicker :
             }
             DeckPickerContextMenuOption.CREATE_SHORTCUT -> {
                 Timber.i("ContextMenu: Create icon for a deck")
-                createIcon(this, deckId)
+                viewModel.createIcon(deckId)
             }
             DeckPickerContextMenuOption.RENAME_DECK -> {
                 Timber.i("ContextMenu: Rename deck selected")
@@ -903,7 +910,7 @@ open class DeckPicker :
             }
             DeckPickerContextMenuOption.EXPORT_DECK -> {
                 Timber.i("ContextMenu: Export deck selected")
-                exportDeck(deckId)
+                viewModel.exportDeck(deckId)
             }
             DeckPickerContextMenuOption.UNBURY -> {
                 Timber.i("ContextMenu: Unbury deck selected")
@@ -2155,40 +2162,23 @@ open class DeckPicker :
         // Also forget the last deck used by the Browser
         CardBrowser.clearLastDeckId()
         viewModel.focusedDeck = did
-        val deck = getNodeByDid(did)
-        if (deck.hasCardsReadyToStudy()) {
+
+        // TODO: Reuse dueTree from ViewModel instead of recalculating for better performance.
+        val deck = withCol { sched.deckDueTree().find(did) }
+        if (deck?.hasCardsReadyToStudy() == true) {
             openReviewerOrStudyOptions(selectionType)
             return
         }
 
-        if (!deck.filtered && isDeckAndSubdeckEmpty(did)) {
+        val isEmpty = withCol { decks.cardCount(did, includeSubdecks = true) == 0 }
+        if (!deck?.filtered!! && isEmpty) {
             showEmptyDeckSnackbar()
             updateUi()
         } else {
             onDeckCompleted()
         }
     }
 
-    /**
-     * Return the position of the deck in the deck list. If the deck is a child of a collapsed deck
-     * (i.e., not visible in the deck list), then the position of the parent deck is returned instead.
-     *
-     * An invalid deck ID will return position 0.
-     */
-    private fun findDeckPosition(did: DeckId): Int {
-        deckListAdapter.currentList.forEachIndexed { index, treeNode ->
-            if (treeNode.did == did) {
-                return index
-            }
-        }
-
-        // If the deck is not in our list, we search again using the immediate parent
-        // If the deck is not found, return 0
-        val collapsedDeck = dueTree?.find(did) ?: return 0
-        val parent = collapsedDeck.parent?.get() ?: return 0
-        return findDeckPosition(parent.did)
-    }
-
     /**
      * @see DeckPickerViewModel.updateDeckList
      */
@@ -2199,28 +2189,16 @@ open class DeckPicker :
         }
     }
 
-    /**
-     * Get the [DeckNode] identified by [did] from [DeckAdapter].
-     */
-    private fun DeckPicker.getNodeByDid(did: DeckId): DeckNode = deckListAdapter.currentList[findDeckPosition(did)].deckNode
-
-    fun exportDeck(did: DeckId) {
-        ExportDialogFragment.newInstance(did).show(supportFragmentManager, "exportOptions")
-    }
-
-    private fun createIcon(
-        context: Context,
-        did: DeckId,
-    ) {
+    private fun createIcon(shortcutData: ShortcutData) {
         // This code should not be reachable with lower versions
         val shortcut =
             ShortcutInfoCompat
-                .Builder(this, did.toString())
+                .Builder(this, shortcutData.deckId.toString())
                 .setIntent(
-                    intentToReviewDeckFromShortcuts(context, did),
-                ).setIcon(IconCompat.createWithResource(context, R.mipmap.ic_launcher))
-                .setShortLabel(Decks.basename(getColUnsafe.decks.name(did)))
-                .setLongLabel(getColUnsafe.decks.name(did))
+                    intentToReviewDeckFromShortcuts(this, shortcutData.deckId),
+                ).setIcon(IconCompat.createWithResource(this, R.mipmap.ic_launcher))
+                .setShortLabel(shortcutData.shortLabel)
+                .setLongLabel(shortcutData.longLabel)
                 .build()
         try {
             val success = ShortcutManagerCompat.requestPinShortcut(this, shortcut, null)
@@ -2240,23 +2218,25 @@ open class DeckPicker :
 
     /** Disables the shortcut of the deck and the children belonging to it.*/
     @NeedsTest("ensure collapsed decks are also deleted")
-    private fun disableDeckAndChildrenShortcuts(did: DeckId) {
-        val deckTreeDids = dueTree?.find(did)?.map { it.did.toString() } ?: listOf()
+    private fun disableDeckAndChildrenShortcuts(deckTreeDids: List<String>) {
         val errorMessage: CharSequence = getString(R.string.deck_shortcut_doesnt_exist)
         ShortcutUtils.disableShortcuts(this, deckTreeDids, errorMessage)
     }
 
     fun renameDeckDialog(did: DeckId) {
-        val currentName = getColUnsafe.decks.name(did)
-        val createDeckDialog = CreateDeckDialog(this@DeckPicker, R.string.rename_deck, CreateDeckDialog.DeckDialogType.RENAME_DECK, null)
-        createDeckDialog.deckName = currentName
-        createDeckDialog.onNewDeckCreated = {
-            dismissAllDialogFragments()
-            deckListAdapter.notifyDataSetChanged()
-            updateDeckList()
-            tryShowStudyOptionsPanel()
+        launchCatchingTask {
+            val currentName = withCol { decks.name(did) }
+            val createDeckDialog =
+                CreateDeckDialog(this@DeckPicker, R.string.rename_deck, CreateDeckDialog.DeckDialogType.RENAME_DECK, null)
+            createDeckDialog.deckName = currentName
+            createDeckDialog.onNewDeckCreated = {
+                dismissAllDialogFragments()
+                deckListAdapter.notifyDataSetChanged()
+                updateDeckList()
+                tryShowStudyOptionsPanel()
+            }
+            createDeckDialog.showDialog()
         }
-        createDeckDialog.showDialog()
     }
 
     /**
@@ -2288,11 +2268,7 @@ open class DeckPicker :
     fun rebuildFiltered(did: DeckId) {
         launchCatchingTask {
             withProgress(resources.getString(R.string.rebuild_filtered_deck)) {
-                withCol {
-                    Timber.d("rebuildFiltered: doInBackground - RebuildCram")
-                    decks.select(did)
-                    sched.rebuildFilteredDeck(decks.selected())
-                }
+                viewModel.rebuildFilteredDeck(did).join()
             }
             updateDeckList()
             tryShowStudyOptionsPanel()
@@ -2461,18 +2437,6 @@ open class DeckPicker :
         }
     }
 
-    /**
-     * Returns if the deck and its subdecks are all empty.
-     *
-     * @param did The id of a deck with no pending cards to review
-     */
-    private suspend fun isDeckAndSubdeckEmpty(did: DeckId): Boolean {
-        val node = getNodeByDid(did)
-        return withCol {
-            node.all { decks.isEmpty(it.did) }
-        }
-    }
-
     override fun getApkgFileImportResultLauncher(): ActivityResultLauncher<Intent> = apkgFileImportResultLauncher
 
     override fun getCsvFileImportResultLauncher(): ActivityResultLauncher<Intent> = csvImportResultLauncher
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt
@@ -36,12 +36,14 @@ import com.ichi2.anki.InitialActivity
 import com.ichi2.anki.OnErrorListener
 import com.ichi2.anki.PermissionSet
 import com.ichi2.anki.browser.BrowserDestination
+import com.ichi2.anki.common.annotations.NeedsTest
 import com.ichi2.anki.configureRenderingMode
 import com.ichi2.anki.launchCatchingIO
 import com.ichi2.anki.libanki.CardId
 import com.ichi2.anki.libanki.Consts
 import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID
 import com.ichi2.anki.libanki.DeckId
+import com.ichi2.anki.libanki.Decks
 import com.ichi2.anki.libanki.sched.DeckNode
 import com.ichi2.anki.libanki.undoAvailable
 import com.ichi2.anki.libanki.undoLabel
@@ -124,7 +126,7 @@ class DeckPickerViewModel :
                 data = tree.filterAndFlattenDisplay(filter, currentDeckId),
                 hasSubDecks = tree.children.any { it.children.any() },
             )
-        }
+        }.stateIn(viewModelScope, SharingStarted.Eagerly, initialValue = FlattenedDeckList.empty)
 
     /**
      * @see deleteDeck
@@ -134,6 +136,9 @@ class DeckPickerViewModel :
     val emptyCardsNotification = MutableSharedFlow<EmptyCardsResult>(extraBufferCapacity = 1)
     val flowOfDestination = MutableSharedFlow<Destination>(extraBufferCapacity = 1)
     override val onError = MutableSharedFlow<String>(extraBufferCapacity = 1)
+    val flowOfExportDeck = MutableSharedFlow<DeckId>()
+    val flowOfCreateShortcut = MutableSharedFlow<ShortcutData>()
+    val flowOfDisableShortcuts = MutableSharedFlow<List<String>>()
 
     /**
      * A notification that the study counts have changed
@@ -257,6 +262,20 @@ class DeckPickerViewModel :
             flowOfDeckCountsChanged.emit(Unit)
         }
 
+    /**
+     * Rebuilds a filtered deck with its current filter settings
+     */
+    @CheckResult
+    fun rebuildFilteredDeck(deckId: DeckId): Job =
+        viewModelScope.launch {
+            Timber.i("rebuilding filtered deck %s", deckId)
+            withCol {
+                decks.select(deckId)
+                sched.rebuildFilteredDeck(decks.selected())
+            }
+            flowOfDeckCountsChanged.emit(Unit)
+        }
+
     fun browseCards(deckId: DeckId) =
         launchCatchingIO {
             withCol { decks.select(deckId) }
@@ -391,6 +410,63 @@ class DeckPickerViewModel :
             flowOfRefreshDeckList.emit(Unit)
         }
 
+    /**
+     * Requests export for the specified deck
+     */
+    fun exportDeck(deckId: DeckId) =
+        launchCatchingIO {
+            flowOfExportDeck.emit(deckId)
+        }
+
+    /**
+     * Find the position of a deck in the flattened deck list.
+     * If the deck is a child of a collapsed deck, returns the position of the parent deck.
+     * Returns 0 if the deck is not found.
+     */
+    fun findDeckPosition(deckId: DeckId): Int {
+        val currentDeckList = flowOfDeckList.value.data
+        currentDeckList.forEachIndexed { index, treeNode ->
+            if (treeNode.did == deckId) {
+                return index
+            }
+        }
+
+        // If the deck is not in our list, search using the immediate parent
+        val collapsedDeck = dueTree?.find(deckId) ?: return 0
+        val parent = collapsedDeck.parent?.get() ?: return 0
+        return findDeckPosition(parent.did)
+    }
+
+    /**
+     * Prepares data for creating a deck shortcut
+     */
+    fun createIcon(deckId: DeckId) =
+        launchCatchingIO {
+            val (shortLabel, longLabel) =
+                withCol {
+                    val fullName = decks.name(deckId)
+                    Pair(
+                        Decks.basename(fullName),
+                        fullName,
+                    )
+                }
+            flowOfCreateShortcut.emit(
+                ShortcutData(
+                    deckId = deckId,
+                    shortLabel = shortLabel,
+                    longLabel = longLabel,
+                ),
+            )
+        }
+
+    /** Disables the shortcut of the deck and the children belonging to it.*/
+    @NeedsTest("ensure collapsed decks are also deleted")
+    fun disableDeckAndChildrenShortcuts(deckId: DeckId) =
+        launchCatchingIO {
+            val deckTreeDids = dueTree?.find(deckId)?.map { it.did.toString() } ?: emptyList()
+            flowOfDisableShortcuts.emit(deckTreeDids)
+        }
+
     sealed class StartupResponse {
         data class RequestPermissions(
             val requiredPermissions: PermissionSet,
@@ -566,6 +642,17 @@ data class EmptyCardsResult(
 
 fun DeckNode.onlyHasDefaultDeck() = children.singleOrNull()?.did == DEFAULT_DECK_ID
 
+/**
+ * Data for creating a deck shortcut
+ * @param shortLabel the basename of the deck (e.g., "Verbs" for "Language::English::Verbs")
+ * @param longLabel the full deck name (e.g., "Language::English::Verbs")
+ */
+data class ShortcutData(
+    val deckId: DeckId,
+    val shortLabel: String,
+    val longLabel: String,
+)
+
 enum class SyncIconState {
     Normal,
     PendingChanges,
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt
@@ -469,6 +469,8 @@ class DeckPickerTest : RobolectricTest() {
         deckPicker {
             val didA = addDeck("Deck 1")
             supportFragmentManager.selectContextMenuOption(DeckPickerContextMenuOption.CREATE_SHORTCUT, didA)
+            // Wait for the shortcut creation to complete
+            ShadowLooper.runUiThreadTasksIncludingDelayedTasks()
             assertEquals(
                 "Deck 1",
                 ShortcutManagerCompat.getShortcuts(this, ShortcutManagerCompat.FLAG_MATCH_PINNED).first().shortLabel,
diff --git a/libanki/src/test/java/com/ichi2/anki/libanki/PythonTypesTest.kt b/libanki/src/test/java/com/ichi2/anki/libanki/PythonTypesTest.kt
