Merge branch 'MDL-35419_23' of git://github.com/timhunt/moodle into M…

…OODLE_23_STABLE
moodle · Sep 17, 2012 · c59d51b · c59d51b
2 parents b050380 + c21f901
commit c59d51b
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 11 deletions.
diff --git a/question/behaviour/behaviourbase.php b/question/behaviour/behaviourbase.php
@@ -477,9 +477,9 @@ public function process_comment(question_attempt_pending_step $pendingstep) {
      */
     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
@@ -789,6 +789,33 @@ public static function int_to_roman($number) {
         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
@@ -882,13 +882,8 @@ protected function get_resume_data() {
     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
@@ -191,4 +191,14 @@ public function test_int_to_roman_not_int() {
         $this->setExpectedException('moodle_exception');
         question_utils::int_to_roman(1.5);
     }
-}
+
+    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'));
+    }
+}