Engine: a grouped action that closes its document succeeds

Session::grouped ended its group after the action, and when the action had closed the group's document (file.close, or file.open taking the place of an untouched new workbook) the group had gone with it, so the action was reported as failed although it had done what it was asked. A group that is gone because its document is gone now counts as ended; a group ended by someone else while the document is open is still an error.

crates/engine/src/lib.rs

@@ -652,11 +652,15 @@ impl Session {
     /// is rolled back and its error is returned. Groups nest; the outermost one makes the step.
     pub fn grouped<R>(&mut self, label: &str, f: impl FnOnce(&mut Session) -> Result<R>) -> Result<R> {
         let token = self.group_begin(label)?;
+        let doc = token.doc();
         match f(self) {
-            Ok(r) => {
-                self.group_end(token)?;
-                Ok(r)
-            }
+            Ok(r) => match self.group_end(token) {
+                Ok(_) => Ok(r),
+                // `f` closed the group's document (`file.close`, or `file.open` taking the place of
+                // an untouched new workbook), and the group went with it: nothing is left to undo.
+                Err(_) if !self.docs.iter().any(|d| d.uid == doc) => Ok(r),
+                Err(e) => Err(e),
+            },
             Err(e) => {
                 // The error from `f` is the one to report. A rollback fails only when the group
                 // has already ended, and then there is nothing left to undo.

crates/engine/src/tests/seams.rs

@@ -170,6 +170,27 @@ fn a_long_group_keeps_the_history_before_it() {
     assert_eq!(d.undo.iter().filter(|e| e.label == "Enter Cell").count(), 99);
 }
 
+#[test]
+fn a_grouped_action_that_closes_its_document_succeeds() {
+    // The group goes with the document; the action did what it was asked.
+    let mut s = s();
+    let uid = s.doc().unwrap().uid;
+    s.grouped("Close", |s| s.execute("file.close", json!({}))).unwrap();
+    assert!(s.docs.iter().all(|d| d.uid != uid) && !s.in_group(uid));
+    // Opening a file into an untouched new workbook takes its place the same way.
+    s.new_workbook();
+    let uid = s.doc().unwrap().uid;
+    let bytes = serde_json::to_vec(&gridcraft_model::Workbook::new()).unwrap();
+    s.grouped("Open", |s| s.execute("file.open", json!({"name": "theirs.json", "base64": crate::io::base64_encode(&bytes)}))).unwrap();
+    assert!(s.docs.iter().all(|d| d.uid != uid) && !s.in_group(uid) && s.groups.is_empty());
+    // A group ended by someone else while its document is still open is still an error.
+    let err = s.grouped("Ended", |s| {
+        let token = s.groups.last().map(|g| crate::GroupToken { id: g.id, doc: g.doc });
+        s.group_end(token.ok_or_else(|| EngineError::Other("no group".into()))?).map(|_| ())
+    });
+    assert!(err.unwrap_err().to_string().contains("already ended"));
+}
+
 #[test]
 fn closing_a_document_drops_its_groups() {
     let mut s = s();