Skip to content

Commit

Permalink
MDL-31226: quiz: Fix message for attempts built on previous.
Browse files Browse the repository at this point in the history
At the moment, when attempt is built on the last one, "not yet answered"
message is shown, which confuses many people. This patch modifies the state to
"complete" for attempt based on previous and modifies the output string.

Many thanks to Tim Hunt for guiding me through quiz infrastructure and some code
suggestions.
  • Loading branch information
kabalin committed Aug 2, 2013
1 parent cd08b5c commit 05342e9
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 5 deletions.
1 change: 1 addition & 0 deletions lang/en/question.php
Expand Up @@ -363,6 +363,7 @@
$string['notgraded'] = 'Not graded'; $string['notgraded'] = 'Not graded';
$string['notshown'] = 'Not shown'; $string['notshown'] = 'Not shown';
$string['notyetanswered'] = 'Not yet answered'; $string['notyetanswered'] = 'Not yet answered';
$string['notchanged'] = 'Not changed since last attempt';
$string['notyourpreview'] = 'This preview does not belong to you'; $string['notyourpreview'] = 'This preview does not belong to you';
$string['options'] = 'Options'; $string['options'] = 'Options';
$string['parent'] = 'Parent'; $string['parent'] = 'Parent';
Expand Down
28 changes: 26 additions & 2 deletions question/behaviour/behaviourbase.php
Expand Up @@ -347,15 +347,32 @@ public abstract function summarise_action(question_attempt_step $step);
* Initialise the first step in a question attempt when a new * Initialise the first step in a question attempt when a new
* {@link question_attempt} is being started. * {@link question_attempt} is being started.
* *
* This method must call $this->question->start_attempt($step), and may * This method must call $this->question->start_attempt($step, $variant), and may
* perform additional processing if the model requries it. * perform additional processing if the behaviour requries it.
* *
* @param question_attempt_step $step the first step of the * @param question_attempt_step $step the first step of the
* question_attempt being started. * question_attempt being started.
* @param int $variant which variant of the question to use. * @param int $variant which variant of the question to use.
*/ */
public function init_first_step(question_attempt_step $step, $variant) { public function init_first_step(question_attempt_step $step, $variant) {
$this->question->start_attempt($step, $variant); $this->question->start_attempt($step, $variant);
$step->set_state(question_state::$todo);
}

/**
* When an attempt is started based on a previous attempt (see
* {@link question_attempt::start_based_on}) this method is called to setup
* the new attempt.
*
* This method must call $this->question->apply_attempt_state($step), and may
* perform additional processing if the behaviour requries it.
*
* @param question_attempt_step The first step of the {@link question_attempt}
* being loaded.
*/
public function apply_attempt_state(question_attempt_step $step) {
$this->question->apply_attempt_state($step);
$step->set_state(question_state::$todo);
} }


/** /**
Expand Down Expand Up @@ -546,6 +563,13 @@ public function required_question_definition_type() {
return 'question_manually_gradable'; return 'question_manually_gradable';
} }


public function apply_attempt_state(question_attempt_step $step) {
parent::apply_attempt_state($step);
if ($this->question->is_complete_response($step->get_qt_data())) {
$step->set_state(question_state::$complete);
}
}

/** /**
* Work out whether the response in $pendingstep are significantly different * Work out whether the response in $pendingstep are significantly different
* from the last set of responses we have stored. * from the last set of responses we have stored.
Expand Down
9 changes: 8 additions & 1 deletion question/behaviour/deferredcbm/tests/walkthrough_test.php
Expand Up @@ -47,6 +47,7 @@ public function test_deferred_cbm_truefalse_high_certainty() {


// Verify. // Verify.
$this->check_current_state(question_state::$todo); $this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_contains_question_text_expectation($tf), $this->get_contains_question_text_expectation($tf),
Expand All @@ -62,6 +63,7 @@ public function test_deferred_cbm_truefalse_high_certainty() {


// Verify. // Verify.
$this->check_current_state(question_state::$complete); $this->check_current_state(question_state::$complete);
$this->check_output_contains_lang_string('answersaved', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_contains_tf_true_radio_expectation(true, true), $this->get_contains_tf_true_radio_expectation(true, true),
Expand Down Expand Up @@ -122,6 +124,7 @@ public function test_deferred_cbm_truefalse_low_certainty() {


// Verify. // Verify.
$this->check_current_state(question_state::$todo); $this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_does_not_contain_correctness_expectation(), $this->get_does_not_contain_correctness_expectation(),
Expand All @@ -133,6 +136,7 @@ public function test_deferred_cbm_truefalse_low_certainty() {


// Verify. // Verify.
$this->check_current_state(question_state::$complete); $this->check_current_state(question_state::$complete);
$this->check_output_contains_lang_string('answersaved', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output($this->get_does_not_contain_correctness_expectation(), $this->check_current_output($this->get_does_not_contain_correctness_expectation(),
$this->get_contains_cbm_radio_expectation(1, true, true), $this->get_contains_cbm_radio_expectation(1, true, true),
Expand All @@ -159,6 +163,7 @@ public function test_deferred_cbm_truefalse_default_certainty() {


// Verify. // Verify.
$this->check_current_state(question_state::$todo); $this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_does_not_contain_correctness_expectation(), $this->get_does_not_contain_correctness_expectation(),
Expand Down Expand Up @@ -219,7 +224,8 @@ public function test_deferredcbm_resume_multichoice_single() {
$this->quba->start_question_based_on($this->slot, $oldqa); $this->quba->start_question_based_on($this->slot, $oldqa);


// Verify. // Verify.
$this->check_current_state(question_state::$todo); $this->check_current_state(question_state::$complete);
$this->check_output_contains_lang_string('notchanged', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_contains_mc_radio_expectation($wrongindex, true, true), $this->get_contains_mc_radio_expectation($wrongindex, true, true),
Expand Down Expand Up @@ -255,6 +261,7 @@ public function test_deferred_cbm_truefalse_no_certainty_feedback_when_not_answe


// Verify. // Verify.
$this->check_current_state(question_state::$todo); $this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_does_not_contain_correctness_expectation(), $this->get_does_not_contain_correctness_expectation(),
Expand Down
95 changes: 95 additions & 0 deletions question/behaviour/deferredfeedback/tests/walkthrough_test.php
Expand Up @@ -47,6 +47,7 @@ public function test_deferredfeedback_feedback_truefalse() {


// Check the initial state. // Check the initial state.
$this->check_current_state(question_state::$todo); $this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output($this->get_contains_question_text_expectation($tf), $this->check_current_output($this->get_contains_question_text_expectation($tf),
$this->get_does_not_contain_feedback_expectation()); $this->get_does_not_contain_feedback_expectation());
Expand All @@ -60,6 +61,7 @@ public function test_deferredfeedback_feedback_truefalse() {
$this->process_submission(array('answer' => 1)); $this->process_submission(array('answer' => 1));


$this->check_current_state(question_state::$complete); $this->check_current_state(question_state::$complete);
$this->check_output_contains_lang_string('answersaved', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output($this->get_contains_tf_true_radio_expectation(true, true), $this->check_current_output($this->get_contains_tf_true_radio_expectation(true, true),
$this->get_does_not_contain_correctness_expectation(), $this->get_does_not_contain_correctness_expectation(),
Expand Down Expand Up @@ -121,6 +123,7 @@ public function test_deferredfeedback_feedback_multichoice_single() {
$rightindex = $this->get_mc_right_answer_index($mc); $rightindex = $this->get_mc_right_answer_index($mc);


$this->check_current_state(question_state::$todo); $this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_contains_question_text_expectation($mc), $this->get_contains_question_text_expectation($mc),
Expand All @@ -134,6 +137,7 @@ public function test_deferredfeedback_feedback_multichoice_single() {


// Verify. // Verify.
$this->check_current_state(question_state::$complete); $this->check_current_state(question_state::$complete);
$this->check_output_contains_lang_string('answersaved', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_contains_mc_radio_expectation($rightindex, true, true), $this->get_contains_mc_radio_expectation($rightindex, true, true),
Expand Down Expand Up @@ -173,6 +177,10 @@ public function test_deferredfeedback_resume_multichoice_single() {
$this->start_attempt_at_question($mc, 'deferredfeedback', 3); $this->start_attempt_at_question($mc, 'deferredfeedback', 3);
$rightindex = $this->get_mc_right_answer_index($mc); $rightindex = $this->get_mc_right_answer_index($mc);
$wrongindex = ($rightindex + 1) % 3; $wrongindex = ($rightindex + 1) % 3;

$this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null);
$this->process_submission(array('answer' => $wrongindex)); $this->process_submission(array('answer' => $wrongindex));
$this->quba->finish_all_questions(); $this->quba->finish_all_questions();


Expand All @@ -193,7 +201,94 @@ public function test_deferredfeedback_resume_multichoice_single() {
$this->quba->start_question_based_on($this->slot, $oldqa); $this->quba->start_question_based_on($this->slot, $oldqa);


// Verify. // Verify.
$this->check_current_state(question_state::$complete);
$this->check_output_contains_lang_string('notchanged', 'question');
$this->check_current_mark(null);
$this->check_current_output(
$this->get_contains_mc_radio_expectation($wrongindex, true, true),
$this->get_does_not_contain_feedback_expectation(),
$this->get_does_not_contain_correctness_expectation());

// Now get it right.
$this->process_submission(array('answer' => $rightindex));
$this->quba->finish_all_questions();

// Verify.
$this->check_current_state(question_state::$gradedright);
$this->check_current_mark(3);
$this->check_current_output(
$this->get_contains_mc_radio_expectation($rightindex, false, true),
$this->get_contains_correct_expectation());
}

public function test_deferredfeedback_resume_multichoice_single_emptyanswer_first() {

// Create a multiple-choice question.
$mc = test_question_maker::make_a_multichoice_single_question();

// Attempt it and submit empty.
$this->start_attempt_at_question($mc, 'deferredfeedback', 3);
$rightindex = $this->get_mc_right_answer_index($mc);
$wrongindex = ($rightindex + 1) % 3;

$this->check_current_state(question_state::$todo); $this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null);
$this->process_submission(array('-submit' => 1));
$this->quba->finish_all_questions();

// Verify.
$this->check_current_state(question_state::$gaveup);
$this->check_current_mark(null);
$this->check_current_output(
$this->get_contains_mc_radio_expectation(0, false, false),
$this->get_contains_mc_radio_expectation(1, false, false),
$this->get_contains_mc_radio_expectation(2, false, false),
$this->get_contains_general_feedback_expectation($mc));

// Save the old attempt.
$oldqa = $this->quba->get_question_attempt($this->slot);

// Reinitialise.
$this->setUp();
$this->quba->set_preferred_behaviour('deferredfeedback');
$this->slot = $this->quba->add_question($mc, 3);
$this->quba->start_question_based_on($this->slot, $oldqa);

// Verify.
$this->check_current_state(question_state::$todo);
$this->check_output_contains_lang_string('notyetanswered', 'question');
$this->check_current_mark(null);
$this->check_current_output(
$this->get_contains_mc_radio_expectation(0, true, false),
$this->get_contains_mc_radio_expectation(1, true, false),
$this->get_contains_mc_radio_expectation(2, true, false),
$this->get_does_not_contain_feedback_expectation(),
$this->get_does_not_contain_correctness_expectation());

// Now get it wrong.
$this->process_submission(array('answer' => $wrongindex));
$this->quba->finish_all_questions();

// Verify.
$this->check_current_state(question_state::$gradedwrong);
$this->check_current_mark(-1);
$this->check_current_output(
$this->get_contains_mc_radio_expectation($wrongindex, false, true),
$this->get_contains_incorrect_expectation());

// Save the old attempt.
$oldqa = $this->quba->get_question_attempt($this->slot);

// Reinitialise.
$this->setUp();
$this->quba->set_preferred_behaviour('deferredfeedback');
$this->slot = $this->quba->add_question($mc, 3);
$this->quba->start_question_based_on($this->slot, $oldqa);

// Verify.
$this->check_current_state(question_state::$complete);
$this->check_output_contains_lang_string('notchanged', 'question');
$this->check_current_mark(null); $this->check_current_mark(null);
$this->check_current_output( $this->check_current_output(
$this->get_contains_mc_radio_expectation($wrongindex, true, true), $this->get_contains_mc_radio_expectation($wrongindex, true, true),
Expand Down
8 changes: 6 additions & 2 deletions question/engine/questionattempt.php
Expand Up @@ -547,6 +547,10 @@ public function get_state() {
* @return string A brief textual description of the current state. * @return string A brief textual description of the current state.
*/ */
public function get_state_string($showcorrectness) { public function get_state_string($showcorrectness) {
// Special case when attempt is based on previous one, see MDL-31226.
if ($this->get_num_steps() == 1 && $this->get_state() == question_state::$complete) {
return get_string('notchanged', 'question');
}
return $this->behaviour->get_state_string($showcorrectness); return $this->behaviour->get_state_string($showcorrectness);
} }


Expand Down Expand Up @@ -832,9 +836,9 @@ public function start($preferredbehaviour, $variant, $submitteddata = array(),


// Initialise the first step. // Initialise the first step.
$firststep = new question_attempt_step($submitteddata, $timestamp, $userid, $existingstepid); $firststep = new question_attempt_step($submitteddata, $timestamp, $userid, $existingstepid);
$firststep->set_state(question_state::$todo);
if ($submitteddata) { if ($submitteddata) {
$this->question->apply_attempt_state($firststep); $firststep->set_state(question_state::$complete);
$this->behaviour->apply_attempt_state($firststep);
} else { } else {
$this->behaviour->init_first_step($firststep, $variant); $this->behaviour->init_first_step($firststep, $variant);
} }
Expand Down
7 changes: 7 additions & 0 deletions question/engine/tests/helpers.php
Expand Up @@ -684,6 +684,13 @@ protected function render() {
$this->currentoutput = $this->quba->render_question($this->slot, $this->displayoptions); $this->currentoutput = $this->quba->render_question($this->slot, $this->displayoptions);
} }


protected function check_output_contains_lang_string($identifier, $component = '', $a = null) {
$this->render();
$string = get_string($identifier, $component, $a);
$this->assertContains($string, $this->currentoutput,
'Expected string ' . $string . ' not found in ' . $this->currentoutput);
}

/** /**
* @param $condition one or more Expectations. (users varargs). * @param $condition one or more Expectations. (users varargs).
*/ */
Expand Down

0 comments on commit 05342e9

Please sign in to comment.