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 <noreply@anthropic.com>
This commit is contained in:
Nathaniel Ledford 2026-10-09 09:05:38 -04:00
parent 31abbcee4f
commit bb50127de4
No known key found for this signature in database
4 changed files with 47 additions and 0 deletions

View File

@ -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<rules::Problem> {
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<PhotoId> {
match self.albums.get(&id) {

View File

@ -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<_>>(), vec!["rule 1: no album 1".to_string()]);
}

View File

@ -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::<Vec<_>>());
}
v
}

View File

@ -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}")]));
}