From 6c1620163248841f677e771a568d99cc46626d13 Mon Sep 17 00:00:00 2001 From: Ruslan Kabalin Date: Mon, 29 Jul 2013 11:05:15 +0100 Subject: [PATCH] MDL-31226: quiz: Fix message for attempts built on previous. 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. --- lang/en/question.php | 1 + question/behaviour/behaviourbase.php | 28 +++++- .../deferredcbm/tests/walkthrough_test.php | 9 +- .../tests/walkthrough_test.php | 95 +++++++++++++++++++ question/engine/questionattempt.php | 8 +- question/engine/tests/helpers.php | 7 ++ 6 files changed, 143 insertions(+), 5 deletions(-) diff --git a/lang/en/question.php b/lang/en/question.php index 53799911dfd65..50f6dc3d91101 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -367,6 +367,7 @@ $string['notgraded'] = 'Not graded'; $string['notshown'] = 'Not shown'; $string['notyetanswered'] = 'Not yet answered'; +$string['notchanged'] = 'Not changed since last attempt'; $string['notyourpreview'] = 'This preview does not belong to you'; $string['options'] = 'Options'; $string['parent'] = 'Parent'; diff --git a/question/behaviour/behaviourbase.php b/question/behaviour/behaviourbase.php index 4219cba241618..4f3e82c27aacd 100644 --- a/question/behaviour/behaviourbase.php +++ b/question/behaviour/behaviourbase.php @@ -347,8 +347,8 @@ public abstract function summarise_action(question_attempt_step $step); * Initialise the first step in a question attempt when a new * {@link question_attempt} is being started. * - * This method must call $this->question->start_attempt($step), and may - * perform additional processing if the model requries it. + * This method must call $this->question->start_attempt($step, $variant), and may + * perform additional processing if the behaviour requries it. * * @param question_attempt_step $step the first step of the * question_attempt being started. @@ -356,6 +356,23 @@ public abstract function summarise_action(question_attempt_step $step); */ public function init_first_step(question_attempt_step $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); } /** @@ -559,6 +576,13 @@ public function required_question_definition_type() { 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 * from the last set of responses we have stored. diff --git a/question/behaviour/deferredcbm/tests/walkthrough_test.php b/question/behaviour/deferredcbm/tests/walkthrough_test.php index 36cc73c965b54..e5fe92f1d619c 100644 --- a/question/behaviour/deferredcbm/tests/walkthrough_test.php +++ b/question/behaviour/deferredcbm/tests/walkthrough_test.php @@ -47,6 +47,7 @@ public function test_deferred_cbm_truefalse_high_certainty() { // 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_question_text_expectation($tf), @@ -62,6 +63,7 @@ public function test_deferred_cbm_truefalse_high_certainty() { // Verify. $this->check_current_state(question_state::$complete); + $this->check_output_contains_lang_string('answersaved', 'question'); $this->check_current_mark(null); $this->check_current_output( $this->get_contains_tf_true_radio_expectation(true, true), @@ -122,6 +124,7 @@ public function test_deferred_cbm_truefalse_low_certainty() { // 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_does_not_contain_correctness_expectation(), @@ -133,6 +136,7 @@ public function test_deferred_cbm_truefalse_low_certainty() { // Verify. $this->check_current_state(question_state::$complete); + $this->check_output_contains_lang_string('answersaved', 'question'); $this->check_current_mark(null); $this->check_current_output($this->get_does_not_contain_correctness_expectation(), $this->get_contains_cbm_radio_expectation(1, true, true), @@ -159,6 +163,7 @@ public function test_deferred_cbm_truefalse_default_certainty() { // 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_does_not_contain_correctness_expectation(), @@ -219,7 +224,8 @@ public function test_deferredcbm_resume_multichoice_single() { $this->quba->start_question_based_on($this->slot, $oldqa); // 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_output( $this->get_contains_mc_radio_expectation($wrongindex, true, true), @@ -255,6 +261,7 @@ public function test_deferred_cbm_truefalse_no_certainty_feedback_when_not_answe // 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_does_not_contain_correctness_expectation(), diff --git a/question/behaviour/deferredfeedback/tests/walkthrough_test.php b/question/behaviour/deferredfeedback/tests/walkthrough_test.php index 1bc806709fdd9..22bae00fcc466 100644 --- a/question/behaviour/deferredfeedback/tests/walkthrough_test.php +++ b/question/behaviour/deferredfeedback/tests/walkthrough_test.php @@ -47,6 +47,7 @@ public function test_deferredfeedback_feedback_truefalse() { // Check the initial state. $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_question_text_expectation($tf), $this->get_does_not_contain_feedback_expectation()); @@ -60,6 +61,7 @@ public function test_deferredfeedback_feedback_truefalse() { $this->process_submission(array('answer' => 1)); $this->check_current_state(question_state::$complete); + $this->check_output_contains_lang_string('answersaved', 'question'); $this->check_current_mark(null); $this->check_current_output($this->get_contains_tf_true_radio_expectation(true, true), $this->get_does_not_contain_correctness_expectation(), @@ -121,6 +123,7 @@ public function test_deferredfeedback_feedback_multichoice_single() { $rightindex = $this->get_mc_right_answer_index($mc); $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_question_text_expectation($mc), @@ -134,6 +137,7 @@ public function test_deferredfeedback_feedback_multichoice_single() { // Verify. $this->check_current_state(question_state::$complete); + $this->check_output_contains_lang_string('answersaved', 'question'); $this->check_current_mark(null); $this->check_current_output( $this->get_contains_mc_radio_expectation($rightindex, true, true), @@ -173,6 +177,10 @@ public function test_deferredfeedback_resume_multichoice_single() { $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_output_contains_lang_string('notyetanswered', 'question'); + $this->check_current_mark(null); $this->process_submission(array('answer' => $wrongindex)); $this->quba->finish_all_questions(); @@ -193,7 +201,94 @@ public function test_deferredfeedback_resume_multichoice_single() { $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_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_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_output( $this->get_contains_mc_radio_expectation($wrongindex, true, true), diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php index 8def433118218..9bbf55ae0adad 100644 --- a/question/engine/questionattempt.php +++ b/question/engine/questionattempt.php @@ -576,6 +576,10 @@ public function get_state() { * @return string A brief textual description of the current state. */ 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); } @@ -886,9 +890,9 @@ public function start($preferredbehaviour, $variant, $submitteddata = array(), // Initialise the first step. $firststep = new question_attempt_step($submitteddata, $timestamp, $userid, $existingstepid); - $firststep->set_state(question_state::$todo); if ($submitteddata) { - $this->question->apply_attempt_state($firststep); + $firststep->set_state(question_state::$complete); + $this->behaviour->apply_attempt_state($firststep); } else { $this->behaviour->init_first_step($firststep, $variant); } diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php index ece0ed7da0df4..3113cb34afa63 100644 --- a/question/engine/tests/helpers.php +++ b/question/engine/tests/helpers.php @@ -824,6 +824,13 @@ protected function check_output_contains_hidden_input($name, $value) { 'Looking for a hidden input with attributes ' . html_writer::attributes($attributes) . ' in ' . $this->currentoutput); } + 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); + } + protected function get_tag_matcher($tag, $attributes) { return array( 'tag' => $tag,