diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index caa53611f..50e8c1bbe 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -330,11 +330,14 @@ pub fn specs() -> Vec { if f.library_folder.is_some() && s.source == LibrarySource::LibraryFolder { return Err(bad("library.filter", "a folder is already shown (library.source): show another source first")); } - // checked like a smart album's rules: an unknown field or a value that can't match is an error, not an empty grid + // checked like a smart album's rules when this call sets them: an unknown field or a value + // that can't match is an error, not an empty grid. Rules that stopped checking since + // (their album was deleted) don't block other filter changes. if let Some(rules) = f.rule_set.as_mut() { rules.upgrade(); } - if let Some(problems) = f.rule_set.as_ref().map(|rs| rs.check(&s.catalog)).filter(|p| !p.is_empty()) { + let sets_rules = p.get("ruleSet").is_some_and(|r| !r.is_null()); + if let Some(problems) = f.rule_set.as_ref().filter(|_| sets_rules).map(|rs| rs.check(&s.catalog)).filter(|p| !p.is_empty()) { return Err(bad("library.filter", problems.iter().map(ToString::to_string).collect::>().join("; "))); } s.filter = f; @@ -1315,7 +1318,7 @@ fn merge_filter(base: &lightcraft_catalog::Filter, patch: &Value, c: &str) -> Re Ok(f) } -/// `patch` merged onto `base`, refused when its rule set has problems (`RuleSet::check_for`; +/// `patch` merged onto `base`, refused when the rule set it sets has problems (`RuleSet::check_for`; /// `owner` is the smart album the rules are for, so testing an album that leads back to it is a /// problem too). fn merge_rules( @@ -1326,7 +1329,10 @@ fn merge_rules( owner: Option, ) -> Result { let f = merge_filter(base, patch, c)?; - let problems = f.rule_set.as_ref().map(|r| r.check_for(cat, owner)).unwrap_or_default(); + // only rules this change sets are checked: one that stopped checking since (its album was + // deleted) doesn't block editing the album's other settings + let sets_rules = patch.get("ruleSet").is_some_and(|r| !r.is_null()); + let problems = f.rule_set.as_ref().filter(|_| sets_rules).map(|r| r.check_for(cat, owner)).unwrap_or_default(); if !problems.is_empty() { return Err(bad(c, problems.iter().map(ToString::to_string).collect::>().join("; "))); } diff --git a/crates/engine/src/tests_library.rs b/crates/engine/src/tests_library.rs index 4dcf2ba97..9475ae21e 100644 --- a/crates/engine/src/tests_library.rs +++ b/crates/engine/src/tests_library.rs @@ -637,3 +637,26 @@ fn rules_from_the_view_cant_loop() { assert_eq!(s.catalog.album(lightcraft_catalog::AlbumId(b)).unwrap().smart, before, "B unchanged"); assert!(s.catalog.smart_album_problems(lightcraft_catalog::AlbumId(b)).is_empty()); } + +/// A rule that stops checking later (the album it tests is deleted) doesn't lock up what doesn't +/// touch it: the filter bar keeps working, and an album's other settings can still be edited. A +/// change to the rules themselves is still checked. +#[test] +fn a_stale_rule_doesnt_block_other_changes() { + 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!({"rules": [{"field": "album", "op": "isNot", "value": trip}]}); + s.execute("library.filter", &serde_json::json!({"ruleSet": rules})).unwrap(); + let smart = + s.execute("album.createSmart", &serde_json::json!({"name": "Not Trip", "rules": {"ruleSet": rules}})).unwrap()["id"].as_u64().unwrap(); + s.execute("album.delete", &serde_json::json!({"id": trip})).unwrap(); + // the filter bar: a rating still applies on top of the stale rule + s.execute("library.filter", &serde_json::json!({"rating": 3})).unwrap(); + assert_eq!(s.filter.rating, 3); + // the album: a partial edit that leaves its rules alone + s.execute("album.setRules", &serde_json::json!({"id": smart, "rules": {"rating": 2}})).unwrap(); + // changing the rules is still checked + let bad = serde_json::json!({"ruleSet": {"rules": [{"field": "rating", "op": "gte", "value": 9}]}}); + assert!(s.execute("library.filter", &bad).is_err()); + assert!(s.execute("album.setRules", &serde_json::json!({"id": smart, "rules": bad})).is_err()); +}