shifathosn
/
lightcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

LRC-LIB-SMARTCOLL: Update Rules from Current Filter refuses loops 

From review: album.setRules with fromView saved the current view
unchecked. Showing smart album A ("Album isn't B") and updating B from
that view made B test itself, which the rule check is meant to refuse.
The view is still saved as it is, but a loop for the album being
updated is refused (Issue::AlbumLoop), and the sidebar's menu item
shows the reason instead of ignoring the result.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Nathaniel Ledford 2026-10-09 10:15:12 -04:00
parent 2ad901e6dc
commit d48f5eaabc
No known key found for this signature in database
3 changed files with 36 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
crates/engine/src/cmd/library.rs
View File

@ -860,7 +860,22 @@ pub fn specs() -> Vec<CommandSpec> {
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<String> = 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();

16
crates/engine/src/tests_library.rs
View File

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

5
crates/ui-egui/src/panels/left.rs
View File

@ -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