Merge branch 'MDL-78547' of https://github.com/timhunt/moodle

lib/db/install.xml

@@ -1615,7 +1615,7 @@
       <FIELDS>
         <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
         <FIELD NAME="questionattemptid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Foreign key, references question_attempt.id"/>
-        <FIELD NAME="sequencenumber" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Numbers the steps in a question attempt sequentially."/>
+        <FIELD NAME="sequencenumber" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Numbers the steps in a question attempt sequentially from 0."/>
         <FIELD NAME="state" TYPE="char" LENGTH="13" NOTNULL="true" SEQUENCE="false" COMMENT="One of the constants defined by the question_state class, giving the state of the question at the end of this step."/>
         <FIELD NAME="fraction" TYPE="number" LENGTH="12" NOTNULL="false" SEQUENCE="false" DECIMALS="7" COMMENT="The grade for this question, when graded out of 1. Needs to be multiplied by question_attempt.maxmark to get the actual mark for the question."/>
         <FIELD NAME="timecreated" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Time-stamp of the action that lead to this state being created."/>

mod/quiz/attempt.php

@@ -122,6 +122,10 @@ if (!$attemptobj->set_currentpage($page)) {
     redirect($attemptobj->start_attempt_url(null, $attemptobj->get_currentpage()));
 }
 
+if ($attemptobj->is_own_preview()) {
+    $attemptobj->update_questions_to_new_version_if_changed();
+}
+
 // Initialise the JavaScript.
 $headtags = $attemptobj->get_html_head_contributions($page);
 $PAGE->requires->js_init_call('M.mod_quiz.init_attempt_form', null, false, quiz_get_js_module());

mod/quiz/classes/question/bank/qbank_helper.php

@@ -16,11 +16,13 @@
 
 namespace mod_quiz\question\bank;
 
+use context_module;
 use core_question\local\bank\question_version_status;
 use core_question\local\bank\random_question_loader;
 use core_question\question_reference_manager;
 use qbank_tagquestion\tag_condition;
 use qubaid_condition;
+use stdClass;
 
 defined('MOODLE_INTERNAL') || die();
 
@@ -41,7 +43,7 @@ class qbank_helper {
      * Get the available versions of a question where one of the version has the given question id.
      *
      * @param int $questionid id of a question.
-     * @return \stdClass[] other versions of this question. Each object has fields versionid,
+     * @return stdClass[] other versions of this question. Each object has fields versionid,
      *       version and questionid. Array is returned most recent version first.
      */
     public static function get_version_options(int $questionid): array {
@@ -78,11 +80,11 @@ class qbank_helper {
      *   randomtags, and note that these also have a ->name set and ->qtype set to 'random'.
      *
      * @param int $quizid the id of the quiz to load the data for.
-     * @param \context_module $quizcontext the context of this quiz.
+     * @param context_module $quizcontext the context of this quiz.
      * @param int|null $slotid optional, if passed only load the data for this one slot (if it is in this quiz).
      * @return array indexed by slot, with information about the content of each slot.
      */
-    public static function get_question_structure(int $quizid, \context_module $quizcontext,
+    public static function get_question_structure(int $quizid, context_module $quizcontext,
             int $slotid = null): array {
         global $DB;
 
@@ -207,10 +209,10 @@ class qbank_helper {
     /**
      * Get this list of random selection tag ids from one of the slots returned by get_question_structure.
      *
-     * @param \stdClass $slotdata one of the array elements returned by get_question_structure.
+     * @param stdClass $slotdata one of the array elements returned by get_question_structure.
      * @return array list of tag ids.
      */
-    public static function get_tag_ids_for_slot(\stdClass $slotdata): array {
+    public static function get_tag_ids_for_slot(stdClass $slotdata): array {
         $tagids = [];
         if (!isset($slotdata->filtercondition['filter'])) {
             return $tagids;
@@ -225,10 +227,10 @@ class qbank_helper {
     /**
      * Given a slot from the array returned by get_question_structure, describe the random question it represents.
      *
-     * @param \stdClass $slotdata one of the array elements returned by get_question_structure.
+     * @param stdClass $slotdata one of the array elements returned by get_question_structure.
      * @return string that can be used to display the random slot.
      */
-    public static function describe_random_question(\stdClass $slotdata): string {
+    public static function describe_random_question(stdClass $slotdata): string {
         $qtagids = self::get_tag_ids_for_slot($slotdata);
 
         if ($qtagids) {
@@ -248,12 +250,12 @@ class qbank_helper {
      * Choose question for redo in a particular slot.
      *
      * @param int $quizid the id of the quiz to load the data for.
-     * @param \context_module $quizcontext the context of this quiz.
+     * @param context_module $quizcontext the context of this quiz.
      * @param int $slotid optional, if passed only load the data for this one slot (if it is in this quiz).
      * @param qubaid_condition $qubaids attempts to consider when avoiding picking repeats of random questions.
      * @return int the id of the question to use.
      */
-    public static function choose_question_for_redo(int $quizid, \context_module $quizcontext,
+    public static function choose_question_for_redo(int $quizid, context_module $quizcontext,
             int $slotid, qubaid_condition $qubaids): int {
         $slotdata = self::get_question_structure($quizid, $quizcontext, $slotid);
         $slotdata = reset($slotdata);
@@ -274,4 +276,99 @@ class qbank_helper {
         }
         return $newqusetionid;
     }
+
+    /**
+     * Check all the questions in an attempt and return information about their versions.
+     *
+     * Once a quiz attempt has been started, it continues to use the version of each question
+     * it was started with. This checks the version used for each question, against the
+     * quiz settings for that slot, and returns which version would be used if the quiz
+     * attempt was being started now.
+     *
+     * There are several cases for each slot:
+     * - If this slot is currently set to use version 'Always latest' (which includes
+     *   random slots) and if there is now a newer version than the one in the attempt,
+     *   use that.
+     * - If the slot is currently set to use a fixed version of the question, and that
+     *   is different from the version currently in the attempt, use that.
+     * - Otherwise, use the same version.
+     *
+     * This is used in places like the re-grade code.
+     *
+     * The returned data probably contains a bit more information than is strictly needed,
+     * (see the SQL for details) but returning a few extra ints is fast, and this could
+     * prove invaluable when debugging. The key information is probably:
+     * - questionattemptslot <-- array key
+     * - questionattemptid
+     * - currentversion
+     * - currentquestionid
+     * - newversion
+     * - newquestionid
+     *
+     * @param stdClass $attempt a quiz_attempt database row.
+     * @param context_module $quizcontext the quiz context for the quiz the attempt belongs to.
+     * @return array for each question_attempt in the quiz attempt, information about whether it is using
+     *      the latest version of the question. Array indexed by questionattemptslot.
+     */
+    public static function get_version_information_for_questions_in_attempt(
+        stdClass $attempt,
+        context_module $quizcontext,
+    ): array {
+        global $DB;
+
+        return $DB->get_records_sql("
+            SELECT qa.slot AS questionattemptslot,
+                   qa.id AS questionattemptid,
+                   slot.slot AS quizslot,
+                   slot.id AS quizslotid,
+                   qr.id AS questionreferenceid,
+                   currentqv.version AS currentversion,
+                   currentqv.questionid AS currentquestionid,
+                   newqv.version AS newversion,
+                   newqv.questionid AS newquestionid
+
+              -- Start with the question currently used in the attempt.
+              FROM {question_attempts} qa
+              JOIN {question_versions} currentqv ON currentqv.questionid = qa.questionid
+
+              -- Join in the question metadata which says if this is a qa from a 'Try another question like this one'.
+              JOIN {question_attempt_steps} firststep ON firststep.questionattemptid = qa.id
+                           AND firststep.sequencenumber = 0
+         LEFT JOIN {question_attempt_step_data} otherslotinfo ON otherslotinfo.attemptstepid = firststep.id
+                           AND otherslotinfo.name = :otherslotmetadataname
+
+              -- Join in the quiz slot information, and hence for non-random slots, the questino_reference.
+              JOIN {quiz_slots} slot ON slot.quizid = :quizid
+                           AND slot.slot = COALESCE({$DB->sql_cast_char2int('otherslotinfo.value', true)}, qa.slot)
+         LEFT JOIN {question_references} qr ON qr.usingcontextid = :quizcontextid
+                           AND qr.component = 'mod_quiz'
+                           AND qr.questionarea = 'slot'
+                           AND qr.itemid = slot.id
+
+              -- Finally, get the new version for this slot.
+              JOIN {question_versions} newqv ON newqv.questionbankentryid = currentqv.questionbankentryid
+                           AND newqv.version = COALESCE(
+                               -- If the quiz setting say use a particular version, use that.
+                               qr.version,
+                               -- Otherwise, we need the latest non-draft version of the current questions.
+                               (SELECT MAX(version)
+                                  FROM {question_versions}
+                                 WHERE questionbankentryid = currentqv.questionbankentryid AND status <> :draft),
+                                -- Otherwise, there is not a suitable other version, so stick with the current one.
+                                currentqv.version
+                            )
+
+             -- We want this for questions in the current attempt.
+             WHERE qa.questionusageid = :questionusageid
+
+          -- Order not essential, but fast and good for debugging.
+          ORDER BY qa.slot
+        ", [
+            'otherslotmetadataname' => ':_originalslot',
+            'quizid' => $attempt->quiz,
+            'quizcontextid' => $quizcontext->id,
+            'draft' => question_version_status::QUESTION_STATUS_DRAFT,
+            'questionusageid' => $attempt->uniqueid,
+        ]);
+    }
 }

mod/quiz/classes/quiz_attempt.php

@@ -2341,4 +2341,52 @@ class quiz_attempt {
         }
         return $totalunanswered;
     }
+
+    /**
+     * If any questions in this attempt have changed, update the attempts.
+     *
+     * For now, this should only be done for previews.
+     *
+     * When we update the question, we keep the same question (in the case of random questions)
+     * and the same variant (if this question has variants). If possible, we use regrade to
+     * preserve any interaction that has been had with this question (e.g. a saved answer) but
+     * if that is not possible, we put in a newly started attempt.
+     */
+    public function update_questions_to_new_version_if_changed(): void {
+        global $DB;
+
+        $versioninformation = qbank_helper::get_version_information_for_questions_in_attempt(
+            $this->attempt, $this->get_context());
+
+        $anychanges = false;
+        foreach ($versioninformation as $slotinformation) {
+            if ($slotinformation->currentquestionid == $slotinformation->newquestionid) {
+                continue;
+            }
+
+            $anychanges = true;
+
+            $slot = $slotinformation->questionattemptslot;
+            $newquestion = question_bank::load_question($slotinformation->newquestionid);
+            if (empty($this->quba->validate_can_regrade_with_other_version($slot, $newquestion))) {
+                // We can use regrade to replace the question while preserving any existing state.
+                $finished = $this->get_attempt()->state == self::FINISHED;
+                $this->quba->regrade_question($slot, $finished, null, $newquestion);
+            } else {
+                // So much has changed, we have to replace the question with a new attempt.
+                $oldvariant = $this->get_question_attempt($slot)->get_variant();
+                $slot = $this->quba->add_question_in_place_of_other($slot, $newquestion, null, false);
+                $this->quba->start_question($slot, $oldvariant);
+            }
+        }
+
+        if ($anychanges) {
+            question_engine::save_questions_usage_by_activity($this->quba);
+            if ($this->attempt->state == self::FINISHED) {
+                $this->attempt->sumgrades = $this->quba->get_total_mark();
+                $DB->update_record('quiz_attempts', $this->attempt);
+                $this->recompute_final_grade();
+            }
+        }
+    }
 }

mod/quiz/locallib.php

@@ -35,6 +35,7 @@ require_once($CFG->libdir . '/completionlib.php');
 require_once($CFG->libdir . '/filelib.php');
 require_once($CFG->libdir . '/questionlib.php');
 
+use core_question\local\bank\condition;
 use mod_quiz\access_manager;
 use mod_quiz\event\attempt_submitted;
 use mod_quiz\grade_calculator;
@@ -42,6 +43,7 @@ use mod_quiz\question\bank\qbank_helper;
 use mod_quiz\question\display_options;
 use mod_quiz\quiz_attempt;
 use mod_quiz\quiz_settings;
+use mod_quiz\structure;
 use qbank_previewquestion\question_preview_options;
 
 /**
@@ -1904,8 +1906,16 @@ function quiz_add_random_questions(stdClass $quiz, int $addonpage, int $category
     );
 
     $settings = quiz_settings::create($quiz->id);
-    $structure = \mod_quiz\structure::create_for_quiz($settings);
-    $structure->add_random_questions($addonpage, $number, $categoryid);
+    $structure = structure::create_for_quiz($settings);
+    $structure->add_random_questions($addonpage, $number, [
+        'filter' => [
+            'category' => [
+                'jointype' => condition::JOINTYPE_DEFAULT,
+                'values' => [$categoryid],
+                'filteroptions' => ['includesubcategories' => false],
+            ],
+        ],
+    ]);
 }
 
 /**

mod/quiz/report/overview/report.php

@@ -42,18 +42,6 @@ require_once($CFG->dirroot . '/mod/quiz/report/overview/overview_table.php');
  */
 class quiz_overview_report extends attempts_report {
 
-    /**
-     * @var array|null cached copy of qbank_helper::get_question_structure for use during regrades.
-     */
-    protected $structureforregrade = null;
-
-    /**
-     * @var array|null used during regrades, to cache which new questionid to use for each old on.
-     *      for random questions, stores oldquestionid => newquestionid.
-     *      See get_new_question_for_regrade.
-     */
-    protected $newquestionidsforold = null;
-
     public function display($quiz, $cm, $course) {
         global $DB, $PAGE;
 
@@ -352,6 +340,8 @@ class quiz_overview_report extends attempts_report {
         $transaction = $DB->start_delegated_transaction();
 
         $quba = question_engine::load_questions_usage_by_activity($attempt->uniqueid);
+        $versioninformation = qbank_helper::get_version_information_for_questions_in_attempt(
+            $attempt, $this->context);
 
         if (is_null($slots)) {
             $slots = $quba->get_slots();
@@ -362,7 +352,7 @@ class quiz_overview_report extends attempts_report {
         foreach ($slots as $slot) {
             $qqr = new stdClass();
             $qqr->oldfraction = $quba->get_question_fraction($slot);
-            $otherquestionversion = $this->get_new_question_for_regrade($attempt, $quba, $slot);
+            $otherquestionversion = question_bank::load_question($versioninformation[$slot]->newquestionid);
 
             $message = $quba->validate_can_regrade_with_other_version($slot, $otherquestionversion);
             if ($message) {
@@ -415,47 +405,6 @@ class quiz_overview_report extends attempts_report {
         $this->newquestionidsforold = null;
     }
 
-    /**
-     * Work out of we should be using a new question version for a particular slot in a regrade.
-     *
-     * @param stdClass $attempt the attempt being regraded.
-     * @param question_usage_by_activity $quba the question_usage corresponding to that.
-     * @param int $slot which slot is currently being regraded.
-     * @return question_definition other question version to use for this slot.
-     */
-    protected function get_new_question_for_regrade(stdClass $attempt,
-            question_usage_by_activity $quba, int $slot): question_definition {
-
-        // If the cache is empty, get information about all the slots.
-        if ($this->structureforregrade === null) {
-            $this->newquestionidsforold = [];
-            // Load the data about all the non-random slots now.
-            $this->structureforregrade = qbank_helper::get_question_structure(
-                    $attempt->quiz, $this->context);
-        }
-
-        // Because of 'Redo question in attempt' feature, we need to find the original slot number.
-        $originalslot = $quba->get_question_attempt_metadata($slot, 'originalslot') ?? $slot;
-
-        // If this is a non-random slot, we will have the right info cached.
-        if ($this->structureforregrade[$originalslot]->qtype != 'random') {
-            // This is a non-random slot.
-            return question_bank::load_question($this->structureforregrade[$originalslot]->questionid);
-        }
-
-        // We must be dealing with a random question. Check that cache.
-        $currentquestion = $quba->get_question_attempt($originalslot)->get_question(false);
-        if (isset($this->newquestionidsforold[$currentquestion->id])) {
-            return question_bank::load_question($this->newquestionidsforold[$currentquestion->id]);
-        }
-
-        // This is a random question we have not seen yet. Find the latest version.
-        $versionsoptions = qbank_helper::get_version_options($currentquestion->id);
-        $latestversion = reset($versionsoptions);
-        $this->newquestionidsforold[$currentquestion->id] = $latestversion->questionid;
-        return question_bank::load_question($latestversion->questionid);
-    }
-
     /**
      * Regrade attempts for this quiz, exactly which attempts are regraded is
      * controlled by the parameters.

mod/quiz/review.php

@@ -101,6 +101,7 @@ if ($options->flags == question_display_options::EDITABLE && optional_param('sav
 // Work out appropriate title and whether blocks should be shown.
 if ($attemptobj->is_own_preview()) {
     navigation_node::override_active_url($attemptobj->start_attempt_url());
+    $attemptobj->update_questions_to_new_version_if_changed();
 
 } else {
     if (empty($attemptobj->get_quiz()->showblocks) && !$attemptobj->is_preview_user()) {

mod/quiz/tests/attempt_walkthrough_test.php

@@ -19,6 +19,7 @@ namespace mod_quiz;
 use moodle_url;
 use question_bank;
 use question_engine;
+use mod_quiz\question\bank\qbank_helper;
 
 defined('MOODLE_INTERNAL') || die();
 
@@ -150,6 +151,52 @@ class attempt_walkthrough_test extends \advanced_testcase {
         $gradebookitem = array_shift($gradebookgrades->items);
         $gradebookgrade = array_shift($gradebookitem->grades);
         $this->assertEquals(100, $gradebookgrade->grade);
+
+        // Update question in quiz.
+        $newsa = $questiongenerator->update_question($saq, null,
+            ['name' => 'This is the second version of shortanswer']);
+        $newnumbq = $questiongenerator->update_question($numq, null,
+            ['name' => 'This is the second version of numerical']);
+        $newmatch = $questiongenerator->update_question($matchq, null,
+            ['name' => 'This is the second version of match']);
+        $newdescription = $questiongenerator->update_question($description, null,
+            ['name' => 'This is the second version of description']);
+
+        // Update the attempt to use this questions.
+        // Would not normally be done for a non-preview, but this is just a unit test.
+        $attemptobj->update_questions_to_new_version_if_changed();
+
+        // Verify.
+        $this->assertEquals($newsa->id, $attemptobj->get_question_attempt(1)->get_question_id());
+        $this->assertEquals($newnumbq->id, $attemptobj->get_question_attempt(2)->get_question_id());
+        $this->assertEquals($newmatch->id, $attemptobj->get_question_attempt(3)->get_question_id());
+        $this->assertEquals($newdescription->id, $attemptobj->get_question_attempt(4)->get_question_id());
+
+        // Repeat the checks from above.
+        $this->assertEquals(1, $attemptobj->get_attempt_number());
+        $this->assertEquals(3, $attemptobj->get_sum_marks());
+        $this->assertEquals(true, $attemptobj->is_finished());
+        $this->assertEquals($timenow, $attemptobj->get_submitted_date());
+        $this->assertEquals($user1->id, $attemptobj->get_userid());
+        $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question());
+        $this->assertEquals(0, $attemptobj->get_number_of_unanswered_questions());
+
+        // Re-load quiz attempt data and repeat the verification.
+        $attemptobj = quiz_attempt::create($attempt->id);
+
+        $this->assertEquals($newsa->id, $attemptobj->get_question_attempt(1)->get_question_id());
+        $this->assertEquals($newnumbq->id, $attemptobj->get_question_attempt(2)->get_question_id());
+        $this->assertEquals($newmatch->id, $attemptobj->get_question_attempt(3)->get_question_id());
+        $this->assertEquals($newdescription->id, $attemptobj->get_question_attempt(4)->get_question_id());
+
+        // Repeat the checks from above.
+        $this->assertEquals(1, $attemptobj->get_attempt_number());
+        $this->assertEquals(3, $attemptobj->get_sum_marks());
+        $this->assertEquals(true, $attemptobj->is_finished());
+        $this->assertEquals($timenow, $attemptobj->get_submitted_date());
+        $this->assertEquals($user1->id, $attemptobj->get_userid());
+        $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question());
+        $this->assertEquals(0, $attemptobj->get_number_of_unanswered_questions());
     }
 
     /**

mod/quiz/tests/behat/attempt_basic.feature

@@ -214,3 +214,23 @@ Feature: Attempt a quiz
     And I follow "Finish review"
     And I should not see "Re-attempt quiz"
     And I should see "No more attempts are allowed"
+
+  @javascript
+  Scenario: Student still sees the same version after the question is edited.
+    Given I am on the "Quiz 1" "mod_quiz > View" page logged in as "student"
+    And I press "Attempt quiz"
+    And I should see "First question"
+    And I click on "False" "radio" in the "First question" "question"
+    And I press "Finish attempt ..."
+    And I log out
+    And I am on the "Quiz 1" "mod_quiz > View" page logged in as "admin"
+    And I press "Preview quiz"
+    And I click on "Edit question" "link" in the "First question" "question"
+    And I set the field "Question text" to "First question version 2"
+    And I press "id_submitbutton"
+    And I should see "v2 (latest)" in the "First question" "question"
+    And I log out
+    When I am on the "Quiz 1" "mod_quiz > View" page logged in as "student"
+    And I press "Continue your attempt"
+    Then I should see "First question"
+    And I should not see "First question version 2"

mod/quiz/tests/behat/preview.feature

@@ -80,3 +80,36 @@ Feature: Preview a quiz as a teacher
     When I press "Preview quiz"
     Then I should see "Question 1"
     And "Start a new preview" "button" should exist
+
+  @javascript
+  Scenario: Teacher responses should be cleared after updating the question too much in the preview.
+    Given the following "activities" exist:
+      | activity | name   | course |
+      | quiz     | Quiz 3 | C1     |
+    And the following "questions" exist:
+      | questioncategory | qtype       | name             | questiontext |
+      | Test questions   | multichoice | Multi-choice-002 | one_of_four  |
+    And quiz "Quiz 3" contains the following questions:
+      | question         | page |
+      | Multi-choice-002 | 1    |
+    When I am on the "Quiz 3" "mod_quiz > View" page logged in as "teacher"
+    And I press "Preview quiz"
+    And I should see "one_of_four"
+    And I should see "v1 (latest)"
+    And I click on "One" "qtype_multichoice > Answer"
+    And I click on "Two" "qtype_multichoice > Answer"
+    And I press "Finish attempt ..."
+    And I press "Return to attempt"
+    And I click on "Edit question" "link" in the "Question 1" "question"
+    And I set the field "Question text" to "one_of_four version 2"
+    And I set the field "Choice 4" to ""
+    And I press "id_submitbutton"
+    Then I should see "one_of_four version 2"
+    And I should see "v2 (latest)"
+    And I should see "One"
+    And I should see "Two"
+    And I should see "Three"
+    And I should not see "Four"
+    And "input[type=checkbox][name$=choice0]:checked" "css_element" should not exist
+    And "input[type=checkbox][name$=choice1]:checked" "css_element" should not exist
+    And "input[type=checkbox][name$=choice2]:checked" "css_element" should not exist

mod/quiz/tests/quiz_question_version_test.php

@@ -16,6 +16,7 @@
 
 namespace mod_quiz;
 
+use core_question\local\bank\condition;
 use mod_quiz\external\submit_question_version;
 use mod_quiz\question\bank\qbank_helper;
 
@@ -31,7 +32,7 @@ require_once(__DIR__ . '/quiz_question_helper_test_trait.php');
  * @copyright  2021 Catalyst IT Australia Pty Ltd
  * @author     Safat Shahin <safatshahin@catalyst-au.net>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- * @coversDefaultClass \mod_quiz\question\bank\qbank_helper
+ * @covers \mod_quiz\question\bank\qbank_helper
  */
 class quiz_question_version_test extends \advanced_testcase {
     use \quiz_question_helper_test_trait;
@@ -53,8 +54,6 @@ class quiz_question_version_test extends \advanced_testcase {
 
     /**
      * Test the quiz question data for changed version in the slots.
-     *
-     * @covers ::get_version_options
      */
     public function test_quiz_questions_for_changed_versions() {
         $this->resetAfterTest();
@@ -134,8 +133,6 @@ class quiz_question_version_test extends \advanced_testcase {
 
     /**
      * Test if changing the version of the slot changes the attempts.
-     *
-     * @covers ::get_version_options
      */
     public function test_quiz_question_attempts_with_changed_version() {
         $this->resetAfterTest();
@@ -151,7 +148,7 @@ class quiz_question_version_test extends \advanced_testcase {
         $questiongenerator->update_question($numq, null, ['name' => 'This is the second version']);
         $questiongenerator->update_question($numq, null, ['name' => 'This is the third version']);
         quiz_add_quiz_question($numq->id, $quiz);
-        list($quizobj, $quba, $attemptobj) = $this->attempt_quiz($quiz, $this->student);
+        [, , $attemptobj] = $this->attempt_quiz($quiz, $this->student);
         $this->assertEquals('This is the third version', $attemptobj->get_question_attempt(1)->get_question()->name);
         // Create the quiz object.
         $quizobj = \mod_quiz\quiz_settings::create($quiz->id);
@@ -171,17 +168,88 @@ class quiz_question_version_test extends \advanced_testcase {
         // Change to version 1.
         $this->expectException('moodle_exception');
         submit_question_version::execute($slot->id, (int)$selectversions[1]->version);
-        list($quizobj, $quba, $attemptobj) = $this->attempt_quiz($quiz, $this->student, 2);
+        [, , $attemptobj] = $this->attempt_quiz($quiz, $this->student, 2);
         $this->assertEquals('This is the first version', $attemptobj->get_question_attempt(1)->get_question()->name);
         // Change to version 2.
         submit_question_version::execute($slot->id, (int)$selectversions[2]->version);
-        list($quizobj, $quba, $attemptobj) = $this->attempt_quiz($quiz, $this->student, 3);
+        [, , $attemptobj] = $this->attempt_quiz($quiz, $this->student, 3);
         $this->assertEquals('This is the second version', $attemptobj->get_question_attempt(1)->get_question()->name);
         // Create another version.
         $questiongenerator->update_question($numq, null, ['name' => 'This is the latest version']);
         // Change to always latest.
         submit_question_version::execute($slot->id, 0);
-        list($quizobj, $quba, $attemptobj) = $this->attempt_quiz($quiz, $this->student, 4);
+        [, , $attemptobj] = $this->attempt_quiz($quiz, $this->student, 4);
         $this->assertEquals('This is the latest version', $attemptobj->get_question_attempt(1)->get_question()->name);
     }
+
+    public function test_get_version_information_for_questions_in_attempt(): void {
+        $this->resetAfterTest();
+        /** @var \mod_quiz_generator $quizgenerator */
+        $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz');
+        /** @var \core_question_generator $questiongenerator */
+        $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
+
+        // Make two categories, each with a question.
+        $coursecontext = \context_course::instance($this->course->id);
+        $cat = $questiongenerator->create_question_category(
+            ['name' => 'Non-random questions', 'context' => $coursecontext->id]);
+        $randomcat = $questiongenerator->create_question_category(
+            ['name' => 'Random questions', 'context' => $coursecontext->id]);
+        $q1 = $questiongenerator->create_question('truefalse', null, ['category' => $cat->id]);
+        $q2 = $questiongenerator->create_question('truefalse', null, ['category' => $randomcat->id]);
+
+        // Make the quiz, adding q1, and a random question from randomcat.
+        $quiz = $quizgenerator->create_instance([
+            'course' => $this->course->id,
+            'grade' => 100.0,
+            'sumgrades' => 2,
+            'canredoquestions' => 1,
+            'preferredbehaviour' => 'immediatefeedback',
+        ]);
+        $quizobj = quiz_settings::create($quiz->id);
+        quiz_add_quiz_question($q1->id, $quiz);
+        $structure = $quizobj->get_structure();
+        $structure->add_random_questions(0, 1, [
+            'filter' => [
+                'category' => [
+                    'jointype' => condition::JOINTYPE_DEFAULT,
+                    'values' => [$randomcat->id],
+                    'filteroptions' => ['includesubcategories' => false],
+                ],
+            ],
+        ]);
+
+        // Student starts attempt.
+        $quizobj = quiz_settings::create($quiz->id);
+        $attempt = quiz_prepare_and_start_new_attempt($quizobj, 1, null);
+        $attemptobj = quiz_attempt::create($attempt->id);
+
+        // Answer both questions.
+        $postdata = $questiongenerator->get_simulated_post_data_for_questions_in_usage(
+            $attemptobj->get_question_usage(),
+            [1 => 'True', 2 => 'False'],
+            true,
+        );
+        $attemptobj->process_submitted_actions(time(), false, $postdata);
+
+        // Redo both questions - need to re-create attemptobj each time.
+        $attemptobj = quiz_attempt::create($attempt->id);
+        $attemptobj->process_redo_question(1, time());
+        $attemptobj = quiz_attempt::create($attempt->id);
+        $attemptobj->process_redo_question(2, time());
+
+        // Edit both questions to make a second version.
+        $questiongenerator->update_question($q1);
+        $questiongenerator->update_question($q2);
+
+        // Finally! call the method we want to test.
+        $versioninfo = qbank_helper::get_version_information_for_questions_in_attempt(
+            $attemptobj->get_attempt(), $attemptobj->get_context());
+
+        // Verify - all questions should now want to be V2 for various reasons.
+        $this->assertEquals(2, $versioninfo[1]->newversion);
+        $this->assertEquals(2, $versioninfo[2]->newversion);
+        $this->assertEquals(2, $versioninfo[3]->newversion);
+        $this->assertEquals(2, $versioninfo[4]->newversion);
+    }
 }

mod/quiz/upgrade.txt

@@ -16,6 +16,9 @@ This file describes API changes in the quiz code.
   the review_page() method to an instance of a new templateable class attempt_summary_information for displaying this.
   The $summarydata argument of review_question_page has also been changed to an attempt_summary_information.
 * In the renderer, the view_table has been deprecated. Please use the list_of_attempts renderable instead.
+* There is a new function qbank_helper::get_version_information_for_questions_in_attempt to efficiently
+  check whether each question in an attempt is using the latest versions. This is used by re-grading,
+  replacing some old code there, and to update teacher previews automatically.
 
 === 4.3 ===
 

question/engine/questionusage.php

@@ -174,8 +174,10 @@ class question_usage_by_activity {
 
     /**
      * Add another question to this usage, in the place of an existing slot.
-     * The question_attempt that was in that slot is moved to the end at a new
-     * slot number, which is returned.
+     *
+     * Depending on $keepoldquestionattempt, the question_attempt that was in
+     * that slot is moved to the end at a new slot number, which is returned.
+     * Otherwise the existing attempt is completely removed and replaced.
      *
      * The added question is not started until you call {@link start_question()}
      * on it.
@@ -185,14 +187,18 @@ class question_usage_by_activity {
      * @param number $maxmark the maximum this question will be marked out of in
      *      this attempt (optional). If not given, the max mark from the $qa we
      *      are replacing is used.
+     * @param bool $keepoldquestionattempt if true (the default) we keep the existing
+     *      question_attempt, moving it to a new slot
      * @return int the new slot number of the question that was displaced.
      */
-    public function add_question_in_place_of_other($slot, question_definition $question, $maxmark = null) {
-        $newslot = $this->next_slot_number();
+    public function add_question_in_place_of_other(
+        $slot,
+        question_definition $question,
+        $maxmark = null,
+        bool $keepoldquestionattempt = true,
+    ) {
 
         $oldqa = $this->get_question_attempt($slot);
-        $oldqa->set_slot($newslot);
-        $this->questionattempts[$newslot] = $oldqa;
 
         if ($maxmark === null) {
             $maxmark = $oldqa->get_max_mark();
@@ -200,10 +206,26 @@ class question_usage_by_activity {
 
         $qa = new question_attempt($question, $this->get_id(), $this->observer, $maxmark);
         $qa->set_slot($slot);
-        $this->questionattempts[$slot] = $qa;
 
-        $this->observer->notify_attempt_moved($oldqa, $slot);
-        $this->observer->notify_attempt_added($qa);
+        if ($keepoldquestionattempt) {
+            $newslot = $this->next_slot_number();
+            $oldqa->set_slot($newslot);
+            $this->questionattempts[$newslot] = $oldqa;
+
+            $this->observer->notify_attempt_moved($oldqa, $slot);
+            $this->observer->notify_attempt_added($qa);
+
+        } else {
+            $newslot = $slot;
+            $qa->set_database_id($oldqa->get_database_id());
+
+            foreach ($oldqa->get_step_iterator() as $oldstep) {
+                $this->observer->notify_step_deleted($oldstep, $oldqa);
+            }
+            $this->observer->notify_attempt_modified($qa);
+        }
+
+        $this->questionattempts[$slot] = $qa;
 
         return $newslot;
     }

question/engine/tests/unitofwork_test.php

@@ -507,4 +507,25 @@ class unitofwork_test extends \data_loading_method_test_base {
         $this->assertEquals(array($newslot => array('metathingy' => $this->quba->get_question_attempt($newslot))),
                 $this->observer->get_metadata_added());
     }
+
+    /**
+     * Test add_question_in_place_of_other function.
+     *
+     * @covers ::add_question_in_place_of_other
+     */
+    public function test_replace_old_attempt(): void {
+        // Create a new question.
+        $q = \test_question_maker::make_question('truefalse');
+        $currentquestion = $this->quba->get_question_attempt($this->slot)->get_question();
+        // Replace the current question in the slot with a new one.
+        $slot = $this->quba->add_question_in_place_of_other($this->slot, $q, null, false);
+        $newquestion = $this->quba->get_question_attempt($slot)->get_question();
+
+        $this->assertEquals($this->slot, $slot);
+        $this->assertEquals($q->name, $newquestion->name);
+        $this->assertCount(4, $this->observer->get_steps_deleted());
+        $this->assertCount(1, $this->observer->get_attempts_modified());
+        $this->assertCount(0, $this->observer->get_attempts_added());
+        $this->assertNotEquals($currentquestion->id, $newquestion->id);
+    }
 }

question/engine/upgrade.txt

@@ -1,5 +1,12 @@
 This files describes API changes for the core question engine.
 
+=== 4.4 ===
+
+* The method question_usage_by_activity::add_question_in_place_of_other has been made more flexible.
+  There is a new argument $keepoldquestionattempt. That defaults to true, which behaves the same as
+  the old API, but if you pass false, then the newly added question_attempt completely replaces the
+  existing one in-place.
+
 === 4.2 ===
 
 * A `$questionidentifier` property has been added to `\question_display_options` to enable question type plugins to associate the

question/tests/generator/lib.php

@@ -202,13 +202,13 @@ class core_question_generator extends component_generator_base {
      * responses to a number of questions within a question usage.
      *
      * In the responses array, the array keys are the slot numbers for which a response will
-     * be submitted. You can submit a response to any number of responses within the usage.
+     * be submitted. You can submit a response to any number of questions within the usage.
      * There is no need to do them all. The values are a string representation of the response.
      * The exact meaning of that depends on the particular question type. These strings
      * are passed to the un_summarise_response method of the question to decode.
      *
      * @param question_usage_by_activity $quba the question usage.
-     * @param array $responses the resonses to submit, in the format described above.
+     * @param array $responses the responses to submit, in the format described above.
      * @param bool $checkbutton if simulate a click on the check button for each question, else simulate save.
      *      This should only be used with behaviours that have a check button.
      * @return array that can be passed to methods like $quba->process_all_actions as simulated POST data.