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

…o MOODLE_402_STABLE

lib/db/install.xml

@@ -1579,7 +1579,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

@@ -120,6 +120,10 @@
     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,9 +16,11 @@
 
 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 qubaid_condition;
+use stdClass;
 
 defined('MOODLE_INTERNAL') || die();
 
@@ -39,7 +41,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 {
@@ -76,11 +78,11 @@ public static function get_version_options(int $questionid): array {
      *   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;
 
@@ -203,10 +205,10 @@ public static function get_question_structure(int $quizid, \context_module $quiz
     /**
      * 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 = [];
         foreach ($slotdata->randomtags as $taginfo) {
             [$id] = explode(',', $taginfo, 2);
@@ -218,10 +220,10 @@ public static function get_tag_ids_for_slot(\stdClass $slotdata): array {
     /**
      * 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 {
         global $DB;
         $category = $DB->get_record('question_categories', ['id' => $slotdata->category]);
         return \question_bank::get_qtype('random')->question_name(
@@ -232,12 +234,12 @@ public static function describe_random_question(\stdClass $slotdata): string {
      * 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);
@@ -257,4 +259,99 @@ public static function choose_question_for_redo(int $quizid, \context_module $qu
         }
         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

@@ -2336,4 +2336,52 @@ public function get_number_of_unanswered_questions(): int {
         }
         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/report/overview/report.php

@@ -42,18 +42,6 @@
  */
 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 @@ public function regrade_attempt($attempt, $dryrun = false, $slots = null): array
         $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 @@ public function regrade_attempt($attempt, $dryrun = false, $slots = null): array
         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 @@ public function clear_regrade_date_cache(): void {
         $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

@@ -100,6 +100,7 @@
 // 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()) {