diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index a9843156..caa53611 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -860,7 +860,22 @@ pub fn specs() -> Vec { let id = album_param(p, "id", "album.setRules")?; let cur = s.catalog.album(id).and_then(|a| a.smart.as_deref().cloned()).ok_or_else(|| bad("album.setRules", "not a smart album"))?; let rules = if bool_or(p, "fromView", false) { - view_rules(s) + // the view is saved as it is, but never as a loop (showing A, which tests this + // album, and saving that view here would make it test itself) + let view = view_rules(s); + let loops: Vec = view + .rule_set + .as_ref() + .map(|r| r.check_for(&s.catalog, Some(id))) + .unwrap_or_default() + .iter() + .filter(|p| p.issue == lightcraft_catalog::rules::Issue::AlbumLoop) + .map(ToString::to_string) + .collect(); + if !loops.is_empty() { + return Err(bad("album.setRules", loops.join("; "))); + } + view } else { let replace = bool_or(p, "replace", false); let folder = cur.library_folder.clone(); diff --git a/crates/engine/src/tests_library.rs b/crates/engine/src/tests_library.rs index 926482b5..4dcf2ba9 100644 --- a/crates/engine/src/tests_library.rs +++ b/crates/engine/src/tests_library.rs @@ -621,3 +621,19 @@ fn smart_album_excluding_a_smart_album() { 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}"); } + +/// "Update Rules from Current Filter" refuses a loop too: showing A ("Album isn't B") and +/// updating B from that view would make B test itself. +#[test] +fn rules_from_the_view_cant_loop() { + let mut s = crate::Session::with_demo(); + let b = s.execute("album.createSmart", &serde_json::json!({"name": "B", "rules": {"rating": 2}})).unwrap()["id"].as_u64().unwrap(); + let rules = serde_json::json!({"ruleSet": {"rules": [{"field": "album", "op": "isNot", "value": b}]}}); + let a = s.execute("album.createSmart", &serde_json::json!({"name": "A", "rules": rules})).unwrap()["id"].as_u64().unwrap(); + s.execute("library.source", &serde_json::json!({"kind": "album", "id": a})).unwrap(); + let before = s.catalog.album(lightcraft_catalog::AlbumId(b)).unwrap().smart.clone(); + let r = s.execute("album.setRules", &serde_json::json!({"id": b, "fromView": true})); + assert!(r.is_err_and(|e| e.to_string().contains("would make this album include itself")), "refused"); + 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()); +} diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index 8f843d94..5a21b54e 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -1059,7 +1059,10 @@ fn folder_menu(app: &mut LightcraftApp, resp: &egui::Response, a: &Album) { let _ = app.run("dialog.smartAlbum", json!({"id": a.id.0})); } if a.is_smart() && ui.button(crate::i18n::tr("Update Rules from Current Filter")).clicked() { - let _ = app.run("album.setRules", json!({"id": a.id.0, "fromView": true})); + // refused when the view would make the album test itself: say why + if let Err(e) = app.run("album.setRules", json!({"id": a.id.0, "fromView": true})) { + app.toast(ui.ctx(), e); + } } if !a.folder { // export: show the album, select its photos, then the dialog / a preset