refactor(deck-picker): introduce UiEvent

david-allison · david-allison · commit 1ffadf484248 · 2026-04-07T22:26:34.000+01:00
A common pattern in MVI (and some of Compose) is to encapsulate all
one-shot side-effects into a wrapper interface.

This is implemented via a Channel, enforcing correct one-shot and
buffering behavior

This pattern allows:

* Removal of 'StateFlow-related' bugs
* Removal of 'val' flows
* Simple 'Result' classes
* Fewer methods handling 'Unit' in the DeckPicker

Assisted-by: Claude Opus 4.6
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
@@ -114,6 +114,7 @@ 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.deckpicker.UiEvent
 import com.ichi2.anki.dialogs.AsyncDialogFragment
 import com.ichi2.anki.dialogs.BackupPromptDialog
 import com.ichi2.anki.dialogs.CreateDeckDialog
@@ -833,24 +834,28 @@ open class DeckPicker :
                 .show()
         }
 
-        viewModel.deckDeletedNotification.launchCollectionInLifecycleScope(::onDeckDeleted)
-        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)
+        viewModel.uiEvents.launchCollectionInLifecycleScope { event ->
+            when (event) {
+                is UiEvent.DeckDeleted -> onDeckDeleted(event.result)
+                is UiEvent.EmptyCardsDeleted -> onCardsEmptied(event.result)
+                is UiEvent.Navigate -> onDestinationChanged(event.destination)
+                is UiEvent.ShowError -> onError(event.message)
+                is UiEvent.ExportDeck -> onExportDeck(event.deckId)
+                is UiEvent.CreateShortcut -> createIcon(event.data)
+                is UiEvent.DisableShortcuts -> disableDeckAndChildrenShortcuts(event.deckIds)
+                UiEvent.PromptUpdateScheduler -> onPromptUserToUpdateScheduler(Unit)
+                UiEvent.UndoUpdated -> onUndoUpdated(Unit)
+                UiEvent.DecksReloaded -> onDecksReloaded(Unit)
+                UiEvent.DeckCountsChanged -> onDeckCountsChanged(Unit)
+            }
+        }
         viewModel.flowOfStudiedTodayStats.launchCollectionInLifecycleScope(::onStudiedTodayChanged)
         viewModel.flowOfDeckListInInitialState.filterNotNull().launchCollectionInLifecycleScope(::onCollectionStatusChanged)
         viewModel.flowOfCardsDue.launchCollectionInLifecycleScope(::onCardsDueChanged)
         viewModel.flowOfCollectionHasNoCards.launchCollectionInLifecycleScope(::onStudyOptionsVisibilityChanged)
         viewModel.flowOfDeckList.launchCollectionInLifecycleScope(::onDeckListChanged)
         viewModel.flowOfFocusedDeck.launchCollectionInLifecycleScope(::onFocusedDeckChanged)
         viewModel.flowOfResizingDividerVisible.launchCollectionInLifecycleScope(::onResizingDividerVisibilityChanged)
-        viewModel.flowOfDecksReloaded.launchCollectionInLifecycleScope(::onDecksReloaded)
         viewModel.flowOfStartupResponse.filterNotNull().launchCollectionInLifecycleScope(::onStartupResponse)
     }
 
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt
@@ -55,6 +55,7 @@ import com.ichi2.anki.performBackupInBackground
 import com.ichi2.anki.reviewreminders.ScheduleRemindersDestination
 import com.ichi2.anki.settings.Prefs
 import com.ichi2.anki.syncAuth
+import com.ichi2.anki.ui.ChannelUiEventHost
 import com.ichi2.anki.utils.Destination
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
@@ -128,22 +129,22 @@ class DeckPickerViewModel :
         }.stateIn(viewModelScope, SharingStarted.Eagerly, initialValue = FlattenedDeckList.empty)
 
     /**
-     * @see deleteDeck
-     * @see DeckDeletionResult
+     * Single channel of one-shot UI events for [DeckPicker].
+     *
+     * Use this for navigation, snackbars, dialogs, error messages, and other
+     * fire-once events. Do *not* use it for ongoing state — keep that as `StateFlow`.
      */
-    val deckDeletedNotification = MutableSharedFlow<DeckDeletionResult>(extraBufferCapacity = 1)
-    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>>()
+    private val events = ChannelUiEventHost<UiEvent>()
+    val uiEvents = events.uiEvents
 
     /**
-     * A notification that the study counts have changed
+     * Errors emitted via the standard `launchCatchingIO { }` machinery.
+     *
+     * This satisfies [OnErrorListener] (which `launchCatchingIO` depends on) and is
+     * forwarded into [uiEvents] as [UiEvent.ShowError] in [init].
+     * Prefer collecting [uiEvents] in the UI rather than this flow directly.
      */
-    // TODO: most of the recalculation should be moved inside the ViewModel
-    val flowOfDeckCountsChanged = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
+    override val onError = MutableSharedFlow<String>(extraBufferCapacity = 1)
 
     var loadDeckCounts: Job? = null
         private set
@@ -154,10 +155,6 @@ class DeckPickerViewModel :
      */
     private var schedulerUpgradeDialogShownForVersion: Long? = null
 
-    val flowOfPromptUserToUpdateScheduler = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
-
-    val flowOfUndoUpdated = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
-
     val flowOfCollectionHasNoCards = MutableStateFlow(true)
 
     val flowOfDeckListInInitialState =
@@ -182,16 +179,13 @@ class DeckPickerViewModel :
             !(isInInitialState == true || hasNoCards)
         }
 
-    // HACK: dismiss a legacy progress bar
-    // TODO: Replace with better progress handling for first load/corrupt collections
-    // This MutableSharedFlow has replay=1 due to a race condition between its collector being started
-    // and a possible early emission that occurs when the user is on a metered network and a dialog has to show up
-    // to ask the user if they want to trigger a sync. Normally, the spinning progress indicator is
-    // dismissed via an emission to this flow after the sync is completed, but if the metered network
-    // warning dialog appears, we should immediately refresh the UI in case the user decides not to sync.
-    // Otherwise, the progress indicator remains indefinitely. This replay=1 ensures that the collector will
-    // receive the dismissal event even if it starts after the emission.
-    val flowOfDecksReloaded = MutableSharedFlow<Unit>(extraBufferCapacity = 1, replay = 1)
+    init {
+        // Bridge `OnErrorListener.onError` (driven by `launchCatchingIO { }`) into the
+        // unified side-effects channel so the UI only needs one collector.
+        viewModelScope.launch {
+            onError.collect { events.emit(UiEvent.ShowError(it)) }
+        }
+    }
 
     /**
      * Deletes the provided deck, child decks. and all cards inside.
@@ -209,8 +203,10 @@ class DeckPickerViewModel :
             // to match and avoid unnecessary scrolls in `renderPage()`.
             focusedDeck = Consts.DEFAULT_DECK_ID
 
-            deckDeletedNotification.emit(
-                DeckDeletionResult(deckName = deckName, cardsDeleted = changes.count),
+            events.emit(
+                UiEvent.DeckDeleted(
+                    DeckDeletionResult(deckName = deckName, cardsDeleted = changes.count),
+                ),
             )
         }
 
@@ -249,7 +245,7 @@ class DeckPickerViewModel :
             }
         }
         val result = undoableOp { removeCardsAndOrphanedNotes(toDelete) }
-        emptyCardsNotification.emit(EmptyCardsResult(cardsDeleted = result.count))
+        events.emit(UiEvent.EmptyCardsDeleted(EmptyCardsResult(cardsDeleted = result.count)))
     }
 
     // TODO: move withProgress to the ViewModel, so we don't return 'Job'
@@ -258,7 +254,7 @@ class DeckPickerViewModel :
             Timber.i("empty filtered deck %s", deckId)
             withCol { decks.select(deckId) }
             undoableOp { sched.emptyFilteredDeck(decks.selected()) }
-            flowOfDeckCountsChanged.emit(Unit)
+            events.emit(UiEvent.DeckCountsChanged)
         }
 
     /**
@@ -272,13 +268,13 @@ class DeckPickerViewModel :
                 decks.select(deckId)
                 sched.rebuildFilteredDeck(decks.selected())
             }
-            flowOfDeckCountsChanged.emit(Unit)
+            events.emit(UiEvent.DeckCountsChanged)
         }
 
     fun browseCards(deckId: DeckId) =
         launchCatchingIO {
             withCol { decks.select(deckId) }
-            flowOfDestination.emit(BrowserDestination.ToDeck(deckId))
+            events.emit(UiEvent.Navigate(BrowserDestination.ToDeck(deckId)))
         }
 
     fun addNote(
@@ -288,13 +284,13 @@ class DeckPickerViewModel :
         if (deckId != null && setAsCurrent) {
             withCol { decks.select(deckId) }
         }
-        flowOfDestination.emit(NoteEditorLauncher.AddNote(deckId))
+        events.emit(UiEvent.Navigate(NoteEditorLauncher.AddNote(deckId)))
     }
 
     /**
      * Opens the Manage Note Types screen.
      */
-    fun openManageNoteTypes() = launchCatchingIO { flowOfDestination.emit(ManageNoteTypesDestination()) }
+    fun openManageNoteTypes() = launchCatchingIO { events.emit(UiEvent.Navigate(ManageNoteTypesDestination())) }
 
     /**
      * Opens study options for the provided deck
@@ -308,7 +304,7 @@ class DeckPickerViewModel :
     ) = launchCatchingIO {
         // open cram options if filtered deck, otherwise open regular options
         val filtered = isFiltered ?: withCol { decks.isFiltered(deckId) }
-        flowOfDestination.emit(DeckOptionsDestination(deckId = deckId, isFiltered = filtered))
+        events.emit(UiEvent.Navigate(DeckOptionsDestination(deckId = deckId, isFiltered = filtered)))
     }
 
     fun unburyDeck(deckId: DeckId) =
@@ -318,7 +314,7 @@ class DeckPickerViewModel :
 
     fun scheduleReviewReminders(deckId: DeckId) =
         viewModelScope.launch {
-            flowOfDestination.emit(ScheduleRemindersDestination(deckId))
+            events.emit(UiEvent.Navigate(ScheduleRemindersDestination(deckId)))
         }
 
     /**
@@ -371,17 +367,17 @@ class DeckPickerViewModel :
 
                 if (currentSchedulerVersion == 1L && schedulerUpgradeDialogShownForVersion != 1L) {
                     schedulerUpgradeDialogShownForVersion = 1L
-                    flowOfPromptUserToUpdateScheduler.emit(Unit)
+                    events.emit(UiEvent.PromptUpdateScheduler)
                 } else {
                     schedulerUpgradeDialogShownForVersion = currentSchedulerVersion
                 }
 
                 // TODO: This is in the wrong place
                 // current deck may have changed
                 focusedDeck = withCol { decks.current().id }
-                flowOfUndoUpdated.emit(Unit)
+                events.emit(UiEvent.UndoUpdated)
 
-                flowOfDecksReloaded.emit(Unit)
+                events.emit(UiEvent.DecksReloaded)
             }
         this.loadDeckCounts = loadDeckCounts
         return loadDeckCounts
@@ -414,7 +410,7 @@ class DeckPickerViewModel :
      */
     fun exportDeck(deckId: DeckId) =
         launchCatchingIO {
-            flowOfExportDeck.emit(deckId)
+            events.emit(UiEvent.ExportDeck(deckId))
         }
 
     /**
@@ -449,11 +445,13 @@ class DeckPickerViewModel :
                         fullName,
                     )
                 }
-            flowOfCreateShortcut.emit(
-                ShortcutData(
-                    deckId = deckId,
-                    shortLabel = shortLabel,
-                    longLabel = longLabel,
+            events.emit(
+                UiEvent.CreateShortcut(
+                    ShortcutData(
+                        deckId = deckId,
+                        shortLabel = shortLabel,
+                        longLabel = longLabel,
+                    ),
                 ),
             )
         }
@@ -462,7 +460,7 @@ class DeckPickerViewModel :
     fun disableDeckAndChildrenShortcuts(deckId: DeckId) =
         launchCatchingIO {
             val deckTreeDids = dueTree?.find(deckId)?.map { it.did.toString() } ?: emptyList()
-            flowOfDisableShortcuts.emit(deckTreeDids)
+            events.emit(UiEvent.DisableShortcuts(deckTreeDids))
         }
 
     sealed class StartupResponse {
@@ -657,6 +655,49 @@ enum class SyncIconState {
     NotLoggedIn,
 }
 
+/**
+ * One-shot UI events emitted by [DeckPickerViewModel].
+ *
+ * @see com.ichi2.anki.ui.UiEventHost
+ */
+sealed interface UiEvent {
+    data class DeckDeleted(
+        val result: DeckDeletionResult,
+    ) : UiEvent
+
+    data class EmptyCardsDeleted(
+        val result: EmptyCardsResult,
+    ) : UiEvent
+
+    data class Navigate(
+        val destination: Destination,
+    ) : UiEvent
+
+    data class ShowError(
+        val message: String,
+    ) : UiEvent
+
+    data class ExportDeck(
+        val deckId: DeckId,
+    ) : UiEvent
+
+    data class CreateShortcut(
+        val data: ShortcutData,
+    ) : UiEvent
+
+    data class DisableShortcuts(
+        val deckIds: List<String>,
+    ) : UiEvent
+
+    data object PromptUpdateScheduler : UiEvent
+
+    data object UndoUpdated : UiEvent
+
+    data object DecksReloaded : UiEvent
+
+    data object DeckCountsChanged : UiEvent
+}
+
 /** Menu state data for the options menu */
 data class OptionsMenuState(
     val searchIcon: Boolean,
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/ui/UiEventHost.kt b/AnkiDroid/src/main/java/com/ichi2/anki/ui/UiEventHost.kt
@@ -0,0 +1,52 @@
+/*
+ *  Copyright (c) 2026 David Allison <davidallisongithub@gmail.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify it under
+ *  the terms of the GNU General Public License as published by the Free Software
+ *  Foundation; either version 3 of the License, or (at your option) any later
+ *  version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT ANY
+ *  WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ *  PARTICULAR PURPOSE. See the GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along with
+ *  this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package com.ichi2.anki.ui
+
+import kotlinx.coroutines.channels.Channel
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.receiveAsFlow
+
+/**
+ * One-shot UI events from a ViewModel to its UI, using a single hot Flow.
+ *
+ * Each event must be delivered EXACTLY once, buffered while the UI is in the background,
+ * and never replayed on re-subscription.
+ *
+ * This is the wrong choice for ongoing state (use `StateFlow` instead).
+ */
+interface UiEventHost<T : Any> {
+    val uiEvents: Flow<T>
+
+    suspend fun emit(event: T)
+
+    fun tryEmit(event: T): Boolean
+}
+
+/**
+ * Implements [UiEventHost] using a [Channel]
+ *
+ * @see UiEventHost
+ */
+class ChannelUiEventHost<T : Any>(
+    capacity: Int = Channel.BUFFERED,
+) : UiEventHost<T> {
+    private val channel = Channel<T>(capacity)
+    override val uiEvents: Flow<T> = channel.receiveAsFlow()
+
+    override suspend fun emit(event: T) = channel.send(event)
+
+    override fun tryEmit(event: T): Boolean = channel.trySend(event).isSuccess
+}
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt
@@ -22,6 +22,7 @@ import app.cash.turbine.test
 import com.ichi2.anki.common.time.TimeManager
 import com.ichi2.anki.common.utils.annotation.KotlinCleanup
 import com.ichi2.anki.deckpicker.DeckPickerViewModel
+import com.ichi2.anki.deckpicker.UiEvent
 import com.ichi2.anki.dialogs.DatabaseErrorDialog
 import com.ichi2.anki.dialogs.DatabaseErrorDialog.DatabaseErrorDialogType
 import com.ichi2.anki.dialogs.DeckPickerContextMenu
@@ -46,6 +47,7 @@ import com.ichi2.testutils.grantWritePermissions
 import com.ichi2.testutils.revokeWritePermissions
 import com.ichi2.testutils.withDeniedPermissions
 import com.ichi2.testutils.withWritePermissions
+import kotlinx.coroutines.flow.filterIsInstance
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.containsInAnyOrder
 import org.hamcrest.Matchers.containsString
@@ -428,9 +430,9 @@ class DeckPickerTest : RobolectricTest() {
                 deckId: DeckId,
             ): Intent {
                 var result: Destination? = null
-                viewModel.flowOfDestination.test(1.seconds) {
+                viewModel.uiEvents.filterIsInstance<UiEvent.Navigate>().test(1.seconds) {
                     supportFragmentManager.selectContextMenuOption(option, deckId)
-                    result = awaitItem()
+                    result = awaitItem().destination
                 }
                 return result!!.toIntent(this)
             }
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt
