diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 775106802..a07eb8b48 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -854,7 +854,7 @@ pub fn specs() -> Vec { "Edit Smart Album", [], None, - "{id, rules?: partial Filter merged onto the current rules (null clears a field), replace?: bool, fromView?: bool (use the current view)}", + "{id, rules?: partial Filter merged onto the current rules (null clears a field), replace?: bool, fromView?: bool (use the current view), name?: also rename it (blank keeps the name) — one undo step, both or neither}", always, |s, p| { let id = album_param(p, "id", "album.setRules")?; @@ -873,7 +873,13 @@ pub fn specs() -> Vec { } r }; - s.commit("Edit Smart Album", Op::SetAlbumRules { id, rules: Box::new(rules) })?; + let set = Op::SetAlbumRules { id, rules: Box::new(rules) }; + // a new name goes with the rules: one step, applied (and undone) together + let rename = str_param(p, "name").map(str::trim).filter(|n| !n.is_empty() && s.catalog.album(id).is_some_and(|a| a.name != *n)); + match rename { + Some(name) => s.commit("Edit Smart Album", Op::Batch { ops: vec![Op::RenameAlbum { id, name: name.to_string() }, set] })?, + None => s.commit("Edit Smart Album", set)?, + }; Ok(json!({"count": s.catalog.album_count(id)})) } ), diff --git a/crates/engine/src/tests_library.rs b/crates/engine/src/tests_library.rs index 9b680c0fc..45b400a5a 100644 --- a/crates/engine/src/tests_library.rs +++ b/crates/engine/src/tests_library.rs @@ -532,3 +532,26 @@ fn old_album_operator_rules_stay_editable() { assert!(saved.contains(r#""op":"isNot""#), "{saved}"); s.execute("library.filter", &serde_json::json!({"ruleSet": {"rules": [{"field": "album", "op": "lte", "value": trip}]}})).unwrap(); } + +/// Renaming and changing the rules is one step: both or neither, undone together. +#[test] +fn set_rules_with_a_name_is_one_step() { + let mut s = crate::Session::with_demo(); + let id = s.execute("album.createSmart", &serde_json::json!({"name": "Old", "rules": {"rating": 3}})).unwrap()["id"].as_u64().unwrap(); + let album = |s: &crate::Session| s.catalog.album(lightcraft_catalog::AlbumId(id)).unwrap().clone(); + let good = serde_json::json!({"ruleSet": {"rules": [{"field": "rating", "op": "gte", "value": 4}]}}); + let bad = serde_json::json!({"ruleSet": {"rules": [{"field": "captureDate", "op": "is", "value": "banana"}]}}); + // refused rules: the name stays too + assert!(s.execute("album.setRules", &serde_json::json!({"id": id, "name": "New", "replace": true, "rules": bad})).is_err()); + assert_eq!(album(&s).name, "Old"); + let undo = s.undo.len(); + s.execute("album.setRules", &serde_json::json!({"id": id, "name": "New", "replace": true, "rules": good})).unwrap(); + assert_eq!((album(&s).name.as_str(), s.undo.len()), ("New", undo + 1), "one undo step"); + assert!(album(&s).smart.unwrap().rule_set.is_some()); + s.execute("edit.undo", &serde_json::json!({})).unwrap(); + assert_eq!(album(&s).name, "Old"); + assert!(album(&s).smart.unwrap().rule_set.is_none(), "undone together"); + // a blank name keeps the album's + s.execute("album.setRules", &serde_json::json!({"id": id, "name": " ", "rules": {"rating": 5}})).unwrap(); + assert_eq!(album(&s).name, "Old"); +} diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index d29fd84fb..954cd9f59 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -1103,12 +1103,8 @@ pub fn confirm_dialog(app: &mut LightcraftApp, dlg: &Dialog) -> Result { - if app.session.catalog.album(lightcraft_catalog::AlbumId(*id)).is_some_and(|a| a.name != name) { - app.run("album.rename", json!({"id": id, "name": name}))?; - } - app.run("album.setRules", json!({"id": id, "replace": true, "rules": {"ruleSet": rules}})) - } + // the name and the rules together: one undo step, both or neither + Some(id) => app.run("album.setRules", json!({"id": id, "name": name, "replace": true, "rules": {"ruleSet": rules}})), None => created_in(app, "album.createSmart", json!({"name": name, "rules": {"ruleSet": rules}, "parent": parent})), } } diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index ca0537070..0a1d37b4d 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -446,6 +446,20 @@ fn smart_album_edit_with_bad_rules_keeps_the_name() { let _ = h.request("ui.dialog.confirm", json!({}), T); let album = h.app.session.catalog.album(lightcraft_catalog::AlbumId(id)).expect("album"); assert_eq!(album.name, "Keep", "neither OK nor confirm renamed it"); + // fixed: OK renames it and sets the rules in one undo step + let Some(crate::state::Dialog::SmartRules { rules, .. }) = &mut h.app.ui.dialog else { panic!("the editor stays open") }; + *rules = serde_json::from_value(json!({"rules": [{"field": "captureDate", "op": "is", "value": "2026"}]})).unwrap(); + h.settle(SETTLE); + let undo = h.app.session.undo.len(); + let r = h.request("ui.clickWidget", json!({"id": "button:dialogOk"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + let album = h.app.session.catalog.album(lightcraft_catalog::AlbumId(id)).expect("album"); + assert_eq!((album.name.as_str(), h.app.session.undo.len()), ("Renamed", undo + 1)); + exec(&mut h, "edit.undo", json!({})); + let album = h.app.session.catalog.album(lightcraft_catalog::AlbumId(id)).expect("album"); + assert_eq!(album.name, "Keep", "one undo takes back the name with the rules"); + assert!(album.smart.as_ref().is_some_and(|f| f.rule_set.is_none())); } #[test] diff --git a/docs/whats-new.md b/docs/whats-new.md index 134de17b3..5b97d1c85 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -17,6 +17,8 @@ way round, "in the last 0 days", a colour label that doesn't exist, "contains" with nothing to look for, an album that is gone or smart, or an empty group is reported with its rule ("rule 2.1: rating is 0–5, not 9") instead of quietly matching nothing or everything. New date rules start at the current year. +- Editing a smart album's name and rules is one step: both or neither, and one undo takes both back + (`album.setRules` takes `name`). - The rule editor marks each rule that needs fixing right under it and keeps OK disabled until they are fixed. An Album rule picks its album from a list (is / isn't) instead of taking an id. - Yes/no fields (Has Edits, Cropped, Has GPS…) read Yes / No instead of true / false. A rule sent as `"false"` now