fixup! MDL-82587 mod_quiz: Add slot version updated event

mod/quiz/amd/src/question_slot.js

@@ -45,14 +45,15 @@ const setQuestionVersion = (slotId, newVersion) => fetchMany([{
  */
 const registerEventListeners = () => {
     document.addEventListener('change', e => {
+        window.console.log('changed');
         if (!e.target.matches('[data-action="mod_quiz-select_slot"][data-slot-id]')) {
             return;
         }
 
         const slotId = e.target.dataset.slotId;
         const newVersion = parseInt(e.target.value);
 
-        setQuestionVersion(slotId, newVersion)
+        setQuestionVersion(slotId, newVersion === 0 ? null : newVersion)
             .then(() => {
                 location.reload();
                 return;
@@ -61,16 +62,9 @@ const registerEventListeners = () => {
     });
 };
 
-/** @property {Boolean} eventsRegistered If the event has been registered or not */
-let eventsRegistered = false;
-
 /**
  * Entrypoint of the js.
  */
 export const init = () => {
-    if (eventsRegistered) {
-        return;
-    }
-
     registerEventListeners();
 };

mod/quiz/classes/event/slot_version_updated.php

@@ -39,20 +39,14 @@
  */
 class slot_version_updated extends \core\event\base {
 
-    /**
-     * Initialise the event.
-     */
+    #[\Override]
     protected function init(): void {
         $this->data['objecttable'] = 'quiz_slots';
         $this->data['crud'] = 'u';
         $this->data['edulevel'] = self::LEVEL_TEACHING;
     }
 
-    /**
-     * Get event name.
-     *
-     * @return string
-     */
+    #[\Override]
     public static function get_name(): string {
         return get_string('eventslotversionupdated', 'mod_quiz');
     }
@@ -63,9 +57,11 @@ public static function get_name(): string {
      * @return string
      */
     public function get_description(): string {
+        $previousversion = $this->other['previousversion'] ?? 'Always latest';
+        $newversion = $this->other['newversion'] ?? 'Always latest';
         return "The user with id '$this->userid' updated the slot with id '{$this->objectid}' " .
             "belonging to the quiz with course module id '$this->contextinstanceid'. " .
-            "Its question version was changed from '{$this->other['previousversion']}' to '{$this->other['newversion']}'.";
+            "Its question version was changed from '$previousversion' to '$newversion'.";
     }
 
     /**

mod/quiz/classes/external/submit_question_version.php

@@ -46,19 +46,20 @@ class submit_question_version extends external_api {
      */
     public static function execute_parameters(): external_function_parameters {
         return new external_function_parameters([
-            'slotid' => new external_value(PARAM_INT, ''),
-            'newversion' => new external_value(PARAM_INT, '')
+            'slotid' => new external_value(PARAM_INT),
+            'newversion' => new external_value(PARAM_INT),
         ]);
     }
 
     /**
      * Set the questions slot parameters to display the question template.
      *
-     * @param int $slotid Slot id to display.
-     * @param int $newversion the version to set. 0 means 'always latest'.
+     * @param int $slotid Slot ID to display.
+     * @param int|null $newversion The version to set. Passing null means 'always latest'.
+     *                             For historical reasons, 0 also means 'always latest'.
      * @return array
      */
-    public static function execute(int $slotid, int $newversion): array {
+    public static function execute(int $slotid, ?int $newversion): array {
         global $DB;
         $params = self::validate_parameters(self::execute_parameters(), ['slotid' => $slotid, 'newversion' => $newversion]);
         $slot = $DB->get_record('quiz_slots', ['id' => $slotid], '*', MUST_EXIST);
@@ -68,7 +69,12 @@ public static function execute(int $slotid, int $newversion): array {
         require_capability('mod/quiz:manage', $context);
 
         $quizobj = quiz_settings::create($slot->quizid);
-        return ['result' => $quizobj->get_structure()->update_slot_version($slot->id, $params['newversion'])];
+
+        // This WS historically (and wrongly) accepted 0 to mean 'always latest'. The correct behaviour
+        // is that null implies awlays latest. To preserve backwards compatibility, we continue to accept
+        // 0, but just turn it in to null before passing to the appropriate API. See: MDL-82587.
+        $newversionnormalised = $params['newversion'] === 0 ? null : $params['newversion'];
+        return ['result' => $quizobj->get_structure()->update_slot_version($slot->id, $newversionnormalised)];
     }
 
     /**

mod/quiz/classes/output/edit_renderer.php

@@ -521,6 +521,8 @@ public function questions_in_section(structure $structure, $section,
         foreach ($structure->get_slots_in_section($section->id) as $slot) {
             $output .= $this->question_row($structure, $slot, $contexts, $pagevars, $pageurl);
         }
+
+        $this->page->requires->js_call_amd('mod_quiz/question_slot', 'init');
         return html_writer::tag('ul', $output, ['class' => 'section img-text']);
     }
 
@@ -776,7 +778,6 @@ public function question(structure $structure, int $slot, \moodle_url $pageurl)
         if ($structure->get_slot_by_number($slot)->qtype !== 'random') {
             $data['versionselection'] = true;
             $data['versionoption'] = $structure->get_version_choices_for_slot($slot);
-            $this->page->requires->js_call_amd('mod_quiz/question_slot', 'init', [$slotid]);
         }
 
         // Render the question slot template.

mod/quiz/classes/structure.php

@@ -1170,33 +1170,51 @@ public function update_slot_maxmark($slot, $maxmark) {
     }
 
     /**
-     * Update slot question version.
+     * Update the question version for a given slot, if necessary.
      *
      * @param int $id ID of row from the quiz_slots table.
-     * @param int $newversion The new question version for the slot.
-     * @return bool
+     * @param int|null $newversion The new question version for the slot.
+     *                             A null value means 'Always latest'.
+     * @return bool True if the version was updated, false if no update was required.
+     * @throws coding_exception If the specified version does not exist.
      */
-    public function update_slot_version(int $id, int $newversion): bool {
+    public function update_slot_version(int $id, ?int $newversion): bool {
         global $DB;
 
+
         $slot = $this->get_slot_by_id($id);
         $context = $this->quizobj->get_context();
-        $params = ['usingcontextid' => $context->id, 'component' => 'mod_quiz', 'questionarea' => 'slot', 'itemid' => $slot->id];
-        $reference = $DB->get_record('question_references', $params, '*', MUST_EXIST);
-        $oldversion = $reference->version;
+        $referenceparams = ['usingcontextid' => $context->id, 'component' => 'mod_quiz', 'questionarea' => 'slot', 'itemid' => $slot->id];
+        $reference = $DB->get_record('question_references', $referenceparams, '*', MUST_EXIST);
+        $oldversion = (int)$reference->version;
         $reference->version = $newversion === 0 ? null : $newversion;
+        $existsparams = ['questionbankentryid' => $reference->questionbankentryid, 'version' => $newversion];
+        $versionexists = $DB->record_exists('question_versions', $existsparams);
+
+        // We are attempting to switch to an existing version.
+        // Verify that the version we want to switch to exists.
+        if (!is_null($newversion) && !$versionexists) {
+            throw new coding_exception('Version: ' . $newversion . ' does not exist for question bank entry: ' . $reference->questionbankentryid);
+        }
+
+        if ($newversion === $oldversion) {
+            return false;
+        }
 
+        $transaction = $DB->start_delegated_transaction();
+        $DB->update_record('question_references', $reference);
         slot_version_updated::create([
             'context' => $this->quizobj->get_context(),
             'objectid' => $slot->id,
             'other' => [
                 'quizid' => $this->get_quizid(),
-                'previousversion' => $oldversion ?? get_string('alwayslatest', 'quiz'),
-                'newversion' => $reference->version ?? get_string('alwayslatest', 'quiz'),
+                'previousversion' => $oldversion,
+                'newversion' => $reference->version,
             ],
         ])->trigger();
+        $transaction->allow_commit();
 
-        return $DB->update_record('question_references', $reference);
+        return true;
     }
 
     /**

mod/quiz/tests/event/events_test.php

@@ -37,6 +37,7 @@
  * @category   phpunit
  * @copyright  2013 Adrian Greeve
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @covers \mod_quiz\external\submit_question_version
  */
 class events_test extends \advanced_testcase {
 
@@ -1207,11 +1208,6 @@ public function test_slot_mark_updated(): void {
         $this->assertEventContextNotUsed($event);
     }
 
-    /**
-     * Test the slot version updated event.
-     *
-     * @covers \mod_quiz\external\submit_question_version
-     */
     public function test_slot_version_updated(): void {
         $quizobj = $this->prepare_quiz();
         $this->setAdminUser();

mod/quiz/tests/structure_test.php

@@ -16,6 +16,9 @@
 
 namespace mod_quiz;
 
+use core\exception\coding_exception;
+use question_bank;
+
 defined('MOODLE_INTERNAL') || die();
 
 global $CFG;
@@ -904,6 +907,40 @@ public function test_update_question_dependency(): void {
         $this->assertEquals(0, $structure->is_question_dependent_on_previous_slot(2));
     }
 
+    public function test_update_slot_version(): void {
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz');
+        $quiz = $quizgenerator->create_instance(['course' => $course->id, 'questionsperpage' => 0,
+                'grade' => 100.0, 'sumgrades' => 2]);
+
+        get_coursemodule_from_instance('quiz', $quiz->id, $course->id);
+
+        $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
+        $cat = $questiongenerator->create_question_category();
+        $numq = $questiongenerator->create_question('numerical', null, ['category' => $cat->id]);
+        $questiongenerator->update_question($numq, null, ['name' => 'Second version of numq']);
+        quiz_add_quiz_question($numq->id, $quiz);
+
+        $quizobj = quiz_settings::create($quiz->id);
+        $quizobj->preload_questions();
+        [$question] = array_values($quizobj->get_questions(null, false));
+        $structure = $quizobj->get_structure();
+
+        // Updating to a version which exists, should succeed.
+        $this->assertTrue($structure->update_slot_version($question->slotid, 2));
+
+        // Updating to the same version as the current version should return false.
+        $this->assertFalse($structure->update_slot_version($question->slotid, 2));
+
+        // Updating to a version which does not exists, should throw exception.
+        $this->expectException(coding_exception::class);
+        $this->expectExceptionMessage('Version: 3 does not exist for question bank entry: ' . $question->questionbankentryid);
+        $structure->update_slot_version($question->slotid, 3);
+
+    }
+
     public function test_update_slot_grade_item(): void {
         $quizobj = $this->create_test_quiz([
                 ['TF1', 1, 'truefalse'],