From db02c95eb5374c851206becb1fcef0d120a2cd35 Mon Sep 17 00:00:00 2001
From: Abdo <abdo@abdnh.net>
Date: Sun, 11 Feb 2024 09:04:10 +0300
Subject: [PATCH] Fix maybe_coalesce_note_undo_entry() (#2992)

* Fix maybe_coalesce_note_undo_entry()

* Use .count()

* Only check .first()

* Explicitly match on [note change, collection modification] (dae)
---
 rslib/src/notes/undo.rs | 33 +++++++++++++++++----------------
 rslib/src/undo/mod.rs   | 19 +++++++++++++++++--
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/rslib/src/notes/undo.rs b/rslib/src/notes/undo.rs
index 095886d8f..7807042d8 100644
--- a/rslib/src/notes/undo.rs
+++ b/rslib/src/notes/undo.rs
@@ -2,6 +2,7 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 use super::NoteTags;
+use crate::collection::undo::UndoableCollectionChange;
 use crate::prelude::*;
 use crate::undo::UndoableChange;
 
@@ -64,22 +65,22 @@ impl Collection {
         if changes.op != Op::UpdateNote {
             return;
         }
-
-        if let Some(previous_op) = self.previous_undo_op() {
-            if previous_op.kind != Op::UpdateNote {
-                return;
-            }
-
-            if let (
-                Some(UndoableChange::Note(UndoableNoteChange::Updated(previous))),
-                Some(UndoableChange::Note(UndoableNoteChange::Updated(current))),
-            ) = (
-                previous_op.changes.last(),
-                self.current_undo_op().and_then(|op| op.changes.last()),
-            ) {
-                if previous.id == current.id && previous_op.timestamp.elapsed_secs() < 60 {
-                    self.pop_last_change();
-                }
+        let Some(previous_op) = self.previous_undo_op() else {
+            return;
+        };
+        if previous_op.kind != Op::UpdateNote {
+            return;
+        }
+        let Some(current_op) = self.current_undo_op() else {
+            return;
+        };
+        if let (
+            [UndoableChange::Note(UndoableNoteChange::Updated(previous)), UndoableChange::Collection(UndoableCollectionChange::Modified(_))],
+            [UndoableChange::Note(UndoableNoteChange::Updated(current)), UndoableChange::Collection(UndoableCollectionChange::Modified(_))],
+        ) = (&previous_op.changes[..], &current_op.changes[..])
+        {
+            if previous.id == current.id && previous_op.timestamp.elapsed_secs() < 60 {
+                self.clear_last_op();
             }
         }
     }
diff --git a/rslib/src/undo/mod.rs b/rslib/src/undo/mod.rs
index 80eee90ea..c3f81e84e 100644
--- a/rslib/src/undo/mod.rs
+++ b/rslib/src/undo/mod.rs
@@ -266,14 +266,14 @@ impl Collection {
     }
 
     /// Used for coalescing successive note updates.
-    pub(crate) fn pop_last_change(&mut self) -> Option<UndoableChange> {
+    pub(crate) fn clear_last_op(&mut self) {
         self.state
             .undo
             .current_step
             .as_mut()
             .expect("no operation active")
             .changes
-            .pop()
+            .clear()
     }
 
     /// Return changes made by the current op. Must only be called in a
@@ -546,4 +546,19 @@ mod test {
 
         Ok(())
     }
+
+    #[test]
+    fn coalesce_note_undo_entries() -> Result<()> {
+        let mut col = Collection::new();
+        let nt = col.get_notetype_by_name("Basic")?.unwrap();
+        let mut note = nt.new_note();
+        col.add_note(&mut note, DeckId(1))?;
+        note.set_field(0, "foo")?;
+        col.update_note(&mut note)?;
+        note.set_field(0, "bar")?;
+        col.update_note(&mut note)?;
+        assert_eq!(col.state.undo.undo_steps.len(), 2);
+
+        Ok(())
+    }
 }