make flag changes in the reviewer undoable

This splits update_card() into separate undoable/non-undoable ops
like the change to notes in b4396b94abdeba3347d30025c5c0240d991006c9

It means that actions get a blanket 'Update Card' description - in the
future we'll probably want to either add specific actions to the backend,
or allow an enum or string to be passed in to describe the op.

Other changes:
- card.flush() can no longer be used to add new cards. Card creation
is only supposed to be done in response to changes in a note's fields,
and this functionality was only exposed because the card generation
hadn't been migrated to the backend at that point. As far as I'm aware,
only Arthur's "copy notes" add-on used this functionality, and that should
be an easy fix - when the new note is added, the associated cards will
be generated, and they can then be retrieved with note.cards()
- tidy ups/PEP8

ftl/core/undo.ftl

@@ -14,3 +14,4 @@ undo-unbury-unsuspend = Unbury/Unsuspend
 undo-add-note = Add Note
 undo-update-tag = Update Tag
 undo-update-note = Update Note
+undo-update-card = Update Card

pylib/anki/cards.py

@@ -74,19 +74,9 @@ def _load_from_backend_card(self, c: _pb.Card) -> None:
         self.flags = c.flags
         self.data = c.data
 
-    def _bugcheck(self) -> None:
-        if (
-            self.queue == QUEUE_TYPE_REV
-            and self.odue
-            and not self.col.decks.isDyn(self.did)
-        ):
-            hooks.card_odue_was_invalid()
-
-    def flush(self) -> None:
-        self._bugcheck()
-        hooks.card_will_flush(self)
+    def _to_backend_card(self) -> _pb.Card:
         # mtime & usn are set by backend
-        card = _pb.Card(
+        return _pb.Card(
             id=self.id,
             note_id=self.nid,
             deck_id=self.did,
@@ -104,10 +94,15 @@ def flush(self) -> None:
             flags=self.flags,
             data=self.data,
         )
+
+    def flush(self) -> None:
+        hooks.card_will_flush(self)
         if self.id != 0:
-            self.col._backend.update_card(card)
+            self.col._backend.update_card(
+                card=self._to_backend_card(), skip_undo_entry=True
+            )
         else:
-            self.id = self.col._backend.add_card(card)
+            raise Exception("card.flush() expects an existing card")
 
     def question(self, reload: bool = False, browser: bool = False) -> str:
         return self.render_output(reload, browser).question_and_style()
@@ -197,9 +192,14 @@ def __repr__(self) -> str:
         del d["timerStarted"]
         return f"{super().__repr__()} {pprint.pformat(d, width=300)}"
 
-    def userFlag(self) -> int:
+    def user_flag(self) -> int:
         return self.flags & 0b111
 
-    def setUserFlag(self, flag: int) -> None:
+    def set_user_flag(self, flag: int) -> None:
         assert 0 <= flag <= 7
         self.flags = (self.flags & ~0b111) | flag
+
+    # legacy
+
+    userFlag = user_flag
+    setUserFlag = set_user_flag

pylib/anki/collection.py

@@ -319,17 +319,23 @@ def beforeUpload(self) -> None:
     # Object creation helpers
     ##########################################################################
 
-    def getCard(self, id: int) -> Card:
+    def get_card(self, id: int) -> Card:
         return Card(self, id)
 
+    def update_card(self, card: Card) -> None:
+        """Save card changes to database, and add an undo entry.
+        Unlike card.flush(), this will invalidate any current checkpoint."""
+        self._backend.update_card(card=card._to_backend_card(), skip_undo_entry=False)
+
     def get_note(self, id: int) -> Note:
         return Note(self, id=id)
 
     def update_note(self, note: Note) -> None:
         """Save note changes to database, and add an undo entry.
         Unlike note.flush(), this will invalidate any current checkpoint."""
-        self._backend.update_note(note=note.to_backend_note(), skip_undo_entry=False)
+        self._backend.update_note(note=note._to_backend_note(), skip_undo_entry=False)
 
+    getCard = get_card
     getNote = get_note
 
     # Utils
@@ -367,7 +373,7 @@ def newNote(self, forDeck: bool = True) -> Note:
         return Note(self, self.models.current(forDeck))
 
     def add_note(self, note: Note, deck_id: int) -> None:
-        note.id = self._backend.add_note(note=note.to_backend_note(), deck_id=deck_id)
+        note.id = self._backend.add_note(note=note._to_backend_note(), deck_id=deck_id)
 
     def remove_notes(self, note_ids: Sequence[int]) -> None:
         hooks.notes_will_be_deleted(self, note_ids)
@@ -953,7 +959,7 @@ def _closeLog(self) -> None:
     # Card Flags
     ##########################################################################
 
-    def setUserFlag(self, flag: int, cids: List[int]) -> None:
+    def set_user_flag_for_cards(self, flag: int, cids: List[int]) -> None:
         assert 0 <= flag <= 7
         self.db.execute(
             "update cards set flags = (flags & ~?) | ?, usn=?, mod=? where id in %s"

pylib/anki/notes.py

@@ -53,7 +53,7 @@ def _load_from_backend_note(self, n: _pb.Note) -> None:
         self.fields = list(n.fields)
         self._fmap = self.col.models.fieldMap(self.model())
 
-    def to_backend_note(self) -> _pb.Note:
+    def _to_backend_note(self) -> _pb.Note:
         hooks.note_will_flush(self)
         return _pb.Note(
             id=self.id,
@@ -69,7 +69,9 @@ def flush(self) -> None:
         """This preserves any current checkpoint.
         For an undo entry, use col.update_note() instead."""
         assert self.id != 0
-        self.col._backend.update_note(note=self.to_backend_note(), skip_undo_entry=True)
+        self.col._backend.update_note(
+            note=self._to_backend_note(), skip_undo_entry=True
+        )
 
     def __repr__(self) -> str:
         d = dict(self.__dict__)
@@ -124,7 +126,7 @@ def model(self) -> Optional[NoteType]:
     _model = property(model)
 
     def cloze_numbers_in_fields(self) -> Sequence[int]:
-        return self.col._backend.cloze_numbers_in_note(self.to_backend_note())
+        return self.col._backend.cloze_numbers_in_note(self._to_backend_note())
 
     # Dict interface
     ##################################################
@@ -187,5 +189,5 @@ def setTagsFromStr(self, tags: str) -> None:
     def dupeOrEmpty(self) -> int:
         "1 if first is empty; 2 if first is a duplicate, 0 otherwise."
         return self.col._backend.note_is_duplicate_or_empty(
-            self.to_backend_note()
+            self._to_backend_note()
         ).state

pylib/anki/template.py

@@ -178,7 +178,7 @@ def fields(self) -> Dict[str, str]:
                 fields["Card"] = self._template["name"]
             else:
                 fields["Card"] = ""
-            flag = self._card.userFlag()
+            flag = self._card.user_flag()
             fields["CardFlag"] = flag and f"flag{flag}" or ""
             self._fields = fields
 
@@ -239,7 +239,7 @@ def _partially_render(self) -> PartiallyRenderedCard:
         if self._template:
             # card layout screen
             out = self._col._backend.render_uncommitted_card(
-                note=self._note.to_backend_note(),
+                note=self._note._to_backend_note(),
                 card_ord=self._card.ord,
                 template=to_json_bytes(self._template),
                 fill_empty=self._fill_empty,

pylib/tests/test_flags.py

@@ -13,30 +13,30 @@ def test_flags():
     c.flags = origBits
     c.flush()
     # no flags to start with
-    assert c.userFlag() == 0
+    assert c.user_flag() == 0
     assert len(col.findCards("flag:0")) == 1
     assert len(col.findCards("flag:1")) == 0
     # set flag 2
-    col.setUserFlag(2, [c.id])
+    col.set_user_flag_for_cards(2, [c.id])
     c.load()
-    assert c.userFlag() == 2
+    assert c.user_flag() == 2
     assert c.flags & origBits == origBits
     assert len(col.findCards("flag:0")) == 0
     assert len(col.findCards("flag:2")) == 1
     assert len(col.findCards("flag:3")) == 0
     # change to 3
-    col.setUserFlag(3, [c.id])
+    col.set_user_flag_for_cards(3, [c.id])
     c.load()
-    assert c.userFlag() == 3
+    assert c.user_flag() == 3
     # unset
-    col.setUserFlag(0, [c.id])
+    col.set_user_flag_for_cards(0, [c.id])
     c.load()
-    assert c.userFlag() == 0
+    assert c.user_flag() == 0
 
     # should work with Cards method as well
-    c.setUserFlag(2)
-    assert c.userFlag() == 2
-    c.setUserFlag(3)
-    assert c.userFlag() == 3
-    c.setUserFlag(0)
-    assert c.userFlag() == 0
+    c.set_user_flag(2)
+    assert c.user_flag() == 2
+    c.set_user_flag(3)
+    assert c.user_flag() == 3
+    c.set_user_flag(0)
+    assert c.user_flag() == 0

pylib/tests/test_schedv1.py

@@ -559,6 +559,8 @@ def test_suspend():
 
 def test_cram():
     col = getEmptyCol()
+    opt = col.models.byName("Basic (and reversed card)")
+    col.models.setCurrent(opt)
     note = col.newNote()
     note["Front"] = "one"
     col.addNote(note)
@@ -654,10 +656,9 @@ def test_cram():
     c.load()
     assert col.sched.answerButtons(c) == 4
     # add a sibling so we can test minSpace, etc
-    c.col = None
-    c2 = copy.deepcopy(c)
-    c2.col = c.col = col
-    c2.id = 0
+    note["Back"] = "foo"
+    note.flush()
+    c2 = note.cards()[1]
     c2.ord = 1
     c2.due = 325
     c2.flush()

qt/aqt/browser.py

@@ -411,8 +411,8 @@ def paint(
             option.direction = Qt.RightToLeft
 
         col = None
-        if c.userFlag() > 0:
-            col = getattr(colors, f"FLAG{c.userFlag()}_BG")
+        if c.user_flag() > 0:
+            col = getattr(colors, f"FLAG{c.user_flag()}_BG")
         elif c.note().has_tag("Marked"):
             col = colors.MARKED_BG
         elif c.queue == QUEUE_TYPE_SUSPENDED:
@@ -1286,13 +1286,13 @@ def onSetFlag(self, n: int) -> None:
 
     def _on_set_flag(self, n: int) -> None:
         # flag needs toggling off?
-        if n == self.card.userFlag():
+        if n == self.card.user_flag():
             n = 0
-        self.col.setUserFlag(n, self.selectedCards())
+        self.col.set_user_flag_for_cards(n, self.selectedCards())
         self.model.reset()
 
     def _updateFlagsMenu(self) -> None:
-        flag = self.card and self.card.userFlag()
+        flag = self.card and self.card.user_flag()
         flag = flag or 0
 
         f = self.form

qt/aqt/reviewer.py

@@ -216,7 +216,7 @@ def autoplay(self, card: Card) -> bool:
         return card.autoplay()
 
     def _update_flag_icon(self) -> None:
-        self.web.eval(f"_drawFlag({self.card.userFlag()});")
+        self.web.eval(f"_drawFlag({self.card.user_flag()});")
 
     def _update_mark_icon(self) -> None:
         self.web.eval(f"_drawMark({json.dumps(self.card.note().has_tag('marked'))});")
@@ -290,10 +290,10 @@ def _shortcutKeys(
             ("m", self.showContextMenu),
             ("r", self.replayAudio),
             (Qt.Key_F5, self.replayAudio),
-            ("Ctrl+1", lambda: self.setFlag(1)),
-            ("Ctrl+2", lambda: self.setFlag(2)),
-            ("Ctrl+3", lambda: self.setFlag(3)),
-            ("Ctrl+4", lambda: self.setFlag(4)),
+            ("Ctrl+1", lambda: self.set_flag_on_current_card(1)),
+            ("Ctrl+2", lambda: self.set_flag_on_current_card(2)),
+            ("Ctrl+3", lambda: self.set_flag_on_current_card(3)),
+            ("Ctrl+4", lambda: self.set_flag_on_current_card(4)),
             ("*", self.toggle_mark_on_current_note),
             ("=", self.bury_current_note),
             ("-", self.bury_current_card),
@@ -698,33 +698,33 @@ def onLeech(self, card: Card) -> None:
 
     # note the shortcuts listed here also need to be defined above
     def _contextMenu(self) -> List[Any]:
-        currentFlag = self.card and self.card.userFlag()
+        currentFlag = self.card and self.card.user_flag()
         opts = [
             [
                 tr(TR.STUDYING_FLAG_CARD),
                 [
                     [
                         tr(TR.ACTIONS_RED_FLAG),
                         "Ctrl+1",
-                        lambda: self.setFlag(1),
+                        lambda: self.set_flag_on_current_card(1),
                         dict(checked=currentFlag == 1),
                     ],
                     [
                         tr(TR.ACTIONS_ORANGE_FLAG),
                         "Ctrl+2",
-                        lambda: self.setFlag(2),
+                        lambda: self.set_flag_on_current_card(2),
                         dict(checked=currentFlag == 2),
                     ],
                     [
                         tr(TR.ACTIONS_GREEN_FLAG),
                         "Ctrl+3",
-                        lambda: self.setFlag(3),
+                        lambda: self.set_flag_on_current_card(3),
                         dict(checked=currentFlag == 3),
                     ],
                     [
                         tr(TR.ACTIONS_BLUE_FLAG),
                         "Ctrl+4",
-                        lambda: self.setFlag(4),
+                        lambda: self.set_flag_on_current_card(4),
                         dict(checked=currentFlag == 4),
                     ],
                 ],
@@ -782,12 +782,13 @@ def _addMenuItems(self, m: QMenu, rows: Sequence) -> None:
     def onOptions(self) -> None:
         self.mw.onDeckConf(self.mw.col.decks.get(self.card.odid or self.card.did))
 
-    def setFlag(self, flag: int) -> None:
+    def set_flag_on_current_card(self, flag: int) -> None:
         # need to toggle off?
-        if self.card.userFlag() == flag:
+        if self.card.user_flag() == flag:
             flag = 0
-        self.card.setUserFlag(flag)
-        self.card.flush()
+        self.card.set_user_flag(flag)
+        self.mw.col.update_card(self.card)
+        self.mw.update_undo_actions()
         self._update_flag_icon()
 
     def toggle_mark_on_current_note(self) -> None:
@@ -797,8 +798,8 @@ def toggle_mark_on_current_note(self) -> None:
         else:
             note.add_tag("marked")
         self.mw.col.update_note(note)
-        self._update_mark_icon()
         self.mw.update_undo_actions()
+        self._update_mark_icon()
 
     def on_set_due(self) -> None:
         if self.mw.state != "review" or not self.card:
@@ -863,3 +864,4 @@ def onReplayRecorded(self) -> None:
     onSuspendCard = suspend_current_card
     onDelete = delete_current_note
     onMark = toggle_mark_on_current_note
+    setFlag = set_flag_on_current_card

rslib/backend.proto

@@ -162,8 +162,7 @@ service BackendService {
   // cards
 
   rpc GetCard(CardID) returns (Card);
-  rpc UpdateCard(Card) returns (Empty);
-  rpc AddCard(Card) returns (CardID);
+  rpc UpdateCard(UpdateCardIn) returns (Empty);
   rpc RemoveCards(RemoveCardsIn) returns (Empty);
   rpc SetDeck(SetDeckIn) returns (Empty);
 
@@ -953,6 +952,11 @@ message UpdateNoteIn {
   bool skip_undo_entry = 2;
 }
 
+message UpdateCardIn {
+  Card card = 1;
+  bool skip_undo_entry = 2;
+}
+
 message EmptyCardsReport {
   message NoteWithEmptyCards {
     int64 note_id = 1;