[criticalAY]

Note

Assisted-by: Opus 5.5

Purpose / Description

Deck Picker crashed if the backend failed while deleting a deck, deleting empty cards, rebuilding or emptying a filtered deck, selecting a new deck or refreshing the list. The ViewModel now catches these errors and shows them, as David asked on #21831.

• Depends on feat: Deckpicker progress adoption #21831.

Fixes

• Follow-up to feat: Deckpicker progress adoption #21831

Approach

check commit messages

How Has This Been Tested?

added tests, the error ones crashed before the fix

Learning (optional, can help others)

NA

Checklist

Please, go through these checks before submitting the PR.

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[github-actions]

Snapshot diff report: Previous regressions were resolved by the latest commit.

[criticalAY]

Legit failure, fixed it

[mikehardy]

Assisted-by: cursor-grok-4.6-medium

The stopped-lifecycle and database-corruption tests from the last request are on this tip. ViewModel failures now go through MessageQueue into showException instead of crashing.

Rebased d8d16bd onto main (7e29d24) cleanly and did not push. Local tip is 695da54f. Lint (lintAll, ktlintCheck, lint-rules:test) passed in 55s. Unit (jacocoUnitTestReport) failed once on CardContentProviderStartupTest and a flaky ReviewerNoParamTest; both passed alone, and neither class is in this diff. DeckPickerErrorTest was 9/9. Emulator tests on emulator-5554 (API 36, playRelease) passed. The XML noise was the usual phone-sized failures plus a tablet assumption in DeckPickerImportTest, which this diff does not touch. ReviewRemindersTest was 9/9.

showException is the same dialog path runCatching already used, including corruption recovery. Delete, empty cards, filtered rebuild and empty, select, and deck-list reload catch into a queue that DeckPicker drains after CREATED and withStarted. Rebuild refresh stays on the ViewModel flow. The Activity no longer also calls updateDeckList and tryShowStudyOptionsPanel after join.

some acceptable findings, no action required, just leaving for posterity: Approach is "check commit messages"; launchCatchingIO still uses the string onError SharedFlow.

[david-allison]

@criticalAY Tests are failing

[david-allison]

GPT-6, different recipe, hopefully fewer rounds with more issues in them

diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryHandoffProbeTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryHandoffProbeTest.kt
new file mode 100644
--- /dev/null
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryHandoffProbeTest.kt
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package com.ichi2.anki
+
+import androidx.appcompat.app.AlertDialog
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.LifecycleRegistry
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import anki.backend.backendError
+import anki.collection.OpChangesWithCount
+import com.ichi2.anki.backend.DatabaseCorruption
+import com.ichi2.anki.dialogs.DatabaseErrorDialog
+import kotlinx.coroutines.runBlocking
+import net.ankiweb.rsdroid.Backend
+import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException
+import net.ankiweb.rsdroid.BackendFactory
+import net.ankiweb.rsdroid.exceptions.BackendIoException
+import org.junit.After
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.shadows.ShadowDialog
+import kotlin.test.assertSame
+
+@RunWith(AndroidJUnit4::class)
+class DeckPickerRecoveryHandoffProbeTest : RobolectricTest() {
+    @After
+    fun resetBackendFactory() {
+        BackendFactory.setOverride(null)
+    }
+
+    @Test
+    fun `a queued error rechecks recovery after waiting for STARTED`() {
+        var corrupt = true
+        runBlocking { CollectionManager.discardBackend() }
+        BackendFactory.setOverride {
+            object : Backend() {
+                override fun removeDecks(dids: Iterable<Long>): OpChangesWithCount =
+                    if (corrupt) {
+                        throw BackendDbCorruptException(backendError { message = "database disk image is malformed" })
+                    } else {
+                        throw BackendIoException(backendError { message = "operation failed" })
+                    }
+            }
+        }
+        val target = addDeck("Target")
+        val previouslyDetected = DatabaseCorruption.isDetected
+        DatabaseCorruption.isDetected = false
+        try {
+            withDeckPicker(deckCount = 0) { picker ->
+                advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true }
+                throwOnShowError = false
+                val first = picker.deleteDeck(target)
+                advanceRobolectricLooperUntil {
+                    first.isCompleted && picker.supportFragmentManager.fragments
+                        .filterIsInstance<DatabaseErrorDialog>().any { it.dialog?.isShowing == true }
+                }
+                val firstRecovery = picker.supportFragmentManager.fragments.filterIsInstance<DatabaseErrorDialog>().single()
+                corrupt = false
+                val second = picker.deleteDeck(target)
+                advanceRobolectricLooperUntil { second.isCompleted && picker.viewModel.loadDeckCounts?.isCompleted == true }
+                advanceRobolectricLooper()
+
+                val lifecycle = picker.lifecycle as LifecycleRegistry
+                lifecycle.currentState = Lifecycle.State.CREATED
+                firstRecovery.dismiss()
+                advanceRobolectricLooper()
+
+                // A later async check can open another recovery dialog while the queued
+                // error is suspended waiting for the activity to start again.
+                picker.showDatabaseErrorDialog(DatabaseErrorDialog.DatabaseErrorDialogType.DIALOG_LOAD_FAILED)
+                advanceRobolectricLooper()
+                val secondRecovery = picker.supportFragmentManager.fragments.filterIsInstance<DatabaseErrorDialog>().single()
+                val secondRecoveryDialog = secondRecovery.requireDialog()
+                lifecycle.currentState = Lifecycle.State.RESUMED
+                advanceRobolectricLooper()
+                try {
+                    assertSame(
+                        secondRecoveryDialog,
+                        ShadowDialog.getLatestDialog(),
+                        "The queued generic error must not cover the new recovery dialog after resuming",
+                    )
+                } finally {
+                    (ShadowDialog.getLatestDialog() as? AlertDialog)?.dismiss()
+                    secondRecovery.dismiss()
+                }
+            }
+        } finally {
+            DatabaseCorruption.isDetected = previouslyDetected
+        }
+    }
+}
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerDeletionSafetyProbeTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerDeletionSafetyProbeTest.kt
new file mode 100644
--- /dev/null
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerDeletionSafetyProbeTest.kt
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package com.ichi2.anki
+
+import android.view.KeyEvent
+import android.widget.TextView
+import androidx.appcompat.app.AlertDialog
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import anki.backend.backendError
+import anki.collection.OpChanges
+import anki.decks.DeckTreeNode
+import com.ichi2.anki.dialogs.DeckPickerContextMenu.DeckPickerContextMenuOption
+import com.ichi2.anki.dialogs.DeckPickerContextMenuResult
+import com.ichi2.anki.dialogs.setDeckPickerContextMenuResult
+import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID
+import com.ichi2.anki.reviewreminders.ScheduleRemindersFragment
+import com.ichi2.anki.settings.Prefs
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withContext
+import net.ankiweb.rsdroid.Backend
+import net.ankiweb.rsdroid.BackendFactory
+import net.ankiweb.rsdroid.exceptions.BackendIoException
+import org.junit.After
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.annotation.Config
+import org.robolectric.shadows.ShadowDialog
+import kotlin.test.assertEquals
+import kotlin.test.assertIs
+import kotlin.test.assertNotNull
+import kotlin.test.assertNull
+
+@RunWith(AndroidJUnit4::class)
+@Config(qualifiers = "xlarge")
+class DeckPickerDeletionSafetyProbeTest : RobolectricTest() {
+    private val failure = BackendIoException(backendError { message = "Backend operation failed" })
+
+    @After
+    fun resetBackendFactory() {
+        BackendFactory.setOverride(null)
+    }
+
+    @Test
+    fun `missing study options pane does not authorize deleting the fallback deck`() {
+        withPartlyAppliedDeletion(reminders = true) { picker, _, _ ->
+            assertIs<ScheduleRemindersFragment>(picker.supportFragmentManager.findFragmentById(R.id.studyoptions_fragment))
+            assertNull(picker.fragment)
+
+            pressDelete(picker)
+            advanceRobolectricLooperUntil { picker.viewModel.progressManager.progress.value is com.ichi2.anki.progress.ViewModelProgress.Idle }
+            assertEquals(1, col.decks.cardCount(DEFAULT_DECK_ID, includeSubdecks = true),
+                "Without a visible study pane, Shift+Delete must not delete the fallback deck")
+        }
+    }
+
+    @Test
+    fun `deletion checks the rendered deck rather than a newer model selection`() {
+        withPartlyAppliedDeletion { picker, _, allowReads ->
+            val pane = assertNotNull(picker.fragment)
+            val deckName = pane.requireView().findViewById<TextView>(R.id.studyoptions_deck_name)
+            assertEquals("Deleted", deckName.text.toString())
+            // Complete a recovery refresh before the main thread renders its new StateFlow value.
+            allowReads()
+            runBlocking { withContext(Dispatchers.Default) { pane.viewModel.refreshData() } }
+            assertEquals(DEFAULT_DECK_ID, pane.selectedDeckId)
+            assertEquals("Deleted", deckName.text.toString())
+
+            pressDelete(picker)
+            advanceRobolectricLooperUntil { picker.viewModel.progressManager.progress.value is com.ichi2.anki.progress.ViewModelProgress.Idle }
+            assertEquals(1, col.decks.cardCount(DEFAULT_DECK_ID, includeSubdecks = true),
+                "Shift+Delete must not delete Default while the pane still displays the deleted deck")
+        }
+    }
+
+    private fun pressDelete(picker: DeckPicker) {
+        picker.onKeyUp(
+            KeyEvent.KEYCODE_DEL,
+            KeyEvent(0, 0, KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL, 0, KeyEvent.META_SHIFT_ON),
+        )
+    }
+
+    private fun withPartlyAppliedDeletion(reminders: Boolean = false, block: (DeckPicker, Long, () -> Unit) -> Unit) {
+        var failSelection = false
+        var failReload = false
+        runBlocking { CollectionManager.discardBackend() }
+        BackendFactory.setOverride {
+            object : Backend() {
+                override fun setCurrentDeck(did: Long): OpChanges {
+                    if (failSelection) {
+                        failSelection = false
+                        failReload = true
+                        throw failure
+                    }
+                    return super.setCurrentDeck(did)
+                }
+                override fun deckTree(now: Long): DeckTreeNode = if (failReload) throw failure else super.deckTree(now)
+            }
+        }
+        addNoteToDeck(DEFAULT_DECK_ID)
+        val deleted = addDeck("Deleted", setAsSelected = true)
+        val survivor = addDeck("Survivor")
+        addNoteToDeck(deleted)
+        withDeckPicker(deckCount = 0) { picker ->
+            picker.awaitVisibleDecks(listOf(DEFAULT_DECK_ID, deleted, survivor))
+            advanceRobolectricLooperUntil {
+                picker.viewModel.loadDeckCounts?.isCompleted == true && picker.fragment?.selectedDeckId == deleted
+            }
+            if (reminders) {
+                Prefs.newReviewRemindersEnabled = true
+                picker.supportFragmentManager.setDeckPickerContextMenuResult(
+                    DeckPickerContextMenuResult(deleted, DeckPickerContextMenuOption.SCHEDULE_REMINDERS),
+                )
+                advanceRobolectricLooper()
+                assertIs<ScheduleRemindersFragment>(picker.supportFragmentManager.findFragmentById(R.id.studyoptions_fragment))
+            }
+            throwOnShowError = false
+            failSelection = true
+            val deletion = picker.deleteDeck(deleted)
+            advanceRobolectricLooperUntil { deletion.isCompleted && picker.viewModel.loadDeckCounts?.isCompleted == true }
+            while (picker.viewModel.pendingErrors.value.isNotEmpty()) {
+                val id = picker.viewModel.pendingErrors.value.first().id
+                advanceRobolectricLooperUntil { (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true }
+                ShadowDialog.getLatestDialog().dismiss()
+                advanceRobolectricLooperUntil { picker.viewModel.pendingErrors.value.firstOrNull()?.id != id }
+            }
+            assertEquals(DEFAULT_DECK_ID, col.decks.selected())
+            block(picker, deleted) { failReload = false }
+        }
+    }
+}
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryTransactionProbeTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryTransactionProbeTest.kt
new file mode 100644
--- /dev/null
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryTransactionProbeTest.kt
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package com.ichi2.anki
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import anki.backend.backendError
+import anki.collection.OpChangesWithCount
+import anki.decks.DeckTreeNode
+import com.ichi2.anki.backend.DatabaseCorruption
+import com.ichi2.anki.dialogs.DatabaseErrorDialog
+import kotlinx.coroutines.runBlocking
+import net.ankiweb.rsdroid.Backend
+import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException
+import net.ankiweb.rsdroid.BackendFactory
+import org.junit.After
+import org.junit.Test
+import org.junit.runner.RunWith
+import kotlin.test.assertEquals
+
+@RunWith(AndroidJUnit4::class)
+class DeckPickerRecoveryTransactionProbeTest : RobolectricTest() {
+    @After
+    fun resetBackendFactory() {
+        BackendFactory.setOverride(null)
+    }
+
+    @Test
+    fun `queued corruption opens recovery after a database confirmation is dismissed`() {
+        val corruption = BackendDbCorruptException(backendError { message = "database disk image is malformed" })
+        var corrupt = false
+        runBlocking { CollectionManager.discardBackend() }
+        BackendFactory.setOverride {
+            object : Backend() {
+                override fun removeDecks(dids: Iterable<Long>): OpChangesWithCount {
+                    corrupt = true
+                    throw corruption
+                }
+                override fun deckTree(now: Long): DeckTreeNode = if (corrupt) throw corruption else super.deckTree(now)
+            }
+        }
+        addDeck("Target")
+        val previouslyDetected = DatabaseCorruption.isDetected
+        DatabaseCorruption.isDetected = false
+        try {
+            withDeckPicker(deckCount = 0) { picker ->
+                advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true }
+                throwOnShowError = false
+                picker.showDatabaseErrorDialog(DatabaseErrorDialog.DatabaseErrorDialogType.DIALOG_CONFIRM_DATABASE_CHECK)
+                val confirmation = picker.supportFragmentManager.fragments.filterIsInstance<DatabaseErrorDialog>().single()
+                // A background refresh can discover corruption while the confirmation is open.
+                corrupt = true
+                picker.viewModel.reloadDeckCounts()
+                advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true }
+                advanceRobolectricLooper()
+                assertEquals(1, picker.viewModel.pendingErrors.value.size)
+
+                confirmation.dismiss()
+                advanceRobolectricLooperUntil { picker.viewModel.pendingErrors.value.isEmpty() }
+                try {
+                    assertEquals(1, picker.supportFragmentManager.fragments.filterIsInstance<DatabaseErrorDialog>()
+                        .count { it.dialog?.isShowing == true },
+                        "The queued corruption must open recovery now, rather than becoming a background notification")
+                } finally {
+                    picker.supportFragmentManager.fragments.filterIsInstance<DatabaseErrorDialog>().forEach { it.dismiss() }
+                }
+            }
+        } finally {
+            DatabaseCorruption.isDetected = previouslyDetected
+        }
+    }
+}

[david-allison]

Multiple issues, consider a commit for each

diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRenderedSelectionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRenderedSelectionTest.kt
new file mode 100644
index 0000000000..ace904ace8
--- /dev/null
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRenderedSelectionTest.kt
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package com.ichi2.anki
+
+import android.view.KeyEvent
+import android.widget.TextView
+import androidx.core.widget.doAfterTextChanged
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.LifecycleRegistry
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.ichi2.anki.progress.ViewModelProgress
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.runBlocking
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.annotation.Config
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+import kotlin.concurrent.thread
+import kotlin.test.assertEquals
+import kotlin.test.assertNotNull
+import kotlin.test.assertTrue
+
+@RunWith(AndroidJUnit4::class)
+@Config(qualifiers = "xlarge")
+class DeckPickerRenderedSelectionTest : RobolectricTest() {
+    @Test
+    fun `resuming the pane during a refresh cannot authorize deleting a different deck`() {
+        val shown = addDeck("Shown", setAsSelected = true)
+        val other = addDeck("Other")
+        addNoteToDeck(shown)
+        addNoteToDeck(other)
+        withDeckPicker(deckCount = 0) { picker ->
+            advanceRobolectricLooperUntil { picker.fragment?.shownDeckId == shown && picker.viewModel.loadDeckCounts?.isCompleted == true }
+            advanceRobolectricLooper()
+            val pane = assertNotNull(picker.fragment)
+            val label = pane.requireView().findViewById<TextView>(R.id.studyoptions_deck_name)
+            assertEquals("Shown", label.text.toString())
+            val lifecycle = pane.viewLifecycleOwner.lifecycle as LifecycleRegistry
+            lifecycle.currentState = Lifecycle.State.CREATED
+            col.decks.select(other)
+
+            // Pause the collection worker after the ID is published, before StudyOptionsState.
+            val idPublished = CountDownLatch(1)
+            val releaseRefresh = CountDownLatch(1)
+            val observer =
+                CoroutineScope(Dispatchers.Unconfined).launch {
+                    pane.viewModel.flowOfSelectedDeckId.collect { id ->
+                        if (id == other) {
+                            idPublished.countDown()
+                            check(releaseRefresh.await(10, TimeUnit.SECONDS)) { "Timed out waiting to release the study options refresh" }
+                        }
+                    }
+                }
+            val previousQueue = CollectionManager.setTestDispatcher(Dispatchers.IO.limitedParallelism(1), useReentrantLock = false)
+            val refresh = thread(name = "study-options-refresh") { runBlocking { pane.viewModel.refreshData() } }
+            try {
+                assertTrue(idPublished.await(5, TimeUnit.SECONDS), "The refresh did not publish the new deck ID")
+                var renderedAgain = false
+                label.doAfterTextChanged { renderedAgain = true }
+                lifecycle.currentState = Lifecycle.State.RESUMED
+                advanceRobolectricLooperUntil { renderedAgain }
+                assertEquals("Shown", label.text.toString())
+
+                picker.onKeyUp(
+                    KeyEvent.KEYCODE_DEL,
+                    KeyEvent(0, 0, KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL, 0, KeyEvent.META_SHIFT_ON),
+                )
+                releaseRefresh.countDown()
+                refresh.join()
+                advanceRobolectricLooperUntil { picker.viewModel.progressManager.progress.value is ViewModelProgress.Idle }
+                assertNotNull(
+                    CollectionManager.getColUnsafe().decks.byName("Other"),
+                    "Shift+Delete must not delete Other while the pane displays Shown",
+                )
+            } finally {
+                releaseRefresh.countDown()
+                refresh.join()
+                observer.cancel()
+                CollectionManager.setTestDispatcher(previousQueue)
+            }
+        }
+    }
+}
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRefreshSelectionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRefreshSelectionTest.kt
new file mode 100644
index 0000000000..3633390634
--- /dev/null
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRefreshSelectionTest.kt
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package com.ichi2.anki
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.ichi2.anki.deckpicker.DeckPickerViewModel
+import kotlinx.coroutines.Job
+import net.ankiweb.rsdroid.Backend
+import net.ankiweb.rsdroid.BackendFactory
+import org.junit.After
+import org.junit.Test
+import org.junit.runner.RunWith
+import kotlin.test.assertEquals
+
+@RunWith(AndroidJUnit4::class)
+class DeckPickerRefreshSelectionTest : RobolectricTest() {
+    @After
+    fun resetBackend() {
+        BackendFactory.setOverride(null)
+    }
+
+    @Test
+    fun `refresh does not replace a newer focused deck with its initial selection`() =
+        runTest {
+            var duringStats: (() -> Unit)? = null
+            CollectionManager.discardBackend()
+            BackendFactory.setOverride {
+                object : Backend() {
+                    override fun studiedToday(): String {
+                        val action = duringStats
+                        duringStats = null
+                        action?.invoke()
+                        return super.studiedToday()
+                    }
+                }
+            }
+            val old = addDeck("Old", setAsSelected = true)
+            val latest = addDeck("Latest")
+            val model = DeckPickerViewModel()
+            model.reloadDeckCounts().join()
+            assertEquals(old, model.focusedDeck)
+            lateinit var selection: Job
+            // Place the newer selection after the refresh reads its first collection snapshot.
+            duringStats = { selection = model.selectDeck(latest) }
+            model.reloadDeckCounts().join()
+            selection.join()
+            assertEquals(latest, col.decks.selected())
+            assertEquals(
+                latest,
+                model.focusedDeck,
+                "A refresh must not recenter the list on the deck that was selected before the user's newer selection",
+            )
+        }
+}