From c3a89cc550ba17d693d9b50d12ca54e11550332e Mon Sep 17 00:00:00 2001
From: "Eloy Lafuente (stronk7)" <stronk7@moodle.org>
Date: Sun, 31 Oct 2021 16:17:35 +0100
Subject: [PATCH 1/2] MDL-72950 restore: Avoid qcat conflicts with matching
 contexts

Though it's really rare for this to happen (it only was discovered
when running unit tests with Oracle), it's possible to get
problems restoring courses when context ids in the backup file
do match existing contexts in the restore target site in certain
ways (see the issue for more information).

This change just ensures that every call to the method (that
happens 4 times, for sys, coursecat, course and module levels):

prechek_precheck_qbanks_by_level()

Is processed always for that level. Before the patch it was
possible (hardly but possible) to return question categories
belonging to another level when some qcats contexts were matching
between the backup file and the target site.
---
 backup/util/dbops/restore_dbops.class.php | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php
index 91be3c13619..fc0067ee771 100644
--- a/backup/util/dbops/restore_dbops.class.php
+++ b/backup/util/dbops/restore_dbops.class.php
@@ -619,7 +619,8 @@ abstract class restore_dbops {
             // Top-level category counter.
             $topcats = 0;
             // get categories in context (bank)
-            $categories = self::restore_get_question_categories($restoreid, $contextid);
+            $categories = self::restore_get_question_categories($restoreid, $contextid, $contextlevel);
+
             // cache permissions if $targetcontext is found
             if ($targetcontext = self::restore_find_best_target_context($categories, $courseid, $contextlevel)) {
                 $canmanagecategory = has_capability('moodle/question:managecategory', $targetcontext, $userid);
@@ -765,8 +766,13 @@ abstract class restore_dbops {
     /**
      * Return one array of question_category records for
      * a given restore operation and one restore context (question bank)
+     *
+     * @param string $restoreid Unique identifier of the restore operation being performed.
+     * @param int $contextid Context id we want question categories to be returned.
+     * @param int $contextlevel Context level we want to restrict the returned categories.
+     * @return array Question categories for the given context id and level.
      */
-    public static function restore_get_question_categories($restoreid, $contextid) {
+    public static function restore_get_question_categories($restoreid, $contextid, $contextlevel) {
         global $DB;
 
         $results = array();
@@ -776,7 +782,14 @@ abstract class restore_dbops {
                                           AND itemname = 'question_category'
                                           AND parentitemid = ?", array($restoreid, $contextid));
         foreach ($qcats as $qcat) {
-            $results[$qcat->itemid] = backup_controller_dbops::decode_backup_temp_info($qcat->info);
+            $result = backup_controller_dbops::decode_backup_temp_info($qcat->info);
+            // Filter out found categories that belong to another context level.
+            // (this can happen when a higher level category becomes remapped to
+            // a context id that, by coincidence, matches a context id of another
+            // category at lower level). See MDL-72950 for more info.
+            if ($result->contextlevel == $contextlevel) {
+                $results[$qcat->itemid] = $result;
+            }
         }
         $qcats->close();
 

From 63b2bed3f02c86045239055340b7f22561c69154 Mon Sep 17 00:00:00 2001
From: "Eloy Lafuente (stronk7)" <stronk7@moodle.org>
Date: Sun, 31 Oct 2021 16:24:54 +0100
Subject: [PATCH 2/2] MDL-72950 phpunit: Remove skipped tests now that the
 issue is fixed.

The problem (coincidence of contexts) was detected when running
oracle phpunit tests, so we skipped the tests as part of MDL-72743.

Now, with the problem fixed (previous commit), we can bring back
that test.
---
 mod/quiz/tests/restore_attempt_test.php | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/mod/quiz/tests/restore_attempt_test.php b/mod/quiz/tests/restore_attempt_test.php
index 2ed2e854b30..5fd21c18c9b 100644
--- a/mod/quiz/tests/restore_attempt_test.php
+++ b/mod/quiz/tests/restore_attempt_test.php
@@ -47,11 +47,6 @@ class restore_attempt_test extends \advanced_testcase {
     public function test_restore_question_attempts_missing_users(): void {
         global $DB, $USER;
 
-        // TODO: Remove this once MDL-72950 is fixed.
-        if ($DB->get_dbfamily() == 'oracle') {
-            $this->markTestSkipped("Skipping for Oracle until MDL-72950 is fixed.");
-        }
-
         $this->resetAfterTest();
         $this->setAdminUser();