From 1871b5766366e4faa3f828c038aab94c42d7863d Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sun, 13 Feb 2022 16:27:33 +1000 Subject: [PATCH] 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 --- rslib/src/card/mod.rs | 6 +++--- rslib/src/collection/transact.rs | 4 ++-- rslib/src/ops.rs | 2 -- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/rslib/src/card/mod.rs b/rslib/src/card/mod.rs index cc7102c90..ae5cd56bd 100644 --- a/rslib/src/card/mod.rs +++ b/rslib/src/card/mod.rs @@ -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; } } diff --git a/rslib/src/collection/transact.rs b/rslib/src/collection/transact.rs index e40892616..16c7575ed 100644 --- a/rslib/src/collection/transact.rs +++ b/rslib/src/collection/transact.rs @@ -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(), } }; diff --git a/rslib/src/ops.rs b/rslib/src/ops.rs index e6600790e..734df8412 100644 --- a/rslib/src/ops.rs +++ b/rslib/src/ops.rs @@ -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(),