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.

diff --git a/lang/en/question.php b/lang/en/question.php
index 5379991..50f6dc3 100644 (file)
--- a/lang/en/question.php
+++ b/lang/en/question.php
@@ -367,6 +367,7 @@ $string['notflagged'] = 'Not flagged';
 $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 a92edcc..a538452 100644 (file)
--- a/question/behaviour/behaviourbase.php
+++ b/question/behaviour/behaviourbase.php
@@ -330,8 +330,8 @@ abstract class question_behaviour {
      * 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.
@@ -339,6 +339,23 @@ abstract class question_behaviour {
      */
     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);
     }
 
     /**
@@ -542,6 +559,13 @@ abstract class question_behaviour_with_save extends question_behaviour {
         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 36cc73c..e5fe92f 100644 (file)
--- a/question/behaviour/deferredcbm/tests/walkthrough_test.php
+++ b/question/behaviour/deferredcbm/tests/walkthrough_test.php
@@ -47,6 +47,7 @@ class qbehaviour_deferredcbm_walkthrough_test extends qbehaviour_walkthrough_tes
 
         // 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 @@ class qbehaviour_deferredcbm_walkthrough_test extends qbehaviour_walkthrough_tes
 
         // 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 @@ class qbehaviour_deferredcbm_walkthrough_test extends qbehaviour_walkthrough_tes
 
         // 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 @@ class qbehaviour_deferredcbm_walkthrough_test extends qbehaviour_walkthrough_tes
 
         // 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 @@ class qbehaviour_deferredcbm_walkthrough_test extends qbehaviour_walkthrough_tes
 
         // 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 @@ class qbehaviour_deferredcbm_walkthrough_test extends qbehaviour_walkthrough_tes
         $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 @@ class qbehaviour_deferredcbm_walkthrough_test extends qbehaviour_walkthrough_tes
 
         // 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 1bc8067..22bae00 100644 (file)
--- a/question/behaviour/deferredfeedback/tests/walkthrough_test.php
+++ b/question/behaviour/deferredfeedback/tests/walkthrough_test.php
@@ -47,6 +47,7 @@ class qbehaviour_deferredfeedback_walkthrough_test extends qbehaviour_walkthroug
 
         // 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 @@ class qbehaviour_deferredfeedback_walkthrough_test extends qbehaviour_walkthroug
         $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 @@ class qbehaviour_deferredfeedback_walkthrough_test extends qbehaviour_walkthroug
         $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 @@ class qbehaviour_deferredfeedback_walkthrough_test extends qbehaviour_walkthroug
 
         // 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 @@ class qbehaviour_deferredfeedback_walkthrough_test extends qbehaviour_walkthroug
         $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 @@ class qbehaviour_deferredfeedback_walkthrough_test extends qbehaviour_walkthroug
         $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 8def433..9bbf55a 100644 (file)
--- a/question/engine/questionattempt.php
+++ b/question/engine/questionattempt.php
@@ -576,6 +576,10 @@ class question_attempt {
      * @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 @@ class question_attempt {
 
         // 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 ece0ed7..3113cb3 100644 (file)
--- a/question/engine/tests/helpers.php
+++ b/question/engine/tests/helpers.php
@@ -824,6 +824,13 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase {
                 '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,