diff --git a/lib/db/install.xml b/lib/db/install.xml
index 518e510b245..1e719c7a844 100644
--- a/lib/db/install.xml
+++ b/lib/db/install.xml
@@ -1475,7 +1475,7 @@
-
+
diff --git a/lib/questionlib.php b/lib/questionlib.php
index f7a2eff980b..762b7d30081 100644
--- a/lib/questionlib.php
+++ b/lib/questionlib.php
@@ -28,6 +28,8 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
+use core_question\local\bank\question_version_status;
+use core_question\question_reference_manager;
defined('MOODLE_INTERNAL') || die();
@@ -122,6 +124,10 @@ function questions_in_use($questionids): bool {
return true;
}
+ if (question_reference_manager::questions_with_references($questionids)) {
+ return true;
+ }
+
// Check if any plugins are using these questions.
$callbacksbytype = get_plugins_with_function('questions_in_use');
foreach ($callbacksbytype as $callbacks) {
@@ -360,8 +366,10 @@ function question_delete_question($questionid): void {
$questionstocheck[] = $question->parent;
}
- // Do not delete a question if it is used by an activity module
+ // Do not delete a question if it is used by an activity module. Just mark the version hidden.
if (questions_in_use($questionstocheck)) {
+ $DB->set_field('question_versions', 'status',
+ question_version_status::QUESTION_STATUS_HIDDEN, ['questionid' => $questionid]);
return;
}
@@ -1486,9 +1494,10 @@ function question_has_capability_on($questionorid, $cap, $notused = -1): bool {
/**
* Require capability on question.
*
- * @param object $question
- * @param string $cap
- * @return bool
+ * @param int|stdClass|question_definition $question object or id.
+ * If an object is passed, it should include ->contextid and ->createdby.
+ * @param string $cap 'add', 'edit', 'view', 'use', 'move' or 'tag'.
+ * @return bool true if the user has the capability. Throws exception if not.
*/
function question_require_capability_on($question, $cap): bool {
if (!question_has_capability_on($question, $cap)) {
diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php
index b8fc686a9fc..bdfc85229cf 100644
--- a/mod/quiz/lib.php
+++ b/mod/quiz/lib.php
@@ -1432,25 +1432,15 @@ function quiz_get_post_actions() {
}
/**
+ * Standard callback used by questions_in_use.
+ *
* @param array $questionids of question ids.
* @return bool whether any of these questions are used by any instance of this module.
*/
function quiz_questions_in_use($questionids) {
- global $DB;
- list($test, $params) = $DB->get_in_or_equal($questionids);
- $params['component'] = 'mod_quiz';
- $params['questionarea'] = 'slot';
- $sql = "SELECT qs.id
- FROM {quiz_slots} qs
- JOIN {question_references} qr ON qr.itemid = qs.id
- JOIN {question_bank_entries} qbe ON qbe.id = qr.questionbankentryid
- JOIN {question_versions} qv ON qv.questionbankentryid = qbe.id
- WHERE qv.questionid $test
- AND qr.component = ?
- AND qr.questionarea = ?";
- return $DB->record_exists_sql($sql, $params) || question_engine::questions_in_use(
- $questionids, new qubaid_join('{quiz_attempts} quiza',
- 'quiza.uniqueid', 'quiza.preview = 0'));
+ return question_engine::questions_in_use($questionids,
+ new qubaid_join('{quiz_attempts} quiza', 'quiza.uniqueid',
+ 'quiza.preview = 0'));
}
/**
diff --git a/question/bank/deletequestion/delete.php b/question/bank/deletequestion/delete.php
index b41ba32bdae..dbb697aafec 100644
--- a/question/bank/deletequestion/delete.php
+++ b/question/bank/deletequestion/delete.php
@@ -83,12 +83,7 @@ if ($deleteselected && ($confirm = optional_param('confirm', '', PARAM_ALPHANUM)
foreach ($questionlist as $questionid) {
$questionid = (int)$questionid;
question_require_capability_on($questionid, 'edit');
- if (questions_in_use(array($questionid))) {
- $DB->set_field('question_versions', 'status',
- \core_question\local\bank\question_version_status::QUESTION_STATUS_HIDDEN, ['questionid' => $questionid]);
- } else {
- question_delete_question($questionid);
- }
+ question_delete_question($questionid);
}
}
redirect($returnurl);
diff --git a/question/classes/question_reference_manager.php b/question/classes/question_reference_manager.php
new file mode 100644
index 00000000000..ef1d3c69896
--- /dev/null
+++ b/question/classes/question_reference_manager.php
@@ -0,0 +1,77 @@
+.
+
+namespace core_question;
+
+use core_question\local\bank\question_version_status;
+
+/**
+ * This class should provide an API for managing question_references.
+ *
+ * Unfortunately, question_references were introduced in the DB structure
+ * without an nice API. This class is being added later, and is currently
+ * terribly incomplete, but hopefully it can be improved in time.
+ *
+ * @package core_question
+ * @copyright 2023 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class question_reference_manager {
+ /**
+ * Return a list of those questions from the list passed in, which are referenced.
+ *
+ * A question is referenced if either:
+ * - There is a question_reference pointing at exactly that version of that question; or
+ * - There is an 'always latest' reference, and the question id is the latest non-draft version
+ * of that question_bank_entry.
+ *
+ * @param array $questionids a list of question ids to check.
+ * @return array a list of the question ids from the input array which are referenced.
+ */
+ public static function questions_with_references(array $questionids): array {
+ global $DB;
+
+ if (empty($questionids)) {
+ return [];
+ }
+
+ [$qidtest, $params] = $DB->get_in_or_equal($questionids, SQL_PARAMS_NAMED, 'outerqid');
+ [$lqidtest, $lparams] = $DB->get_in_or_equal($questionids, SQL_PARAMS_NAMED, 'innerqid');
+
+ return $DB->get_fieldset_sql("
+ SELECT qv.questionid
+
+ FROM {question_versions} qv
+
+ -- This is a performant to get the latest non-draft version for each
+ -- question_bank_entry that relates to one of our questionids.
+ LEFT JOIN (
+ SELECT lqv.questionbankentryid,
+ MAX(lv.version) AS latestusableversion
+ FROM {question_versions} lqv
+ JOIN {question_versions} lv ON lv.questionbankentryid = lqv.questionbankentryid
+ WHERE lqv.questionid $lqidtest
+ AND lv.status <> :draft
+ GROUP BY lqv.questionbankentryid
+ ) latestversions ON latestversions.questionbankentryid = qv.questionbankentryid
+
+ JOIN {question_references} qr ON qr.questionbankentryid = qv.questionbankentryid
+ AND (qr.version = qv.version OR qr.version IS NULL AND qv.version = latestversions.latestusableversion)
+
+ WHERE qv.questionid $qidtest
+ ", array_merge($params, $lparams, ['draft' => question_version_status::QUESTION_STATUS_DRAFT]));
+ }
+}
diff --git a/question/engine/tests/question_reference_manager_test.php b/question/engine/tests/question_reference_manager_test.php
new file mode 100644
index 00000000000..69d104aa157
--- /dev/null
+++ b/question/engine/tests/question_reference_manager_test.php
@@ -0,0 +1,123 @@
+.
+
+namespace core_question;
+
+use advanced_testcase;
+use context_system;
+use core_question\local\bank\question_version_status;
+use core_question_generator;
+
+/**
+ * Unit tests for the {@see question_reference_manager} class.
+ *
+ * @package core_question
+ * @category test
+ * @copyright 2011 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @covers \core_question\question_reference_manager
+ */
+class question_reference_manager_test extends advanced_testcase {
+
+ public function test_questions_with_references() {
+ global $DB;
+ $this->resetAfterTest();
+
+ /** @var core_question_generator $questiongenerator */
+ $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
+ $systemcontext = context_system::instance();
+
+ // Create three questions, each with three versions.
+ // In each case, the third version is draft.
+ $cat = $questiongenerator->create_question_category();
+ $q1v1 = $questiongenerator->create_question('truefalse', null, ['name' => 'Q1V1', 'category' => $cat->id]);
+ $q1v2 = $questiongenerator->update_question($q1v1, null, ['name' => 'Q1V2']);
+ $q1v3 = $questiongenerator->update_question($q1v2, null,
+ ['name' => 'Q1V3', 'status' => question_version_status::QUESTION_STATUS_DRAFT]);
+ $q2v1 = $questiongenerator->create_question('truefalse', null, ['name' => 'Q2V1', 'category' => $cat->id]);
+ $q2v2 = $questiongenerator->update_question($q2v1, null, ['name' => 'Q2V2']);
+ $q2v3 = $questiongenerator->update_question($q2v2, null,
+ ['name' => 'Q2V3', 'status' => question_version_status::QUESTION_STATUS_DRAFT]);
+ $q3v1 = $questiongenerator->create_question('truefalse', null, ['name' => 'Q3V1', 'category' => $cat->id]);
+ $q3v2 = $questiongenerator->update_question($q3v1, null, ['name' => 'Q3V2']);
+ $q3v3 = $questiongenerator->update_question($q3v2, null,
+ ['name' => 'Q3V3', 'status' => question_version_status::QUESTION_STATUS_DRAFT]);
+
+ // Create specific references to Q2V1 and Q2V3.
+ $DB->insert_record('question_references', ['usingcontextid' => $systemcontext->id,
+ 'component' => 'core_question', 'questionarea' => 'test', 'itemid' => 0,
+ 'questionbankentryid' => $q2v1->questionbankentryid, 'version' => 1]);
+ $DB->insert_record('question_references', ['usingcontextid' => $systemcontext->id,
+ 'component' => 'core_question', 'questionarea' => 'test', 'itemid' => 1,
+ 'questionbankentryid' => $q2v1->questionbankentryid, 'version' => 3]);
+
+ // Create an always-latest reference to Q3.
+ $DB->insert_record('question_references', ['usingcontextid' => $systemcontext->id,
+ 'component' => 'core_question', 'questionarea' => 'test', 'itemid' => 2,
+ 'questionbankentryid' => $q3v1->questionbankentryid, 'version' => null]);
+
+ // Verify which versions of Q1 are used.
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([$q1v1->id]));
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([$q1v2->id]));
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([$q1v3->id]));
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([$q1v1->id, $q1v2->id, $q1v3->id]));
+
+ // Verify which versions of Q2 are used.
+ $this->assertEqualsCanonicalizing([$q2v1->id],
+ question_reference_manager::questions_with_references([$q2v1->id]));
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([$q2v2->id]));
+ $this->assertEqualsCanonicalizing([$q2v3->id],
+ question_reference_manager::questions_with_references([$q2v3->id]));
+ $this->assertEqualsCanonicalizing([$q2v1->id, $q2v3->id],
+ question_reference_manager::questions_with_references([$q2v1->id, $q2v2->id, $q2v3->id]));
+
+ // Verify which versions of Q1 are used.
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([$q3v1->id]));
+ $this->assertEqualsCanonicalizing([$q3v2->id],
+ question_reference_manager::questions_with_references([$q3v2->id]));
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([$q3v3->id]));
+ $this->assertEqualsCanonicalizing([$q3v2->id],
+ question_reference_manager::questions_with_references([$q3v1->id, $q3v2->id, $q3v3->id]));
+
+ // Do some combined queries.
+ $this->assertEqualsCanonicalizing([$q2v1->id, $q2v3->id, $q3v2->id],
+ question_reference_manager::questions_with_references([
+ $q1v1->id, $q1v2->id, $q1v3->id,
+ $q2v1->id, $q2v2->id, $q2v3->id,
+ $q3v1->id, $q3v2->id, $q3v3->id]));
+ $this->assertEqualsCanonicalizing([$q2v1->id, $q2v3->id, $q3v2->id],
+ question_reference_manager::questions_with_references([$q2v1->id, $q2v3->id, $q3v2->id]));
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([
+ $q1v1->id, $q1v2->id, $q1v3->id,
+ $q2v2->id,
+ $q3v1->id, $q3v3->id]));
+
+ // Test some edge cases.
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([]));
+ $this->assertEqualsCanonicalizing([],
+ question_reference_manager::questions_with_references([-1]));
+
+ }
+}
diff --git a/question/tests/generator/lib.php b/question/tests/generator/lib.php
index 05427126e45..a1d3deb6be8 100644
--- a/question/tests/generator/lib.php
+++ b/question/tests/generator/lib.php
@@ -118,11 +118,12 @@ class core_question_generator extends component_generator_base {
* @param string $which as for the corresponding argument of
* {@link question_test_helper::get_question_form_data}. null for the default one.
* @param array|stdClass $overrides any fields that should be different from the base example.
- * @return stdClass the question data.
+ * @return stdClass the question data, including version info and questionbankentryid
*/
public function update_question($question, $which = null, $overrides = null) {
global $CFG, $DB;
require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
+ $question = clone($question);
$qtype = $question->qtype;
@@ -144,6 +145,11 @@ class core_question_generator extends component_generator_base {
}
$DB->update_record('question', $question);
}
+ $questionversion = $DB->get_record('question_versions', ['questionid' => $question->id], '*', MUST_EXIST);
+ $question->versionid = $questionversion->id;
+ $question->questionbankentryid = $questionversion->questionbankentryid;
+ $question->version = $questionversion->version;
+ $question->status = $questionversion->status;
return $question;
}
diff --git a/question/tests/version_test.php b/question/tests/version_test.php
index 43c6c2e2ed8..2a431637a95 100644
--- a/question/tests/version_test.php
+++ b/question/tests/version_test.php
@@ -16,6 +16,7 @@
namespace core_question;
+use core_question\local\bank\question_version_status;
use question_bank;
/**
@@ -89,7 +90,7 @@ class version_test extends \advanced_testcase {
$this->assertEquals($questionversion->questionbankentryid, $questiondefinition->questionbankentryid);
// If a question is updated, a new version should be created.
- $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
+ $question = $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
$newquestiondefinition = question_bank::load_question($question->id);
// The version should be 2.
$this->assertEquals('2', $newquestiondefinition->version);
@@ -112,7 +113,7 @@ class version_test extends \advanced_testcase {
$questionfirstversionid = $question->id;
// Create a new version and try to remove it.
- $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
+ $question = $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
// The new version and bank entry record should exist.
$sql = "SELECT q.id, qv.id AS versionid, qv.questionbankentryid
@@ -158,12 +159,14 @@ class version_test extends \advanced_testcase {
* @covers ::question_delete_question
*/
public function test_delete_question_in_use() {
+ global $DB;
+
$qcategory = $this->qgenerator->create_question_category(['contextid' => $this->context->id]);
$question = $this->qgenerator->create_question('shortanswer', null, ['category' => $qcategory->id]);
$questionfirstversionid = $question->id;
// Create a new version and try to remove it after adding it to a quiz.
- $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
+ $question = $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
// Add it to the quiz.
quiz_add_quiz_question($question->id, $this->quiz);
@@ -173,11 +176,13 @@ class version_test extends \advanced_testcase {
// Try to delete old version.
question_delete_question($questionfirstversionid);
- // The questions should exist even after trying to remove it.
- $questionversion1 = question_bank::load_question($question->id);
- $questionversion2 = question_bank::load_question($questionfirstversionid);
- $this->assertEquals($questionversion1->id, $question->id);
- $this->assertEquals($questionversion2->id, $questionfirstversionid);
+ // The used question version should exist even after trying to remove it, but now hidden.
+ $questionversion2 = question_bank::load_question($question->id);
+ $this->assertEquals($question->id, $questionversion2->id);
+ $this->assertEquals(question_version_status::QUESTION_STATUS_HIDDEN, $questionversion2->status);
+
+ // The unused version should be completely gone.
+ $this->assertFalse($DB->record_exists('question', ['id' => $questionfirstversionid]));
}
/**
@@ -233,10 +238,10 @@ class version_test extends \advanced_testcase {
$questionid1 = $question->id;
// Create a new version and try to remove it after adding it to a quiz.
- $this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
+ $question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
$questionid2 = $question->id;
// Change the id number and get the question object.
- $this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
+ $question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
$questionid3 = $question->id;
// The new version and bank entry record should exist.
@@ -270,10 +275,10 @@ class version_test extends \advanced_testcase {
$questionid1 = $question->id;
// Create a new version.
- $this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
+ $question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
$questionid2 = $question->id;
// Change the id number and get the question object.
- $this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
+ $question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
$questionid3 = $question->id;
$questiondefinition = question_bank::get_all_versions_of_question($question->id);
diff --git a/question/upgrade.txt b/question/upgrade.txt
index d477f1a797c..ab40d694f66 100644
--- a/question/upgrade.txt
+++ b/question/upgrade.txt
@@ -1,5 +1,16 @@
This files describes API changes for code that uses the question API.
+=== 4.1.4 ===
+
+1) The core_question_generator::update_question has been changed so that it no longer modifies the $question
+ object that was passed in. Instead, the update question is returned (which was already the case).
+ If you were relying on the old behavioru in your tests, you will need a change like
+ $questiongenerator->update_question($question, ...);
+ to
+ $question = $questiongenerator->update_question($question, ...);
+ Also, the $question object returned now has fields questionbankentryid, versionid, version and status.
+
+
=== 4.1 ===
1) get_bulk_action_key() in core_question\local\bank\bulk_action_base class is deprecated and renamed to get_key().