From f119c5a3fe37deb84eb2dfe032a531093d70476b Mon Sep 17 00:00:00 2001 From: Tomo Tsuyuki Date: Wed, 28 Feb 2024 15:29:20 +1100 Subject: [PATCH] MDL-78902 quiz restore: fix restore of 3.x backups with random questions ... where the random questions come from the quiz module context. Co-authored-by: Tim Hunt --- backup/moodle2/restore_stepslib.php | 24 ++++++ .../backup/moodle2/restore_quiz_stepslib.php | 7 +- mod/quiz/tests/backup/restore_39_test.php | 81 ++++++++++++++++++ ..._with_random_question_from_mod_context.mbz | Bin 0 -> 4508 bytes 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 mod/quiz/tests/backup/restore_39_test.php create mode 100644 mod/quiz/tests/fixtures/moodle_39_quiz_with_random_question_from_mod_context.mbz diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index a652f344d59..86a3522e9dd 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -5440,6 +5440,25 @@ class restore_move_module_questions_categories extends restore_execution_step { ]; $params += $categoryidparams; $DB->execute($sqlupdate, $params); + + // As explained in {@see restore_quiz_activity_structure_step::process_quiz_question_legacy_instance()} + // question_set_references relating to random questions restored from old backups, + // which pick from context_module question_categores, will have been restored with the wrong questioncontextid. + // So, now, we need to find those, and updated the questioncontextid. + // We can only find them by picking apart the filter conditions, and seeign which categories they refer to. + + // We need to check all the question_set_references belonging to this context_module. + $references = $DB->get_records('question_set_references', ['usingcontextid' => $newcontext->newitemid]); + foreach ($references as $reference) { + $filtercondition = json_decode($reference->filtercondition); + if (!empty($filtercondition->questioncategoryid) && + in_array($filtercondition->questioncategoryid, $categoryids)) { + // This is one of ours, update the questionscontextid. + $DB->set_field('question_set_references', + 'questionscontextid', $newcontext->newitemid, + ['id' => $reference->id]); + } + } } // Now set the parent id for the question categories that were in the top category in the course context @@ -6245,6 +6264,11 @@ trait restore_question_set_reference_data_trait { if ($context = $this->get_mappingid('context', $data->questionscontextid)) { $data->questionscontextid = $context; + } else { + $this->log('question_set_reference with old id ' . $data->id . + ' referenced question context ' . $data->questionscontextid . + ' which was not included in the backup. Therefore, this has been ' . + ' restored with the old questionscontextid.', backup::LOG_WARNING); } $filtercondition['cat'] = implode(',', [ diff --git a/mod/quiz/backup/moodle2/restore_quiz_stepslib.php b/mod/quiz/backup/moodle2/restore_quiz_stepslib.php index b7ff48de789..539f0dcc270 100644 --- a/mod/quiz/backup/moodle2/restore_quiz_stepslib.php +++ b/mod/quiz/backup/moodle2/restore_quiz_stepslib.php @@ -349,12 +349,17 @@ class restore_quiz_activity_structure_step extends restore_questions_activity_st if ($question->qtype === 'random') { // Set reference data. - $questionsetreference = new \stdClass(); + $questionsetreference = new stdClass(); $questionsetreference->usingcontextid = context_module::instance(get_coursemodule_from_instance( "quiz", $module->id, $module->course)->id)->id; $questionsetreference->component = 'mod_quiz'; $questionsetreference->questionarea = 'slot'; $questionsetreference->itemid = $data->id; + // If, in the orginal quiz that was backed up, this random question was pointing to a + // category in the quiz question bank, then (for reasons explained in {@see restore_move_module_questions_categories}) + // right now, $question->questioncontextid will incorrectly point to the course contextid. + // This will get fixed up later in restore_move_module_questions_categories + // as part of moving the question categories to the right place. $questionsetreference->questionscontextid = $question->questioncontextid; $filtercondition = new stdClass(); $filtercondition->questioncategoryid = $question->category; diff --git a/mod/quiz/tests/backup/restore_39_test.php b/mod/quiz/tests/backup/restore_39_test.php new file mode 100644 index 00000000000..91be76fa7c9 --- /dev/null +++ b/mod/quiz/tests/backup/restore_39_test.php @@ -0,0 +1,81 @@ +. + +namespace mod_quiz\backup; + +use advanced_testcase; +use backup; +use restore_controller; + +/** + * Test restoring 3.9 backups including random questions. + * + * @package mod_quiz + * @copyright 2024 Tomo Tsuyuki + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \restore_move_module_questions_categories + */ +final class restore_39_test extends advanced_testcase { + + public function test_restore_random_question_39(): void { + global $DB, $USER; + + $this->resetAfterTest(); + $this->setAdminUser(); + + // The example Moodle 3.9 backup file used in this test is an activity-level backup of a quiz. + // So, the backup contains just the quiz-level question bank which contains: + // | - Question category: Top + // | - Question category: Default for Test MDL-78902 quiz + // | - Question: Test MDL-78902 T/F question + // | - Question: Random (Default for Test MDL-78902 quiz) + // The quiz itself contains 1 question, the random question. + // So, during the restore, the quiz_slot needs to be updated to use a question_set_reference. + $backupfile = 'moodle_39_quiz_with_random_question_from_mod_context'; + + // Extract backup file. + $backupid = $backupfile; + $backuppath = make_backup_temp_directory($backupid); + get_file_packer('application/vnd.moodle.backup')->extract_to_pathname( + __DIR__ . "/../fixtures/$backupfile.mbz", $backuppath); + + // Restore the quiz activity in the backup from Moodle 3.9 to a new course. + $coursecat = self::getDataGenerator()->create_category(); + $course = self::getDataGenerator()->create_course(['category' => $coursecat->id]); + $rc = new restore_controller($backupid, $course->id, backup::INTERACTIVE_NO, + backup::MODE_GENERAL, $USER->id, backup::TARGET_EXISTING_ADDING); + $this->assertTrue($rc->execute_precheck()); + $rc->execute_plan(); + $rc->destroy(); + + // Get information about the quiz activity and confirm the references are correct. + $modinfo = get_fast_modinfo($course->id); + $quizzes = array_values($modinfo->get_instances_of('quiz')); + // Get contextid for the restored quiz activity. + $contextid = $quizzes[0]->context->id; + $qcats = $DB->get_records('question_categories', ['contextid' => $contextid], 'parent'); + // Confirm there are 2 question categories for the restored quiz activity. + $this->assertEquals(['top', 'Default for Test MDL-78902 quiz'], array_column($qcats, 'name')); + // Get question_set_references records for the restored quiz activity. + $references = $DB->get_records('question_set_references', ['usingcontextid' => $contextid]); + foreach ($references as $reference) { + $filtercondition = json_decode($reference->filtercondition); + // Confirm the questionscontextid is set correctly, which is from filter question category id. + $this->assertEquals($reference->questionscontextid, + $qcats[$filtercondition->questioncategoryid]->contextid); + } + } +} diff --git a/mod/quiz/tests/fixtures/moodle_39_quiz_with_random_question_from_mod_context.mbz b/mod/quiz/tests/fixtures/moodle_39_quiz_with_random_question_from_mod_context.mbz new file mode 100644 index 0000000000000000000000000000000000000000..ef2a668db9015824598ea06dfcf4e428dbefa593 GIT binary patch literal 4508 zcmV;N5o7KjiwFP!000001MD4XuiG{m{DCDi$8Dv`wmVRQG>oOH+bxOWbd}{`Md=;KmFZEA2`J+%W!*HD z(MT*9Cr{~-4i&&8%Pf1((9^N2;yHW)s8$T?_?9K(W$E#i&Mi>a7mclH6i>&+pz`Pp)3gpUsF+7;*IS%D1mX z0^77<=USd=Ud^8w2Bk}bEtn&DnvEm?!WSS!<8d6#ZGScrF$tGe+a<{7CPz~MB8ej! znT_}-f(KCqqB$LZ-HA0D5eUvUg!3Mp9sa=Cz!`2@A{hNbz}vWpmkF4|#OzZ7XRuMK zya2mkEt4o*u_$?o8U zIn3i7t`F)Z4TQ+bU|ryk5&2?bHNUj_{N=-e_1DRGwAg>o8aulEhnhUt|2{%j`%mTM z0gnLgl|l564F|%=zq~@?b7h2CT7vr$5!j|n#s-9;=}{a)@UZX>F42s)-~4dxPo}1A zkYR%j7}!{sd4>!={*_>GJ-ZD`WD(^kAt>W5$aVl*6#gwxVUiVq;YdUvlXx4K3`rsc znZb67DzFLCD1q%W52`XNswE_tjdUDI0j+Hab|8;4YSu-R5bWTR9OOBOmN4mU49Ajr zL?J5xp>T~k8#NG-)M1c9XvSB$*dpsNLK>9N=I_d4A%<5Sq!4Cl5)`FGMghED5!ixq zli^`bjuL_hgz+{=b{XuNVs6hyDu58NGOD<1m|-)9q{;yrlW3{tNq)F9$?DgmTe1K~QQlKN&74p^1W>(m=`)R3U?+xXbbg8XW$~ zin~lfIY))W9|XG0v%3P;34~RSW;NHdkq%-SkpvGGQYiW*fo%@9c@t)L70hH8hh;^l zuNp`YE|V;LUoaXPqZWeC&zQD@`%M4`J$?FCN9!4&=1|jM2(=nXlHFynkK+`utg!KD zg7fy=CDl4k7s%uXVF)#GQGg|mQkqd0A||DaIE90G8Zzl=LgDepnqDyqRE{LWg9!c= zV5S2??x}h~&Nj$kW0kp~4RvjL1)#d%S( z9uW~iumJ(89#lj2CG{k+GRkmRgYjz6GH^*#$~a@zQ6ylHhC)*en(@GbDbZ#sF?DHx zN}Uyx_1c^pKn_uQl$uFyMmdOARx|b^t$KKBlgGmFj&y)xmPblV)9aBOpN)v3LMWX< z4r{iLDID5SI~9+X^`B{5mcIUTAZS?s^$@zQ|FBSCg+^Zs#X8JJ4KNIX^MiPuZb4eg zgbI|vioq&eCLG=)KMju$(EqXH4f@|l=&JuzD6kfj?*$eeW&s*t6clFZDqgE(9L6An z{$D7xoOwA3k2d``{hI#!u07~~AEB%MgIhLS)2~YxdB5$~ zNG3E0Z^tZ$Ws6`>+pr0FgrTFj*d@UO_WjdOjw1VBh19$p#Ar5g6p;;1`o$2)|4-ce z^h-vdEf0pm-wc@o5Ea!ff@+EIa+HyD-f=l@&xKnJ*{jICjEe@(BepMOMWI<{;UdI?94|MJxnKBh~+ zOIK$IM+Sbg`;0u=^dD-ws{f`x9@hW;gd_I<0o$)1rtE*8aB^o~Lpa|5XF0?DuYSVd z|NCbL<#CoX@o3S1XYA?zKhyPx@xPaFl>YN=|DY`Mcv+Q9^R+lpeEs@~3MMy6J#ji< z7v$(BzS!`j0I?;yD>rD{n|z35Ff#H+h+YD(swBaO1zy1^3;S-^^PO9cveV0H-UG1P z#On>(3+3OuEP1v|GJI2>ed>9GEKMG8jqwf1CU89oeadKV$_W_o&@QZh%r30IZx^OQsa!~|h3OU;u74i7qIdUhVb^T}S`#+{R?Em%?y3hZRoKW2$ZxeCl z;ZYhH+(UnzP;ea((q zUi>f~RJLBsD2;jT@TaX()wW;>HOZGsY7n@iHtY789r0HU%Y2yv<60 z9{9lPzZ#5?lJ_0Jg1F%=_TU2p)QJ}O?I3_2V_1~g0^GwnP8uW&{6G_X1Ft(dVE|Wr z>llIW?(Xs|E9aZC+!ZfIw1uyo=?$Dd2FWHX(9=jdwU$W%CufToy~Bs*eEDJ%6q~s_ z8Lvz*nFP~S=q@d=i~`4UCsS`a0rnJxQnrohH6C=U60SYQQc3}BCZ`WFl-Vv0i+Vcc z#i<&Sc^$ON46o@5buWE$cEtuR501%CB-oQ7w0orx&Z#*;BEO%iITw&dG!Mu1>~Uir zjHWpU{!$oHy?SDS)}`j5fOC2YZ^xGs*(6zk6^rXrT_%K>v--2 zIKjm7ZEqYowrl!eB|rOtSJuqH!F7wY&_*68A{^8Z&PKJzJz`k4oLrctZ@3D}B6X`mTZdeL6uw3x(+KkUIl273n-HoR}M z#~S4qY>b01foi$zeN9yKL|i6&ZVeHXOdQ4N-7n}ipU_N4|Orgmy;hl)Jkhm13!GD6zoF^ibtqDr8LjM zFi5lXVH^LsuW8Q0RDZmp*9voa+r!m52UqfxSf3p6G^S^U<$mEk3(GLu_AWomAX@Kj z*t2kC&j^tF;(fL+wnpMmTA!6pvURVGEN`*zw+&}u&CbFugY|6_8ph`=gNVmdrsog-|6W2@{nu|ku@gVF8&P;)+#V}0{Fm8Iaq!D7KbLw= zBrhF}lTp_To~pA(0Q>DiuX zKmYIjf5s`S0U;+z>9I9aJ-{X%^)PEkJ=)Iz&GhH|M+#uyg|$7s<14-M4Oj(LQW~%U zjJE$`xJ=Kz+W;e?rQ|h1#pR7)6dTyskrAitpC^pZ8tU@d%^8Hidw6TRZyJRF-Amjf z1=H#D1acrj)UG`silXPW7J4^&!&|gAQDq4HsP(5w`Q^@t;{4@RQs!4iQ9dN#Dm%5DBS>>d&joV5x6F8YFPRAQ3W{YB0@N)F-cJiAi-HoZ8s$H~ z(4hL^SPhL8;2q2!t~3RpnN}DyC9V^%@Lm(zKN14SVKri*H+6;4XzEYfH0rMi@=AvL;YLL{z22SujF463KC;44j`N^`p*W zFJ>T{SlUQbW-L0U+n-PD8UH2?;~)C#jphOHryz~8t?_xw6zGe?qoH}sNM)XVq%zMm zQkm@|RgT;9)Wei-O`d=E^?UH||9#*vg)#uaueX1Aj~}MS&g7V3ij!Y-m^!l-c3F+o zfy2{3A3-?zpM5lhR{yW%)$ad$!~Nf0!cqVKp*eqRuuE5F2uBBeZ|l!+@eK&^O$oyq zzp(um_V{<>H{=li-5@UcG7j*qM5;z)pF(f;ISfq>5+8^Lm0vkKJM^;00030{{sM=7mb<#m;e9<)}vVf literal 0 HcmV?d00001