MDL-27418 correct validation of numerical answers in qtype numeric.

This mostly affects locales that use , as decimal point.

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

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';

question/type/numerical/simpletest/testform.php

@@ -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'));
+    }
+}