Browse files

Merge branch 'MDL-35370_23' of git://github.com/timhunt/moodle into M…

…OODLE_23_STABLE
  • Loading branch information...
2 parents d0154d2 + ad1a52e commit d9cada0a9d8279d65b40dc94ad20eca0307f2b6f @danpoltawski danpoltawski committed Oct 2, 2012
View
30 question/type/multianswer/renderer.php
@@ -177,13 +177,18 @@ public function subquestion(question_attempt $qa, question_display_options $opti
if ($subq->qtype->name() == 'shortanswer') {
$matchinganswer = $subq->get_matching_answer(array('answer' => $response));
} else if ($subq->qtype->name() == 'numerical') {
- $matchinganswer = $subq->get_matching_answer($response, 1);
+ list($value, $unit, $multiplier) = $subq->ap->apply_units($response, '');
+ $matchinganswer = $subq->get_matching_answer($value, 1);
} else {
$matchinganswer = $subq->get_matching_answer($response);
}
if (!$matchinganswer) {
- $matchinganswer = new question_answer(0, '', null, '', FORMAT_HTML);
+ if (is_null($response) || $response === '') {
+ $matchinganswer = new question_answer(0, '', null, '', FORMAT_HTML);
+ } else {
+ $matchinganswer = new question_answer(0, '', 0.0, '', FORMAT_HTML);
+ }
}
// Work out a good input field size.
@@ -281,6 +286,9 @@ public function subquestion(question_attempt $qa, question_display_options $opti
$order = $subq->get_order($qa);
$correctresponses = $subq->get_correct_response();
$rightanswer = $subq->answers[$order[reset($correctresponses)]];
+ if (!$matchinganswer) {
+ $matchinganswer = new question_answer(0, '', null, '', FORMAT_HTML);
+ }
$feedbackpopup = $this->feedback_popup($subq, $matchinganswer->fraction,
$subq->format_text($matchinganswer->feedback, $matchinganswer->feedbackformat,
$qa, 'question', 'answerfeedback', $matchinganswer->id),
@@ -366,16 +374,30 @@ public function subquestion(question_attempt $qa, question_display_options $opti
$result .= $this->all_choices_wrapper_end();
+ $feedback = array();
if ($options->feedback && $options->marks >= question_display_options::MARK_AND_MAX &&
$subq->maxmark > 0) {
$a = new stdClass();
$a->mark = format_float($fraction * $subq->maxmark, $options->markdp);
$a->max = format_float($subq->maxmark, $options->markdp);
- $result .= html_writer::tag('div', get_string('markoutofmax', 'question', $a),
- array('class' => 'outcome'));
+ $feedback[] = html_writer::tag('div', get_string('markoutofmax', 'question', $a));
+ }
+
+ if ($options->rightanswer) {
+ foreach ($subq->answers as $ans) {
+ if (question_state::graded_state_for_fraction($ans->fraction) ==
+ question_state::$gradedright) {
+ $feedback[] = get_string('correctansweris', 'qtype_multichoice',
+ $subq->format_text($ans->answer, $ans->answerformat,
+ $qa, 'question', 'answer', $ansid));
+ break;
+ }
+ }
}
+ $result .= html_writer::nonempty_tag('div', implode('<br />', $feedback), array('class' => 'outcome'));
+
return $result;
}
View
46 question/type/multianswer/tests/helper.php
@@ -38,7 +38,7 @@
*/
class qtype_multianswer_test_helper extends question_test_helper {
public function get_test_questions() {
- return array('twosubq', 'fourmc');
+ return array('twosubq', 'fourmc', 'numericalzero');
}
/**
@@ -287,4 +287,48 @@ public function make_multianswer_question_fourmc() {
return $q;
}
+
+ /**
+ * Makes a multianswer question with one numerical subquestion, right answer 0.
+ * This is used for testing the MDL-35370 bug.
+ * @return qtype_multianswer_question
+ */
+ public function make_multianswer_question_numericalzero() {
+ question_bank::load_question_definition_classes('multianswer');
+ $q = new qtype_multianswer_question();
+ test_question_maker::initialise_a_question($q);
+ $q->name = 'Numerical zero';
+ $q->questiontext =
+ 'Enter zero: {#1}.';
+ $q->generalfeedback = '';
+ $q->qtype = question_bank::get_qtype('multianswer');
+
+ $q->textfragments = array(
+ 'Enter zero: ',
+ '.',
+ );
+ $q->places = array('1' => '1');
+
+ // Numerical subquestion.
+ question_bank::load_question_definition_classes('numerical');
+ $sub = new qtype_numerical_question();
+ test_question_maker::initialise_a_question($sub);
+ $sub->name = 'Numerical zero';
+ $sub->questiontext = '{1:NUMERICAL:=0:0}';
+ $sub->questiontextformat = FORMAT_HTML;
+ $sub->generalfeedback = '';
+ $sub->generalfeedbackformat = FORMAT_HTML;
+ $sub->answers = array(
+ 13 => new qtype_numerical_answer(13, '0', 1.0, '', FORMAT_HTML, 0),
+ );
+ $sub->qtype = question_bank::get_qtype('numerical');
+ $sub->ap = new qtype_numerical_answer_processor(array());
+ $sub->maxmark = 1;
+
+ $q->subquestions = array(
+ 1 => $sub,
+ );
+
+ return $q;
+ }
}
View
114 question/type/multianswer/tests/walkthrough_test.php
@@ -37,9 +37,15 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_multianswer_walkthrough_test extends qbehaviour_walkthrough_test_base {
+
+ protected function get_contains_subq_status(question_state $state) {
+ return new question_pattern_expectation('~' .
+ preg_quote($state->default_string(true), '~') . '<br />~');
+ }
+
public function test_deferred_feedback() {
- // Create a gapselect question.
+ // Create a multianswer question.
$q = test_question_maker::make_question('multianswer', 'fourmc');
$this->start_attempt_at_question($q, 'deferredfeedback', 4);
@@ -86,4 +92,110 @@ public function test_deferred_feedback() {
$this->get_contains_partcorrect_expectation(),
$this->get_does_not_contain_validation_error_expectation());
}
+
+ public function test_deferred_feedback_numericalzero_not_answered() {
+ // Tests the situation found in MDL-35370.
+
+ // Create a multianswer question with one numerical subquestion, right answer zero.
+ $q = test_question_maker::make_question('multianswer', 'numericalzero');
+ $this->start_attempt_at_question($q, 'deferredfeedback', 1);
+
+ // 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());
+
+ // Now submit all and finish.
+ $this->process_submission(array('-finish' => 1));
+
+ // Verify.
+ $this->check_current_state(question_state::$gaveup);
+ $this->check_current_mark(null);
+ $this->check_current_output(
+ $this->get_contains_marked_out_of_summary(),
+ new question_pattern_expectation('~<input[^>]* class="incorrect" [^>]*/>~'),
+ $this->get_contains_subq_status(question_state::$gaveup),
+ $this->get_does_not_contain_validation_error_expectation());
+ }
+
+ public function test_deferred_feedback_numericalzero_0_answer() {
+ // Tests the situation found in MDL-35370.
+
+ // Create a multianswer question with one numerical subquestion, right answer zero.
+ $q = test_question_maker::make_question('multianswer', 'numericalzero');
+ $this->start_attempt_at_question($q, 'deferredfeedback', 1);
+
+ // 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());
+
+ // Save a the correct answer.
+ $this->process_submission(array('sub1_answer' => '0'));
+
+ // Verify.
+ $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());
+
+ // Now submit all and finish.
+ $this->process_submission(array('-finish' => 1));
+
+ // Verify.
+ $this->check_current_state(question_state::$gradedright);
+ $this->check_current_mark(1);
+ $this->check_current_output(
+ $this->get_contains_mark_summary(1),
+ $this->get_contains_correct_expectation(),
+ $this->get_contains_subq_status(question_state::$gradedright),
+ $this->get_does_not_contain_validation_error_expectation());
+ }
+
+ public function test_deferred_feedback_numericalzero_0_wrong() {
+ // Tests the situation found in MDL-35370.
+
+ // Create a multianswer question with one numerical subquestion, right answer zero.
+ $q = test_question_maker::make_question('multianswer', 'numericalzero');
+ $this->start_attempt_at_question($q, 'deferredfeedback', 1);
+
+ // 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());
+
+ // Save a the correct answer.
+ $this->process_submission(array('sub1_answer' => '42'));
+
+ // Verify.
+ $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());
+
+ // Now submit all and finish.
+ $this->process_submission(array('-finish' => 1));
+
+ // Verify.
+ $this->check_current_state(question_state::$gradedwrong);
+ $this->check_current_mark(0);
+ $this->check_current_output(
+ $this->get_contains_mark_summary(0),
+ $this->get_contains_incorrect_expectation(),
+ $this->get_contains_subq_status(question_state::$gradedwrong),
+ $this->get_does_not_contain_validation_error_expectation());
+ }
}
View
12 question/type/numerical/question.php
@@ -179,6 +179,10 @@ public function get_correct_response() {
* @return question_answer the matching answer.
*/
public function get_matching_answer($value, $multiplier) {
+ if (is_null($value) || $value === '') {
+ return null;
+ }
+
if (!is_null($multiplier)) {
$scaledvalue = $value * $multiplier;
} else {
@@ -193,6 +197,7 @@ public function get_matching_answer($value, $multiplier) {
return $answer;
}
}
+
return null;
}
@@ -273,18 +278,17 @@ public function classify_response(array $response) {
public function check_file_access($qa, $options, $component, $filearea, $args,
$forcedownload) {
if ($component == 'question' && $filearea == 'answerfeedback') {
- $question = $qa->get_question();
$currentanswer = $qa->get_last_qt_var('answer');
if ($this->has_separate_unit_field()) {
$selectedunit = $qa->get_last_qt_var('unit');
} else {
$selectedunit = null;
}
- list($value, $unit, $multiplier) = $question->ap->apply_units(
+ list($value, $unit, $multiplier) = $this->ap->apply_units(
$currentanswer, $selectedunit);
- $answer = $question->get_matching_answer($value, $multiplier);
+ $answer = $this->get_matching_answer($value, $multiplier);
$answerid = reset($args); // itemid is answer id.
- return $options->feedback && $answerid == $answer->id;
+ return $options->feedback && $answer && $answerid == $answer->id;
} else if ($component == 'question' && $filearea == 'hint') {
return $this->check_hint_file_access($qa, $options, $args);
View
5 question/type/numerical/tests/question_test.php
@@ -181,6 +181,11 @@ public function test_summarise_response() {
$this->assertEquals('3.1', $num->summarise_response(array('answer' => '3.1')));
}
+ public function test_summarise_response_zero() {
+ $num = test_question_maker::make_question('numerical');
+ $this->assertEquals('0', $num->summarise_response(array('answer' => '0')));
+ }
+
public function test_summarise_response_unit() {
$num = test_question_maker::make_question('numerical', 'unit');
$this->assertEquals('3.1', $num->summarise_response(array('answer' => '3.1')));
View
6 question/type/shortanswer/question.php
@@ -78,6 +78,10 @@ public function get_answers() {
}
public function compare_response_with_answer(array $response, question_answer $answer) {
+ if (!array_key_exists('answer', $response) || is_null($response['answer'])) {
+ return false;
+ }
+
return self::compare_string_with_wildcard(
$response['answer'], $answer->answer, !$this->usecase);
}
@@ -129,7 +133,7 @@ public function check_file_access($qa, $options, $component, $filearea,
$currentanswer = $qa->get_last_qt_var('answer');
$answer = $qa->get_question()->get_matching_answer(array('answer' => $currentanswer));
$answerid = reset($args); // itemid is answer id.
- return $options->feedback && $answerid == $answer->id;
+ return $options->feedback && $answer && $answerid == $answer->id;
} else if ($component == 'question' && $filearea == 'hint') {
return $this->check_hint_file_access($qa, $options, $args);
View
2 question/type/shortanswer/questiontype.php
@@ -117,7 +117,7 @@ public function save_question_options($question) {
$this->save_hints($question);
- // Perform sanity checks on fractional grades
+ // Perform sanity checks on fractional grades.
if ($maxfraction != 1) {
$result->noticeyesno = get_string('fractionsnomax', 'question', $maxfraction * 100);
return $result;

0 comments on commit d9cada0

Please sign in to comment.