L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-86798 questions: Track next version for question bank entries

Browse Source 
This adds a new "nextversion" field to `question_bank_entries` to track
which version number the next version of the question should have. This
ensures that version numbers are not re-used if the latest version is
deleted.

To use this field, you must call
`\core_question\versions::get_next_version()`. This will initialise the
field to the correct value if it is currently null.

If you create a new version using this value, you must then call
`\core_question\versions::increment_next_version()` to increment the
counter.
This commit is contained in:
Mark Johnson
2025-12-19 09:31:14 +00:00
parent 11d0802ca6
commit cfaccbf953
12 changed files with 385 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.upgradenotes/MDL-86798-2025111411591681.yml
+17
View File
@@ -0,0 +1,17 @@
issueNumber: MDL-86798
notes:
core_question:
- message: >
In order to prevent re-use of question version numbers after a version
is deleted, the `nextversion` column was added to
`question_bank_entries`. This serves as a counter incremented each time
a version is created.
Do not query this field directly. Instead use
`core_question\versions::get_next_version()` to read the value, which
will initialise it based on the existing versions if it is not set yet.
By default, it will increment the version number automatically, unless
you pass `increment: false`. Because of this, it is advisable to call
it inside a transaction, that is only committed after the version number
is used in a `question_versions` record.
type: fixed
public/backup/moodle2/backup_stepslib.php
+1
View File
@@ -2649,6 +2649,7 @@ class backup_questions_structure_step extends backup_structure_step {
'questioncategoryid',
'idnumber',
'ownerid',
'nextversion',
]);
$questionversions = new backup_nested_element('question_version');
public/backup/moodle2/restore_stepslib.php
+3
View File
@@ -28,6 +28,7 @@
defined('MOODLE_INTERNAL') || die();
use core_question\local\bank\question_version_status;
use core_question\versions;
/**
* delete old directories and conditionally create backup_temp_ids table
@@ -5355,6 +5356,8 @@ class restore_create_categories_and_questions extends restore_structure_step {
]
);
}
// Ensure the nextversion value has been initialised, and increment it to account for the additional version.
versions::get_next_version($this->latestversion->questionbankentryid);
}
$newqvid = $DB->insert_record('question_versions', $this->latestversion);
$this->set_mapping('question_versions', $oldqvid, $newqvid);
public/lib/db/install.xml
+2 -1
View File
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8" ?>
<XMLDB PATH="lib/db" VERSION="20251118" COMMENT="XMLDB file for core Moodle tables"
<XMLDB PATH="lib/db" VERSION="20251205" COMMENT="XMLDB file for core Moodle tables"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="../../lib/xmldb/xmldb.xsd"
>
@@ -1484,6 +1484,7 @@
<FIELD NAME="questioncategoryid" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="ID of the category this question is part of."/>
<FIELD NAME="idnumber" TYPE="char" LENGTH="100" NOTNULL="false" SEQUENCE="false" COMMENT="Unique identifier, useful especially for mapping to external entities."/>
<FIELD NAME="ownerid" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="userid of person who owns this question bank entry."/>
<FIELD NAME="nextversion" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="The next version number for this question bank entry. This must be incremented each time a new question_version is created."/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id"/>
public/lib/db/upgrade.php
+14
View File
@@ -2369,5 +2369,19 @@ function xmldb_main_upgrade($oldversion) {
upgrade_main_savepoint(true, 2025100601.02);
}
if ($oldversion < 2025100601.04) {
// Define field nextversion to be added to question_bank_entries.
$table = new xmldb_table('question_bank_entries');
$field = new xmldb_field('nextversion', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'ownerid');
// Conditionally launch add field nextversion.
if (!$dbman->field_exists($table, $field)) {
$dbman->add_field($table, $field);
}
// Main savepoint reached.
upgrade_main_savepoint(true, 2025100601.04);
}
return true;
}
public/lib/questionlib.php
+1 -14
View File
@@ -1936,20 +1936,7 @@ function get_question_version($questionid): array {
* @throws dml_exception
*/
function get_next_version(int $questionbankentryid): int {
global $DB;
$sql = "SELECT MAX(qv.version)
FROM {question_versions} qv
JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid
WHERE qbe.id = :id";
$nextversion = $DB->get_field_sql($sql, ['id' => $questionbankentryid]);
if ($nextversion) {
return (int)$nextversion + 1;
}
return 1;
return \core_question\versions::get_next_version($questionbankentryid);
}
/**
public/question/classes/versions.php
+76
View File
@@ -0,0 +1,76 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace core_question;
/**
* Methods for finding and manipulating question versions
*
* @package core_question
* @copyright 2025 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class versions {
/**
* Get the next version number for a question bank entry.
*
* This uses the value in the question bank entry record, but if it's not set, it will calculate it based on the
* current highest version in question_versions.
*
* If calling this with $increment = true, it is a good idea to do this inside a transaction which only commits after the
* new version number has been used to save a new version of the question. This avoids wasting version numbers if an
* error happens.
*
* @param int $questionbankentryid
* @param bool $increment If true, increment the version number by 1 after it is read.
* @return int The number of the next version.
*/
public static function get_next_version(int $questionbankentryid, bool $increment = true): int {
global $DB;
$transaction = $DB->start_delegated_transaction();
$nextversion = $DB->get_field('question_bank_entries', 'nextversion', ['id' => $questionbankentryid]);
if (is_null($nextversion)) {
$nextversion = $DB->get_field_sql(
"SELECT COALESCE(MAX(qv.version), 0) + 1
FROM {question_versions} qv
WHERE qv.questionbankentryid = :qbeid",
['qbeid' => $questionbankentryid],
);
$DB->set_field('question_bank_entries', 'nextversion', $nextversion, ['id' => $questionbankentryid]);
}
if ($increment) {
self::increment_next_version($questionbankentryid);
}
$transaction->allow_commit();
return $nextversion;
}
/**
* Increment the next version by 1 for the question bank entry
*
* @param int $questionbankentryid
*/
protected static function increment_next_version(int $questionbankentryid): bool {
global $DB;
return $DB->execute(
"UPDATE {question_bank_entries}
SET nextversion = nextversion + 1
WHERE id = :id",
['id' => $questionbankentryid],
);
}
}
public/question/format.php
+1
View File
@@ -472,6 +472,7 @@ class qformat_default {
$questionbankentry->questioncategoryid = $question->category;
$questionbankentry->idnumber = $question->idnumber ?? null;
$questionbankentry->ownerid = $question->createdby;
$questionbankentry->nextversion = 2;
$questionbankentry->id = $DB->insert_record('question_bank_entries', $questionbankentry);
// Create a version for each question imported.
$questionversion = new \stdClass();
public/question/tests/backup_test.php
+149
View File
@@ -24,6 +24,7 @@ use question_bank;
defined('MOODLE_INTERNAL') || die();
use backup;
use core\context\module;
use core_question\local\bank\question_bank_helper;
use core_question\local\bank\question_version_status;
use restore_controller;
@@ -899,6 +900,7 @@ final class backup_test extends \advanced_testcase {
$DB->set_field('question_versions', 'version', 3, ['questionid' => $questionv5->id]);
$questionv6 = $questiongenerator->update_question($questionv5, null, ['name' => 'Version 6']);
$DB->set_field('question_versions', 'version', 4, ['questionid' => $questionv6->id]);
$DB->set_field('question_bank_entries', 'nextversion', 5, ['id' => $questionv1->questionbankentryid]);
// Add a quiz specifically using "version 5" (with version number 3).
$quiz2 = self::getDataGenerator()->create_module('quiz', ['course' => $testdata->course->id]);
@@ -975,5 +977,152 @@ final class backup_test extends \advanced_testcase {
$structure3 = $quiz3settings->get_structure();
$this->assertEquals($questionv6->id, $structure3->get_question_in_slot(1)->questionid);
// The nextversion was incremented when the old versions were inserted.
$this->assertEquals(7, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $questionv1->questionbankentryid]));
}
/**
* Restore a backup containing question versions that were deleted, after new versions were created.
*
* Unlike {@see test_restore_backup_containing_deleted_versions}, we do not mess with the version numbers or nextversion
* counter, meaning a gap should be left for the deleted question, and it should be restore in its original place.
*/
public function test_restore_backup_containing_deleted_versions_using_nextversion(): void {
global $DB;
self::setAdminUser();
$this->resetAfterTest();
$questiongenerator = self::getDataGenerator()->get_plugin_generator('core_question');
$testdata = $this->add_course_quiz_and_qbank();
$questionv1 = $testdata->qbankquestion;
$questionv2 = $questiongenerator->update_question($questionv1, null, ['name' => 'Version 2']);
$questionv3 = $questiongenerator->update_question($questionv2, null, ['name' => 'Version 3']);
$questionv4 = $questiongenerator->update_question($questionv3, null, ['name' => 'Version 4']);
$quizsettings = quiz_settings::create($testdata->quiz->id);
$structure1 = $quizsettings->get_structure();
// Set the usage of the question to specifically use version 2.
quiz_add_quiz_question($questionv2->id, $testdata->quiz);
$structure1->update_slot_version($structure1->get_slot_id_for_slot(1), 2);
$backupid = $this->backup_course($testdata->course);
question_delete_question($questionv4->id); // Actually deleted.
question_delete_question($questionv3->id); // Actually deleted.
question_delete_question($questionv2->id); // Hidden, it's being used explicitly.
$questionv5 = $questiongenerator->update_question($questionv1, null, ['name' => 'Version 5']);
$questionv6 = $questiongenerator->update_question($questionv5, null, ['name' => 'Version 6']);
$this->assertEquals(7, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $questionv1->questionbankentryid]));
// Add a quiz specifically using "version 5" .
$quiz2 = self::getDataGenerator()->create_module('quiz', ['course' => $testdata->course->id]);
$quiz2settings = quiz_settings::create($quiz2->id);
quiz_add_quiz_question($questionv5->id, $quiz2);
$structure2 = $quiz2settings->get_structure();
$structure2->update_slot_version($structure2->get_last_slot()->id, 5);
// Add another quiz using the "always latest" version of the question.
$quiz3 = self::getDataGenerator()->create_module('quiz', ['course' => $testdata->course->id]);
$quiz3settings = quiz_settings::create($quiz3->id);
quiz_add_quiz_question($questionv6->id, $quiz3);
$qbe = get_question_bank_entry($questionv1->id);
$versions = $DB->get_records(
'question_versions',
['questionbankentryid' => $qbe->id],
fields: 'version, questionid, status',
);
$this->assertCount(4, $versions);
$this->assertEquals($questionv1->id, $versions[1]->questionid);
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[1]->status);
$this->assertEquals($questionv2->id, $versions[2]->questionid);
$this->assertEquals(question_version_status::QUESTION_STATUS_HIDDEN, $versions[2]->status);
$this->assertArrayNotHasKey(3, $versions);
$this->assertArrayNotHasKey(4, $versions);
$this->assertEquals($questionv5->id, $versions[5]->questionid);
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[5]->status);
$this->assertEquals($questionv6->id, $versions[6]->questionid);
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[6]->status);
$structure1 = $quizsettings->get_structure();
$this->assertEquals($questionv2->id, $structure1->get_question_in_slot(1)->questionid);
$structure2 = $quiz2settings->get_structure();
$this->assertEquals($questionv5->id, $structure2->get_question_in_slot(1)->questionid);
$structure3 = $quiz3settings->get_structure();
$this->assertEquals($questionv6->id, $structure3->get_question_in_slot(1)->questionid);
$this->restore_to_course($backupid, $testdata->course->id);
$versions = $DB->get_records(
'question_versions',
['questionbankentryid' => $qbe->id],
fields: 'version, questionid, status',
);
$this->assertCount(6, $versions);
$this->assertEquals($questionv1->id, $versions[1]->questionid);
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[1]->status);
// Version 2 is restored to "ready" status.
$this->assertEquals($questionv2->id, $versions[2]->questionid);
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[2]->status);
// Version 3 is a new copy of $questionv3 with a different ID but the same content.
$this->assertEquals($questionv3->name, $DB->get_field('question', 'name', ['id' => $versions[3]->questionid]));
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[3]->status);
// Version 4 is a new copy of $questionv4 with a different ID but the same content.
$this->assertEquals($questionv4->name, $DB->get_field('question', 'name', ['id' => $versions[4]->questionid]));
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[4]->status);
// Versions 5 and 6 have had their version numbers bumped up by 1.
$this->assertEquals($questionv5->id, $versions[5]->questionid);
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[5]->status);
$this->assertEquals($questionv6->id, $versions[6]->questionid);
$this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[6]->status);
// Questions referencing specific versions still point to the same question.
$structure1 = $quizsettings->get_structure();
$this->assertEquals($questionv2->id, $structure1->get_question_in_slot(1)->questionid);
$structure2 = $quiz2settings->get_structure();
$this->assertEquals($questionv5->id, $structure2->get_question_in_slot(1)->questionid);
$structure3 = $quiz3settings->get_structure();
$this->assertEquals($questionv6->id, $structure3->get_question_in_slot(1)->questionid);
// The nextversion has not changed during the restore.
$this->assertEquals(7, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $questionv1->questionbankentryid]));
}
/**
* Restoring a question bank entry retains the original nextquestion value.
*
* @return void
* @throws \dml_exception
*/
public function test_backup_question_nextquestion_retained(): void {
global $DB;
self::setAdminUser();
$this->resetAfterTest();
$questiongenerator = self::getDataGenerator()->get_plugin_generator('core_question');
$testdata = $this->add_course_quiz_and_qbank();
$questionv1 = $testdata->qbankquestion;
$questiongenerator->update_question($questionv1, null, ['name' => 'Version 2']);
$this->assertEquals(3, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $questionv1->questionbankentryid]));
$backupid = $this->backup_course($testdata->course);
$newcourse = $this->getDataGenerator()->create_course();
$this->restore_to_course($backupid, $newcourse->id);
$qbanks = get_fast_modinfo($newcourse)->get_instances_of('qbank');
$qbank = reset($qbanks);
$qbankcontext = module::instance($qbank->id);
$category = question_get_default_category($qbankcontext->id);
// The question bank entry for the restored question has the same nextqueston value as the original.
$this->assertEquals(3, $DB->get_field('question_bank_entries', 'nextversion', ['questioncategoryid' => $category->id]));
}
}
public/question/tests/versions_test.php
+118
View File
@@ -0,0 +1,118 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace core_question;
use core\context\module;
/**
* Unit tests for versions
*
* @package core_question
* @copyright 2025 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \core_question\versions
*/
final class versions_test extends \advanced_testcase {
/**
* Generate 3 questions - one with 3 versions, one with 2, and one with 1.
*
* @return array
*/
protected function create_question_versions(): array {
$qbank = $this->getDataGenerator()->create_module('qbank', ['course' => SITEID]);
$qbankcontext = module::instance($qbank->cmid);
$category = question_get_default_category($qbankcontext->id);
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
$q1v1 = $questiongenerator->create_question('shortanswer', null, ['category' => $category->id]);
$q1v2 = $questiongenerator->update_question($q1v1);
$questiongenerator->update_question($q1v2);
$q2v1 = $questiongenerator->create_question('shortanswer', null, ['category' => $category->id]);
$questiongenerator->update_question($q2v1);
$q3v1 = $questiongenerator->create_question('shortanswer', null, ['category' => $category->id]);
return [
$q1v1->questionbankentryid,
$q2v1->questionbankentryid,
$q3v1->questionbankentryid,
];
}
/**
* We should get the correct next version for each question bank entry.
*/
public function test_get_next_version(): void {
$this->resetAfterTest();
[$qbe1, $qbe2, $qbe3] = $this->create_question_versions();
$this->assertEquals(4, versions::get_next_version($qbe1));
$this->assertEquals(3, versions::get_next_version($qbe2));
$this->assertEquals(2, versions::get_next_version($qbe3));
}
/**
* Set the correct next version numbers for existing question bank entries.
*/
public function test_get_next_version_null(): void {
global $DB;
$this->resetAfterTest();
[$qbe1, $qbe2, $qbe3] = $this->create_question_versions();
// Null the nextversion values so they have to be recalculated.
$DB->set_field('question_bank_entries', 'nextversion', null);
$this->assertEquals(4, versions::get_next_version($qbe1));
$this->assertEquals(3, versions::get_next_version($qbe2));
$this->assertEquals(2, versions::get_next_version($qbe3));
}
/**
* The next version should be correctly incremented.
*/
public function test_increment_next_version(): void {
$this->resetAfterTest();
global $DB;
[$qbe1, $qbe2, $qbe3] = $this->create_question_versions();
$this->assertEquals(4, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $qbe1]));
$this->assertEquals(3, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $qbe2]));
$this->assertEquals(2, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $qbe3]));
$this->assertEquals(3, versions::get_next_version($qbe2));
// The specified question bank entry has had its nextversion incremented, the others are the same.
$this->assertEquals(4, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $qbe1]));
$this->assertEquals(4, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $qbe2]));
$this->assertEquals(2, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $qbe3]));
}
/**
* We can get the next version without incrementing it.
*/
public function test_get_without_increment(): void {
$this->resetAfterTest();
global $DB;
[, $qbe2] = $this->create_question_versions();
$this->assertEquals(3, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $qbe2]));
$this->assertEquals(3, versions::get_next_version($qbe2, increment: false));
// The nextversion value has not changed.
$this->assertEquals(3, $DB->get_field('question_bank_entries', 'nextversion', ['id' => $qbe2]));
}
}
public/question/type/questiontypebase.php
+2 -1
View File
@@ -29,6 +29,7 @@ defined('MOODLE_INTERNAL') || die();
require_once($CFG->dirroot . '/question/engine/lib.php');
require_once($CFG->libdir . '/questionlib.php');
use core_question\versions;
/**
* This is the base class for Moodle question types.
@@ -494,7 +495,7 @@ class question_type {
// Get the status field. It comes from the form, but for testing we can.
$status = $form->status ?? $question->status ??
\core_question\local\bank\question_version_status::QUESTION_STATUS_READY;
$questionversion->version = get_next_version($questionbankentry->id);
$questionversion->version = versions::get_next_version($questionbankentry->id);
$questionversion->status = $status;
} else {
$parentversion = get_question_version($form->parent);
public/version.php
+1 -1
View File
@@ -29,7 +29,7 @@
defined('MOODLE_INTERNAL') || die();
$version = 2025100601.03; // 20251006 = branching date YYYYMMDD - do not modify!
$version = 2025100601.04; // 20251006 = branching date YYYYMMDD - do not modify!
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.
$release = '5.1.1+ (Build: 20251219)'; // Human-friendly version name