From 25596a50e568c8364272819bd39c8fa4bad9e6d2 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Tue, 25 Apr 2023 18:04:11 +0100 Subject: [PATCH] MDL-78025 questions: questions_in_use should check question_references This avoids the needs for plugins to do separate queries, which is easier for them, and better performing. --- lib/questionlib.php | 5 + .../classes/question_reference_manager.php | 77 +++++++++++ .../tests/question_reference_manager_test.php | 123 ++++++++++++++++++ 3 files changed, 205 insertions(+) create mode 100644 question/classes/question_reference_manager.php create mode 100644 question/engine/tests/question_reference_manager_test.php diff --git a/lib/questionlib.php b/lib/questionlib.php index c1a432dd8d3..18a6933e5c6 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -29,6 +29,7 @@ */ use core_question\local\bank\question_version_status; +use core_question\question_reference_manager; defined('MOODLE_INTERNAL') || die(); @@ -123,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) { 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])); + + } +}