Merge branch 'wip-MDL-63165-master' of https://github.com/Beedell/moodle

moodle · Aug 28, 2018 · 4f855defce4ba47786447e06490c5fa7fa37086c · 4f855de
2 parents efbbbf3 + d0a6044
commit 4f855defce4ba47786447e06490c5fa7fa37086c
Unified Split

Showing with 114 additions and 21 deletions.

+24 −0 lib/questionlib.php

+21 −0 lib/tests/questionlib_test.php

+68 −20 question/format.php

+1 −1 question/import.php
diff --git a/lib/questionlib.php b/lib/questionlib.php
@@ -1487,6 +1487,30 @@ function question_categorylist($categoryid) {
    return $categorylist;
 }

+/**
+ * Get all parent categories of a given question category in decending order.
+ * @param int $categoryid for which you want to find the parents.
+ * @return array of question category ids of all parents categories.
+ */
+function question_categorylist_parents(int $categoryid) {
+    global $DB;
+    $parent = $DB->get_field('question_categories', 'parent', array('id' => $categoryid));
+    if (!$parent) {
+        return [];
+    }
+    $categorylist = [$parent];
+    $currentid = $parent;
+    while ($currentid) {
+        $currentid = $DB->get_field('question_categories', 'parent', array('id' => $currentid));
+        if ($currentid) {
+            $categorylist[] = $currentid;
+        }
+    }
+    // Present the list in decending order (the top category at the top).
+    $categorylist = array_reverse($categorylist);
+    return $categorylist;
+}
+
 //===========================
 // Import/Export Functions
 //===========================
diff --git a/lib/tests/questionlib_test.php b/lib/tests/questionlib_test.php
@@ -1986,4 +1986,25 @@ public function test_question_has_capability_on_wrong_param_type() {
        $this->expectExceptionMessage('$questionorid parameter needs to be an integer or an object.');
        question_has_capability_on('one', 'tag');
    }
+
+    /**
+     * Test of question_categorylist_parents function.
+     */
+    public function test_question_categorylist_parents() {
+        $this->resetAfterTest();
+        $generator = $this->getDataGenerator();
+        $questiongenerator = $generator->get_plugin_generator('core_question');
+        $category = $generator->create_category();
+        $context = context_coursecat::instance($category->id);
+        // Create a top category.
+        $cat0 = question_get_top_category($context->id, true);
+        // Add sub-categories.
+        $cat1 = $questiongenerator->create_question_category(['parent' => $cat0->id]);
+        $cat2 = $questiongenerator->create_question_category(['parent' => $cat1->id]);
+        // Test the 'get parents' function.
+        $parentcategories = question_categorylist_parents($cat2->id);
+        $this->assertEquals($cat0->id, $parentcategories[0]);
+        $this->assertEquals($cat1->id, $parentcategories[1]);
+        $this->assertCount(2, $parentcategories);
+    }
 }
diff --git a/question/format.php b/question/format.php
@@ -282,11 +282,10 @@ public function importpreprocess() {
    /**
     * Process the file
     * This method should not normally be overidden
-     * @param object $category
     * @return bool success
     */
-    public function importprocess($category) {
-        global $USER, $CFG, $DB, $OUTPUT;
+    public function importprocess() {
+        global $USER, $DB, $OUTPUT;

        // Raise time and memory, as importing can be quite intensive.
        core_php_time_limit::raise();
@@ -543,10 +542,14 @@ protected function create_category_path($catpath) {
            } else if ($category = $DB->get_record('question_categories',
                    array('name' => $catname, 'contextid' => $context->id, 'parent' => $parent))) {
                $parent = $category->id;
-            } else if ($parent == 0) {
-                $category = question_get_top_category($context->id, true);
-                $parent = $category->id;
            } else {
+                if ($catname == 'top') {
+                    // Should not happen, but if it does just move on.
+                    // Occurs when there has been some import/export that has created
+                    // multiple nested 'top' categories (due to old bug solved by MDL-63165).
+                    // Not throwing an error here helps clean up old errors (silently).
+                    continue;
+                }
                require_capability('moodle/question:managecategory', $context);
                // create the new category
                $category = new stdClass();
@@ -556,9 +559,8 @@ protected function create_category_path($catpath) {
                $category->parent = $parent;
                $category->sortorder = 999;
                $category->stamp = make_unique_id_code();
-                $id = $DB->insert_record('question_categories', $category);
-                $category->id = $id;
-                $parent = $id;
+                $category->id = $DB->insert_record('question_categories', $category);
+                $parent = $category->id;
            }
        }
        return $category;
@@ -800,7 +802,13 @@ protected function presave_process($content) {
     * @return  string  The content of the export.
     */
    public function exportprocess($checkcapabilities = true) {
-        global $CFG, $OUTPUT, $DB, $USER;
+        global $CFG, $DB;
+
+        // Get the parents (from database) for this category.
+        $parents = [];
+        if ($this->category) {
+            $parents = question_categorylist_parents($this->category->id);
+        }

        // get the questions (from database) in this category
        // only get q's with no parents (no cloze subquestions specifically)
@@ -821,7 +829,17 @@ public function exportprocess($checkcapabilities = true) {
        // file if selected. 0 means that it will get printed before the 1st question
        $trackcategory = 0;

-        // iterate through questions
+        // Array of categories written to file.
+        $writtencategories = [];
+
+        foreach ($parents as $parent) {
+            $categoryname = $this->get_category_path($parent, $this->contexttofile);
+            // Create 'dummy' question for category export.
+            $dummyquestion = $this->create_dummy_question_representing_category($categoryname);
+            $expout .= $this->writequestion($dummyquestion) . "\n";
+            $writtencategories[] = $parent;
+        }
+
        foreach ($questions as $question) {
            // used by file api
            $contextid = $DB->get_field('question_categories', 'contextid',
@@ -840,19 +858,33 @@ public function exportprocess($checkcapabilities = true) {

            // check if we need to record category change
            if ($this->cattofile) {
+                $addnewcat = false;
                if ($question->category != $trackcategory) {
+                    $addnewcat = true;
                    $trackcategory = $question->category;
                    $categoryname = $this->get_category_path($trackcategory, $this->contexttofile);
-
-                    // create 'dummy' question for category export
-                    $dummyquestion = new stdClass();
-                    $dummyquestion->qtype = 'category';
-                    $dummyquestion->category = $categoryname;
-                    $dummyquestion->name = 'Switch category to ' . $categoryname;
-                    $dummyquestion->id = 0;
-                    $dummyquestion->questiontextformat = '';
-                    $dummyquestion->contextid = 0;
+                }
+                $trackcategoryparents = question_categorylist_parents($trackcategory);
+                // Check if we need to record empty parents categories.
+                foreach ($trackcategoryparents as $trackcategoryparent) {
+                    // If parent wasn't written.
+                    if (!in_array($trackcategoryparent, $writtencategories)) {
+                        // If parent is empty.
+                        if (!count($DB->get_records('question', array('category' => $trackcategoryparent)))) {
+                            $categoryname = $this->get_category_path($trackcategoryparent, $this->contexttofile);
+                            // Create 'dummy' question for parent category.
+                            $dummyquestion = $this->create_dummy_question_representing_category($categoryname);
+                            $expout .= $this->writequestion($dummyquestion) . "\n";
+                            $writtencategories[] = $trackcategoryparent;
+                        }
+                    }
+                }
+                if ($addnewcat && !in_array($trackcategory, $writtencategories)) {
+                    $categoryname = $this->get_category_path($trackcategory, $this->contexttofile);
+                    // Create 'dummy' question for category.
+                    $dummyquestion = $this->create_dummy_question_representing_category($categoryname);
                    $expout .= $this->writequestion($dummyquestion) . "\n";
+                    $writtencategories[] = $trackcategory;
                }
            }

@@ -878,6 +910,22 @@ public function exportprocess($checkcapabilities = true) {
        return $expout;
    }

+    /**
+     * Create 'dummy' question for category export.
+     * @param string $categoryname the name of the category
+     * @return stdClass 'dummy' question for category
+     */
+    protected function create_dummy_question_representing_category(string $categoryname) {
+        $dummyquestion = new stdClass();
+        $dummyquestion->qtype = 'category';
+        $dummyquestion->category = $categoryname;
+        $dummyquestion->id = 0;
+        $dummyquestion->questiontextformat = '';
+        $dummyquestion->contextid = 0;
+        $dummyquestion->name = 'Switch category to ' . $categoryname;
+        return $dummyquestion;
+    }
+
    /**
     * get the category as a path (e.g., tom/dick/harry)
     * @param int id the id of the most nested catgory
diff --git a/question/import.php b/question/import.php
@@ -120,7 +120,7 @@
    }

    // Process the uploaded file
-    if (!$qformat->importprocess($category)) {
+    if (!$qformat->importprocess()) {
        print_error('cannotimport', '', $thispageurl->out());
    }