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

Merge branch 'MDL-85721_405_STABLE' of https://github.com/marxjohnson/moodle into MOODLE_405_STABLE

Browse Source
This commit is contained in:
Mihail Geshoski
2025-07-09 10:20:09 +08:00
parent 4c406b2d81 e721309453
commit 0f5f7c8dbe
14 changed files with 871 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
question/type/multianswer/classes/task/cleanup_duplicate_subquestions.php
+168
View File
@@ -0,0 +1,168 @@
<?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 qtype_multianswer\task;
use context_system;
use core\task\stored_progress_task_trait;
use core_question\local\bank\question_version_status;
use question_bank;
use question_engine_data_mapper;
/**
* Cleanup duplicate subquestions
*
* Due to MDL-85721, there may be duplicated subquestions in the database. These have a question bank entry, question version,
* and question record with a parent, but they are not referred to in that parent's sequence.
*
* @package qtype_multianswer
* @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 cleanup_duplicate_subquestions extends \core\task\adhoc_task {
/**
* @var int The current progress counter.
*/
protected int $counter;
/**
* Find questions where there are other questions with identical text, stamp and multianswer parent
*
* We may have multiple subquestions with the same stamp but different text or parents due to historical bugs,
* so this includes the ID field from one of the duplicates to ensure we have a unique first field.
*
* @return array
*/
public function find_duplicated_subquestions(): array {
global $DB;
return $DB->get_records_sql("
SELECT MIN(subq.id) AS firstid, subq.stamp, subq.questiontext, subq.parent, qm.sequence, COUNT(1) AS count
FROM {question} subq
JOIN {question} q ON q.id = subq.parent
JOIN {question_multianswer} qm ON q.id = qm.question
WHERE q.qtype = 'multianswer'
GROUP BY subq.stamp, subq.questiontext, subq.parent, qm.sequence
HAVING COUNT(1) > 1;
");
}
/**
* Output a progress dot, with a count of the total every 70 items.
*/
protected function increment_progress(): void {
mtrace('.', '');
$this->counter++;
if ($this->counter % 70 === 0) {
mtrace(" {$this->counter}");
}
}
#[\Override]
public function execute() {
global $CFG, $DB;
require_once($CFG->libdir . '/questionlib.php');
$duplicatedsubquestions = $this->find_duplicated_subquestions();
$duplicatedcount = count($duplicatedsubquestions);
if ($duplicatedcount === 0) {
mtrace("No duplicated questions found.");
return;
}
mtrace("Found {$duplicatedcount} subquestions with duplicates.");
foreach ($duplicatedsubquestions as $subquestion) {
// Find instances of the subquestion that do not appear in the sequence of the parent.
[$insql, $inparams] = $DB->get_in_or_equal(explode(',', $subquestion->sequence), equal: false);
$params = array_merge([$subquestion->parent, $subquestion->stamp], $inparams);
$duplicates = $DB->get_records_select('question', "parent = ? AND stamp = ? AND id {$insql}", $params);
$duplicatecount = count($duplicates);
// Delete each duplicate, with a progress bar.
mtrace("");
mtrace("Deleting {$duplicatecount} duplicates of {$subquestion->stamp}:");
$this->counter = 0;
foreach ($duplicates as $duplicate) {
// Based on question_delete_question(), without checking for parent usage or deleting children.
// If the question is being used, just mark it as hidden. Otherwise, delete the question, version and question bank
// entry.
$sql = "SELECT qv.id as versionid,
qv.version,
qbe.id as entryid,
qc.id as categoryid,
ctx.id as contextid
FROM {question} q
LEFT JOIN {question_versions} qv ON qv.questionid = q.id
LEFT JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid
LEFT JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid
LEFT JOIN {context} ctx ON ctx.id = qc.contextid
WHERE q.id = ?";
$questiondata = $DB->get_record_sql($sql, [$duplicate->id]);
// Do not delete a question if it is used by an activity module. Just mark the version hidden.
if (questions_in_use([$duplicate->id])) {
$DB->set_field(
'question_versions',
'status',
question_version_status::QUESTION_STATUS_HIDDEN,
['questionid' => $duplicate->id]
);
$this->increment_progress();
continue;
}
// This sometimes happens in old sites with bad data.
if (!$questiondata->contextid) {
debugging('Deleting question ' . $duplicate->id . ' which is no longer linked to a context. ' .
'Assuming system context to avoid errors, but this may mean that some data like files, ' .
'tags, are not cleaned up.');
$questiondata->contextid = context_system::instance()->id;
$questiondata->categoryid = 0;
}
// Delete previews of the question.
$dm = new question_engine_data_mapper();
$dm->delete_previews($duplicate->id);
// Delete questiontype-specific data.
question_bank::get_qtype($duplicate->qtype, false)->delete_question($duplicate->id, $questiondata->contextid);
// Finally delete the question record itself.
$DB->delete_records('question', ['id' => $duplicate->id]);
$DB->delete_records('question_versions', ['id' => $questiondata->versionid]);
$DB->delete_records('question_references',
[
'version' => $questiondata->version,
'questionbankentryid' => $questiondata->entryid,
]);
delete_question_bank_entry($questiondata->entryid);
question_bank::notify_question_edited($duplicate->id);
// Log the deletion of this question.
$duplicate->category = $questiondata->categoryid;
$duplicate->contextid = $questiondata->contextid;
$event = \core\event\question_deleted::create_from_question_instance($duplicate);
$event->add_record_snapshot('question', $duplicate);
$event->trigger();
$this->increment_progress();
}
}
}
}
question/type/multianswer/db/upgrade.php
+9
View File
@@ -43,5 +43,14 @@ function xmldb_qtype_multianswer_upgrade($oldversion) {
// Automatically generated Moodle v4.5.0 release upgrade line.
// Put any upgrade step following this.
if ($oldversion < 2024100701) {
$task = new \qtype_multianswer\task\cleanup_duplicate_subquestions();
if (count($task->find_duplicated_subquestions()) > 0) {
mtrace('Duplicated subquestions found. Queueing cleanup task.');
\core\task\manager::queue_adhoc_task($task);
}
upgrade_plugin_savepoint(true, 2024100701, 'qtype', 'multianswer');
}
return true;
}
question/type/multianswer/questiontype.php
+5 -23
View File
@@ -162,9 +162,9 @@ class qtype_multianswer extends question_type {
// This function needs to be able to handle the case where the existing set of wrapped
// questions does not match the new set of wrapped questions so that some need to be
// created, some modified and some deleted.
// Unfortunately the code currently simply overwrites existing ones in sequence. This
// will make re-marking after a re-ordering of wrapped questions impossible and
// will also create difficulties if questiontype specific tables reference the id.
// Thanks to versioning, we no longer overwrite existing questions in the sequence
// by re-using IDs, but instead create a new version of each wrapped question for
// the new version of the parent.
// First we get all the existing wrapped questions.
$oldwrappedquestions = [];
@@ -186,30 +186,12 @@ class qtype_multianswer extends question_type {
$sequence = array();
foreach ($question->options->questions as $wrapped) {
if (!empty($wrapped)) {
// If we still have some old wrapped question ids, reuse the next of them.
// If we still have some old wrapped question ids, reuse the next of them to save
// the new version against its question bank entry.
$wrapped->id = 0;
if (is_array($oldwrappedquestions) &&
$oldwrappedquestion = array_shift($oldwrappedquestions)) {
$wrapped->oldid = $oldwrappedquestion->id;
if ($oldwrappedquestion->qtype != $wrapped->qtype) {
switch ($oldwrappedquestion->qtype) {
case 'multichoice':
$DB->delete_records('qtype_multichoice_options',
array('questionid' => $oldwrappedquestion->id));
break;
case 'shortanswer':
$DB->delete_records('qtype_shortanswer_options',
array('questionid' => $oldwrappedquestion->id));
break;
case 'numerical':
$DB->delete_records('question_numerical',
array('question' => $oldwrappedquestion->id));
break;
default:
throw new moodle_exception('qtypenotrecognized',
'qtype_multianswer', '', $oldwrappedquestion->qtype);
}
}
}
}
$wrapped->name = $question->name;
question/type/multianswer/tests/question_type_test.php
+59
View File
@@ -20,7 +20,9 @@ use qtype_multianswer;
use qtype_multianswer_edit_form;
use qtype_multichoice_base;
use question_bank;
use stdClass;
use test_question_maker;
use core\context;
defined('MOODLE_INTERNAL') || die();
@@ -79,11 +81,13 @@ final class question_type_test extends \advanced_testcase {
$q->timemodified = time();
$q->createdby = $USER->id;
$q->modifiedby = $USER->id;
$q->options = new stdClass();
$sadata = new \stdClass();
$sadata->id = 1;
$sadata->qtype = 'shortanswer';
$sadata->defaultmark = 1;
$sadata->options = new stdClass();
$sadata->options->usecase = true;
$sadata->options->answers[1] = (object) array('answer' => 'Bow-wow', 'fraction' => 0);
$sadata->options->answers[2] = (object) array('answer' => 'Wiggly worm', 'fraction' => 0);
@@ -93,6 +97,7 @@ final class question_type_test extends \advanced_testcase {
$mcdata->id = 1;
$mcdata->qtype = 'multichoice';
$mcdata->defaultmark = 1;
$mcdata->options = new stdClass();
$mcdata->options->single = true;
$mcdata->options->answers[1] = (object) array('answer' => 'Dog', 'fraction' => 0);
$mcdata->options->answers[2] = (object) array('answer' => 'Owl', 'fraction' => 1);
@@ -434,4 +439,58 @@ final class question_type_test extends \advanced_testcase {
$this->assertCount(2, $questiondata->options->questions);
$this->assertEquals('subquestion_replacement', $questiondata->options->questions[$questiontodeletekey]->qtype);
}
/**
* Saving a new version of the question should retain the original subquestion versions, with their own qtype data.
*/
public function test_save_question_options(): void {
global $DB;
$this->resetAfterTest(true);
$this->setAdminUser();
$generator = $this->getDataGenerator()->get_plugin_generator('core_question');
$cat = $generator->create_question_category([]);
$question = $generator->create_question('multianswer', 'twosubq', ['category' => $cat->id]);
get_question_options($question);
$originalsubq1 = reset($question->options->questions);
$originalsubq2 = next($question->options->questions);
// Assert that the original subquestions are the expected types, and they have options records.
$this->assertEquals('shortanswer', $originalsubq1->qtype);
$this->assertTrue($DB->record_exists('qtype_shortanswer_options', ['questionid' => $originalsubq1->id]));
$this->assertEquals('multichoice', $originalsubq2->qtype);
$this->assertTrue($DB->record_exists('qtype_multichoice_options', ['questionid' => $originalsubq2->id]));
// Edit the question, replacing the subquestions with two new questions of different types.
$editedquestion = test_question_maker::get_question_data('multianswer', 'twosubq');
$editedquestion->id = $question->id;
$editedquestion->category = $cat->id;
$editedquestion->context = context::instance_by_id($cat->contextid);
$editedsubq1 = test_question_maker::get_question_form_data('multichoice', 'one_of_four');
$editedsubq1->id = $originalsubq1->id;
$editedsubq1->qtype = 'multichoice';
$editedsubq2 = test_question_maker::get_question_form_data('shortanswer', 'frogtoad');
$editedsubq2->id = $originalsubq2->id;
$editedsubq2->qtype = 'shortanswer';
$editedquestion->options->questions = [$editedsubq1, $editedsubq2];
$this->qtype->save_question_options($editedquestion);
$newquestion = $DB->get_record('question', ['id' => $question->id]);
get_question_options($newquestion);
$newsubq1 = reset($newquestion->options->questions);
$newsubq2 = next($newquestion->options->questions);
// The new subquestions are different types, and did not re-use IDs from the original subquestions.
$this->assertEquals('multichoice', $newsubq1->qtype);
$this->assertFalse(in_array($newsubq1->id, [$originalsubq1->id, $originalsubq2->id]));
$this->assertEquals('shortanswer', $newsubq2->qtype);
$this->assertFalse(in_array($newsubq2->id, [$originalsubq1->id, $originalsubq2->id]));
// The original questions and option records still exist.
$this->assertTrue($DB->record_exists('question', ['id' => $originalsubq1->id]));
$this->assertTrue($DB->record_exists('qtype_shortanswer_options', ['questionid' => $originalsubq1->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $originalsubq2->id]));
$this->assertTrue($DB->record_exists('qtype_multichoice_options', ['questionid' => $originalsubq2->id]));
}
}
question/type/multianswer/tests/task/cleanup_duplicate_subquestions_test.php
+310
View File
@@ -0,0 +1,310 @@
<?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 qtype_multianswer\task;
/**
* Unit tests for cleanup_duplicate_subquestions
*
* @package qtype_multianswer
* @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 \qtype_multianswer\task\cleanup_duplicate_subquestions
*/
final class cleanup_duplicate_subquestions_test extends \advanced_testcase {
/**
* Create a multianswer question and duplicate its subquestions.
*
* @return array
* @throws \dml_exception
*/
protected function generate_duplicated_subquestions(): array {
global $DB;
$generator = $this->getDataGenerator()->get_plugin_generator('core_question');
$category = $generator->create_question_category();
$question = $generator->create_question('multianswer', 'twosubq', ['category' => $category->id]);
$subquestions = $DB->get_records('question', ['parent' => $question->id]);
foreach ($subquestions as $subquestion) {
$version = $DB->get_record('question_versions', ['questionid' => $subquestion->id]);
$qbe = $DB->get_record('question_bank_entries', ['id' => $version->questionbankentryid]);
$duplicate = clone($subquestion);
unset($duplicate->id);
$duplicate->id = $DB->insert_record('question', $duplicate);
$duplicateqbe = clone($qbe);
unset($duplicateqbe->id);
$duplicateqbe->id = $DB->insert_record('question_bank_entries', $duplicateqbe);
$duplicateversion = clone($version);
unset($duplicateversion->id);
$duplicateversion->questionid = $duplicate->id;
$duplicateversion->questionbankentryid = $duplicateqbe->id;
$duplicateversion->id = $DB->insert_record('question_versions', $duplicateversion);
$subquestion->duplicate = (object) [
'question' => $duplicate,
'version' => $duplicateversion,
'questionbankentry' => $duplicateqbe,
];
}
return $subquestions;
}
/**
* We should correctly find subquestions with duplicate records.
*/
public function test_find_duplicated_subquestions(): void {
$this->resetAfterTest();
$task = new cleanup_duplicate_subquestions();
$generator = $this->getDataGenerator()->get_plugin_generator('core_question');
$category = $generator->create_question_category();
$generator->create_question('multianswer', 'twosubq', ['category' => $category->id]);
$this->assertEquals(0, count($task->find_duplicated_subquestions()));
$this->generate_duplicated_subquestions();
$this->assertEquals(2, count($task->find_duplicated_subquestions()));
}
/**
* Delete duplicate subquestions, but not the originals.
*/
public function test_execute(): void {
global $DB;
$this->resetAfterTest();
$task = new cleanup_duplicate_subquestions();
$generator = $this->getDataGenerator()->get_plugin_generator('core_question');
$category = $generator->create_question_category();
$normalquestion = $generator->create_question('multianswer', 'twosubq', ['category' => $category->id]);
$normalsubquestions = $DB->get_records('question', ['parent' => $normalquestion->id]);
$duplicatedsubquestions = $this->generate_duplicated_subquestions();
foreach ($duplicatedsubquestions as $subquestion) {
$this->expectOutputRegex("~{$subquestion->stamp}~");
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->parent]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->duplicate->question->id]));
$this->assertTrue($DB->record_exists('question_versions', ['id' => $subquestion->duplicate->version->id]));
$this->assertTrue(
$DB->record_exists('question_bank_entries', ['id' => $subquestion->duplicate->questionbankentry->id]),
);
}
$this->expectOutputRegex('~Found 2 subquestions with duplicates~');
$task->execute();
// The non-duplicated questions should not have been touched.
$this->assertTrue($DB->record_exists('question', ['id' => $normalquestion->id]));
foreach ($normalsubquestions as $subquestion) {
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
}
// The duplicated questions should have the duplicates deleted, but the originals intact.
foreach ($duplicatedsubquestions as $subquestion) {
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->parent]));
$this->assertFalse($DB->record_exists('question', ['id' => $subquestion->duplicate->question->id]));
$this->assertFalse($DB->record_exists('question_versions', ['id' => $subquestion->duplicate->version->id]));
$this->assertFalse(
$DB->record_exists('question_bank_entries', ['id' => $subquestion->duplicate->questionbankentry->id]),
);
}
}
/**
* Don't delete a duplicate subquestion if its somehow being used somewhere.
*
* This should never really happen, but just to be on the safe side.
*/
public function test_execute_with_usage(): void {
global $DB;
$this->resetAfterTest();
$task = new cleanup_duplicate_subquestions();
$course = $this->getDataGenerator()->create_course();
$quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz');
$quiz = $quizgenerator->create_instance(['course' => $course->id]);
$duplicatedsubquestions = $this->generate_duplicated_subquestions();
$firstsubquestion = reset($duplicatedsubquestions);
$secondsubquestion = next($duplicatedsubquestions);
quiz_add_quiz_question($firstsubquestion->duplicate->question->id, $quiz);
foreach ($duplicatedsubquestions as $subquestion) {
$this->expectOutputRegex("~{$subquestion->stamp}~");
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->parent]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->duplicate->question->id]));
$this->assertTrue($DB->record_exists('question_versions', ['id' => $subquestion->duplicate->version->id]));
$this->assertTrue(
$DB->record_exists('question_bank_entries', ['id' => $subquestion->duplicate->questionbankentry->id]),
);
}
$this->expectOutputRegex('~Found 2 subquestions with duplicates~');
$task->execute();
// The subquestion duplicate which was added to the quiz has not been deleted.
$this->assertTrue($DB->record_exists('question', ['id' => $firstsubquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $firstsubquestion->parent]));
$this->assertTrue($DB->record_exists('question', ['id' => $firstsubquestion->duplicate->question->id]));
$this->assertTrue($DB->record_exists('question_versions', ['id' => $firstsubquestion->duplicate->version->id]));
$this->assertTrue(
$DB->record_exists('question_bank_entries', ['id' => $firstsubquestion->duplicate->questionbankentry->id]),
);
// The subquestion duplicate which was not added to the quiz, was deleted.
$this->assertTrue($DB->record_exists('question', ['id' => $secondsubquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $secondsubquestion->parent]));
$this->assertFalse($DB->record_exists('question', ['id' => $secondsubquestion->duplicate->question->id]));
$this->assertFalse($DB->record_exists('question_versions', ['id' => $secondsubquestion->duplicate->version->id]));
$this->assertFalse(
$DB->record_exists('question_bank_entries', ['id' => $secondsubquestion->duplicate->questionbankentry->id]),
);
}
/**
* Delete a duplicate subquestion even if its parent is being used.
*/
public function test_execute_with_parent_usage(): void {
global $DB;
$this->resetAfterTest();
$task = new cleanup_duplicate_subquestions();
$course = $this->getDataGenerator()->create_course();
$quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz');
$quiz = $quizgenerator->create_instance(['course' => $course->id]);
$duplicatedsubquestions = $this->generate_duplicated_subquestions();
$firstsubquestion = reset($duplicatedsubquestions);
quiz_add_quiz_question($firstsubquestion->parent, $quiz);
foreach ($duplicatedsubquestions as $subquestion) {
$this->expectOutputRegex("~{$subquestion->stamp}~");
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->parent]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->duplicate->question->id]));
$this->assertTrue($DB->record_exists('question_versions', ['id' => $subquestion->duplicate->version->id]));
$this->assertTrue(
$DB->record_exists('question_bank_entries', ['id' => $subquestion->duplicate->questionbankentry->id]),
);
}
$this->expectOutputRegex('~Found 2 subquestions with duplicates~');
$task->execute();
// The duplicated questions should have the duplicates deleted, but the originals intact.
foreach ($duplicatedsubquestions as $subquestion) {
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->parent]));
$this->assertFalse($DB->record_exists('question', ['id' => $subquestion->duplicate->question->id]));
$this->assertFalse($DB->record_exists('question_versions', ['id' => $subquestion->duplicate->version->id]));
$this->assertFalse(
$DB->record_exists('question_bank_entries', ['id' => $subquestion->duplicate->questionbankentry->id]),
);
}
}
/**
* For historical reasons, we might have multiple different questions with the same stamp. Ensure we can handle this.
*/
public function test_execute_duplicate_stamp(): void {
global $DB;
$this->resetAfterTest();
$task = new cleanup_duplicate_subquestions();
$generator = $this->getDataGenerator()->get_plugin_generator('core_question');
$category = $generator->create_question_category();
$normalquestion = $generator->create_question('multianswer', 'twosubq', ['category' => $category->id]);
$normalsubquestions = $DB->get_records('question', ['parent' => $normalquestion->id]);
$duplicatedsubquestions1 = array_values($this->generate_duplicated_subquestions());
$duplicatedsubquestions2 = array_values($this->generate_duplicated_subquestions());
$this->expectOutputRegex("~((?!Did you remember to make the first column something unique).)*$~");
foreach ($duplicatedsubquestions1 as $key => $subquestion) {
$this->expectOutputRegex("~{$subquestion->stamp}~");
$this->expectOutputRegex("~((?!{$duplicatedsubquestions2[$key]->stamp}).)*$~");
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->parent]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->duplicate->question->id]));
$this->assertTrue($DB->record_exists('question_versions', ['id' => $subquestion->duplicate->version->id]));
$this->assertTrue(
$DB->record_exists('question_bank_entries', ['id' => $subquestion->duplicate->questionbankentry->id]),
);
$this->assertTrue($DB->record_exists('question', ['id' => $duplicatedsubquestions2[$key]->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $duplicatedsubquestions2[$key]->parent]));
$this->assertTrue($DB->record_exists('question', ['id' => $duplicatedsubquestions2[$key]->duplicate->question->id]));
$this->assertTrue(
$DB->record_exists('question_versions', ['id' => $duplicatedsubquestions2[$key]->duplicate->version->id])
);
$this->assertTrue(
$DB->record_exists(
'question_bank_entries',
['id' => $duplicatedsubquestions2[$key]->duplicate->questionbankentry->id],
),
);
// Set the stamp of the second generated subquestion and its duplicate to match the first.
$DB->update_record(
'question',
(object) [
'id' => $duplicatedsubquestions2[$key]->id,
'stamp' => $subquestion->stamp,
],
);
$DB->update_record(
'question',
(object) [
'id' => $duplicatedsubquestions2[$key]->duplicate->version->questionid,
'stamp' => $subquestion->stamp,
],
);
}
$this->expectOutputRegex('~Found 4 subquestions with duplicates~');
$task->execute();
// The non-duplicated questions should not have been touched.
$this->assertTrue($DB->record_exists('question', ['id' => $normalquestion->id]));
foreach ($normalsubquestions as $subquestion) {
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
}
// The duplicated questions should have the duplicates deleted, but the originals intact.
foreach ($duplicatedsubquestions1 as $key => $subquestion) {
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $subquestion->parent]));
$this->assertFalse($DB->record_exists('question', ['id' => $subquestion->duplicate->question->id]));
$this->assertFalse($DB->record_exists('question_versions', ['id' => $subquestion->duplicate->version->id]));
$this->assertFalse(
$DB->record_exists('question_bank_entries', ['id' => $subquestion->duplicate->questionbankentry->id]),
);
$this->assertTrue($DB->record_exists('question', ['id' => $duplicatedsubquestions2[$key]->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $duplicatedsubquestions2[$key]->parent]));
$this->assertFalse($DB->record_exists('question', ['id' => $duplicatedsubquestions2[$key]->duplicate->question->id]));
$this->assertFalse(
$DB->record_exists('question_versions', ['id' => $duplicatedsubquestions2[$key]->duplicate->version->id]));
$this->assertFalse(
$DB->record_exists(
'question_bank_entries',
['id' => $duplicatedsubquestions2[$key]->duplicate->questionbankentry->id],
),
);
}
}
}
question/type/multianswer/version.php
+1 -1
View File
@@ -26,7 +26,7 @@
defined('MOODLE_INTERNAL') || die();
$plugin->component = 'qtype_multianswer';
$plugin->version = 2024100700;
$plugin->version = 2024100701;
$plugin->requires = 2024100100;
$plugin->dependencies = [
question/type/multichoice/backup/moodle2/restore_qtype_multichoice_plugin.class.php
+15
View File
@@ -160,4 +160,19 @@ class restore_qtype_multichoice_plugin extends restore_qtype_plugin {
return $contents;
}
#[\Override]
public static function convert_backup_to_questiondata(array $backupdata): \stdClass {
global $CFG;
require_once($CFG->dirroot . '/question/type/multichoice/questiontype.php');
$questiondata = parent::convert_backup_to_questiondata($backupdata);
if (count(get_object_vars($questiondata->options)) <= 1) {
// Historically, old versions of multichoice subquestions had their options record deleted.
// As qtype_multichoice::get_question_options() sets default options in this case, we need
// to do the same here. See MDL-85721.
$defaultoptions = (new qtype_multichoice())->create_default_options($questiondata);
$questiondata->options = (object) array_merge((array) $questiondata->options, (array) $defaultoptions);
}
return $questiondata;
}
}
question/type/multichoice/questiontype.php
+1 -1
View File
@@ -88,7 +88,7 @@ class qtype_multichoice extends question_type {
* @param object $question The queston we are working with.
* @return object The options object.
*/
protected function create_default_options($question) {
public function create_default_options($question) {
// Create a default question options record.
$options = new stdClass();
$options->questionid = $question->id;
question/type/multichoice/tests/restore_test.php
+86
View File
@@ -0,0 +1,86 @@
<?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 qtype_multichoice;
/**
* Unit tests for restore_qtype_multichoice_plugin
*
* @package qtype_multichoice
* @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 \restore_qtype_multichoice_plugin
*/
final class restore_test extends \advanced_testcase {
/**
* Duplicate a quiz containing a multichoice question with no options record.
*/
public function test_restore_quiz_with_edited_questions(): void {
global $CFG, $DB, $USER;
require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php');
$this->resetAfterTest();
$this->setAdminUser();
// Create a course and a user with editing teacher capabilities.
$generator = $this->getDataGenerator();
$course1 = $generator->create_course();
$context = \context_course::instance($course1->id);
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
$initialcount = $DB->count_records('question');
// Create a question category.
$cat = $questiongenerator->create_question_category(['contextid' => $context->id]);
// Create a quiz containing a multichoice question from the qbank.
$quiz = $this->getDataGenerator()->get_plugin_generator('mod_quiz')->create_instance(['course' => $course1->id]);
$question = $questiongenerator->create_question('multichoice', 'one_of_four', ['category' => $cat->id]);
quiz_add_quiz_question($question->id, $quiz);
// Delete the multichoice_options record.
$DB->delete_records('qtype_multichoice_options', ['questionid' => $question->id]);
// Confirm we have created 1 additional question.
$this->assertEquals($initialcount + 1, $DB->count_records('question'));
// Backup quiz.
$bc = new \backup_controller(\backup::TYPE_1ACTIVITY, $quiz->cmid, \backup::FORMAT_MOODLE,
\backup::INTERACTIVE_NO, \backup::MODE_IMPORT, $USER->id);
$backupid = $bc->get_backupid();
$bc->execute_plan();
$bc->destroy();
// Restore the backup into the same course.
$rc = new \restore_controller($backupid, $course1->id, \backup::INTERACTIVE_NO, \backup::MODE_IMPORT,
$USER->id, \backup::TARGET_CURRENT_ADDING);
$rc->execute_precheck();
$rc->execute_plan();
$rc->destroy();
$this->assertDebuggingCalled();
// Both quizzes should refer to the same original question.
$quizzes = get_fast_modinfo($course1->id)->get_instances_of('quiz');
$this->assertCount(2, $quizzes);
foreach ($quizzes as $quiz) {
$structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz->instance, $quiz->context);
$this->assertEquals($structure[1]->questionid, $question->id);
}
// There should be no additional questions created during the restore.
$this->assertEquals($initialcount + 1, $DB->count_records('question'));
}
}
question/type/numerical/backup/moodle2/restore_qtype_numerical_plugin.class.php
+26 -5
View File
@@ -83,14 +83,35 @@ class restore_qtype_numerical_plugin extends restore_qtype_plugin {
#[\Override]
public static function convert_backup_to_questiondata(array $backupdata): \stdClass {
global $CFG;
require_once($CFG->dirroot . '/question/type/numerical/questiontype.php');
$questiondata = parent::convert_backup_to_questiondata($backupdata);
foreach ($backupdata['plugin_qtype_numerical_question']['numerical_records']['numerical_record'] as $record) {
foreach ($questiondata->options->answers as &$answer) {
if ($answer->id == $record['answer']) {
$answer->tolerance = $record['tolerance'];
continue 2;
if (count(get_object_vars($questiondata->options)) <= 2) {
// Old question, set defaults.
$qtype = new qtype_numerical();
$questiondata->options->unitgradingtype = 0;
$questiondata->options->unitpenalty = 0.1;
if ($qtype->get_default_numerical_unit($questiondata)) {
$questiondata->options->showunits = $qtype::UNITINPUT;
} else {
$questiondata->options->showunits = $qtype::UNITNONE;
}
$questiondata->options->unitsleft = 0;
}
if (isset($backupdata['plugin_qtype_numerical_question']['numerical_records'])) {
foreach ($backupdata['plugin_qtype_numerical_question']['numerical_records']['numerical_record'] as $record) {
foreach ($questiondata->options->answers as &$answer) {
if ($answer->id == $record['answer']) {
$answer->tolerance = $record['tolerance'];
continue 2;
}
}
}
} else {
// If the numerical record is missing (e.g. MDL-85721), default tolerances to 0.
foreach ($questiondata->options->answers as &$answer) {
$answer->tolerance = 0;
}
}
return $questiondata;
}
question/type/numerical/questiontype.php
+12 -9
View File
@@ -64,22 +64,25 @@ class qtype_numerical extends question_type {
}
public function get_question_options($question) {
global $CFG, $DB, $OUTPUT;
global $DB;
parent::get_question_options($question);
// Get the question answers and their respective tolerances
// Note: question_numerical is an extension of the answer table rather than
// the question table as is usually the case for qtype
// specific tables.
// If the numerical record is missing for some reason (e.g. MDL-85721), use a default tolerance.
if (!$question->options->answers = $DB->get_records_sql(
"SELECT a.*, n.tolerance " .
"FROM {question_answers} a, " .
" {question_numerical} n " .
"WHERE a.question = ? " .
" AND a.id = n.answer " .
"ORDER BY a.id ASC", array($question->id))) {
echo $OUTPUT->notification('Error: Missing question answer for numerical question ' .
"
SELECT a.*, COALESCE(n.tolerance, '0') AS tolerance
FROM {question_answers} a
LEFT JOIN {question_numerical} n ON a.id = n.answer
WHERE a.question = ?
ORDER BY a.id ASC
",
[$question->id],
)) {
debugging('Error: Missing question answer for numerical question ' .
$question->id . '!');
return false;
}
$question->hints = $DB->get_records('question_hints',
question/type/numerical/tests/restore_test.php
+86
View File
@@ -0,0 +1,86 @@
<?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 qtype_numerical;
/**
* Unit tests for restore_qtype_numerical_plugin
*
* @package qtype_numerical
* @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 \restore_qtype_numerical_plugin
*/
final class restore_test extends \advanced_testcase {
/**
* Duplicate a quiz containing a numerical question with no numerical record or numerical_options record.
*/
public function test_restore_quiz_with_edited_questions(): void {
global $CFG, $DB, $USER;
require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php');
$this->resetAfterTest();
$this->setAdminUser();
// Create a course and a user with editing teacher capabilities.
$generator = $this->getDataGenerator();
$course1 = $generator->create_course();
$context = \context_course::instance($course1->id);
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
$initialcount = $DB->count_records('question');
// Create a question category.
$cat = $questiongenerator->create_question_category(['contextid' => $context->id]);
// Create a quiz containing a multichoice question from the qbank.
$quiz = $this->getDataGenerator()->get_plugin_generator('mod_quiz')->create_instance(['course' => $course1->id]);
$question = $questiongenerator->create_question('numerical', 'pi', ['category' => $cat->id]);
quiz_add_quiz_question($question->id, $quiz);
// Delete the numerical and numerical_options record, so we rely on defaults.
$DB->delete_records('question_numerical', ['question' => $question->id]);
$DB->delete_records('question_numerical_options', ['question' => $question->id]);
// Confirm we have created 1 additional question.
$this->assertEquals($initialcount + 1, $DB->count_records('question'));
// Backup quiz.
$bc = new \backup_controller(\backup::TYPE_1ACTIVITY, $quiz->cmid, \backup::FORMAT_MOODLE,
\backup::INTERACTIVE_NO, \backup::MODE_IMPORT, $USER->id);
$backupid = $bc->get_backupid();
$bc->execute_plan();
$bc->destroy();
// Restore the backup into the same course.
$rc = new \restore_controller($backupid, $course1->id, \backup::INTERACTIVE_NO, \backup::MODE_IMPORT,
$USER->id, \backup::TARGET_CURRENT_ADDING);
$rc->execute_precheck();
$rc->execute_plan();
$rc->destroy();
// Both quizzes should refer to the same original question.
$quizzes = get_fast_modinfo($course1->id)->get_instances_of('quiz');
$this->assertCount(2, $quizzes);
foreach ($quizzes as $quiz) {
$structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz->instance, $quiz->context);
$this->assertEquals($structure[1]->questionid, $question->id);
}
// There should be no additional questions created during the restore.
$this->assertEquals($initialcount + 1, $DB->count_records('question'));
}
}
question/type/questiontypebase.php
+4 -6
View File
@@ -904,7 +904,7 @@ class question_type {
* specific information (it is passed by reference).
*/
public function get_question_options($question) {
global $DB, $OUTPUT;
global $DB;
if (!isset($question->options)) {
$question->options = new stdClass();
@@ -921,9 +921,8 @@ class question_type {
$question->options->$field = $extra_data->$field;
}
} else {
echo $OUTPUT->notification('Failed to load question options from the table ' .
debugging('Failed to load question options from the table ' .
$question_extension_table . ' for questionid ' . $question->id);
return false;
}
}
@@ -938,9 +937,8 @@ class question_type {
WHERE qa.question = ?
ORDER BY qa.id", array($question->id));
if (!$answers) {
echo $OUTPUT->notification('Failed to load question answers from the table ' .
$answerextensiontable . 'for questionid ' . $question->id);
return false;
debugging('Failed to load question answers from the table ' .
$answerextensiontable . ' for questionid ' . $question->id);
}
} else {
// Don't check for success or failure because some question types do
question/type/shortanswer/tests/restore_test.php
+89
View File
@@ -0,0 +1,89 @@
<?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 qtype_shortanswer;
/**
* Unit tests for restore_qtype_shortanswer_plugin
*
* @package qtype_shortanswer
* @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 \restore_qtype_shortanswer_plugin
*/
final class restore_test extends \advanced_testcase {
/**
* Duplicate a quiz containing a shortanswer question with no options record.
*/
public function test_restore_quiz_with_edited_questions(): void {
global $CFG, $DB, $USER;
require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php');
$this->resetAfterTest();
$this->setAdminUser();
// Create a course and a user with editing teacher capabilities.
$generator = $this->getDataGenerator();
$course1 = $generator->create_course();
$context = \context_course::instance($course1->id);
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
$initialcount = $DB->count_records('question');
// Create a question category.
$cat = $questiongenerator->create_question_category(['contextid' => $context->id]);
// Create a quiz containing a multichoice question from the qbank.
$quiz = $this->getDataGenerator()->get_plugin_generator('mod_quiz')->create_instance(['course' => $course1->id]);
$question = $questiongenerator->create_question('shortanswer', 'frogtoad', ['category' => $cat->id]);
quiz_add_quiz_question($question->id, $quiz);
// Delete the multichoice_options record.
$DB->delete_records('qtype_shortanswer_options', ['questionid' => $question->id]);
// Confirm we have created 1 additional question.
$this->assertEquals($initialcount + 1, $DB->count_records('question'));
// Backup quiz.
$bc = new \backup_controller(\backup::TYPE_1ACTIVITY, $quiz->cmid, \backup::FORMAT_MOODLE,
\backup::INTERACTIVE_NO, \backup::MODE_IMPORT, $USER->id);
$backupid = $bc->get_backupid();
$bc->execute_plan();
$bc->destroy();
// Restore the backup into the same course.
$rc = new \restore_controller($backupid, $course1->id, \backup::INTERACTIVE_NO, \backup::MODE_IMPORT,
$USER->id, \backup::TARGET_CURRENT_ADDING);
$rc->execute_precheck();
$rc->execute_plan();
$rc->destroy();
$this->assertDebuggingCalled(
"Failed to load question options from the table qtype_shortanswer_options for questionid {$question->id}",
);
// Both quizzes should refer to the same original question.
$quizzes = get_fast_modinfo($course1->id)->get_instances_of('quiz');
$this->assertCount(2, $quizzes);
foreach ($quizzes as $quiz) {
$structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz->instance, $quiz->context);
$this->assertEquals($structure[1]->questionid, $question->id);
}
// There should be no additional questions created during the restore.
$this->assertEquals($initialcount + 1, $DB->count_records('question'));
}
}