update backend to support undoing of notetype changes

ftl/core/undo.ftl

@@ -24,3 +24,6 @@ undo-forget-card = Forget Card
 undo-set-flag = Set Flag
 undo-build-filtered-deck = Build Deck
 undo-expand-collapse = Expand/Collapse
+undo-add-notetype = Add Notetype
+undo-remove-notetype = Remove Notetype
+undo-update-notetype = Update Notetype

rslib/backend.proto

@@ -200,6 +200,8 @@ service ConfigService {
 }
 
 service NotetypesService {
+  rpc AddNotetype(Notetype) returns (OpChangesWithId);
+  rpc UpdateNotetype(Notetype) returns (OpChanges);
   rpc AddOrUpdateNotetype(AddOrUpdateNotetypeIn) returns (NotetypeId);
   rpc GetStockNotetypeLegacy(StockNotetype) returns (Json);
   rpc GetNotetype(NotetypeId) returns (Notetype);
@@ -207,7 +209,7 @@ service NotetypesService {
   rpc GetNotetypeNames(Empty) returns (NotetypeNames);
   rpc GetNotetypeNamesAndCounts(Empty) returns (NotetypeUseCounts);
   rpc GetNotetypeIdByName(String) returns (NotetypeId);
-  rpc RemoveNotetype(NotetypeId) returns (Empty);
+  rpc RemoveNotetype(NotetypeId) returns (OpChanges);
 }
 
 service CardRenderingService {
@@ -503,14 +505,14 @@ message Notetype {
 
     OptionalUInt32 ord = 1;
     string name = 2;
-    uint32 mtime_secs = 3;
+    int64 mtime_secs = 3;
     sint32 usn = 4;
     Config config = 5;
   }
 
   int64 id = 1;
   string name = 2;
-  uint32 mtime_secs = 3;
+  int64 mtime_secs = 3;
   sint32 usn = 4;
   Config config = 7;
   repeated Field fields = 8;

rslib/src/backend/notetypes.rs

@@ -10,14 +10,35 @@ use crate::{
 };
 
 impl NotetypesService for Backend {
+    fn add_notetype(&self, input: pb::Notetype) -> Result<pb::OpChangesWithId> {
+        let mut notetype: Notetype = input.into();
+        self.with_col(|col| {
+            Ok(col
+                .add_notetype(&mut notetype)?
+                .map(|_| notetype.id.0)
+                .into())
+        })
+    }
+
+    fn update_notetype(&self, input: pb::Notetype) -> Result<pb::OpChanges> {
+        let mut notetype: Notetype = input.into();
+        self.with_col(|col| col.update_notetype(&mut notetype))
+            .map(Into::into)
+    }
+
     fn add_or_update_notetype(&self, input: pb::AddOrUpdateNotetypeIn) -> Result<pb::NotetypeId> {
         self.with_col(|col| {
             let legacy: NotetypeSchema11 = serde_json::from_slice(&input.json)?;
             let mut nt: Notetype = legacy.into();
+            if !input.preserve_usn_and_mtime {
+                nt.set_modified(col.usn()?);
+            }
             if nt.id.0 == 0 {
                 col.add_notetype(&mut nt)?;
+            } else if !input.preserve_usn_and_mtime {
+                col.update_notetype(&mut nt)?;
             } else {
-                col.update_notetype(&mut nt, input.preserve_usn_and_mtime)?;
+                col.add_or_update_notetype_with_existing_id(&mut nt)?;
             }
             Ok(pb::NotetypeId { ntid: nt.id.0 })
         })
@@ -91,8 +112,22 @@ impl NotetypesService for Backend {
         })
     }
 
-    fn remove_notetype(&self, input: pb::NotetypeId) -> Result<pb::Empty> {
+    fn remove_notetype(&self, input: pb::NotetypeId) -> Result<pb::OpChanges> {
         self.with_col(|col| col.remove_notetype(input.into()))
             .map(Into::into)
     }
 }
+
+impl From<pb::Notetype> for Notetype {
+    fn from(n: pb::Notetype) -> Self {
+        Notetype {
+            id: n.id.into(),
+            name: n.name,
+            mtime_secs: n.mtime_secs.into(),
+            usn: n.usn.into(),
+            fields: n.fields.into_iter().map(Into::into).collect(),
+            templates: n.templates.into_iter().map(Into::into).collect(),
+            config: n.config.unwrap_or_default(),
+        }
+    }
+}

rslib/src/dbcheck.rs

@@ -259,7 +259,9 @@ impl Collection {
                 let ctx = genctx.get_or_insert_with(|| {
                     CardGenContext::new(&nt, self.get_last_deck_added_to_for_notetype(nt.id), usn)
                 });
-                self.update_note_inner_generating_cards(&ctx, &mut note, &original, false, norm)?;
+                self.update_note_inner_generating_cards(
+                    &ctx, &mut note, &original, false, norm, true,
+                )?;
             }
         }
 

rslib/src/findreplace.rs

@@ -95,6 +95,7 @@ impl Collection {
                 changed,
                 generate_cards: true,
                 mark_modified: true,
+                update_tags: false,
             })
         })
     }

rslib/src/notes/mod.rs

@@ -34,6 +34,7 @@ pub(crate) struct TransformNoteOutput {
     pub changed: bool,
     pub generate_cards: bool,
     pub mark_modified: bool,
+    pub update_tags: bool,
 }
 
 #[derive(Debug, PartialEq, Clone)]
@@ -387,7 +388,7 @@ impl Collection {
         let last_deck = self.get_last_deck_added_to_for_notetype(note.notetype_id);
         let ctx = CardGenContext::new(&nt, last_deck, self.usn()?);
         let norm = self.get_bool(BoolKey::NormalizeNoteText);
-        self.update_note_inner_generating_cards(&ctx, note, &existing_note, true, norm)?;
+        self.update_note_inner_generating_cards(&ctx, note, &existing_note, true, norm, true)?;
         Ok(())
     }
 
@@ -398,6 +399,7 @@ impl Collection {
         original: &Note,
         mark_note_modified: bool,
         normalize_text: bool,
+        update_tags: bool,
     ) -> Result<()> {
         self.update_note_inner_without_cards(
             note,
@@ -406,10 +408,13 @@ impl Collection {
             ctx.usn,
             mark_note_modified,
             normalize_text,
+            update_tags,
         )?;
         self.generate_cards_for_existing_note(ctx, note)
     }
 
+    // TODO: refactor into struct
+    #[allow(clippy::clippy::too_many_arguments)]
     pub(crate) fn update_note_inner_without_cards(
         &mut self,
         note: &mut Note,
@@ -418,8 +423,11 @@ impl Collection {
         usn: Usn,
         mark_note_modified: bool,
         normalize_text: bool,
+        update_tags: bool,
     ) -> Result<()> {
-        self.canonify_note_tags(note, usn)?;
+        if update_tags {
+            self.canonify_note_tags(note, usn)?;
+        }
         note.prepare_for_update(nt, normalize_text)?;
         if mark_note_modified {
             note.set_modified(usn);
@@ -453,6 +461,7 @@ impl Collection {
                 changed: true,
                 generate_cards,
                 mark_modified: mark_notes_modified,
+                update_tags: true,
             })
         })
     }
@@ -499,6 +508,7 @@ impl Collection {
                         &original,
                         out.mark_modified,
                         norm,
+                        out.update_tags,
                     )?;
                 } else {
                     self.update_note_inner_without_cards(
@@ -508,6 +518,7 @@ impl Collection {
                         usn,
                         out.mark_modified,
                         norm,
+                        out.update_tags,
                     )?;
                 }
 
@@ -570,6 +581,7 @@ impl Collection {
                 changed,
                 generate_cards: false,
                 mark_modified: true,
+                update_tags: true,
             })
         })
         .map(|_| ())

rslib/src/notetype/fields.rs

@@ -7,7 +7,7 @@ use crate::{
     error::{AnkiError, Result},
 };
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone)]
 pub struct NoteField {
     pub ord: Option<u32>,
     pub name: String,
@@ -24,6 +24,16 @@ impl From<NoteField> for NoteFieldProto {
     }
 }
 
+impl From<NoteFieldProto> for NoteField {
+    fn from(f: NoteFieldProto) -> Self {
+        NoteField {
+            ord: f.ord.map(|n| n.val),
+            name: f.name,
+            config: f.config.unwrap_or_default(),
+        }
+    }
+}
+
 impl NoteField {
     pub fn new(name: impl Into<String>) -> Self {
         NoteField {

rslib/src/notetype/mod.rs

@@ -9,6 +9,7 @@ mod schema11;
 mod schemachange;
 mod stock;
 mod templates;
+pub(crate) mod undo;
 
 use std::{
     collections::{HashMap, HashSet},
@@ -48,7 +49,7 @@ pub(crate) const DEFAULT_CSS: &str = include_str!("styling.css");
 pub(crate) const DEFAULT_LATEX_HEADER: &str = include_str!("header.tex");
 pub(crate) const DEFAULT_LATEX_FOOTER: &str = r"\end{document}";
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone)]
 pub struct Notetype {
     pub id: NotetypeId,
     pub name: String,
@@ -97,41 +98,39 @@ impl Notetype {
 }
 
 impl Collection {
+    /// Add a new notetype, and allocate it an ID.
+    pub fn add_notetype(&mut self, notetype: &mut Notetype) -> Result<OpOutput<()>> {
+        self.transact(Op::AddNotetype, |col| {
+            let usn = col.usn()?;
+            notetype.set_modified(usn);
+            col.add_notetype_inner(notetype, usn)
+        })
+    }
+
     /// Saves changes to a note type. This will force a full sync if templates
     /// or fields have been added/removed/reordered.
-    pub fn update_notetype(&mut self, nt: &mut Notetype, preserve_usn: bool) -> Result<()> {
+    pub fn update_notetype(&mut self, notetype: &mut Notetype) -> Result<OpOutput<()>> {
-        let existing = self.get_notetype(nt.id)?;
+        self.transact(Op::UpdateNotetype, |col| {
-        let norm = self.get_bool(BoolKey::NormalizeNoteText);
+            let original = col
-        nt.prepare_for_update(existing.as_ref().map(AsRef::as_ref))?;
+                .storage
-        self.transact_no_undo(|col| {
+                .get_notetype(notetype.id)?
-            if let Some(existing_notetype) = existing {
+                .ok_or(AnkiError::NotFound)?;
-                if existing_notetype.mtime_secs > nt.mtime_secs {
-                    return Err(AnkiError::invalid_input("attempt to save stale notetype"));
-                }
-                col.update_notes_for_changed_fields(
-                    nt,
-                    existing_notetype.fields.len(),
-                    existing_notetype.config.sort_field_idx,
-                    norm,
-                )?;
-                col.update_cards_for_changed_templates(nt, existing_notetype.templates.len())?;
-            }
-
             let usn = col.usn()?;
-            if !preserve_usn {
+            notetype.set_modified(usn);
-                nt.set_modified(usn);
+            col.add_or_update_notetype_with_existing_id_inner(notetype, Some(original), usn)
-            }
+        })
-            col.ensure_notetype_name_unique(nt, usn)?;
+    }
 
-            col.storage.update_notetype_config(&nt)?;
+    /// Used to support the current importing code; does not mark notetype as modified,
-            col.storage.update_notetype_fields(nt.id, &nt.fields)?;
+    /// and does not support undo.
-            col.storage
+    pub fn add_or_update_notetype_with_existing_id(
-                .update_notetype_templates(nt.id, &nt.templates)?;
+        &mut self,
-
+        notetype: &mut Notetype,
-            // fixme: update cache instead of clearing
+    ) -> Result<()> {
-            col.state.notetype_cache.remove(&nt.id);
+        self.transact_no_undo(|col| {
-
+            let usn = col.usn()?;
-            Ok(())
+            let existing = col.storage.get_notetype(notetype.id)?;
+            col.add_or_update_notetype_with_existing_id_inner(notetype, existing, usn)
         })
     }
 
@@ -169,8 +168,8 @@ impl Collection {
             .collect()
     }
 
-    pub fn remove_notetype(&mut self, ntid: NotetypeId) -> Result<()> {
+    pub fn remove_notetype(&mut self, ntid: NotetypeId) -> Result<OpOutput<()>> {
-        self.transact_no_undo(|col| col.remove_notetype_inner(ntid))
+        self.transact(Op::RemoveNotetype, |col| col.remove_notetype_inner(ntid))
     }
 }
 
@@ -437,7 +436,7 @@ impl From<Notetype> for NotetypeProto {
         NotetypeProto {
             id: nt.id.0,
             name: nt.name,
-            mtime_secs: nt.mtime_secs.0 as u32,
+            mtime_secs: nt.mtime_secs.0,
             usn: nt.usn.0,
             config: Some(nt.config),
             fields: nt.fields.into_iter().map(Into::into).collect(),
@@ -447,21 +446,6 @@ impl From<Notetype> for NotetypeProto {
 }
 
 impl Collection {
-    /// Add a new notetype, and allocate it an ID.
-    pub fn add_notetype(&mut self, nt: &mut Notetype) -> Result<()> {
-        self.transact_no_undo(|col| {
-            let usn = col.usn()?;
-            nt.set_modified(usn);
-            col.add_notetype_inner(nt, usn)
-        })
-    }
-
-    pub(crate) fn add_notetype_inner(&mut self, nt: &mut Notetype, usn: Usn) -> Result<()> {
-        nt.prepare_for_update(None)?;
-        self.ensure_notetype_name_unique(nt, usn)?;
-        self.storage.add_new_notetype(nt)
-    }
-
     pub(crate) fn ensure_notetype_name_unique(
         &self,
         notetype: &mut Notetype,
@@ -482,7 +466,52 @@ impl Collection {
         Ok(())
     }
 
+    /// Caller must set notetype as modified if appropriate.
+    pub(crate) fn add_notetype_inner(&mut self, notetype: &mut Notetype, usn: Usn) -> Result<()> {
+        notetype.prepare_for_update(None)?;
+        self.ensure_notetype_name_unique(notetype, usn)?;
+        self.add_notetype_undoable(notetype)
+    }
+
+    /// - Caller must set notetype as modified if appropriate.
+    /// - This only supports undo when an existing notetype is passed in.
+    fn add_or_update_notetype_with_existing_id_inner(
+        &mut self,
+        notetype: &mut Notetype,
+        original: Option<Notetype>,
+        usn: Usn,
+    ) -> Result<()> {
+        let normalize = self.get_bool(BoolKey::NormalizeNoteText);
+        notetype.prepare_for_update(original.as_ref())?;
+        self.ensure_notetype_name_unique(notetype, usn)?;
+        self.state.notetype_cache.remove(&notetype.id);
+
+        if let Some(original) = original {
+            self.update_notes_for_changed_fields(
+                notetype,
+                original.fields.len(),
+                original.config.sort_field_idx,
+                normalize,
+            )?;
+            self.update_cards_for_changed_templates(notetype, original.templates.len())?;
+            self.update_notetype_undoable(notetype, original)?;
+        } else {
+            // adding with existing id for old undo code, bypass undo
+            self.storage
+                .add_or_update_notetype_with_existing_id(&notetype)?;
+        }
+
+        Ok(())
+    }
+
     fn remove_notetype_inner(&mut self, ntid: NotetypeId) -> Result<()> {
+        let notetype = if let Some(notetype) = self.storage.get_notetype(ntid)? {
+            notetype
+        } else {
+            // already removed
+            return Ok(());
+        };
+
         // remove associated cards/notes
         let usn = self.usn()?;
         let note_ids = self.search_notes_unordered(ntid)?;
@@ -492,7 +521,7 @@ impl Collection {
         self.set_schema_modified()?;
         self.state.notetype_cache.remove(&ntid);
         self.clear_aux_config_for_notetype(ntid)?;
-        self.storage.remove_notetype(ntid)?;
+        self.remove_notetype_only_undoable(notetype)?;
 
         // update last-used notetype
         let all = self.storage.get_all_notetype_names()?;

rslib/src/notetype/schemachange.rs

@@ -1,8 +1,15 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
+use std::collections::HashMap;
+
 use super::{CardGenContext, Notetype};
-use crate::{collection::Collection, error::Result};
+use crate::{
+    collection::Collection,
+    error::Result,
+    match_all,
+    search::{Node, SortMode, TemplateKind},
+};
 
 /// True if any ordinals added, removed or reordered.
 fn ords_changed(ords: &[Option<u32>], previous_len: usize) -> bool {
@@ -16,15 +23,15 @@ fn ords_changed(ords: &[Option<u32>], previous_len: usize) -> bool {
 #[derive(Default, PartialEq, Debug)]
 struct TemplateOrdChanges {
     added: Vec<u32>,
-    removed: Vec<u32>,
+    removed: Vec<u16>,
     // map of old->new
-    moved: Vec<(u32, u32)>,
+    moved: HashMap<u16, u16>,
 }
 
 impl TemplateOrdChanges {
     fn new(ords: Vec<Option<u32>>, previous_len: u32) -> Self {
         let mut changes = TemplateOrdChanges::default();
-        let mut removed: Vec<_> = (0..previous_len).map(|v| Some(v as u32)).collect();
+        let mut removed: Vec<_> = (0..previous_len).map(|v| Some(v as u16)).collect();
         for (idx, old_ord) in ords.into_iter().enumerate() {
             if let Some(old_ord) = old_ord {
                 if let Some(entry) = removed.get_mut(old_ord as usize) {
@@ -34,7 +41,7 @@ impl TemplateOrdChanges {
                 if old_ord == idx as u32 {
                     // no action
                 } else {
-                    changes.moved.push((old_ord as u32, idx as u32));
+                    changes.moved.insert(old_ord as u16, idx as u16);
                 }
             } else {
                 changes.added.push(idx as u32);
@@ -57,6 +64,7 @@ impl Collection {
         previous_sort_idx: u32,
         normalize_text: bool,
     ) -> Result<()> {
+        let usn = self.usn()?;
         let ords: Vec<_> = nt.fields.iter().map(|f| f.ord).collect();
         if !ords_changed(&ords, previous_field_count) {
             if nt.config.sort_field_idx != previous_sort_idx {
@@ -64,8 +72,16 @@ impl Collection {
                 let nids = self.search_notes_unordered(nt.id)?;
                 for nid in nids {
                     let mut note = self.storage.get_note(nid)?.unwrap();
-                    note.prepare_for_update(nt, normalize_text)?;
+                    let original = note.clone();
-                    self.storage.update_note(&note)?;
+                    self.update_note_inner_without_cards(
+                        &mut note,
+                        &original,
+                        nt,
+                        usn,
+                        true,
+                        normalize_text,
+                        false,
+                    )?;
                 }
                 return Ok(());
             } else {
@@ -74,11 +90,13 @@ impl Collection {
             }
         }
 
+        // fields have changed
         self.set_schema_modified()?;
         let nids = self.search_notes_unordered(nt.id)?;
         let usn = self.usn()?;
         for nid in nids {
             let mut note = self.storage.get_note(nid)?.unwrap();
+            let original = note.clone();
             *note.fields_mut() = ords
                 .iter()
                 .map(|f| {
@@ -93,9 +111,15 @@ impl Collection {
                 })
                 .map(Into::into)
                 .collect();
-            note.prepare_for_update(nt, normalize_text)?;
+            self.update_note_inner_without_cards(
-            note.set_modified(usn);
+                &mut note,
-            self.storage.update_note(&note)?;
+                &original,
+                nt,
+                usn,
+                true,
+                normalize_text,
+                false,
+            )?;
         }
         Ok(())
     }
@@ -115,15 +139,42 @@ impl Collection {
         }
 
         self.set_schema_modified()?;
-
+        let usn = self.usn()?;
         let changes = TemplateOrdChanges::new(ords, previous_template_count as u32);
+
+        // remove any cards where the template was deleted
         if !changes.removed.is_empty() {
-            self.storage
+            let ords = Node::any(
-                .remove_cards_for_deleted_templates(nt.id, &changes.removed)?;
+                changes
+                    .removed
+                    .into_iter()
+                    .map(TemplateKind::Ordinal)
+                    .map(Into::into),
+            );
+            self.search_cards_into_table(match_all![nt.id, ords], SortMode::NoOrder)?;
+            for card in self.storage.all_searched_cards()? {
+                self.remove_card_and_add_grave_undoable(card, usn)?;
+            }
+            self.storage.clear_searched_cards_table()?;
         }
+
+        // update ordinals for cards with a repositioned template
         if !changes.moved.is_empty() {
-            self.storage
+            let ords = Node::any(
-                .move_cards_for_repositioned_templates(nt.id, &changes.moved)?;
+                changes
+                    .moved
+                    .keys()
+                    .cloned()
+                    .map(TemplateKind::Ordinal)
+                    .map(Into::into),
+            );
+            self.search_cards_into_table(match_all![nt.id, ords], SortMode::NoOrder)?;
+            for mut card in self.storage.all_searched_cards()? {
+                let original = card.clone();
+                card.template_idx = *changes.moved.get(&card.template_idx).unwrap();
+                self.update_card_inner(&mut card, original, usn)?;
+            }
+            self.storage.clear_searched_cards_table()?;
         }
 
         let last_deck = self.get_last_deck_added_to_for_notetype(nt.id);
@@ -165,7 +216,7 @@ mod test {
             TemplateOrdChanges::new(vec![Some(1)], 2),
             TemplateOrdChanges {
                 removed: vec![0],
-                moved: vec![(1, 0)],
+                moved: vec![(1, 0)].into_iter().collect(),
                 ..Default::default()
             }
         );
@@ -180,7 +231,7 @@ mod test {
             TemplateOrdChanges::new(vec![Some(2), None, Some(0)], 2),
             TemplateOrdChanges {
                 added: vec![1],
-                moved: vec![(2, 0), (0, 2)],
+                moved: vec![(2, 0), (0, 2)].into_iter().collect(),
                 removed: vec![1],
             }
         );
@@ -188,7 +239,7 @@ mod test {
             TemplateOrdChanges::new(vec![None, Some(2), None, Some(4)], 5),
             TemplateOrdChanges {
                 added: vec![0, 2],
-                moved: vec![(2, 1), (4, 3)],
+                moved: vec![(2, 1), (4, 3)].into_iter().collect(),
                 removed: vec![0, 1, 3],
             }
         );
@@ -208,13 +259,13 @@ mod test {
         col.add_note(&mut note, DeckId(1))?;
 
         nt.add_field("three");
-        col.update_notetype(&mut nt, false)?;
+        col.update_notetype(&mut nt)?;
 
         let note = col.storage.get_note(note.id)?.unwrap();
         assert_eq!(note.fields(), &["one".to_string(), "two".into(), "".into()]);
 
         nt.fields.remove(1);
-        col.update_notetype(&mut nt, false)?;
+        col.update_notetype(&mut nt)?;
 
         let note = col.storage.get_note(note.id)?.unwrap();
         assert_eq!(note.fields(), &["one".to_string(), "".into()]);
@@ -231,13 +282,13 @@ mod test {
             .unwrap();
         nt.templates[0].config.q_format += "\n{{#Front}}{{some:Front}}{{Back}}{{/Front}}";
         nt.fields[0].name = "Test".into();
-        col.update_notetype(&mut nt, false)?;
+        col.update_notetype(&mut nt)?;
         assert_eq!(
             &nt.templates[0].config.q_format,
             "{{Test}}\n{{#Test}}{{some:Test}}{{Back}}{{/Test}}"
         );
         nt.fields.remove(0);
-        col.update_notetype(&mut nt, false)?;
+        col.update_notetype(&mut nt)?;
         assert_eq!(&nt.templates[0].config.q_format, "\n{{Back}}");
 
         Ok(())
@@ -263,7 +314,7 @@ mod test {
 
         // add an extra card template
         nt.add_template("card 2", "{{Front}}", "");
-        col.update_notetype(&mut nt, false)?;
+        col.update_notetype(&mut nt)?;
 
         assert_eq!(
             col.search_cards(note.id, SortMode::NoOrder).unwrap().len(),

rslib/src/notetype/stock.rs

@@ -16,7 +16,7 @@ impl SqliteStorage {
     pub(crate) fn add_stock_notetypes(&self, tr: &I18n) -> Result<()> {
         for (idx, mut nt) in all_stock_notetypes(tr).into_iter().enumerate() {
             nt.prepare_for_update(None)?;
-            self.add_new_notetype(&mut nt)?;
+            self.add_notetype(&mut nt)?;
             if idx == Kind::Basic as usize {
                 self.set_config_entry(&ConfigEntry::boxed(
                     ConfigKey::CurrentNotetypeId.into(),

rslib/src/notetype/templates.rs

@@ -11,7 +11,7 @@ use crate::{
     types::Usn,
 };
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone)]
 pub struct CardTemplate {
     pub ord: Option<u32>,
     pub mtime_secs: TimestampSecs,
@@ -58,7 +58,7 @@ impl From<CardTemplate> for CardTemplateProto {
     fn from(t: CardTemplate) -> Self {
         CardTemplateProto {
             ord: t.ord.map(|n| OptionalUInt32 { val: n }),
-            mtime_secs: t.mtime_secs.0 as u32,
+            mtime_secs: t.mtime_secs.0,
             usn: t.usn.0,
             name: t.name,
             config: Some(t.config),
@@ -66,6 +66,18 @@ impl From<CardTemplate> for CardTemplateProto {
     }
 }
 
+impl From<CardTemplateProto> for CardTemplate {
+    fn from(t: CardTemplateProto) -> Self {
+        CardTemplate {
+            ord: t.ord.map(|n| n.val),
+            mtime_secs: t.mtime_secs.into(),
+            usn: t.usn.into(),
+            name: t.name,
+            config: t.config.unwrap_or_default(),
+        }
+    }
+}
+
 impl CardTemplate {
     pub fn new<S1, S2, S3>(name: S1, qfmt: S2, afmt: S3) -> Self
     where

rslib/src/notetype/undo.rs

@@ -0,0 +1,60 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use crate::prelude::*;
+
+#[derive(Debug)]
+
+pub(crate) enum UndoableNotetypeChange {
+    Added(Box<Notetype>),
+    Updated(Box<Notetype>),
+    Removed(Box<Notetype>),
+}
+
+impl Collection {
+    pub(crate) fn undo_notetype_change(&mut self, change: UndoableNotetypeChange) -> Result<()> {
+        match change {
+            UndoableNotetypeChange::Added(nt) => self.remove_notetype_only_undoable(*nt),
+            UndoableNotetypeChange::Updated(nt) => {
+                let current = self
+                    .storage
+                    .get_notetype(nt.id)?
+                    .ok_or_else(|| AnkiError::invalid_input("notetype disappeared"))?;
+                self.update_notetype_undoable(&nt, current)
+            }
+            UndoableNotetypeChange::Removed(nt) => self.restore_deleted_notetype(*nt),
+        }
+    }
+
+    pub(crate) fn remove_notetype_only_undoable(&mut self, notetype: Notetype) -> Result<()> {
+        self.storage.remove_notetype(notetype.id)?;
+        self.save_undo(UndoableNotetypeChange::Removed(Box::new(notetype)));
+        Ok(())
+    }
+
+    pub(super) fn add_notetype_undoable(
+        &mut self,
+        notetype: &mut Notetype,
+    ) -> Result<(), AnkiError> {
+        self.storage.add_notetype(notetype)?;
+        self.save_undo(UndoableNotetypeChange::Added(Box::new(notetype.clone())));
+        Ok(())
+    }
+
+    pub(super) fn update_notetype_undoable(
+        &mut self,
+        notetype: &Notetype,
+        original: Notetype,
+    ) -> Result<()> {
+        self.save_undo(UndoableNotetypeChange::Updated(Box::new(original)));
+        self.storage
+            .add_or_update_notetype_with_existing_id(notetype)
+    }
+
+    fn restore_deleted_notetype(&mut self, notetype: Notetype) -> Result<()> {
+        self.storage
+            .add_or_update_notetype_with_existing_id(&notetype)?;
+        self.save_undo(UndoableNotetypeChange::Added(Box::new(notetype)));
+        Ok(())
+    }
+}

rslib/src/ops.rs

@@ -7,6 +7,7 @@ use crate::prelude::*;
 pub enum Op {
     AddDeck,
     AddNote,
+    AddNotetype,
     AnswerCard,
     BuildFilteredDeck,
     Bury,
@@ -17,6 +18,7 @@ pub enum Op {
     RebuildFilteredDeck,
     RemoveDeck,
     RemoveNote,
+    RemoveNotetype,
     RemoveTag,
     RenameDeck,
     ReparentDeck,
@@ -35,6 +37,7 @@ pub enum Op {
     UpdateNote,
     UpdatePreferences,
     UpdateTag,
+    UpdateNotetype,
     SetCurrentDeck,
 }
 
@@ -72,6 +75,9 @@ impl Op {
             Op::ExpandCollapse => tr.undo_expand_collapse(),
             Op::SetCurrentDeck => tr.browsing_change_deck(),
             Op::UpdateDeckConfig => tr.deck_config_title(),
+            Op::AddNotetype => tr.undo_add_notetype(),
+            Op::RemoveNotetype => tr.undo_remove_notetype(),
+            Op::UpdateNotetype => tr.undo_update_notetype(),
         }
         .into()
     }

rslib/src/storage/notetype/delete_cards_for_template.sql

@@ -1,7 +0,0 @@
-DELETE FROM cards
-WHERE nid IN (
-    SELECT id
-    FROM notes
-    WHERE mid = ?
-  )
-  AND ord = ?;

rslib/src/storage/notetype/mod.rs

@@ -114,11 +114,7 @@ impl SqliteStorage {
             .collect()
     }
 
-    pub(crate) fn update_notetype_fields(
+    fn update_notetype_fields(&self, ntid: NotetypeId, fields: &[NoteField]) -> Result<()> {
-        &self,
-        ntid: NotetypeId,
-        fields: &[NoteField],
-    ) -> Result<()> {
         self.db
             .prepare_cached("delete from fields where ntid=?")?
             .execute(&[ntid])?;
@@ -166,7 +162,7 @@ impl SqliteStorage {
             .collect()
     }
 
-    pub(crate) fn update_notetype_templates(
+    fn update_notetype_templates(
         &self,
         ntid: NotetypeId,
         templates: &[CardTemplate],
@@ -193,11 +189,14 @@ impl SqliteStorage {
         Ok(())
     }
 
-    pub(crate) fn update_notetype_config(&self, nt: &Notetype) -> Result<()> {
+    /// Notetype should have an existing id, and will be added if missing.
-        assert!(nt.id.0 != 0);
+    fn update_notetype_core(&self, nt: &Notetype) -> Result<()> {
-        let mut stmt = self
+        if nt.id.0 == 0 {
-            .db
+            return Err(AnkiError::invalid_input(
-            .prepare_cached(include_str!("update_notetype_core.sql"))?;
+                "notetype with id 0 passed in as existing",
+            ));
+        }
+        let mut stmt = self.db.prepare_cached(include_str!("add_or_update.sql"))?;
         let mut config_bytes = vec![];
         nt.config.encode(&mut config_bytes)?;
         stmt.execute(params![nt.id, nt.name, nt.mtime_secs, nt.usn, config_bytes])?;
@@ -205,7 +204,7 @@ impl SqliteStorage {
         Ok(())
     }
 
-    pub(crate) fn add_new_notetype(&self, nt: &mut Notetype) -> Result<()> {
+    pub(crate) fn add_notetype(&self, nt: &mut Notetype) -> Result<()> {
         assert!(nt.id.0 == 0);
 
         let mut stmt = self.db.prepare_cached(include_str!("add_notetype.sql"))?;
@@ -226,33 +225,15 @@ impl SqliteStorage {
         Ok(())
     }
 
-    /// Used for syncing.
+    /// Used for both regular updates, and for syncing/import.
-    pub(crate) fn add_or_update_notetype(&self, nt: &Notetype) -> Result<()> {
+    pub(crate) fn add_or_update_notetype_with_existing_id(&self, nt: &Notetype) -> Result<()> {
-        let mut stmt = self.db.prepare_cached(include_str!("add_or_update.sql"))?;
+        self.update_notetype_core(nt)?;
-        let mut config_bytes = vec![];
-        nt.config.encode(&mut config_bytes)?;
-        stmt.execute(params![nt.id, nt.name, nt.mtime_secs, nt.usn, config_bytes])?;
-
         self.update_notetype_fields(nt.id, &nt.fields)?;
         self.update_notetype_templates(nt.id, &nt.templates)?;
 
         Ok(())
     }
 
-    pub(crate) fn remove_cards_for_deleted_templates(
-        &self,
-        ntid: NotetypeId,
-        ords: &[u32],
-    ) -> Result<()> {
-        let mut stmt = self
-            .db
-            .prepare(include_str!("delete_cards_for_template.sql"))?;
-        for ord in ords {
-            stmt.execute(params![ntid, ord])?;
-        }
-        Ok(())
-    }
-
     pub(crate) fn remove_notetype(&self, ntid: NotetypeId) -> Result<()> {
         self.db
             .prepare_cached("delete from templates where ntid=?")?
@@ -267,29 +248,6 @@ impl SqliteStorage {
         Ok(())
     }
 
-    pub(crate) fn move_cards_for_repositioned_templates(
-        &self,
-        ntid: NotetypeId,
-        changes: &[(u32, u32)],
-    ) -> Result<()> {
-        let case_clauses: Vec<_> = changes
-            .iter()
-            .map(|(old, new)| format!("when {} then {}", old, new))
-            .collect();
-        let mut sql = format!(
-            "update cards set ord = (case ord {} end) 
-where nid in (select id from notes where mid = ?)
-and ord in ",
-            case_clauses.join(" ")
-        );
-        ids_to_string(
-            &mut sql,
-            &changes.iter().map(|(old, _)| old).collect::<Vec<_>>(),
-        );
-        self.db.prepare(&sql)?.execute(&[ntid])?;
-        Ok(())
-    }
-
     pub(crate) fn existing_cards_for_notetype(
         &self,
         ntid: NotetypeId,
@@ -363,7 +321,7 @@ and ord in ",
                 }
                 nt.name.push('_');
             }
-            self.update_notetype_config(&nt)?;
+            self.update_notetype_core(&nt)?;
             self.update_notetype_fields(ntid, &nt.fields)?;
             self.update_notetype_templates(ntid, &nt.templates)?;
         }

rslib/src/storage/notetype/update_notetype_core.sql

@@ -1,3 +0,0 @@
-INSERT
-  OR REPLACE INTO notetypes (id, name, mtime_secs, usn, config)
-VALUES (?, ?, ?, ?, ?)

rslib/src/sync/mod.rs

@@ -860,7 +860,7 @@ impl Collection {
             };
             if proceed {
                 self.ensure_notetype_name_unique(&mut nt, latest_usn)?;
-                self.storage.add_or_update_notetype(&nt)?;
+                self.storage.add_or_update_notetype_with_existing_id(&nt)?;
                 self.state.notetype_cache.remove(&nt.id);
             }
         }
@@ -1503,7 +1503,7 @@ mod test {
 
         let mut nt = col2.storage.get_notetype(nt.id)?.unwrap();
         nt.name = "newer".into();
-        col2.update_notetype(&mut nt, false)?;
+        col2.update_notetype(&mut nt)?;
 
         // sync the changes back
         let out = ctx.normal_sync(&mut col2).await;

rslib/src/undo/changes.rs

@@ -4,9 +4,9 @@
 use crate::{
     card::undo::UndoableCardChange, collection::undo::UndoableCollectionChange,
     config::undo::UndoableConfigChange, deckconfig::undo::UndoableDeckConfigChange,
-    decks::undo::UndoableDeckChange, notes::undo::UndoableNoteChange, prelude::*,
+    decks::undo::UndoableDeckChange, notes::undo::UndoableNoteChange,
-    revlog::undo::UndoableRevlogChange, scheduler::queue::undo::UndoableQueueChange,
+    notetype::undo::UndoableNotetypeChange, prelude::*, revlog::undo::UndoableRevlogChange,
-    tags::undo::UndoableTagChange,
+    scheduler::queue::undo::UndoableQueueChange, tags::undo::UndoableTagChange,
 };
 
 #[derive(Debug)]
@@ -20,6 +20,7 @@ pub(crate) enum UndoableChange {
     Queue(UndoableQueueChange),
     Config(UndoableConfigChange),
     Collection(UndoableCollectionChange),
+    Notetype(UndoableNotetypeChange),
 }
 
 impl UndoableChange {
@@ -34,6 +35,7 @@ impl UndoableChange {
             UndoableChange::Config(c) => col.undo_config_change(c),
             UndoableChange::DeckConfig(c) => col.undo_deck_config_change(c),
             UndoableChange::Collection(c) => col.undo_collection_change(c),
+            UndoableChange::Notetype(c) => col.undo_notetype_change(c),
         }
     }
 }
@@ -91,3 +93,9 @@ impl From<UndoableCollectionChange> for UndoableChange {
         UndoableChange::Collection(c)
     }
 }
+
+impl From<UndoableNotetypeChange> for UndoableChange {
+    fn from(c: UndoableNotetypeChange) -> Self {
+        UndoableChange::Notetype(c)
+    }
+}

rslib/src/undo/mod.rs

@@ -143,6 +143,7 @@ impl UndoManager {
                 UndoableChange::Config(_) => changes.config = true,
                 UndoableChange::DeckConfig(_) => changes.deck_config = true,
                 UndoableChange::Collection(_) => {}
+                UndoableChange::Notetype(_) => changes.notetype = true,
             }
         }