From fa982a4cc60625d3c532f12eba5a1556bd74384e Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 07:47:15 -0400 Subject: [PATCH] LRC-LIB-SMARTCOLL: rule editor keeps an unreadable yes/no value From review: the editor rewrote a yes/no value that was neither yes nor no (written by an agent or an older version) to true on every frame, so the dialog never showed the problem and OK silently saved "Yes", turning a rule that matched nothing into one that matches. The dropdown now shows such a value as written and changes it only when the user picks Yes or No. Until then the dialog reports the problem and OK refuses it. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/rules_editor.rs | 16 ++++++++++------ crates/ui-egui/src/tests_masking.rs | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/panels/rules_editor.rs b/crates/ui-egui/src/panels/rules_editor.rs index 1ee1872db..b85d7b3be 100644 --- a/crates/ui-egui/src/panels/rules_editor.rs +++ b/crates/ui-egui/src/panels/rules_editor.rs @@ -131,13 +131,17 @@ fn value_editor(ui: &mut egui::Ui, field: &str, op: &str, v: &mut Value, salt: & ); } (Some(Kind::Bool), _) => { - // the catalog's reading of the value ("false" is no), stored back as a plain boolean - let mut b = bool_value(v).unwrap_or(true); - egui::ComboBox::from_id_salt(format!("{salt}-bool")).width(60.0).selected_text(crate::i18n::bool_text(b)).show_ui(ui, |ui| { - ui.selectable_value(&mut b, true, crate::i18n::bool_text(true)); - ui.selectable_value(&mut b, false, crate::i18n::bool_text(false)); + // the catalog's reading of the value ("false" is no); one that is neither yes nor no is + // shown as written and kept until a choice replaces it, so the dialog reports it + let cur = bool_value(v); + let text = cur.map_or_else(|| v.to_string(), |b| crate::i18n::bool_text(b).to_string()); + egui::ComboBox::from_id_salt(format!("{salt}-bool")).width(60.0).selected_text(text).show_ui(ui, |ui| { + for b in [true, false] { + if ui.selectable_label(cur == Some(b), crate::i18n::bool_text(b)).clicked() { + *v = json!(b); + } + } }); - *v = json!(b); } (Some(Kind::Date), _) => text_value(ui, v, 120.0, "2026-04 or 2026-04-12", salt), _ => text_value(ui, v, 140.0, "", salt), diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index cbafe8f1e..fd64b0a94 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -324,6 +324,22 @@ fn smart_album_field_menu_groups_fields_in_submenus() { assert!(!has(&h, "ruleFieldItem:rating:rules-0") && !has(&h, "ruleFieldItem:filePath:rules-0"), "picking a field closes the menu"); } +/// A yes/no rule whose value isn't yes or no (written by an agent or an older version) stays as +/// it is in the editor: the dialog shows the problem and OK doesn't quietly save it as Yes. +#[test] +fn smart_album_editor_keeps_an_unreadable_yes_no_value() { + let mut h = detail("panel.edit"); + exec(&mut h, "dialog.smartAlbum", json!({"name": "Odd"})); + let Some(crate::state::Dialog::SmartRules { rules, .. }) = &mut h.app.ui.dialog else { panic!("no rule editor") }; + rules.rules[0] = serde_json::from_value(json!({"field": "edited", "op": "is", "value": "maybe"})).unwrap(); + h.settle(SETTLE); + let Some(crate::state::Dialog::SmartRules { rules, .. }) = &h.app.ui.dialog else { panic!("no rule editor") }; + let Rule::Field { value, .. } = &rules.rules[0] else { panic!("not a field rule") }; + assert_eq!(value, &json!("maybe"), "drawing the editor doesn't rewrite the value"); + let r = h.request("ui.dialog.confirm", json!({}), T); + assert!(h.app.session.catalog.albums().all(|a| a.name != "Odd"), "OK refuses it: {r}"); +} + #[test] fn g_toggles_grids_and_shift_g_starts_guided_upright() { let mut h = detail("panel.edit");