diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 2b668a647..9d11cc5b3 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -803,10 +803,10 @@ pub fn specs() -> Vec { "{rating: 0..5, ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view", has_selection, |s, p| { - let r = super::f64_req(p, "rating", "photo.rate")? as i64; - if !(0..=5).contains(&r) { - return Err(bad("photo.rate", "rating must be 0..5")); - } + let r = super::index_req(p, "rating", "photo.rate") + .ok() + .filter(|r| *r <= 5) + .ok_or_else(|| bad("photo.rate", "rating must be a whole number 0..5"))?; cull(s, p, |s| for_targets(s, p, "Set Rating", |id| Some(Op::SetRating { id, rating: r as u8 }))) } ), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 218965812..93f4e6adc 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -108,6 +108,20 @@ fn demo_library_loads() { assert!(albums.as_array().unwrap().iter().any(|a| a["folder"] == true)); } +#[test] +fn rating_rejects_fractional_values() { + let mut s = demo(); + let id = s.active().unwrap(); + for bad in [5.9, -0.9, 0.5] { + let err = s.execute("photo.rate", &json!({"rating": bad})).unwrap_err(); + assert!(matches!(&err, crate::EngineError::BadParams { cmd, .. } if cmd == "photo.rate"), "{bad}: {err}"); + assert_eq!(s.catalog.photo(id).unwrap().rating, 0, "{bad} changed the rating"); + } + s.execute("photo.rate", &json!({"rating": 5.0})).unwrap(); + s.execute("photo.rate", &json!({"rating": 3})).unwrap(); + assert_eq!(s.catalog.photo(id).unwrap().rating, 3); +} + #[test] fn rating_flag_undo_redo() { let mut s = demo();