Skip to content

Commit

Permalink
MDL-27418 correct validation of numerical answers in qtype numeric.
Browse files Browse the repository at this point in the history
This mostly affects locales that use , as decimal point.
  • Loading branch information
timhunt committed Jun 28, 2011
1 parent 3aa1597 commit 9048940
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 4 deletions.
26 changes: 23 additions & 3 deletions question/type/numerical/edit_numerical_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');


Expand All @@ -36,6 +37,7 @@
* @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}'),
Expand All @@ -54,6 +56,7 @@ protected function get_per_answer_fields($mform, $label, $gradeoptions,
$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);

Expand Down Expand Up @@ -256,7 +259,7 @@ protected function validate_answers($data, $errors) {
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;
Expand All @@ -267,7 +270,7 @@ protected function validate_answers($data, $errors) {
}
} 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++;
}
}
Expand All @@ -277,6 +280,8 @@ protected function validate_answers($data, $errors) {
if ($maxgrade == false) {
$errors['fraction[0]'] = get_string('fractionsnomax', 'question');
}

return $errors;
}

/**
Expand All @@ -286,7 +291,22 @@ protected function validate_answers($data, $errors) {
* @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;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion question/type/numerical/lang/en/qtype_numerical.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
$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';
Expand Down
80 changes: 80 additions & 0 deletions question/type/numerical/simpletest/testform.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* 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'));
}
}

0 comments on commit 9048940

Please sign in to comment.