Skip to content

Commit

Permalink
fix(empty-cards): undoable & correct result
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
david-allison committed Jan 6, 2025
1 parent 2903a0c commit c946e01
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 9 deletions.
15 changes: 10 additions & 5 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,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
Expand Down Expand Up @@ -628,7 +629,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 =
Expand Down Expand Up @@ -2485,9 +2493,7 @@ open class DeckPicker :
launchCatchingTask {
val emptyCids =
withProgress(R.string.emtpy_cards_finding) {
withCol {
emptyCids()
}
viewModel.findEmptyCards().await()
}
AlertDialog.Builder(this@DeckPicker).show {
setTitle(TR.emptyCardsWindowTitle())
Expand All @@ -2499,10 +2505,9 @@ open class DeckPicker :
setPositiveButton(R.string.dialog_positive_delete) { _, _ ->
launchCatchingTask {
withProgress(TR.emptyCardsDeleting()) {
withCol { removeCardsAndOrphanedNotes(emptyCids) }
viewModel.deleteEmptyCards(emptyCids).join()
}
}
showSnackbar(getString(R.string.empty_cards_deleted, emptyCids.size))
}
setNegativeButton(R.string.dialog_cancel) { _, _ -> }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ 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
import kotlinx.coroutines.async
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.launch

Expand All @@ -36,6 +38,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
Expand Down Expand Up @@ -77,6 +80,22 @@ class DeckPickerViewModel : ViewModel() {
val targetDeckId = withCol { decks.selected() }
deleteDeck(targetDeckId).join()
}

/** Returns a list of cards to be passed to [deleteEmptyCards] (after user confirmation) */
fun findEmptyCards() =
viewModelScope.async {
return@async withCol { emptyCids() }
}

/**
* Removes the provided list of cards from the collection.
* @param cids ids from [findEmptyCards]
*/
fun deleteEmptyCards(cids: List<CardId>) =
viewModelScope.launch {
val result = undoableOp { removeCardsAndOrphanedNotes(cids) }
emptyCardsNotification.emit(EmptyCardsResult(cardsDeleted = result.count))
}
}

/** Result of [DeckPickerViewModel.deleteDeck] */
Expand All @@ -95,3 +114,13 @@ data class DeckDeletionResult(
deckName = deckName,
)
}

/** Result of [DeckPickerViewModel.deleteEmptyCards] */
data class EmptyCardsResult(
val cardsDeleted: Int,
) {
/**
* @see GeneratedTranslations.emptyCardsDeletedCount */
@CheckResult
fun toHumanReadableString() = TR.emptyCardsDeletedCount(cardsDeleted)
}
8 changes: 5 additions & 3 deletions AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt
Original file line number Diff line number Diff line change
Expand Up @@ -531,9 +531,11 @@ class Collection(
Timber.d("removeNotes: %d changes", it.count)
}

fun removeCardsAndOrphanedNotes(cardIds: Iterable<Long>) {
backend.removeCards(cardIds)
}
/**
* @return the number of deleted cards
*/
// if an invalid/duplicate CardId provided, the output count may be less than the input
fun removeCardsAndOrphanedNotes(cardIds: Iterable<CardId>) = backend.removeCards(cardIds)

fun addNote(
note: Note,
Expand Down
1 change: 0 additions & 1 deletion AnkiDroid/src/main/res/values/03-dialogs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* 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().await().also { cardsToEmpty ->
assertThat("there are empty cards", cardsToEmpty, not(empty()))
Timber.d("created %d empty cards: [%s]", cardsToEmpty.size, cardsToEmpty)
}
}
}

0 comments on commit c946e01

Please sign in to comment.