From c468ce592292265c2a361bcda7cd62b37ac0df49 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 13 May 2011 11:04:23 +0100 Subject: [PATCH] MDL-20636 Fix upgrade of numerical questions. --- question/engine/bank.php | 2 + .../db/simpletest/testupgradelibnewqe.php | 196 +++++++++++++++++ question/type/numerical/db/upgradelib.php | 40 +++- question/type/numerical/questiontype.php | 4 +- question/type/numerical/simpletest/helper.php | 2 +- .../simpletest/testanswerprocessor.php | 43 ++-- .../numerical/simpletest/testquestion.php | 2 +- .../db/simpletest/testupgradelibnewqe.php | 198 +++++++++++++++++- 8 files changed, 461 insertions(+), 26 deletions(-) diff --git a/question/engine/bank.php b/question/engine/bank.php index f5837519b5c..fd1acc1a550 100644 --- a/question/engine/bank.php +++ b/question/engine/bank.php @@ -31,6 +31,8 @@ defined('MOODLE_INTERNAL') || die(); +require_once(dirname(__FILE__) . '/../type/questiontype.php'); + /** * This static class provides access to the other question bank. diff --git a/question/type/numerical/db/simpletest/testupgradelibnewqe.php b/question/type/numerical/db/simpletest/testupgradelibnewqe.php index e79e9846974..680d620b57c 100644 --- a/question/type/numerical/db/simpletest/testupgradelibnewqe.php +++ b/question/type/numerical/db/simpletest/testupgradelibnewqe.php @@ -262,4 +262,200 @@ class qtype_numerical_attempt_upgrader_test extends question_attempt_upgrader_te $this->assertEqual($expectedqa, $qa); } + + public function test_numerical_deferredfeedback_required_units() { + $quiz = (object) array( + 'id' => '2', + 'course' => '2', + 'name' => 'Numerical quiz', + 'intro' => '', + 'introformat' => '1', + 'timeopen' => '0', + 'timeclose' => '0', + 'attempts' => '0', + 'attemptonlast' => '0', + 'grademethod' => '1', + 'decimalpoints' => '2', + 'questiondecimalpoints' => '-1', + 'review' => '4459503', + 'questionsperpage' => '1', + 'shufflequestions' => '0', + 'shuffleanswers' => '1', + 'questions' => '6,12,13,15,14,0', + 'sumgrades' => '5.00000', + 'grade' => '10.00000', + 'timecreated' => '0', + 'timemodified' => '1305273177', + 'timelimit' => '0', + 'password' => '', + 'subnet' => '', + 'popup' => '0', + 'delay1' => '0', + 'delay2' => '0', + 'showuserpicture' => '0', + 'showblocks' => '0', + 'preferredbehaviour' => 'deferredfeedback', + ); + $attempt = (object) array( + 'id' => '6', + 'uniqueid' => '6', + 'quiz' => '2', + 'userid' => '2', + 'attempt' => '1', + 'sumgrades' => '4.00000', + 'timestart' => '1305273566', + 'timefinish' => '1305273600', + 'timemodified' => '1305273600', + 'layout' => '6,12,13,15,14,0', + 'preview' => '1', + 'needsupgradetonewqe' => 1, + ); + $question = (object) array( + 'id' => '15', + 'category' => '2', + 'parent' => '0', + 'name' => 'Required units', + 'questiontext' => '

What is twice 1.5 m?

', + 'questiontextformat' => '1', + 'generalfeedback' => '', + 'generalfeedbackformat' => '1', + 'penalty' => '0.1000000', + 'qtype' => 'numerical', + 'length' => '1', + 'stamp' => 'tjh238.vledev2.open.ac.uk+110513075857+rrvakr', + 'version' => 'tjh238.vledev2.open.ac.uk+110513075857+PH0loi', + 'hidden' => '0', + 'timecreated' => '1305273537', + 'timemodified' => '1305273537', + 'createdby' => '2', + 'modifiedby' => '2', + 'maxmark' => '1.0000000', + 'options' => (object) array( + 'answers' => array( + 22 => (object) array( + 'id' => '22', + 'question' => '15', + 'answer' => '3', + 'answerformat' => '0', + 'fraction' => '1.0000000', + 'feedback' => '', + 'feedbackformat' => '1', + 'tolerance' => '', + ), + ), + 'units' => array( + 0 => (object) array( + 'id' => '7', + 'question' => '15', + 'multiplier' => 1, + 'unit' => 'm', + ), + 1 => (object) array( + 'id' => '8', + 'question' => '15', + 'multiplier' => 100, + 'unit' => 'cm', + ), + ), + 'unitgradingtype' => '2', + 'unitpenalty' => '0.5000000', + 'showunits' => '0', + 'unitsleft' => '0', + 'instructions' => '

Write an answer like 3 m.

', + 'instructionsformat' => '1', + ), + 'defaultmark' => '1.0000000', + ); + $qsession = (object) array( + 'id' => '49', + 'attemptid' => '6', + 'questionid' => '15', + 'newest' => '139', + 'newgraded' => '139', + 'sumpenalty' => '0.1000000', + 'manualcomment' => '', + 'manualcommentformat' => '1', + 'flagged' => '0', + ); + $qstates = array( + 129 => (object) array( + 'id' => '129', + 'attempt' => '6', + 'question' => '15', + 'seq_number' => '0', + 'answer' => '|||||', + 'timestamp' => '1305273566', + 'event' => '0', + 'grade' => '0.0000000', + 'raw_grade' => '0.0000000', + 'penalty' => '0.0000000', + ), + 134 => (object) array( + 'id' => '134', + 'attempt' => '6', + 'question' => '15', + 'seq_number' => '1', + 'answer' => '3|||||m', + 'timestamp' => '1305273595', + 'event' => '2', + 'grade' => '0.0000000', + 'raw_grade' => '1.0000000', + 'penalty' => '0.1000000', + ), + 139 => (object) array( + 'id' => '139', + 'attempt' => '6', + 'question' => '15', + 'seq_number' => '2', + 'answer' => '3|||||m', + 'timestamp' => '1305273595', + 'event' => '6', + 'grade' => '1.0000000', + 'raw_grade' => '1.0000000', + 'penalty' => '0.1000000', + ), + ); + + $qa = $this->updater->convert_question_attempt($quiz, $attempt, $question, $qsession, $qstates); + + $expectedqa = (object) array( + 'behaviour' => 'deferredfeedback', + 'questionid' => 15, + 'maxmark' => 1.0000000, + 'minfraction' => 0, + 'flagged' => 0, + 'questionsummary' => 'What is twice 1.5 m?', + 'rightanswer' => '3 m', + 'responsesummary' => '3 m', + 'timemodified' => 1305273595, + 'steps' => array( + 0 => (object) array( + 'sequencenumber' => 0, + 'state' => 'todo', + 'fraction' => null, + 'timecreated' => 1305273566, + 'userid' => 2, + 'data' => array('_separators' => '.$,'), + ), + 1 => (object) array( + 'sequencenumber' => 1, + 'state' => 'complete', + 'fraction' => null, + 'timecreated' => 1305273595, + 'userid' => 2, + 'data' => array('answer' => '3 m'), + ), + 2 => (object) array( + 'sequencenumber' => 2, + 'state' => 'gradedright', + 'fraction' => 1, + 'timecreated' => 1305273595, + 'userid' => 2, + 'data' => array('answer' => '3 m', '-finish' => '1'), + ), + ), + ); + + $this->assertEqual($expectedqa, $qa); + } } diff --git a/question/type/numerical/db/upgradelib.php b/question/type/numerical/db/upgradelib.php index cfba998b1c8..d151ed59f05 100644 --- a/question/type/numerical/db/upgradelib.php +++ b/question/type/numerical/db/upgradelib.php @@ -42,17 +42,46 @@ class qtype_numerical_qe2_attempt_updater extends question_qtype_attempt_updater public function right_answer() { foreach ($this->question->options->answers as $ans) { if ($ans->fraction > 0.999) { - return $ans->answer; + $right = $ans->answer; + + if (empty($this->question->options->units)) { + return $right; + } + + $unit = reset($this->question->options->units); + $unit = $unit->unit; + if (!empty($this->question->options->unitsleft)) { + return $unit . ' ' . $right; + } else { + return $right . ' ' . $unit; + } } } } public function response_summary($state) { - if (!empty($state->answer)) { - return $state->answer; + if (strpos($state->answer, '|||||') === false) { + $answer = $state->answer; + $unit = ''; } else { - return null; + list($answer, $unit) = explode('|||||', $state->answer, 2); } + + if (empty($answer) && empty($unit)) { + $resp = null; + } else { + $resp = $answer; + } + + if (!empty($unit)) { + if (!empty($this->question->options->unitsleft)) { + $resp = trim($unit . ' ' . $resp); + } else { + $resp = trim($resp . ' ' . $unit); + } + } + + return $resp; } public function was_answered($state) { @@ -75,7 +104,8 @@ class qtype_numerical_qe2_attempt_updater extends question_qtype_attempt_updater $data['answer'] = $state->answer; } else { list($answer, $unit) = explode('|||||', $state->answer, 2); - if ($this->question->options->showunits == 1) { + if (!empty($this->question->options->showunits) && + $this->question->options->showunits == 1) { // Multichoice units. $data['answer'] = $answer; $data['unit'] = $unit; diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php index d8d3dc895fe..8bf9416a1bf 100644 --- a/question/type/numerical/questiontype.php +++ b/question/type/numerical/questiontype.php @@ -28,7 +28,7 @@ defined('MOODLE_INTERNAL') || die(); -require_once($CFG->dirroot . '/question/type/numerical/question.php'); +//require_once($CFG->dirroot . '/question/type/numerical/question.php'); /** @@ -809,7 +809,7 @@ class qtype_numerical_answer_processor { } if ($unit && $this->is_known_unit($unit)) { - $value = $numberstring * $this->units[$unit]; + $value = $numberstring / $this->units[$unit]; } else { $value = $numberstring * 1; } diff --git a/question/type/numerical/simpletest/helper.php b/question/type/numerical/simpletest/helper.php index 5be71c49b48..e0ddc4a5719 100644 --- a/question/type/numerical/simpletest/helper.php +++ b/question/type/numerical/simpletest/helper.php @@ -88,7 +88,7 @@ class qtype_numerical_test_helper extends question_test_helper { $num->unitdisplay = qtype_numerical::UNITSELECT; $num->unitgradingtype = qtype_numerical::UNITGRADEDOUTOFMARK; $num->unitpenalty = 0.5; - $num->ap = new qtype_numerical_answer_processor(array('m' => 1, 'cm' => 0.01)); + $num->ap = new qtype_numerical_answer_processor(array('m' => 1, 'cm' => 100)); return $num; } diff --git a/question/type/numerical/simpletest/testanswerprocessor.php b/question/type/numerical/simpletest/testanswerprocessor.php index 7752e45ee64..19d3d44f5c1 100644 --- a/question/type/numerical/simpletest/testanswerprocessor.php +++ b/question/type/numerical/simpletest/testanswerprocessor.php @@ -36,7 +36,7 @@ class testable_qtype_numerical_answer_processor extends qtype_numerical_answer_p class qtype_numerical_answer_processor_test extends UnitTestCase { public function test_parse_response() { $ap = new testable_qtype_numerical_answer_processor( - array('m' => 1, 'cm' => 0.01), false, '.', ','); + array('m' => 1, 'cm' => 100), false, '.', ','); $this->assertEqual(array('3', '142', '', ''), $ap->parse_response('3.142')); $this->assertEqual(array('', '2', '', ''), $ap->parse_response('.2')); @@ -83,30 +83,41 @@ class qtype_numerical_answer_processor_test extends UnitTestCase { $this->assertEqual(array(null, null, null, null), $ap->parse_response(',')); } + protected function verify_value_and_unit($exectedval, $expectedunit, + qtype_numerical_answer_processor $ap, $input, $separateunit = null) { + list($val, $unit) = $ap->apply_units($input, $separateunit); + if (is_null($exectedval)) { + $this->assertNull($val); + } else { + $this->assertWithinMargin($exectedval, $val, 0.0001); + } + $this->assertEqual($expectedunit, $unit); + } + public function test_apply_units() { $ap = new qtype_numerical_answer_processor( - array('m/s' => 1, 'c' => 299792458, 'mph' => 0.44704), false, '.', ','); + array('m/s' => 1, 'c' => 3.3356409519815E-9, 'mph' => 2.2369362920544), false, '.', ','); - $this->assertEqual(array(3e8, 'm/s'), $ap->apply_units('3x10^8 m/s')); - $this->assertEqual(array(3e8, ''), $ap->apply_units('3x10^8')); - $this->assertEqual(array(299792458, 'c'), $ap->apply_units('1c')); - $this->assertEqual(array(0.44704, 'mph'), $ap->apply_units('0001.000 mph')); + $this->verify_value_and_unit(3e8, 'm/s', $ap, '3x10^8 m/s'); + $this->verify_value_and_unit(3e8, '', $ap, '3x10^8'); + $this->verify_value_and_unit(299792458, 'c', $ap, '1c'); + $this->verify_value_and_unit(0.44704, 'mph', $ap, '0001.000 mph'); - $this->assertEqual(array(1, 'frogs'), $ap->apply_units('1 frogs')); - $this->assertEqual(array(null, null), $ap->apply_units('. m/s')); + $this->verify_value_and_unit(1, 'frogs', $ap, '1 frogs'); + $this->verify_value_and_unit(null, null, $ap, '. m/s'); } public function test_apply_units_separate_unit() { $ap = new qtype_numerical_answer_processor( - array('m/s' => 1, 'c' => 299792458, 'mph' => 0.44704), false, '.', ','); + array('m/s' => 1, 'c' => 3.3356409519815E-9, 'mph' => 2.2369362920544), false, '.', ','); - $this->assertEqual(array(3e8, 'm/s'), $ap->apply_units('3x10^8', 'm/s')); - $this->assertEqual(array(3e8, ''), $ap->apply_units('3x10^8', '')); - $this->assertEqual(array(299792458, 'c'), $ap->apply_units('1', 'c')); - $this->assertEqual(array(0.44704, 'mph'), $ap->apply_units('0001.000', 'mph')); + $this->verify_value_and_unit(3e8, 'm/s', $ap, '3x10^8', 'm/s'); + $this->verify_value_and_unit(3e8, '', $ap, '3x10^8', ''); + $this->verify_value_and_unit(299792458, 'c', $ap, '1', 'c'); + $this->verify_value_and_unit(0.44704, 'mph', $ap, '0001.000', 'mph'); - $this->assertEqual(array(1, 'frogs'), $ap->apply_units('1', 'frogs')); - $this->assertEqual(array(null, null), $ap->apply_units('.', 'm/s')); + $this->verify_value_and_unit(1, 'frogs', $ap, '1', 'frogs'); + $this->verify_value_and_unit(null, null, $ap, '.', 'm/s'); } public function test_euro_style() { @@ -117,7 +128,7 @@ class qtype_numerical_answer_processor_test extends UnitTestCase { } public function test_percent() { - $ap = new qtype_numerical_answer_processor(array('%' => 0.01), false, '.', ','); + $ap = new qtype_numerical_answer_processor(array('%' => 100), false, '.', ','); $this->assertEqual(array('0.03', '%'), $ap->apply_units('3%')); $this->assertEqual(array('1e-8', '%'), $ap->apply_units('1e-6 %')); diff --git a/question/type/numerical/simpletest/testquestion.php b/question/type/numerical/simpletest/testquestion.php index 70d0b2b7752..8d8aef30ebd 100644 --- a/question/type/numerical/simpletest/testquestion.php +++ b/question/type/numerical/simpletest/testquestion.php @@ -63,7 +63,7 @@ class qtype_numerical_question_test extends UnitTestCase { public function test_grading_with_units() { $question = test_question_maker::make_question('numerical'); $question->ap = new qtype_numerical_answer_processor( - array('m' => 1, 'cm' => 0.01), false, '.', ','); + array('m' => 1, 'cm' => 100), false, '.', ','); $this->assertEqual(array(1, question_state::$gradedright), $question->grade_response(array('answer' => '3.14 frogs'))); diff --git a/question/type/random/db/simpletest/testupgradelibnewqe.php b/question/type/random/db/simpletest/testupgradelibnewqe.php index d46dc64e773..d580b7f6e26 100644 --- a/question/type/random/db/simpletest/testupgradelibnewqe.php +++ b/question/type/random/db/simpletest/testupgradelibnewqe.php @@ -749,7 +749,7 @@ class qtype_random_attempt_upgrader_test extends question_attempt_upgrader_test_ 'minfraction' => 0, 'flagged' => 0, 'questionsummary' => "The figure below shows a graph for converting between miles and kilometres. \n\n[ The graph has a horizontal axis, labelled miles, with a scale marked from 0 to 50 in steps of 5, and a vertical axis, labelled kilometres, with a scale from 0 to 80 in steps of 10. The straight line of the graph starts at the origin and slopes up to the right, and appears to pass through the point corresponding to 25 miles and 40 kilometres, and through the point corresponding to 50 miles and 80 kilometres. ] \n\nBy taking a reading from the graph, estimate how many kilometres are equivalent to 40 miles. \n\nRound your answer to the nearest kilometre and type it in the box below. \n\n(Your answer should be a single number, without units.)", - 'rightanswer' => '64', + 'rightanswer' => '64 km', 'responsesummary' => '64', 'timemodified' => 1268471437, 'steps' => array( @@ -782,4 +782,200 @@ class qtype_random_attempt_upgrader_test extends question_attempt_upgrader_test_ $this->assertEqual($expectedqa, $qa); } + + public function test_numerical_deferredfeedback_qsession55() { + $quiz = (object) array( + 'id' => '2', + 'course' => '2', + 'name' => 'Numerical quiz', + 'intro' => '', + 'introformat' => '1', + 'timeopen' => '0', + 'timeclose' => '0', + 'attempts' => '0', + 'attemptonlast' => '0', + 'grademethod' => '1', + 'decimalpoints' => '2', + 'questiondecimalpoints' => '-1', + 'review' => '4459503', + 'questionsperpage' => '1', + 'shufflequestions' => '0', + 'shuffleanswers' => '1', + 'questions' => '6,12,13,15,14,0', + 'sumgrades' => '5.00000', + 'grade' => '10.00000', + 'timecreated' => '0', + 'timemodified' => '1305273177', + 'timelimit' => '0', + 'password' => '', + 'subnet' => '', + 'popup' => '0', + 'delay1' => '0', + 'delay2' => '0', + 'showuserpicture' => '0', + 'showblocks' => '0', + 'preferredbehaviour' => 'deferredfeedback', + ); + $attempt = (object) array( + 'id' => '7', + 'uniqueid' => '7', + 'quiz' => '2', + 'userid' => '3', + 'attempt' => '1', + 'sumgrades' => '5.00000', + 'timestart' => '1305273645', + 'timefinish' => '1305273672', + 'timemodified' => '1305273672', + 'layout' => '6,12,13,15,14,0', + 'preview' => '0', + 'needsupgradetonewqe' => 1, + ); + $question = (object) array( + 'id' => '14', + 'category' => '2', + 'parent' => '0', + 'name' => 'MC units', + 'questiontext' => '

What is twice 1.5 m?

', + 'questiontextformat' => '1', + 'generalfeedback' => '', + 'generalfeedbackformat' => '1', + 'penalty' => '0.1000000', + 'qtype' => 'numerical', + 'length' => '1', + 'stamp' => 'tjh238.vledev2.open.ac.uk+110513075703+anXKfw', + 'version' => 'tjh238.vledev2.open.ac.uk+110513075807+HiXe4P', + 'hidden' => '0', + 'timecreated' => '1305273423', + 'timemodified' => '1305273487', + 'createdby' => '2', + 'modifiedby' => '2', + 'maxmark' => '1.0000000', + 'options' => (object) array( + 'answers' => array( + 21 => (object) array( + 'id' => '21', + 'question' => '14', + 'answer' => '3', + 'answerformat' => '0', + 'fraction' => '1.0000000', + 'feedback' => '', + 'feedbackformat' => '1', + 'tolerance' => '', + ), + ), + 'units' => array( + 0 => (object) array( + 'id' => '5', + 'question' => '14', + 'multiplier' => 1, + 'unit' => 'm', + ), + 1 => (object) array( + 'id' => '6', + 'question' => '14', + 'multiplier' => 100, + 'unit' => 'cm', + ), + ), + 'unitgradingtype' => '2', + 'unitpenalty' => '0.5000000', + 'showunits' => '1', + 'unitsleft' => '0', + 'instructions' => '

Write an answer like 3 m.

', + 'instructionsformat' => '1', + ), + 'defaultmark' => '1.0000000', + ); + $qsession = (object) array( + 'id' => '55', + 'attemptid' => '7', + 'questionid' => '14', + 'newest' => '155', + 'newgraded' => '155', + 'sumpenalty' => '0.1000000', + 'manualcomment' => '', + 'manualcommentformat' => '1', + 'flagged' => '0', + ); + $qstates = array( + 145 => (object) array( + 'id' => '145', + 'attempt' => '7', + 'question' => '14', + 'seq_number' => '0', + 'answer' => '|||||', + 'timestamp' => '1305273645', + 'event' => '0', + 'grade' => '0.0000000', + 'raw_grade' => '0.0000000', + 'penalty' => '0.0000000', + ), + 150 => (object) array( + 'id' => '150', + 'attempt' => '7', + 'question' => '14', + 'seq_number' => '1', + 'answer' => '300|||||cm', + 'timestamp' => '1305273666', + 'event' => '2', + 'grade' => '0.0000000', + 'raw_grade' => '1.0000000', + 'penalty' => '0.1000000', + ), + 155 => (object) array( + 'id' => '155', + 'attempt' => '7', + 'question' => '14', + 'seq_number' => '2', + 'answer' => '300|||||cm', + 'timestamp' => '1305273666', + 'event' => '6', + 'grade' => '1.0000000', + 'raw_grade' => '1.0000000', + 'penalty' => '0.1000000', + ), + ); + + $qa = $this->updater->convert_question_attempt($quiz, $attempt, $question, $qsession, $qstates); + + $expectedqa = (object) array( + 'behaviour' => 'deferredfeedback', + 'questionid' => 14, + 'maxmark' => 1.0000000, + 'minfraction' => 0, + 'flagged' => 0, + 'questionsummary' => 'What is twice 1.5 m?', + 'rightanswer' => '3 m', + 'responsesummary' => '300 cm', + 'timemodified' => 1305273666, + 'steps' => array( + 0 => (object) array( + 'sequencenumber' => 0, + 'state' => 'todo', + 'fraction' => null, + 'timecreated' => 1305273645, + 'userid' => 3, + 'data' => array('_separators' => '.$,'), + ), + 1 => (object) array( + 'sequencenumber' => 1, + 'state' => 'complete', + 'fraction' => null, + 'timecreated' => 1305273666, + 'userid' => 3, + 'data' => array('answer' => '300', 'unit' => 'cm'), + ), + 2 => (object) array( + 'sequencenumber' => 2, + 'state' => 'gradedright', + 'fraction' => 1, + 'timecreated' => 1305273666, + 'userid' => 3, + 'data' => array('answer' => '300', 'unit' => 'cm', '-finish' => 1), + ), + ), + ); + + $this->assertEqual($expectedqa, $qa); + } }