MDL-35419 question manual grading: valdiation must handle 0,5
authorTim Hunt <T.J.Hunt@open.ac.uk>
Thu, 13 Sep 2012 15:33:15 +0000 (16:33 +0100)
committerTim Hunt <T.J.Hunt@open.ac.uk>
Thu, 13 Sep 2012 15:33:15 +0000 (16:33 +0100)
Yet another , as decimal separator issue!

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

question/behaviour/behaviourbase.php
question/engine/lib.php
question/engine/questionattempt.php
question/engine/tests/questionutils_test.php

index 39d6c02..d40d62a 100644 (file)
@@ -477,7 +477,7 @@ 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_FLOAT);
+        $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);
index d9c7d0f..9fdfc03 100644 (file)
@@ -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));
+    }
 }
 
 
index 8c88d50..57041f0 100644 (file)
@@ -883,12 +883,7 @@ class question_attempt {
         switch ($type) {
             case self::PARAM_MARK:
                 // Special case to work around PARAM_FLOAT 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_FLOAT);
-                }
+                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);
index e51d4db..e5935f4 100644 (file)
@@ -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'));
+    }
+}