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

MDL-84591 questions: Save question in use when a course is deleted

Browse Source 
question_category_delete_safe checks if questions in the category to be
deleted are in use, and if so, saves them to a new qbank instance to
avoid breaking existing quizzes.

However, if the whole course is being deleted, the qbank is created in
the current course as question_category_delete_safe does not know is is
about to be deleted, and the questions are lost when the deletion
occurs.

This change passes down a flag to question_category_delete_safe so it
knows it was called as part of a whole course deletion, and moves saved
questions to the site course instead of the current course.
This commit is contained in:
Mark Johnson
2025-03-11 11:00:40 +00:00
parent 8cb5d000a1
commit c9828a8105
3 changed files with 58 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/moodlelib.php
+3 -2
View File
@@ -4735,11 +4735,12 @@ function delete_course($courseorid, $showfeedback = true) {
* @param int $courseid The id of the course that is being deleted
* @param bool $showfeedback Whether to display notifications of each action the function performs.
* @param array $options extra options
* @param bool $coursedeletion Are we calling this as part of deleting the course?
* @return bool true if all the removals succeeded. false if there were any failures. If this
* method returns false, some of the removals will probably have succeeded, and others
* failed, but you have no way of knowing which.
*/
function remove_course_contents($courseid, $showfeedback = true, ?array $options = null) {
function remove_course_contents($courseid, $showfeedback = true, ?array $options = null, bool $coursedeletion = true) {
global $CFG, $DB, $OUTPUT;
require_once($CFG->libdir.'/badgeslib.php');
@@ -4822,7 +4823,7 @@ function remove_course_contents($courseid, $showfeedback = true, ?array $options
foreach ($instances as $cm) {
if ($cm->id) {
// Delete activity context questions and question categories.
question_delete_activity($cm);
question_delete_activity($cm, coursedeletion: $coursedeletion);
// Notify the competency subsystem.
\core_competency\api::hook_course_module_deleted($cm);
lib/questionlib.php
+9 -6
View File
@@ -233,8 +233,9 @@ function match_grade_options($gradeoptionsfull, $grade, $matchgrades = 'error')
* NOTE: this function is called from lib/db/upgrade.php
*
* @param object|core_course_category $category course category object
* @param bool $coursedeletion Is the course this category is under being deleted? If so, move saved questions to the site course.
*/
function question_category_delete_safe($category): void {
function question_category_delete_safe($category, bool $coursedeletion = false): void {
global $DB;
$criteria = ['questioncategoryid' => $category->id];
$context = context::instance_by_id($category->contextid, IGNORE_MISSING);
@@ -270,7 +271,7 @@ function question_category_delete_safe($category): void {
if ($context !== false) {
$name = $context->get_context_name();
$parentcontext = $context->get_course_context(false);
$course = $parentcontext ? get_course($parentcontext->instanceid) : get_site();
$course = ($parentcontext && !$coursedeletion) ? get_course($parentcontext->instanceid) : get_site();
}
$qbank = core_question\local\bank\question_bank_helper::get_default_open_instance_system_type($course, true);
question_save_from_deletion(array_keys($questionids), $qbank->context->id, $name, $rescue);
@@ -422,9 +423,10 @@ function question_delete_question($questionid): void {
* All question categories and their questions are deleted for this context id.
*
* @param int $contextid The contextid to delete question categories from
* @param bool $coursedeletion Are we calling this as part of deleting the course the context is under?
* @return array only returns an empty array for backwards compatibility.
*/
function question_delete_context($contextid): array {
function question_delete_context($contextid, bool $coursedeletion = false): array {
global $DB;
$fields = 'id, parent, name, contextid';
@@ -432,7 +434,7 @@ function question_delete_context($contextid): array {
// Sort categories following their tree (parent-child) relationships this will make the feedback more readable.
$categories = sort_categories_by_tree($categories);
foreach ($categories as $category) {
question_category_delete_safe($category);
question_category_delete_safe($category, $coursedeletion);
}
}
return [];
@@ -481,11 +483,12 @@ function question_save_from_deletion($questionids, $newcontextid, $oldplace, $ne
*
* @param object $cm the course module object representing the activity
* @param bool $notused the argument is not used any more. Kept for backwards compatibility.
* @param bool $coursedeletion Are we calling this as part of deleting the course the activity belongs to?
* @return boolean
*/
function question_delete_activity($cm, $notused = false): bool {
function question_delete_activity($cm, $notused = false, bool $coursedeletion = false): bool {
$modcontext = context_module::instance($cm->id);
question_delete_context($modcontext->id);
question_delete_context($modcontext->id, $coursedeletion);
return true;
}
lib/tests/questionlib_test.php
+46 -7
View File
@@ -342,27 +342,66 @@ final class questionlib_test extends \advanced_testcase {
}
/**
* This function tests the question_category_delete_safe function.
* Test parameters for calling question_category_delete_safe
*
* @return array
*/
public function test_question_category_delete_safe(): void {
public static function delete_category_parameters(): array {
return [
'Delete category' => [
'coursedeletion' => false,
],
'Delete category with course' => [
'coursedeletion' => true,
],
];
}
/**
* This function tests the question_category_delete_safe function.
*
* @param bool $coursedeletion If true, simulate calling question_category_delete_safe as part of deletion of the whole course.
* @dataProvider delete_category_parameters
* @covers ::question_category_delete_safe
*/
public function test_question_category_delete_safe(bool $coursedeletion): void {
global $DB;
$this->resetAfterTest(true);
$this->setAdminUser();
list($category, $course, $quiz, $qcat, $questions) = $this->setup_quiz_and_questions();
[, $course, , $qcat, $questions] = $this->setup_quiz_and_questions();
question_category_delete_safe($qcat);
$targetcourseid = $coursedeletion ? SITEID : $course->id;
question_category_delete_safe($qcat, $coursedeletion);
// Verify category deleted.
$criteria = array('id' => $qcat->id);
$criteria = ['id' => $qcat->id];
$this->assertEquals(0, $DB->count_records('question_categories', $criteria));
// Verify questions deleted or moved.
$this->assert_category_contains_questions($qcat->id, 0);
// Verify question not deleted.
$criteria = array('id' => $questions[0]->id);
$this->assertEquals(1, $DB->count_records('question', $criteria));
$criteria = ['id' => $questions[0]->id];
$savedquestion = $DB->get_record_sql(
"SELECT q.*, qbe.questioncategoryid
FROM {question} q
JOIN {question_versions} qv ON qv.questionid = q.id
JOIN {question_bank_entries} qbe ON qv.questionbankentryid = qbe.id",
$criteria
);
$this->assertNotEmpty($savedquestion);
// Verify question now sits in a system qbank in the target course.
$this->assertNotEquals($qcat->id, $savedquestion->id);
$newcategory = $DB->get_record('question_categories', ['id' => $savedquestion->questioncategoryid], strictness: MUST_EXIST);
$newcategorycontext = context::instance_by_id($newcategory->contextid);
$this->assertEquals(\context_module::LEVEL, $newcategorycontext->contextlevel);
[$newcourse, $newcm] = get_course_and_cm_from_cmid($newcategorycontext->instanceid);
$this->assertEquals($newcm->modname, 'qbank');
$this->assertEquals(question_bank_helper::TYPE_SYSTEM, $DB->get_field('qbank', 'type', ['id' => $newcm->instance]));
$this->assertEquals($targetcourseid, $newcourse->id);
}
/**