From e2b09c98ab020c0ebf5d2076dd6ec16e28d4f218 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:18:11 +0530 Subject: [PATCH 01/30] refactor: extract showException from runCatching So an exception caught outside an activity coroutine, for example in a ViewModel, can be shown the same way, with database corruption still opening the recovery dialog. Assisted-by: Claude Opus 5.5 --- .../java/com/ichi2/anki/CoroutineHelpers.kt | 92 ++++++++++--------- 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt index 5e4eb5049373..246dc570164b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt @@ -158,53 +158,61 @@ suspend fun FragmentActivity.runCatching( showError(exc.localizedMessage!!, exc.toCrashReportData(this, reportException = false)) return null } - when (exc) { - is CancellationException -> { - throw exc // CancellationException should be re-thrown to propagate it to the parent coroutine - } - is BackendInterruptedException -> { - Timber.w(exc, errorMessage) - exc.localizedMessage?.let { showSnackbar(it) } - } - is StorageNotConfiguredException -> { - // expected before first-run setup completes: no crash report - // edge case when 'ensureStorageIsReady' was insufficient - Timber.w(exc, errorMessage) - if (!isFinishing) redirectToMainEntryPoint() - } - is BackendNetworkException, is BackendSyncException, is StorageAccessException, is BackendCardTypeException, - is CollectionLockedException, - -> { - // these exceptions do not generate worthwhile crash reports - Timber.i("Showing error dialog but not sending a crash report.") - showError(exc.localizedMessage!!, exc.toCrashReportData(this, reportException = false)) - } - // must precede `is BackendException`: BackendDbCorruptException is a subclass - is SQLiteDatabaseCorruptException, is BackendDbCorruptException -> { - Timber.e(exc, errorMessage) - DatabaseCorruption.isDetected = true - if (callerTrace != null) Timber.e(callerTrace) - (this as? AnkiActivity) - ?.showDatabaseErrorDialog( - errorDialogType = DatabaseErrorDialogType.DIALOG_LOAD_FAILED, - exceptionData = DatabaseErrorDialog.CustomExceptionData.fromException(exc), - ) - } - is BackendException -> { - Timber.e(exc, errorMessage) - if (callerTrace != null) Timber.e(callerTrace) - showError(exc.localizedMessage!!, exc.toCrashReportData(this)) - } - else -> { - Timber.e(exc, errorMessage) - if (callerTrace != null) Timber.e(callerTrace) - showError(exc) - } + if (exc is CancellationException) { + throw exc // CancellationException should be re-thrown to propagate it to the parent coroutine } + showException(exc, errorMessage, callerTrace) } return null } +fun FragmentActivity.showException( + exc: Exception, + errorMessage: String? = null, + callerTrace: String? = null, +) { + when (exc) { + is BackendInterruptedException -> { + Timber.w(exc, errorMessage) + exc.localizedMessage?.let { showSnackbar(it) } + } + is StorageNotConfiguredException -> { + // expected before first-run setup completes: no crash report + // edge case when 'ensureStorageIsReady' was insufficient + Timber.w(exc, errorMessage) + if (!isFinishing) redirectToMainEntryPoint() + } + is BackendNetworkException, is BackendSyncException, is StorageAccessException, is BackendCardTypeException, + is CollectionLockedException, + -> { + // these exceptions do not generate worthwhile crash reports + Timber.i("Showing error dialog but not sending a crash report.") + showError(exc.localizedMessage!!, exc.toCrashReportData(this, reportException = false)) + } + // must precede `is BackendException`: BackendDbCorruptException is a subclass + is SQLiteDatabaseCorruptException, is BackendDbCorruptException -> { + Timber.e(exc, errorMessage) + DatabaseCorruption.isDetected = true + if (callerTrace != null) Timber.e(callerTrace) + (this as? AnkiActivity) + ?.showDatabaseErrorDialog( + errorDialogType = DatabaseErrorDialogType.DIALOG_LOAD_FAILED, + exceptionData = DatabaseErrorDialog.CustomExceptionData.fromException(exc), + ) + } + is BackendException -> { + Timber.e(exc, errorMessage) + if (callerTrace != null) Timber.e(callerTrace) + showError(exc.localizedMessage!!, exc.toCrashReportData(this)) + } + else -> { + Timber.e(exc, errorMessage) + if (callerTrace != null) Timber.e(callerTrace) + showError(exc) + } + } +} + /** * Launch a job that catches any uncaught errors and reports them to the user. * Errors from the backend contain localized text that is often suitable to show to the user as-is. From e9582f93a12635d4671d4a825c57f9904cdabc3d Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:24:21 +0530 Subject: [PATCH 02/30] fix(deck-picker): show errors instead of crashing Deleting a deck, rebuilding or emptying a filtered deck, selecting a new deck, collapsing or expanding a deck, or refreshing the deck list crashed the app when the backend failed. The ViewModel now catches these errors and keeps them until the Deck Picker can show them, the same way launchCatchingTask did. A deck is only shown collapsed or expanded once the collection has saved it, so a failed save leaves the list matching the collection. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 16 +- .../anki/deckpicker/DeckPickerViewModel.kt | 45 ++- .../ichi2/anki/utils/MessageQueueObserver.kt | 14 +- .../com/ichi2/anki/DeckPickerErrorTest.kt | 267 ++++++++++++++++++ 4 files changed, 316 insertions(+), 26 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index d52ef721781f..ee20711e857a 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -206,6 +206,7 @@ import com.ichi2.anki.utils.ext.positionIsVisible import com.ichi2.anki.utils.ext.setFragmentResultListener import com.ichi2.anki.utils.ext.setImageDrawableSafe import com.ichi2.anki.utils.ext.showDialogFragment +import com.ichi2.anki.utils.observeMessages import com.ichi2.anki.widgets.DeckAdapter import com.ichi2.anki.widgets.DeckHierarchyLinesDecoration import com.ichi2.anki.worker.SyncMediaWorker @@ -1009,6 +1010,7 @@ open class DeckPicker : viewModel.flowOfCreateShortcut.launchCollectionInLifecycleScope(::createIcon) viewModel.flowOfDisableShortcuts.launchCollectionInLifecycleScope(::disableDeckAndChildrenShortcuts) viewModel.onError.launchCollectionInLifecycleScope(::onError) + observeMessages(viewModel.pendingErrors, viewModel::errorShown) { showException(it) } viewModel.flowOfPromptUserToUpdateScheduler.launchCollectionInLifecycleScope(::onPromptUserToUpdateScheduler) viewModel.flowOfOptionsMenuState.filterNotNull().launchCollectionInLifecycleScope(::onOptionsMenuUpdated) viewModel.flowOfStudiedTodayStats.launchCollectionInLifecycleScope(::onStudiedTodayChanged) @@ -2409,15 +2411,9 @@ open class DeckPicker : * Deletes the provided deck, child decks, and all cards inside. * @param did ID of the deck to delete */ - fun deleteDeck(did: DeckId) = - launchCatchingTask { - viewModel.deleteDeck(did).join() - } + fun deleteDeck(did: DeckId) = viewModel.deleteDeck(did) - private fun deleteSelectedDeck() = - launchCatchingTask { - viewModel.deleteSelectedDeck().join() - } + private fun deleteSelectedDeck() = viewModel.deleteSelectedDeck() @NeedsTest("14285: regression test to ensure UI is updated after this call") fun rebuildFiltered(did: DeckId) { @@ -2429,9 +2425,7 @@ open class DeckPicker : } private fun emptyFiltered(did: DeckId) { - launchCatchingTask { - viewModel.emptyFilteredDeck(did).join() - } + viewModel.emptyFilteredDeck(did) } override fun onAttachedToWindow() { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index d3d9eeedfe09..9b1c7ba82b47 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -45,6 +45,10 @@ import com.ichi2.anki.settings.Prefs import com.ichi2.anki.sync.syncStatus import com.ichi2.anki.syncAuth import com.ichi2.anki.utils.Destination +import com.ichi2.anki.utils.MessageQueue +import com.ichi2.anki.utils.MessageQueue.MessageId +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableSharedFlow @@ -59,6 +63,8 @@ import kotlinx.coroutines.withContext import net.ankiweb.rsdroid.RustCleanup import net.ankiweb.rsdroid.exceptions.BackendNetworkException import timber.log.Timber +import kotlin.coroutines.CoroutineContext +import kotlin.coroutines.EmptyCoroutineContext import com.ichi2.anki.common.destinations.Destination as NavigateDestination /** @@ -142,6 +148,8 @@ class DeckPickerViewModel : val flowOfDestination = MutableSharedFlow(extraBufferCapacity = 1) val flowOfNavigate = MutableSharedFlow(extraBufferCapacity = 1) override val onError = MutableSharedFlow(extraBufferCapacity = 1) + private val errorQueue = MessageQueue() + val pendingErrors = errorQueue.messages val flowOfExportDeck = MutableSharedFlow() val flowOfCreateShortcut = MutableSharedFlow() val flowOfDisableShortcuts = MutableSharedFlow>() @@ -238,14 +246,29 @@ class DeckPickerViewModel : flowOfStudyDeck.emit(deckId) } + fun errorShown(id: MessageId) = errorQueue.acknowledge(id) + + private fun launchCatchingTask( + context: CoroutineContext = EmptyCoroutineContext, + block: suspend CoroutineScope.() -> Unit, + ): Job = + viewModelScope.launch(context) { + try { + block() + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + withContext(Dispatchers.Main.immediate) { errorQueue.enqueue(e) } + } + } + /** * Deletes the provided deck, child decks. and all cards inside. * * @param did ID of the deck to delete */ - @CheckResult fun deleteDeck(did: DeckId) = - viewModelScope.launch { + launchCatchingTask { progressManager.withProgress(message = DeckPickerProgress.DELETING_DECK) { val deckName = withCol { decks.getLegacy(did)!!.name } val changes = @@ -267,9 +290,8 @@ class DeckPickerViewModel : /** * Deletes the currently selected deck */ - @CheckResult fun deleteSelectedDeck() = - viewModelScope.launch { + launchCatchingTask { progressManager.withProgress(message = DeckPickerProgress.DELETING_DECK) { val targetDeckId = withCol { decks.selected() } deleteDeck(targetDeckId).join() @@ -305,7 +327,7 @@ class DeckPickerViewModel : } fun emptyFilteredDeck(deckId: DeckId): Job = - viewModelScope.launch { + launchCatchingTask { progressManager.withProgress { Timber.i("empty filtered deck %s", deckId) withCol { decks.select(deckId) } @@ -319,7 +341,7 @@ class DeckPickerViewModel : */ @CheckResult fun rebuildFilteredDeck(deckId: DeckId): Job = - viewModelScope.launch { + launchCatchingTask { progressManager.withProgress(message = DeckPickerProgress.REBUILDING_FILTERED_DECK) { Timber.i("rebuilding filtered deck %s", deckId) withCol { @@ -334,7 +356,7 @@ class DeckPickerViewModel : * Marks [deckId] as the currently selected deck and updates the selection in the deck list. */ fun selectDeck(deckId: DeckId) = - viewModelScope.launch { + launchCatchingTask { // TODO: should we always reset the Card Browser default deck here? withCol { decks.select(deckId) } focusedDeck = deckId @@ -457,7 +479,7 @@ class DeckPickerViewModel : fun reloadDeckCounts(): Job { loadDeckCounts?.cancel() val loadDeckCounts = - viewModelScope.launch(Dispatchers.IO) { + launchCatchingTask(Dispatchers.IO) { Timber.d("Refreshing deck list") val (deckDueTree, collectionHasNoCards) = withCol { @@ -507,18 +529,19 @@ class DeckPickerViewModel : } fun toggleDeckExpand(deckId: DeckId) = - viewModelScope.launch { + launchCatchingTask { // update stored state dueTree?.find(deckId)?.run { - collapsed = !collapsed + val collapse = !collapsed // Anki uses scope as Reviewer in deck browser withCol { decks.setCollapsed( did, - collapsed = collapsed, + collapsed = collapse, SetDeckCollapsedRequest.Scope.REVIEWER, ) } + collapsed = collapse } flowOfRefreshDeckList.emit(Unit) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/utils/MessageQueueObserver.kt b/AnkiDroid/src/main/java/com/ichi2/anki/utils/MessageQueueObserver.kt index 6e1842c1975f..67d9faa5a665 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/utils/MessageQueueObserver.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/utils/MessageQueueObserver.kt @@ -6,6 +6,7 @@ package com.ichi2.anki.utils import androidx.appcompat.app.AlertDialog import androidx.fragment.app.Fragment import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle import androidx.lifecycle.withStarted @@ -22,12 +23,17 @@ fun Fragment.observeMessages( messages: StateFlow>>, acknowledge: (MessageId) -> Unit, show: suspend (T) -> Unit, +) = viewLifecycleOwner.observeMessages(messages, acknowledge, show) + +fun LifecycleOwner.observeMessages( + messages: StateFlow>>, + acknowledge: (MessageId) -> Unit, + show: suspend (T) -> Unit, ) { - val owner = viewLifecycleOwner - owner.lifecycleScope.launch { - owner.repeatOnLifecycle(Lifecycle.State.CREATED) { + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.CREATED) { messages.mapNotNull { it.firstOrNull() }.collect { pending -> - owner.withStarted {} + withStarted {} show(pending.message) acknowledge(pending.id) } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt new file mode 100644 index 000000000000..8662741d7c1b --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -0,0 +1,267 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// SPDX-FileCopyrightText: Copyright (c) 2026 Ashish Yadav +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.backend.backendError +import anki.collection.OpChanges +import anki.collection.OpChangesWithCount +import anki.decks.DeckTreeNode +import anki.decks.SetDeckCollapsedRequest +import com.ichi2.anki.backend.DatabaseCorruption +import com.ichi2.anki.dialogs.DatabaseErrorDialog +import com.ichi2.anki.dialogs.DeckPickerContextMenu.DeckPickerContextMenuOption +import com.ichi2.anki.dialogs.DeckPickerContextMenuResult +import com.ichi2.anki.dialogs.setDeckPickerContextMenuResult +import com.ichi2.anki.dialogs.utils.input +import com.ichi2.anki.dialogs.utils.performPositiveClick +import com.ichi2.anki.libanki.DeckId +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.jupiter.api.Assertions.assertAll +import org.junit.runner.RunWith +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowDialog +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class DeckPickerErrorTest : RobolectricTest() { + private val failure = + BackendIoException( + backendError { + kind = BackendError.Kind.IO_ERROR + message = "Operation not permitted (os error 1)" + }, + ) + + @After + fun resetBackendFactory() { + BackendFactory.setOverride(null) + } + + @Test + fun `failed deck deletion while stopped shows an error on resume`() { + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = throw failure + } + } + val deckId = addDeck("Deck") + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + val lifecycle = deckPicker.lifecycle as LifecycleRegistry + // ViewModel work can finish after the activity stops collecting its error flow. + lifecycle.currentState = Lifecycle.State.CREATED + advanceRobolectricLooperUntil { deckPicker.viewModel.onError.subscriptionCount.value == 0 } + try { + val job = deckPicker.deleteDeck(deckId) + advanceRobolectricLooperUntil { job.isCompleted } + } finally { + lifecycle.currentState = Lifecycle.State.RESUMED + } + assertErrorShown() + } + } + + @Test + fun `database corruption during deck deletion opens recovery dialog`() { + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = + throw BackendDbCorruptException( + backendError { message = "database disk image is malformed" }, + ) + } + } + val deckId = addDeck("Deck") + val previouslyDetected = DatabaseCorruption.isDetected + DatabaseCorruption.isDetected = false + try { + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + val job = deckPicker.deleteDeck(deckId) + advanceRobolectricLooperUntil { + job.isCompleted && (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true + } + + assertAll( + { assertTrue(DatabaseCorruption.isDetected, "Backend corruption must be recorded") }, + { + assertNotNull( + deckPicker.supportFragmentManager.fragments + .filterIsInstance() + .singleOrNull(), + "Backend corruption must open the recovery dialog, not a generic error alert", + ) + }, + ) + } + } finally { + DatabaseCorruption.isDetected = previouslyDetected + } + } + + @Test + fun `failed deck deletion shows an error`() { + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = throw failure + } + } + val deckId = addDeck("Deck") + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + deckPicker.selectContextMenuOption(DeckPickerContextMenuOption.DELETE_DECK, deckId) + + assertErrorShown() + } + } + + @Test + @Config(qualifiers = "xlarge") + fun `failed selected deck deletion shows an error`() { + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = throw failure + } + } + addDeck("Deck", setAsSelected = true) + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + shadowOf(deckPicker).clickMenuItem(R.id.action_deck_delete) + + assertErrorShown() + } + } + + @Test + fun `failed filtered deck rebuild shows an error`() { + useBackend { + object : Backend() { + override fun rebuildFilteredDeck(did: Long): OpChangesWithCount = throw failure + } + } + val deckId = addDynamicDeck("Filtered") + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + deckPicker.selectContextMenuOption(DeckPickerContextMenuOption.CUSTOM_STUDY_REBUILD, deckId) + + assertErrorShown() + } + } + + @Test + fun `failed filtered deck emptying shows an error`() { + useBackend { + object : Backend() { + override fun emptyFilteredDeck(did: Long): OpChanges = throw failure + } + } + val deckId = addDynamicDeck("Filtered") + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + deckPicker.selectContextMenuOption(DeckPickerContextMenuOption.CUSTOM_STUDY_EMPTY, deckId) + + assertErrorShown() + } + } + + @Test + fun `failed selection of a new deck shows an error`() { + useBackend { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges = throw failure + } + } + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + deckPicker.showCreateDeckDialog() + (ShadowDialog.getLatestDialog() as AlertDialog).apply { + input = "Deck" + performPositiveClick() + } + + assertErrorShown() + } + } + + @Test + fun `failed deck list update shows an error`() { + var failDeckTree = false + useBackend { + object : Backend() { + override fun deckTree(now: Long): DeckTreeNode = if (failDeckTree) throw failure else super.deckTree(now) + } + } + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + failDeckTree = true + deckPicker.viewModel.updateDeckList() + + assertErrorShown() + } + } + + @Test + fun `failed deck expand toggle shows an error and keeps the saved state`() { + useBackend { + object : Backend() { + override fun setDeckCollapsed( + deckId: Long, + collapsed: Boolean, + scope: SetDeckCollapsedRequest.Scope, + ): OpChanges = throw failure + } + } + val parent = addDeck("Parent") + addDeck("Parent::Child") + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + advanceRobolectricLooperUntil { deckPicker.viewModel.loadDeckCounts?.isCompleted == true } + val job = deckPicker.viewModel.toggleDeckExpand(parent) + advanceRobolectricLooperUntil { job.isCompleted } + + assertErrorShown() + assertEquals( + col.decks.get(parent)!!.collapsed, + deckPicker.viewModel.dueTree + ?.find(parent) + ?.collapsed, + ) + } + } + + private fun useBackend(backend: () -> Backend) { + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { backend() } + } + + private fun DeckPicker.selectContextMenuOption( + option: DeckPickerContextMenuOption, + deckId: DeckId, + ) = supportFragmentManager.setDeckPickerContextMenuResult(DeckPickerContextMenuResult(deckId, option)) + + private fun assertErrorShown() { + advanceRobolectricLooperUntil { (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true } + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } +} From 8ae3b1e9bd7f320396000645d5a050ec6a4587fa Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 13 Oct 2026 02:40:24 +0530 Subject: [PATCH 03/30] fix(empty-cards): show errors instead of crashing Deleting empty cards crashed the app when the backend failed. The ViewModel now catches the error and shows it instead. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 2 +- .../anki/dialogs/EmptyCardsDialogFragment.kt | 5 +---- .../com/ichi2/anki/DeckPickerErrorTest.kt | 20 +++++++++++++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 9b1c7ba82b47..c012624120a6 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -308,7 +308,7 @@ class DeckPickerViewModel : fun deleteEmptyCards( report: EmptyCardsReport, preserveNotes: Boolean, - ) = viewModelScope.launch { + ) = launchCatchingTask { progressManager.withProgress(message = DeckPickerProgress.DELETING_EMPTY_CARDS) { // https://github.com/ankitects/anki/blob/39e293b27d36318e00131fd10144755eec8d1922/qt/aqt/emptycards.py#L98-L109 val toDelete = mutableListOf() diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/EmptyCardsDialogFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/EmptyCardsDialogFragment.kt index 112c2fc3c272..eeb1bf7e4d65 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/EmptyCardsDialogFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/EmptyCardsDialogFragment.kt @@ -39,7 +39,6 @@ import com.ichi2.anki.databinding.DialogEmptyCardsBinding import com.ichi2.anki.dialogs.EmptyCardsUiState.EmptyCardsSearchFailure import com.ichi2.anki.dialogs.EmptyCardsUiState.EmptyCardsSearchResult import com.ichi2.anki.dialogs.EmptyCardsUiState.SearchingForEmptyCards -import com.ichi2.anki.launchCatchingTask import com.ichi2.anki.libanki.NoteId import com.ichi2.anki.libanki.emptyCids import com.ichi2.anki.ui.internationalization.sentenceCase @@ -296,7 +295,5 @@ fun DeckPicker.startDeletingEmptyCards( report.emptyCids().size, keepNotes, ) - launchCatchingTask { - viewModel.deleteEmptyCards(report, keepNotes).join() - } + viewModel.deleteEmptyCards(report, keepNotes) } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 8662741d7c1b..adf9f6d67cee 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -17,6 +17,7 @@ import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.DeckPickerContextMenu.DeckPickerContextMenuOption import com.ichi2.anki.dialogs.DeckPickerContextMenuResult import com.ichi2.anki.dialogs.setDeckPickerContextMenuResult +import com.ichi2.anki.dialogs.startDeletingEmptyCards import com.ichi2.anki.dialogs.utils.input import com.ichi2.anki.dialogs.utils.performPositiveClick import com.ichi2.anki.libanki.DeckId @@ -250,6 +251,25 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `failed empty cards deletion shows an error`() { + useBackend { + object : Backend() { + override fun removeCards(cardIds: Iterable): OpChangesWithCount = throw failure + } + } + addStandardNoteType("Empty", arrayOf("Front"), "", "") + addNoteUsingNoteTypeName("Empty", "Front") + val report = col.getEmptyCards() + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + deckPicker.startDeletingEmptyCards(report, keepNotes = false) + + assertErrorShown() + } + } + private fun useBackend(backend: () -> Backend) { runBlocking { CollectionManager.discardBackend() } BackendFactory.setOverride { backend() } From 9e4c51d909cd68e1148fc310df349fb515d7e3b4 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 13 Oct 2026 02:44:45 +0530 Subject: [PATCH 04/30] refactor(deck-picker): refresh once after rebuild Rebuilding a filtered deck refreshed the deck list and the study options panel twice: once from the ViewModel and once from the Deck Picker. Keep the ViewModel's refresh, with the test issue 14285 asked for. Assisted-by: Claude Opus 5.5 --- .../src/main/java/com/ichi2/anki/DeckPicker.kt | 7 +------ .../ichi2/anki/deckpicker/DeckPickerViewModel.kt | 1 - .../src/test/java/com/ichi2/anki/DeckPickerTest.kt | 13 +++++++++++++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index ee20711e857a..bb3cf2fcbcd5 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -2415,13 +2415,8 @@ open class DeckPicker : private fun deleteSelectedDeck() = viewModel.deleteSelectedDeck() - @NeedsTest("14285: regression test to ensure UI is updated after this call") fun rebuildFiltered(did: DeckId) { - launchCatchingTask { - viewModel.rebuildFilteredDeck(did).join() - updateDeckList() - tryShowStudyOptionsPanel() - } + viewModel.rebuildFilteredDeck(did) } private fun emptyFiltered(did: DeckId) { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index c012624120a6..de7d48a75277 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -339,7 +339,6 @@ class DeckPickerViewModel : /** * Rebuilds a filtered deck with its current filter settings */ - @CheckResult fun rebuildFilteredDeck(deckId: DeckId): Job = launchCatchingTask { progressManager.withProgress(message = DeckPickerProgress.REBUILDING_FILTERED_DECK) { diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt index 3869c10bebfd..1d4abbb464e7 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt @@ -757,6 +757,19 @@ class DeckPickerTest : RobolectricTest() { } } + @Test + fun `rebuilding a filtered deck refreshes the deck list - issue 14285`() = + deckPicker { + repeat(4) { addBasicNote("$it", "") } + val deckId = addDynamicDeck("Filtered", search = "") + selectContextMenuOption(ContextMenuOption.CUSTOM_STUDY_EMPTY, deckId) + advanceRobolectricLooperUntil { dueTree?.find(deckId)?.newCount == 0 } + + selectContextMenuOption(ContextMenuOption.CUSTOM_STUDY_REBUILD, deckId) + + advanceRobolectricLooperUntil { dueTree?.find(deckId)?.newCount == 4 } + } + private suspend fun TestScope.withQueuedCollectionAccess(block: suspend () -> T): T { val previousQueue = CollectionManager.setTestDispatcher(StandardTestDispatcher(testScheduler), useReentrantLock = false) try { From 80f03a81206f353e4061c915219b2432431de0c5 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:58:24 +0530 Subject: [PATCH 05/30] fix(deck-picker): keep an error until its dialog is dismissed A Deck Picker error counted as handled as soon as its dialog opened, so a configuration change such as a font size change lost it before it was read. The error now stays queued until the dialog is dismissed and is shown again on the recreated screen. The old dialog closes with its screen without sending its crash report, so the error is reported once, when the user dismisses it. Assisted-by: Claude Opus 5.5 --- .../java/com/ichi2/anki/CoroutineHelpers.kt | 14 +-- .../main/java/com/ichi2/anki/DeckPicker.kt | 3 +- .../ichi2/anki/utils/MessageQueueObserver.kt | 16 ++++ .../anki/DeckPickerAdditionalReviewTest.kt | 82 +++++++++++++++++ .../com/ichi2/anki/DeckPickerErrorTest.kt | 91 +++++++++++++++++++ .../anki/utils/MessageQueueObserverTest.kt | 14 +++ 6 files changed, 213 insertions(+), 7 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerAdditionalReviewTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt index 246dc570164b..e32df2690fce 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt @@ -170,7 +170,7 @@ fun FragmentActivity.showException( exc: Exception, errorMessage: String? = null, callerTrace: String? = null, -) { +): AlertDialog? { when (exc) { is BackendInterruptedException -> { Timber.w(exc, errorMessage) @@ -187,7 +187,7 @@ fun FragmentActivity.showException( -> { // these exceptions do not generate worthwhile crash reports Timber.i("Showing error dialog but not sending a crash report.") - showError(exc.localizedMessage!!, exc.toCrashReportData(this, reportException = false)) + return showError(exc.localizedMessage!!, exc.toCrashReportData(this, reportException = false)) } // must precede `is BackendException`: BackendDbCorruptException is a subclass is SQLiteDatabaseCorruptException, is BackendDbCorruptException -> { @@ -203,14 +203,15 @@ fun FragmentActivity.showException( is BackendException -> { Timber.e(exc, errorMessage) if (callerTrace != null) Timber.e(callerTrace) - showError(exc.localizedMessage!!, exc.toCrashReportData(this)) + return showError(exc.localizedMessage!!, exc.toCrashReportData(this)) } else -> { Timber.e(exc, errorMessage) if (callerTrace != null) Timber.e(callerTrace) - showError(exc) + return showError(exc) } } + return null } /** @@ -334,14 +335,14 @@ fun Fragment.launchCatchingViewTask( fun Context.showError( message: CharSequence, crashReportData: CrashReportData?, -) { +): AlertDialog? { crashReportData.throwIfDialogUnusable(message.toString()) Timber.i("Error dialog displayed") val helpAction = crashReportData?.helpAction?.takeIf { it.canExecute(this) } - try { + return try { AlertDialog .Builder(this) .create { @@ -369,6 +370,7 @@ fun Context.showError( // issue 12718: activity provided by `context` was not running Timber.w(ex, "unable to display error dialog") crashReportData?.sendCrashReport() + null } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index bb3cf2fcbcd5..46a5124f98df 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -198,6 +198,7 @@ import com.ichi2.anki.ui.internationalization.sentenceCase import com.ichi2.anki.ui.windows.permissions.PermissionsActivity import com.ichi2.anki.utils.Destination import com.ichi2.anki.utils.ShortcutUtils +import com.ichi2.anki.utils.awaitDismissal import com.ichi2.anki.utils.ext.dismissAllDialogFragments import com.ichi2.anki.utils.ext.doOnScrolled import com.ichi2.anki.utils.ext.launchCollectionInLifecycleScope @@ -1010,7 +1011,7 @@ open class DeckPicker : viewModel.flowOfCreateShortcut.launchCollectionInLifecycleScope(::createIcon) viewModel.flowOfDisableShortcuts.launchCollectionInLifecycleScope(::disableDeckAndChildrenShortcuts) viewModel.onError.launchCollectionInLifecycleScope(::onError) - observeMessages(viewModel.pendingErrors, viewModel::errorShown) { showException(it) } + observeMessages(viewModel.pendingErrors, viewModel::errorShown) { showException(it)?.awaitDismissal() } viewModel.flowOfPromptUserToUpdateScheduler.launchCollectionInLifecycleScope(::onPromptUserToUpdateScheduler) viewModel.flowOfOptionsMenuState.filterNotNull().launchCollectionInLifecycleScope(::onOptionsMenuUpdated) viewModel.flowOfStudiedTodayStats.launchCollectionInLifecycleScope(::onStudiedTodayChanged) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/utils/MessageQueueObserver.kt b/AnkiDroid/src/main/java/com/ichi2/anki/utils/MessageQueueObserver.kt index 67d9faa5a665..17bd737a7121 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/utils/MessageQueueObserver.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/utils/MessageQueueObserver.kt @@ -3,9 +3,11 @@ package com.ichi2.anki.utils +import androidx.activity.ComponentDialog import androidx.appcompat.app.AlertDialog import androidx.fragment.app.Fragment import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle @@ -49,3 +51,17 @@ suspend fun AlertDialog.Builder.showAndAwaitDismissal(block: AlertDialog.Builder continuation.invokeOnCancellation { dialog.dismiss() } } } + +suspend fun ComponentDialog.awaitDismissal() { + suspendCancellableCoroutine { continuation -> + lifecycle.addObserver( + LifecycleEventObserver { _, event -> + if (event == Lifecycle.Event.ON_DESTROY) continuation.resume(Unit) + }, + ) + continuation.invokeOnCancellation { + setOnDismissListener(null) + dismiss() + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerAdditionalReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerAdditionalReviewTest.kt new file mode 100644 index 000000000000..bec62343b4ae --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerAdditionalReviewTest.kt @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +import android.view.ContextThemeWrapper +import androidx.appcompat.app.AlertDialog +import androidx.core.content.edit +import androidx.test.core.app.ActivityScenario +import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.collection.OpChangesWithCount +import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.utils.awaitDismissal +import com.ichi2.testutils.BackupManagerTestUtilities +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import net.ankiweb.rsdroid.Backend +import net.ankiweb.rsdroid.BackendFactory +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.shadows.ShadowDialog +import kotlin.test.assertFalse +import com.ichi2.anki.common.android.R as CommonR + +@RunWith(AndroidJUnit4::class) +class DeckPickerAdditionalReviewTest : RobolectricTest() { + @After + fun resetBackendFactory() { + BackendFactory.setOverride(null) + } + + @Test + fun `cancelling the deck picker dialog wait releases the window`() = + runTest { + val dialog = AlertDialog.Builder(ContextThemeWrapper(targetContext, CommonR.style.Theme_Light)).show() + val waiting = launch(start = CoroutineStart.UNDISPATCHED) { dialog.awaitDismissal() } + try { + waiting.cancel() + waiting.join() + assertFalse(dialog.isShowing, "Destroying the owner must dismiss its queued error dialog") + } finally { + dialog.dismiss() + } + } + + @Test + fun `recreating deck picker dismisses the previous error dialog`() { + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = + throw IllegalStateException("Review probe: deletion failed") + } + } + val deckId = addDeck("Deck") + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + lateinit var originalDialog: AlertDialog + scenario.onActivity { picker -> + picker.viewModel.awaitStartup() + val job = picker.deleteDeck(deckId) + advanceRobolectricLooperUntil { + job.isCompleted && (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true + } + originalDialog = ShadowDialog.getLatestDialog() as AlertDialog + } + scenario.recreate() + scenario.onActivity { + try { + assertFalse(originalDialog.isShowing, "The destroyed activity's error window must be released") + } finally { + originalDialog.dismiss() + } + } + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index adf9f6d67cee..c8ab123c6d7f 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -3,8 +3,10 @@ package com.ichi2.anki import androidx.appcompat.app.AlertDialog +import androidx.core.content.edit import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleRegistry +import androidx.test.core.app.ActivityScenario import androidx.test.ext.junit.runners.AndroidJUnit4 import anki.backend.BackendError import anki.backend.backendError @@ -13,6 +15,10 @@ import anki.collection.OpChangesWithCount import anki.decks.DeckTreeNode import anki.decks.SetDeckCollapsedRequest import com.ichi2.anki.backend.DatabaseCorruption +import com.ichi2.anki.common.crashreporting.CrashReportService +import com.ichi2.anki.common.crashreporting.CrashReporter +import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.deckpicker.DeckPickerViewModel import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.DeckPickerContextMenu.DeckPickerContextMenuOption import com.ichi2.anki.dialogs.DeckPickerContextMenuResult @@ -21,6 +27,7 @@ import com.ichi2.anki.dialogs.startDeletingEmptyCards import com.ichi2.anki.dialogs.utils.input import com.ichi2.anki.dialogs.utils.performPositiveClick import com.ichi2.anki.libanki.DeckId +import com.ichi2.testutils.BackupManagerTestUtilities import kotlinx.coroutines.runBlocking import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException @@ -30,11 +37,17 @@ import org.junit.After import org.junit.Test import org.junit.jupiter.api.Assertions.assertAll import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config import org.robolectric.shadows.ShadowDialog import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNotSame +import kotlin.test.assertSame import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @@ -76,6 +89,84 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `unacknowledged deck deletion error survives activity recreation`() { + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = throw failure + } + } + val deckId = addDeck("Deck") + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + lateinit var retainedViewModel: DeckPickerViewModel + lateinit var originalDialog: AlertDialog + scenario.onActivity { deckPicker -> + deckPicker.viewModel.awaitStartup() + retainedViewModel = deckPicker.viewModel + val job = deckPicker.deleteDeck(deckId) + advanceRobolectricLooperUntil { job.isCompleted } + assertErrorShown() + originalDialog = ShadowDialog.getLatestDialog() as AlertDialog + } + + // Recreate without dismissing the error, as for a font-scale configuration change. + scenario.recreate() + scenario.onActivity { deckPicker -> + assertSame(retainedViewModel, deckPicker.viewModel) + deckPicker.viewModel.awaitStartup() + advanceRobolectricLooper() + val restoredDialog = ShadowDialog.getLatestDialog() as? AlertDialog + assertNotNull(restoredDialog, "The unacknowledged error must be shown again") + assertNotSame(originalDialog, restoredDialog, "The old activity's dialog must not count as restored") + assertTrue(restoredDialog.isShowing) + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } + } + } + + @Test + fun `error shown again after recreation is reported once`() { + val crashReporter = mock() + val originalReporter = CrashReportService.getReporter() + CrashReportService.setReporter(crashReporter) + try { + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = throw IllegalStateException("deletion failed") + } + } + val deckId = addDeck("Deck") + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + scenario.onActivity { deckPicker -> + deckPicker.viewModel.awaitStartup() + deckPicker.deleteDeck(deckId) + advanceRobolectricLooperUntil { (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true } + } + scenario.recreate() + scenario.onActivity { deckPicker -> + deckPicker.viewModel.awaitStartup() + advanceRobolectricLooperUntil { (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true } + ShadowDialog.getLatestDialog().dismiss() + advanceRobolectricLooper() + } + } + + verify(crashReporter).sendExceptionReport(any(), anyOrNull(), anyOrNull(), any()) + } finally { + CrashReportService.setReporter(originalReporter) + } + } + @Test fun `database corruption during deck deletion opens recovery dialog`() { useBackend { diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/utils/MessageQueueObserverTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/utils/MessageQueueObserverTest.kt index 21fb59ec5105..a06b9cfe56e9 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/utils/MessageQueueObserverTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/utils/MessageQueueObserverTest.kt @@ -17,6 +17,7 @@ import com.ichi2.testutils.EmptyAnkiActivity import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runCurrent import org.junit.Test import org.junit.runner.RunWith import org.robolectric.Robolectric @@ -71,6 +72,19 @@ class MessageQueueObserverTest : RobolectricTest() { assertTrue(shadowOf(dialog).hasBeenDismissed()) } + @Test + fun `awaiting a dialog returns once it is dismissed`() = + runTest { + val dialog = AlertDialog.Builder(ContextThemeWrapper(targetContext, CommonR.style.Theme_Light)).show() + val waiting = launch(start = CoroutineStart.UNDISPATCHED) { dialog.awaitDismissal() } + assertTrue(waiting.isActive) + + dialog.dismiss() + runCurrent() + + assertTrue(waiting.isCompleted) + } + private fun observeInFragment(observe: Fragment.() -> Unit): ActivityController = Robolectric.buildActivity(EmptyAnkiActivity::class.java).setup().also { host -> val fragment = ObservingFragment().apply { this.observe = observe } From 52d688399fcd4d4704af3bee0832ceb7a11439e6 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Sat, 24 Oct 2026 05:09:52 +0530 Subject: [PATCH 06/30] fix(deck-picker): skip the follow-up when selecting a deck fails Opening a deck's menu and creating a deck both select the deck first. When that failed, the menu still opened, covering the error (or the database recovery dialog) and acting on the previously selected deck, and a new deck still refreshed the list, the study options panel and the menu. These now only happen once the deck is selected. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 15 +-- .../anki/deckpicker/DeckPickerViewModel.kt | 27 +++-- .../anki/DeckPickerErrorQueueReviewTest.kt | 106 ++++++++++++++++++ .../com/ichi2/anki/DeckPickerErrorTest.kt | 33 ++++++ 4 files changed, 165 insertions(+), 16 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorQueueReviewTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 46a5124f98df..6963df7d7884 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -947,6 +947,12 @@ open class DeckPicker : fragment?.refreshInterface() } + fun onCreatedDeckSelected(param: Unit) { + updateDeckList() + tryShowStudyOptionsPanel() + invalidateOptionsMenu() + } + fun onStartupResponse(response: StartupResponse) { Timber.d("onStartupResponse: %s", response) when (response) { @@ -1022,6 +1028,7 @@ open class DeckPicker : viewModel.flowOfFocusedDeck.launchCollectionInLifecycleScope(::onFocusedDeckChanged) viewModel.flowOfResizingDividerVisible.launchCollectionInLifecycleScope(::onResizingDividerVisibilityChanged) viewModel.flowOfDecksReloaded.launchCollectionInLifecycleScope(::onDecksReloaded) + viewModel.flowOfCreatedDeckSelected.launchCollectionInLifecycleScope(::onCreatedDeckSelected) viewModel.flowOfStartupResponse.filterNotNull().launchCollectionInLifecycleScope(::onStartupResponse) viewModel.flowOfShowContextMenu.launchCollectionInLifecycleScope(::showDeckPickerContextMenu) viewModel.flowOfShowRightClickContextMenu.launchCollectionInLifecycleScope(::showDeckPickerRightClickContextMenu) @@ -2400,13 +2407,7 @@ open class DeckPicker : * [StudyOptionsFragment] panel follow the new deck instead of the previously current deck, which * is often the hidden Default deck. */ - private fun onDeckCreated(deckId: DeckId) = - launchCatchingTask { - viewModel.selectDeck(deckId).join() - updateDeckList() - tryShowStudyOptionsPanel() - invalidateOptionsMenu() - } + private fun onDeckCreated(deckId: DeckId) = viewModel.selectCreatedDeck(deckId) /** * Deletes the provided deck, child decks, and all cards inside. diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index de7d48a75277..55df1cf6c4af 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -354,14 +354,23 @@ class DeckPickerViewModel : /** * Marks [deckId] as the currently selected deck and updates the selection in the deck list. */ - fun selectDeck(deckId: DeckId) = + fun selectDeck(deckId: DeckId) = launchCatchingTask { markDeckSelected(deckId) } + + val flowOfCreatedDeckSelected = MutableSharedFlow(extraBufferCapacity = 1) + + fun selectCreatedDeck(deckId: DeckId) = launchCatchingTask { - // TODO: should we always reset the Card Browser default deck here? - withCol { decks.select(deckId) } - focusedDeck = deckId - flowOfRefreshDeckList.emit(Unit) + markDeckSelected(deckId) + flowOfCreatedDeckSelected.emit(Unit) } + private suspend fun markDeckSelected(deckId: DeckId) { + // TODO: should we always reset the Card Browser default deck here? + withCol { decks.select(deckId) } + focusedDeck = deckId + flowOfRefreshDeckList.emit(Unit) + } + fun browseCards(deckId: DeckId) = launchCatchingIO { withCol { decks.select(deckId) } @@ -389,8 +398,8 @@ class DeckPickerViewModel : val flowOfShowRightClickContextMenu = MutableSharedFlow(extraBufferCapacity = 1) fun requestContextMenu(deckId: DeckId) = - viewModelScope.launch { - selectDeck(deckId).join() + launchCatchingTask { + markDeckSelected(deckId) flowOfShowContextMenu.emit(deckId) } @@ -398,8 +407,8 @@ class DeckPickerViewModel : deckId: DeckId, x: Float, y: Float, - ) = viewModelScope.launch { - selectDeck(deckId).join() + ) = launchCatchingTask { + markDeckSelected(deckId) flowOfShowRightClickContextMenu.emit(RightClickMenuRequest(deckId, x, y)) } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorQueueReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorQueueReviewTest.kt new file mode 100644 index 000000000000..f4048f19d036 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorQueueReviewTest.kt @@ -0,0 +1,106 @@ +// 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.backend.backendError +import anki.collection.OpChanges +import com.ichi2.anki.backend.DatabaseCorruption +import com.ichi2.anki.dialogs.DatabaseErrorDialog +import com.ichi2.anki.dialogs.DeckPickerContextMenu +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 kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class DeckPickerErrorQueueReviewTest : RobolectricTest() { + private val failure = + BackendIoException( + backendError { + kind = BackendError.Kind.IO_ERROR + message = "Review probe: backend write failed" + }, + ) + + @After + fun resetBackendFactory() { + BackendFactory.setOverride(null) + } + + @Test + fun `failed context selection does not open a context menu`() { + var failSelection = false + useBackend { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges = if (failSelection) throw failure else super.setCurrentDeck(did) + } + } + val deckId = addDeck("Target") + withDeckPicker(deckCount = 0) { picker -> + throwOnShowError = false + val previousSelection = col.decks.selected() + failSelection = true + val request = picker.viewModel.requestContextMenu(deckId) + advanceRobolectricLooperUntil { request.isCompleted } + advanceRobolectricLooper() + assertEquals(previousSelection, col.decks.selected()) + assertEquals(1, picker.viewModel.pendingErrors.value.size) + assertFalse( + picker.supportFragmentManager.fragments.any { it is DeckPickerContextMenu }, + "The context menu must not cover the selection error and use the previous deck's state", + ) + } + } + + @Test + fun `failed context selection preserves corruption recovery`() { + var failSelection = false + useBackend { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges = + if (failSelection) { + throw BackendDbCorruptException( + backendError { + message = "database disk image is malformed" + }, + ) + } else { + super.setCurrentDeck(did) + } + } + } + val deckId = addDeck("Target") + val previouslyDetected = DatabaseCorruption.isDetected + DatabaseCorruption.isDetected = false + try { + withDeckPicker(deckCount = 0) { picker -> + throwOnShowError = false + failSelection = true + val request = picker.viewModel.requestContextMenu(deckId) + advanceRobolectricLooperUntil { request.isCompleted } + advanceRobolectricLooper() + assertTrue(DatabaseCorruption.isDetected) + assertTrue( + picker.supportFragmentManager.fragments.any { it is DatabaseErrorDialog }, + "A failed selection must not replace the corruption recovery dialog with a context menu", + ) + } + } finally { + DatabaseCorruption.isDetected = previouslyDetected + } + } + + private fun useBackend(backend: () -> Backend) { + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { backend() } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index c8ab123c6d7f..b0bb66f7bc3c 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -14,6 +14,7 @@ import anki.collection.OpChanges import anki.collection.OpChangesWithCount import anki.decks.DeckTreeNode import anki.decks.SetDeckCollapsedRequest +import app.cash.turbine.test import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.crashreporting.CrashReporter @@ -28,6 +29,8 @@ import com.ichi2.anki.dialogs.utils.input import com.ichi2.anki.dialogs.utils.performPositiveClick import com.ichi2.anki.libanki.DeckId import com.ichi2.testutils.BackupManagerTestUtilities +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.runBlocking import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException @@ -294,6 +297,16 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `failed selection does not open the right click context menu`() = + assertFailedSelectionSkips({ flowOfShowRightClickContextMenu }) { deckId -> + requestRightClickContextMenu(deckId, x = 10f, y = 20f) + } + + @Test + fun `failed selection of a created deck does not update the screen`() = + assertFailedSelectionSkips({ flowOfCreatedDeckSelected }) { deckId -> selectCreatedDeck(deckId) } + @Test fun `failed deck list update shows an error`() { var failDeckTree = false @@ -366,6 +379,26 @@ class DeckPickerErrorTest : RobolectricTest() { BackendFactory.setOverride { backend() } } + private fun assertFailedSelectionSkips( + followUp: DeckPickerViewModel.() -> Flow, + request: DeckPickerViewModel.(DeckId) -> Job, + ) = runTest { + useBackend { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges = throw failure + } + } + val deckId = addDeck("Target") + val viewModel = DeckPickerViewModel() + + viewModel.followUp().test { + viewModel.request(deckId).join() + + expectNoEvents() + } + assertEquals(1, viewModel.pendingErrors.value.size) + } + private fun DeckPicker.selectContextMenuOption( option: DeckPickerContextMenuOption, deckId: DeckId, From b6bd7ac86ddc57c1d34a9ec96f9bc2ea19adb29d Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Sat, 24 Oct 2026 05:10:09 +0530 Subject: [PATCH 07/30] fix(deck-picker): keep the deck list when the current deck can't be read A refresh that couldn't read the current deck crashed the app while redrawing the deck list. The list now keeps showing the last decks it loaded, and the error is shown like the others, so database corruption, for example while filtering, opens the recovery dialog. A refresh reads the current deck first, so it still reports one error. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 25 ++++++++--- .../anki/DeckPickerAdditionalReviewTest.kt | 44 +++++++++++++++++++ .../anki/DeckPickerErrorQueueReviewTest.kt | 31 +++++++++++++ .../com/ichi2/anki/DeckPickerErrorTest.kt | 22 ++++++++++ 4 files changed, 115 insertions(+), 7 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 55df1cf6c4af..5e4fff9b9ded 100644 --- 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 kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update @@ -126,15 +127,23 @@ class DeckPickerViewModel : // TODO: use flowOfFocusedDeck once it's set on all instances // This flow can outlive the visible screen. Do not reopen the collection just to redraw it. val currentDeckId = - withOpenColOrNull { decks.current().id } - ?: return@combine FlattenedDeckList.empty + try { + withOpenColOrNull { decks.current().id } + ?: return@combine FlattenedDeckList.empty + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + reportError(e) + return@combine null + } Timber.i("currentDeckId: %d", currentDeckId) FlattenedDeckList( data = tree.filterAndFlattenDisplay(DeckFilters.create(query), currentDeckId), hasSubDecks = tree.children.any { it.children.any() }, ) - }.stateIn(viewModelScope, SharingStarted.Eagerly, initialValue = FlattenedDeckList.empty) + }.filterNotNull() + .stateIn(viewModelScope, SharingStarted.Eagerly, initialValue = FlattenedDeckList.empty) /** * @see deleteDeck @@ -258,10 +267,12 @@ class DeckPickerViewModel : } catch (e: CancellationException) { throw e } catch (e: Exception) { - withContext(Dispatchers.Main.immediate) { errorQueue.enqueue(e) } + reportError(e) } } + private suspend fun reportError(e: Exception) = withContext(Dispatchers.Main.immediate) { errorQueue.enqueue(e) } + /** * Deletes the provided deck, child decks. and all cards inside. * @@ -489,9 +500,9 @@ class DeckPickerViewModel : val loadDeckCounts = launchCatchingTask(Dispatchers.IO) { Timber.d("Refreshing deck list") - val (deckDueTree, collectionHasNoCards) = + val (deckDueTree, collectionHasNoCards, currentDeckId) = withCol { - Pair(sched.deckDueTree(), isEmpty) + Triple(sched.deckDueTree(), isEmpty, decks.current().id) } dueTree = deckDueTree @@ -516,7 +527,7 @@ class DeckPickerViewModel : // TODO: This is in the wrong place // current deck may have changed - focusedDeck = withCol { decks.current().id } + focusedDeck = currentDeckId refreshUndoMenuState() // Reopening can leave the counts and selection unchanged after a skipped display refresh. diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerAdditionalReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerAdditionalReviewTest.kt index bec62343b4ae..a879c5d284f6 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerAdditionalReviewTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerAdditionalReviewTest.kt @@ -7,14 +7,19 @@ import androidx.appcompat.app.AlertDialog import androidx.core.content.edit import androidx.test.core.app.ActivityScenario import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.backend.backendError import anki.collection.OpChangesWithCount +import com.google.protobuf.ByteString +import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.utils.awaitDismissal import com.ichi2.testutils.BackupManagerTestUtilities import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.launch 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 @@ -30,6 +35,45 @@ class DeckPickerAdditionalReviewTest : RobolectricTest() { BackendFactory.setOverride(null) } + @Test + fun `corruption while applying a deck filter opens recovery`() { + var failRead = false + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun getDeckLegacy(did: Long): ByteString = + if (failRead) { + throw BackendDbCorruptException(backendError { message = "database disk image is malformed" }) + } else { + super.getDeckLegacy(did) + } + } + } + val deckId = addDeck("Existing") + val previouslyDetected = DatabaseCorruption.isDetected + DatabaseCorruption.isDetected = false + try { + withDeckPicker(deckCount = 0) { picker -> + throwOnShowError = false + advanceRobolectricLooperUntil { + picker.viewModel.loadDeckCounts?.isCompleted == true && + picker.viewModel.flowOfDeckList.value.data + .any { it.did == deckId } + } + failRead = true + picker.viewModel.updateDeckFilter("no matching deck") + advanceRobolectricLooperUntil( + lazyMessage = { "Corruption while filtering must open database recovery" }, + ) { + DatabaseCorruption.isDetected && + picker.supportFragmentManager.fragments.any { it is DatabaseErrorDialog } + } + } + } finally { + DatabaseCorruption.isDetected = previouslyDetected + } + } + @Test fun `cancelling the deck picker dialog wait releases the window`() = runTest { diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorQueueReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorQueueReviewTest.kt index f4048f19d036..680a951e60ce 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorQueueReviewTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorQueueReviewTest.kt @@ -6,6 +6,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import anki.backend.BackendError import anki.backend.backendError import anki.collection.OpChanges +import com.google.protobuf.ByteString import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.DeckPickerContextMenu @@ -61,6 +62,36 @@ class DeckPickerErrorQueueReviewTest : RobolectricTest() { } } + @Test + fun `deck refresh handles a failure reading the current deck`() { + var failRead = false + useBackend { + object : Backend() { + override fun getDeckLegacy(did: Long): ByteString = + if (failRead) { + throw BackendIoException( + backendError { + kind = BackendError.Kind.IO_ERROR + message = "Review probe: current deck read failed" + }, + ) + } else { + super.getDeckLegacy(did) + } + } + } + addDeck("Existing") + withDeckPicker(deckCount = 0) { picker -> + throwOnShowError = false + addDeck("New deck to refresh") + failRead = true + val refresh = picker.viewModel.reloadDeckCounts() + advanceRobolectricLooperUntil { refresh.isCompleted } + advanceRobolectricLooper() + assertEquals(1, picker.viewModel.pendingErrors.value.size) + } + } + @Test fun `failed context selection preserves corruption recovery`() { var failSelection = false diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index b0bb66f7bc3c..d8ce4a103aca 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -15,6 +15,7 @@ import anki.collection.OpChangesWithCount import anki.decks.DeckTreeNode import anki.decks.SetDeckCollapsedRequest import app.cash.turbine.test +import com.google.protobuf.ByteString import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.crashreporting.CrashReporter @@ -325,6 +326,27 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `deck list keeps its decks when filtering cannot read the current deck`() { + var failRead = false + useBackend { + object : Backend() { + override fun getDeckLegacy(did: Long): ByteString = if (failRead) throw failure else super.getDeckLegacy(did) + } + } + val deckId = addDeck("Existing") + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + failRead = true + deckPicker.viewModel.updateDeckFilter("no matching deck") + + assertErrorShown() + val visibleDecks = deckPicker.viewModel.flowOfDeckList.value.data + assertTrue(visibleDecks.any { it.did == deckId }) + } + } + @Test fun `failed deck expand toggle shows an error and keeps the saved state`() { useBackend { From 8061807faf829c2f529807f5d89a87b74cdbd5b2 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Mon, 26 Oct 2026 01:09:31 +0530 Subject: [PATCH 08/30] fix(deck-picker): show errors from the other deck actions through the queue Errors from actions such as renaming, opening deck options or unburying were plain messages: they were lost while the screen was stopped and corruption didn't open the recovery dialog. They now use the same queue as the other Deck Picker errors. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 23 +++---- .../com/ichi2/anki/DeckPickerErrorTest.kt | 61 +++++++++++++++++++ 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 5e4fff9b9ded..921ba2ce8806 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -26,6 +26,7 @@ import com.ichi2.anki.common.destinations.DeckOptionsDestination import com.ichi2.anki.common.destinations.NoteEditorDestination import com.ichi2.anki.common.utils.ext.schedulerVersion import com.ichi2.anki.configureRenderingMode +import com.ichi2.anki.ioDispatcher import com.ichi2.anki.launchCatchingIO import com.ichi2.anki.libanki.CardId import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID @@ -229,7 +230,7 @@ class DeckPickerViewModel : /** Prepares a confirmation dialog for deleting the selected deck. */ fun requestDeleteDeckConfirmation() = - launchCatchingIO { + launchCatchingTask(ioDispatcher) { val request = withCol { val deck = decks.current() @@ -244,13 +245,13 @@ class DeckPickerViewModel : } fun requestRenameSelectedDeck() = - launchCatchingIO { + launchCatchingTask(ioDispatcher) { val deckId = withCol { decks.selected() } flowOfRenameDeck.emit(deckId) } fun requestStudySelectedDeck() = - launchCatchingIO { + launchCatchingTask(ioDispatcher) { val deckId = withCol { decks.selected() } flowOfStudyDeck.emit(deckId) } @@ -383,7 +384,7 @@ class DeckPickerViewModel : } fun browseCards(deckId: DeckId) = - launchCatchingIO { + launchCatchingTask(ioDispatcher) { withCol { decks.select(deckId) } flowOfNavigate.emit(BrowserDestination.ToDeck(deckId)) } @@ -391,7 +392,7 @@ class DeckPickerViewModel : fun addNote( deckId: DeckId?, setAsCurrent: Boolean, - ) = launchCatchingIO { + ) = launchCatchingTask(ioDispatcher) { if (deckId != null && setAsCurrent) { withCol { decks.select(deckId) } } @@ -426,7 +427,7 @@ class DeckPickerViewModel : /** * Opens the Manage Note Types screen. */ - fun openManageNoteTypes() = launchCatchingIO { flowOfDestination.emit(ManageNoteTypesDestination()) } + fun openManageNoteTypes() = launchCatchingTask(ioDispatcher) { flowOfDestination.emit(ManageNoteTypesDestination()) } /** * Opens study options for the provided deck @@ -437,14 +438,14 @@ class DeckPickerViewModel : fun openDeckOptions( deckId: DeckId, isFiltered: Boolean? = null, - ) = launchCatchingIO { + ) = launchCatchingTask(ioDispatcher) { // open cram options if filtered deck, otherwise open regular options val filtered = isFiltered ?: withCol { decks.isFiltered(deckId) } flowOfNavigate.emit(DeckOptionsDestination(deckId = deckId, isFiltered = filtered)) } fun unburyDeck(deckId: DeckId) = - launchCatchingIO { + launchCatchingTask(ioDispatcher) { undoableOp { sched.unburyDeck(deckId) } } @@ -569,7 +570,7 @@ class DeckPickerViewModel : * Requests export for the specified deck */ fun exportDeck(deckId: DeckId) = - launchCatchingIO { + launchCatchingTask(ioDispatcher) { flowOfExportDeck.emit(deckId) } @@ -596,7 +597,7 @@ class DeckPickerViewModel : * Prepares data for creating a deck shortcut */ fun createIcon(deckId: DeckId) = - launchCatchingIO { + launchCatchingTask(ioDispatcher) { val (shortLabel, longLabel) = withCol { val fullName = decks.name(deckId) @@ -616,7 +617,7 @@ class DeckPickerViewModel : /** Disables the shortcut of the deck and the children belonging to it.*/ fun disableDeckAndChildrenShortcuts(deckId: DeckId) = - launchCatchingIO { + launchCatchingTask(ioDispatcher) { val deckTreeDids = dueTree?.find(deckId)?.map { it.did.toString() } ?: emptyList() flowOfDisableShortcuts.emit(deckTreeDids) } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index d8ce4a103aca..a7bd3928667f 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -14,6 +14,7 @@ import anki.collection.OpChanges import anki.collection.OpChangesWithCount import anki.decks.DeckTreeNode import anki.decks.SetDeckCollapsedRequest +import anki.scheduler.UnburyDeckRequest import app.cash.turbine.test import com.google.protobuf.ByteString import com.ichi2.anki.backend.DatabaseCorruption @@ -347,6 +348,66 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `corruption while opening deck options opens recovery dialog`() { + var failedDeck: DeckId? = null + useBackend { + object : Backend() { + override fun getDeckLegacy(did: Long): ByteString = + if (did == failedDeck) { + throw BackendDbCorruptException(backendError { message = "database disk image is malformed" }) + } else { + super.getDeckLegacy(did) + } + } + } + val deckId = addDeck("Deck") + val previouslyDetected = DatabaseCorruption.isDetected + DatabaseCorruption.isDetected = false + try { + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + failedDeck = deckId + deckPicker.viewModel.openDeckOptions(deckId) + + advanceRobolectricLooperUntil { + DatabaseCorruption.isDetected && + deckPicker.supportFragmentManager.fragments.any { it is DatabaseErrorDialog } + } + } + } finally { + DatabaseCorruption.isDetected = previouslyDetected + } + } + + @Test + fun `failed unbury while stopped shows an error on resume`() { + useBackend { + object : Backend() { + override fun unburyDeck( + deckId: Long, + mode: UnburyDeckRequest.Mode, + ): OpChanges = throw failure + } + } + val deckId = addDeck("Deck") + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + val lifecycle = deckPicker.lifecycle as LifecycleRegistry + lifecycle.currentState = Lifecycle.State.CREATED + advanceRobolectricLooperUntil { deckPicker.viewModel.onError.subscriptionCount.value == 0 } + try { + val job = deckPicker.viewModel.unburyDeck(deckId) + advanceRobolectricLooperUntil { job.isCompleted } + } finally { + lifecycle.currentState = Lifecycle.State.RESUMED + } + + assertErrorShown() + } + } + @Test fun `failed deck expand toggle shows an error and keeps the saved state`() { useBackend { From 3ebfe0f5ce0592644c55bc6b94c67036947c8f52 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Mon, 26 Oct 2026 03:02:33 +0530 Subject: [PATCH 09/30] test(deck-picker): wait for the deck rows before clicking one The deletion test clicked the "Other" deck as soon as the deck tree had loaded, but the rows are drawn a moment later, so on a slow runner the click could come before the row existed. It now waits until both decks are shown. Assisted-by: Claude Opus 5.5 --- AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt index 1d4abbb464e7..a20ed07e7f5c 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt @@ -354,7 +354,7 @@ class DeckPickerTest : RobolectricTest() { val selected = col.decks.selected() lateinit var deckPicker: DeckPicker withDeckPicker(deckCount = 0) { deckPicker = it } - advanceRobolectricLooperUntil { deckPicker.dueTree?.find(other) != null } + deckPicker.awaitVisibleDecks(listOf(deleted, other)) val deletion = withQueuedCollectionAccess { From 53417b291420a57ae5f075d28c938870c178aab7 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 27 Oct 2026 01:10:42 +0530 Subject: [PATCH 10/30] fix(deck-picker): show errors once database recovery closes The next queued error, or a message such as a failed background backup, opened on top of the database recovery dialog and hid it. They now wait until no recovery dialog is open and the screen is started, and check again right before they show, in case another recovery dialog opened while they waited. They also let a closing dialog be fully removed first, so a queued corruption error opens recovery instead of becoming a notification. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 49 ++++++++- .../ichi2/anki/DeckPickerErrorRecoveryTest.kt | 104 ++++++++++++++++++ .../com/ichi2/anki/DeckPickerErrorTest.kt | 96 ++++++++++++++++ .../ichi2/anki/DeckPickerFurtherReviewTest.kt | 103 +++++++++++++++++ .../DeckPickerRecoveryHandoffProbeTest.kt | 99 +++++++++++++++++ .../DeckPickerRecoveryTransactionProbeTest.kt | 84 ++++++++++++++ 6 files changed, 529 insertions(+), 6 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorRecoveryTest.kt create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryHandoffProbeTest.kt create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryTransactionProbeTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 6963df7d7884..b654345f8943 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -68,6 +68,7 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle +import androidx.lifecycle.withStarted import androidx.recyclerview.widget.LinearLayoutManager import androidx.swiperefreshlayout.widget.SwipeRefreshLayout import androidx.work.NetworkType @@ -138,6 +139,7 @@ import com.ichi2.anki.deckpicker.SyncIconState import com.ichi2.anki.dialogs.AsyncDialogFragment import com.ichi2.anki.dialogs.BackupPromptDialog import com.ichi2.anki.dialogs.CreateDeckDialog +import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.DatabaseErrorDialog.CustomExceptionData import com.ichi2.anki.dialogs.DatabaseErrorDialog.DatabaseErrorDialogType import com.ichi2.anki.dialogs.DeckPickerAnalyticsOptInDialog @@ -233,8 +235,10 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import kotlinx.coroutines.yield import net.ankiweb.rsdroid.Translations import net.ankiweb.rsdroid.exceptions.BackendInvalidInputException.BackendCollectionNotOpenException import timber.log.Timber @@ -494,6 +498,31 @@ open class DeckPicker : ) } + private fun findDatabaseRecovery() = + supportFragmentManager.fragments.firstOrNull { + it is DatabaseErrorDialog && it.lifecycle.currentState != Lifecycle.State.DESTROYED + } + + private suspend fun awaitDatabaseRecovery() { + while (true) { + val recovery = findDatabaseRecovery() ?: return + recovery.lifecycle.currentStateFlow.first { it == Lifecycle.State.DESTROYED } + yield() + } + } + + private suspend fun withStartedAfterRecovery(block: () -> Unit) { + do { + awaitDatabaseRecovery() + val recoveryOpened = + withStarted { + val opened = findDatabaseRecovery() != null + if (!opened) block() + opened + } + } while (recoveryOpened) + } + // ---------------------------------------------------------------------------- // ANDROID ACTIVITY METHODS // ---------------------------------------------------------------------------- @@ -994,11 +1023,15 @@ open class DeckPicker : } fun onError(errorMessage: String) { - AlertDialog - .Builder(this) - .setTitle(CommonString.vague_error) - .setMessage(errorMessage) - .show() + launchCatchingTask { + withStartedAfterRecovery { + AlertDialog + .Builder(this@DeckPicker) + .setTitle(CommonString.vague_error) + .setMessage(errorMessage) + .show() + } + } } viewModel.deckDeletedNotification.launchCollectionInLifecycleScope(::onDeckDeleted) @@ -1017,7 +1050,11 @@ open class DeckPicker : viewModel.flowOfCreateShortcut.launchCollectionInLifecycleScope(::createIcon) viewModel.flowOfDisableShortcuts.launchCollectionInLifecycleScope(::disableDeckAndChildrenShortcuts) viewModel.onError.launchCollectionInLifecycleScope(::onError) - observeMessages(viewModel.pendingErrors, viewModel::errorShown) { showException(it)?.awaitDismissal() } + observeMessages(viewModel.pendingErrors, viewModel::errorShown) { error -> + var dialog: AlertDialog? = null + withStartedAfterRecovery { dialog = showException(error) } + dialog?.awaitDismissal() + } viewModel.flowOfPromptUserToUpdateScheduler.launchCollectionInLifecycleScope(::onPromptUserToUpdateScheduler) viewModel.flowOfOptionsMenuState.filterNotNull().launchCollectionInLifecycleScope(::onOptionsMenuUpdated) viewModel.flowOfStudiedTodayStats.launchCollectionInLifecycleScope(::onStudiedTodayChanged) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorRecoveryTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorRecoveryTest.kt new file mode 100644 index 000000000000..675a962b07dd --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorRecoveryTest.kt @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +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.assertEquals +import kotlin.test.assertSame + +@RunWith(AndroidJUnit4::class) +class DeckPickerErrorRecoveryTest : RobolectricTest() { + private val failure = BackendIoException(backendError { message = "Backend operation failed" }) + + @After + fun resetBackendFactory() { + BackendFactory.setOverride(null) + } + + @Test + fun `queued error waits until database recovery is dismissed`() { + var corrupt = true + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount { + if (corrupt) { + throw BackendDbCorruptException(backendError { message = "database disk image is malformed" }) + } + throw failure + } + } + } + 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 lifecycle = picker.lifecycle as LifecycleRegistry + lifecycle.currentState = Lifecycle.State.CREATED + try { + // ViewModel operations can finish while the screen is stopped. + val first = picker.deleteDeck(target) + advanceRobolectricLooperUntil { first.isCompleted } + corrupt = false + val second = picker.deleteDeck(target) + advanceRobolectricLooperUntil { second.isCompleted } + assertEquals(2, picker.viewModel.pendingErrors.value.size) + } finally { + lifecycle.currentState = Lifecycle.State.RESUMED + } + advanceRobolectricLooperUntil { + picker.supportFragmentManager.fragments + .filterIsInstance() + .any { it.dialog?.isShowing == true } + } + advanceRobolectricLooper() + val recovery = + picker.supportFragmentManager.fragments + .filterIsInstance() + .single() + val recoveryDialog = recovery.requireDialog() + assertSame( + recoveryDialog, + ShadowDialog.getLatestDialog(), + "The next queued error must not cover database recovery before it is dismissed", + ) + + recovery.dismiss() + advanceRobolectricLooperUntil { + val latest = ShadowDialog.getLatestDialog() + latest !== recoveryDialog && latest?.isShowing == true + } + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + ShadowDialog.getLatestDialog().dismiss() + advanceRobolectricLooperUntil { + picker.viewModel.pendingErrors.value + .isEmpty() + } + } + } finally { + DatabaseCorruption.isDetected = previouslyDetected + } + } + + private fun useBackend(backend: () -> Backend) { + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { backend() } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index a7bd3928667f..00e2a019e508 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -210,6 +210,74 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `error message waits until database recovery is closed`() = + withCorruptDeletionRecovery { deckPicker, recovery -> + val recoveryDialog = recovery.requireDialog() + deckPicker.viewModel.onError.tryEmit("Backup failed") + advanceRobolectricLooper() + assertSame(recoveryDialog, ShadowDialog.getLatestDialog()) + + recovery.dismiss() + advanceRobolectricLooperUntil { + val latest = ShadowDialog.getLatestDialog() + latest !== recoveryDialog && latest?.isShowing == true + } + assertEquals("Backup failed", getAlertDialogText(checkDismissed = true)) + } + + @Test + fun `error message waiting for recovery is not shown while stopped`() = + withCorruptDeletionRecovery { deckPicker, recovery -> + val recoveryDialog = recovery.requireDialog() + deckPicker.viewModel.onError.tryEmit("Backup failed") + advanceRobolectricLooper() + + val lifecycle = deckPicker.lifecycle as LifecycleRegistry + lifecycle.currentState = Lifecycle.State.CREATED + try { + recovery.dismiss() + advanceRobolectricLooper() + assertSame(recoveryDialog, ShadowDialog.getLatestDialog()) + } finally { + lifecycle.currentState = Lifecycle.State.RESUMED + } + advanceRobolectricLooperUntil { + val latest = ShadowDialog.getLatestDialog() + latest !== recoveryDialog && latest?.isShowing == true + } + assertEquals("Backup failed", getAlertDialogText(checkDismissed = true)) + } + + @Test + fun `error message waits for a recovery dialog opened while stopped`() = + withCorruptDeletionRecovery { deckPicker, recovery -> + deckPicker.viewModel.onError.tryEmit("Backup failed") + advanceRobolectricLooper() + val lifecycle = deckPicker.lifecycle as LifecycleRegistry + lifecycle.currentState = Lifecycle.State.CREATED + recovery.dismiss() + advanceRobolectricLooper() + deckPicker.showDatabaseErrorDialog(DatabaseErrorDialog.DatabaseErrorDialogType.DIALOG_LOAD_FAILED) + advanceRobolectricLooper() + val nextRecovery = + deckPicker.supportFragmentManager.fragments + .filterIsInstance() + .single() + val nextRecoveryDialog = nextRecovery.requireDialog() + + lifecycle.currentState = Lifecycle.State.RESUMED + advanceRobolectricLooper() + assertSame(nextRecoveryDialog, ShadowDialog.getLatestDialog()) + + nextRecovery.dismiss() + advanceRobolectricLooperUntil { + val latest = ShadowDialog.getLatestDialog() + latest !== nextRecoveryDialog && latest?.isShowing == true + } + assertEquals("Backup failed", getAlertDialogText(checkDismissed = true)) + } + @Test fun `failed deck deletion shows an error`() { useBackend { @@ -482,6 +550,34 @@ class DeckPickerErrorTest : RobolectricTest() { assertEquals(1, viewModel.pendingErrors.value.size) } + private fun withCorruptDeletionRecovery(block: (DeckPicker, DatabaseErrorDialog) -> Unit) { + val corruption = BackendDbCorruptException(backendError { message = "database disk image is malformed" }) + var corrupt = false + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = throw corruption + + override fun deckTree(now: Long): DeckTreeNode = if (corrupt) throw corruption else super.deckTree(now) + } + } + val deckId = addDeck("Deck") + val previouslyDetected = DatabaseCorruption.isDetected + DatabaseCorruption.isDetected = false + try { + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + corrupt = true + val deletion = deckPicker.deleteDeck(deckId) + val recoveries = { deckPicker.supportFragmentManager.fragments.filterIsInstance() } + advanceRobolectricLooperUntil { deletion.isCompleted && recoveries().any { it.dialog?.isShowing == true } } + + block(deckPicker, recoveries().single()) + } + } finally { + DatabaseCorruption.isDetected = previouslyDetected + } + } + private fun DeckPicker.selectContextMenuOption( option: DeckPickerContextMenuOption, deckId: DeckId, diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt new file mode 100644 index 000000000000..13f762d479c2 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +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.assertEquals +import kotlin.test.assertSame + +@RunWith(AndroidJUnit4::class) +class DeckPickerFurtherReviewTest : RobolectricTest() { + private val failure = BackendIoException(backendError { message = "Backend operation failed" }) + + @After + fun resetBackendFactory() { + BackendFactory.setOverride(null) + } + + @Test + fun `error waiting for recovery is not shown while owner is stopped`() { + var corrupt = true + useBackend { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = + if (corrupt) { + throw BackendDbCorruptException(backendError { message = "database disk image is malformed" }) + } else { + throw failure + } + } + } + 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() + .any { it.dialog?.isShowing == true } + } + val recovery = + picker.supportFragmentManager.fragments + .filterIsInstance() + .single() + val recoveryDialog = recovery.requireDialog() + corrupt = false + val second = picker.deleteDeck(target) + advanceRobolectricLooperUntil { second.isCompleted && picker.viewModel.loadDeckCounts?.isCompleted == true } + advanceRobolectricLooper() + assertSame(recoveryDialog, ShadowDialog.getLatestDialog()) + + val lifecycle = picker.lifecycle as LifecycleRegistry + lifecycle.currentState = Lifecycle.State.CREATED + try { + recovery.dismiss() + advanceRobolectricLooper() + assertSame( + recoveryDialog, + ShadowDialog.getLatestDialog(), + "The next queued error must wait for STARTED after recovery finishes in the background", + ) + } finally { + lifecycle.currentState = Lifecycle.State.RESUMED + } + advanceRobolectricLooperUntil { + ShadowDialog.getLatestDialog().let { it !== recoveryDialog && it?.isShowing == true } + } + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + ShadowDialog.getLatestDialog().dismiss() + advanceRobolectricLooperUntil { + picker.viewModel.pendingErrors.value + .isEmpty() + } + } + } finally { + DatabaseCorruption.isDetected = previouslyDetected + } + } + + private fun useBackend(backend: () -> Backend) { + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { backend() } + } +} 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 index 000000000000..dd9b54b31825 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryHandoffProbeTest.kt @@ -0,0 +1,99 @@ +// 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): 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() + .any { it.dialog?.isShowing == true } + } + val firstRecovery = + picker.supportFragmentManager.fragments + .filterIsInstance() + .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() + .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/DeckPickerRecoveryTransactionProbeTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryTransactionProbeTest.kt new file mode 100644 index 000000000000..9d30cbebe3f4 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRecoveryTransactionProbeTest.kt @@ -0,0 +1,84 @@ +// 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): 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() + .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() + .count { it.dialog?.isShowing == true }, + "The queued corruption must open recovery now, rather than becoming a background notification", + ) + } finally { + picker.supportFragmentManager.fragments + .filterIsInstance() + .forEach { it.dismiss() } + } + } + } finally { + DatabaseCorruption.isDetected = previouslyDetected + } + } +} From 9aab5d71be5211df6ff59446af26623644a5496e Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 27 Oct 2026 01:10:58 +0530 Subject: [PATCH 11/30] fix(deck-picker): reload the deck list after a partly applied action Deleting a deck, and emptying or rebuilding a filtered deck, take more than one backend call. If a later one failed, for example selecting the deck that replaces a deleted one, the error was shown but the deck list and the tablet study options kept showing the old state. The same happened when selecting a newly created deck failed, so the new deck was missing from the list. The Deck Picker now reloads after these failures. It skips the reload when the database is corrupt, as the recovery dialog handles that and a reload would only queue a second recovery prompt. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 12 ++-- .../ichi2/anki/DeckPickerErrorRecoveryTest.kt | 57 +++++++++++++++++++ .../com/ichi2/anki/DeckPickerErrorTest.kt | 54 ++++++++++++++++++ .../ichi2/anki/DeckPickerFurtherReviewTest.kt | 37 ++++++++++++ 4 files changed, 156 insertions(+), 4 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 921ba2ce8806..4e58f0a78a5a 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -4,6 +4,7 @@ package com.ichi2.anki.deckpicker import android.annotation.SuppressLint import android.content.SharedPreferences +import android.database.sqlite.SQLiteDatabaseCorruptException import android.os.Build import androidx.annotation.CheckResult import androidx.lifecycle.ViewModel @@ -62,6 +63,7 @@ import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException import net.ankiweb.rsdroid.RustCleanup import net.ankiweb.rsdroid.exceptions.BackendNetworkException import timber.log.Timber @@ -260,6 +262,7 @@ class DeckPickerViewModel : private fun launchCatchingTask( context: CoroutineContext = EmptyCoroutineContext, + reloadOnFailure: Boolean = false, block: suspend CoroutineScope.() -> Unit, ): Job = viewModelScope.launch(context) { @@ -269,6 +272,7 @@ class DeckPickerViewModel : throw e } catch (e: Exception) { reportError(e) + if (reloadOnFailure && e !is BackendDbCorruptException && e !is SQLiteDatabaseCorruptException) reloadDeckCounts() } } @@ -280,7 +284,7 @@ class DeckPickerViewModel : * @param did ID of the deck to delete */ fun deleteDeck(did: DeckId) = - launchCatchingTask { + launchCatchingTask(reloadOnFailure = true) { progressManager.withProgress(message = DeckPickerProgress.DELETING_DECK) { val deckName = withCol { decks.getLegacy(did)!!.name } val changes = @@ -339,7 +343,7 @@ class DeckPickerViewModel : } fun emptyFilteredDeck(deckId: DeckId): Job = - launchCatchingTask { + launchCatchingTask(reloadOnFailure = true) { progressManager.withProgress { Timber.i("empty filtered deck %s", deckId) withCol { decks.select(deckId) } @@ -352,7 +356,7 @@ class DeckPickerViewModel : * Rebuilds a filtered deck with its current filter settings */ fun rebuildFilteredDeck(deckId: DeckId): Job = - launchCatchingTask { + launchCatchingTask(reloadOnFailure = true) { progressManager.withProgress(message = DeckPickerProgress.REBUILDING_FILTERED_DECK) { Timber.i("rebuilding filtered deck %s", deckId) withCol { @@ -371,7 +375,7 @@ class DeckPickerViewModel : val flowOfCreatedDeckSelected = MutableSharedFlow(extraBufferCapacity = 1) fun selectCreatedDeck(deckId: DeckId) = - launchCatchingTask { + launchCatchingTask(reloadOnFailure = true) { markDeckSelected(deckId) flowOfCreatedDeckSelected.emit(Unit) } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorRecoveryTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorRecoveryTest.kt index 675a962b07dd..047d4c7aa7ab 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorRecoveryTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorRecoveryTest.kt @@ -2,13 +2,16 @@ 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.OpChanges import anki.collection.OpChangesWithCount import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.dialogs.DatabaseErrorDialog +import com.ichi2.anki.widgets.DeckAdapter import kotlinx.coroutines.runBlocking import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException @@ -17,6 +20,7 @@ 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.assertSame @@ -30,6 +34,59 @@ class DeckPickerErrorRecoveryTest : RobolectricTest() { BackendFactory.setOverride(null) } + @Test + @Config(qualifiers = "xlarge") + fun `failed replacement selection reconciles the deck list and tablet selection`() { + var failSelection = false + useBackend { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges { + if (failSelection) { + failSelection = false + throw failure + } + return super.setCurrentDeck(did) + } + } + } + addNoteToDeck(1L) + val deleted = addDeck("Deleted", setAsSelected = true) + val survivor = addDeck("Survivor") + addNoteToDeck(deleted) + + withDeckPicker(deckCount = 0) { picker -> + picker.awaitVisibleDecks(listOf(1L, deleted, survivor)) + advanceRobolectricLooperUntil { + picker.viewModel.loadDeckCounts?.isCompleted == true && picker.fragment?.viewModel?.selectedDeckId == deleted + } + throwOnShowError = false + + // Removing the selected deck commits before selecting its replacement can fail. + failSelection = true + val deletion = picker.deleteDeck(deleted) + advanceRobolectricLooperUntil { + deletion.isCompleted && (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true + } + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + ShadowDialog.getLatestDialog().dismiss() + + // Shift+Delete acts on the backend's selected deck. A stale tablet pane can make + // retrying deletion erase Default's cards while still displaying the deleted deck. + // Permit either reconciliation of the committed deletion or an atomic rollback. + val adapter = picker.deckPickerBinding.decks.adapter as DeckAdapter + advanceRobolectricLooperUntil( + lazyMessage = { + "After failed replacement selection, the deck list and tablet pane must match the collection: " + + "visible=${adapter.currentList.map { it.did }}, " + + "displayed=${picker.fragment?.viewModel?.selectedDeckId}, selected=${col.decks.selected()}" + }, + ) { + adapter.currentList.map { it.did } == col.decks.allNamesAndIds(skipEmptyDefault = true).map { it.id } && + picker.fragment?.viewModel?.selectedDeckId == col.decks.selected() + } + } + } + @Test fun `queued error waits until database recovery is dismissed`() { var corrupt = true diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 00e2a019e508..aca3b1c8cc62 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -29,6 +29,7 @@ import com.ichi2.anki.dialogs.setDeckPickerContextMenuResult import com.ichi2.anki.dialogs.startDeletingEmptyCards import com.ichi2.anki.dialogs.utils.input import com.ichi2.anki.dialogs.utils.performPositiveClick +import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID import com.ichi2.anki.libanki.DeckId import com.ichi2.testutils.BackupManagerTestUtilities import kotlinx.coroutines.Job @@ -210,6 +211,18 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `corrupt deletion does not queue a second recovery prompt`() = + withCorruptDeletionRecovery { deckPicker, _ -> + advanceRobolectricLooperUntil { deckPicker.viewModel.loadDeckCounts?.isCompleted == true } + advanceRobolectricLooper() + + assertTrue( + deckPicker.viewModel.pendingErrors.value + .isEmpty(), + ) + } + @Test fun `error message waits until database recovery is closed`() = withCorruptDeletionRecovery { deckPicker, recovery -> @@ -347,6 +360,25 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + @Config(qualifiers = "xlarge") + fun `failed filtered deck rebuild still shows the deck it selected`() = + assertFailedFilteredDeckActionShowsSelection(DeckPickerContextMenuOption.CUSTOM_STUDY_REBUILD) { failing -> + object : Backend() { + override fun rebuildFilteredDeck(did: Long): OpChangesWithCount = + if (failing()) throw failure else super.rebuildFilteredDeck(did) + } + } + + @Test + @Config(qualifiers = "xlarge") + fun `failed filtered deck emptying still shows the deck it selected`() = + assertFailedFilteredDeckActionShowsSelection(DeckPickerContextMenuOption.CUSTOM_STUDY_EMPTY) { failing -> + object : Backend() { + override fun emptyFilteredDeck(did: Long): OpChanges = if (failing()) throw failure else super.emptyFilteredDeck(did) + } + } + @Test fun `failed selection of a new deck shows an error`() { useBackend { @@ -550,6 +582,28 @@ class DeckPickerErrorTest : RobolectricTest() { assertEquals(1, viewModel.pendingErrors.value.size) } + private fun assertFailedFilteredDeckActionShowsSelection( + option: DeckPickerContextMenuOption, + backend: (failing: () -> Boolean) -> Backend, + ) { + var failing = false + useBackend { backend { failing } } + addNoteToDeck(DEFAULT_DECK_ID) + val filtered = addDynamicDeck("Filtered") + col.decks.select(DEFAULT_DECK_ID) + + withDeckPicker(deckCount = 0) { deckPicker -> + throwOnShowError = false + advanceRobolectricLooperUntil { deckPicker.fragment?.viewModel?.selectedDeckId == DEFAULT_DECK_ID } + failing = true + deckPicker.selectContextMenuOption(option, filtered) + + assertErrorShown() + assertEquals(filtered, col.decks.selected()) + advanceRobolectricLooperUntil { deckPicker.fragment?.viewModel?.selectedDeckId == filtered } + } + } + private fun withCorruptDeletionRecovery(block: (DeckPicker, DatabaseErrorDialog) -> Unit) { val corruption = BackendDbCorruptException(backendError { message = "database disk image is malformed" }) var corrupt = false diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt index 13f762d479c2..c9aea976f730 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt @@ -2,13 +2,17 @@ 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.OpChanges import anki.collection.OpChangesWithCount import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.dialogs.DatabaseErrorDialog +import com.ichi2.anki.dialogs.utils.input +import com.ichi2.anki.dialogs.utils.performPositiveClick import kotlinx.coroutines.runBlocking import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException @@ -96,6 +100,39 @@ class DeckPickerFurtherReviewTest : RobolectricTest() { } } + @Test + fun `a created deck remains visible when selecting it fails`() { + var failSelection = false + useBackend { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges = if (failSelection) throw failure else super.setCurrentDeck(did) + } + } + val existing = addDeck("Existing") + withDeckPicker(deckCount = 0) { picker -> + picker.awaitVisibleDecks(listOf(existing)) + advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true } + throwOnShowError = false + failSelection = true + picker.showCreateDeckDialog() + (ShadowDialog.getLatestDialog() as AlertDialog).apply { + input = "New deck" + performPositiveClick() + } + advanceRobolectricLooperUntil { + picker.viewModel.pendingErrors.value + .isNotEmpty() + } + ShadowDialog.getLatestDialog().dismiss() + advanceRobolectricLooperUntil { + picker.viewModel.pendingErrors.value + .isEmpty() + } + val created = col.decks.byName("New deck")!!.id + picker.awaitVisibleDecks(listOf(existing, created)) + } + } + private fun useBackend(backend: () -> Backend) { runBlocking { CollectionManager.discardBackend() } BackendFactory.setOverride { backend() } From b30d64f58430c4292cf85a5a84348b0e1e07357b Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:25:32 +0530 Subject: [PATCH 12/30] fix(deck-picker): only delete the deck the side panel shows On tablets, Delete deck in the toolbar and Shift+Delete delete the selected deck without asking, because the side panel shows it. If a failure left the panel showing another deck, for example one whose deletion had already gone through, or the panel showed something else such as review reminders, they deleted a deck that wasn't shown. They now go by the deck the panel last drew, since its data can change before it redraws, and reload the deck list when that isn't the selected deck or the panel shows no deck. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 2 +- .../com/ichi2/anki/StudyOptionsFragment.kt | 6 + .../anki/deckpicker/DeckPickerViewModel.kt | 6 +- .../anki/DeckPickerDeletionSafetyProbeTest.kt | 154 ++++++++++++++++++ .../ichi2/anki/DeckPickerFurtherReviewTest.kt | 75 +++++++++ .../deckpicker/DeckPickerViewModelTest.kt | 5 +- 6 files changed, 244 insertions(+), 4 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerDeletionSafetyProbeTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index b654345f8943..37370556b594 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -2452,7 +2452,7 @@ open class DeckPicker : */ fun deleteDeck(did: DeckId) = viewModel.deleteDeck(did) - private fun deleteSelectedDeck() = viewModel.deleteSelectedDeck() + private fun deleteSelectedDeck() = viewModel.deleteSelectedDeck(shownDeckId = fragment?.shownDeckId) fun rebuildFiltered(did: DeckId) { viewModel.rebuildFilteredDeck(did) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt index 98f9c5a0dbf3..98eeef31b093 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt @@ -76,6 +76,11 @@ class StudyOptionsFragment : @VisibleForTesting internal val viewModel: StudyOptionsViewModel by viewModels() + val selectedDeckId: DeckId get() = viewModel.selectedDeckId + + var shownDeckId: DeckId? = null + private set + private lateinit var deckInfoLayout: Group private lateinit var buttonStart: Button private lateinit var textDeckName: TextView @@ -374,6 +379,7 @@ class StudyOptionsFragment : displayedDeckId = viewModel.selectedDeckId updateDeckNameView(state) + shownDeckId = viewModel.selectedDeckId when (state) { is StudyOptionsState.Empty -> { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 4e58f0a78a5a..faa48d0b0fa4 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -306,10 +306,14 @@ class DeckPickerViewModel : /** * Deletes the currently selected deck */ - fun deleteSelectedDeck() = + fun deleteSelectedDeck(shownDeckId: DeckId?) = launchCatchingTask { progressManager.withProgress(message = DeckPickerProgress.DELETING_DECK) { val targetDeckId = withCol { decks.selected() } + if (targetDeckId != shownDeckId) { + reloadDeckCounts() + return@withProgress + } deleteDeck(targetDeckId).join() } } 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 index 000000000000..89a0fa89539a --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerDeletionSafetyProbeTest.kt @@ -0,0 +1,154 @@ +// 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(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(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(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/DeckPickerFurtherReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt index c9aea976f730..daa6688d5b29 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFurtherReviewTest.kt @@ -2,6 +2,7 @@ package com.ichi2.anki +import android.view.KeyEvent import androidx.appcompat.app.AlertDialog import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleRegistry @@ -9,10 +10,13 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import anki.backend.backendError import anki.collection.OpChanges import anki.collection.OpChangesWithCount +import anki.decks.DeckTreeNode import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.utils.input import com.ichi2.anki.dialogs.utils.performPositiveClick +import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID +import com.ichi2.anki.progress.ViewModelProgress import kotlinx.coroutines.runBlocking import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException @@ -21,6 +25,7 @@ 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.assertSame @@ -100,6 +105,76 @@ class DeckPickerFurtherReviewTest : RobolectricTest() { } } + @Test + @Config(qualifiers = "xlarge") + fun `failed recovery refresh cannot leave a destructive shortcut acting on another deck`() { + var failSelection = false + var failReload = false + useBackend { + 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 + return 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?.viewModel?.selectedDeckId == deleted + } + throwOnShowError = false + failSelection = true + val deletion = picker.deleteDeck(deleted) + advanceRobolectricLooperUntil { + deletion.isCompleted && picker.viewModel.loadDeckCounts?.isCompleted == true + } + while (picker.viewModel.pendingErrors.value + .isNotEmpty() + ) { + val pending = + picker.viewModel.pendingErrors.value + .first() + .id + advanceRobolectricLooperUntil { (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true } + ShadowDialog.getLatestDialog().dismiss() + advanceRobolectricLooperUntil { + picker.viewModel.pendingErrors.value + .firstOrNull() + ?.id != pending + } + } + + // A fix may reconcile or clear the pane. If it still displays the deleted deck, + // Shift+Delete must be blocked rather than deleting the backend's fallback deck. + if (picker.fragment?.viewModel?.selectedDeckId == deleted) { + picker.onKeyUp( + KeyEvent.KEYCODE_DEL, + KeyEvent(0, 0, KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL, 0, KeyEvent.META_SHIFT_ON), + ) + advanceRobolectricLooperUntil { picker.viewModel.progressManager.progress.value is ViewModelProgress.Idle } + } + assertEquals( + 1, + col.decks.cardCount(DEFAULT_DECK_ID, includeSubdecks = true), + "Retrying the displayed deletion removed the cards from a different deck", + ) + } + } + @Test fun `a created deck remains visible when selecting it fails`() { var failSelection = false diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt index b8f927f936e3..7130ff2cd468 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt @@ -310,11 +310,12 @@ class DeckPickerViewModelTest : RobolectricTest() { @Test fun `deleteSelectedDeck reports progress before reading the selected deck`() = runTest { - col.decks.select(addDeck("Deleted")) + val deckId = addDeck("Deleted") + col.decks.select(deckId) DeckPickerViewModel().run { withQueuedCollectionAccess { - val deletion = deleteSelectedDeck() + val deletion = deleteSelectedDeck(shownDeckId = deckId) val active = assertIs>(progressManager.progress.value) assertEquals(DeckPickerProgress.DELETING_DECK, active.message) deletion.join() From 94d23a4deda26e1bd3b05bbd08cb5327276a8b23 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:21:57 +0530 Subject: [PATCH 13/30] fix(deck-picker): take the shown deck from the state the panel drew The side panel recorded the deck it had drawn by asking its ViewModel, which a refresh can move to another deck before the panel draws it. If the panel redrew its older state in between, for example when it started again during the refresh, Shift+Delete deleted the new deck while the panel still showed the old one. The panel's state now carries its deck, and the panel records that deck when it draws the state. Assisted-by: Claude Opus 5.5 --- .../com/ichi2/anki/StudyOptionsFragment.kt | 2 +- .../com/ichi2/anki/StudyOptionsViewModel.kt | 14 +++ .../anki/DeckPickerRenderedSelectionTest.kt | 86 +++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRenderedSelectionTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt index 98eeef31b093..590d9fa9ccaf 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt @@ -379,7 +379,7 @@ class StudyOptionsFragment : displayedDeckId = viewModel.selectedDeckId updateDeckNameView(state) - shownDeckId = viewModel.selectedDeckId + shownDeckId = state.deckIdOrNull() when (state) { is StudyOptionsState.Empty -> { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsViewModel.kt index cc6c1662c723..7f8421df1998 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsViewModel.kt @@ -128,17 +128,20 @@ class StudyOptionsViewModel : when { study.isEmptyRegularDeck -> StudyOptionsState.Empty( + deckId = deckId, deckName = deck.name, data = data, ) study.availability == StudyAvailability.NoCardsAvailable -> StudyOptionsState.Congrats( + deckId = deckId, deckName = deck.name, isDynamic = isDynamic, data = data, ) else -> StudyOptionsState.StudyOptions( + deckId = deckId, deckName = deck.name, deckDescription = description, isDynamic = isDynamic, @@ -171,6 +174,7 @@ sealed interface StudyOptionsState { data object Loading : StudyOptionsState data class StudyOptions( + val deckId: DeckId, val deckName: String, val deckDescription: String?, val isDynamic: Boolean, @@ -178,12 +182,14 @@ sealed interface StudyOptionsState { ) : StudyOptionsState data class Congrats( + val deckId: DeckId, val deckName: String, val isDynamic: Boolean, val data: DeckStudyData, ) : StudyOptionsState data class Empty( + val deckId: DeckId, val deckName: String, val data: DeckStudyData, ) : StudyOptionsState @@ -206,3 +212,11 @@ fun StudyOptionsState.deckNameOrNull(): String? = is StudyOptionsState.Empty -> deckName is StudyOptionsState.Loading -> null } + +fun StudyOptionsState.deckIdOrNull(): DeckId? = + when (this) { + is StudyOptionsState.StudyOptions -> deckId + is StudyOptionsState.Congrats -> deckId + is StudyOptionsState.Empty -> deckId + is StudyOptionsState.Loading -> null + } 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 000000000000..ace904ace83a --- /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(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) + } + } + } +} From 30011770920ac2efb14166d04b3cea7f9e12df16 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:22:09 +0530 Subject: [PATCH 14/30] fix(deck-picker): don't let a refresh undo a newer deck selection A refresh reads the current deck first and moves the deck list's focus to it at the end, so a deck selected while the refresh ran was replaced by the one the refresh had read, and the list jumped back to it. The refresh now leaves the focus alone if it changed in the meantime. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 7 ++- .../anki/DeckPickerRefreshSelectionTest.kt | 54 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRefreshSelectionTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index faa48d0b0fa4..79d6c667c05c 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -67,6 +67,7 @@ import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptE import net.ankiweb.rsdroid.RustCleanup import net.ankiweb.rsdroid.exceptions.BackendNetworkException import timber.log.Timber +import java.util.concurrent.atomic.AtomicInteger import kotlin.coroutines.CoroutineContext import kotlin.coroutines.EmptyCoroutineContext import com.ichi2.anki.common.destinations.Destination as NavigateDestination @@ -107,9 +108,12 @@ class DeckPickerViewModel : */ val flowOfFocusedDeck = MutableStateFlow(null) + private val focusedDeckChanges = AtomicInteger() + var focusedDeck: DeckId? get() = flowOfFocusedDeck.value set(value) { + focusedDeckChanges.incrementAndGet() flowOfFocusedDeck.value = value } @@ -509,6 +513,7 @@ class DeckPickerViewModel : val loadDeckCounts = launchCatchingTask(Dispatchers.IO) { Timber.d("Refreshing deck list") + val focusChangesBeforeRefresh = focusedDeckChanges.get() val (deckDueTree, collectionHasNoCards, currentDeckId) = withCol { Triple(sched.deckDueTree(), isEmpty, decks.current().id) @@ -536,7 +541,7 @@ class DeckPickerViewModel : // TODO: This is in the wrong place // current deck may have changed - focusedDeck = currentDeckId + if (focusedDeckChanges.get() == focusChangesBeforeRefresh) focusedDeck = currentDeckId refreshUndoMenuState() // Reopening can leave the counts and selection unchanged after a skipped display refresh. 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 000000000000..363339063467 --- /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", + ) + } +} From 39f92ce770808d72bea3902a22037a53a767e98e Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 03:24:00 +0530 Subject: [PATCH 15/30] fix(deck-picker): update the focused deck through the collection queue A refresh checked that nobody had changed the focused deck and then set it, but a deck selected between the check and the update was still replaced by the refresh's older deck. The check and the update now run together on the collection queue, and selecting or deleting a deck updates the focus there too, so they can't interleave. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 12 ++- .../DeckPickerFocusAtomicityRegressionTest.kt | 82 +++++++++++++++++++ 2 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFocusAtomicityRegressionTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 79d6c667c05c..337aea6c7c3f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -299,7 +299,7 @@ class DeckPickerViewModel : filter = DeckFilters.create(flowOfDeckSearchQuery.value), ) } - focusedDeck = withCol { decks.selected() } + withCol { focusedDeck = decks.selected() } deckDeletedNotification.emit( DeckDeletionResult(deckName = deckName, cardsDeleted = changes.count), @@ -390,8 +390,10 @@ class DeckPickerViewModel : private suspend fun markDeckSelected(deckId: DeckId) { // TODO: should we always reset the Card Browser default deck here? - withCol { decks.select(deckId) } - focusedDeck = deckId + withCol { + decks.select(deckId) + focusedDeck = deckId + } flowOfRefreshDeckList.emit(Unit) } @@ -541,7 +543,9 @@ class DeckPickerViewModel : // TODO: This is in the wrong place // current deck may have changed - if (focusedDeckChanges.get() == focusChangesBeforeRefresh) focusedDeck = currentDeckId + withCol { + if (focusedDeckChanges.get() == focusChangesBeforeRefresh) focusedDeck = currentDeckId + } refreshUndoMenuState() // Reopening can leave the counts and selection unchanged after a skipped display refresh. diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFocusAtomicityRegressionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFocusAtomicityRegressionTest.kt new file mode 100644 index 000000000000..ded55ca4891b --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFocusAtomicityRegressionTest.kt @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +import android.os.Looper +import androidx.lifecycle.viewModelScope +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.deckpicker.DeckPickerViewModel +import com.ichi2.anki.libanki.DeckId +import kotlinx.coroutines.ExperimentalForInheritanceCoroutinesApi +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.MutableStateFlow +import org.junit.Test +import org.junit.runner.RunWith +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.concurrent.thread +import kotlin.test.assertEquals + +@OptIn(ExperimentalForInheritanceCoroutinesApi::class) +@RunWith(AndroidJUnit4::class) +class DeckPickerFocusAtomicityRegressionTest : RobolectricTest() { + @Test + fun `refresh does not overwrite a selection completed before focus publication`() { + ensureCollectionLoadIsSynchronous() + val old = addDeck("Old", setAsSelected = true) + val latest = addDeck("Latest") + val model = DeckPickerViewModel() + val initialRefresh = model.reloadDeckCounts() + advanceRobolectricLooperUntil { initialRefresh.isCompleted } + assertEquals(old, model.focusedDeck) + + val publishingRefresh = CountDownLatch(1) + val releaseRefresh = CountDownLatch(1) + val selectionCompleted = CountDownLatch(1) + val pauseOnce = AtomicBoolean(true) + val original = model.flowOfFocusedDeck + val gated = + object : MutableStateFlow by original { + override var value: DeckId? + get() = original.value + set(value) { + // Pause a background publication after the version guard, without changing + // the flow consumed by the UI. A publication confined to Main can finish + // first: it cannot race with a user selection running on Main. + if (value == old && Looper.myLooper() != Looper.getMainLooper() && pauseOnce.compareAndSet(true, false)) { + publishingRefresh.countDown() + check(releaseRefresh.await(10, TimeUnit.SECONDS)) { "Timed out releasing the refresh" } + } + original.value = value + } + } + val focusFlow = model.javaClass.getDeclaredField("flowOfFocusedDeck").apply { isAccessible = true } + focusFlow.set(model, gated) + val refresh = model.reloadDeckCounts() + var releaseWorker: Thread? = null + try { + advanceRobolectricLooperUntil { publishingRefresh.count == 0L || refresh.isCompleted } + releaseWorker = + thread(name = "release-deck-focus-refresh") { + // A correct implementation may serialize selection behind the refresh. + // Release that case after a bounded wait so it can finish as well. + selectionCompleted.await(2, TimeUnit.SECONDS) + releaseRefresh.countDown() + } + val selection = model.selectDeck(latest) + selection.invokeOnCompletion { selectionCompleted.countDown() } + advanceRobolectricLooperUntil { selection.isCompleted && refresh.isCompleted } + + assertEquals(latest, col.decks.selected(), "The backend should keep the user's newer selection") + assertEquals(latest, model.focusedDeck, "The refresh must not overwrite the newer selection's focus") + } finally { + selectionCompleted.countDown() + releaseRefresh.countDown() + releaseWorker?.join() + advanceRobolectricLooperUntil { refresh.isCompleted } + focusFlow.set(model, original) + model.viewModelScope.cancel() + } + } +} From b5f5cc6799d4dc4fafe5e52b7d142f75a9f0a1dc Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 03:24:12 +0530 Subject: [PATCH 16/30] fix(deck-picker): don't lose a collapse to a second tap or a refresh The deck list only shows a deck as collapsed or expanded once the collection has saved it, so a second tap before that save finished read the old state and saved the same value again. Collapsing a deck while a refresh was reading the decks could also be undone when that refresh showed its older copy of the list. Collapsing and expanding now happen one at a time, after any refresh that is already running. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 29 +++--- .../com/ichi2/anki/DeckPickerErrorTest.kt | 91 +++++++++++++++++++ 2 files changed, 109 insertions(+), 11 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 337aea6c7c3f..ae786c9bb2be 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -62,6 +62,8 @@ import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException import net.ankiweb.rsdroid.RustCleanup @@ -565,20 +567,25 @@ class DeckPickerViewModel : isDeckSearchExpanded = isExpanded } + private val deckExpansion = Mutex() + fun toggleDeckExpand(deckId: DeckId) = launchCatchingTask { - // update stored state - dueTree?.find(deckId)?.run { - val collapse = !collapsed - // Anki uses scope as Reviewer in deck browser - withCol { - decks.setCollapsed( - did, - collapsed = collapse, - SetDeckCollapsedRequest.Scope.REVIEWER, - ) + deckExpansion.withLock { + loadDeckCounts?.join() + // update stored state + dueTree?.find(deckId)?.run { + val collapse = !collapsed + // Anki uses scope as Reviewer in deck browser + withCol { + decks.setCollapsed( + did, + collapsed = collapse, + SetDeckCollapsedRequest.Scope.REVIEWER, + ) + } + collapsed = collapse } - collapsed = collapse } flowOfRefreshDeckList.emit(Unit) } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index aca3b1c8cc62..882d91495890 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -32,6 +32,7 @@ import com.ichi2.anki.dialogs.utils.performPositiveClick import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID import com.ichi2.anki.libanki.DeckId import com.ichi2.testutils.BackupManagerTestUtilities +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.runBlocking @@ -50,6 +51,9 @@ import org.mockito.kotlin.verify import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config import org.robolectric.shadows.ShadowDialog +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertNotSame @@ -538,6 +542,84 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `expanding a deck again while it is collapsing restores it`() { + val saving = CountDownLatch(1) + val releaseSave = CountDownLatch(1) + val pauseSave = AtomicBoolean(true) + useBackend { + object : Backend() { + override fun setDeckCollapsed( + deckId: Long, + collapsed: Boolean, + scope: SetDeckCollapsedRequest.Scope, + ): OpChanges { + if (pauseSave.compareAndSet(true, false)) { + saving.countDown() + check(releaseSave.await(10, TimeUnit.SECONDS)) { "Timed out waiting to save the deck" } + } + return super.setDeckCollapsed(deckId, collapsed, scope) + } + } + } + val parent = addDeck("Parent") + addDeck("Parent::Child") + val viewModel = DeckPickerViewModel() + + val initiallyCollapsed = col.decks.get(parent)!!.collapsed + + withCollectionThread { + val initialRefresh = viewModel.reloadDeckCounts() + advanceRobolectricLooperUntil { initialRefresh.isCompleted } + val first = viewModel.toggleDeckExpand(parent) + assertTrue(saving.await(5, TimeUnit.SECONDS)) + val second = viewModel.toggleDeckExpand(parent) + releaseSave.countDown() + advanceRobolectricLooperUntil { first.isCompleted && second.isCompleted } + } + + assertEquals(initiallyCollapsed, col.decks.get(parent)!!.collapsed) + assertEquals(initiallyCollapsed, viewModel.dueTree?.find(parent)?.collapsed) + } + + @Test + fun `a refresh does not undo a collapse saved while it reads the decks`() { + val readingDecks = CountDownLatch(1) + val releaseRead = CountDownLatch(1) + val pauseRead = AtomicBoolean(false) + useBackend { + object : Backend() { + override fun deckTree(now: Long): DeckTreeNode { + if (pauseRead.compareAndSet(true, false)) { + readingDecks.countDown() + check(releaseRead.await(10, TimeUnit.SECONDS)) { "Timed out waiting to read the decks" } + } + return super.deckTree(now) + } + } + } + val parent = addDeck("Parent") + addDeck("Parent::Child") + val viewModel = DeckPickerViewModel() + + val initiallyCollapsed = col.decks.get(parent)!!.collapsed + + withCollectionThread { + val initialRefresh = viewModel.reloadDeckCounts() + advanceRobolectricLooperUntil { initialRefresh.isCompleted } + addDeck("Other") + pauseRead.set(true) + val refresh = viewModel.reloadDeckCounts() + assertTrue(readingDecks.await(5, TimeUnit.SECONDS)) + val toggle = viewModel.toggleDeckExpand(parent) + releaseRead.countDown() + advanceRobolectricLooperUntil { refresh.isCompleted && toggle.isCompleted } + } + + assertEquals(!initiallyCollapsed, col.decks.get(parent)!!.collapsed) + assertEquals(!initiallyCollapsed, viewModel.dueTree?.find(parent)?.collapsed) + } + @Test fun `failed empty cards deletion shows an error`() { useBackend { @@ -562,6 +644,15 @@ class DeckPickerErrorTest : RobolectricTest() { BackendFactory.setOverride { backend() } } + private fun withCollectionThread(block: () -> Unit) { + val previousQueue = CollectionManager.setTestDispatcher(Dispatchers.IO.limitedParallelism(1), useReentrantLock = false) + try { + block() + } finally { + CollectionManager.setTestDispatcher(previousQueue) + } + } + private fun assertFailedSelectionSkips( followUp: DeckPickerViewModel.() -> Flow, request: DeckPickerViewModel.(DeckId) -> Job, From c42efcedc9467df504015f4eea4cca0f70df0a7e Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:48:33 +0530 Subject: [PATCH 17/30] fix(deck-picker): keep the deck tree in step with the collection Waiting for a running refresh before collapsing a deck wasn't enough: a second refresh could replace the one being waited for and still show a tree it read before the collapse was saved. A refresh now shows its tree, and whether the collection has cards, in the same collection call that reads them, and collapsing reads, saves and updates the deck in one collection call, so a tree read before a change can't be shown after it. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 31 ++--- .../DeckPickerReplacementRefreshReviewTest.kt | 117 ++++++++++++++++++ 2 files changed, 130 insertions(+), 18 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerReplacementRefreshReviewTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index ae786c9bb2be..93a81ba492d7 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -62,8 +62,6 @@ import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException import net.ankiweb.rsdroid.RustCleanup @@ -518,13 +516,15 @@ class DeckPickerViewModel : launchCatchingTask(Dispatchers.IO) { Timber.d("Refreshing deck list") val focusChangesBeforeRefresh = focusedDeckChanges.get() - val (deckDueTree, collectionHasNoCards, currentDeckId) = + val currentDeckId = withCol { - Triple(sched.deckDueTree(), isEmpty, decks.current().id) + val deckDueTree = sched.deckDueTree() + val collectionHasNoCards = isEmpty + val currentDeckId = decks.current().id + dueTree = deckDueTree + flowOfCollectionHasNoCards.value = collectionHasNoCards + currentDeckId } - dueTree = deckDueTree - - flowOfCollectionHasNoCards.value = collectionHasNoCards // TODO: This is in the wrong place // Backend returns studiedToday() with newlines for HTML formatting,so we replace them with spaces. @@ -567,23 +567,18 @@ class DeckPickerViewModel : isDeckSearchExpanded = isExpanded } - private val deckExpansion = Mutex() - fun toggleDeckExpand(deckId: DeckId) = launchCatchingTask { - deckExpansion.withLock { - loadDeckCounts?.join() + withCol { // update stored state dueTree?.find(deckId)?.run { val collapse = !collapsed // Anki uses scope as Reviewer in deck browser - withCol { - decks.setCollapsed( - did, - collapsed = collapse, - SetDeckCollapsedRequest.Scope.REVIEWER, - ) - } + decks.setCollapsed( + did, + collapsed = collapse, + SetDeckCollapsedRequest.Scope.REVIEWER, + ) collapsed = collapse } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerReplacementRefreshReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerReplacementRefreshReviewTest.kt new file mode 100644 index 000000000000..d5b04456eabb --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerReplacementRefreshReviewTest.kt @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +import androidx.lifecycle.viewModelScope +import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.decks.DeckTreeNode +import com.ichi2.anki.deckpicker.DeckPickerViewModel +import com.ichi2.anki.libanki.sched.DeckNode +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalForInheritanceCoroutinesApi +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.runBlocking +import net.ankiweb.rsdroid.Backend +import net.ankiweb.rsdroid.BackendFactory +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.concurrent.thread +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +@OptIn(ExperimentalForInheritanceCoroutinesApi::class) +@RunWith(AndroidJUnit4::class) +class DeckPickerReplacementRefreshReviewTest : RobolectricTest() { + @After + fun resetBackend() = BackendFactory.setOverride(null) + + @Test + fun `refresh replacing the one awaited by a toggle must not publish stale expansion`() { + val readingFirst = CountDownLatch(1) + val releaseFirst = CountDownLatch(1) + val pauseFirst = AtomicBoolean(false) + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun deckTree(now: Long): DeckTreeNode { + if (pauseFirst.compareAndSet(true, false)) { + readingFirst.countDown() + check(releaseFirst.await(10, TimeUnit.SECONDS)) + } + return super.deckTree(now) + } + } + } + val parent = addDeck("Parent") + addDeck("Parent::Child") + val model = DeckPickerViewModel() + val initial = model.reloadDeckCounts() + advanceRobolectricLooperUntil { initial.isCompleted } + val initiallyCollapsed = model.dueTree!!.find(parent)!!.collapsed + val treeField = model.javaClass.getDeclaredField("flowOfDeckDueTree").apply { isAccessible = true } + + @Suppress("UNCHECKED_CAST") + val original = treeField.get(model) as MutableStateFlow + val publishingReplacement = CountDownLatch(1) + val releaseReplacement = CountDownLatch(1) + val gatePublication = AtomicBoolean(true) + treeField.set( + model, + object : MutableStateFlow by original { + override var value: DeckNode? + get() = original.value + set(value) { + if (gatePublication.compareAndSet(true, false)) { + publishingReplacement.countDown() + check(releaseReplacement.await(10, TimeUnit.SECONDS)) + } + original.value = value + } + }, + ) + val previousQueue = CollectionManager.setTestDispatcher(Dispatchers.IO.limitedParallelism(1), useReentrantLock = false) + try { + addDeck("New deck that makes the replacement snapshot different") + pauseFirst.set(true) + val first = model.reloadDeckCounts() + assertTrue(readingFirst.await(5, TimeUnit.SECONDS)) + // This action waits for the first refresh, which a subsequent refresh cancels. + val toggle = model.toggleDeckExpand(parent) + val replacement = model.reloadDeckCounts() + releaseFirst.countDown() + // A shared lock may let the toggle finish before the replacement reads its tree. + // Give that correct implementation a chance to drain Main as well. + if (!publishingReplacement.await(2, TimeUnit.SECONDS)) { + advanceRobolectricLooperUntil { publishingReplacement.count == 0L } + } + val toggleCompleted = CountDownLatch(1) + toggle.invokeOnCompletion { toggleCompleted.countDown() } + val releaser = + thread(name = "release-replacement-refresh") { + // A correct implementation can serialize the toggle behind this refresh. + // Permit that ordering while exposing the stale publication in the current code. + toggleCompleted.await(2, TimeUnit.SECONDS) + releaseReplacement.countDown() + } + advanceRobolectricLooperUntil { first.isCompleted && toggle.isCompleted && replacement.isCompleted } + releaser.join() + assertEquals(!initiallyCollapsed, col.decks.get(parent)!!.collapsed) + assertEquals( + col.decks.get(parent)!!.collapsed, + model.dueTree!!.find(parent)!!.collapsed, + "The replacement refresh must not undo the saved expansion in the displayed tree", + ) + } finally { + releaseFirst.countDown() + releaseReplacement.countDown() + treeField.set(model, original) + model.viewModelScope.cancel() + CollectionManager.setTestDispatcher(previousQueue) + } + } +} From dddeb6d8bf663ecce4bb57e453a3fa6ef9642871 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:48:36 +0530 Subject: [PATCH 18/30] fix(deck-picker): focus the deck that is current when a refresh ends A refresh moved the focus to the deck it read when it started, unless the focus had been set in the meantime. A deck selected another way while it ran, for example by rebuilding a filtered deck, was then replaced by the older one. The refresh now reads the current deck again when it sets the focus. It still reads it before showing the tree too, so a deck it can't read stops the refresh before the list changes. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 25 +++++----------- .../com/ichi2/anki/DeckPickerErrorTest.kt | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 93a81ba492d7..7739789d7ecc 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -67,7 +67,6 @@ import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptE import net.ankiweb.rsdroid.RustCleanup import net.ankiweb.rsdroid.exceptions.BackendNetworkException import timber.log.Timber -import java.util.concurrent.atomic.AtomicInteger import kotlin.coroutines.CoroutineContext import kotlin.coroutines.EmptyCoroutineContext import com.ichi2.anki.common.destinations.Destination as NavigateDestination @@ -108,12 +107,9 @@ class DeckPickerViewModel : */ val flowOfFocusedDeck = MutableStateFlow(null) - private val focusedDeckChanges = AtomicInteger() - var focusedDeck: DeckId? get() = flowOfFocusedDeck.value set(value) { - focusedDeckChanges.incrementAndGet() flowOfFocusedDeck.value = value } @@ -515,16 +511,13 @@ class DeckPickerViewModel : val loadDeckCounts = launchCatchingTask(Dispatchers.IO) { Timber.d("Refreshing deck list") - val focusChangesBeforeRefresh = focusedDeckChanges.get() - val currentDeckId = - withCol { - val deckDueTree = sched.deckDueTree() - val collectionHasNoCards = isEmpty - val currentDeckId = decks.current().id - dueTree = deckDueTree - flowOfCollectionHasNoCards.value = collectionHasNoCards - currentDeckId - } + withCol { + val deckDueTree = sched.deckDueTree() + val collectionHasNoCards = isEmpty + decks.current() + dueTree = deckDueTree + flowOfCollectionHasNoCards.value = collectionHasNoCards + } // TODO: This is in the wrong place // Backend returns studiedToday() with newlines for HTML formatting,so we replace them with spaces. @@ -545,9 +538,7 @@ class DeckPickerViewModel : // TODO: This is in the wrong place // current deck may have changed - withCol { - if (focusedDeckChanges.get() == focusChangesBeforeRefresh) focusedDeck = currentDeckId - } + withCol { focusedDeck = decks.current().id } refreshUndoMenuState() // Reopening can leave the counts and selection unchanged after a skipped display refresh. diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 882d91495890..5a869ab82af6 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -620,6 +620,35 @@ class DeckPickerErrorTest : RobolectricTest() { assertEquals(!initiallyCollapsed, viewModel.dueTree?.find(parent)?.collapsed) } + @Test + fun `a refresh focuses a deck selected elsewhere while it runs`() { + val selectOther = AtomicBoolean(false) + var other = 0L + useBackend { + object : Backend() { + override fun studiedToday(): String { + if (selectOther.compareAndSet(true, false)) setCurrentDeck(other) + return super.studiedToday() + } + } + } + val first = addDeck("First", setAsSelected = true) + other = addDeck("Other") + val viewModel = DeckPickerViewModel() + + withCollectionThread { + val initialRefresh = viewModel.reloadDeckCounts() + advanceRobolectricLooperUntil { initialRefresh.isCompleted } + assertEquals(first, viewModel.focusedDeck) + selectOther.set(true) + val refresh = viewModel.reloadDeckCounts() + advanceRobolectricLooperUntil { refresh.isCompleted } + } + + assertEquals(other, col.decks.selected()) + assertEquals(other, viewModel.focusedDeck) + } + @Test fun `failed empty cards deletion shows an error`() { useBackend { From a42b1b8dbe61a8e5a5e6b4df8239dd2fd456f609 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:48:40 +0530 Subject: [PATCH 19/30] fix(deck-picker): empty the filtered deck that was chosen Emptying a filtered deck selected it, then emptied whichever deck was selected in a separate collection call, so a deck tapped in between was emptied instead. It now empties the deck that was chosen. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 2 +- .../com/ichi2/anki/DeckPickerErrorTest.kt | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 7739789d7ecc..850e54f8c240 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -351,7 +351,7 @@ class DeckPickerViewModel : progressManager.withProgress { Timber.i("empty filtered deck %s", deckId) withCol { decks.select(deckId) } - undoableOp { sched.emptyFilteredDeck(decks.selected()) } + undoableOp { sched.emptyFilteredDeck(deckId) } flowOfDeckCountsChanged.emit(Unit) } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 5a869ab82af6..222d0c67bf6a 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -54,6 +54,7 @@ import org.robolectric.shadows.ShadowDialog import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicLong import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertNotSame @@ -649,6 +650,41 @@ class DeckPickerErrorTest : RobolectricTest() { assertEquals(other, viewModel.focusedDeck) } + @Test + fun `emptying a filtered deck empties it even if another deck is selected meanwhile`() { + val selectOther = AtomicBoolean(false) + val emptied = AtomicLong(0) + var filtered = 0L + var other = 0L + var selection: Job? = null + lateinit var viewModel: DeckPickerViewModel + useBackend { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges { + val changes = super.setCurrentDeck(did) + if (did == filtered && selectOther.compareAndSet(true, false)) selection = viewModel.selectDeck(other) + return changes + } + + override fun emptyFilteredDeck(did: Long): OpChanges { + emptied.set(did) + return super.emptyFilteredDeck(did) + } + } + } + filtered = addDynamicDeck("Filtered") + other = addDeck("Other") + viewModel = DeckPickerViewModel() + + withCollectionThread { + selectOther.set(true) + val empty = viewModel.emptyFilteredDeck(filtered) + advanceRobolectricLooperUntil { empty.isCompleted && selection?.isCompleted == true } + } + + assertEquals(filtered, emptied.get()) + } + @Test fun `failed empty cards deletion shows an error`() { useBackend { From 711c753e90a6b8da6b063e35374f6cf0e5bb6134 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:48:43 +0530 Subject: [PATCH 20/30] fix(deck-picker): don't queue an error that is already waiting A failure that kept happening, such as a refresh that couldn't read the decks, was queued again by every refresh and by the screen restarting, so the same error came back after each dismissal and sent another crash report. An error that matches one still waiting is now dropped. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 6 +++++- .../java/com/ichi2/anki/DeckPickerErrorTest.kt | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 850e54f8c240..78e92a5af049 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -276,7 +276,11 @@ class DeckPickerViewModel : } } - private suspend fun reportError(e: Exception) = withContext(Dispatchers.Main.immediate) { errorQueue.enqueue(e) } + private suspend fun reportError(e: Exception) = + withContext(Dispatchers.Main.immediate) { + val alreadyPending = errorQueue.messages.value.any { it.message.javaClass == e.javaClass && it.message.message == e.message } + if (!alreadyPending) errorQueue.enqueue(e) + } /** * Deletes the provided deck, child decks. and all cards inside. diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 222d0c67bf6a..327f835fbd47 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -685,6 +685,23 @@ class DeckPickerErrorTest : RobolectricTest() { assertEquals(filtered, emptied.get()) } + @Test + fun `a refresh failing again while its error is pending queues it once`() { + useBackend { + object : Backend() { + override fun deckTree(now: Long): DeckTreeNode = throw failure + } + } + + runTest { + val viewModel = DeckPickerViewModel() + viewModel.reloadDeckCounts().join() + viewModel.reloadDeckCounts().join() + + assertEquals(1, viewModel.pendingErrors.value.size) + } + } + @Test fun `failed empty cards deletion shows an error`() { useBackend { From 0046b2db527fa1484f70ec8d670ea48522349fab Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:04:28 +0530 Subject: [PATCH 21/30] fix(deck-picker): set a tapped deck's focus in the collection queue Tapping a deck set the focus once its collection call had returned, so a deck selected another way in the meantime, for example by long pressing it, lost the focus to the tapped one. The tap now sets it in the same collection call that selects the deck, like the other selections, and the studied today line is written the same way. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 2 +- .../anki/deckpicker/DeckPickerViewModel.kt | 2 +- .../com/ichi2/anki/DeckPickerErrorTest.kt | 97 +++++++++++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 37370556b594..3c4a95c2f81c 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -2328,12 +2328,12 @@ open class DeckPicker : val study = withCol { decks.select(did) + viewModel.focusedDeck = did currentDeckStudyState() } deckListAdapter.updateSelectedDeck(did) // Also forget the last deck used by the Browser CardBrowser.clearLastDeckId() - viewModel.focusedDeck = did if (study.availability == StudyAvailability.CardsAvailable) { openReviewerOrStudyOptions(selectionType) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 78e92a5af049..4ebde269621d 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -525,7 +525,7 @@ class DeckPickerViewModel : // TODO: This is in the wrong place // Backend returns studiedToday() with newlines for HTML formatting,so we replace them with spaces. - flowOfStudiedTodayStats.value = withCol { sched.studiedToday().replace("\n", " ") } + withCol { flowOfStudiedTodayStats.value = sched.studiedToday().replace("\n", " ") } /** * Checks the current scheduler version and prompts the upgrade dialog if using the legacy version. diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 327f835fbd47..a86430dab2a0 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -2,10 +2,12 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 Ashish Yadav package com.ichi2.anki +import android.view.View import androidx.appcompat.app.AlertDialog import androidx.core.content.edit import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleRegistry +import androidx.lifecycle.viewModelScope import androidx.test.core.app.ActivityScenario import androidx.test.ext.junit.runners.AndroidJUnit4 import anki.backend.BackendError @@ -31,10 +33,14 @@ import com.ichi2.anki.dialogs.utils.input import com.ichi2.anki.dialogs.utils.performPositiveClick import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID import com.ichi2.anki.libanki.DeckId +import com.ichi2.anki.widgets.DeckAdapter import com.ichi2.testutils.BackupManagerTestUtilities import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalForInheritanceCoroutinesApi import kotlinx.coroutines.Job +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.runBlocking import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException @@ -54,12 +60,15 @@ import org.robolectric.shadows.ShadowDialog import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicLong import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertNotSame import kotlin.test.assertSame import kotlin.test.assertTrue +import kotlin.time.Duration.Companion.seconds +import kotlin.time.TimeSource @RunWith(AndroidJUnit4::class) class DeckPickerErrorTest : RobolectricTest() { @@ -721,6 +730,78 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `tapping a deck keeps the focus of a selection that finished first`() { + val tapped = addDeck("Tapped") + val latest = addDeck("Latest") + addNoteToDeck(tapped) + + withDeckPicker(deckCount = 0) { deckPicker -> + val viewModel = deckPicker.viewModel + advanceRobolectricLooperUntil { viewModel.loadDeckCounts?.isCompleted == true } + val row = deckPicker.deckRow(tapped) + + withCollectionThread { + row.performClick() + viewModel.selectDeck(latest) + awaitWithoutMainLooper { viewModel.focusedDeck == latest } + advanceRobolectricLooper() + } + + assertEquals(latest, col.decks.selected()) + assertEquals(latest, viewModel.focusedDeck) + } + } + + @OptIn(ExperimentalForInheritanceCoroutinesApi::class) + @Test + fun `a replaced refresh does not overwrite the newer studied today line`() { + val reads = AtomicInteger() + useBackend { + object : Backend() { + override fun studiedToday(): String = "Studied ${reads.incrementAndGet()}" + } + } + val viewModel = DeckPickerViewModel() + val initialRefresh = viewModel.reloadDeckCounts() + advanceRobolectricLooperUntil { initialRefresh.isCompleted } + + val stale = "Studied ${reads.get() + 1}" + val pausedStale = CountDownLatch(1) + val wroteNewer = CountDownLatch(1) + val original = viewModel.flowOfStudiedTodayStats + val field = viewModel.javaClass.getDeclaredField("flowOfStudiedTodayStats").apply { isAccessible = true } + field.set( + viewModel, + object : MutableStateFlow by original { + override var value: String + get() = original.value + set(value) { + if (value == stale) { + pausedStale.countDown() + wroteNewer.await(2, TimeUnit.SECONDS) + } else { + wroteNewer.countDown() + } + original.value = value + } + }, + ) + try { + withCollectionThread { + val replaced = viewModel.reloadDeckCounts() + assertTrue(pausedStale.await(5, TimeUnit.SECONDS)) + val replacement = viewModel.reloadDeckCounts() + advanceRobolectricLooperUntil { replaced.isCompleted && replacement.isCompleted } + } + + assertEquals("Studied ${reads.get()}", viewModel.flowOfStudiedTodayStats.value) + } finally { + field.set(viewModel, original) + viewModel.viewModelScope.cancel() + } + } + private fun useBackend(backend: () -> Backend) { runBlocking { CollectionManager.discardBackend() } BackendFactory.setOverride { backend() } @@ -735,6 +816,22 @@ class DeckPickerErrorTest : RobolectricTest() { } } + private fun awaitWithoutMainLooper(condition: () -> Boolean) { + val start = TimeSource.Monotonic.markNow() + while (!condition()) { + check(start.elapsedNow() < 5.seconds) { "Timed out waiting for the collection thread" } + Thread.sleep(10) + } + } + + private fun DeckPicker.deckRow(deckId: DeckId): View { + val decks = deckPickerBinding.decks + val adapter = decks.adapter as DeckAdapter + val position = { adapter.currentList.indexOfFirst { it.did == deckId } } + advanceRobolectricLooperUntil { position() >= 0 && decks.findViewHolderForAdapterPosition(position()) != null } + return (decks.findViewHolderForAdapterPosition(position()) as DeckAdapter.ViewHolder).binding.deckLayout + } + private fun assertFailedSelectionSkips( followUp: DeckPickerViewModel.() -> Flow, request: DeckPickerViewModel.(DeckId) -> Job, From 068707370214b904709d9299c76bb572007cdf95 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:04:28 +0530 Subject: [PATCH 22/30] fix(deck-picker): delete a deck once when asked to twice Deleting the same deck twice in quick succession, for example by pressing Shift+Delete twice, deleted it and then showed an error for the second request. The second request now finds the deck gone and does nothing. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 21 ++++++++----- .../com/ichi2/anki/DeckPickerErrorTest.kt | 31 +++++++++++++++++++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 4ebde269621d..15078a9b7c94 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -11,6 +11,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import anki.card_rendering.EmptyCardsReport import anki.collection.OpChanges +import anki.collection.OpChangesWithCount import anki.decks.SetDeckCollapsedRequest import anki.i18n.GeneratedTranslations import anki.sync.SyncStatusResponse @@ -290,19 +291,25 @@ class DeckPickerViewModel : fun deleteDeck(did: DeckId) = launchCatchingTask(reloadOnFailure = true) { progressManager.withProgress(message = DeckPickerProgress.DELETING_DECK) { - val deckName = withCol { decks.getLegacy(did)!!.name } + var deckName: String? = null val changes = undoableOp { - removeDeckAndSelectAdjacent( - deckId = did, - visibleDeckIds = flowOfDeckList.value.data.map { it.did }, - filter = DeckFilters.create(flowOfDeckSearchQuery.value), - ) + deckName = decks.getLegacy(did)?.name + if (deckName == null) { + OpChangesWithCount.getDefaultInstance() + } else { + removeDeckAndSelectAdjacent( + deckId = did, + visibleDeckIds = flowOfDeckList.value.data.map { it.did }, + filter = DeckFilters.create(flowOfDeckSearchQuery.value), + ) + } } + val deletedDeckName = deckName ?: return@withProgress withCol { focusedDeck = decks.selected() } deckDeletedNotification.emit( - DeckDeletionResult(deckName = deckName, cardsDeleted = changes.count), + DeckDeletionResult(deckName = deletedDeckName, cardsDeleted = changes.count), ) } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index a86430dab2a0..5221140e7982 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -41,7 +41,10 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.joinAll import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException import net.ankiweb.rsdroid.BackendFactory @@ -65,6 +68,7 @@ import java.util.concurrent.atomic.AtomicLong import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertNotSame +import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue import kotlin.time.Duration.Companion.seconds @@ -753,6 +757,24 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + fun `deleting a deck twice deletes it once without an error`() = + runTest { + val deckId = addDeck("Deck") + val viewModel = DeckPickerViewModel() + + viewModel.deckDeletedNotification.test { + withQueuedCollectionAccess { + joinAll(viewModel.deleteDeck(deckId), viewModel.deleteDeck(deckId)) + } + + assertEquals("Deck", awaitItem().deckName) + expectNoEvents() + } + assertNull(col.decks.getLegacy(deckId)) + assertTrue(viewModel.pendingErrors.value.isEmpty()) + } + @OptIn(ExperimentalForInheritanceCoroutinesApi::class) @Test fun `a replaced refresh does not overwrite the newer studied today line`() { @@ -816,6 +838,15 @@ class DeckPickerErrorTest : RobolectricTest() { } } + private suspend fun TestScope.withQueuedCollectionAccess(block: suspend () -> T): T { + val previousQueue = CollectionManager.setTestDispatcher(StandardTestDispatcher(testScheduler), useReentrantLock = false) + try { + return block() + } finally { + CollectionManager.setTestDispatcher(previousQueue) + } + } + private fun awaitWithoutMainLooper(condition: () -> Boolean) { val start = TimeSource.Monotonic.markNow() while (!condition()) { From 2f3cfa1b192da2b07cd1036e54f8a45e77b9b780 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:04:28 +0530 Subject: [PATCH 23/30] fix(deck-picker): drop corruption errors that waited for recovery A corruption error that came in while the recovery dialog was open waited for it to close and then opened it again, even after the collection had been repaired. Such an error is now dropped when the recovery dialog closes. Errors that waited for other database dialogs are still shown. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 24 ++++++++++++------- .../ichi2/anki/dialogs/DatabaseErrorDialog.kt | 2 +- .../com/ichi2/anki/DeckPickerErrorTest.kt | 16 +++++++++++++ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 3c4a95c2f81c..e1388f1c9357 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -16,6 +16,7 @@ import android.content.Context import android.content.Intent import android.content.SharedPreferences import android.database.SQLException +import android.database.sqlite.SQLiteDatabaseCorruptException import android.graphics.Color import android.graphics.PixelFormat import android.os.Build @@ -239,6 +240,7 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.coroutines.yield +import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException import net.ankiweb.rsdroid.Translations import net.ankiweb.rsdroid.exceptions.BackendInvalidInputException.BackendCollectionNotOpenException import timber.log.Timber @@ -499,25 +501,28 @@ open class DeckPicker : } private fun findDatabaseRecovery() = - supportFragmentManager.fragments.firstOrNull { - it is DatabaseErrorDialog && it.lifecycle.currentState != Lifecycle.State.DESTROYED + supportFragmentManager.fragments.filterIsInstance().firstOrNull { + it.lifecycle.currentState != Lifecycle.State.DESTROYED } - private suspend fun awaitDatabaseRecovery() { + private suspend fun awaitDatabaseRecovery(): Boolean { + var corruptionPromptClosed = false while (true) { - val recovery = findDatabaseRecovery() ?: return + val recovery = findDatabaseRecovery() ?: return corruptionPromptClosed + if (recovery.requireDialogType() == DatabaseErrorDialogType.DIALOG_LOAD_FAILED) corruptionPromptClosed = true recovery.lifecycle.currentStateFlow.first { it == Lifecycle.State.DESTROYED } yield() } } - private suspend fun withStartedAfterRecovery(block: () -> Unit) { + private suspend fun withStartedAfterRecovery(block: (afterCorruptionPrompt: Boolean) -> Unit) { + var afterCorruptionPrompt = false do { - awaitDatabaseRecovery() + if (awaitDatabaseRecovery()) afterCorruptionPrompt = true val recoveryOpened = withStarted { val opened = findDatabaseRecovery() != null - if (!opened) block() + if (!opened) block(afterCorruptionPrompt) opened } } while (recoveryOpened) @@ -1052,7 +1057,10 @@ open class DeckPicker : viewModel.onError.launchCollectionInLifecycleScope(::onError) observeMessages(viewModel.pendingErrors, viewModel::errorShown) { error -> var dialog: AlertDialog? = null - withStartedAfterRecovery { dialog = showException(error) } + withStartedAfterRecovery { afterCorruptionPrompt -> + val corruption = error is BackendDbCorruptException || error is SQLiteDatabaseCorruptException + if (!afterCorruptionPrompt || !corruption) dialog = showException(error) + } dialog?.awaitDismissal() } viewModel.flowOfPromptUserToUpdateScheduler.launchCollectionInLifecycleScope(::onPromptUserToUpdateScheduler) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt index bf6c5df3ecac..a4ed09ce3683 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt @@ -637,7 +637,7 @@ class DatabaseErrorDialog : AsyncDialogFragment() { override val dialogHandlerMessage get() = ShowDatabaseErrorDialog(requireDialogType()) - private fun requireDialogType() = BundleCompat.getParcelable(requireArguments(), ARG_DIALOG, DatabaseErrorDialogType::class.java)!! + fun requireDialogType() = BundleCompat.getParcelable(requireArguments(), ARG_DIALOG, DatabaseErrorDialogType::class.java)!! private val exceptionData: CustomExceptionData? by lazy { BundleCompat.getParcelable(requireArguments(), ARG_EXCEPTION, CustomExceptionData::class.java) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 5221140e7982..7aeaea1116b8 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -775,6 +775,22 @@ class DeckPickerErrorTest : RobolectricTest() { assertTrue(viewModel.pendingErrors.value.isEmpty()) } + @Test + fun `a corruption found while recovery is open does not reopen it`() = + withCorruptDeletionRecovery { deckPicker, recovery -> + val refresh = deckPicker.viewModel.reloadDeckCounts() + advanceRobolectricLooperUntil { refresh.isCompleted } + advanceRobolectricLooper() + + recovery.dismiss() + advanceRobolectricLooperUntil { + deckPicker.viewModel.pendingErrors.value + .isEmpty() + } + + assertTrue(deckPicker.supportFragmentManager.fragments.none { it is DatabaseErrorDialog }) + } + @OptIn(ExperimentalForInheritanceCoroutinesApi::class) @Test fun `a replaced refresh does not overwrite the newer studied today line`() { From b4aac1e2150cb03b90eccb11054f3f492ba55d16 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:04:28 +0530 Subject: [PATCH 24/30] fix(deck-picker): show side panel errors through the error queue On tablets the side panel showed its own refresh errors straight away, so a failure that it and the deck list both hit was shown up to three times on each return to the app, without waiting for the screen to start or for database recovery. The panel now passes them to the deck picker's queue. Assisted-by: Claude Opus 5.5 --- .../com/ichi2/anki/StudyOptionsFragment.kt | 12 +++++- .../anki/deckpicker/DeckPickerViewModel.kt | 2 +- .../com/ichi2/anki/DeckPickerErrorTest.kt | 37 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt index 590d9fa9ccaf..c073c8758392 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt @@ -57,6 +57,7 @@ import com.ichi2.anki.utils.ext.launchCollectionInViewLifecycleScope import com.ichi2.anki.utils.ext.setFragmentResultListener import com.ichi2.anki.utils.ext.showDialogFragment import com.ichi2.ui.CollectionMediaImageGetter +import kotlinx.coroutines.CancellationException import org.intellij.lang.annotations.Language import timber.log.Timber @@ -356,7 +357,16 @@ class StudyOptionsFragment : fun refreshInterface() { Timber.d("Refreshing StudyOptionsFragment") - launchCatchingViewTask { viewModel.refreshData() } + launchCatchingViewTask { + try { + viewModel.refreshData() + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + val deckPicker = activity as? DeckPicker ?: throw e + deckPicker.viewModel.reportError(e) + } + } } private fun rebuildUi(state: StudyOptionsState) { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 15078a9b7c94..ae0620e3679e 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -277,7 +277,7 @@ class DeckPickerViewModel : } } - private suspend fun reportError(e: Exception) = + suspend fun reportError(e: Exception) = withContext(Dispatchers.Main.immediate) { val alreadyPending = errorQueue.messages.value.any { it.message.javaClass == e.javaClass && it.message.message == e.message } if (!alreadyPending) errorQueue.enqueue(e) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 7aeaea1116b8..a7cda5f1f9d5 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -775,6 +775,43 @@ class DeckPickerErrorTest : RobolectricTest() { assertTrue(viewModel.pendingErrors.value.isEmpty()) } + @Test + @Config(qualifiers = "xlarge") + fun `a failing refresh shows one error with the side panel`() { + var failing = false + useBackend { + object : Backend() { + override fun deckTree(now: Long): DeckTreeNode = if (failing) throw failure else super.deckTree(now) + } + } + addNoteToDeck(DEFAULT_DECK_ID) + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + scenario.onActivity { deckPicker -> + deckPicker.viewModel.awaitStartup() + advanceRobolectricLooperUntil { deckPicker.fragment?.shownDeckId == DEFAULT_DECK_ID } + } + val shownBefore = ShadowDialog.getShownDialogs().size + failing = true + scenario.moveToState(Lifecycle.State.CREATED) + scenario.moveToState(Lifecycle.State.RESUMED) + scenario.onActivity { deckPicker -> + advanceRobolectricLooperUntil { + deckPicker.viewModel.pendingErrors.value + .isNotEmpty() + } + assertErrorShown() + advanceRobolectricLooper() + + assertEquals(1, ShadowDialog.getShownDialogs().drop(shownBefore).count { it.isShowing }) + } + } + } + @Test fun `a corruption found while recovery is open does not reopen it`() = withCorruptDeletionRecovery { deckPicker, recovery -> From 23dbade1f89193e34d0bbbd8a5b2ac6675d307db Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:04:28 +0530 Subject: [PATCH 25/30] fix(deck-picker): hide the spinner when the deck list fails to load Only a successful refresh hid the loading spinner, so after a failed first load it kept spinning over an empty screen. A failed refresh now hides it too. Assisted-by: Claude Opus 5.5 --- .../anki/deckpicker/DeckPickerViewModel.kt | 62 ++++++++++--------- .../com/ichi2/anki/DeckPickerErrorTest.kt | 27 ++++++++ 2 files changed, 60 insertions(+), 29 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index ae0620e3679e..5ded79358cd3 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -62,6 +62,7 @@ import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import net.ankiweb.rsdroid.BackendException.BackendDbException.BackendDbCorruptException @@ -521,40 +522,43 @@ class DeckPickerViewModel : loadDeckCounts?.cancel() val loadDeckCounts = launchCatchingTask(Dispatchers.IO) { - Timber.d("Refreshing deck list") - withCol { - val deckDueTree = sched.deckDueTree() - val collectionHasNoCards = isEmpty - decks.current() - dueTree = deckDueTree - flowOfCollectionHasNoCards.value = collectionHasNoCards - } + try { + Timber.d("Refreshing deck list") + withCol { + val deckDueTree = sched.deckDueTree() + val collectionHasNoCards = isEmpty + decks.current() + dueTree = deckDueTree + flowOfCollectionHasNoCards.value = collectionHasNoCards + } - // TODO: This is in the wrong place - // Backend returns studiedToday() with newlines for HTML formatting,so we replace them with spaces. - withCol { flowOfStudiedTodayStats.value = sched.studiedToday().replace("\n", " ") } + // TODO: This is in the wrong place + // Backend returns studiedToday() with newlines for HTML formatting,so we replace them with spaces. + withCol { flowOfStudiedTodayStats.value = sched.studiedToday().replace("\n", " ") } - /** - * Checks the current scheduler version and prompts the upgrade dialog if using the legacy version. - * Ensures the dialog is only shown once per collection load, even if [updateDeckList()] is called multiple times. - */ - val currentSchedulerVersion = withCol { config.schedulerVersion } + /** + * Checks the current scheduler version and prompts the upgrade dialog if using the legacy version. + * Ensures the dialog is only shown once per collection load, even if [updateDeckList()] is called multiple times. + */ + val currentSchedulerVersion = withCol { config.schedulerVersion } - if (currentSchedulerVersion == 1L && schedulerUpgradeDialogShownForVersion != 1L) { - schedulerUpgradeDialogShownForVersion = 1L - flowOfPromptUserToUpdateScheduler.emit(Unit) - } else { - schedulerUpgradeDialogShownForVersion = currentSchedulerVersion - } + if (currentSchedulerVersion == 1L && schedulerUpgradeDialogShownForVersion != 1L) { + schedulerUpgradeDialogShownForVersion = 1L + flowOfPromptUserToUpdateScheduler.emit(Unit) + } else { + schedulerUpgradeDialogShownForVersion = currentSchedulerVersion + } - // TODO: This is in the wrong place - // current deck may have changed - withCol { focusedDeck = decks.current().id } - refreshUndoMenuState() + // TODO: This is in the wrong place + // current deck may have changed + withCol { focusedDeck = decks.current().id } + refreshUndoMenuState() - // Reopening can leave the counts and selection unchanged after a skipped display refresh. - flowOfRefreshDeckList.emit(Unit) - flowOfDecksReloaded.emit(Unit) + // Reopening can leave the counts and selection unchanged after a skipped display refresh. + flowOfRefreshDeckList.emit(Unit) + } finally { + if (isActive) flowOfDecksReloaded.emit(Unit) + } } this.loadDeckCounts = loadDeckCounts return loadDeckCounts diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index a7cda5f1f9d5..9e2368af8dd3 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -5,6 +5,7 @@ package com.ichi2.anki import android.view.View import androidx.appcompat.app.AlertDialog import androidx.core.content.edit +import androidx.core.view.isVisible import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleRegistry import androidx.lifecycle.viewModelScope @@ -23,6 +24,7 @@ import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.crashreporting.CrashReporter import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.databinding.IncludeAnkiProgressBinding import com.ichi2.anki.deckpicker.DeckPickerViewModel import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.DeckPickerContextMenu.DeckPickerContextMenuOption @@ -66,6 +68,7 @@ import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicLong import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNotSame import kotlin.test.assertNull @@ -775,6 +778,30 @@ class DeckPickerErrorTest : RobolectricTest() { assertTrue(viewModel.pendingErrors.value.isEmpty()) } + @Test + fun `a failed first deck list load stops the loading spinner`() { + useBackend { + object : Backend() { + override fun deckTree(now: Long): DeckTreeNode = throw failure + } + } + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + scenario.onActivity { deckPicker -> + deckPicker.viewModel.awaitStartup() + assertErrorShown() + advanceRobolectricLooperUntil { deckPicker.viewModel.loadDeckCounts?.isCompleted == true } + advanceRobolectricLooper() + + assertFalse(IncludeAnkiProgressBinding.bind(deckPicker.deckPickerBinding.root).progressBar.isVisible) + } + } + } + @Test @Config(qualifiers = "xlarge") fun `a failing refresh shows one error with the side panel`() { From 5693672b22c3bd4a2637a28e58c19b913a9e464e Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:29:42 +0530 Subject: [PATCH 26/30] fix(deck-picker): report menu refresh errors through the error queue The menu refresh after an action ran in the activity. If it failed after deleting a deck the app crashed, and after creating a deck the error dialog was lost on rotation. The refresh now runs in the ViewModel and reports to the error queue. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 4 +- .../anki/deckpicker/DeckPickerViewModel.kt | 2 + ...ckerPostDeletionMenuErrorRegressionTest.kt | 98 +++++++++++++++++++ 3 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerPostDeletionMenuErrorRegressionTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index e1388f1c9357..1e789a0d0424 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -1334,7 +1334,7 @@ open class DeckPicker : // into CollectionManager, and awaiting that. createMenuJob = launchCatchingTask { - viewModel.refreshMenuState() + viewModel.updateMenuState().join() updateSearchVisibilityFromState(menu) restoreSearchState(menu) updateDeckRelatedMenuItems(menu) @@ -2624,7 +2624,7 @@ open class DeckPicker : changes: OpChanges, handler: Any?, ) { - lifecycleScope.launch { viewModel.refreshMenuState() } + viewModel.updateMenuState() if (changes.studyQueues && handler !== this && handler !== viewModel) { if (!activityPaused) { // No need to update while the activity is paused, because `onResume` calls `refreshState` that calls `updateDeckList`. diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 5ded79358cd3..4d4a3a17541b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -775,6 +775,8 @@ class DeckPickerViewModel : */ val optionsMenuState: OptionsMenuState? get() = flowOfOptionsMenuState.value + fun updateMenuState(): Job = launchCatchingTask { refreshMenuState() } + /** * Recomputes the full options menu state from the current collection. */ diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerPostDeletionMenuErrorRegressionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerPostDeletionMenuErrorRegressionTest.kt new file mode 100644 index 000000000000..77ba2d84dd94 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerPostDeletionMenuErrorRegressionTest.kt @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +import androidx.appcompat.app.AlertDialog +import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.backend.backendError +import anki.collection.OpChangesWithCount +import anki.collection.UndoStatus +import com.ichi2.anki.dialogs.utils.input +import com.ichi2.anki.dialogs.utils.performPositiveClick +import com.ichi2.testutils.ext.menu +import kotlinx.coroutines.runBlocking +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.shadows.ShadowDialog +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.test.assertEquals +import kotlin.test.assertNull + +@RunWith(AndroidJUnit4::class) +class DeckPickerPostDeletionMenuErrorRegressionTest : RobolectricTest() { + @After + fun resetBackend() = BackendFactory.setOverride(null) + + @Test + fun `backend failure refreshing the menu after deck deletion is handled`() { + val failUndoReads = AtomicBoolean(false) + val failure = BackendIoException(backendError { message = "Cannot read undo status after deletion" }) + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun removeDecks(dids: Iterable): OpChangesWithCount = + super.removeDecks(dids).also { failUndoReads.set(true) } + + override fun getUndoStatus(): UndoStatus = if (failUndoReads.getAndSet(false)) throw failure else super.getUndoStatus() + } + } + val target = addDeck("Delete this deck") + withDeckPicker(deckCount = 0) { picker -> + advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true } + throwOnShowError = false + try { + val deletion = picker.deleteDeck(target) + advanceRobolectricLooperUntil { + deletion.isCompleted && picker.viewModel.loadDeckCounts?.isCompleted == true + } + advanceRobolectricLooper() + assertEquals(1, picker.viewModel.pendingErrors.value.size, "The menu refresh error must be retained for display") + assertNull(col.decks.getLegacy(target), "The write succeeded before the menu refresh failed") + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } finally { + failUndoReads.set(false) + } + } + } + + @Test + fun `menu refresh after creating a deck retains its backend error`() { + val failUndoRead = AtomicBoolean(false) + val failure = BackendIoException(backendError { message = "Cannot refresh the created deck menu" }) + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun getUndoStatus(): UndoStatus = if (failUndoRead.getAndSet(false)) throw failure else super.getUndoStatus() + } + } + addDeck("Existing") + withDeckPicker(deckCount = 0) { picker -> + advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true } + throwOnShowError = false + picker.showCreateDeckDialog() + (ShadowDialog.getLatestDialog() as AlertDialog).apply { + input = "Created" + performPositiveClick() + } + advanceRobolectricLooperUntil { + col.decks.byName("Created")?.id == col.decks.selected() && + picker.viewModel.loadDeckCounts?.isCompleted == true + } + advanceRobolectricLooper() + // Creation invalidates the menu. Recreate it as the framework does, and fail + // only the read performed by this follow-up refresh after selection succeeds. + val menu = picker.menu() + menu.clear() + failUndoRead.set(true) + picker.onCreateOptionsMenu(menu) + advanceRobolectricLooperUntil { picker.createMenuJob?.isCompleted == true } + advanceRobolectricLooper() + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + assertEquals(1, picker.viewModel.pendingErrors.value.size, "The created-deck follow-up must retain its error") + } + } +} From c1539ca11dbc981cc6ab25755cef05b9b9f7663c Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:29:46 +0530 Subject: [PATCH 27/30] fix(deck-picker): read context menu details with the selection Long press and right click selected the deck, then read its name and flags in the activity, so a failed read crashed the app. The ViewModel now reads them after selecting and reports a failure like other deck actions. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 8 ++- .../DeckPickerMenuContentProvider.kt | 24 ++++--- .../anki/deckpicker/DeckPickerViewModel.kt | 17 ++++- .../anki/dialogs/DeckPickerContextMenu.kt | 29 +++++---- ...rContextMenuMetadataErrorRegressionTest.kt | 64 +++++++++++++++++++ .../deckpicker/DeckPickerViewModelTest.kt | 2 +- 6 files changed, 110 insertions(+), 34 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerContextMenuMetadataErrorRegressionTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 1e789a0d0424..46d8b51f3d33 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -485,16 +485,18 @@ open class DeckPicker : } } - private suspend fun showDeckPickerContextMenu(deckId: DeckId) { - val menu = DeckPickerContextMenu.newInstance(deckId) + private fun showDeckPickerContextMenu(request: DeckPickerViewModel.ContextMenuRequest) { + val menu = DeckPickerContextMenu.newInstance(request.deckId, request.deckName, request.isFiltered, request.hasBuried) CardBrowser.clearLastDeckId() showDialogFragment(menu) } - private suspend fun showDeckPickerRightClickContextMenu(request: DeckPickerViewModel.RightClickMenuRequest) { + private fun showDeckPickerRightClickContextMenu(request: DeckPickerViewModel.RightClickMenuRequest) { DeckPickerMenuContentProvider.show( deckPicker = this@DeckPicker, deckId = request.deckId, + isDynamic = request.isFiltered, + hasBuriedInDeck = request.hasBuried, x = request.x, y = request.y, ) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/contextmenu/DeckPickerMenuContentProvider.kt b/AnkiDroid/src/main/java/com/ichi2/anki/contextmenu/DeckPickerMenuContentProvider.kt index 903d1c4ebb22..58739d75f335 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/contextmenu/DeckPickerMenuContentProvider.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/contextmenu/DeckPickerMenuContentProvider.kt @@ -5,7 +5,6 @@ package com.ichi2.anki.contextmenu import android.view.Menu import android.view.MenuItem -import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.DeckPicker import com.ichi2.anki.dialogs.DeckPickerContextMenu import com.ichi2.anki.dialogs.DeckPickerContextMenuResult @@ -50,25 +49,24 @@ class DeckPickerMenuContentProvider( companion object { /** - * Builds a [DeckPickerMenuContentProvider] for [deckId] (reading the dynamic / - * has-buried flags from the collection) and shows it as a popup anchored at - * ([x], [y]) on the deck picker's recycler view. + * Builds a [DeckPickerMenuContentProvider] for [deckId] and shows it as a popup + * anchored at ([x], [y]) on the deck picker's recycler view. */ - suspend fun show( + fun show( deckPicker: DeckPicker, deckId: DeckId, + isDynamic: Boolean, + hasBuriedInDeck: Boolean, x: Float, y: Float, ) { val provider = - withCol { - DeckPickerMenuContentProvider( - id = deckId, - isDynamic = decks.isFiltered(deckId), - hasBuriedInDeck = sched.haveBuried(), - deckPicker = deckPicker, - ) - } + DeckPickerMenuContentProvider( + id = deckId, + isDynamic = isDynamic, + hasBuriedInDeck = hasBuriedInDeck, + deckPicker = deckPicker, + ) val anchorParent = deckPicker.deckPickerBinding.deckPickerContent val target = deckPicker.deckPickerBinding.decks MouseContextMenuHandler(viewGroup = anchorParent, menuContentProvider = provider) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 4d4a3a17541b..3b02de64efc0 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -421,12 +421,21 @@ class DeckPickerViewModel : flowOfNavigate.emit(NoteEditorDestination.AddNote(deckId)) } - val flowOfShowContextMenu = MutableSharedFlow(extraBufferCapacity = 1) + data class ContextMenuRequest( + val deckId: DeckId, + val deckName: String, + val isFiltered: Boolean, + val hasBuried: Boolean, + ) + + val flowOfShowContextMenu = MutableSharedFlow(extraBufferCapacity = 1) data class RightClickMenuRequest( val deckId: DeckId, val x: Float, val y: Float, + val isFiltered: Boolean, + val hasBuried: Boolean, ) val flowOfShowRightClickContextMenu = MutableSharedFlow(extraBufferCapacity = 1) @@ -434,7 +443,8 @@ class DeckPickerViewModel : fun requestContextMenu(deckId: DeckId) = launchCatchingTask { markDeckSelected(deckId) - flowOfShowContextMenu.emit(deckId) + val request = withCol { ContextMenuRequest(deckId, decks.name(deckId), decks.isFiltered(deckId), sched.haveBuried()) } + flowOfShowContextMenu.emit(request) } fun requestRightClickContextMenu( @@ -443,7 +453,8 @@ class DeckPickerViewModel : y: Float, ) = launchCatchingTask { markDeckSelected(deckId) - flowOfShowRightClickContextMenu.emit(RightClickMenuRequest(deckId, x, y)) + val request = withCol { RightClickMenuRequest(deckId, x, y, decks.isFiltered(deckId), sched.haveBuried()) } + flowOfShowRightClickContextMenu.emit(request) } /** diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckPickerContextMenu.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckPickerContextMenu.kt index 850022f79f6b..d8e82daef5d4 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckPickerContextMenu.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckPickerContextMenu.kt @@ -11,7 +11,6 @@ import androidx.appcompat.app.AlertDialog import androidx.fragment.app.FragmentActivity import androidx.fragment.app.FragmentManager import com.ichi2.anki.CollectionManager.TR -import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.CommonString import com.ichi2.anki.analytics.AnalyticsDialogFragment import com.ichi2.anki.compat.requireSerializableCompat @@ -91,20 +90,22 @@ class DeckPickerContextMenu : AnalyticsDialogFragment() { companion object { /** - * Builds a [DeckPickerContextMenu] for [deckId], reading the deck's name and - * the dynamic / has-buried flags from the collection. + * Builds a [DeckPickerContextMenu] for [deckId]. */ - suspend fun newInstance(deckId: DeckId): DeckPickerContextMenu = - withCol { - DeckPickerContextMenu().apply { - arguments = - Bundle().apply { - putLong(ARG_DECK_ID, deckId) - putString(ARG_DECK_NAME, decks.name(deckId)) - putBoolean(ARG_DECK_IS_DYN, decks.isFiltered(deckId)) - putBoolean(ARG_DECK_HAS_BURIED_IN_DECK, sched.haveBuried()) - } - } + fun newInstance( + deckId: DeckId, + deckName: String, + isFiltered: Boolean, + hasBuried: Boolean, + ): DeckPickerContextMenu = + DeckPickerContextMenu().apply { + arguments = + Bundle().apply { + putLong(ARG_DECK_ID, deckId) + putString(ARG_DECK_NAME, deckName) + putBoolean(ARG_DECK_IS_DYN, isFiltered) + putBoolean(ARG_DECK_HAS_BURIED_IN_DECK, hasBuried) + } } @VisibleForTesting diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerContextMenuMetadataErrorRegressionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerContextMenuMetadataErrorRegressionTest.kt new file mode 100644 index 000000000000..b5ebc1abf3d2 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerContextMenuMetadataErrorRegressionTest.kt @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.backend.backendError +import com.google.protobuf.ByteString +import com.ichi2.anki.dialogs.DeckPickerContextMenu +import kotlinx.coroutines.runBlocking +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 kotlin.test.assertEquals +import kotlin.test.assertFalse + +@RunWith(AndroidJUnit4::class) +class DeckPickerContextMenuMetadataErrorRegressionTest : RobolectricTest() { + @After + fun resetBackend() = BackendFactory.setOverride(null) + + @Test + fun `long press handles a backend read failure after selecting the deck`() = checkMetadataFailure(rightClick = false) + + @Test + fun `right click handles a backend read failure after selecting the deck`() = checkMetadataFailure(rightClick = true) + + private fun checkMetadataFailure(rightClick: Boolean) { + var failReads = false + val failure = BackendIoException(backendError { message = "Cannot read context menu deck metadata" }) + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun getDeckLegacy(did: Long): ByteString = if (failReads) throw failure else super.getDeckLegacy(did) + } + } + val target = addDeck("Target") + withDeckPicker(deckCount = 0) { picker -> + advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true } + throwOnShowError = false + failReads = true + try { + val request = + if (rightClick) { + picker.viewModel.requestRightClickContextMenu(target, 1f, 1f) + } else { + picker.viewModel.requestContextMenu(target) + } + advanceRobolectricLooperUntil { request.isCompleted } + advanceRobolectricLooper() + assertEquals(target, col.decks.selected(), "Selection succeeds before the metadata read fails") + assertEquals(1, picker.viewModel.pendingErrors.value.size) + assertFalse(picker.supportFragmentManager.fragments.any { it is DeckPickerContextMenu }) + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + // RobolectricTest's exception rule also requires that the UI flow handler did + // not let the same backend failure escape as an uncaught coroutine exception. + } finally { + failReads = false + } + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt index 7130ff2cd468..206ed19bd2b3 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt @@ -450,7 +450,7 @@ class DeckPickerViewModelTest : RobolectricTest() { viewModel.flowOfShowContextMenu.test { viewModel.requestContextMenu(deckId).join() - assertEquals(deckId, awaitItem()) + assertEquals(deckId, awaitItem().deckId) } } } From dc727add350f50c4ecc766b8bfc6224f33bb8004 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:29:49 +0530 Subject: [PATCH 28/30] fix(deck-picker): keep deck selection errors until they are seen Tapping a deck or pressing S selected it in the activity, so its error dialog was lost on rotation, and a failure that finished after rotating was dropped. The selection now runs in the ViewModel and reports to the error queue. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 11 +- .../anki/deckpicker/DeckPickerViewModel.kt | 15 ++ ...owSelectionErrorLifecycleRegressionTest.kt | 173 ++++++++++++++++++ 3 files changed, 194 insertions(+), 5 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRowSelectionErrorLifecycleRegressionTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 46d8b51f3d33..84acd147b974 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -2336,11 +2336,12 @@ open class DeckPicker : } val study = - withCol { - decks.select(did) - viewModel.focusedDeck = did - currentDeckStudyState() - } + viewModel + .withColAsync { + decks.select(did) + viewModel.focusedDeck = did + currentDeckStudyState() + }.await() ?: return deckListAdapter.updateSelectedDeck(did) // Also forget the last deck used by the Browser CardBrowser.clearLastDeckId() diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 3b02de64efc0..6a9e56b71b9c 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -31,6 +31,7 @@ import com.ichi2.anki.configureRenderingMode import com.ichi2.anki.ioDispatcher import com.ichi2.anki.launchCatchingIO import com.ichi2.anki.libanki.CardId +import com.ichi2.anki.libanki.Collection import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID import com.ichi2.anki.libanki.DeckId import com.ichi2.anki.libanki.Decks @@ -52,8 +53,10 @@ import com.ichi2.anki.utils.MessageQueue import com.ichi2.anki.utils.MessageQueue.MessageId import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job +import kotlinx.coroutines.async import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted @@ -278,6 +281,18 @@ class DeckPickerViewModel : } } + fun withColAsync(block: Collection.() -> T): Deferred = + viewModelScope.async { + try { + withCol(block) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + reportError(e) + null + } + } + suspend fun reportError(e: Exception) = withContext(Dispatchers.Main.immediate) { val alreadyPending = errorQueue.messages.value.any { it.message.javaClass == e.javaClass && it.message.message == e.message } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRowSelectionErrorLifecycleRegressionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRowSelectionErrorLifecycleRegressionTest.kt new file mode 100644 index 000000000000..35c57747efd6 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerRowSelectionErrorLifecycleRegressionTest.kt @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +import android.view.KeyEvent +import androidx.appcompat.app.AlertDialog +import androidx.core.content.edit +import androidx.test.core.app.ActivityScenario +import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.backend.backendError +import anki.collection.OpChanges +import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.deckpicker.DeckPickerViewModel +import com.ichi2.anki.widgets.DeckAdapter +import com.ichi2.testutils.BackupManagerTestUtilities +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.runBlocking +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.shadows.ShadowDialog +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNotSame +import kotlin.test.assertSame +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class DeckPickerRowSelectionErrorLifecycleRegressionTest : RobolectricTest() { + @After + fun resetBackend() = BackendFactory.setOverride(null) + + @Test + fun `unacknowledged deck row selection error survives recreation`() = checkSelectionError(studyShortcut = false) + + @Test + fun `unacknowledged study shortcut selection error survives recreation`() = checkSelectionError(studyShortcut = true) + + private fun checkSelectionError(studyShortcut: Boolean) { + var failSelection = false + val failure = BackendIoException(backendError { message = "Cannot select this deck" }) + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges = if (failSelection) throw failure else super.setCurrentDeck(did) + } + } + val target = addDeck("Target") + addNoteToDeck(target) + ensureCollectionLoadIsSynchronous() + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + lateinit var originalDialog: AlertDialog + scenario.onActivity { picker -> + val decks = picker.deckPickerBinding.decks + val adapter = decks.adapter as DeckAdapter + val position = { adapter.currentList.indexOfFirst { it.did == target } } + advanceRobolectricLooperUntil { + picker.viewModel.loadDeckCounts?.isCompleted == true && position() >= 0 && + decks.findViewHolderForAdapterPosition(position()) != null + } + val row = (decks.findViewHolderForAdapterPosition(position()) as DeckAdapter.ViewHolder).binding.deckLayout + failSelection = true + if (studyShortcut) { + picker.onKeyUp(KeyEvent.KEYCODE_S, KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_S)) + } else { + row.performClick() + } + advanceRobolectricLooperUntil { (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true } + failSelection = false + originalDialog = ShadowDialog.getLatestDialog() as AlertDialog + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } + try { + scenario.recreate() + scenario.onActivity { picker -> + advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true } + advanceRobolectricLooper() + val restoredDialog = ShadowDialog.getLatestDialog() as? AlertDialog + assertNotNull(restoredDialog, "The unacknowledged selection error must be shown again") + assertNotSame(originalDialog, restoredDialog, "The previous activity's window does not restore the error") + assertTrue(restoredDialog.isShowing) + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } + } finally { + failSelection = false + originalDialog.dismiss() + } + } + } + + @Test + fun `selection failure finishing after recreation reaches the retained error queue`() { + val selectionStarted = CountDownLatch(1) + val releaseSelection = CountDownLatch(1) + val gateSelection = AtomicBoolean(false) + val failure = BackendIoException(backendError { message = "Selection failed after recreation" }) + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun setCurrentDeck(did: Long): OpChanges { + if (gateSelection.compareAndSet(true, false)) { + selectionStarted.countDown() + check(releaseSelection.await(20, TimeUnit.SECONDS)) { "Timed out releasing selection" } + throw failure + } + return super.setCurrentDeck(did) + } + } + } + val target = addDeck("Target") + addNoteToDeck(target) + ensureCollectionLoadIsSynchronous() + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + var restoreQueue: (() -> Unit)? = null + lateinit var initialRefresh: Job + lateinit var retainedModel: DeckPickerViewModel + try { + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + scenario.onActivity { picker -> + val decks = picker.deckPickerBinding.decks + val adapter = decks.adapter as DeckAdapter + val position = { adapter.currentList.indexOfFirst { it.did == target } } + advanceRobolectricLooperUntil { + picker.viewModel.loadDeckCounts?.isCompleted == true && position() >= 0 && + decks.findViewHolderForAdapterPosition(position()) != null + } + val row = (decks.findViewHolderForAdapterPosition(position()) as DeckAdapter.ViewHolder).binding.deckLayout + retainedModel = picker.viewModel + initialRefresh = assertNotNull(picker.viewModel.loadDeckCounts) + val previousQueue = CollectionManager.setTestDispatcher(Dispatchers.IO.limitedParallelism(1), useReentrantLock = false) + restoreQueue = { CollectionManager.setTestDispatcher(previousQueue) } + gateSelection.set(true) + row.performClick() + } + assertTrue(selectionStarted.await(5, TimeUnit.SECONDS), "Selection must be inside the backend before recreation") + // The old Activity's coroutine is cancelled while its native call is still running. + scenario.recreate() + releaseSelection.countDown() + scenario.onActivity { picker -> + assertSame(retainedModel, picker.viewModel) + advanceRobolectricLooperUntil { + picker.viewModel.loadDeckCounts !== initialRefresh && picker.viewModel.loadDeckCounts?.isCompleted == true + } + advanceRobolectricLooper() + assertEquals( + 1, + picker.viewModel.pendingErrors.value.size, + "The real backend failure must outlive the cancelled UI caller", + ) + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } + } + } finally { + releaseSelection.countDown() + restoreQueue?.invoke() + } + } +} From c8f7856c61fd622eaf1b9faf38e514cb53752cc7 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:29:52 +0530 Subject: [PATCH 29/30] fix(deck-picker): keep side panel errors until they are seen On tablets the panel showed Rebuild and Empty errors itself, so they were lost on rotation, and a refresh that failed after the panel was replaced lost its error. Unbury crashed on a failure. They now run in the deck list's ViewModel and report to its error queue. Assisted-by: Claude Opus 5.5 --- .../com/ichi2/anki/StudyOptionsFragment.kt | 27 ++-- .../com/ichi2/anki/StudyOptionsViewModel.kt | 10 +- .../anki/deckpicker/DeckPickerViewModel.kt | 2 + .../com/ichi2/anki/DeckPickerErrorTest.kt | 50 ++++++ ...ilteredPaneErrorLifecycleRegressionTest.kt | 148 ++++++++++++++++++ .../ichi2/anki/StudyOptionsViewModelTest.kt | 4 +- 6 files changed, 222 insertions(+), 19 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFilteredPaneErrorLifecycleRegressionTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt index c073c8758392..4ae70133a261 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt @@ -57,7 +57,7 @@ import com.ichi2.anki.utils.ext.launchCollectionInViewLifecycleScope import com.ichi2.anki.utils.ext.setFragmentResultListener import com.ichi2.anki.utils.ext.showDialogFragment import com.ichi2.ui.CollectionMediaImageGetter -import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope import org.intellij.lang.annotations.Language import timber.log.Timber @@ -257,6 +257,15 @@ class StudyOptionsFragment : totalCardsCount = studyOptionsView.findViewById(R.id.studyoptions_total_count) } + private fun launchReportingTask(block: suspend CoroutineScope.() -> Unit) { + val deckPicker = activity as? DeckPicker + if (deckPicker == null) { + launchCatchingTask(block = block) + } else { + deckPicker.viewModel.launchReportingErrors(block) + } + } + private fun showCustomStudyContextMenu() { val dialog = CustomStudyDialog.createInstance(deckId = viewModel.selectedDeckId) requireActivity().showDialogFragment(dialog) @@ -292,18 +301,18 @@ class StudyOptionsFragment : } R.id.action_unbury -> { Timber.i("StudyOptionsFragment:: unbury button pressed") - viewModel.unbury() + launchReportingTask { viewModel.unbury() } item.isVisible = false return true } R.id.action_rebuild -> { Timber.i("StudyOptionsFragment:: rebuild cram deck button pressed") - launchCatchingTask { viewModel.rebuildCram() } + launchReportingTask { viewModel.rebuildCram() } return true } R.id.action_empty -> { Timber.i("StudyOptionsFragment:: empty cram deck button pressed") - launchCatchingTask { viewModel.emptyCram() } + launchReportingTask { viewModel.emptyCram() } return true } else -> return false @@ -358,13 +367,11 @@ class StudyOptionsFragment : fun refreshInterface() { Timber.d("Refreshing StudyOptionsFragment") launchCatchingViewTask { - try { + val deckPicker = activity as? DeckPicker + if (deckPicker == null) { viewModel.refreshData() - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - val deckPicker = activity as? DeckPicker ?: throw e - deckPicker.viewModel.reportError(e) + } else { + deckPicker.viewModel.launchReportingErrors { viewModel.refreshData() }.join() } } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsViewModel.kt index 7f8421df1998..489ca303883f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsViewModel.kt @@ -3,7 +3,6 @@ package com.ichi2.anki import androidx.lifecycle.ViewModel -import androidx.lifecycle.viewModelScope import anki.collection.OpChanges import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.CollectionManager.withOpenColOrNull @@ -15,10 +14,8 @@ import com.ichi2.anki.progress.ProgressManager import com.ichi2.anki.study.DeckKind import com.ichi2.anki.study.StudyAvailability import com.ichi2.anki.study.currentDeckStudyState -import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.launch import timber.log.Timber import java.io.File @@ -72,10 +69,9 @@ class StudyOptionsViewModel : } } - fun unbury(): Job = - viewModelScope.launch { - undoableOp { sched.unburyDeck(selectedDeckId) } - } + suspend fun unbury() { + undoableOp { sched.unburyDeck(selectedDeckId) } + } /** * See https://github.com/ankitects/anki/blob/b05c9d15986ab4e33daa2a47a947efb066bb69b6/qt/aqt/overview.py#L226-L272 diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt index 6a9e56b71b9c..5453d4a7f393 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt @@ -293,6 +293,8 @@ class DeckPickerViewModel : } } + fun launchReportingErrors(block: suspend CoroutineScope.() -> Unit): Job = launchCatchingTask(block = block) + suspend fun reportError(e: Exception) = withContext(Dispatchers.Main.immediate) { val alreadyPending = errorQueue.messages.value.any { it.message.javaClass == e.javaClass && it.message.message == e.message } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 9e2368af8dd3..287af000c333 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -4,6 +4,7 @@ package com.ichi2.anki import android.view.View import androidx.appcompat.app.AlertDialog +import androidx.appcompat.view.menu.MenuBuilder import androidx.core.content.edit import androidx.core.view.isVisible import androidx.lifecycle.Lifecycle @@ -839,6 +840,24 @@ class DeckPickerErrorTest : RobolectricTest() { } } + @Test + @Config(qualifiers = "xlarge") + fun `a failing side panel unbury keeps its error across recreation`() = + assertErrorSurvivesRecreation( + backend = { failNext -> + object : Backend() { + override fun unburyDeck( + deckId: Long, + mode: UnburyDeckRequest.Mode, + ): OpChanges = if (failNext.getAndSet(false)) throw failure else super.unburyDeck(deckId, mode) + } + }, + ) { failNext -> + advanceRobolectricLooperUntil { fragment?.shownDeckId == DEFAULT_DECK_ID } + failNext.set(true) + fragment!!.onMenuItemSelected(MenuBuilder(this).add(0, R.id.action_unbury, 0, "Unbury")) + } + @Test fun `a corruption found while recovery is open does not reopen it`() = withCorruptDeletionRecovery { deckPicker, recovery -> @@ -1018,6 +1037,37 @@ class DeckPickerErrorTest : RobolectricTest() { deckId: DeckId, ) = supportFragmentManager.setDeckPickerContextMenuResult(DeckPickerContextMenuResult(deckId, option)) + private fun assertErrorSurvivesRecreation( + backend: (failNext: AtomicBoolean) -> Backend, + trigger: DeckPicker.(failNext: AtomicBoolean) -> Unit, + ) { + val failNext = AtomicBoolean(false) + useBackend { backend(failNext) } + addNoteToDeck(DEFAULT_DECK_ID) + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + lateinit var firstDialog: AlertDialog + scenario.onActivity { deckPicker -> + deckPicker.viewModel.awaitStartup() + deckPicker.trigger(failNext) + assertErrorShown() + firstDialog = ShadowDialog.getLatestDialog() as AlertDialog + } + scenario.recreate() + scenario.onActivity { deckPicker -> + advanceRobolectricLooperUntil { deckPicker.viewModel.loadDeckCounts?.isCompleted == true } + assertErrorShown() + assertNotSame(firstDialog, ShadowDialog.getLatestDialog()) + assertEquals(1, deckPicker.viewModel.pendingErrors.value.size) + } + firstDialog.dismiss() + } + } + private fun assertErrorShown() { advanceRobolectricLooperUntil { (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true } assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFilteredPaneErrorLifecycleRegressionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFilteredPaneErrorLifecycleRegressionTest.kt new file mode 100644 index 000000000000..02f4e4935387 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerFilteredPaneErrorLifecycleRegressionTest.kt @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki + +import androidx.appcompat.app.AlertDialog +import androidx.appcompat.view.menu.MenuBuilder +import androidx.core.content.edit +import androidx.test.core.app.ActivityScenario +import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.backend.backendError +import anki.collection.OpChanges +import anki.scheduler.CongratsInfoResponse +import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.deckpicker.DeckPickerViewModel +import com.ichi2.testutils.BackupManagerTestUtilities +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.runBlocking +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 java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNotSame +import kotlin.test.assertSame +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +@Config(qualifiers = "xlarge") +class DeckPickerFilteredPaneErrorLifecycleRegressionTest : RobolectricTest() { + @After + fun resetBackend() = BackendFactory.setOverride(null) + + @Test + fun `unacknowledged tablet empty filtered deck error survives recreation`() { + val failure = BackendIoException(backendError { message = "Cannot empty the filtered deck" }) + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun emptyFilteredDeck(did: Long): OpChanges = throw failure + } + } + val deckId = addDynamicDeck("Filtered") + ensureCollectionLoadIsSynchronous() + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + lateinit var originalDialog: AlertDialog + scenario.onActivity { picker -> + advanceRobolectricLooperUntil { + picker.viewModel.loadDeckCounts?.isCompleted == true && picker.fragment?.shownDeckId == deckId + } + val pane = assertNotNull(picker.fragment) + pane.onMenuItemSelected(MenuBuilder(picker).add(0, R.id.action_empty, 0, "Empty")) + advanceRobolectricLooperUntil { (ShadowDialog.getLatestDialog() as? AlertDialog)?.isShowing == true } + originalDialog = ShadowDialog.getLatestDialog() as AlertDialog + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } + try { + scenario.recreate() + scenario.onActivity { picker -> + advanceRobolectricLooperUntil { picker.viewModel.loadDeckCounts?.isCompleted == true } + advanceRobolectricLooper() + val restoredDialog = ShadowDialog.getLatestDialog() as? AlertDialog + assertNotNull(restoredDialog, "The unacknowledged pane error must be shown again") + assertNotSame(originalDialog, restoredDialog, "The previous activity's window does not restore the error") + assertTrue(restoredDialog.isShowing) + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } + } finally { + originalDialog.dismiss() + } + } + } + + @Test + fun `pane refresh failure finishing after recreation reaches the retained error queue`() { + val readStarted = CountDownLatch(1) + val releaseRead = CountDownLatch(1) + val gateRead = AtomicBoolean(false) + val failure = BackendIoException(backendError { message = "Pane refresh failed after recreation" }) + runBlocking { CollectionManager.discardBackend() } + BackendFactory.setOverride { + object : Backend() { + override fun congratsInfo(): CongratsInfoResponse { + if (gateRead.compareAndSet(true, false)) { + readStarted.countDown() + check(releaseRead.await(20, TimeUnit.SECONDS)) { "Timed out releasing pane refresh" } + throw failure + } + return super.congratsInfo() + } + } + } + addBasicNote() + ensureCollectionLoadIsSynchronous() + setIntroductionSlidesShown(true) + BackupManagerTestUtilities.setupSpaceForBackup(targetContext) + targetContext.sharedPrefs().edit { putBoolean("backupPromptDisabled", true) } + throwOnShowError = false + var restoreQueue: (() -> Unit)? = null + lateinit var initialRefresh: Job + lateinit var retainedModel: DeckPickerViewModel + try { + ActivityScenario.launch(DeckPicker::class.java).use { scenario -> + scenario.onActivity { picker -> + advanceRobolectricLooperUntil { + picker.viewModel.loadDeckCounts?.isCompleted == true && picker.fragment?.shownDeckId != null + } + advanceRobolectricLooper() + retainedModel = picker.viewModel + val previousQueue = CollectionManager.setTestDispatcher(Dispatchers.IO.limitedParallelism(1), useReentrantLock = false) + restoreQueue = { CollectionManager.setTestDispatcher(previousQueue) } + gateRead.set(true) + picker.updateDeckList() + advanceRobolectricLooperUntil { readStarted.count == 0L } + initialRefresh = assertNotNull(picker.viewModel.loadDeckCounts) + } + // The metadata read belongs to the old pane, which detaches during recreation. + scenario.recreate() + releaseRead.countDown() + scenario.onActivity { picker -> + assertSame(retainedModel, picker.viewModel) + advanceRobolectricLooperUntil { + picker.viewModel.loadDeckCounts !== initialRefresh && picker.viewModel.loadDeckCounts?.isCompleted == true + } + advanceRobolectricLooper() + assertEquals(1, picker.viewModel.pendingErrors.value.size, "Detaching the pane must not discard its backend failure") + assertEquals(failure.localizedMessage, getAlertDialogText(checkDismissed = true)) + } + } + } finally { + releaseRead.countDown() + restoreQueue?.invoke() + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/StudyOptionsViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/StudyOptionsViewModelTest.kt index cb6405de344b..dff43b1340af 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/StudyOptionsViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/StudyOptionsViewModelTest.kt @@ -220,7 +220,7 @@ class StudyOptionsViewModelTest : JvmTest() { viewModel.refreshData() ensureOpsExecuted(1) { - viewModel.unbury().join() + viewModel.unbury() } } @@ -265,7 +265,7 @@ class StudyOptionsViewModelTest : JvmTest() { addNoteToDeck(other, queueType = QueueType.ManuallyBuried) col.decks.select(other) - viewModel.unbury().join() + viewModel.unbury() assertEquals(QueueType.New, col.getCard(col.findCards("deck:Shown").single()).queue) assertEquals(QueueType.ManuallyBuried, col.getCard(col.findCards("deck:Other").single()).queue) From 2dd0d21558aa4568f87a401e97b114aeb9aaec0f Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:29:55 +0530 Subject: [PATCH 30/30] fix(deck-picker): keep rename and review completion errors Reading the deck name for Rename, and the study state when a review session ends, still ran in the activity, so their errors were lost on rotation. Both now read through the ViewModel. Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 4 +- .../com/ichi2/anki/DeckPickerErrorTest.kt | 37 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 84acd147b974..5d86c93b5e30 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -1623,7 +1623,7 @@ open class DeckPicker : private fun processReviewResults(resultCode: Int) { if (resultCode == AbstractFlashcardViewer.RESULT_NO_MORE_CARDS) { launchCatchingTask { - val state = withCol { currentDeckStudyState() } + val state = viewModel.withColAsync { currentDeckStudyState() }.await() ?: return@launchCatchingTask StudyCompletionUi.onReviewsCompleted(this@DeckPicker, state) } fragment?.refreshInterface() @@ -2413,7 +2413,7 @@ open class DeckPicker : fun renameDeckDialog(did: DeckId) { launchCatchingTask { - val currentName = withCol { decks.name(did) } + val currentName = viewModel.withColAsync { decks.name(did) }.await() ?: return@launchCatchingTask val createDeckDialog = CreateDeckDialog( context = this@DeckPicker, diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt index 287af000c333..da95269a3e46 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerErrorTest.kt @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 Ashish Yadav package com.ichi2.anki +import android.view.KeyEvent import android.view.View import androidx.appcompat.app.AlertDialog import androidx.appcompat.view.menu.MenuBuilder @@ -18,9 +19,11 @@ import anki.collection.OpChanges import anki.collection.OpChangesWithCount import anki.decks.DeckTreeNode import anki.decks.SetDeckCollapsedRequest +import anki.scheduler.CongratsInfoResponse import anki.scheduler.UnburyDeckRequest import app.cash.turbine.test import com.google.protobuf.ByteString +import com.ichi2.anki.AbstractFlashcardViewer.Companion.RESULT_NO_MORE_CARDS import com.ichi2.anki.backend.DatabaseCorruption import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.crashreporting.CrashReporter @@ -858,6 +861,40 @@ class DeckPickerErrorTest : RobolectricTest() { fragment!!.onMenuItemSelected(MenuBuilder(this).add(0, R.id.action_unbury, 0, "Unbury")) } + @Test + fun `a failing rename read keeps its error across recreation`() = + assertErrorSurvivesRecreation( + backend = { failNext -> + object : Backend() { + override fun getDeckLegacy(did: Long): ByteString = + if (failNext.getAndSet(false)) throw failure else super.getDeckLegacy(did) + } + }, + ) { failNext -> + advanceRobolectricLooper() + failNext.set(true) + renameDeckDialog(DEFAULT_DECK_ID) + } + + @Test + fun `a failing read after reviews keeps its error across recreation`() = + assertErrorSurvivesRecreation( + backend = { failNext -> + object : Backend() { + override fun congratsInfo(): CongratsInfoResponse = + if (failNext.getAndSet(false)) throw failure else super.congratsInfo() + } + }, + ) { failNext -> + advanceRobolectricLooper() + val shadow = shadowOf(this) + onKeyUp(KeyEvent.KEYCODE_S, KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_S)) + advanceRobolectricLooperUntil { shadow.peekNextStartedActivity() != null } + val review = assertNotNull(shadow.nextStartedActivityForResult) + failNext.set(true) + activityResultRegistry.dispatchResult(review.requestCode, RESULT_NO_MORE_CARDS, null) + } + @Test fun `a corruption found while recovery is open does not reopen it`() = withCorruptDeletionRecovery { deckPicker, recovery ->