From 34134cdfc0be0d0507af4ea5cc5fecb4d9be11e1 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 16 May 2023 15:26:22 +0100 Subject: [PATCH] MDL-75696 quiz: Fix restoring pre-4.0 quizzes with random questions Restoring multiple quizzes from a pre-4.0 backup was broken when the quizzes shared a random question. This is because after the first quiz created a set reference in place of the random question, it deleted the question record so it was not there for the second quiz to use. This change tracks the IDs of random questions so they can be deleted at the end. --- .../backup/moodle2/restore_quiz_stepslib.php | 16 +++++- .../pre-40-shared-random-question.mbz | Bin 0 -> 10985 bytes mod/quiz/tests/quiz_question_restore_test.php | 53 ++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 mod/quiz/tests/fixtures/pre-40-shared-random-question.mbz diff --git a/mod/quiz/backup/moodle2/restore_quiz_stepslib.php b/mod/quiz/backup/moodle2/restore_quiz_stepslib.php index 37713d33b75..b8d882249e9 100644 --- a/mod/quiz/backup/moodle2/restore_quiz_stepslib.php +++ b/mod/quiz/backup/moodle2/restore_quiz_stepslib.php @@ -40,6 +40,11 @@ class restore_quiz_activity_structure_step extends restore_questions_activity_st */ protected $legacyshufflequestionsoption = false; + /** + * @var array Track old question ids that need to be removed at the end of the restore. + */ + protected $oldquestionids = []; + protected function define_structure() { $paths = array(); @@ -342,8 +347,7 @@ class restore_quiz_activity_structure_step extends restore_questions_activity_st $filtercondition->includingsubcategories = $data->includingsubcategories ?? false; $questionsetreference->filtercondition = json_encode($filtercondition); $DB->insert_record('question_set_references', $questionsetreference); - // Cleanup leftover random qtype data from question table. - question_delete_question($question->questionid); + $this->oldquestionids[$question->questionid] = 1; } else { // Reference data. $questionreference = new \stdClass(); @@ -604,4 +608,12 @@ class restore_quiz_activity_structure_step extends restore_questions_activity_st 'shufflequestions' => $this->legacyshufflequestionsoption)); } } + + protected function after_restore() { + parent::after_restore(); + // Delete old random questions that have been converted to set references. + foreach (array_keys($this->oldquestionids) as $oldquestionid) { + question_delete_question($oldquestionid); + } + } } diff --git a/mod/quiz/tests/fixtures/pre-40-shared-random-question.mbz b/mod/quiz/tests/fixtures/pre-40-shared-random-question.mbz new file mode 100644 index 0000000000000000000000000000000000000000..9dcf49e8557d98d3f935b8ee34ebce9b547eb95d GIT binary patch literal 10985 zcma)hWmKIpvo7xLEo|Hi#jQ9LFYXS--L1&RDeh9-y|_bhch|zExVzhX_|Ca&-Sg|N zn?I9DCdsTPnaQ&fvPdMT|1Pj6`f`aoP4U^s|xAsDav?h z>KV;3LP9W| zE@i#H#D4O+;8Bkm@S&ySc28A>-oA#waQ8F6o-bqk$Od9BOD6#pS#IOFM1A*f!&ZN^ zI&_%6@Oea1KOJy2K=t1pa~0)sS~-&280j-_Fm+D3N51yO9)ecl7ZNOx(plwPu7nix z8+Y4=n@<<&6s+S;Ms({8`Ex0;kSD&Ye5taMrYCrm&QE5CBbV8>{iAaQK1$-{Ck^yL z|0;P&TgUL`*e+zM-9V>lnv+;@e2s_&jieWe(q!lBNSIswc8sYe>?|A5h8{ybv9xHd z{lbVc7USg}HSBcyEF_^;P1&BFc?=IW2nUzD&MXaIV%1J{11uyRn!TY3qdF_nqYIPFKLXtW0sPo4=+;zk6K&s7G-* zO+lYfn6l^5Z0ehPK;1E!5H|@Iua7MEeH&ruw%7pG2A1x6N#cZyql)wrig=x) z1ERZL>>HV^Y;N3eB&f~A)L+W$uXaRwsq#Mh%Nv#t)b6)6oH_qUd=gRrmU$DW`GLz< zx%bZAP8$g-R$-5^sF*PT=T{PS^z>VY+${R}MKmWuhRUq9jx4CK#G`?3r-3nfN@(dm zez$i?J|ZG??BP&oS?Hyv3pD|LWgU@64VHNW!AjO*EZ}6zmhc3#=XK4fF-hcgcj5b* zrvvWL5c!T>JY19PG1k&t3BQ95Cl`?S*gF!rD(^=8@wCZE*+Z{Wdc?SvZi%|23{$vp zrZ1LfCET6*w5K_VcIxb!Trm;#hT8|1*X#5OTEl^KP@6aH-N^ zYVCYP{_O(nFhW_3!p+YXP`Ib`_WHP&Q$?ILV_zt{Egizd7k+Dde{JaZ2k9)x`%BI_ z!DAohkSZ4Z^GQt+j7*}!{f;L!=XS{{x8H-4A^{#lu!r28Fff(WI_qC&$QA0B70JH7G24O+j~sR8vWWuY2fQJz^USA7 z%1>>PPaNq+){(n1H4unfD~5SlveJ@CV7|6qg1gPEd`9$&bg`UVb(=tEHMZ0xv%K0Y660<}Ts;9uxx?M?e8rPe__!z}hehW38p!I~3l3iQePZH#HS z^;7kiM`i2mv3GjRAtd(8!Tb<|enj4XK!LYu$qfLF$N_YrGViYd_e8PfKs5!84>wyE z(cDzBAI_&)?$)WpVDAUxR9g(!fgcZ-H=Ol>k1*KCk?9vH^1VLnH{p!kuPU5&B!917Hw8kezCBvWoG zq1h$0>ohqN$5G`i{tK_1Np>>}z+~KF*T;%1_82QEZ%JOrO+~3j@WIogiDu&AG?lTGiTlX2;`v160psXt@7)df2yH*ZS}~1=LIMmb7D8-H|%M;^7jlUM@aEUeBS|&Is$oCr8t29!- zMlImxgnsTscijPTVKBcoMxMd>EClSKA_+gHPyU4Xwm;lKgqt^B-s;aw?d~tSd?X!- z&m${Hh0K&+GfPK_9r*d^idSRo4Hyu{q+D^~qajAnD1f^j5sO`dA`FD572`M7Q z`BnX?5)el^n;83_^V5EgE8SBa7)LdqamR?^Nort85{a)-Vz?_)w}038n})YgxF|bU z$MiK{Datiyo!hPKQv8ARjTyVX+0RZ(my^(uSWbcNT*i}Nqidng?~uZ1kThd;B6zeM z2c&c}SvMnC@x6HI&p<`$4s!fN1Eq4^>(@V1-tI6a?aK&1mkgWB~Ugv9-I>7=dR~Q5A-r7b&5%xlRok zXlh{12<&IOt#XkRS}{YCxJMJFN~E(Wa@&V_5r~vWdoG(oX6IIG-o0yKfCEF z>&}oEIb`QpRD{r?le*cqZt-uTz`4+cgDj;evIecbuK;tO`=@)5xEFQ|Q^e=!K&lC1 z`(ufx)K7@3BhBAFm>-&O|HwHdZA|nS^0(KoPE?)L_lX^8yJ58ty4hos^6(g+Y>pK) zbUb2Rht5udLr`n-= zJSy#A>+!>#dm*%dyiE_&Fu!hao!`dHE_rknOeI*MwPLtfzhL8(Kj$b`ox;p=tIEFw zU}RT(SM(T}@_)D&j+wm|o|cI71&muaAD?rEJj*W$0fC-JGXE5lB?v&yGzt9OzwR3H zRGR{VJui&#&y2nPlxFVZ(R5t@ef;x8GlUjx60*R;`!PONOhblxD`Ju0O3zx5FHHwp zowEA+QDi7J3|39}c;`!Z*{|;7Wr3%7XRTlyh%26``uzvOIcJUqoMZQ z+D?+KM7RE9BF*&#ys(E+0RMs!4+JgRn`2(Xvzrxy~h<>qyT138CHzPrEn(_=@{4^`0@)(b35k%iEw!9SM9rxaJM zqH6r)>jlEk7ykyW$!^a&3aND(#f}EujMhmMDkhsI*9xOkvWXV#xVB@WC^*B=#S2eg z+D7@3J!)(dXH9iI);X0b>~aavHh0K$Jxwe@{B9Q<{02hH6n4$EK#KLHux_JRS)+C@ z^KEC3he8WainAt7`;$#8cK##h;CS^lu>Ws>sAcL>IQFWv>Yd8seTnNNsPB0Bqci$d zalK|=>ac827$TGSfJ5vb{&>DRYd;xtsN7~7e^z@}Hk4 zg^71a?;g8Og`NS@UFAEV3*Y{Aq3+>!9v|#y?p6M1`1M{m03Z{o-~i&UNr3DB!7+$s zed}J*Cq}1oPtZz2C9TM*&L%W+p+A1J3=oUV=DDRK}0lx8Py5!rQkn_-fp+8E<2_(zHt5P&`UiKZp_XA|YYd)8^B{{wx$fVm(kb$^p;lct3anm`-%ek;%nV7|=pn$~ld=(I=Q9R;ERv6UMEt zwqD3COcH4g$8#T1A8l6KanM5wQLY7Q=z{# z27@yw*_>+#mqHPmgFS8AXAr>|Ys4=V%DPGMHW>Y4c)4EhPpt(~L9KEbd!w-VH0j9}kEjPHPy_sNGkw!0fJY@Ch_@9=fc!6-TXylHOqY(fyXr>7aw zK3@lW)RxYtpdep~&EZkj2x5Z|lE&pQ3u6k}rPd9{Chdyi4Gai0aM5|i)TVd>OcdJ! zVKkjBj-|C4Iv9eKQoJb|1x2Vg8mPXl^{MCyyFLInIQJGT+EbR9(zT4pB9pZ~DxP zAJ-Hj%C<@%IU)t=%^fawLk@eYu6^=6O9~NXKx0CSU~gunU+(WU5|C#?TkLIeSAzej zO@wFh-B>9tK18{04xe#K%GV|=v!cm<80j~cI0A-En6ZJp)rGAg^!$b&Ilht{pGY!7 zne`I}nLhJWe1fz8nNbFHdtVU{y_S+Gw$`*muez2HbZaNqqY7!lW41j+P;AUbXrdW9 zTuUJ_H_}}^`DQDJ%DdQNH^R^$m`Y-H)VgEjJ5{t7psrg!meUSk=J;LS~%PiDfsEQ55j>2*6#BdM?J#={RhG%)M7;yN8 zcW|yKY5^|afj&P`R2SG>e=U>MJn-(0zAwpb6b9Y-FJW1@o`6nkJ-!WX%#e8x_49}- zKXoFb%nH9cq781ak$Oul@F!`4qZ_qu(`5&Wt?hR!7YiX`EZ+0IqbP(p)7vzpL!lD^i|&I# zQgZIH$qJhzKB2&`Gm2m`qr#-QWX>6t@?FF(qZXGr#7DsH$)lhM&vsCu0goi2EROAo zyTGaePdq)-;?dVeJfpYYd0LNZ)re{=uU>;*2g6;Hzm%P`o@7-nI)68}bM>Z9m7rYJ zQ(o(ytjv@Ga;l0AN?^>h#=SsHQbeq1dU7?=8B;`DPhj00E&nV6GE!B`qn4dwLVUVYJd zwZ;=6hvLkN-LX9+TtjZeqsKb<>c!#q7#QiL)Co~L!@KFSj;NU%vn3x_-xZRT3$sro zTlGj8;Yhv;#yApYqJD8ph*cN3uxX4yQ~JVBFzi^ zJ840$;rG>AaZ)SKbaV$LGd9i%|5J0sMWf81%Txu~uTeRnzuUizGF1h@HcnN4aY9E) z$z4abQR3^G#jx2VdcFSCSA!5UUk4YzAKO27t=EX&6*awjKGyv&o@{S{#A^*k1e%Nc zCMb7rS{yt~OktvS{|pjp8~R4Vfd0qEdkzDpO*+>JF(7x^4GSqLFG$3{5d;UXXon~| z(|h-?|0YHs>J|hCQ*ug<#p;U~&_B83VzAS`O=g3FvfgJGz%KBY!05E^G_d6EpZog> z59W;!d3|r66Z%WQ`d$gok`WQCQ_i`ZE@pkmB$Ow{E-*@*@=_j3Q+m!!ZL9$)Rp0Qj zSQAr57t%5g}MnX}JQLHf!(sU`^pYAGo6!q)$p?%&xmT#}#uVm~b{{ zagh}`&AUrJ*`Fi+u|u&#O)B`ttc0=HECR3Si>lU8Q${RM>bw-O_Q0<+8Oi<;Dj z#x$LtwHHbN>p*>Auyb^P5WqT(4pUl0jAHQPGXwPOJUUF_egY1W;5zx`%)3&yo6y_D zwH3P8+P36x>E}ZS+gJ^K=;yl6KX-5D8fvinF>Z4DlOAmTX;{@Gksj9^tpqPyKUp;87r z)?e}kCv0|2bE~LUk7GJ;$b6+5mOeaMNi~%Ceb5Vw?W9_@iF#?ET5Y}@9!L<-E`1%Vm+C&6FI9>0VCFJ5^V{Y);R?D!lN`#McAH_0TGuN&%k~AjX zzAu)V+NzU*dQ$;!^M=R8U!ye-_Q72B_PBgwS%s2&UzgEJrVUqO`E>$Rs9}F$gd9!l zog#{S6s<7QK1Vwu!vHGnYSd@nHFwXOplEcj~z0H@_s^pP`|h z^~cLm1gCC)+m~rns68oslC2@bUbQFU3!a`m9Hz(!#>gLlW337Av%fG6g0DGumqXVL zl7RGK^-hb`Vmh6CC&T%K!EF1i%hOA7b25DQ`)9wt$gd9Rdvj5RhZpA;YsI36n_U!C$^^s!RAqvgtln8rMT8--4K<*BGW|&j@IKgc1$JyIuX2w@7*u!<;_@b$Ng=Em9y; zFidwA=t=8l>k*(VEjE?KBEXG-g&9vu;aCvUEqP(FV)+m19!tPbYnrOc20J1X>Em+t zGxRfk0!PmE%s{dJR~iYkjU)ZwG?a zCGhzeU1GfX6!33OmPjP{IF^vsQ(g?tTx2oq76Q1AkfE<*lH~Zlb@;BPpZtZVI0>_H z13TJ;28r562jiuT(~4rGn2Bgg2_-7w^-zggEQk)0@Ox3?n28!h`l+_W=&05TgUx~Bic8^yPZ^a!O zA%%`+=j&wU>s*Abw_;`r8O9lrVngosHNMhEvC&40aZnp@B6gZZz?NFJSU9rr_dx{* zj2SaE{S(e1-|2-qXr*m{QZt@J=nTP>KjTZ{fkLZ4D%$+vN|7}a(rpNJb&7@c)}7ub zjT9YDHRndV1v5fBH|8PxE`ldNN%ytZrPeHtoL4Nd+naL5>}d1Yp{= z2l>*59`tD49&hFd>f%x|Tf)IPhCijeVE>s)i-&i4%>L^mZj_K(uF!*H4uygBzeyKW{_Vi?ne z7?%(XJbF=Qo@8HT5oXn}2BRpX+0vlfu$(Z;DHj)`6i%SdU$T ztP`7*10rxk*k5X07zqygmF%~p{bXYS<)$@*0}!DPfZOvF?+f5!S9Sq>a1BpgLS9mf zE?2$quMXG;p246Yb~gYM8u7V{56kn4U%>|;p4T0JB5O}E3$(sNXAj6cIPb!M-uDCA zH}Hq*@@g-cRSE2kqj&?+kZG$PCBKT@2+$wmG_>=k|01{>Km6JwO_}lqebF^^ zXmITy!g|##gk3;_x}` zFlzgaUM}5(?qVh!Dh9^JHi@}So$6IMi5V|KDVh9RZU(~cSZ$0KWn~6lnI@SgiI8#* zRmTJ-t{Yp4KHixn4BSL@3(To-sc#~bau(oY*RS?*IewerXMO)qn`3+ zc;J+Hh9c$K^{?bzOa|z(IFiCs47vFq-1Q)}K6CfQxcJqxftv%7X}I3QJyrXUfh{qX zP7y>==_m?!wKzllQL$QylX2?1Nz)0Sj#6qa>o12NUaJHhg>gwT0sP-*W#_zX096L> z>1|H9F>>xcK(YDvvFkhHXRP#JE)7)bd+ILmGTp@S~qT9)7 zfpgm-GZ~w?5{AqC1l(kQU~!t+i`8pwjzc|elx@xN%NyLVJkIUMyB64RguiPqd=ZK` zkM@FTgJ3$$o`hP-MN)eMd+lbAf{_~yq<)+zp4VUGG}{#)&s2^5-x{9NK1Pw7!=a{k zcU}42W7qjGd351X6%=0d<9v+^ta5%@9JZ=^=8g5}g0qS{@$-v96{lgq z&4=sEP_s!cpw>}GH&?KMfndVTp`nf7i-$+2`&OoC*ZV_^h!-d>f=>HvYV_vR#&0ykkM6=G3L!=`D<{`h4 z|5P``WSgbZUIvd^+j_I@D+1Our@;rCHat%Dy--{_Xj530{OPtbZ0b+aT?QOGw2)Vr ztoX`0sZzg>7fjZq=mB4fYzqELGDYme|0dv|mCl`z(9ZE#*ev zvFo`NZI-AUA!O~ukAXuOH1iCdIO45Sbx<%nDd2|jVrV|iZalm(ywUB*m z{dC+3pE`7$wo-W6AnFOR(Y>Vy;p$5LyRPCvDuv}a5=mNoS1V#C^NdKMwarp%t2y}B z1d!KpZq6))^o!CB|L>6j2@dLBo#(Z7-3V(+5^H)VbzA9$lE{okBfQ@pI@5eA8M{JI zM2ni;{-B^(Q-7jqaCrUOA-96F{cQ2|ET>HOySmx(m-h71sVo4q3c!=wDXC&pI0CnL zzXf5>JVD4E&z`EkBTm|GY5( z$FC64O})qclWwy#!S&I^3fW3;M-upQ-HvZdjYqsILnxUkz^5M3M+f@~p~v^YCd=@C z79Or`d>tZpta?Fp$7&bK6y4wq{}v1~6=$LB#CNqW68e22Wem^6R8^9QP|&Z*jcE>A zClck9#If7{X@wnYQaC{+(}7@DL`BtnfzBvTp${wYMtg|k_b&`h@{F& zfc31-`oDlLC_1X@A)eA_8Ic)d9MpH?6-xXGISveW)DaUyqUr7!_iVlpKlq`B@Pjb) zhK3rrgPtpdPRuQ8q;2ZYWJ%u}raNlS>+Z{GN7DZUeK$2D*9_f~xr)ffKnM)l?&`1y z(B9MO8$Sly?xQdpNazh$Di#RpF1iEzAZ$K=azLXfKT4WLyAkd%M!Ok=lCLf#leI@V zpj_(T$T;#5@>9F|(L@!@%Y1EHtcZAX=r07CE`fb=-8?LTwY3``R5SCj!+@JNs<)W9 zTe|0-;j7F0MW-#w7e6&079nj!TuyB;R8%+dxtB8gl;^AN55huhq$OtpTjTE4P!ZNF z*KOrwSZDxOzDsFG_8sb-d0peq+0k&x*P?rXmT#p z1hqlG%L*U*sX$7xn+9es9>9fOuWJ3n2Y?ebH zjWFtJ-w4gD??U5NO=(n6@NfS!U68?FJHp7|hy?r>%d+UQ;iltJt+rag!4bv9?HsW* zP;bSlfVBP$mQKq)NBKVK!(-Q!v%7$X@aXqY24)LVQj^%IT1+_+c5|9tOHtgtm1h`h z$^ErVhCH4*8A2K}3S?5&<~%}0l-0@^^YyqDZ+EfQ@crGURiT62k7EBMrDxSBytEh> ztQhuh`t+ejr8L(zV$aOU&!kGN(vPvDohk%gY%90#^ZVgJaK2(EPoWqUvI&=Yp#Zn# zNE$ka1{$W#W2QY3*kVmqZ>=;`D+AtQy5B?|6|Z>UdTWYU%?>j0!p5?9P^!u**Q^`M1=%QR?Ls!4=vd$xNf4ua1?>rp0N(miFV0wqW;3xwp%Fr-FB1Z zu+4Yw~_T;35?T=_A& zsM?fWcboL;omDsBethn}$Q{mgP6-V7T6!c$vK`h| zfw84s0RG?7yNT#UiemtO{3=EyFN^F7k$f&iO9Av;qB(foP?$lkgn0OMDMy2|g!MSo z1h&sQaqs%@E3KxE0g@GmE@$A{FVUz$Z*+jPh|T(+L9G-qdTO^5fT?){6x;)d(5Jw@ zTVL7-BmaZ09sbW3K#+*x3xaX)a3hcdZKPLCWDo=Y`4H@W5m_^ED(nh$3DCb93jU1{ zU$HO^tAB!+ZjhxV(91f=8w; zKP?RYqQKaQdxq!Ki2E)rNh2t940gQ6V^oj@;SS;mE(FK>i8`S2o^cOJt3%axYqkpYii0Pko+Iypet) zvScFgluu)Id*Aiu{eSleyliy`w(Xq)!Jq~Ff{@pH)N+j037rZdC` z=zAwCCVv2aB)v@_Tox>^=5X<5Z&ayA0HsE|Bp(a1L_;**`!|{R0}lJ Fe*xIt7pVXM literal 0 HcmV?d00001 diff --git a/mod/quiz/tests/quiz_question_restore_test.php b/mod/quiz/tests/quiz_question_restore_test.php index b157099aa2e..5743ec4b2a4 100644 --- a/mod/quiz/tests/quiz_question_restore_test.php +++ b/mod/quiz/tests/quiz_question_restore_test.php @@ -380,6 +380,59 @@ class quiz_question_restore_test extends \advanced_testcase { } + /** + * Test pre 4.0 quiz restore for random question used on multiple quizzes. + * + * @covers ::process_quiz_question_legacy_instance + */ + public function test_pre_4_quiz_restore_shared_random_question() { + global $USER, $DB; + $this->resetAfterTest(); + + $backupid = 'abc'; + $backuppath = make_backup_temp_directory($backupid); + get_file_packer('application/vnd.moodle.backup')->extract_to_pathname( + __DIR__ . "/fixtures/pre-40-shared-random-question.mbz", $backuppath); + + // Do the restore to new course with default settings. + $categoryid = $DB->get_field_sql("SELECT MIN(id) FROM {course_categories}"); + $newcourseid = \restore_dbops::create_new_course('Test fullname', 'Test shortname', $categoryid); + $rc = new \restore_controller($backupid, $newcourseid, \backup::INTERACTIVE_NO, \backup::MODE_GENERAL, $USER->id, + \backup::TARGET_NEW_COURSE); + + $this->assertTrue($rc->execute_precheck()); + $rc->execute_plan(); + $rc->destroy(); + + // Get the information about the resulting course and check that it is set up correctly. + // Each quiz should contain an instance of the random question. + $modinfo = get_fast_modinfo($newcourseid); + $quizzes = $modinfo->get_instances_of('quiz'); + $this->assertCount(2, $quizzes); + foreach ($quizzes as $quiz) { + $quizobj = \quiz::create($quiz->instance); + $structure = structure::create_for_quiz($quizobj); + + // Are the correct slots returned? + $slots = $structure->get_slots(); + $this->assertCount(1, $slots); + + $quizobj->preload_questions(); + $quizobj->load_questions(); + $questions = $quizobj->get_questions(); + $this->assertCount(1, $questions); + } + + // Count the questions for course question bank. + // We should have a single question, the random question should have been deleted after the restore. + $this->assertEquals(1, $this->question_count(\context_course::instance($newcourseid)->id)); + $this->assertEquals(1, $this->question_count(\context_course::instance($newcourseid)->id, + "AND q.qtype <> 'random'")); + + // Count the questions in quiz qbank. + $this->assertEquals(0, $this->question_count($quizobj->get_context()->id)); + } + /** * Ensure that question slots are correctly backed up and restored with all properties. *