photo.label: a missing label, or one that isn't text, is an error, not a clear

photo.label read its label with str_param, which is None both when the
label is missing and when it isn't a string, and None meant "none": so
photo.label {} or {"label": 3} cleared the colour label of every selected
photo. Only "none" or null clear now; anything else that isn't a known
label is an error. Every caller in the app already passes a label.
This commit is contained in:
Bruno-Alumn
2026-10-21 01:49:30 +00:00
parent 622988d7f7
commit 205a7ec120
2 changed files with 27 additions and 3 deletions
+7 -3
View File
@@ -804,9 +804,13 @@ pub fn specs() -> Vec<CommandSpec> {
"{label: red|yellow|green|blue|purple|none, ids?}",
has_selection,
|s, p| {
let l = match str_param(p, "label") {
None | Some("none") => None,
Some(x) => Some(ColorLabel::parse(x).ok_or_else(|| bad("photo.label", "unknown label"))?),
// `none` (or null) clears; a missing label or one that isn't text is an error, not a clear
let l = match p.get("label") {
Some(Value::Null) => None,
Some(Value::String(x)) if x == "none" => None,
Some(Value::String(x)) => Some(ColorLabel::parse(x).ok_or_else(|| bad("photo.label", "unknown label"))?),
None => return Err(bad("photo.label", "missing `label` (red|yellow|green|blue|purple|none)")),
Some(_) => return Err(bad("photo.label", "label must be red|yellow|green|blue|purple|none")),
};
for_targets(s, p, "Set Color Label", |id| Some(Op::SetLabel { id, label: l }))
}
+20
View File
@@ -143,3 +143,23 @@ fn mask_and_spot_commands_refuse_ids_that_are_not_ids() {
s.execute("spot.select", &json!({"index": null})).unwrap();
assert_eq!(state(&s).0.len(), 1);
}
/// `photo.label` without a label, or with one that isn't text, cleared the colour label of every
/// selected photo (a missing `label` read as `none`); now it is an error and only `none` or null clear.
#[test]
fn photo_label_refuses_a_missing_or_non_text_label() {
let (mut s, vis, _) = session();
s.execute("photo.label", &json!({"label": "red"})).unwrap();
let labels = |s: &Session| vis.iter().take(2).map(|id| s.catalog.photo(*id).unwrap().label).collect::<Vec<_>>();
let before = labels(&s);
assert!(before.iter().all(Option::is_some));
for params in [json!({}), json!({"label": 3}), json!({"label": true}), json!({"label": ["red"]}), json!({"label": "crimson"})] {
assert!(s.execute("photo.label", &params).is_err(), "{params}");
assert_eq!(labels(&s), before, "{params} changed the labels");
}
s.execute("photo.label", &json!({"label": null})).unwrap();
assert!(labels(&s).iter().all(Option::is_none));
s.execute("photo.label", &json!({"label": "green"})).unwrap();
s.execute("photo.label", &json!({"label": "none"})).unwrap();
assert!(labels(&s).iter().all(Option::is_none));
}