support undoing deck mutations

This required refactoring the deck code a bit to split up the 'update'
and 'add' cases better.

rslib/src/backend/mod.rs

@@ -825,7 +825,7 @@ impl BackendService for Backend {
             if input.preserve_usn_and_mtime {
                 col.transact(None, |col| {
                     let usn = col.usn()?;
-                    col.add_or_update_single_deck(&mut deck, usn)
+                    col.add_or_update_single_deck_with_existing_id(&mut deck, usn)
                 })?;
             } else {
                 col.add_or_update_deck(&mut deck)?;

rslib/src/decks/mod.rs

@@ -5,7 +5,9 @@ pub use crate::backend_proto::{
     deck_kind::Kind as DeckKind, filtered_search_term::FilteredSearchOrder, Deck as DeckProto,
     DeckCommon, DeckKind as DeckKindProto, FilteredDeck, FilteredSearchTerm, NormalDeck,
 };
-use crate::{backend_proto as pb, markdown::render_markdown, text::sanitize_html_no_images};
+use crate::{
+    backend_proto as pb, markdown::render_markdown, text::sanitize_html_no_images, undo::Undo,
+};
 use crate::{
     collection::Collection,
     deckconf::DeckConfID,
@@ -251,7 +253,7 @@ impl Collection {
     }
 
     /// Normalize deck name and rename if not unique. Bumps mtime and usn if
-    /// deck was modified.
+    /// name was changed, but otherwise leaves it the same.
     fn prepare_deck_for_update(&mut self, deck: &mut Deck, usn: Usn) -> Result<()> {
         if let Cow::Owned(name) = normalize_native_name(&deck.name) {
             deck.name = name;
@@ -268,18 +270,28 @@ impl Collection {
         self.transact(None, |col| {
             let usn = col.usn()?;
 
-            deck.set_modified(usn);
+            col.prepare_deck_for_update(deck, usn)?;
 
             if deck.id.0 == 0 {
-                col.prepare_deck_for_update(deck, usn)?;
+                // TODO: undo support
                 col.match_or_create_parents(deck, usn)?;
+                deck.set_modified(usn);
                 col.storage.add_deck(deck)
             } else if let Some(existing_deck) = col.storage.get_deck(deck.id)? {
-                if existing_deck.name != deck.name {
+                let name_changed = existing_deck.name != deck.name;
-                    col.update_renamed_deck(existing_deck, deck, usn)
+                if name_changed {
-                } else {
+                    // match closest parent name
-                    col.add_or_update_single_deck(deck, usn)
+                    col.match_or_create_parents(deck, usn)?;
+                    // rename children
+                    col.rename_child_decks(&existing_deck, &deck.name, usn)?;
                 }
+                col.update_single_deck_no_check(deck, &existing_deck, usn)?;
+                if name_changed {
+                    // after updating, we need to ensure all grandparents exist, which may not be the case
+                    // in the parent->child case
+                    col.create_missing_parents(&deck.name, usn)?;
+                }
+                Ok(())
             } else {
                 Err(AnkiError::invalid_input("updating non-existent deck"))
             }
@@ -289,9 +301,28 @@ impl Collection {
     /// Add/update a single deck when syncing/importing. Ensures name is unique
     /// & normalized, but does not check parents/children or update mtime
     /// (unless the name was changed). Caller must set up transaction.
-    pub(crate) fn add_or_update_single_deck(&mut self, deck: &mut Deck, usn: Usn) -> Result<()> {
+    /// TODO: undo support
+    pub(crate) fn add_or_update_single_deck_with_existing_id(
+        &mut self,
+        deck: &mut Deck,
+        usn: Usn,
+    ) -> Result<()> {
         self.state.deck_cache.clear();
         self.prepare_deck_for_update(deck, usn)?;
+        self.storage.add_or_update_deck_with_existing_id(deck)
+    }
+
+    /// Update an individual, existing deck. Caller is responsible for ensuring deck
+    /// is normalized, matches parents, and is not a duplicate name. Bumps mtime.
+    pub(crate) fn update_single_deck_no_check(
+        &mut self,
+        deck: &mut Deck,
+        original: &Deck,
+        usn: Usn,
+    ) -> Result<()> {
+        self.state.deck_cache.clear();
+        deck.set_modified(usn);
+        self.save_undo(Box::new(DeckUpdated(original.clone())));
         self.storage.update_deck(deck)
     }
 
@@ -316,7 +347,7 @@ impl Collection {
         deck.id = did;
         deck.name = format!("recovered{}", did);
         deck.set_modified(usn);
-        self.add_or_update_single_deck(&mut deck, usn)
+        self.add_or_update_single_deck_with_existing_id(&mut deck, usn)
     }
 
     pub fn get_or_create_normal_deck(&mut self, human_name: &str) -> Result<Deck> {
@@ -331,37 +362,17 @@ impl Collection {
         }
     }
 
-    fn update_renamed_deck(&mut self, existing: Deck, updated: &mut Deck, usn: Usn) -> Result<()> {
-        self.state.deck_cache.clear();
-        // ensure name normalized
-        if let Cow::Owned(name) = normalize_native_name(&updated.name) {
-            updated.name = name;
-        }
-        // match closest parent name
-        self.match_or_create_parents(updated, usn)?;
-        // ensure new name is unique
-        self.ensure_deck_name_unique(updated, usn)?;
-        // rename children
-        self.rename_child_decks(&existing, &updated.name, usn)?;
-        // save deck
-        updated.set_modified(usn);
-        self.storage.update_deck(updated)?;
-        // after updating, we need to ensure all grandparents exist, which may not be the case
-        // in the parent->child case
-        self.create_missing_parents(&updated.name, usn)
-    }
-
     fn rename_child_decks(&mut self, old: &Deck, new_name: &str, usn: Usn) -> Result<()> {
         let children = self.storage.child_decks(old)?;
         let old_component_count = old.name.matches('\x1f').count() + 1;
 
         for mut child in children {
+            let original = child.clone();
             let child_components: Vec<_> = child.name.split('\x1f').collect();
             let child_only = &child_components[old_component_count..];
             let new_name = format!("{}\x1f{}", new_name, child_only.join("\x1f"));
             child.name = new_name;
-            child.set_modified(usn);
+            self.update_single_deck_no_check(&mut child, &original, usn)?;
-            self.storage.update_deck(&child)?;
         }
 
         Ok(())
@@ -475,7 +486,7 @@ impl Collection {
             // fixme: separate key
             deck.name = self.i18n.tr(TR::DeckConfigDefaultName).into();
             deck.set_modified(usn);
-            self.add_or_update_single_deck(&mut deck, usn)?;
+            self.add_or_update_single_deck_with_existing_id(&mut deck, usn)?;
         } else {
             self.storage.remove_deck(deck.id)?;
             self.storage.add_deck_grave(deck.id, usn)?;
@@ -587,10 +598,10 @@ impl Collection {
     where
         F: FnOnce(&mut DeckCommon),
     {
+        let original = deck.clone();
         deck.reset_stats_if_day_changed(today);
         mutator(&mut deck.common);
-        deck.set_modified(usn);
+        self.update_single_deck_no_check(deck, &original, usn)
-        self.add_or_update_single_deck(deck, usn)
     }
 
     pub fn drag_drop_decks(
@@ -636,6 +647,19 @@ impl Collection {
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct DeckUpdated(Deck);
+
+impl Undo for DeckUpdated {
+    fn undo(mut self: Box<Self>, col: &mut crate::collection::Collection, usn: Usn) -> Result<()> {
+        let current = col
+            .storage
+            .get_deck(self.0.id)?
+            .ok_or_else(|| AnkiError::invalid_input("deck disappeared"))?;
+        col.update_single_deck_no_check(&mut self.0, &current, usn)
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::{human_deck_name_to_native, immediate_parent_name, normalize_native_name};

rslib/src/scheduler/answering/mod.rs

@@ -262,15 +262,12 @@ impl Collection {
                 current_state, answer.current_state,
             )));
         }
-
         if let Some(revlog_partial) = updater.apply_study_state(current_state, answer.new_state)? {
             self.add_partial_revlog(revlog_partial, usn, &answer)?;
         }
         self.update_deck_stats_from_answer(usn, &answer, &updater)?;
-        let timing = updater.timing;
-
         self.maybe_bury_siblings(&original, &updater.config)?;
-
+        let timing = updater.timing;
         let mut card = updater.into_card();
         self.update_card(&mut card, &original, usn)?;
         if answer.new_state.leeched() {

rslib/src/scheduler/answering/undo.rs

@@ -92,6 +92,9 @@ mod test {
             assert_eq!(note.tags, vec!["leech".to_string()]);
             assert_eq!(col.storage.all_tags()?.is_empty(), false);
 
+            let deck = col.get_deck(DeckID(1))?.unwrap();
+            assert_eq!(deck.common.review_studied, 1);
+
             Ok(())
         };
 
@@ -113,6 +116,9 @@ mod test {
             assert_eq!(note.tags.is_empty(), true);
             assert_eq!(col.storage.all_tags()?.is_empty(), true);
 
+            let deck = col.get_deck(DeckID(1))?.unwrap();
+            assert_eq!(deck.common.review_studied, 0);
+
             Ok(())
         };
 

rslib/src/storage/deck/add_or_update_deck.sql

@@ -0,0 +1,3 @@
+INSERT
+  OR REPLACE INTO decks (id, name, mtime_secs, usn, common, kind)
+VALUES (?, ?, ?, ?, ?, ?)

rslib/src/storage/deck/mod.rs

@@ -100,23 +100,53 @@ impl SqliteStorage {
             .db
             .prepare(include_str!("alloc_id.sql"))?
             .query_row(&[TimestampMillis::now()], |r| r.get(0))?;
-        self.update_deck(deck).map_err(|err| {
+        self.add_or_update_deck_with_existing_id(deck)
-            // restore id of 0
+            .map_err(|err| {
-            deck.id.0 = 0;
+                // restore id of 0
-            err
+                deck.id.0 = 0;
-        })
+                err
+            })
     }
 
     pub(crate) fn update_deck(&self, deck: &Deck) -> Result<()> {
         if deck.id.0 == 0 {
             return Err(AnkiError::invalid_input("deck with id 0"));
         }
-        self.add_or_update_deck(deck)
+        let mut stmt = self.db.prepare_cached(include_str!("update_deck.sql"))?;
+        let mut common = vec![];
+        deck.common.encode(&mut common)?;
+        let kind_enum = DeckKindProto {
+            kind: Some(deck.kind.clone()),
+        };
+        let mut kind = vec![];
+        kind_enum.encode(&mut kind)?;
+        let count = stmt.execute(params![
+            deck.name,
+            deck.mtime_secs,
+            deck.usn,
+            common,
+            kind,
+            deck.id
+        ])?;
+
+        if count == 0 {
+            Err(AnkiError::invalid_input(
+                "update_deck() called with non-existent deck",
+            ))
+        } else {
+            Ok(())
+        }
     }
 
-    /// Used for syncing; will keep existing ID.
+    /// Used for syncing; will keep existing ID. Shouldn't be used to add new decks locally,
-    pub(crate) fn add_or_update_deck(&self, deck: &Deck) -> Result<()> {
+    /// since it does not allocate an id.
-        let mut stmt = self.db.prepare_cached(include_str!("update_deck.sql"))?;
+    pub(crate) fn add_or_update_deck_with_existing_id(&self, deck: &Deck) -> Result<()> {
+        if deck.id.0 == 0 {
+            return Err(AnkiError::invalid_input("deck with id 0"));
+        }
+        let mut stmt = self
+            .db
+            .prepare_cached(include_str!("add_or_update_deck.sql"))?;
         let mut common = vec![];
         deck.common.encode(&mut common)?;
         let kind_enum = DeckKindProto {
@@ -310,7 +340,7 @@ impl SqliteStorage {
         deck.id.0 = 1;
         // fixme: separate key
         deck.name = i18n.tr(TR::DeckConfigDefaultName).into();
-        self.update_deck(&deck)
+        self.add_or_update_deck_with_existing_id(&deck)
     }
 
     pub(crate) fn upgrade_decks_to_schema15(&self, server: bool) -> Result<()> {
@@ -332,7 +362,7 @@ impl SqliteStorage {
                 deck.name.push('_');
                 deck.set_modified(usn);
             }
-            self.update_deck(&deck)?;
+            self.add_or_update_deck_with_existing_id(&deck)?;
         }
         self.db.execute("update col set decks = ''", NO_PARAMS)?;
         Ok(())

rslib/src/storage/deck/update_deck.sql

@@ -1,3 +1,7 @@
-INSERT
+UPDATE decks
-  OR REPLACE INTO decks (id, name, mtime_secs, usn, common, kind)
+SET name = ?,
-VALUES (?, ?, ?, ?, ?, ?)
+  mtime_secs = ?,
+  usn = ?,
+  common = ?,
+  kind = ?
+WHERE id = ?

rslib/src/sync/mod.rs

@@ -882,7 +882,7 @@ impl Collection {
             if proceed {
                 let mut deck = deck.into();
                 self.ensure_deck_name_unique(&mut deck, latest_usn)?;
-                self.storage.add_or_update_deck(&deck)?;
+                self.storage.add_or_update_deck_with_existing_id(&deck)?;
                 self.state.deck_cache.remove(&deck.id);
             }
         }