Merge PR #2755

crates/engine/src/cmd/library.rs

@@ -803,10 +803,10 @@ pub fn specs() -> Vec<CommandSpec> {
             "{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 })))
             }
         ),

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