Skip to content

Commit

Permalink
Merge branch 'MDL-35370' of git://github.com/timhunt/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
@danpoltawski
danpoltawski committed Oct 2, 2012
2 parents c07189e + b2a79cc commit 76811d0
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 12 deletions.
30 changes: 26 additions & 4 deletions question/type/multianswer/renderer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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;
}

Expand Down
46 changes: 45 additions & 1 deletion question/type/multianswer/tests/helper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

/**
Expand Down Expand Up @@ -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;
}
}
114 changes: 113 additions & 1 deletion question/type/multianswer/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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());
}
}
12 changes: 8 additions & 4 deletions question/type/numerical/question.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -193,6 +197,7 @@ public function get_matching_answer($value, $multiplier) {
return $answer;
}
}

return null;
}

Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions question/type/numerical/tests/question_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')));
Expand Down
6 changes: 5 additions & 1 deletion question/type/shortanswer/question.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion question/type/shortanswer/questiontype.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 76811d0

Please sign in to comment.