From bb50127de4fca9d64ba4a2edf001f799416caec0 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 09:05:38 -0400 Subject: [PATCH] LRC-LIB-SMARTCOLL: report saved smart albums whose rules no longer check Rules are checked when they are made, but a saved smart album can go bad later. The usual case is a rule testing an album that is then deleted; it silently matches nothing. Catalog::smart_album_problems(id) checks a saved album's rules as they are now (RuleSet::upgrade, then RuleSet::check). It is empty for a sound smart album, a plain album or no album. albums.list reports `problems` for every smart album, e.g. ["rule 1: no album 8"]. Co-Authored-By: Claude Opus 5.5 --- crates/catalog/src/lib.rs | 9 +++++++++ crates/catalog/src/tests.rs | 20 ++++++++++++++++++++ crates/engine/src/cmd/query.rs | 2 ++ crates/engine/src/tests_library.rs | 16 ++++++++++++++++ 4 files changed, 47 insertions(+) diff --git a/crates/catalog/src/lib.rs b/crates/catalog/src/lib.rs index 5017423e..c47763b1 100644 --- a/crates/catalog/src/lib.rs +++ b/crates/catalog/src/lib.rs @@ -330,6 +330,15 @@ impl Catalog { } } + /// What is wrong with a saved smart album's rules now (`RuleSet::check`, after + /// `RuleSet::upgrade`): typically a rule testing an album that has since been deleted. Empty + /// for a sound smart album, a plain album or no album. + pub fn smart_album_problems(&self, id: AlbumId) -> Vec { + let Some(mut rules) = self.albums.get(&id).and_then(|a| a.smart.as_ref()).and_then(|f| f.rule_set.clone()) else { return Vec::new() }; + rules.upgrade(); + rules.check(self) + } + /// The photos of an album: the stored list, or a smart album's current matches (id order). pub fn album_photos(&self, id: AlbumId) -> Vec { match self.albums.get(&id) { diff --git a/crates/catalog/src/tests.rs b/crates/catalog/src/tests.rs index 8d34d5ef..84bfdd0d 100644 --- a/crates/catalog/src/tests.rs +++ b/crates/catalog/src/tests.rs @@ -456,3 +456,23 @@ fn random_sort_has_no_date_headers() { let ids = many(&mut c, 5); assert!(c.date_runs(&ids, SortKey::Random, GroupBy::Day).is_empty()); } + +/// A saved smart album whose rules no longer check (the album a rule tests was deleted) is found, +/// with its problems; a good one has none; an old album operator is read as it was meant. +#[test] +fn smart_album_problems_flag_stale_rules() { + let mut c = Catalog::new(); + c.apply(Op::AddAlbum { album: Album::new(AlbumId(1), "Trip") }).unwrap(); + let smart = |id: u64, rules: serde_json::Value| { + let f: Filter = serde_json::from_value(serde_json::json!({"ruleSet": {"rules": rules}})).unwrap(); + Album { smart: Some(Box::new(f)), ..Album::new(AlbumId(id), "Smart") } + }; + c.apply(Op::AddAlbum { album: smart(2, serde_json::json!([{"field": "album", "op": "is", "value": 1}])) }).unwrap(); + c.apply(Op::AddAlbum { album: smart(3, serde_json::json!([{"field": "album", "op": "gte", "value": 1}])) }).unwrap(); + assert!(c.smart_album_problems(AlbumId(2)).is_empty()); + assert!(c.smart_album_problems(AlbumId(3)).is_empty(), "an old operator is upgraded, not a problem"); + assert!(c.smart_album_problems(AlbumId(1)).is_empty(), "a plain album has no rules"); + c.apply(Op::RemoveAlbum { id: AlbumId(1) }).unwrap(); + let p = c.smart_album_problems(AlbumId(2)); + assert_eq!(p.iter().map(ToString::to_string).collect::>(), vec!["rule 1: no album 1".to_string()]); +} diff --git a/crates/engine/src/cmd/query.rs b/crates/engine/src/cmd/query.rs index 824039e4..d8ff2242 100644 --- a/crates/engine/src/cmd/query.rs +++ b/crates/engine/src/cmd/query.rs @@ -44,6 +44,8 @@ fn album_json(a: &Album, all: &[Album], cat: &lightcraft_catalog::Catalog) -> Va v["smart"] = json!(true); v["rules"] = serde_json::to_value(rules).unwrap_or_default(); v["rulesText"] = json!(rules.describe()); + // rules that no longer check (an album they test was deleted…), as `rule 1: …` + v["problems"] = json!(cat.smart_album_problems(a.id).iter().map(ToString::to_string).collect::>()); } v } diff --git a/crates/engine/src/tests_library.rs b/crates/engine/src/tests_library.rs index 45b400a5..921027c9 100644 --- a/crates/engine/src/tests_library.rs +++ b/crates/engine/src/tests_library.rs @@ -555,3 +555,19 @@ fn set_rules_with_a_name_is_one_step() { s.execute("album.setRules", &serde_json::json!({"id": id, "name": " ", "rules": {"rating": 5}})).unwrap(); assert_eq!(album(&s).name, "Old"); } + +/// albums.list says which smart albums have rules that no longer check, and why. +#[test] +fn album_list_reports_problems() { + let mut s = crate::Session::with_demo(); + let trip = s.execute("album.create", &serde_json::json!({"name": "Trip", "addSelected": false})).unwrap()["id"].as_u64().unwrap(); + let rules = serde_json::json!({"ruleSet": {"rules": [{"field": "album", "op": "is", "value": trip}]}}); + let id = s.execute("album.createSmart", &serde_json::json!({"name": "In Trip", "rules": rules})).unwrap()["id"].as_u64().unwrap(); + let find = |s: &mut crate::Session| { + let list = s.execute("albums.list", &serde_json::json!({})).unwrap(); + list.as_array().unwrap().iter().find(|a| a["id"] == id).cloned().unwrap() + }; + assert_eq!(find(&mut s)["problems"], serde_json::json!([])); + s.execute("album.delete", &serde_json::json!({"id": trip})).unwrap(); + assert_eq!(find(&mut s)["problems"], serde_json::json!([format!("rule 1: no album {trip}")])); +}