diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index 3e9cded58..14a369507 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -160,6 +160,15 @@ fn has_person(p: &Photo, name: &str) -> bool { impl Filter { /// Human-readable summary of the active rules (smart album tooltips, `album.list`). pub fn describe(&self) -> String { + self.describe_in(None) + } + + /// [`Filter::describe`] naming the albums its rules test (`RuleSet::describe_with`). + pub fn describe_with(&self, cat: &crate::Catalog) -> String { + self.describe_in(Some(cat)) + } + + fn describe_in(&self, cat: Option<&crate::Catalog>) -> String { let mut v: Vec = Vec::new(); if self.rating > 0 { let op = match self.rating_op { @@ -207,7 +216,7 @@ impl Filter { v.push(format!("“{}”", self.text.trim())); } if let Some(rs) = self.rule_set.as_ref().filter(|r| !r.rules.is_empty()) { - v.push(rs.describe()); + v.push(cat.map_or_else(|| rs.describe(), |c| rs.describe_with(c))); } if v.is_empty() { "all photos".into() } else { v.join(", ") } } diff --git a/crates/catalog/src/rules.rs b/crates/catalog/src/rules.rs index 4f34fd5bf..675094f96 100644 --- a/crates/catalog/src/rules.rs +++ b/crates/catalog/src/rules.rs @@ -443,6 +443,15 @@ pub(crate) fn evaluating(id: crate::AlbumId, f: impl FnOnce() -> T) -> Option Some(f()) } +/// How a summary names an Album rule's album: “Name”, or `#9` when there is no such album. +pub fn album_name(value: &Value, cat: Option<&Catalog>) -> String { + let id = album_rule_id(value); + match id.and_then(|id| cat.and_then(|c| c.album(id))) { + Some(a) => format!("“{}”", a.name), + None => id.map_or_else(|| value.to_string(), |id| format!("#{}", id.0)), + } +} + /// What a yes/no rule's value means: `true`, `"yes"`, `1` (or `1.0`) or no value is yes; `false`, /// `"no"`, `0` is no (strings in any case); `None` for anything else, which matches nothing. pub fn bool_value(value: &Value) -> Option { @@ -915,6 +924,16 @@ impl RuleSet { /// A short readable summary ("rating is ≥ 3 and keywords contains travel"). pub fn describe(&self) -> String { + self.describe_in(None) + } + + /// [`RuleSet::describe`] naming the albums Album rules test ("album isn't “Excluded + /// Photos”"); one that is gone shows as `#9`. + pub fn describe_with(&self, cat: &Catalog) -> String { + self.describe_in(Some(cat)) + } + + fn describe_in(&self, cat: Option<&Catalog>) -> String { let join = match self.mode { Match::All => " and ", Match::Any => " or ", @@ -924,11 +943,12 @@ impl RuleSet { .rules .iter() .map(|r| match r { - Rule::Group { group } => format!("({})", group.describe()), + Rule::Group { group } => format!("({})", group.describe_in(cat)), Rule::Field { field, op, value } => { let label = field_label(field).unwrap_or(field).to_lowercase(); let op = field_kind(field).and_then(|k| ops_for(k).iter().find(|o| o.0 == op)).map_or(op.as_str(), |o| o.1); let v = match value { + _ if field == "album" && cat.is_some() => album_name(value, cat), _ if field_kind(field) == Some(Kind::Bool) => { bool_value(value).map_or_else(|| value.to_string(), |b| bool_label(b).to_lowercase()) } diff --git a/crates/catalog/src/tests.rs b/crates/catalog/src/tests.rs index c6e25317f..ec78153be 100644 --- a/crates/catalog/src/tests.rs +++ b/crates/catalog/src/tests.rs @@ -549,3 +549,18 @@ fn smart_album_loops_are_refused_and_survived() { assert!(c.smart_album_problems(AlbumId(a)).iter().any(|p| p.issue == crate::rules::Issue::AlbumLoop), "album {a} is reported"); } } + +/// Summaries name the albums Album rules test ("album isn't “Excluded Photos”"), not their ids; +/// one that is gone shows as its number. +#[test] +fn summaries_name_albums() { + let mut c = Catalog::new(); + c.apply(Op::AddAlbum { album: Album::new(AlbumId(4), "Excluded Photos") }).unwrap(); + let rules: RuleSet = serde_json::from_value(serde_json::json!({"rules": [ + {"field": "keywords", "op": "contains", "value": "travel"}, + {"group": {"match": "any", "rules": [{"field": "album", "op": "isNot", "value": 4}, {"field": "album", "op": "is", "value": 9}]}}]})) + .unwrap(); + assert_eq!(rules.describe_with(&c), "keywords contains travel and (album isn't “Excluded Photos” or album is #9)"); + let f = Filter { rule_set: Some(rules), ..Default::default() }; + assert!(f.describe_with(&c).contains("album isn't “Excluded Photos”")); +} diff --git a/crates/engine/src/cmd/query.rs b/crates/engine/src/cmd/query.rs index d8ff22424..45904e0bb 100644 --- a/crates/engine/src/cmd/query.rs +++ b/crates/engine/src/cmd/query.rs @@ -43,7 +43,7 @@ fn album_json(a: &Album, all: &[Album], cat: &lightcraft_catalog::Catalog) -> Va if let Some(rules) = &a.smart { v["smart"] = json!(true); v["rules"] = serde_json::to_value(rules).unwrap_or_default(); - v["rulesText"] = json!(rules.describe()); + v["rulesText"] = json!(rules.describe_with(cat)); // 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::>()); } diff --git a/crates/engine/src/tests_library.rs b/crates/engine/src/tests_library.rs index 6e20f9985..926482b53 100644 --- a/crates/engine/src/tests_library.rs +++ b/crates/engine/src/tests_library.rs @@ -616,4 +616,8 @@ fn smart_album_excluding_a_smart_album() { &serde_json::json!({"id": excluded, "rules": {"ruleSet": {"rules": [{"field": "album", "op": "is", "value": travel}]}}, "replace": true}), ); assert!(looped.is_err_and(|e| e.to_string().contains("would make this album include itself")), "a loop is refused"); + // agents read the album by name in the summary + let list = s.execute("albums.list", &serde_json::json!({})).unwrap(); + let travel_json = list.as_array().unwrap().iter().find(|a| a["id"] == travel).cloned().unwrap(); + assert!(travel_json["rulesText"].as_str().unwrap_or("").contains("album isn't “Excluded Photos”"), "{travel_json}"); } diff --git a/crates/engine/src/view.rs b/crates/engine/src/view.rs index 16c9f8a0c..5b32999bb 100644 --- a/crates/engine/src/view.rs +++ b/crates/engine/src/view.rs @@ -212,7 +212,7 @@ pub fn filter_chips(f: &Filter, cat: &Catalog) -> Vec { _ => {} } if let Some(rs) = f.rule_set.as_ref().filter(|r| !r.rules.is_empty()) { - add(format!("Rules: {}", rs.describe()), json!({"ruleSet": Null})); + add(format!("Rules: {}", rs.describe_with(cat)), json!({"ruleSet": Null})); } if !f.only.is_empty() { add(format!("Only {} photos", f.only.len()), json!({"only": []})); diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index efeaadb88..2661095d1 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -318,7 +318,7 @@ pub fn color_label(catalog: &lightcraft_catalog::Catalog, label: lightcraft_cata /// A legacy smart-album filter summary, using the same display labels as filter chips. pub fn filter_label(filter: &lightcraft_catalog::Filter, catalog: &lightcraft_catalog::Catalog) -> String { if language() == Locale::En { - filter.describe() + filter.describe_with(catalog) } else { lightcraft_engine::filter_chips(filter, catalog) .iter() @@ -378,8 +378,8 @@ fn bool_text_in(language: Locale, b: bool) -> &'static str { } /// A rule summary for display, keeping free-text rule values verbatim. -pub fn rules_label(rules: &lightcraft_catalog::RuleSet) -> String { - fn describe(rules: &lightcraft_catalog::RuleSet, depth: usize) -> String { +pub fn rules_label(rules: &lightcraft_catalog::RuleSet, catalog: &lightcraft_catalog::Catalog) -> String { + fn describe(rules: &lightcraft_catalog::RuleSet, catalog: &lightcraft_catalog::Catalog, depth: usize) -> String { use lightcraft_catalog::{ Match, Rule, rules::{FIELDS, Kind, ops_for}, @@ -391,11 +391,13 @@ pub fn rules_label(rules: &lightcraft_catalog::RuleSet) -> String { .rules .iter() .map(|rule| match rule { - Rule::Group { group } => format!("({})", describe(group, depth + 1)), + Rule::Group { group } => format!("({})", describe(group, catalog, depth + 1)), Rule::Field { field, op, value } => { let Some((_, label, kind)) = FIELDS.iter().find(|entry| entry.0 == field) else { return field.clone() }; let operator = ops_for(*kind).iter().find(|entry| entry.0 == op).map_or(op.as_str(), |entry| entry.1); let text = match kind { + // the album's name, as people know it (a user's name is never translated) + Kind::Album => lightcraft_catalog::rules::album_name(value, Some(catalog)), Kind::Choice(_) => value.as_str().map_or_else(|| value.to_string(), |id| choice_text(field, id).to_string()), Kind::Bool => lightcraft_catalog::rules::bool_value(value).map_or_else(|| value.to_string(), |b| bool_text(b).to_string()), _ if matches!(op.as_str(), "inLast" | "notInLast") => format!( @@ -417,7 +419,7 @@ pub fn rules_label(rules: &lightcraft_catalog::RuleSet) -> String { if rules.mode == Match::None { format!("{} ({text})", tr("none of")) } else { text } } // Preserve the established source-language summary. - if language() == Locale::En { rules.describe() } else { describe(rules, 0) } + if language() == Locale::En { rules.describe_with(catalog) } else { describe(rules, catalog, 0) } } #[cfg(test)] @@ -865,7 +867,16 @@ mod tests { mode: lightcraft_catalog::Match::All, rules: vec![lightcraft_catalog::Rule::Field { field: "keywords".into(), op: "contains".into(), value: serde_json::json!("Color") }], }; - assert_eq!(rules_label(&rules), "Stichwörter enthält Color"); + let mut catalog = lightcraft_catalog::Catalog::new(); + catalog + .apply(lightcraft_catalog::Op::AddAlbum { album: lightcraft_catalog::Album::new(lightcraft_catalog::AlbumId(4), "Ausgeschlossen") }) + .unwrap(); + assert_eq!(rules_label(&rules, &catalog), "Stichwörter enthält Color"); + let album = lightcraft_catalog::RuleSet { + mode: lightcraft_catalog::Match::All, + rules: vec![lightcraft_catalog::Rule::Field { field: "album".into(), op: "isNot".into(), value: serde_json::json!(4) }], + }; + assert!(rules_label(&album, &catalog).ends_with(" “Ausgeschlossen”"), "{}", rules_label(&album, &catalog)); let rules = lightcraft_catalog::RuleSet { mode: lightcraft_catalog::Match::All, rules: vec![lightcraft_catalog::Rule::Field { @@ -874,13 +885,13 @@ mod tests { value: serde_json::json!("publicDomain"), }], }; - assert!(rules_label(&rules).ends_with(" Gemeinfrei"), "{}", rules_label(&rules)); + assert!(rules_label(&rules, &catalog).ends_with(" Gemeinfrei"), "{}", rules_label(&rules, &catalog)); for value in [serde_json::json!(false), serde_json::json!("false")] { let rules = lightcraft_catalog::RuleSet { mode: lightcraft_catalog::Match::All, rules: vec![lightcraft_catalog::Rule::Field { field: "edited".into(), op: "is".into(), value }], }; - assert!(rules_label(&rules).ends_with(" Nein"), "{}", rules_label(&rules)); + assert!(rules_label(&rules, &catalog).ends_with(" Nein"), "{}", rules_label(&rules, &catalog)); } let filter = lightcraft_catalog::Filter { labels: vec![lightcraft_catalog::ColorLabel::Red, lightcraft_catalog::ColorLabel::Blue], diff --git a/crates/ui-egui/src/panels/chips.rs b/crates/ui-egui/src/panels/chips.rs index 38dc01e45..d1acd15a5 100644 --- a/crates/ui-egui/src/panels/chips.rs +++ b/crates/ui-egui/src/panels/chips.rs @@ -37,7 +37,7 @@ pub(crate) fn display_label(chip: &FilterChip, filter: &lightcraft_catalog::Filt if chip.clear.get("ruleSet").is_some() && let Some(rules) = &filter.rule_set { - return format!("{}: {}", tr("Rules"), rules_label(rules)); + return format!("{}: {}", tr("Rules"), rules_label(rules, catalog)); } if chip.clear.get("label").is_some() { let mut labels = filter.labels.clone();