diff --git a/question/behaviour/behaviourbase.php b/question/behaviour/behaviourbase.php index cd527ef3a74..1f6beb214df 100644 --- 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 d9c7d0fe19f..9fdfc033aa1 100644 --- 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 1be7df264b0..d89f0f71580 100644 --- 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 e51d4db9000..e5935f40424 100644 --- 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')); + } +}