diff --git a/lib/form/float.php b/lib/form/float.php new file mode 100644 index 00000000000..8f375c2b986 --- /dev/null +++ b/lib/form/float.php @@ -0,0 +1,187 @@ +. + + +/** + * Float type form element + * + * Contains HTML class for a float type element + * + * @package core_form + * @category form + * @copyright 2019 Shamim Rezaie + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->libdir . '/form/text.php'); + +/** + * Float type form element. + * + * This is preferred over the text element when working with float numbers, and takes care of the fact that different languages + * may use different symbols as the decimal separator. + * Using this element, submitted float numbers will be automatically translated from the localised format into the computer format, + * and vice versa when they are being displayed. + * + * @copyright 2019 Shamim Rezaie + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class MoodleQuickForm_float extends MoodleQuickForm_text { + + /** + * MoodleQuickForm_float constructor. + * + * @param string $elementName (optional) name of the float field + * @param string $elementLabel (optional) float field label + * @param string $attributes (optional) Either a typical HTML attribute string or an associative array + */ + public function __construct($elementName = null, $elementLabel = null, $attributes = null) { + parent::__construct($elementName, $elementLabel, $attributes); + $this->_type = 'float'; + } + + /** + * Called by HTML_QuickForm whenever form event is made on this element. + * + * @param string $event Name of event + * @param mixed $arg event arguments + * @param object $caller calling object + * @return bool + */ + public function onQuickFormEvent($event, $arg, &$caller) { + switch ($event) { + case 'updateValue': + if ($value = $this->_findValue($caller->_constantValues)) { + $value = $this->format_float($value); + } + if (null === $value) { + $value = $this->_findValue($caller->_submitValues); + if (null === $value) { + if ($value = $this->_findValue($caller->_defaultValues)) { + $value = $this->format_float($value); + } + } + } + if (null !== $value) { + parent::setValue($value); + } + return true; + case 'createElement': + $caller->setType($arg[0], PARAM_RAW_TRIMMED); + default: + return parent::onQuickFormEvent($event, $arg, $caller); + } + } + + /** + * Checks that the submitted value is a valid float number. + * + * @param string $value The localised float number that is submitted. + * @return string|null Validation error message or null. + */ + public function validateSubmitValue($value) { + if (false === unformat_float($value, true)) { + return get_string('err_numeric', 'core_form'); + } + } + + /** + * Sets the value of the form element. + * + * @param string $value Default value of the form element + */ + public function setValue($value) { + $value = $this->format_float($value); + parent::setValue($value); + } + + /** + * Returns the value of the form element. + * + * @return false|float + */ + public function getValue() { + $value = parent::getValue(); + if ($value) { + $value = unformat_float($value, true); + } + return $value; + } + + /** + * Returns a 'safe' element's value. + * + * @param array $submitValues array of submitted values to search + * @param bool $assoc whether to return the value as associative array + * @return mixed + */ + public function exportValue(&$submitValues, $assoc = false) { + $value = $this->_findValue($submitValues); + if (null === $value) { + $value = $this->getValue(); + } else if ($value) { + $value = unformat_float($value, true); + } + return $this->_prepareValue($value, $assoc); + } + + /** + * Used by getFrozenHtml() to pass the element's value if _persistantFreeze is on. + * + * @return string + */ + public function _getPersistantData() { + if (!$this->_persistantFreeze) { + return ''; + } else { + $id = $this->getAttribute('id'); + if (isset($id)) { + // Id of persistant input is different then the actual input. + $id = array('id' => $id . '_persistant'); + } else { + $id = array(); + } + + return '_getAttrString(array( + 'type' => 'hidden', + 'name' => $this->getAttribute('name'), + 'value' => $this->getAttribute('value') + ) + $id) . ' />'; + } + } + + /** + * Given a float, prints it nicely. + * This function reserves the number of decimal places. + * + * @param float|null $value The float number to format + * @return string Localised float + */ + private function format_float($value) { + if (is_numeric($value)) { + if ($value > 0) { + $decimals = strlen($value) - strlen(floor($value)) - 1; + } else { + $decimals = strlen($value) - strlen(ceil($value)) - 1; + } + $value = format_float($value, $decimals); + } + return $value; + } +} diff --git a/lib/form/group.php b/lib/form/group.php index f4f241fd045..c8a3935d935 100644 --- a/lib/form/group.php +++ b/lib/form/group.php @@ -257,4 +257,22 @@ class MoodleQuickForm_group extends HTML_QuickForm_group implements templatable } $renderer->finishGroup($this); } + + /** + * Calls the validateSubmitValue function for the containing elements and returns an error string as soon as it finds one. + * + * @param array $values Values of the containing elements. + * @return string|null Validation error message or null. + */ + public function validateSubmitValue($values) { + foreach ($this->_elements as $element) { + if (method_exists($element, 'validateSubmitValue')) { + $value = $values[$element->getName()] ?? null; + $result = $element->validateSubmitValue($value); + if (!empty($result) && is_string($result)) { + return $result; + } + } + } + } } diff --git a/lib/form/tests/float_test.php b/lib/form/tests/float_test.php new file mode 100644 index 00000000000..edb72fb1e87 --- /dev/null +++ b/lib/form/tests/float_test.php @@ -0,0 +1,99 @@ +. + +/** + * Unit tests for float form element. + * + * @package core_form + * @category test + * @copyright 2019 Shamim Rezaie + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->libdir . '/form/float.php'); + +/** + * Unit tests for MoodleQuickForm_float + * + * Contains test cases for testing MoodleQuickForm_float + * + * @package core_form + * @copyright 2019 Shamim Rezaie + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_form_float_testcase extends advanced_testcase { + + /** + * Define a local decimal separator. + * + * It is not possible to directly change the result of get_string in + * a unit test. Instead, we create a language pack for language 'xx' in + * dataroot and make langconfig.php with the string we need to change. + * The example separator used here is 'X'. + */ + protected function define_local_decimal_separator() { + global $SESSION, $CFG; + + $SESSION->lang = 'xx'; + $langconfig = "dataroot . '/lang/xx'; + check_dir_exists($langfolder); + file_put_contents($langfolder . '/langconfig.php', $langconfig); + } + + /** + * Testcase to check generated timestamp + */ + public function test_exportValue() { + $element = new MoodleQuickForm_float('testel'); + + $value = ['testel' => 3.14]; + $this->assertEquals(3.14, $element->exportValue($value)); + + $value = ['testel' => '3.14']; + $this->assertEquals(3.14, $element->exportValue($value)); + + $value = ['testel' => '-3.14']; + $this->assertEquals(-3.14, $element->exportValue($value)); + + $value = ['testel' => '3.14blah']; + $this->assertEquals(false, $element->exportValue($value)); + + $value = ['testel' => 'blah']; + $this->assertEquals(false, $element->exportValue($value)); + + // Tests with a localised decimal separator. + $this->define_local_decimal_separator(); + + $value = ['testel' => 3.14]; + $this->assertEquals(3.14, $element->exportValue($value)); + + $value = ['testel' => '3X14']; + $this->assertEquals(3.14, $element->exportValue($value)); + + $value = ['testel' => '-3X14']; + $this->assertEquals(-3.14, $element->exportValue($value)); + + $value = ['testel' => '3X14blah']; + $this->assertEquals(false, $element->exportValue($value)); + + $value = ['testel' => 'blah']; + $this->assertEquals(false, $element->exportValue($value)); + } +} diff --git a/lib/formslib.php b/lib/formslib.php index 2950a03bce0..8ed10a4387c 100644 --- a/lib/formslib.php +++ b/lib/formslib.php @@ -3253,6 +3253,7 @@ MoodleQuickForm::registerElementType('editor', "$CFG->libdir/form/editor.php", ' MoodleQuickForm::registerElementType('filemanager', "$CFG->libdir/form/filemanager.php", 'MoodleQuickForm_filemanager'); MoodleQuickForm::registerElementType('filepicker', "$CFG->libdir/form/filepicker.php", 'MoodleQuickForm_filepicker'); MoodleQuickForm::registerElementType('filetypes', "$CFG->libdir/form/filetypes.php", 'MoodleQuickForm_filetypes'); +MoodleQuickForm::registerElementType('float', "$CFG->libdir/form/float.php", 'MoodleQuickForm_float'); MoodleQuickForm::registerElementType('grading', "$CFG->libdir/form/grading.php", 'MoodleQuickForm_grading'); MoodleQuickForm::registerElementType('group', "$CFG->libdir/form/group.php", 'MoodleQuickForm_group'); MoodleQuickForm::registerElementType('header', "$CFG->libdir/form/header.php", 'MoodleQuickForm_header'); diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 321560d3856..42e5ca1fcea 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -137,14 +137,17 @@ define('PARAM_FILE', 'file'); * * Note that you should not use PARAM_FLOAT for numbers typed in by the user. * It does not work for languages that use , as a decimal separator. - * Instead, do something like - * $rawvalue = required_param('name', PARAM_RAW); - * // ... other code including require_login, which sets current lang ... - * $realvalue = unformat_float($rawvalue); - * // ... then use $realvalue + * Use PARAM_LOCALISEDFLOAT instead. */ define('PARAM_FLOAT', 'float'); +/** + * PARAM_LOCALISEDFLOAT - a localised real/floating point number. + * This is preferred over PARAM_FLOAT for numbers typed in by the user. + * Cleans localised numbers to computer readable numbers; false for invalid numbers. + */ +define('PARAM_LOCALISEDFLOAT', 'localisedfloat'); + /** * PARAM_HOST - expected fully qualified domain name (FQDN) or an IPv4 dotted quad (IP address) */ @@ -843,6 +846,10 @@ function clean_param($param, $type) { // Convert to float. return (float)$param; + case PARAM_LOCALISEDFLOAT: + // Convert to float. + return unformat_float($param, true); + case PARAM_ALPHA: // Remove everything not `a-z`. return preg_replace('/[^a-zA-Z]/i', '', $param); diff --git a/lib/questionlib.php b/lib/questionlib.php index 6d96f8452a3..a089358fa57 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -806,7 +806,7 @@ function question_preview_url($questionid, $preferredbehaviour = null, } if (!is_null($maxmark)) { - $params['maxmark'] = $maxmark; + $params['maxmark'] = format_float($maxmark, strlen($maxmark), true, true); } if (!is_null($displayoptions)) { diff --git a/lib/tests/behat/behat_data_generators.php b/lib/tests/behat/behat_data_generators.php index 51664c8d749..ce1584b4bdf 100644 --- a/lib/tests/behat/behat_data_generators.php +++ b/lib/tests/behat/behat_data_generators.php @@ -208,6 +208,10 @@ class behat_data_generators extends behat_base { 'required' => array('user', 'contact'), 'switchids' => array('user' => 'userid', 'contact' => 'contactid') ), + 'language customisations' => array( + 'datagenerator' => 'customlang', + 'required' => array('component', 'stringid', 'value'), + ), ); /** @@ -415,6 +419,61 @@ class behat_data_generators extends behat_base { $this->datagenerator->create_block($data['blockname'], $data, $data); } + /** + * Creates language customisation. + * + * @throws Exception + * @throws dml_exception + * @param array $data + * @return void + */ + protected function process_customlang($data) { + global $CFG, $DB, $USER; + + require_once($CFG->dirroot . '/' . $CFG->admin . '/tool/customlang/locallib.php'); + require_once($CFG->libdir . '/adminlib.php'); + + if (empty($data['component'])) { + throw new Exception('\'customlang\' requires the field \'component\' type to be specified'); + } + + if (empty($data['stringid'])) { + throw new Exception('\'customlang\' requires the field \'stringid\' to be specified'); + } + + if (!isset($data['value'])) { + throw new Exception('\'customlang\' requires the field \'value\' to be specified'); + } + + $now = time(); + + tool_customlang_utils::checkout($USER->lang); + + $record = $DB->get_record_sql("SELECT s.* + FROM {tool_customlang} s + JOIN {tool_customlang_components} c ON s.componentid = c.id + WHERE c.name = ? AND s.lang = ? AND s.stringid = ?", + array($data['component'], $USER->lang, $data['stringid'])); + + if (empty($data['value']) && !is_null($record->local)) { + $record->local = null; + $record->modified = 1; + $record->outdated = 0; + $record->timecustomized = null; + $DB->update_record('tool_customlang', $record); + tool_customlang_utils::checkin($USER->lang); + } + + if (!empty($data['value']) && $data['value'] != $record->local) { + $record->local = $data['value']; + $record->modified = 1; + $record->outdated = 0; + $record->timecustomized = $now; + $DB->update_record('tool_customlang', $record); + tool_customlang_utils::checkin($USER->lang); + } + } + /** * Adapter to enrol_user() data generator. * @throws Exception diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index db9ce6d793b..f27a6f59d9e 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -525,6 +525,38 @@ class core_moodlelib_testcase extends advanced_testcase { $this->assertSame('', clean_param('user_', PARAM_COMPONENT)); } + public function test_clean_param_localisedfloat() { + + $this->assertSame(0.5, clean_param('0.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('0X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(0.5, clean_param('.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(10.5, clean_param('10.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('10X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(1000.5, clean_param('1 000.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('1 000X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('1.000.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('1X000X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('nan', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('10.6blah', PARAM_LOCALISEDFLOAT)); + + // Tests with a localised decimal separator. + $this->define_local_decimal_separator(); + + $this->assertSame(0.5, clean_param('0.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(0.5, clean_param('0X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(0.5, clean_param('.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(0.5, clean_param('X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(10.5, clean_param('10.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(10.5, clean_param('10X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(1000.5, clean_param('1 000.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(1000.5, clean_param('1 000X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('1.000.5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('1X000X5', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('nan', PARAM_LOCALISEDFLOAT)); + $this->assertSame(false, clean_param('10X6blah', PARAM_LOCALISEDFLOAT)); + } + public function test_is_valid_plugin_name() { $this->assertTrue(is_valid_plugin_name('forum')); $this->assertTrue(is_valid_plugin_name('forum2')); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index d73a1fe4616..30dbd057adb 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -48,6 +48,7 @@ is disabled). * The following functions have been updated to support the new usage: - make_pluginfile_url - file_rewrite_pluginfile_urls +* New mform element 'float' handles localised floating point numbers. === 3.6 === diff --git a/question/previewlib.php b/question/previewlib.php index 588e435a4ca..6c11625e391 100644 --- a/question/previewlib.php +++ b/question/previewlib.php @@ -52,9 +52,8 @@ class preview_options_form extends moodleform { get_string('howquestionsbehave', 'question'), $behaviours); $mform->addHelpButton('behaviour', 'howquestionsbehave', 'question'); - $mform->addElement('text', 'maxmark', get_string('markedoutof', 'question'), + $mform->addElement('float', 'maxmark', get_string('markedoutof', 'question'), array('size' => '5')); - $mform->setType('maxmark', PARAM_FLOAT); if ($this->_customdata['maxvariant'] > 1) { $variants = range(1, $this->_customdata['maxvariant']); @@ -152,7 +151,7 @@ class question_preview_options extends question_display_options { protected function get_field_types() { return array( 'behaviour' => PARAM_ALPHA, - 'maxmark' => PARAM_FLOAT, + 'maxmark' => PARAM_LOCALISEDFLOAT, 'variant' => PARAM_INT, 'correctness' => PARAM_BOOL, 'marks' => PARAM_INT, diff --git a/question/type/calculated/datasetitems_form.php b/question/type/calculated/datasetitems_form.php index 08b8d56dca0..21ac15f32fa 100644 --- a/question/type/calculated/datasetitems_form.php +++ b/question/type/calculated/datasetitems_form.php @@ -146,8 +146,7 @@ class question_dataset_dependent_items_form extends question_wizard_form { } else { $name = get_string('wildcard', 'qtype_calculated', $datasetdef->name); } - $mform->addElement('text', "number[{$j}]", $name); - $mform->setType("number[{$j}]", PARAM_RAW); // This parameter will be validated in validation(). + $mform->addElement('float', "number[{$j}]", $name); $this->qtypeobj->custom_generator_tools_part($mform, $idx, $j); $idx++; $mform->addElement('hidden', "definition[{$j}]"); @@ -174,7 +173,7 @@ class question_dataset_dependent_items_form extends question_wizard_form { $mform->addElement('static', 'answercomment[' . ($this->noofitems+$key1) . ']', $ans); $mform->addElement('hidden', 'tolerance['.$key.']', ''); - $mform->setType('tolerance['.$key.']', PARAM_RAW); + $mform->setType('tolerance['.$key.']', PARAM_FLOAT); // No need to worry about localisation, as the value of this field will not be shown to users anymore. $mform->setAdvanced('tolerance['.$key.']', true); $mform->addElement('hidden', 'tolerancetype['.$key.']', ''); $mform->setType('tolerancetype['.$key.']', PARAM_RAW); @@ -188,9 +187,8 @@ class question_dataset_dependent_items_form extends question_wizard_form { } else if ( $ans !== '' ) { $mform->addElement('static', 'answercomment[' . ($this->noofitems+$key1) . ']', $ans); - $mform->addElement('text', 'tolerance['.$key.']', + $mform->addElement('float', 'tolerance['.$key.']', get_string('tolerance', 'qtype_calculated')); - $mform->setType('tolerance['.$key.']', PARAM_RAW); $mform->setAdvanced('tolerance['.$key.']', true); $mform->addElement('select', 'tolerancetype['.$key.']', get_string('tolerancetype', 'qtype_numerical'), @@ -287,17 +285,17 @@ class question_dataset_dependent_items_form extends question_wizard_form { foreach ($this->datasetdefs as $defkey => $datasetdef) { if ($k > 0) { if ($datasetdef->category == 0 ) { - $mform->addElement('text', "number[{$j}]", + $mform->addElement('float', "number[{$j}]", get_string('wildcard', 'qtype_calculated', $datasetdef->name)); } else { - $mform->addElement('text', "number[{$j}]", get_string( + $mform->addElement('float', "number[{$j}]", get_string( 'sharedwildcard', 'qtype_calculated', $datasetdef->name)); } } else { $mform->addElement('hidden', "number[{$j}]" , ''); + $mform->setType("number[{$j}]", PARAM_LOCALISEDFLOAT); // Localisation handling has to be done manually. } - $mform->setType("number[{$j}]", PARAM_RAW); // This parameter will be validated in validation(). $mform->addElement('hidden', "itemid[{$j}]"); $mform->setType("itemid[{$j}]", PARAM_INT); @@ -408,7 +406,13 @@ class question_dataset_dependent_items_form extends question_wizard_form { $data = array(); foreach ($this->datasetdefs as $defid => $datasetdef) { if (isset($datasetdef->items[$itemnumber])) { - $formdata["number[{$j}]"] = $datasetdef->items[$itemnumber]->value; + $value = $datasetdef->items[$itemnumber]->value; + if ($this->_form->getElementType("number[{$j}]") == 'hidden') { + // Some of the number elements are from the float type and some are from the hidden type. + // We need to manually handle localised floats for hidden elements. + $value = format_float($value, strlen($value), true, true); + } + $formdata["number[{$j}]"] = $value; $formdata["definition[{$j}]"] = $defid; $formdata["itemid[{$j}]"] = $datasetdef->items[$itemnumber]->id; $data[$datasetdef->name] = $datasetdef->items[$itemnumber]->value; @@ -438,11 +442,16 @@ class question_dataset_dependent_items_form extends question_wizard_form { foreach ($this->datasetdefs as $defid => $datasetdef) { if (!optional_param('updatedatasets', false, PARAM_BOOL) && !optional_param('updateanswers', false, PARAM_BOOL)) { - $formdata["number[{$j}]"] = $this->qtypeobj->generate_dataset_item( - $datasetdef->options); + $value = $this->qtypeobj->generate_dataset_item($datasetdef->options); } else { - $formdata["number[{$j}]"] = $this->_form->getElementValue("number[{$j}]"); + $value = $this->_form->getElementValue("number[{$j}]"); } + if ($this->_form->getElementType("number[{$j}]") == 'hidden') { + // Some of the number elements are from the float type and some are from the hidden type. + // We need to manually handle localised floats for hidden elements. + $value = format_float($value, strlen($value), true, true); + } + $formdata["number[{$j}]"] = $value; $formdata["definition[{$j}]"] = $defid; $formdata["itemid[{$j}]"] = isset($datasetdef->items[$itemnumber]) ? $datasetdef->items[$itemnumber]->id : 0; @@ -455,7 +464,6 @@ class question_dataset_dependent_items_form extends question_wizard_form { $j = $this->noofitems * count($this->datasetdefs) + 1; if (!$this->regenerate && !optional_param('updatedatasets', false, PARAM_BOOL) && !optional_param('updateanswers', false, PARAM_BOOL)) { - $idx = 1; $itemnumber = $this->noofitems + 1; foreach ($this->datasetdefs as $defid => $datasetdef) { if (isset($datasetdef->items[$itemnumber])) { diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php index d7eb25445a3..a6d321da2dd 100644 --- a/question/type/calculated/questiontype.php +++ b/question/type/calculated/questiontype.php @@ -730,14 +730,12 @@ class qtype_calculated extends question_type { public function custom_generator_tools_part($mform, $idx, $j) { $minmaxgrp = array(); - $minmaxgrp[] = $mform->createElement('text', "calcmin[{$idx}]", + $minmaxgrp[] = $mform->createElement('float', "calcmin[{$idx}]", get_string('calcmin', 'qtype_calculated')); - $minmaxgrp[] = $mform->createElement('text', "calcmax[{$idx}]", + $minmaxgrp[] = $mform->createElement('float', "calcmax[{$idx}]", get_string('calcmax', 'qtype_calculated')); $mform->addGroup($minmaxgrp, 'minmaxgrp', get_string('minmax', 'qtype_calculated'), ' - ', false); - $mform->setType("calcmin[{$idx}]", PARAM_FLOAT); - $mform->setType("calcmax[{$idx}]", PARAM_FLOAT); $precisionoptions = range(0, 10); $mform->addElement('select', "calclength[{$idx}]", @@ -754,7 +752,6 @@ class qtype_calculated extends question_type { foreach ($datasetdefs as $datasetdef) { if (preg_match('~^(uniform|loguniform):([^:]*):([^:]*):([0-9]*)$~', $datasetdef->options, $regs)) { - $defid = "{$datasetdef->type}-{$datasetdef->category}-{$datasetdef->name}"; $formdata["calcdistribution[{$idx}]"] = $regs[1]; $formdata["calcmin[{$idx}]"] = $regs[2]; $formdata["calcmax[{$idx}]"] = $regs[3]; diff --git a/question/type/calculatedsimple/edit_calculatedsimple_form.php b/question/type/calculatedsimple/edit_calculatedsimple_form.php index 54fb4f49e93..a48372e02fc 100644 --- a/question/type/calculatedsimple/edit_calculatedsimple_form.php +++ b/question/type/calculatedsimple/edit_calculatedsimple_form.php @@ -147,7 +147,7 @@ class qtype_calculatedsimple_edit_form extends qtype_calculated_edit_form { if ($dummyform->answer = optional_param_array('answer', '', PARAM_NOTAGS)) { // There is always at least one answer... $fraction = optional_param_array('fraction', '', PARAM_FLOAT); - $tolerance = optional_param_array('tolerance', '', PARAM_FLOAT); + $tolerance = optional_param_array('tolerance', '', PARAM_LOCALISEDFLOAT); $tolerancetype = optional_param_array('tolerancetype', '', PARAM_FLOAT); $correctanswerlength = optional_param_array('correctanswerlength', '', PARAM_INT); $correctanswerformat = optional_param_array('correctanswerformat', '', PARAM_INT); @@ -169,9 +169,9 @@ class qtype_calculatedsimple_edit_form extends qtype_calculated_edit_form { $this->datasetdefs = array(); // Rebuild datasetdefs from old values. if ($olddef = optional_param_array('datasetdef', '', PARAM_RAW)) { - $calcmin = optional_param_array('calcmin', '', PARAM_FLOAT); + $calcmin = optional_param_array('calcmin', '', PARAM_LOCALISEDFLOAT); $calclength = optional_param_array('calclength', '', PARAM_INT); - $calcmax = optional_param_array('calcmax', '', PARAM_FLOAT); + $calcmax = optional_param_array('calcmax', '', PARAM_LOCALISEDFLOAT); $oldoptions = optional_param_array('defoptions', '', PARAM_RAW); $newdatasetvalues = false; $sizeofolddef = count($olddef); @@ -255,7 +255,7 @@ class qtype_calculatedsimple_edit_form extends qtype_calculated_edit_form { !($datasettoremove ||$newdataset ||$newdatasetvalues)) { $i = 1; $fromformdefinition = optional_param_array('definition', '', PARAM_NOTAGS); - $fromformnumber = optional_param_array('number', '', PARAM_RAW);// This parameter will be validated in the form. + $fromformnumber = optional_param_array('number', '', PARAM_LOCALISEDFLOAT); $fromformitemid = optional_param_array('itemid', '', PARAM_INT); ksort($fromformdefinition); @@ -354,7 +354,7 @@ class qtype_calculatedsimple_edit_form extends qtype_calculated_edit_form { $j = $this->noofitems * count($this->datasetdefs); for ($itemnumber = $this->noofitems; $itemnumber >= 1; $itemnumber--) { $data = array(); - $numbererrors = array(); + $numbererrors = ''; $comment = new stdClass(); $comment->stranswers = array(); $comment->outsidelimit = false; @@ -495,7 +495,7 @@ class qtype_calculatedsimple_edit_form extends qtype_calculated_edit_form { for ($i = $this->noofitems; $i >= 1; $i--) { foreach ($this->datasetdefs as $defkey => $datasetdef) { if ($k > 0 || $this->outsidelimit || !empty($this->numbererrors)) { - $mform->addElement('text', "number[{$j}]", get_string( + $mform->addElement('float', "number[{$j}]", get_string( 'wildcard', 'qtype_calculatedsimple', $datasetdef->name)); $mform->setAdvanced("number[{$j}]", true); if (!empty($this->numbererrors['number['.$j.']'])) { @@ -505,10 +505,13 @@ class qtype_calculatedsimple_edit_form extends qtype_calculated_edit_form { $mform->setAdvanced("numbercomment[{$j}]", true); } } else { - $mform->addElement('hidden', "number[{$j}]", get_string( - 'wildcard', 'qtype_calculatedsimple', $datasetdef->name)); + $mform->addElement('hidden', "number[{$j}]", ''); + $mform->setType("number[{$j}]", PARAM_LOCALISEDFLOAT); // Localisation handling has to be done manually. + if (isset($this->formdata["number[{$j}]"])) { + $number = $this->formdata["number[{$j}]"]; + $this->formdata["number[{$j}]"] = format_float($number, strlen($number), true, true); + } } - $mform->setType("number[{$j}]", PARAM_RAW); // This parameter will be validated in validation(). $mform->addElement('hidden', "itemid[{$j}]"); $mform->setType("itemid[{$j}]", PARAM_INT); diff --git a/question/type/calculatedsimple/questiontype.php b/question/type/calculatedsimple/questiontype.php index 68f45cb8202..ae2faa30a44 100644 --- a/question/type/calculatedsimple/questiontype.php +++ b/question/type/calculatedsimple/questiontype.php @@ -238,14 +238,12 @@ class qtype_calculatedsimple extends qtype_calculated { public function custom_generator_tools_part($mform, $idx, $j) { $minmaxgrp = array(); - $minmaxgrp[] = $mform->createElement('text', "calcmin[{$idx}]", + $minmaxgrp[] = $mform->createElement('float', "calcmin[{$idx}]", get_string('calcmin', 'qtype_calculated')); - $minmaxgrp[] = $mform->createElement('text', "calcmax[{$idx}]", + $minmaxgrp[] = $mform->createElement('float', "calcmax[{$idx}]", get_string('calcmax', 'qtype_calculated')); $mform->addGroup($minmaxgrp, 'minmaxgrp', get_string('minmax', 'qtype_calculated'), ' - ', false); - $mform->setType("calcmin[{$idx}]", PARAM_FLOAT); - $mform->setType("calcmax[{$idx}]", PARAM_FLOAT); $precisionoptions = range(0, 10); $mform->addElement('select', "calclength[{$idx}]", diff --git a/question/type/calculatedsimple/tests/questiontype_test.php b/question/type/calculatedsimple/tests/questiontype_test.php index 1b9771ab720..3f4ca511eed 100644 --- a/question/type/calculatedsimple/tests/questiontype_test.php +++ b/question/type/calculatedsimple/tests/questiontype_test.php @@ -115,8 +115,8 @@ class qtype_calculatedsimple_test extends advanced_testcase { for ($itemno = 1; $itemno <= 10; $itemno++) { $item = $datasetloader->get_values($itemno); - $this->assertEquals($formdata->number[($itemno -1)*2 + 2], $item['a']); - $this->assertEquals($formdata->number[($itemno -1)*2 + 1], $item['b']); + $this->assertEquals((float)$formdata->number[($itemno -1)*2 + 2], $item['a']); + $this->assertEquals((float)$formdata->number[($itemno -1)*2 + 1], $item['b']); } } } diff --git a/question/type/edit_question_form.php b/question/type/edit_question_form.php index 7b5fb9cde0f..735615a48d8 100644 --- a/question/type/edit_question_form.php +++ b/question/type/edit_question_form.php @@ -187,9 +187,8 @@ abstract class question_edit_form extends question_wizard_form { $mform->setType('questiontext', PARAM_RAW); $mform->addRule('questiontext', null, 'required', null, 'client'); - $mform->addElement('text', 'defaultmark', get_string('defaultmark', 'question'), + $mform->addElement('float', 'defaultmark', get_string('defaultmark', 'question'), array('size' => 7)); - $mform->setType('defaultmark', PARAM_FLOAT); $mform->setDefault('defaultmark', 1); $mform->addRule('defaultmark', null, 'required', null, 'client'); diff --git a/question/type/numerical/edit_numerical_form.php b/question/type/numerical/edit_numerical_form.php index 88027619d06..6788058255f 100644 --- a/question/type/numerical/edit_numerical_form.php +++ b/question/type/numerical/edit_numerical_form.php @@ -57,9 +57,8 @@ class qtype_numerical_edit_form extends question_edit_form { $repeated = parent::get_per_answer_fields($mform, $label, $gradeoptions, $repeatedoptions, $answersoption); - $tolerance = $mform->createElement('text', 'tolerance', + $tolerance = $mform->createElement('float', 'tolerance', get_string('answererror', 'qtype_numerical'), array('size' => 15)); - $repeatedoptions['tolerance']['type'] = PARAM_FLOAT; $repeatedoptions['tolerance']['default'] = 0; $elements = $repeated[0]->getElements(); $elements[0]->setSize(15); @@ -91,9 +90,8 @@ class qtype_numerical_edit_form extends question_edit_form { get_string('unithandling', 'qtype_numerical'), $unitoptions); $penaltygrp = array(); - $penaltygrp[] = $mform->createElement('text', 'unitpenalty', + $penaltygrp[] = $mform->createElement('float', 'unitpenalty', get_string('unitpenalty', 'qtype_numerical'), array('size' => 6)); - $mform->setType('unitpenalty', PARAM_FLOAT); $mform->setDefault('unitpenalty', 0.1000000); $unitgradingtypes = array( @@ -148,7 +146,6 @@ class qtype_numerical_edit_form extends question_edit_form { $repeatedoptions['unit']['disabledif'] = array('unitrole', 'eq', qtype_numerical::UNITNONE); $repeatedoptions['unit']['type'] = PARAM_NOTAGS; $repeatedoptions['multiplier']['disabledif'] = array('unitrole', 'eq', qtype_numerical::UNITNONE); - $repeatedoptions['multiplier']['type'] = PARAM_NUMBER; $mform->disabledIf('addunits', 'unitrole', 'eq', qtype_numerical::UNITNONE); @@ -186,7 +183,7 @@ class qtype_numerical_edit_form extends question_edit_form { protected function unit_group($mform) { $grouparray = array(); $grouparray[] = $mform->createElement('text', 'unit', get_string('unit', 'qtype_numerical'), array('size'=>10)); - $grouparray[] = $mform->createElement('text', 'multiplier', + $grouparray[] = $mform->createElement('float', 'multiplier', get_string('multiplier', 'qtype_numerical'), array('size'=>10)); return $grouparray; @@ -213,6 +210,11 @@ class qtype_numerical_edit_form extends question_edit_form { unset($this->_form->_defaultValues["tolerance[{$key}]"]); $question->tolerance[$key] = $answer->tolerance; + + if (is_numeric($question->answer[$key])) { + $question->answer[$key] = format_float($question->answer[$key], strlen($question->answer[$key]), true, true); + } + $key++; } @@ -291,7 +293,7 @@ class qtype_numerical_edit_form extends question_edit_form { if ($data['fraction'][$key] == 1) { $maxgrade = true; } - if ($answer !== '*' && !is_numeric($data['tolerance'][$key])) { + if ($answer !== '*' && $data['tolerance'][$key] === false) { $errors['answeroptions['.$key.']'] = get_string('xmustbenumeric', 'qtype_numerical', get_string('acceptederror', 'qtype_numerical')); @@ -361,7 +363,7 @@ class qtype_numerical_edit_form extends question_edit_form { if (empty($trimmedmultiplier)) { $errors['units[' . $key . ']'] = get_string('youmustenteramultiplierhere', 'qtype_numerical'); - } else if (!is_numeric($trimmedmultiplier)) { + } else if ($trimmedmultiplier === false) { $errors['units[' . $key . ']'] = get_string('xmustbenumeric', 'qtype_numerical', get_string('multiplier', 'qtype_numerical')); diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php index 82f6898acde..6f39b42e5ce 100644 --- a/question/type/numerical/questiontype.php +++ b/question/type/numerical/questiontype.php @@ -553,7 +553,7 @@ class qtype_numerical_answer_processor { } /** - * @return book If the student's response contains a '.' or a ',' that + * @return bool If the student's response contains a '.' or a ',' that * matches the thousands separator in the current locale. In this case, the * parsing in apply_unit can give a result that the student did not expect. */ @@ -651,7 +651,7 @@ class qtype_numerical_answer_processor { if (strpos($response, '.') !== false || substr_count($response, ',') > 1) { $response = str_replace(',', '', $response); } else { - $response = str_replace(',', '.', $response); + $response = str_replace([$this->thousandssep, $this->decsep, ','], ['', '.', '.'], $response); } $regex = '[+-]?(?:\d+(?:\\.\d*)?|\\.\d+)(?:e[-+]?\d+)?'; diff --git a/question/type/numerical/tests/behat/add.feature b/question/type/numerical/tests/behat/add.feature new file mode 100644 index 00000000000..8133a892b6d --- /dev/null +++ b/question/type/numerical/tests/behat/add.feature @@ -0,0 +1,39 @@ +@qtype @qtype_numerical +Feature: Test creating a Numerical question + As a teacher + In order to test my students + I need to be able to create a Numerical question + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | T1 | Teacher1 | teacher1@example.com | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Question bank" in current page administration + + Scenario: Create a Numerical question + When I add a "Numerical" question filling the form with: + | Question name | Numerical-001 | + | Question text | What is the average of 4, 5, 6 and 10? | + | Default mark | 1 | + | General feedback | The average is 6.25 | + | id_answer_0 | 6.25 | + | id_tolerance_0 | 0.05 | + | id_fraction_0 | 100% | + | id_answer_1 | 2#25 | + | id_tolerance_1 | 0#05 | + | id_fraction_1 | 0% | + | id_answer_2 | 5,1 | + | id_tolerance_2 | 0 | + | id_fraction_2 | 100% | + Then I should see "Numerical-001" diff --git a/question/type/numerical/tests/behat/backup_and_restore.feature b/question/type/numerical/tests/behat/backup_and_restore.feature new file mode 100644 index 00000000000..faac89a51b1 --- /dev/null +++ b/question/type/numerical/tests/behat/backup_and_restore.feature @@ -0,0 +1,57 @@ +@qtype @qtype_numerical +Feature: Test duplicating a quiz containing a Numerical question + As a teacher + In order re-use my courses containing Numerical questions + I need to be able to backup and restore them + + Background: + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | name | template | + | Test questions | numerical | Numerical-001 | pi | + And the following "activities" exist: + | activity | name | course | idnumber | + | quiz | Test quiz | C1 | quiz1 | + And quiz "Test quiz" contains the following questions: + | Numerical-001 | 1 | + And I log in as "admin" + And I am on "Course 1" course homepage + + @javascript + Scenario: Backup and restore a course containing a Numerical question + When I backup "Course 1" course using this options: + | Confirmation | Filename | test_backup.mbz | + And I restore "test_backup.mbz" backup into a new course using this options: + | Schema | Course name | Course 2 | + And I navigate to "Question bank" in current page administration + And I click on "Edit" "link" in the "Numerical-001" "table_row" + Then the following fields match these values: + | Question name | Numerical-001 | + | Question text | What is pi to two d.p.? | + | General feedback | Generalfeedback: 3.14 is the right answer. | + | Default mark | 1 | + | id_answer_0 | 3.14 | + | id_tolerance_0 | 0 | + | id_fraction_0 | 100% | + | id_feedback_0 | Very good. | + | id_answer_1 | 3.142 | + | id_tolerance_1 | 0 | + | id_fraction_1 | None | + | id_feedback_1 | Too accurate. | + | id_answer_2 | 3.1 | + | id_tolerance_2 | 0 | + | id_fraction_2 | None | + | id_feedback_2 | Not accurate enough. | + | id_answer_3 | 3 | + | id_tolerance_3 | 0 | + | id_fraction_3 | None | + | id_feedback_3 | Not accurate enough. | + | id_answer_4 | * | + | id_tolerance_4 | 0 | + | id_fraction_4 | None | + | id_feedback_4 | Completely wrong. | diff --git a/question/type/numerical/tests/behat/edit.feature b/question/type/numerical/tests/behat/edit.feature new file mode 100644 index 00000000000..53435104f21 --- /dev/null +++ b/question/type/numerical/tests/behat/edit.feature @@ -0,0 +1,49 @@ +@qtype @qtype_numerical +Feature: Test editing a Numerical question + As a teacher + In order to be able to update my Numerical question + I need to edit them + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | T1 | Teacher1 | teacher1@example.com | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | name | template | + | Test questions | numerical | Numerical for editing | pi | + And the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Question bank" in current page administration + + Scenario: Edit a Numerical question + When I click on "Edit" "link" in the "Numerical for editing" "table_row" + Then the field "id_answer_0" matches value "3#14" + When I set the following fields to these values: + | Question name | | + And I press "id_submitbutton" + Then I should see "You must supply a value here." + When I set the following fields to these values: + | Question name | Edited Numerical name | + And I press "id_submitbutton" + Then I should see "Edited Numerical name" + When I click on "Edit" "link" in the "Edited Numerical name" "table_row" + And I set the following fields to these values: + | id_answer_1 | 3#141592 | + | id_tolerance_1 | 0#005 | + | id_answer_2 | 3.05 | + | id_tolerance_2 | 0.005 | + | id_answer_3 | 3,01 | + And I press "id_submitbutton" + Then I should see "Edited Numerical name" diff --git a/question/type/numerical/tests/behat/export.feature b/question/type/numerical/tests/behat/export.feature new file mode 100644 index 00000000000..7d3f920819c --- /dev/null +++ b/question/type/numerical/tests/behat/export.feature @@ -0,0 +1,36 @@ +@qtype @qtype_numerical +Feature: Test exporting Numerical questions + As a teacher + In order to be able to reuse my Numerical questions + I need to export them + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | T1 | Teacher1 | teacher1@example.com | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | name | template | + | Test questions | numerical | Numerical-001 | pi | + | Test questions | numerical | Numerical-002 | pi3tries | + And I log in as "teacher1" + And I am on "Course 1" course homepage + + Scenario: Export a Numerical question + When I navigate to "Question bank > Export" in current page administration + And I set the field "id_format_xml" to "1" + And I press "Export questions to file" + Then following "click here" should download between "3650" and "3750" bytes + # If the download step is the last in the scenario then we can sometimes run + # into the situation where the download page causes a http redirect but behat + # has already conducted its reset (generating an error). By putting a logout + # step we avoid behat doing the reset until we are off that page. + And I log out diff --git a/question/type/numerical/tests/behat/import.feature b/question/type/numerical/tests/behat/import.feature new file mode 100644 index 00000000000..6995a63dc0f --- /dev/null +++ b/question/type/numerical/tests/behat/import.feature @@ -0,0 +1,30 @@ +@qtype @qtype_numerical +Feature: Test importing Numerical questions + As a teacher + In order to reuse Numerical questions + I need to import them + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | T1 | Teacher1 | teacher1@example.com | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And I log in as "teacher1" + And I am on "Course 1" course homepage + + @javascript @_file_upload + Scenario: import Numerical question. + When I navigate to "Question bank > Import" in current page administration + And I set the field "id_format_xml" to "1" + And I upload "question/type/numerical/tests/fixtures/testquestion.moodle.xml" file to "Import" filemanager + And I press "id_submitbutton" + Then I should see "Parsing questions from import file." + And I should see "Importing 1 questions from file" + And I should see "1. What is the average of 4, 5, 6 and 10?" + And I press "Continue" + And I should see "Numerical-001" diff --git a/question/type/numerical/tests/behat/preview.feature b/question/type/numerical/tests/behat/preview.feature new file mode 100644 index 00000000000..c794d889e69 --- /dev/null +++ b/question/type/numerical/tests/behat/preview.feature @@ -0,0 +1,51 @@ +@qtype @qtype_numerical +Feature: Preview a Numerical question + As a teacher + In order to check my Numerical questions will work for students + I need to preview them + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | T1 | Teacher1 | teacher1@example.com | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | name | template | + | Test questions | numerical | Numerical-001 | pi | + | Test questions | numerical | Numerical-002 | pi3tries | + And the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + Given I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Question bank" in current page administration + + @javascript @_switch_window + Scenario: Preview a Numerical question and submit a correct response. + When I click on "Preview" "link" in the "Numerical-001" "table_row" + And I switch to "questionpreview" window + Then I should see "What is pi to two d.p.?" + When I set the field "How questions behave" to "Immediate feedback" + And I press "Start again with these options" + And I set the field with xpath "//span[@class='answer']//input[contains(@id, '1_answer')]" to "3.14" + And I press "Check" + Then I should see "Very good." + And I should see "Mark 1#00 out of 1#00" + When I press "Start again" + And I set the field with xpath "//span[@class='answer']//input[contains(@id, '1_answer')]" to "3,14" + And I press "Check" + Then I should see "Please enter your answer without using the thousand separator (,)." + When I press "Start again" + And I set the field with xpath "//span[@class='answer']//input[contains(@id, '1_answer')]" to "3#14" + And I press "Check" + Then I should see "Very good." + And I should see "Mark 1#00 out of 1#00" + And I switch to the main window diff --git a/question/type/numerical/tests/fixtures/testquestion.moodle.xml b/question/type/numerical/tests/fixtures/testquestion.moodle.xml new file mode 100644 index 00000000000..7966cad7b45 --- /dev/null +++ b/question/type/numerical/tests/fixtures/testquestion.moodle.xml @@ -0,0 +1,39 @@ + + + + + + $course$/Default for C1 + + + + + + + + Numerical-001 + + + What is the average of 4, 5, 6 and 10? + + + The average is 6.25 + + 1.0000000 + 0.3333333 + + 6.25 + + + + 0.05 + + + First hint + + + Second hint + + + + diff --git a/theme/boost/templates/core_form/element-float-inline.mustache b/theme/boost/templates/core_form/element-float-inline.mustache new file mode 100644 index 00000000000..ab31355a494 --- /dev/null +++ b/theme/boost/templates/core_form/element-float-inline.mustache @@ -0,0 +1 @@ +{{> core_form/element-text-inline }} diff --git a/theme/boost/templates/core_form/element-float.mustache b/theme/boost/templates/core_form/element-float.mustache new file mode 100644 index 00000000000..b7270e2ef71 --- /dev/null +++ b/theme/boost/templates/core_form/element-float.mustache @@ -0,0 +1 @@ +{{> core_form/element-text }}