Merge branch 'MDL-78025_401' of https://github.com/timhunt/moodle into MOODLE_401_STABLE

This commit is contained in:
Ilya Tregubov 2023-05-02 11:43:30 +08:00
commit d28386a29d
9 changed files with 255 additions and 39 deletions

View File

@ -1475,7 +1475,7 @@
<FIELD NAME="questionarea" TYPE="char" LENGTH="50" NOTNULL="false" SEQUENCE="false" COMMENT="Depending on the component, which area the question is used in (e.g. slot for quiz)."/>
<FIELD NAME="itemid" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="Plugin specific id (e.g. slotid for quiz) where its used."/>
<FIELD NAME="questionbankentryid" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="ID of the question bank entry this question is part of."/>
<FIELD NAME="version" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="Version number for the question where NULL means use the latest ready version."/>
<FIELD NAME="version" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="Version number for the question where NULL means use the latest non-draft version."/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id"/>

View File

@ -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)) {

View File

@ -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'));
}
/**

View File

@ -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);

View File

@ -0,0 +1,77 @@
<?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_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]));
}
}

View File

@ -0,0 +1,123 @@
<?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 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]));
}
}

View File

@ -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;
}

View File

@ -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);

View File

@ -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().