From fb9f3ee58cfdc7d64b94d3dc516370e19c75e442 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Thu, 13 Nov 2025 09:12:49 +0000 Subject: [PATCH] MDL-86798 questions: Restore backups containing deleted versions If the latest version of a question is deleted, and then a new one created, the new one will currently re-use the version number of the deleted version. If the original version was backed up before it was deleted, then restored later, we end up with two questions in one question bank entry that have the same version. To resolve this, when restoring a question, we check if there is already a version with the same version number, then bump it and any higher versions up by 1. We also check for any references specifically using these versions and bump those up by 1. Any references using "always latest" will be fine, as the latest version will still be the latest. Additionally, if deleting a version made it hidden, and the backup contains the same version in "ready" state, restoring the backup will restore it to "ready" state. --- backup/moodle2/restore_stepslib.php | 60 ++++++++++++++- question/tests/backup_test.php | 113 ++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index 48df6d8d60c..d8d892490f9 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -27,6 +27,8 @@ defined('MOODLE_INTERNAL') || die(); +use core_question\local\bank\question_version_status; + /** * delete old directories and conditionally create backup_temp_ids table */ @@ -5310,9 +5312,53 @@ class restore_create_categories_and_questions extends restore_structure_step { $oldqvid = $this->latestversion->id; $this->latestversion->questionbankentryid = $this->latestqbe->newid; $this->latestversion->questionid = $newitemid; + // In case the backed up version was deleted and a new one created in its place, increase the version numbers of + // conflicting versions to make room for this one. + $transaction = $DB->start_delegated_transaction(); + if ( + $DB->record_exists( + 'question_versions', + [ + 'questionbankentryid' => $this->latestversion->questionbankentryid, + 'version' => $this->latestversion->version, + ], + ) + ) { + // We'll update each higher version and any references one-at-a-time, starting with the highest, to avoid + // creating a duplicate questionbankentryid-version combination in question_versions. + $moveversions = $DB->get_records_select( + 'question_versions', + 'questionbankentryid = :questionbankentryid AND version >= :oldversion', + [ + 'questionbankentryid' => $this->latestversion->questionbankentryid, + 'oldversion' => $this->latestversion->version, + ], + 'version DESC', + ); + foreach ($moveversions as $moveversion) { + $DB->set_field( + 'question_versions', + 'version', + $moveversion->version + 1, + [ + 'questionbankentryid' => $moveversion->questionbankentryid, + 'version' => $moveversion->version, + ] + ); + $DB->set_field( + 'question_references', + 'version', + $moveversion->version + 1, + [ + 'questionbankentryid' => $moveversion->questionbankentryid, + 'version' => $moveversion->version, + ] + ); + } + } $newqvid = $DB->insert_record('question_versions', $this->latestversion); $this->set_mapping('question_versions', $oldqvid, $newqvid); - + $transaction->allow_commit(); } else { // By performing this set_mapping() we make get_old/new_parentid() to work for all the // children elements of the 'question' one (so qtype plugins will know the question they belong to). @@ -5320,6 +5366,18 @@ class restore_create_categories_and_questions extends restore_structure_step { // Also create the question_bank_entry and version mappings, if required. $newquestionversion = $DB->get_record('question_versions', ['questionid' => $questionmapping->newitemid]); + // Restore the version to ready state if it has been hidden. + if ( + $newquestionversion->status == question_version_status::QUESTION_STATUS_HIDDEN + && $this->latestversion->status == question_version_status::QUESTION_STATUS_READY + ) { + $DB->set_field( + 'question_versions', + 'status', + question_version_status::QUESTION_STATUS_READY, + ['questionid' => $questionmapping->newitemid], + ); + } $this->set_mapping('question_versions', $this->latestversion->id, $newquestionversion->id); if (empty($this->latestqbe->newid)) { $this->latestqbe->oldid = $this->latestqbe->id; diff --git a/question/tests/backup_test.php b/question/tests/backup_test.php index 782bcc448eb..a4110ab38d4 100644 --- a/question/tests/backup_test.php +++ b/question/tests/backup_test.php @@ -25,6 +25,7 @@ defined('MOODLE_INTERNAL') || die(); use backup; use core_question\local\bank\question_bank_helper; +use core_question\local\bank\question_version_status; use restore_controller; use restore_dbops; @@ -863,4 +864,116 @@ final class backup_test extends \advanced_testcase { $quizcatq = reset($quizcatqs); $this->assertEquals($expectedidentifiers[$i], $quizcatq->name); } + + /** + * Restore a backup containing question versions that were deleted, after new versions were created in their place. + * + * The new versions and any references to them should have their version numbers bumped up, and the original versions + * restored to their original numbers. + */ + public function test_restore_backup_containing_deleted_versions(): void { + global $DB; + self::setAdminUser(); + $this->resetAfterTest(); + $questiongenerator = self::getDataGenerator()->get_plugin_generator('core_question'); + $testdata = $this->add_course_quiz_and_qbank(); + $questionv1 = $testdata->qbankquestion; + $questionv2 = $questiongenerator->update_question($questionv1, null, ['name' => 'Version 2']); + $questionv3 = $questiongenerator->update_question($questionv2, null, ['name' => 'Version 3']); + $questionv4 = $questiongenerator->update_question($questionv3, null, ['name' => 'Version 4']); + + $quizsettings = quiz_settings::create($testdata->quiz->id); + $structure1 = $quizsettings->get_structure(); + + // Set the usage of the question to specifically use version 2. + quiz_add_quiz_question($questionv2->id, $testdata->quiz); + $structure1->update_slot_version($structure1->get_slot_id_for_slot(1), 2); + + $backupid = $this->backup_course($testdata->course); + + question_delete_question($questionv4->id); // Actually deleted. + question_delete_question($questionv3->id); // Actually deleted. + question_delete_question($questionv2->id); // Hidden, it's being used explicitly. + + $questionv5 = $questiongenerator->update_question($questionv1, null, ['name' => 'Version 5']); + $DB->set_field('question_versions', 'version', 3, ['questionid' => $questionv5->id]); + $questionv6 = $questiongenerator->update_question($questionv5, null, ['name' => 'Version 6']); + $DB->set_field('question_versions', 'version', 4, ['questionid' => $questionv6->id]); + + // Add a quiz specifically using "version 5" (with version number 3). + $quiz2 = self::getDataGenerator()->create_module('quiz', ['course' => $testdata->course->id]); + $quiz2settings = quiz_settings::create($quiz2->id); + quiz_add_quiz_question($questionv5->id, $quiz2); + $structure2 = $quiz2settings->get_structure(); + $structure2->update_slot_version($structure2->get_last_slot()->id, 3); + + // Add another quiz using the "always latest" version of the question. + $quiz3 = self::getDataGenerator()->create_module('quiz', ['course' => $testdata->course->id]); + $quiz3settings = quiz_settings::create($quiz3->id); + quiz_add_quiz_question($questionv6->id, $quiz3); + + $qbe = get_question_bank_entry($questionv1->id); + $versions = $DB->get_records( + 'question_versions', + ['questionbankentryid' => $qbe->id], + fields: 'version, questionid, status', + ); + + $this->assertCount(4, $versions); + + $this->assertEquals($questionv1->id, $versions[1]->questionid); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[1]->status); + $this->assertEquals($questionv2->id, $versions[2]->questionid); + $this->assertEquals(question_version_status::QUESTION_STATUS_HIDDEN, $versions[2]->status); + $this->assertEquals($questionv5->id, $versions[3]->questionid); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[3]->status); + $this->assertEquals($questionv6->id, $versions[4]->questionid); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[4]->status); + + $structure1 = $quizsettings->get_structure(); + $this->assertEquals($questionv2->id, $structure1->get_question_in_slot(1)->questionid); + + $structure2 = $quiz2settings->get_structure(); + $this->assertEquals($questionv5->id, $structure2->get_question_in_slot(1)->questionid); + + $structure3 = $quiz3settings->get_structure(); + $this->assertEquals($questionv6->id, $structure3->get_question_in_slot(1)->questionid); + + $this->restore_to_course($backupid, $testdata->course->id); + + $versions = $DB->get_records( + 'question_versions', + ['questionbankentryid' => $qbe->id], + fields: 'version, questionid, status', + ); + + $this->assertCount(6, $versions); + + $this->assertEquals($questionv1->id, $versions[1]->questionid); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[1]->status); + // Version 2 is restored to "ready" status. + $this->assertEquals($questionv2->id, $versions[2]->questionid); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[2]->status); + // Version 3 is a new copy of $questionv3 with a different ID but the same content. + $this->assertEquals($questionv3->name, $DB->get_field('question', 'name', ['id' => $versions[3]->questionid])); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[3]->status); + // Version 4 is a new copy of $questionv4 with a different ID but the same content. + $this->assertEquals($questionv4->name, $DB->get_field('question', 'name', ['id' => $versions[4]->questionid])); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[4]->status); + // Versions 5 and 6 have had their version numbers bumped up by 1. + $this->assertEquals($questionv5->id, $versions[5]->questionid); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[5]->status); + $this->assertEquals($questionv6->id, $versions[6]->questionid); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $versions[6]->status); + + // Questions referencing specific versions still point to the same question with its new version number. + $structure1 = $quizsettings->get_structure(); + $this->assertEquals($questionv2->id, $structure1->get_question_in_slot(1)->questionid); + + $structure2 = $quiz2settings->get_structure(); + $this->assertEquals($questionv5->id, $structure2->get_question_in_slot(1)->questionid); + + $structure3 = $quiz3settings->get_structure(); + $this->assertEquals($questionv6->id, $structure3->get_question_in_slot(1)->questionid); + } }