LRC-LIB-SMARTCOLL: rename and rules of a smart album in one step

The rule editor renamed an album (album.rename) and then set its rules
(album.setRules) as two commits. If the second failed (refused rules
before the dialog checked them, or a save error), the album kept the new
name without the rules, and undo needed two steps.

album.setRules takes an optional `name` (blank keeps the album's) and
commits the rename and the rules as one Op::Batch: both or neither, one
undo step. The dialog edits through it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Nathaniel Ledford
2026-10-09 10:54:57 -04:00
co-authored by Claude Opus 5.5
parent 920658c76d
commit 31abbcee4f
5 changed files with 49 additions and 8 deletions
+8 -2
View File
@@ -854,7 +854,7 @@ pub fn specs() -> Vec<CommandSpec> {
"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<CommandSpec> {
}
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)}))
}
),
+23
View File
@@ -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");
}
+2 -6
View File
@@ -1103,12 +1103,8 @@ pub fn confirm_dialog(app: &mut LightcraftApp, dlg: &Dialog) -> Result<serde_jso
}
let name = if name.trim().is_empty() { "Smart Album".to_string() } else { name.trim().to_string() };
match id {
Some(id) => {
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})),
}
}
+14
View File
@@ -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]
+2
View File
@@ -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