Empty cards become undoable (#3386)

* Empty cards is undoable

If there was a reason for this operation not to be undoable, I can't easily guess it. My main hyposhesis was that the number of deleted card may be too big. But I realized that deleting a deck is undoable and may delete as many note.

As you may know, I realized that only the undoable operations triggered notification in AnkiDroid that we may have to update the UI. And while I just wanted to trigger more notifications, some reviewers thought it would be nicer if the operation were returning a OpChanges. So here it's done. If you would please consider merging it.

I decided to introduce a new string because the closest strings I could find currently are "Empty cards..." and the trailing commas don't seem nice in "undo". And the title, which we may not be able to reuse in all language

* Don't count cards that have already been removed (dae)

ftl/core/actions.ftl

@@ -12,6 +12,7 @@ actions-decks = Decks
 actions-decrement-value = Decrement value
 actions-delete = Delete
 actions-export = Export
+actions-empty-cards = Empty Cards
 actions-filter = Filter
 actions-help = Help
 actions-increment-value = Increment value

proto/anki/cards.proto

@@ -13,7 +13,7 @@ import "anki/collection.proto";
 service CardsService {
   rpc GetCard(CardId) returns (Card);
   rpc UpdateCards(UpdateCardsRequest) returns (collection.OpChanges);
-  rpc RemoveCards(RemoveCardsRequest) returns (generic.Empty);
+  rpc RemoveCards(RemoveCardsRequest) returns (collection.OpChangesWithCount);
   rpc SetDeck(SetDeckRequest) returns (collection.OpChangesWithCount);
   rpc SetFlag(SetFlagRequest) returns (collection.OpChangesWithCount);
 }

pylib/anki/collection.py

@@ -604,9 +604,11 @@ class Collection(DeprecatedNamesMixin):
     def card_count(self) -> Any:
         return self.db.scalar("select count() from cards")
 
-    def remove_cards_and_orphaned_notes(self, card_ids: Sequence[CardId]) -> None:
+    def remove_cards_and_orphaned_notes(
+        self, card_ids: Sequence[CardId]
+    ) -> OpChangesWithCount:
         "You probably want .remove_notes_by_card() instead."
-        self._backend.remove_cards(card_ids=card_ids)
+        return self._backend.remove_cards(card_ids=card_ids)
 
     def set_deck(self, card_ids: Sequence[CardId], deck_id: int) -> OpChangesWithCount:
         return self._backend.set_deck(card_ids=card_ids, deck_id=deck_id)

rslib/src/card/mod.rs

@@ -327,13 +327,15 @@ impl Collection {
 
     /// Remove cards and any resulting orphaned notes.
     /// Expects a transaction.
-    pub(crate) fn remove_cards_and_orphaned_notes(&mut self, cids: &[CardId]) -> Result<()> {
+    pub(crate) fn remove_cards_and_orphaned_notes(&mut self, cids: &[CardId]) -> Result<usize> {
         let usn = self.usn()?;
         let mut nids = HashSet::new();
+        let mut card_count = 0;
         for cid in cids {
             if let Some(card) = self.storage.get_card(*cid)? {
                 nids.insert(card.note_id);
                 self.remove_card_and_add_grave_undoable(card, usn)?;
+                card_count += 1;
             }
         }
         for nid in nids {
@@ -342,7 +344,7 @@ impl Collection {
             }
         }
 
-        Ok(())
+        Ok(card_count)
     }
 
     pub fn set_deck(&mut self, cards: &[CardId], deck_id: DeckId) -> Result<OpOutput<usize>> {

rslib/src/card/service.rs

@@ -14,6 +14,7 @@ use crate::error::OrNotFound;
 use crate::notes::NoteId;
 use crate::prelude::TimestampSecs;
 use crate::prelude::Usn;
+use crate::undo::Op;
 
 impl crate::services::CardsService for Collection {
     fn get_card(
@@ -44,17 +45,20 @@ impl crate::services::CardsService for Collection {
             .map(Into::into)
     }
 
-    fn remove_cards(&mut self, input: anki_proto::cards::RemoveCardsRequest) -> error::Result<()> {
+    fn remove_cards(
-        self.transact_no_undo(|col| {
+        &mut self,
+        input: anki_proto::cards::RemoveCardsRequest,
+    ) -> error::Result<anki_proto::collection::OpChangesWithCount> {
+        self.transact(Op::EmptyCards, |col| {
             col.remove_cards_and_orphaned_notes(
                 &input
                     .card_ids
                     .into_iter()
                     .map(Into::into)
                     .collect::<Vec<_>>(),
-            )?;
+            )
-            Ok(())
         })
+        .map(Into::into)
     }
 
     fn set_deck(

rslib/src/ops.rs

@@ -15,6 +15,7 @@ pub enum Op {
     ChangeNotetype,
     ClearUnusedTags,
     CreateCustomStudy,
+    EmptyCards,
     EmptyFilteredDeck,
     FindAndReplace,
     ImageOcclusion,
@@ -57,6 +58,7 @@ impl Op {
             Op::AnswerCard => tr.actions_answer_card(),
             Op::Bury => tr.studying_bury(),
             Op::CreateCustomStudy => tr.actions_custom_study(),
+            Op::EmptyCards => tr.actions_empty_cards(),
             Op::Import => tr.actions_import(),
             Op::RemoveDeck => tr.decks_delete_deck(),
             Op::RemoveNote => tr.studying_delete_note(),