Skip to content

Commit

Permalink
refactor(deck-picker): deleteDeck => viewModel
Browse files Browse the repository at this point in the history
* inline confirmDeckDeletion
  * removing unnecessary 'dismiss' calls
* extract `deleteDeck`
* introduce `DeckDeletionResult`
  • Loading branch information
david-allison committed Jan 6, 2025
1 parent d6d17f1 commit 2903a0c
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 29 deletions.
50 changes: 24 additions & 26 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import com.ichi2.anki.android.input.ShortcutGroup
import com.ichi2.anki.android.input.shortcut
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.dialogs.AsyncDialogFragment
import com.ichi2.anki.dialogs.BackupPromptDialog
Expand Down Expand Up @@ -168,7 +169,6 @@ import com.ichi2.libanki.Decks
import com.ichi2.libanki.MediaCheckResult
import com.ichi2.libanki.exception.ConfirmModSchemaException
import com.ichi2.libanki.sched.DeckNode
import com.ichi2.libanki.undoableOp
import com.ichi2.libanki.utils.TimeManager
import com.ichi2.ui.AccessibleSearchView
import com.ichi2.ui.BadgeDrawableBuilder
Expand Down Expand Up @@ -617,6 +617,18 @@ open class DeckPicker :
.build(),
onReceiveContentListener,
)

setupFlows()
}

private fun setupFlows() {
fun onDeckDeleted(result: DeckDeletionResult) {
showSnackbar(result.toHumanReadableString(), Snackbar.LENGTH_SHORT) {
setAction(R.string.undo) { undo() }
}
}

viewModel.deckDeletedNotification.launchCollectionInLifecycleScope(::onDeckDeleted)
}

private val onReceiveContentListener =
Expand Down Expand Up @@ -654,8 +666,10 @@ open class DeckPicker :
/* we can only disable the shortcut for now as it is restricted by Google https://issuetracker.google.com/issues/68949561?pli=1#comment4
* if fixed or given free hand to delete the shortcut with the help of API update this method and use the new one
*/
// TODO: it feels buggy that this is not called on all deck deletion paths
disableDeckAndChildrenShortcuts(deckId)
confirmDeckDeletion(deckId)
dismissAllDialogFragments()
deleteDeck(deckId)
}
DeckPickerContextMenuOption.DECK_OPTIONS -> {
Timber.i("ContextMenu: Open deck options selected")
Expand Down Expand Up @@ -1150,8 +1164,9 @@ open class DeckPicker :
}
R.id.action_deck_delete -> {
launchCatchingTask {
val targetDeckId = withCol { decks.selected() }
confirmDeckDeletion(targetDeckId)
withProgress(resources.getString(R.string.delete_deck)) {
viewModel.deleteSelectedDeck().join()
}
}
return true
}
Expand Down Expand Up @@ -2398,31 +2413,14 @@ open class DeckPicker :
createDeckDialog.showDialog()
}

fun confirmDeckDeletion(did: DeckId): Job {
// No confirmation required, as undoable
dismissAllDialogFragments()
return deleteDeck(did)
}

/**
* Deletes the provided deck, child decks. and all cards inside.
* Use [.confirmDeckDeletion] for a confirmation dialog
* @param did the deck to delete
* Deletes the provided deck, child decks, and all cards inside.
* @param did ID of the deck to delete
*/
fun deleteDeck(did: DeckId): Job =
fun deleteDeck(did: DeckId) =
launchCatchingTask {
val deckName = withCol { decks.get(did)!!.name }
val changes =
withProgress(resources.getString(R.string.delete_deck)) {
undoableOp {
decks.remove(listOf(did))
}
}
// After deletion: decks.current() reverts to Default, necessitating `focusedDeck`
// to match and avoid unnecessary scrolls in `renderPage()`.
viewModel.focusedDeck = Consts.DEFAULT_DECK_ID
showSnackbar(TR.browsingCardsDeletedWithDeckname(changes.count, deckName), Snackbar.LENGTH_SHORT) {
setAction(R.string.undo) { undo() }
withProgress(resources.getString(R.string.delete_deck)) {
viewModel.deleteDeck(did).join()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,82 @@

package com.ichi2.anki.deckpicker

import androidx.annotation.CheckResult
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
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.Consts
import com.ichi2.libanki.DeckId
import com.ichi2.libanki.undoableOp
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.launch

/** ViewModel for [DeckPicker] */
class DeckPickerViewModel : ViewModel() {
/**
* @see deleteDeck
* @see DeckDeletionResult
*/
val deckDeletedNotification = MutableSharedFlow<DeckDeletionResult>()

/**
* Keep track of which deck was last given focus in the deck list. If we find that this value
* has changed between deck list refreshes, we need to recenter the deck list to the new current
* deck.
*/
// TODO: This should later be handled as a Flow
var focusedDeck: DeckId = 0

/**
* Deletes the provided deck, child decks. and all cards inside.
*
* This is a slow operation and should be inside `withProgress`
*
* @param did ID of the deck to delete
*/
@CheckResult // This is a slow operation and should be inside `withProgress`
fun deleteDeck(did: DeckId) =
viewModelScope.launch {
val deckName = withCol { decks.get(did)!!.name }
val changes = undoableOp { decks.remove(listOf(did)) }
// After deletion: decks.current() reverts to Default, necessitating `focusedDeck`
// to match and avoid unnecessary scrolls in `renderPage()`.
focusedDeck = Consts.DEFAULT_DECK_ID

deckDeletedNotification.emit(
DeckDeletionResult(deckName = deckName, cardsDeleted = changes.count),
)
}

/**
* Deletes the currently selected deck
*
* This is a slow operation and should be inside `withProgress`
*/
@CheckResult
fun deleteSelectedDeck() =
viewModelScope.launch {
val targetDeckId = withCol { decks.selected() }
deleteDeck(targetDeckId).join()
}
}

/** Result of [DeckPickerViewModel.deleteDeck] */
data class DeckDeletionResult(
val deckName: String,
val cardsDeleted: Int,
) {
/**
* @see GeneratedTranslations.browsingCardsDeletedWithDeckname
*/
// TODO: Somewhat questionable meaning: {count} cards deleted from {deck_name}.
@CheckResult
fun toHumanReadableString() =
TR.browsingCardsDeletedWithDeckname(
count = cardsDeleted,
deckName = deckName,
)
}
3 changes: 1 addition & 2 deletions AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,7 @@ class DeckPickerTest : RobolectricTest() {
DeckPicker::class.java,
Intent(),
)
deckPicker.confirmDeckDeletion(did)
advanceRobolectricLooperWithSleep()
deckPicker.viewModel.deleteDeck(did).join()
assertThat("deck was deleted", col.decks.count(), equalTo(1))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class CreateDeckDialogTest : RobolectricTest() {

// After the last deck was created, delete a deck
if (decksCount() >= 10) {
deckPicker.confirmDeckDeletion(did)
deckPicker.viewModel.deleteDeck(did).join()
assertEquals(deckCounter.decrementAndGet(), decksCount())

assertEquals(deckCounter.get(), decksCount())
Expand Down

0 comments on commit 2903a0c

Please sign in to comment.