MDL-35419 question manual grading: valdiation must handle 0,5

Yet another , as decimal separator issue!

To fix this nicely, I refactored some code into question_utils.

diff --git a/question/behaviour/behaviourbase.php b/question/behaviour/behaviourbase.php
index cd527ef..1f6beb2 100644 (file)
--- a/question/behaviour/behaviourbase.php
+++ b/question/behaviour/behaviourbase.php
@@ -477,9 +477,9 @@ abstract class question_behaviour {
      */
     public static function is_manual_grade_in_range($qubaid, $slot) {
         $prefix = 'q' . $qubaid . ':' . $slot . '_';
-        $mark = optional_param($prefix . '-mark', null, PARAM_NUMBER);
-        $maxmark = optional_param($prefix . '-maxmark', null, PARAM_NUMBER);
-        $minfraction = optional_param($prefix . ':minfraction', null, PARAM_NUMBER);
+        $mark = question_utils::optional_param_mark($prefix . '-mark');
+        $maxmark = optional_param($prefix . '-maxmark', null, PARAM_FLOAT);
+        $minfraction = optional_param($prefix . ':minfraction', null, PARAM_FLOAT);
         return is_null($mark) || ($mark >= $minfraction * $maxmark && $mark <= $maxmark);
     }
 

diff --git a/question/engine/lib.php b/question/engine/lib.php
index d9c7d0f..9fdfc03 100644 (file)
--- a/question/engine/lib.php
+++ b/question/engine/lib.php
@@ -789,6 +789,33 @@ abstract class question_utils {
         return self::$thousands[$number / 1000 % 10] . self::$hundreds[$number / 100 % 10] .
                 self::$tens[$number / 10 % 10] . self::$units[$number % 10];
     }
+
+    /**
+     * Typically, $mark will have come from optional_param($name, null, PARAM_RAW_TRIMMED).
+     * This method copes with:
+     *  - keeping null or '' input unchanged.
+     *  - nubmers that were typed as either 1.00 or 1,00 form.
+     *
+     * @param string|null $mark raw use input of a mark.
+     * @return float|string|null cleaned mark as a float if possible. Otherwise '' or null.
+     */
+    public static function clean_param_mark($mark) {
+        if ($mark === '' || is_null($mark)) {
+            return $mark;
+        }
+
+        return clean_param(str_replace(',', '.', $mark), PARAM_FLOAT);
+    }
+
+    /**
+     * Get a sumitted variable (from the GET or POST data) that is a mark.
+     * @param string $parname the submitted variable name.
+     * @return float|string|null cleaned mark as a float if possible. Otherwise '' or null.
+     */
+    public static function optional_param_mark($parname) {
+        return self::clean_param_mark(
+                optional_param($parname, null, PARAM_RAW_TRIMMED));
+    }
 }
 
 

diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php
index 1be7df2..d89f0f7 100644 (file)
--- a/question/engine/questionattempt.php
+++ b/question/engine/questionattempt.php
@@ -882,13 +882,8 @@ class question_attempt {
     public function get_submitted_var($name, $type, $postdata = null) {
         switch ($type) {
             case self::PARAM_MARK:
-                // Special case to work around PARAM_NUMBER converting '' to 0.
-                $mark = $this->get_submitted_var($name, PARAM_RAW_TRIMMED, $postdata);
-                if ($mark === '' || is_null($mark)) {
-                    return $mark;
-                } else {
-                    return clean_param(str_replace(',', '.', $mark), PARAM_NUMBER);
-                }
+                // Special case to work around PARAM_FLOAT converting '' to 0.
+                return question_utils::clean_param_mark($this->get_submitted_var($name, PARAM_RAW_TRIMMED, $postdata));
 
             case self::PARAM_FILES:
                 return $this->process_response_files($name, $name, $postdata);

diff --git a/question/engine/tests/questionutils_test.php b/question/engine/tests/questionutils_test.php
index e51d4db..e5935f4 100644 (file)
--- a/question/engine/tests/questionutils_test.php
+++ b/question/engine/tests/questionutils_test.php
@@ -191,4 +191,14 @@ class question_utils_test extends advanced_testcase {
         $this->setExpectedException('moodle_exception');
         question_utils::int_to_roman(1.5);
     }
-}
\ No newline at end of file
+
+    public function test_clean_param_mark() {
+        $this->assertNull(question_utils::clean_param_mark(null));
+        $this->assertSame('', question_utils::clean_param_mark(''));
+        $this->assertSame(0.0, question_utils::clean_param_mark('0'));
+        $this->assertSame(1.5, question_utils::clean_param_mark('1.5'));
+        $this->assertSame(1.5, question_utils::clean_param_mark('1,5'));
+        $this->assertSame(-1.5, question_utils::clean_param_mark('-1.5'));
+        $this->assertSame(-1.5, question_utils::clean_param_mark('-1,5'));
+    }
+}