don't put flag changes in the undo queue, and don't bump mtime

This is not ideal, but I struggled to come up with a better solution.

Background:

- The scheduler records the mtime of cards as it's building the queues,
and will throw an error in get_queued_cards() if the card on the DB
has a different mtime. This is to catch bugs - any operation that modifies
cards should be triggering a queue rebuild, or should adjust the queues
appropriately.
- The review screen skips the usual queue rebuild redraw, and directly
updates the flag icon. This is because a rebuild could cause a different
card to appear, or the answer side to switch back to the question side,
neither of which the user expects when they flag a card.

The current behaviour was broken: the queue rebuilding was still happening
on the backend, and the frontend was just failing to reflect it.

I initially tried to special-case Op::SetFlag, having it skip the queue
rebuild, and having set_card_flag() update the mtimes in the active
queue. But those mutations weren't captured by the undo log, so they
didn't get undone when undoing the set flag operation. We could perhaps
work around it by adding a separate undo entry to capture the mutation,
but it started to feel like it would be a pain to maintain moving forward.

By skipping the undo queue and retaining the same mtime, no queue
rebuild is required. Because we're setting usn, the cards will still
sync, but as mtime is not bumped, in the case of a conflict, an older
unsynced change from another client may revert the flag change.

Fixes https://forums.ankiweb.net/t/anki-2-1-50-beta-1-2/15608/145
This commit is contained in:
Damien Elmes 2022-02-13 16:27:33 +10:00
parent 1253dc3a3b
commit 1871b57663
3 changed files with 5 additions and 7 deletions

View File

@ -278,12 +278,12 @@ impl Collection {
self.storage.set_search_table_to_card_ids(cards, false)?;
let usn = self.usn()?;
self.transact(Op::SetFlag, |col| {
self.transact(Op::SkipUndo, |col| {
let mut count = 0;
for mut card in col.storage.all_searched_cards()? {
let original = card.clone();
if card.set_flag(flag) {
col.update_card_inner(&mut card, original, usn)?;
card.usn = usn;
col.storage.update_card(&card)?;
count += 1;
}
}

View File

@ -31,9 +31,9 @@ impl Collection {
changes
} else {
self.clear_study_queues();
// dummy value for transact_no_undo() case
// dummy value that will be discarded
OpChanges {
op: Op::SetFlag,
op: Op::SkipUndo,
changes: StateChanges::default(),
}
};

View File

@ -29,7 +29,6 @@ pub enum Op {
ScheduleAsNew,
SetCardDeck,
SetDueDate,
SetFlag,
SortCards,
Suspend,
UnburyUnsuspend,
@ -68,7 +67,6 @@ impl Op {
Op::UpdatePreferences => tr.preferences_preferences(),
Op::UpdateTag => tr.actions_update_tag(),
Op::SetCardDeck => tr.browsing_change_deck(),
Op::SetFlag => tr.actions_set_flag(),
Op::FindAndReplace => tr.browsing_find_and_replace(),
Op::ClearUnusedTags => tr.browsing_clear_unused_tags(),
Op::SortCards => tr.browsing_reschedule(),