Resolve every pattern name before a batch delete removes any of them. (#4132)

* Resolve every pattern name before a batch delete removes any of them.

A missing name used to delete the earlier presets and then return an error. The pattern library is not on the document undo stack, so that partial delete could not be undone.

Fixes #4103

* Pattern batch delete: reject non-string entries and resolve against the library only

A document-only pattern resolved through pattern_cmds::resolve, so the batch
deleted earlier entries before pattern.delete refused it. Non-string array
members were silently dropped (ported from #4127).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/presets/patterns.rs

@@ -125,13 +125,32 @@ fn edit(s: &mut Session, p: &Value) -> Result<Value> {
         "rename" => super::call(s, "pattern.rename", json!({"pattern": req_str(p, "preset", CMD)?, "name": req_str(p, "name", CMD)?}))?,
         "delete" => {
             let keys: Vec<String> = match p.get("preset") {
-                Some(Value::Array(a)) => a.iter().filter_map(Value::as_str).map(str::to_string).collect(),
+                Some(Value::Array(a)) => a
+                    .iter()
+                    .map(|v| v.as_str().map(str::to_string).ok_or_else(|| bad(CMD, "`preset` must contain only pattern ids or names")))
+                    .collect::<Result<_>>()?,
                 _ => vec![req_str(p, "preset", CMD)?.to_string()],
             };
-            for k in &keys {
-                super::call(s, "pattern.delete", json!({"pattern": k}))?;
+            if keys.is_empty() {
+                return Err(bad(CMD, "missing `preset`"));
             }
-            json!({"deleted": keys.len()})
+            // Resolve every reference first. Deleting as we go left the library partly
+            // removed when a later name was missing, and Undo cannot put a preset back (#4103).
+            let mut ids = Vec::new();
+            for k in &keys {
+                // Only library patterns can be deleted here; a document-only pattern must
+                // fail the whole batch up front rather than midway through it.
+                let id = photocraft_doc::pattern::find(&s.patterns.items, k, k)
+                    .map(|q| q.id.clone())
+                    .ok_or_else(|| bad(CMD, format!("no library pattern \"{k}\"")))?;
+                if !ids.contains(&id) {
+                    ids.push(id);
+                }
+            }
+            for id in &ids {
+                super::call(s, "pattern.delete", json!({"pattern": id}))?;
+            }
+            json!({"deleted": ids.len()})
         }
         "deleteGroup" => {
             // Deleting a folder deletes its patterns, as in Photoshop.

crates/engine/src/presets/tests.rs

@@ -385,6 +385,43 @@ fn pattern_groups_select_apply_and_sync() {
     assert!(crate::pattern_cmds::resolve(&s, "Weave").is_none());
 }
 
+#[test]
+fn pattern_batch_delete_keeps_the_library_when_one_name_is_missing() {
+    let mut s = session(8);
+    let before = s.execute("pattern.presets.list", json!({})).unwrap();
+    let err = s.execute("pattern.presets.edit", json!({"action": "delete", "preset": ["Bricks", "missing"]})).unwrap_err();
+    assert!(err.to_string().contains("missing"), "{err}");
+    assert_eq!(s.execute("pattern.presets.list", json!({})).unwrap(), before);
+    assert!(crate::pattern_cmds::resolve(&s, "Bricks").is_some());
+}
+
+#[test]
+fn pattern_batch_delete_rejects_non_string_members_without_deleting() {
+    for invalid in [json!(null), json!(7), json!({})] {
+        let mut s = session(8);
+        let before = s.execute("pattern.presets.list", json!({})).unwrap();
+        assert!(s.execute("pattern.presets.edit", json!({"action": "delete", "preset": ["Bricks", invalid]})).is_err());
+        assert_eq!(s.execute("pattern.presets.list", json!({})).unwrap(), before);
+    }
+}
+
+#[test]
+fn pattern_batch_delete_refuses_a_document_only_pattern_up_front() {
+    let mut s = session(8);
+    let added = s.execute("edit.definePattern", json!({"name": "Document Only"})).unwrap();
+    let id = added["pattern"].as_str().unwrap().to_string();
+    let pattern = s.patterns.items.iter().find(|p| p.id == id).unwrap().clone();
+    s.edit("Embed pattern fixture", |doc, _| {
+        doc.patterns.push(pattern.clone());
+        Ok(())
+    })
+    .unwrap();
+    s.execute("pattern.delete", json!({"pattern": id})).unwrap();
+    let before = s.patterns.items.clone();
+    assert!(s.execute("pattern.presets.edit", json!({"action": "delete", "preset": ["Bricks", id]})).is_err());
+    assert_eq!(s.patterns.items, before, "nothing is deleted when one entry is not in the library");
+}
+
 // ------------------------------------------------------------------ styles
 
 #[test]