From 5d2465c3f4b5e0644d5daa830fdf478c61642f4b Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Wed, 27 Apr 2011 18:00:14 +0100 Subject: [PATCH] MDL-20636 numerical qtype: assorted changes 1. database upgrade to merge instructions into the questiontext field, and remove the UNITDISPLAY option. 2. Changes to the validation in deferred feedback mode, so students are warned about incomplete answers. 3. Using this to wark students if they enter an answer that is not recognised as a number. --- backup/moodle2/backup_qtype_plugin.class.php | 14 +- .../behaviour/deferredfeedback/behaviour.php | 27 +++ .../format/gift/simpletest/testgiftformat.php | 2 - .../ddwtos/simpletest/testwalkthrough.php | 4 +- .../type/match/simpletest/testwalkthrough.php | 2 +- .../backup_qtype_numerical_plugin.class.php | 2 +- question/type/numerical/db/install.xml | 6 +- question/type/numerical/db/upgrade.php | 173 ++++++++++++++++- .../type/numerical/edit_numerical_form.php | 22 --- .../numerical/lang/en/qtype_numerical.php | 9 +- question/type/numerical/question.php | 42 +++- question/type/numerical/questiontype.php | 35 ++-- .../simpletest/testanswerprocessor.php | 6 +- .../numerical/simpletest/testquestion.php | 2 +- .../numerical/simpletest/testwalkthrough.php | 181 ++++++++++++++++++ question/type/numerical/version.php | 2 +- 16 files changed, 440 insertions(+), 89 deletions(-) create mode 100644 question/type/numerical/simpletest/testwalkthrough.php diff --git a/backup/moodle2/backup_qtype_plugin.class.php b/backup/moodle2/backup_qtype_plugin.class.php index c0d48436f4260..c5cc3d36f7c80 100644 --- a/backup/moodle2/backup_qtype_plugin.class.php +++ b/backup/moodle2/backup_qtype_plugin.class.php @@ -101,8 +101,7 @@ protected function add_question_numerical_options($element) { // Define the elements $options = new backup_nested_element('numerical_options'); $option = new backup_nested_element('numerical_option', array('id'), array( - 'instructions', 'instructionsformat', 'showunits', 'unitsleft', - 'unitgradingtype', 'unitpenalty')); + 'showunits', 'unitsleft', 'unitgradingtype', 'unitpenalty')); // Build the tree $element->add_child($options); @@ -192,15 +191,4 @@ public static function get_components_and_fileareas($filter = null) { } return $components; } - - /** - * Returns one array with filearea => mappingname elements for the qtype - * - * Used by {@link get_components_and_fileareas} to know about all the qtype - * files to be processed both in backup and restore. - */ - public static function get_qtype_fileareas() { - // By default, return empty array, only qtypes having own fileareas will override this - return array(); - } } diff --git a/question/behaviour/deferredfeedback/behaviour.php b/question/behaviour/deferredfeedback/behaviour.php index 579c0076e8a7a..c739795714d59 100644 --- a/question/behaviour/deferredfeedback/behaviour.php +++ b/question/behaviour/deferredfeedback/behaviour.php @@ -68,6 +68,33 @@ public function process_action(question_attempt_pending_step $pendingstep) { } } + /* + * Like the parent method, except that when a respones is gradable, but not + * completely, we move it to the invalid state. + * + * TODO refactor, to remove the duplication. + */ + public function process_save(question_attempt_pending_step $pendingstep) { + if ($this->qa->get_state()->is_finished()) { + return question_attempt::DISCARD; + } else if (!$this->qa->get_state()->is_active()) { + throw new coding_exception('Question is not active, cannot process_actions.'); + } + + if ($this->is_same_response($pendingstep)) { + return question_attempt::DISCARD; + } + + if ($this->is_complete_response($pendingstep)) { + $pendingstep->set_state(question_state::$complete); + } else if ($this->question->is_gradable_response($pendingstep->get_qt_data())) { + $pendingstep->set_state(question_state::$invalid); + } else { + $pendingstep->set_state(question_state::$todo); + } + return question_attempt::KEEP; + } + public function summarise_action(question_attempt_step $step) { if ($step->has_behaviour_var('comment')) { return $this->summarise_manual_comment($step); diff --git a/question/format/gift/simpletest/testgiftformat.php b/question/format/gift/simpletest/testgiftformat.php index 14171bbc3fc5e..54bc5720eedc2 100644 --- a/question/format/gift/simpletest/testgiftformat.php +++ b/question/format/gift/simpletest/testgiftformat.php @@ -563,8 +563,6 @@ public function test_export_numerical() { 'options' => (object) array( 'id' => 123, 'question' => 666, - 'instructions' => '', - 'instructionsformat' => FORMAT_MOODLE, 'showunits' => 0, 'unitsleft' => 0, 'showunits' => 2, diff --git a/question/type/ddwtos/simpletest/testwalkthrough.php b/question/type/ddwtos/simpletest/testwalkthrough.php index 11adf6388b084..1435dd1e0f579 100644 --- a/question/type/ddwtos/simpletest/testwalkthrough.php +++ b/question/type/ddwtos/simpletest/testwalkthrough.php @@ -198,7 +198,7 @@ public function test_deferred_feedback() { $this->process_submission(array('p1' => '1', 'p2' => '2')); // Verify. - $this->check_current_state(question_state::$todo); + $this->check_current_state(question_state::$invalid); $this->check_current_mark(null); $this->check_current_output( $this->get_contains_drop_box_expectation('p1', 1, false), @@ -338,7 +338,7 @@ public function test_deferred_feedback_partial_answer() { $this->process_submission(array('p1' => '1', 'p2' => '0', 'p3' => '0')); // Verify. - $this->check_current_state(question_state::$todo); + $this->check_current_state(question_state::$invalid); $this->check_current_mark(null); $this->check_current_output( $this->get_contains_drop_box_expectation('p1', 1, false), diff --git a/question/type/match/simpletest/testwalkthrough.php b/question/type/match/simpletest/testwalkthrough.php index ea884decf3713..1fc86401d03a3 100644 --- a/question/type/match/simpletest/testwalkthrough.php +++ b/question/type/match/simpletest/testwalkthrough.php @@ -124,7 +124,7 @@ public function test_deferred_feedback_partial_answer() { 'sub1' => $orderforchoice[2], 'sub2' => '0', 'sub3' => '0')); // Verify. - $this->check_current_state(question_state::$todo); + $this->check_current_state(question_state::$invalid); $this->check_current_mark(null); $this->check_current_output( $this->get_contains_select_expectation('sub0', $choices, 1, true), diff --git a/question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php b/question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php index 9c91ff0b70da7..a0693955bd388 100644 --- a/question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php +++ b/question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php @@ -83,6 +83,6 @@ protected function define_question_plugin_structure() { * files to be processed both in backup and restore. */ public static function get_qtype_fileareas() { - return array('instruction' => 'question_created'); + return array(); } } diff --git a/question/type/numerical/db/install.xml b/question/type/numerical/db/install.xml index f5cc856bf0bd5..21b76313ecfbb 100644 --- a/question/type/numerical/db/install.xml +++ b/question/type/numerical/db/install.xml @@ -22,10 +22,8 @@ - - - - + + diff --git a/question/type/numerical/db/upgrade.php b/question/type/numerical/db/upgrade.php index 447454b8c8ac6..84f2047f2e663 100644 --- a/question/type/numerical/db/upgrade.php +++ b/question/type/numerical/db/upgrade.php @@ -59,7 +59,7 @@ function xmldb_qtype_numerical_upgrade($oldversion) { if (!$dbman->table_exists($table)) { // $dbman->create_table doesnt return a result, we just have to trust it $dbman->create_table($table); - }//else + } upgrade_plugin_savepoint(true, 2009100100, 'qtype', 'numerical'); } @@ -96,5 +96,176 @@ function xmldb_qtype_numerical_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2009100101, 'qtype', 'numerical'); } + if ($oldversion < 2011042600) { + // Get rid of the instructions field by adding it to the qestion + // text. Also, if the unit was set to be displayed beside the input, + // deal with that within the question text too. + + // The hard-coded constants used here are: + // 2 = the old qtype_numerical::UNITDISPLAY for ->showunits + // 3 = qtype_numerical::UNITNONE + + $fs = get_file_storage(); + + $rs = $DB->get_recordset_sql(' + SELECT q.id AS questionid, + q.questiontext, + q.questiontextformat, + qc.contextid, + qno.id AS qnoid, + qno.instructions, + qno.instructionsformat, + qno.showunits, + qno.unitsleft, + qnu.unit AS defaultunit + + FROM {question} q + FROM {question_categories} qc ON qc.id = q.category + JOIN {question_numerical_options} qno ON qno.question = q.id + JOIN {question_numerical_units} qnu ON qnu.id = ( + SELECT min(id) + FROM {question_numerical_units} + WHERE question = q.id AND ABS(multiplier - 1) < 0.0000000001)'); + foreach ($rs as $numericaloptions) { + if ($numericaloptions->showunits != 2 && empty($numericaloptions->instructions)) { + // Nothing to do for this question. + continue; + } + + $ishtml = qtype_numerical_convert_text_format($numericaloptions); + + $response = '_______________'; + if ($numericaloptions->showunits == 2) { + if ($numericaloptions->unitsleft) { + $response = $numericaloptions->defaultunit . ' _______________'; + } else { + $response = '_______________ ' . $numericaloptions->defaultunit; + } + + $DB->set_field('question_numerical_options', 'showunits', 3, + array('id' => $numericaloptions->qnoid)); + } + + if ($ishtml) { + $numericaloptions->questiontext .= '

' . $response . '

'; + } else { + $numericaloptions->questiontext .= "\n\n" . $response; + } + + if (!empty($numericaloptions->instructions)) { + if ($ishtml) { + $numericaloptions->questiontext .= $numericaloptions->instructions; + } else { + $numericaloptions->questiontext .= "\n\n" . $numericaloptions->instructions; + } + + $oldfiles = $fs->get_area_files($numericaloptions->contextid, + 'qtype_numerical', 'instruction', $numericaloptions->questionid, 'id', false); + foreach ($oldfiles as $oldfile) { + $filerecord = new stdClass(); + $filerecord->component = 'question'; + $filerecord->filearea = 'questiontext'; + $fs->create_file_from_storedfile($filerecord, $oldfile); + } + + if ($oldfiles) { + $fs->delete_area_files($numericaloptions->contextid, + 'qtype_numerical', 'instruction', $numericaloptions->questionid); + } + } + + $updaterecord = new stdClass(); + $updaterecord->id = $numericaloptions->questionid; + $updaterecord->questiontext = $numericaloptions->questiontext; + $updaterecord->questiontextformat = $numericaloptions->questiontextformat; + $DB->update_record('question', $updaterecord); + } + $rs->close(); + } + + if ($oldversion < 2011042601) { + // Define field instructions to be dropped from question_numerical_options + $table = new xmldb_table('question_numerical_options'); + $field = new xmldb_field('instructions'); + + // Conditionally launch drop field instructions + if ($dbman->field_exists($table, $field)) { + $dbman->drop_field($table, $field); + } + + // numerical savepoint reached + upgrade_plugin_savepoint(true, 2011042601, 'qtype', 'numerical'); + } + + if ($oldversion < 2011042602) { + // Define field instructionsformat to be dropped from question_numerical_options + $table = new xmldb_table('question_numerical_options'); + $field = new xmldb_field('instructionsformat'); + + // Conditionally launch drop field instructionsformat + if ($dbman->field_exists($table, $field)) { + $dbman->drop_field($table, $field); + } + + // numerical savepoint reached + upgrade_plugin_savepoint(true, 2011042602, 'qtype', 'numerical'); + } + return true; } + + +/** + * Convert the ->questiontext and ->instructions fields to have the same text format. + * If they are already the same, do nothing. Otherwise, this method works by + * converting both to HTML. + * @param $numericaloptions the data to convert. + * @return bool true if the resulting fields are in HTML, as opposed to one of + * the text-based formats. + */ +function qtype_numerical_convert_text_format($numericaloptions) { + if ($numericaloptions->questiontextformat == $numericaloptions->instructionsformat) { + // Nothing to do: + return $numericaloptions->questiontextformat == FORMAT_HTML; + } + + if ($numericaloptions->questiontextformat != FORMAT_HTML) { + $numericaloptions->questiontext = qtype_numerical_convert_to_html( + $numericaloptions->questiontext, $numericaloptions->questiontextformat); + $numericaloptions->questiontextformat = FORMAT_HTML; + } + + if ($numericaloptions->instructionsformat != FORMAT_HTML) { + $numericaloptions->instructions = qtype_numerical_convert_to_html( + $numericaloptions->instructions, $numericaloptions->instructionsformat); + $numericaloptions->instructionsformat = FORMAT_HTML; + } + + return true; +} + +/** + * Convert some content to HTML. + * @param string $text the content to convert to HTML + * @param int $oldformat One of the FORMAT_... constants. + */ +function qtype_numerical_convert_to_html($text, $oldformat) { + switch ($oldformat) { + // Similar to format_text. + + case FORMAT_PLAIN: + $text = s($text); + $text = str_replace(' ', '  ', $text); + $text = nl2br($text); + return $text; + + case FORMAT_MARKDOWN: + return markdown_to_html($text); + + case FORMAT_MOODLE: + return text_to_html($text, null, $options['para'], $options['newlines']); + + default: + throw new coding_exception('Unexpected text format when upgrading numerical questions.'); + } +} diff --git a/question/type/numerical/edit_numerical_form.php b/question/type/numerical/edit_numerical_form.php index e416531686881..aeff0251fbb1b 100644 --- a/question/type/numerical/edit_numerical_form.php +++ b/question/type/numerical/edit_numerical_form.php @@ -71,7 +71,6 @@ protected function add_unit_options($mform) { $unitoptions = array( qtype_numerical::UNITNONE => get_string('onlynumerical', 'qtype_numerical'), - qtype_numerical::UNITDISPLAY => get_string('oneunitshown', 'qtype_numerical'), qtype_numerical::UNITOPTIONAL => get_string('manynumerical', 'qtype_numerical'), qtype_numerical::UNITGRADED => get_string('unitgraded', 'qtype_numerical'), ); @@ -112,19 +111,12 @@ protected function add_unit_options($mform) { get_string('unitposition', 'qtype_numerical'), $unitsleftoptions); $mform->setDefault('unitsleft', 0); - $mform->addElement('editor', 'instructions', - get_string('instructions', 'qtype_numerical'), null, $this->editoroptions); - $mform->setType('instructions', PARAM_RAW); - $mform->addHelpButton('instructions', 'numericalinstructions', 'qtype_numerical'); - $mform->disabledIf('penaltygrp', 'unitrole', 'eq', qtype_numerical::UNITNONE); - $mform->disabledIf('penaltygrp', 'unitrole', 'eq', qtype_numerical::UNITDISPLAY); $mform->disabledIf('penaltygrp', 'unitrole', 'eq', qtype_numerical::UNITOPTIONAL); $mform->disabledIf('unitsleft', 'unitrole', 'eq', qtype_numerical::UNITNONE); $mform->disabledIf('multichoicedisplay', 'unitrole', 'eq', qtype_numerical::UNITNONE); - $mform->disabledIf('multichoicedisplay', 'unitrole', 'eq', qtype_numerical::UNITDISPLAY); $mform->disabledIf('multichoicedisplay', 'unitrole', 'eq', qtype_numerical::UNITOPTIONAL); } @@ -234,20 +226,6 @@ protected function data_preprocessing_unit_options($question) { $question->unitrole = $question->options->showunits; } - // Instructions field. - $draftitemid = file_get_submitted_draft_itemid('instruction'); - $question->instructions['text'] = file_prepare_draft_area( - $draftitemid, // draftid - $this->context->id, // context - 'qtype_' . $this->qtype(), // component - 'instruction', // filarea - !empty($question->id) ? (int) $question->id : null, // itemid - $this->fileoptions, // options - $question->options->instructions // text - ); - $question->instructions['itemid'] = $draftitemid ; - $question->instructions['format'] = $question->options->instructionsformat; - return $question; } diff --git a/question/type/numerical/lang/en/qtype_numerical.php b/question/type/numerical/lang/en/qtype_numerical.php index 91cb9276122e6..abe1c0600b908 100644 --- a/question/type/numerical/lang/en/qtype_numerical.php +++ b/question/type/numerical/lang/en/qtype_numerical.php @@ -33,12 +33,13 @@ $string['decfractionofquestiongrade'] = 'as a fraction (0-1) of the question grade'; $string['decfractionofresponsegrade'] = 'as a fraction (0-1) of the response grade'; $string['decimalformat'] = 'decimals'; -$string['editableunittext'] = 'a text input element'; +$string['editableunittext'] = 'the text input element'; $string['editingnumerical'] = 'Editing a Numerical question'; $string['errornomultiplier'] = 'You must specify a multiplier for this unit.'; $string['errorrepeatedunit'] = 'You cannot have two units with the same name.'; $string['geometric'] = 'Geometric'; -$string['instructions'] = 'Instructions '; +$string['invalidnumber'] = 'You must enter a valid number.'; +$string['invalidnumbernounit'] = 'You must enter a valid number. Do not include a unit in your response.'; $string['invalidnumericanswer'] = 'One of the answers you entered was not a valid number.'; $string['invalidnumerictolerance'] = 'One of the tolerances you entered was not a valid number.'; $string['leftexample'] = 'on the left, for example $1.00 or £1.00'; @@ -51,8 +52,6 @@ $string['numerical_help'] = 'From the student perspective, a numerical question looks just like a short-answer question. The difference is that numerical answers are allowed to have an accepted error. This allows a fixed range of answers to be evaluated as one answer. For example, if the answer is 10 with an accepted error of 2, then any number between 8 and 12 will be accepted as correct. '; $string['numerical_link'] = 'question/type/numerical'; $string['numericalsummary'] = 'Allows a numerical response, possibly with units, that is graded by comparing against various model answers, possibly with tolerances.'; -$string['numericalinstructions'] = 'Instructions'; -$string['numericalinstructions_help'] = 'Enter any instructions you would like to give the student about how to complete their response.'; $string['numericalmultiplier'] = 'Multiplier'; $string['numericalmultiplier_help'] = 'The multiplier is the factor by which the correct numerical response will be multiplied. @@ -100,7 +99,7 @@ * the wrong unit name is entered into the unit input, or * a unit is entered into the value input box'; $string['unitappliedpenalty'] = 'These marks include a penalty of {$a} for bad unit.'; -$string['unitposition'] = 'Units are displayed'; +$string['unitposition'] = 'Units go'; $string['unitnotselected'] = 'No unit selected'; $string['unithandling'] = 'Unit handling'; $string['validnumberformats'] = 'Valid number formats'; diff --git a/question/type/numerical/question.php b/question/type/numerical/question.php index a879977cbf7bc..60453ea6102af 100644 --- a/question/type/numerical/question.php +++ b/question/type/numerical/question.php @@ -40,7 +40,7 @@ class qtype_numerical_question extends question_graded_automatically { /** @var array of question_answer. */ public $answers = array(); - /** @var int one of the constants UNITNONE, UNITDISPLAY, UNITSELECT or UNITINPUT. */ + /** @var int one of the constants UNITNONE, UNITSELECT or UNITINPUT. */ public $unitdisplay; /** @var int one of the constants UNITGRADEDOUTOFMARK or UNITGRADEDOUTOFMAX. */ public $unitgradingtype; @@ -76,16 +76,43 @@ public function summarise_response(array $response) { } } - public function is_complete_response(array $response) { + public function is_gradable_response(array $response) { return array_key_exists('answer', $response) && ($response['answer'] || $response['answer'] === '0' || $response['answer'] === 0); } + public function is_complete_response(array $response) { + if (!$this->is_gradable_response($response)) { + return false; + } + + list($value, $unit) = $this->ap->apply_units($response['answer']); + if (is_null($value)) { + return false; + } + + if ($this->unitdisplay == qtype_numerical::UNITNONE && $unit) { + return false; + } + + return true; + } + public function get_validation_error(array $response) { - if ($this->is_gradable_response($response)) { - return ''; + if (!$this->is_gradable_response($response)) { + return get_string('pleaseenterananswer', 'qtype_numerical'); } - return get_string('pleaseenterananswer', 'qtype_numerical'); + + list($value, $unit) = $this->ap->apply_units($response['answer']); + if (is_null($value)) { + return get_string('invalidnumber', 'qtype_numerical'); + } + + if ($this->unitdisplay == qtype_numerical::UNITNONE && $unit) { + return get_string('invalidnumbernounit', 'qtype_numerical'); + } + + return ''; } public function is_same_response(array $prevresponse, array $newresponse) { @@ -129,7 +156,7 @@ public function get_correct_answer() { } protected function apply_unit_penalty($fraction, $unit) { - if (!empty($unit)) { + if (!empty($unit) && $this->ap->is_known_unit($unit)) { return $fraction; } @@ -178,9 +205,6 @@ function check_file_access($question, $state, $options, $contextid, $component, } else if ($component == 'question' && $filearea == 'hint') { return $this->check_hint_file_access($qa, $options, $args); - } else if ($component == 'qtype_numerical' && $filearea == 'instruction') { - return true; - } else { return parent::check_file_access($qa, $options, $component, $filearea, $args, $forcedownload); } diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php index caa62be0f1819..a3cf6f8426e4c 100644 --- a/question/type/numerical/questiontype.php +++ b/question/type/numerical/questiontype.php @@ -45,7 +45,6 @@ class qtype_numerical extends question_type { const UNITSELECT = 1; const UNITNONE = 3; - const UNITDISPLAY = 2; const UNITGRADED = 1; const UNITOPTIONAL = 0; @@ -79,7 +78,7 @@ public function get_question_options($question) { array('questionid' => $question->id), 'id ASC'); $this->get_numerical_units($question); - //get_numerical_options() need to know if there are units + // get_numerical_options() need to know if there are units // to set correctly default values $this->get_numerical_options($question); @@ -139,16 +138,12 @@ public function get_numerical_options($question) { $question->options->showunits = self::UNITNONE ; } $question->options->unitsleft = 0; - $question->options->instructions = ''; - $question->options->instructionsformat = editors_get_preferred_format(); } else { $question->options->unitgradingtype = $options->unitgradingtype; $question->options->unitpenalty = $options->unitpenalty; $question->options->showunits = $options->showunits; $question->options->unitsleft = $options->unitsleft; - $question->options->instructions = $options->instructions; - $question->options->instructionsformat = $options->instructionsformat; } return true; @@ -267,7 +262,6 @@ function save_unit_options($question) { if (!$options) { $options = new stdClass(); $options->question = $question->id; - $options->instructions = ''; $options->id = $DB->insert_record('question_numerical_options', $options); } @@ -302,10 +296,6 @@ function save_unit_options($question) { $options->unitsleft = !empty($question->unitsleft); - $options->instructions = $this->import_or_save_files($question->instructions, - $question->context, 'qtype_'.$question->qtype , 'instruction', $question->id); - $options->instructionsformat = $question->instructions['format']; - $DB->update_record('question_numerical_options', $options); // Report any problems. @@ -407,11 +397,6 @@ function print_question_formulation_and_controls(&$question, &$state, $cmoptions $formatoptions->noclean = true; $formatoptions->para = false; $nameprefix = $question->name_prefix; - $component = 'qtype_' . $question->qtype; - // rewrite instructions text - $question->options->instructions = quiz_rewrite_question_urls( - $question->options->instructions, 'pluginfile.php', $context->id, $component, - 'instruction', array($state->attempt, $state->question), $question->id); /// Print question text and media $questiontext = format_text($question->questiontext, @@ -672,9 +657,6 @@ function move_files($questionid, $oldcontextid, $newcontextid) { parent::move_files($questionid, $oldcontextid, $newcontextid); $this->move_files_in_answers($questionid, $oldcontextid, $newcontextid); - - $fs->move_area_files_to_new_context($oldcontextid, - $newcontextid, 'qtype_numerical', 'instruction', $questionid); } protected function delete_files($questionid, $contextid) { @@ -682,7 +664,6 @@ protected function delete_files($questionid, $contextid) { parent::delete_files($questionid, $contextid); $this->delete_files_in_answers($questionid, $contextid); - $fs->delete_area_files($contextid, 'qtype_numerical', 'instruction', $questionid); } } @@ -797,9 +778,6 @@ protected function parse_response($response) { $unit = substr($response, strlen($matchedpart)); } $unit = trim($unit); - if ($unit && !array_key_exists($unit, $this->units)) { - $unit = ''; - } return array($beforepoint, $decimals, $exponent, $unit); } @@ -826,7 +804,7 @@ public function apply_units($response) { $numberstring .= 'e' . $exponent; } - if ($unit) { + if ($unit && $this->is_known_unit($unit)) { $value = $numberstring * $this->units[$unit]; } else { $value = $numberstring * 1; @@ -855,4 +833,13 @@ public function add_unit($answer, $unit = null) { return $answer . ' ' . $unit; } } + + /** + * Is this unit recognised. + * @param string $unit the unit + * @return bool whether this is a unit we recognise. + */ + public function is_known_unit($unit) { + return array_key_exists($unit, $this->units); + } } diff --git a/question/type/numerical/simpletest/testanswerprocessor.php b/question/type/numerical/simpletest/testanswerprocessor.php index d1986aa8c06a3..5ef9620de1a43 100644 --- a/question/type/numerical/simpletest/testanswerprocessor.php +++ b/question/type/numerical/simpletest/testanswerprocessor.php @@ -77,7 +77,7 @@ public function test_parse_response() { $ap->parse_response('1,000.00 m')); $this->assertEqual(array(null, null, null, null), $ap->parse_response('frog')); - $this->assertEqual(array('3', '', '', ''), $ap->parse_response('3 frogs')); + $this->assertEqual(array('3', '', '', 'frogs'), $ap->parse_response('3 frogs')); $this->assertEqual(array(null, null, null, null), $ap->parse_response('. m')); $this->assertEqual(array(null, null, null, null), $ap->parse_response('.e8 m')); $this->assertEqual(array(null, null, null, null), $ap->parse_response(',')); @@ -92,7 +92,7 @@ public function test_apply_units() { $this->assertEqual(array(299792458, 'c'), $ap->apply_units('1c')); $this->assertEqual(array(0.44704, 'mph'), $ap->apply_units('0001.000 mph')); - $this->assertEqual(array(1, ''), $ap->apply_units('1 frogs')); + $this->assertEqual(array(1, 'frogs'), $ap->apply_units('1 frogs')); $this->assertEqual(array(null, null), $ap->apply_units('. m/s')); } @@ -120,6 +120,6 @@ public function test_currency() { $this->assertEqual(array('100', '$'), $ap->apply_units('$100.')); $this->assertEqual(array('100.00', '$'), $ap->apply_units('$100.00')); $this->assertEqual(array('100', ''), $ap->apply_units('100')); - $this->assertEqual(array('100', ''), $ap->apply_units('frog 100')); + $this->assertEqual(array('100', 'frog'), $ap->apply_units('frog 100')); } } diff --git a/question/type/numerical/simpletest/testquestion.php b/question/type/numerical/simpletest/testquestion.php index d788abddc5484..7d61a58f27e69 100644 --- a/question/type/numerical/simpletest/testquestion.php +++ b/question/type/numerical/simpletest/testquestion.php @@ -39,7 +39,7 @@ public function test_is_complete_response() { $this->assertFalse($question->is_complete_response(array())); $this->assertTrue($question->is_complete_response(array('answer' => '0'))); $this->assertTrue($question->is_complete_response(array('answer' => 0))); - $this->assertTrue($question->is_complete_response(array('answer' => 'test'))); + $this->assertFalse($question->is_complete_response(array('answer' => 'test'))); } public function test_is_gradable_response() { diff --git a/question/type/numerical/simpletest/testwalkthrough.php b/question/type/numerical/simpletest/testwalkthrough.php new file mode 100644 index 0000000000000..0240f17cbf174 --- /dev/null +++ b/question/type/numerical/simpletest/testwalkthrough.php @@ -0,0 +1,181 @@ +. + +/** + * This file contains overall tests of numerical questions. + * + * @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/engine/simpletest/helpers.php'); +require_once($CFG->dirroot . '/question/type/numerical/simpletest/helper.php'); + + +/** + * Unit tests for the numerical question type. + * + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class qtype_numerical_walkthrough_test extends qbehaviour_walkthrough_test_base { + public function test_interactive_currency() { + + // Create a gapselect question. + $q = test_question_maker::make_question('numerical', 'currency'); + $q->hints = array( + new question_hint(1, 'This is the first hint.', FORMAT_HTML), + new question_hint(2, 'This is the second hint.', FORMAT_HTML), + ); + $this->start_attempt_at_question($q, 'interactive', 3); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation(), + $this->get_does_not_contain_validation_error_expectation(), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Submit blank. + $this->process_submission(array('-submit' => 1, 'answer' => '')); + + // Verify. + $this->check_current_state(question_state::$invalid); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation(), + $this->get_contains_validation_error_expectation(), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Sumit something that does not look liek a number. + $this->process_submission(array('-submit' => 1, 'answer' => 'newt')); + + // Verify. + $this->check_current_state(question_state::$invalid); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation(), + $this->get_contains_validation_error_expectation(), + new PatternExpectation('/' . + preg_quote(get_string('invalidnumber', 'qtype_numerical') . '/')), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Now get it right. + $this->process_submission(array('-submit' => 1, 'answer' => '$1332')); + + // Verify. + $this->check_current_state(question_state::$gradedright); + $this->check_current_mark(3); + $this->check_current_output( + $this->get_contains_mark_summary(3), + $this->get_contains_submit_button_expectation(false), + $this->get_contains_correct_expectation(), + $this->get_does_not_contain_validation_error_expectation(), + $this->get_no_hint_visible_expectation()); + $this->assertEqual('$1332', + $this->quba->get_response_summary($this->slot)); + } + + public function test_deferredfeedback_currency() { + + // Create a gapselect question. + $q = test_question_maker::make_question('numerical', 'currency'); + $this->start_attempt_at_question($q, 'deferredfeedback', 3); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_does_not_contain_feedback_expectation(), + $this->get_does_not_contain_validation_error_expectation(), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Submit blank. + $this->process_submission(array('answer' => '')); + + // Verify. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_does_not_contain_feedback_expectation(), + $this->get_does_not_contain_validation_error_expectation(), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Sumit something that does not look like a number. + $this->process_submission(array('answer' => '$newt')); + + // Verify. + $this->check_current_state(question_state::$invalid); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_does_not_contain_feedback_expectation(), + $this->get_contains_validation_error_expectation(), + new PatternExpectation('/' . + preg_quote(get_string('invalidnumber', 'qtype_numerical') . '/')), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Now put in the right answer but without a unit. + $this->process_submission(array('answer' => '1332')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_does_not_contain_feedback_expectation(), + $this->get_does_not_contain_validation_error_expectation(), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Submit all and finish. + $this->process_submission(array('-finish' => '1')); + + // Verify. + $this->check_current_state(question_state::$gradedpartial); + $this->check_current_mark(2.4); + $this->check_current_output( + $this->get_contains_mark_summary(2.4), + $this->get_contains_partcorrect_expectation(), + $this->get_does_not_contain_validation_error_expectation(), + $this->get_no_hint_visible_expectation()); + $this->assertEqual('1332', + $this->quba->get_response_summary($this->slot)); + } + + // Todo. Test validation if you try to type a unit into a question that does + // not expect one. +} diff --git a/question/type/numerical/version.php b/question/type/numerical/version.php index 9001eb36044fd..46f8ef804202e 100644 --- a/question/type/numerical/version.php +++ b/question/type/numerical/version.php @@ -26,5 +26,5 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2010090501; +$plugin->version = 2011042602; $plugin->requires = 2010090501;