MDL-75727 question regrading: fix each attempt builds on last

This fixes a regression caused by MDL-74752. If you regraded
a subsequent quiz attempt in a quiz using the 'Each attempt
builds on last' option, then the student's response could get lost.

question/type/match/question.php

@@ -126,7 +126,7 @@ class qtype_match_question extends question_graded_automatically_with_countback
 
     public function update_attempt_state_data_for_new_version(
             question_attempt_step $oldstep, question_definition $otherversion) {
-        parent::update_attempt_state_data_for_new_version($oldstep, $otherversion);
+        $startdata = parent::update_attempt_state_data_for_new_version($oldstep, $otherversion);
 
         // Process stems.
         $mapping = array_combine(array_keys($otherversion->stems), array_keys($this->stems));
@@ -135,6 +135,7 @@ class qtype_match_question extends question_graded_automatically_with_countback
         foreach ($oldstemorder as $oldid) {
             $newstemorder[] = $mapping[$oldid] ?? $oldid;
         }
+        $startdata['_stemorder'] = implode(',', $newstemorder);
 
         // Process choices.
         $mapping = array_combine(array_keys($otherversion->choices), array_keys($this->choices));
@@ -143,8 +144,9 @@ class qtype_match_question extends question_graded_automatically_with_countback
         foreach ($oldchoiceorder as $oldid) {
             $newchoiceorder[] = $mapping[$oldid] ?? $oldid;
         }
+        $startdata['_choiceorder'] = implode(',', $newchoiceorder);
 
-        return ['_stemorder' => implode(',', $newstemorder), '_choiceorder' => implode(',', $newchoiceorder)];
+        return $startdata;
     }
 
     public function get_question_summary() {

question/type/multichoice/question.php

@@ -104,7 +104,7 @@ abstract class qtype_multichoice_base extends question_graded_automatically {
 
     public function update_attempt_state_data_for_new_version(
             question_attempt_step $oldstep, question_definition $otherversion) {
-        parent::update_attempt_state_data_for_new_version($oldstep, $otherversion);
+        $startdata = parent::update_attempt_state_data_for_new_version($oldstep, $otherversion);
 
         $mapping = array_combine(array_keys($otherversion->answers), array_keys($this->answers));
 
@@ -113,8 +113,9 @@ abstract class qtype_multichoice_base extends question_graded_automatically {
         foreach ($oldorder as $oldid) {
             $neworder[] = $mapping[$oldid] ?? $oldid;
         }
+        $startdata['_order'] = implode(',', $neworder);
 
-        return ['_order' => implode(',', $neworder)];
+        return $startdata;
     }
 
     public function get_question_summary() {

question/type/multichoice/tests/walkthrough_test.php

@@ -35,6 +35,8 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
  * @package    qtype_multichoice
  * @copyright  2010 The Open University
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @covers     \qtype_multichoice_single_question
+ * @covers     \qtype_multichoice_single_base
  */
 class walkthrough_test extends \qbehaviour_walkthrough_test_base {
     public function test_deferredfeedback_feedback_multichoice_single() {
@@ -211,6 +213,71 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base {
             new \question_pattern_expectation('/class="r1"/'));
     }
 
+    public function test_each_attempt_builds_on_last_and_regrade() {
+
+        // Create a multichoice, single question.
+        $mc = \test_question_maker::make_a_multichoice_single_question();
+        $mc->shuffleanswers = false;
+
+        $rightchoice = 0;
+
+        $this->start_attempt_at_question(clone($mc), 'deferredfeedback', 3);
+
+        // Submit the answer false (correct).
+        $this->process_submission(['answer' => $rightchoice]);
+
+        // Finish the attempt.
+        $this->quba->finish_all_questions();
+
+        // Check the state.
+        $this->check_current_state(question_state::$gradedright);
+        $this->check_current_mark(3);
+        $this->check_current_output(
+            $this->get_contains_mc_radio_expectation($rightchoice, false, true),
+            $this->get_contains_mc_radio_expectation($rightchoice + 1, false, false),
+            $this->get_contains_mc_radio_expectation($rightchoice + 2, false, false));
+
+        // Start a new attempt based on the first one.
+        $firstattemptqa = $this->quba->get_question_attempt($this->slot);
+        $this->quba = \question_engine::make_questions_usage_by_activity('unit_test',
+                \context_system::instance());
+        $this->quba->set_preferred_behaviour('deferredfeedback');
+        $this->slot = $this->quba->add_question(clone($mc), 3);
+        $this->quba->start_question_based_on($this->slot, $firstattemptqa);
+
+        // Verify.
+        $this->check_current_state(question_state::$complete);
+        $this->check_current_mark(null);
+        $this->check_current_output(
+            $this->get_contains_mc_radio_expectation($rightchoice, true, true),
+            $this->get_contains_mc_radio_expectation($rightchoice + 1, true, false),
+            $this->get_contains_mc_radio_expectation($rightchoice + 2, true, false));
+
+        // Finish the attempt without changing the answer.
+        $this->quba->finish_all_questions();
+
+        // Check the state.
+        $this->check_current_state(question_state::$gradedright);
+        $this->check_current_mark(3);
+        $this->check_current_output(
+            $this->get_contains_mc_radio_expectation($rightchoice, false, true),
+            $this->get_contains_mc_radio_expectation($rightchoice + 1, false, false),
+            $this->get_contains_mc_radio_expectation($rightchoice + 2, false, false));
+
+        // Regrade the attempt - code based on question_usage_by_activity::regrade_question.
+        $oldqa = $this->quba->get_question_attempt($this->slot);
+        $newqa = new \question_attempt(clone($mc),
+                $oldqa->get_usage_id(), $this->quba->get_observer());
+        $newqa->set_database_id($oldqa->get_database_id());
+        $newqa->set_slot($oldqa->get_slot());
+        $newqa->set_max_mark(3);
+        $newqa->regrade($oldqa, true);
+
+        // Check the state.
+        $this->assertEquals(question_state::$gradedright, $newqa->get_state());
+        $this->assertEquals(3, $newqa->get_mark());
+    }
+
     public function test_deferredfeedback_feedback_multichoice_multi_showstandardunstruction_yes() {
 
         // Create a multichoice, multi question.

question/type/questionbase.php

@@ -238,6 +238,9 @@ abstract class question_definition {
      * A different version of the question will have different question_answers with different ids. However, the list of
      * choices should be similar, and so we need to shuffle the new list of ids in the same way that the old one was.
      *
+     * Note: be sure to return all the data that was originally in $oldstep, while updating the fields that
+     * require it. Otherwise you might break features like 'Each attempt builds on last' in the quiz.
+     *
      * This method should only be called if {@see validate_can_regrade_with_other_version()} did not
      * flag up a potential problem. So, this method will throw a {@see coding_exception} if it is not
      * possible to work out a return value.

question/type/truefalse/tests/walkthrough_test.php

@@ -30,6 +30,7 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
  * @package    qtype_truefalse
  * @copyright  2011 The Open University
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @covers     \qtype_truefalse_question
  */
 class walkthrough_test extends \qbehaviour_walkthrough_test_base {
     public function test_false_right_does_not_show_feedback_when_not_answered() {
@@ -67,6 +68,63 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base {
 
     }
 
+    public function test_each_attempt_builds_on_last_and_regrade() {
+
+        // Create a true-false question with correct answer false.
+        $tf = \test_question_maker::make_question('truefalse', 'false');
+        $this->start_attempt_at_question(clone($tf), 'deferredfeedback', 1);
+
+        // Submit the answer false (correct).
+        $this->process_submission(['answer' => 0]);
+
+        // Finish the attempt.
+        $this->quba->finish_all_questions();
+
+        // Check the state.
+        $this->check_current_state(question_state::$gradedright);
+        $this->check_current_mark(1);
+        $this->check_current_output(
+                $this->get_contains_tf_true_radio_expectation(false, false),
+                $this->get_contains_tf_false_radio_expectation(false, true));
+
+        // Start a new attempt based on the first one.
+        $firstattemptqa = $this->quba->get_question_attempt($this->slot);
+        $this->quba = \question_engine::make_questions_usage_by_activity('unit_test',
+                \context_system::instance());
+        $this->quba->set_preferred_behaviour('deferredfeedback');
+        $this->slot = $this->quba->add_question(clone($tf), 1);
+        $this->quba->start_question_based_on($this->slot, $firstattemptqa);
+
+        // Verify.
+        $this->check_current_state(question_state::$complete);
+        $this->check_current_mark(null);
+        $this->check_current_output(
+                $this->get_contains_tf_true_radio_expectation(true, false),
+                $this->get_contains_tf_false_radio_expectation(true, true));
+
+        // Finish the attempt without changing the answer.
+        $this->quba->finish_all_questions();
+
+        // Check the state.
+        $this->check_current_state(question_state::$gradedright);
+        $this->check_current_mark(1);
+        $this->check_current_output(
+                $this->get_contains_tf_true_radio_expectation(false, false),
+                $this->get_contains_tf_false_radio_expectation(false, true));
+
+        // Regrade the attempt - code based on question_usage_by_activity::regrade_question.
+        $oldqa = $this->quba->get_question_attempt($this->slot);
+        $newqa = new \question_attempt(clone($tf),
+                $oldqa->get_usage_id(), $this->quba->get_observer());
+        $newqa->set_database_id($oldqa->get_database_id());
+        $newqa->set_slot($oldqa->get_slot());
+        $newqa->regrade($oldqa, true);
+
+        // Check the state.
+        $this->assertEquals(question_state::$gradedright, $newqa->get_state());
+        $this->assertEquals(1, $newqa->get_mark());
+    }
+
     /**
      * @covers \qtype_truefalse_renderer::formulation_and_controls
      */