diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 6e37058dd..ad740e3f6 100644 --- a/crates/engine/src/lib.rs +++ b/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(&mut self, label: &str, f: impl FnOnce(&mut Session) -> Result) -> Result { 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. diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs index 3216cfd97..a5cd0d868 100644 --- a/crates/engine/src/tests/seams.rs +++ b/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();