From 2d29dde4d0ace4557a41425a1b2a6805bc14c244 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 08:31:20 -0400 Subject: [PATCH] LRC-LIB-SMARTCOLL: a stale album rule no longer drops the filter bar from the view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From review: view_rules merged the filter bar's settings onto a smart album's rules through merge_rules, which now also checks the rules, and fell back to the bare album rules on any problem. A smart album testing an album that was later deleted therefore lost the filter bar (e.g. Rating ≥ 3) from Create Smart Album from Filter and the dialog's count, without a word. merge_filter does the merge alone; merge_rules is merge_filter plus the check, for album.createSmart / setRules. view_rules uses merge_filter. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/library.rs | 14 +++++++++++--- crates/engine/src/tests_organize.rs | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 7d6c30ae3..820f511e5 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -1278,10 +1278,16 @@ pub fn specs() -> Vec { } /// `base` with a partial Filter (JSON) merged on top. -fn merge_rules(base: &lightcraft_catalog::Filter, patch: &Value, c: &str, cat: &lightcraft_catalog::Catalog) -> Result { +/// `patch` merged onto `base`, unchecked (see [`merge_rules`]). +fn merge_filter(base: &lightcraft_catalog::Filter, patch: &Value, c: &str) -> Result { let mut v = serde_json::to_value(base).unwrap_or_default(); lightcraft_develop::presets::deep_merge(&mut v, patch); - let f: lightcraft_catalog::Filter = serde_json::from_value(v).map_err(|e| bad(c, e.to_string()))?; + serde_json::from_value(v).map_err(|e| bad(c, e.to_string())) +} + +/// `patch` merged onto `base`, refused when its rule set has problems (`RuleSet::check`). +fn merge_rules(base: &lightcraft_catalog::Filter, patch: &Value, c: &str, cat: &lightcraft_catalog::Catalog) -> Result { + let f = merge_filter(base, patch, c)?; let problems = f.rule_set.as_ref().map(|r| r.check(cat)).unwrap_or_default(); if !problems.is_empty() { return Err(bad(c, problems.iter().map(ToString::to_string).collect::>().join("; "))); @@ -1320,7 +1326,9 @@ fn view_rules(s: &Session) -> lightcraft_catalog::Filter { } } } - merge_rules(&base, &Value::Object(patch), "", &s.catalog).unwrap_or(base) + // the view as it is: a rule that no longer checks (its album was deleted) just matches + // nothing there, and mustn't cost the filter bar's settings + merge_filter(&base, &Value::Object(patch), "").unwrap_or(base) } None => { let mut f = s.source.to_filter(&s.filter, &s.catalog); diff --git a/crates/engine/src/tests_organize.rs b/crates/engine/src/tests_organize.rs index 7fb24806b..519a2dc6a 100644 --- a/crates/engine/src/tests_organize.rs +++ b/crates/engine/src/tests_organize.rs @@ -79,6 +79,22 @@ fn smart_album_from_smart_album_view_keeps_its_rules() { assert_eq!(s.source, LibrarySource::Album(lightcraft_catalog::AlbumId(id))); } +/// Saving the view of a smart album keeps the filter bar's settings even when the album holds a +/// rule that no longer checks (it tests an album that has since been deleted). +#[test] +fn smart_album_view_keeps_the_filter_bar_over_a_stale_rule() { + let mut s = Session::with_demo(); + let gone = s.execute("album.create", &json!({"name": "Gone", "addSelected": false})).unwrap()["id"].as_u64().unwrap(); + let rules = json!({"ruleSet": {"rules": [{"field": "album", "op": "isNot", "value": gone}]}}); + let id = s.execute("album.createSmart", &json!({"name": "Not in Gone", "rules": rules})).unwrap()["id"].as_u64().unwrap(); + s.execute("album.delete", &json!({"id": gone})).unwrap(); + s.execute("library.source", &json!({"kind": "album", "id": id})).unwrap(); + s.execute("library.filter", &json!({"rating": 3})).unwrap(); + let r = s.view_rules(); + assert_eq!(r.rating, 3, "the filter bar's rating survives"); + assert!(r.rule_set.is_some(), "and so do the album's own rules"); +} + #[test] fn virtual_copies_are_independent_and_persist() { let dir = temp_dir("vc");