fix(empty-cards): undoable & correct result

* We use the backend string
* We use `undoableOp`
* we add 'undo' to the snackbar
* We return the backend result, which may not
 be the supplied input
   - unit tested: duplicates

empty_cards_deleted => TR.emptyCardsDeletedCount

Fixes 16945

https://redirect.github.com/ankitects/anki/pull/3386

Author: david-allison

AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt

@@ -106,6 +106,7 @@ import com.ichi2.anki.deckpicker.BITMAP_BYTES_PER_PIXEL
 import com.ichi2.anki.deckpicker.BackgroundImage
 import com.ichi2.anki.deckpicker.DeckDeletionResult
 import com.ichi2.anki.deckpicker.DeckPickerViewModel
+import com.ichi2.anki.deckpicker.EmptyCardsResult
 import com.ichi2.anki.dialogs.AsyncDialogFragment
 import com.ichi2.anki.dialogs.BackupPromptDialog
 import com.ichi2.anki.dialogs.ConfirmationDialog
@@ -626,7 +627,14 @@ open class DeckPicker :
             }
         }
 
+        fun onCardsEmptied(result: EmptyCardsResult) {
+            showSnackbar(result.toHumanReadableString(), Snackbar.LENGTH_SHORT) {
+                setAction(R.string.undo) { undo() }
+            }
+        }
+
         viewModel.deckDeletedNotification.launchCollectionInLifecycleScope(::onDeckDeleted)
+        viewModel.emptyCardsNotification.launchCollectionInLifecycleScope(::onCardsEmptied)
     }
 
     private val onReceiveContentListener =
@@ -2487,26 +2495,23 @@ open class DeckPicker :
 
     private fun handleEmptyCards() {
         launchCatchingTask {
-            val emptyCids =
+            val emptyCards =
                 withProgress(R.string.emtpy_cards_finding) {
-                    withCol {
-                        emptyCids()
-                    }
+                    viewModel.findEmptyCards()
                 }
             AlertDialog.Builder(this@DeckPicker).show {
                 setTitle(TR.emptyCardsWindowTitle())
-                if (emptyCids.isEmpty()) {
+                if (emptyCards.isEmpty()) {
                     setMessage(TR.emptyCardsNotFound())
                     setPositiveButton(R.string.dialog_ok) { _, _ -> }
                 } else {
-                    setMessage(getString(R.string.empty_cards_count, emptyCids.size))
+                    setMessage(getString(R.string.empty_cards_count, emptyCards.size))
                     setPositiveButton(R.string.dialog_positive_delete) { _, _ ->
                         launchCatchingTask {
                             withProgress(TR.emptyCardsDeleting()) {
-                                withCol { removeCardsAndOrphanedNotes(emptyCids) }
+                                viewModel.deleteEmptyCards(emptyCards).join()
                             }
                         }
-                        showSnackbar(getString(R.string.empty_cards_deleted, emptyCids.size))
                     }
                     setNegativeButton(R.string.dialog_cancel) { _, _ -> }
                 }

AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt

@@ -23,6 +23,7 @@ import anki.i18n.GeneratedTranslations
 import com.ichi2.anki.CollectionManager.TR
 import com.ichi2.anki.CollectionManager.withCol
 import com.ichi2.anki.DeckPicker
+import com.ichi2.libanki.CardId
 import com.ichi2.libanki.Consts
 import com.ichi2.libanki.DeckId
 import com.ichi2.libanki.undoableOp
@@ -36,6 +37,7 @@ class DeckPickerViewModel : ViewModel() {
      * @see DeckDeletionResult
      */
     val deckDeletedNotification = MutableSharedFlow<DeckDeletionResult>()
+    val emptyCardsNotification = MutableSharedFlow<EmptyCardsResult>()
 
     /**
      * Keep track of which deck was last given focus in the deck list. If we find that this value
@@ -77,6 +79,19 @@ class DeckPickerViewModel : ViewModel() {
             val targetDeckId = withCol { decks.selected() }
             deleteDeck(targetDeckId).join()
         }
+
+    /** Returns a list of cards to be passed to [deleteEmptyCards] (after user confirmation) */
+    suspend fun findEmptyCards() = EmptyCards(withCol { emptyCids() })
+
+    /**
+     * Removes the provided list of cards from the collection.
+     * @param emptyCards Cards to be deleted, result of [findEmptyCards]
+     */
+    fun deleteEmptyCards(emptyCards: EmptyCards) =
+        viewModelScope.launch {
+            val result = undoableOp { removeCardsAndOrphanedNotes(emptyCards) }
+            emptyCardsNotification.emit(EmptyCardsResult(cardsDeleted = result.count))
+        }
 }
 
 /** Result of [DeckPickerViewModel.deleteDeck] */
@@ -95,3 +110,21 @@ data class DeckDeletionResult(
             deckName = deckName,
         )
 }
+
+/**
+ * Result of [DeckPickerViewModel.findEmptyCards], used in [DeckPickerViewModel.deleteEmptyCards]
+ */
+@JvmInline
+value class EmptyCards(
+    val cards: List<CardId>,
+) : List<CardId> by cards
+
+/** Result of [DeckPickerViewModel.deleteEmptyCards] */
+data class EmptyCardsResult(
+    val cardsDeleted: Int,
+) {
+    /**
+     * @see GeneratedTranslations.emptyCardsDeletedCount */
+    @CheckResult
+    fun toHumanReadableString() = TR.emptyCardsDeletedCount(cardsDeleted)
+}

AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt

@@ -542,9 +542,11 @@ class Collection(
             Timber.d("removeNotes: %d changes", it.count)
         }
 
-    fun removeCardsAndOrphanedNotes(cardIds: Iterable<Long>) {
-        backend.removeCards(cardIds)
-    }
+    /**
+     * @return the number of deleted cards. **Note:** if an invalid/duplicate [CardId] is provided,
+     * the output count may be less than the input.
+     */
+    fun removeCardsAndOrphanedNotes(cardIds: Iterable<CardId>) = backend.removeCards(cardIds)
 
     fun addNote(
         note: Note,

AnkiDroid/src/main/res/values/03-dialogs.xml

@@ -116,7 +116,6 @@
     <!-- Empty cards -->
     <string name="emtpy_cards_finding">Finding empty cards…</string>
     <string name="empty_cards_count">Cards to delete: %d</string>
-    <string name="empty_cards_deleted">Cards deleted: %d</string>
 
 
     <!-- Multimedia - Edit Field Activity -->

AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt

@@ -0,0 +1,106 @@
+/*
+ *  Copyright (c) 2025 David Allison <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify it under
+ *  the terms of the GNU General Public License as published by the Free Software
+ *  Foundation; either version 3 of the License, or (at your option) any later
+ *  version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT ANY
+ *  WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ *  PARTICULAR PURPOSE. See the GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along with
+ *  this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.ichi2.anki.deckpicker
+
+import androidx.annotation.CheckResult
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import app.cash.turbine.test
+import com.ichi2.anki.RobolectricTest
+import com.ichi2.libanki.CardId
+import com.ichi2.libanki.undoStatus
+import com.ichi2.testutils.ensureOpsExecuted
+import org.hamcrest.CoreMatchers.not
+import org.hamcrest.MatcherAssert.assertThat
+import org.hamcrest.Matchers.empty
+import org.hamcrest.Matchers.equalTo
+import org.junit.Test
+import org.junit.runner.RunWith
+import timber.log.Timber
+
+/** Test of [DeckPickerViewModel] */
+@RunWith(AndroidJUnit4::class)
+class DeckPickerViewModelTest : RobolectricTest() {
+    private val viewModel = DeckPickerViewModel()
+
+    @Test
+    fun `empty cards - flow`() =
+        runTest {
+            val cardsToEmpty = createEmptyCards()
+
+            viewModel.emptyCardsNotification.test {
+                // test a 'normal' deletion
+                viewModel.deleteEmptyCards(cardsToEmpty).join()
+
+                expectMostRecentItem().also {
+                    assertThat("cards deleted", it.cardsDeleted, equalTo(1))
+                }
+
+                // ensure a duplicate output is displayed to the user
+                val newCardsToEmpty = createEmptyCards()
+                viewModel.deleteEmptyCards(newCardsToEmpty).join()
+
+                expectMostRecentItem().also {
+                    assertThat("cards deleted: duplicate output", it.cardsDeleted, equalTo(1))
+                }
+
+                // send the same collection in, but with the same ids.
+                // the output should only show 1 card deleted
+                val emptyCardsSentTwice = createEmptyCards()
+                viewModel.deleteEmptyCards(emptyCardsSentTwice + emptyCardsSentTwice).join()
+
+                expectMostRecentItem().also {
+                    assertThat("cards deleted: duplicate input", it.cardsDeleted, equalTo(1))
+                }
+
+                // test an empty list: a no-op should inform the user, rather than do nothing
+                viewModel.deleteEmptyCards(listOf()).join()
+
+                expectMostRecentItem().also {
+                    assertThat("'no cards deleted' is notified", it.cardsDeleted, equalTo(0))
+                }
+            }
+        }
+
+    @Test
+    fun `empty cards - undoable`() =
+        runTest {
+            val cardsToEmpty = createEmptyCards()
+
+            // ChangeManager assert
+            ensureOpsExecuted(1) {
+                viewModel.deleteEmptyCards(cardsToEmpty).join()
+            }
+
+            // backend assert
+            assertThat("col undo status", col.undoStatus().undo, equalTo("Empty Cards"))
+        }
+
+    @CheckResult
+    private suspend fun createEmptyCards(): List<CardId> {
+        addNoteUsingNoteTypeName("Cloze", "{{c1::Hello}} {{c2::World}}", "").apply {
+            setField(0, "{{c1::Hello}}")
+            col.updateNote(this)
+        }
+        return viewModel.findEmptyCards().also { cardsToEmpty ->
+            assertThat("there are empty cards", cardsToEmpty, not(empty()))
+            Timber.d("created %d empty cards: [%s]", cardsToEmpty.size, cardsToEmpty)
+        }
+    }
+
+    /** test helper to use [deleteEmptyCards] without an [EmptyCards] instance */
+    private fun DeckPickerViewModel.deleteEmptyCards(list: List<CardId>) = deleteEmptyCards(EmptyCards(list))
+}