From 904894070151049e01a9f55fc1de9c4142d98c09 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Tue, 28 Jun 2011 17:36:24 +0100 Subject: [PATCH] MDL-27418 correct validation of numerical answers in qtype numeric. This mostly affects locales that use , as decimal point. --- .../type/numerical/edit_numerical_form.php | 26 +++++- .../numerical/lang/en/qtype_numerical.php | 2 +- .../type/numerical/simpletest/testform.php | 80 +++++++++++++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) create mode 100755 question/type/numerical/simpletest/testform.php diff --git a/question/type/numerical/edit_numerical_form.php b/question/type/numerical/edit_numerical_form.php index f0e1e334fe4..d57e1575bfd 100644 --- a/question/type/numerical/edit_numerical_form.php +++ b/question/type/numerical/edit_numerical_form.php @@ -26,6 +26,7 @@ defined('MOODLE_INTERNAL') || die(); +require_once($CFG->dirroot . '/question/type/edit_question_form.php'); require_once($CFG->dirroot . '/question/type/numerical/questiontype.php'); @@ -36,6 +37,7 @@ require_once($CFG->dirroot . '/question/type/numerical/questiontype.php'); * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class qtype_numerical_edit_form extends question_edit_form { + protected $ap = null; protected function definition_inner($mform) { $this->add_per_answer_fields($mform, get_string('answerno', 'qtype_numerical', '{no}'), @@ -54,6 +56,7 @@ class qtype_numerical_edit_form extends question_edit_form { $tolerance = $mform->createElement('text', 'tolerance', get_string('acceptederror', 'qtype_numerical')); $repeatedoptions['tolerance']['type'] = PARAM_NUMBER; + $repeatedoptions['tolerance']['default'] = 0; array_splice($repeated, 3, 0, array($tolerance)); $repeated[1]->setSize(10); @@ -256,7 +259,7 @@ class qtype_numerical_edit_form extends question_edit_form { if ($trimmedanswer != '') { $answercount++; if (!$this->is_valid_answer($trimmedanswer, $data)) { - $errors['answer[' . $key . ']'] = $this->valid_answer_message(); + $errors['answer[' . $key . ']'] = $this->valid_answer_message($trimmedanswer); } if ($data['fraction'][$key] == 1) { $maxgrade = true; @@ -267,7 +270,7 @@ class qtype_numerical_edit_form extends question_edit_form { } } else if ($data['fraction'][$key] != 0 || !html_is_blank($data['feedback'][$key]['text'])) { - $errors['answer[' . $key . ']'] = $this->valid_answer_message(); + $errors['answer[' . $key . ']'] = $this->valid_answer_message($trimmedanswer); $answercount++; } } @@ -277,6 +280,8 @@ class qtype_numerical_edit_form extends question_edit_form { if ($maxgrade == false) { $errors['fraction[0]'] = get_string('fractionsnomax', 'question'); } + + return $errors; } /** @@ -286,7 +291,22 @@ class qtype_numerical_edit_form extends question_edit_form { * @return bool whether this is a valid answer. */ protected function is_valid_answer($answer, $data) { - return $answer == '*' || is_numeric($answer); + return $answer == '*' || $this->is_valid_number($x); + } + + /** + * Validate that a string is a nubmer formatted correctly for the current locale. + * @param string $x a string + * @return bool whether $x is a number that the numerical question type can interpret. + */ + protected function is_valid_number($x) { + if (is_null($this->ap)) { + $this->ap = new qtype_numerical_answer_processor(array()); + } + + list($value, $unit) = $this->ap->apply_units($x); + + return !is_null($value) && !$unit; } /** diff --git a/question/type/numerical/lang/en/qtype_numerical.php b/question/type/numerical/lang/en/qtype_numerical.php index 9008651a5b3..a8d65fb7f47 100644 --- a/question/type/numerical/lang/en/qtype_numerical.php +++ b/question/type/numerical/lang/en/qtype_numerical.php @@ -27,7 +27,7 @@ $string['acceptederror'] = 'Accepted error'; $string['addingnumerical'] = 'Adding a Numerical question'; $string['addmoreanswerblanks'] = 'Blanks for {no} More Answers'; $string['addmoreunitblanks'] = 'Blanks for {no} More Units'; -$string['answermustbenumberorstar'] = 'The answer must be a number, or \'*\'.'; +$string['answermustbenumberorstar'] = 'The answer must be a number, for example -1.234 or 3e8, or \'*\'.'; $string['answerno'] = 'Answer {$a}'; $string['decfractionofquestiongrade'] = 'as a fraction (0-1) of the question grade'; $string['decfractionofresponsegrade'] = 'as a fraction (0-1) of the response grade'; diff --git a/question/type/numerical/simpletest/testform.php b/question/type/numerical/simpletest/testform.php new file mode 100755 index 00000000000..622ad745f4a --- /dev/null +++ b/question/type/numerical/simpletest/testform.php @@ -0,0 +1,80 @@ +. + +/** + * Unit tests for (some of) question/type/numerical/edit_numerical_form.php. + * + * @package qtype + * @subpackage numerical + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + + +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->dirroot . '/question/type/numerical/edit_numerical_form.php'); + + +/** + * Test sub-class, so we can force the locale. + * + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class test_qtype_numerical_edit_form extends qtype_numerical_edit_form { + public function __construct() { + // Warning, avoid running the parent constructor. That means the form is + // not properly tested but for now that is OK, we are only testing a few + // methods. + $this->ap = new qtype_numerical_answer_processor(array(), false, ',', ' '); + } + public function is_valid_number($x) { + return parent::is_valid_number($x); + } +} + + +/** + * Unit tests for question/type/numerical/edit_numerical_form.php. + * + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class qtype_numerical_form_test extends UnitTestCase { + public static $includecoverage = array( + 'question/type/numerical/edit_numerical_form.php' + ); + + protected $form; + + public function setUp() { + $this->form = new test_qtype_numerical_edit_form(); + } + + public function tearDown() { + $this->form = null; + } + + public function test_is_valid_number() { + $this->assertTrue($this->form->is_valid_number('1,001')); + $this->assertFalse($this->form->is_valid_number('1.001')); + $this->assertTrue($this->form->is_valid_number('1')); + $this->assertTrue($this->form->is_valid_number('1,e8')); + $this->assertFalse($this->form->is_valid_number('1001 xxx')); + $this->assertFalse($this->form->is_valid_number('1.e8')); + } +}