MDL-23545 question: XML import/export add category description

thebenkahn · Sep 14, 2018 · 1dab8fa · 1dab8fa
1 parent 674ef9b
commit 1dab8fa
Show file tree

Hide file tree

Showing 13 changed files with 1,037 additions and 28 deletions.
diff --git a/question/editlib.php b/question/editlib.php
@@ -76,7 +76,7 @@ function get_questions_category( $category, $noparent=false, $recurse=true, $exp
 
     // Get the list of questions for the category
     list($usql, $params) = $DB->get_in_or_equal($categorylist);
-    $questions = $DB->get_records_select('question', "category {$usql} {$npsql}", $params, 'qtype, name');
+    $questions = $DB->get_records_select('question', "category {$usql} {$npsql}", $params, 'category, qtype, name');
 
     // Iterate through questions, getting stuff we need
     $qresults = array();

diff --git a/question/format.php b/question/format.php
@@ -52,6 +52,8 @@ class qformat_default {
     public $translator = null;
     public $canaccessbackupdata = true;
     protected $importcontext = null;
+    /** @var bool $displayprogress Whether to display progress. */
+    public $displayprogress = true;
 
     // functions to indicate import/export functionality
     // override to return true if implemented
@@ -210,6 +212,17 @@ public function set_can_access_backupdata($canaccess) {
         $this->canaccessbackupdata = $canaccess;
     }
 
+    /**
+     * Change whether to display progress messages.
+     * There is normally no need to use this function as the
+     * default for $displayprogress is true.
+     * Set to false for unit tests.
+     * @param bool $displayprogress
+     */
+    public function set_display_progress($displayprogress) {
+        $this->displayprogress = $displayprogress;
+    }
+
     /***********************
      * IMPORTING FUNCTIONS
      ***********************/
@@ -292,7 +305,9 @@ public function importprocess() {
         raise_memory_limit(MEMORY_EXTRA);
 
         // STAGE 1: Parse the file
-        echo $OUTPUT->notification(get_string('parsingquestions', 'question'), 'notifysuccess');
+        if ($this->displayprogress) {
+            echo $OUTPUT->notification(get_string('parsingquestions', 'question'), 'notifysuccess');
+        }
 
         if (! $lines = $this->readdata($this->filename)) {
             echo $OUTPUT->notification(get_string('cannotread', 'question'));
@@ -305,8 +320,10 @@ public function importprocess() {
         }
 
         // STAGE 2: Write data to database
-        echo $OUTPUT->notification(get_string('importingquestions', 'question',
-                $this->count_questions($questions)), 'notifysuccess');
+        if ($this->displayprogress) {
+            echo $OUTPUT->notification(get_string('importingquestions', 'question',
+                    $this->count_questions($questions)), 'notifysuccess');
+        }
 
         // check for errors before we continue
         if ($this->stoponerror and ($this->importerrors>0)) {
@@ -366,7 +383,7 @@ public function importprocess() {
                 if ($this->catfromfile) {
                     // find/create category object
                     $catpath = $question->category;
-                    $newcategory = $this->create_category_path($catpath);
+                    $newcategory = $this->create_category_path($catpath, $question);
                     if (!empty($newcategory)) {
                         $this->category = $newcategory;
                     }
@@ -378,7 +395,9 @@ public function importprocess() {
 
             $count++;
 
-            echo "<hr /><p><b>{$count}</b>. ".$this->format_question_text($question)."</p>";
+            if ($this->displayprogress) {
+                echo "<hr /><p><b>{$count}</b>. " . $this->format_question_text($question) . "</p>";
+            }
 
             $question->category = $this->category->id;
             $question->stamp = make_unique_id_code();  // Set the unique code (not to be changed)
@@ -502,10 +521,10 @@ protected function count_questions($questions) {
      * but if $getcontext is set then ignore the context and use selected category context.
      *
      * @param string catpath delimited category path
-     * @param int courseid course to search for categories
+     * @param object $lastcategoryinfo Contains category information
      * @return mixed category object or null if fails
      */
-    protected function create_category_path($catpath) {
+    protected function create_category_path($catpath, $lastcategoryinfo = null) {
         global $DB;
         $catnames = $this->split_category_path($catpath);
         $parent = 0;
@@ -535,27 +554,47 @@ protected function create_category_path($catpath) {
         $this->importcontext = $context;
 
         // Now create any categories that need to be created.
-        foreach ($catnames as $catname) {
+        foreach ($catnames as $key => $catname) {
             if ($parent == 0) {
                 $category = question_get_top_category($context->id, true);
                 $parent = $category->id;
             } else if ($category = $DB->get_record('question_categories',
                     array('name' => $catname, 'contextid' => $context->id, 'parent' => $parent))) {
+                // Do nothing unless the child category appears before the parent category
+                // in the imported xml file. Because the parent was created without info being available
+                // at that time, this allows the info to be added from the xml data.
+                if ($key == (count($catnames) - 1) && $lastcategoryinfo && $lastcategoryinfo->info !== null &&
+                        $lastcategoryinfo->info !== "" && $category->info == "") {
+                    $category->info = $lastcategoryinfo->info;
+                    if ($lastcategoryinfo->infoformat !== null && $lastcategoryinfo->infoformat !== "") {
+                        $category->infoformat = $lastcategoryinfo->infoformat;
+                    }
+                    $DB->update_record('question_categories', $category);
+                }
                 $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).
+                    // This basically silently cleans up old errors. Not throwing an exception here.
                     continue;
                 }
                 require_capability('moodle/question:managecategory', $context);
-                // create the new category
+                // Create the new category. This will create all the categories in the catpath,
+                // though only the final category will have any info added if available.
                 $category = new stdClass();
                 $category->contextid = $context->id;
                 $category->name = $catname;
                 $category->info = '';
+                // Only add info (category description) for the final category in the catpath.
+                if ($key == (count($catnames) - 1) && $lastcategoryinfo && $lastcategoryinfo->info !== null &&
+                        $lastcategoryinfo->info !== "") {
+                    $category->info = $lastcategoryinfo->info;
+                    if ($lastcategoryinfo->infoformat !== null && $lastcategoryinfo->infoformat !== "") {
+                        $category->infoformat = $lastcategoryinfo->infoformat;
+                    }
+                }
                 $category->parent = $parent;
                 $category->sortorder = 999;
                 $category->stamp = make_unique_id_code();
@@ -832,14 +871,6 @@ public function exportprocess($checkcapabilities = true) {
         // 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',
@@ -862,7 +893,6 @@ public function exportprocess($checkcapabilities = true) {
                 if ($question->category != $trackcategory) {
                     $addnewcat = true;
                     $trackcategory = $question->category;
-                    $categoryname = $this->get_category_path($trackcategory, $this->contexttofile);
                 }
                 $trackcategoryparents = question_categorylist_parents($trackcategory);
                 // Check if we need to record empty parents categories.
@@ -872,17 +902,23 @@ public function exportprocess($checkcapabilities = true) {
                         // 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;
+                            $categoryinfo = $DB->get_record('question_categories', array('id' => $trackcategoryparent),
+                                'name, info, infoformat', MUST_EXIST);
+                            if ($categoryinfo->name != 'top') {
+                                // Create 'dummy' question for parent category.
+                                $dummyquestion = $this->create_dummy_question_representing_category($categoryname, $categoryinfo);
+                                $expout .= $this->writequestion($dummyquestion) . "\n";
+                                $writtencategories[] = $trackcategoryparent;
+                            }
                         }
                     }
                 }
                 if ($addnewcat && !in_array($trackcategory, $writtencategories)) {
                     $categoryname = $this->get_category_path($trackcategory, $this->contexttofile);
+                    $categoryinfo = $DB->get_record('question_categories', array('id' => $trackcategory),
+                            'info, infoformat', MUST_EXIST);
                     // Create 'dummy' question for category.
-                    $dummyquestion = $this->create_dummy_question_representing_category($categoryname);
+                    $dummyquestion = $this->create_dummy_question_representing_category($categoryname, $categoryinfo);
                     $expout .= $this->writequestion($dummyquestion) . "\n";
                     $writtencategories[] = $trackcategory;
                 }
@@ -913,15 +949,18 @@ public function exportprocess($checkcapabilities = true) {
     /**
      * Create 'dummy' question for category export.
      * @param string $categoryname the name of the category
+     * @param object $categoryinfo description of the category
      * @return stdClass 'dummy' question for category
      */
-    protected function create_dummy_question_representing_category(string $categoryname) {
+    protected function create_dummy_question_representing_category(string $categoryname, $categoryinfo) {
         $dummyquestion = new stdClass();
         $dummyquestion->qtype = 'category';
         $dummyquestion->category = $categoryname;
         $dummyquestion->id = 0;
         $dummyquestion->questiontextformat = '';
         $dummyquestion->contextid = 0;
+        $dummyquestion->info = $categoryinfo->info;
+        $dummyquestion->infoformat = $categoryinfo->infoformat;
         $dummyquestion->name = 'Switch category to ' . $categoryname;
         return $dummyquestion;
     }

diff --git a/question/format/gift/tests/behat/import_export.feature b/question/format/gift/tests/behat/import_export.feature
@@ -33,4 +33,4 @@ Feature: Test importing questions from GIFT format.
     And I follow "Export"
     And I set the field "id_format_gift" to "1"
     And I press "Export questions to file"
-    And following "click here" should download between "1650" and "1800" bytes
+    And following "click here" should download between "1600" and "1800" bytes
diff --git a/question/format/upgrade.txt b/question/format/upgrade.txt
@@ -1,5 +1,20 @@
 This files describes API changes for question import/export format plugins.
 
+=== 3.6 ===
+
+* Saving question category descriptions (info) is now supported in Moodle XML import/export format.
+New xml-structure snippet for a question category:
+<question type="category">
+	<category>
+		<text>${$contexttypename}$/{$category_path}</text>
+	</category>
+	<info format="{$format}">
+		<text>{$info_categorydescription}</text>
+	</info>
+</question>
+* The method importprocess() in question/format.php no longer accepts $category as a parameter.
+If required in a plugin then please override this method.
+
 === 2.3 ===
 
 * This plugin type now supports cron in the standard way. If required, Create a

diff --git a/question/format/xml/format.php b/question/format/xml/format.php
@@ -63,6 +63,7 @@ public function mime_type() {
     /**
      * Translate human readable format name
      * into internal Moodle code number
+     * Note the reverse function is called get_format.
      * @param string name format name from xml file
      * @return int Moodle format code
      */
@@ -909,12 +910,20 @@ public function import_calculated($question) {
      * import category. The format is:
      * <question type="category">
      *     <category>tom/dick/harry</category>
+     *     <info format="moodle_auto_format"><text>Category description</text></info>
      * </question>
      */
     protected function import_category($question) {
         $qo = new stdClass();
         $qo->qtype = 'category';
         $qo->category = $this->import_text($question['#']['category'][0]['#']['text']);
+        $qo->info = '';
+        $qo->infoformat = FORMAT_MOODLE;
+        if (array_key_exists('info', $question['#'])) {
+            $qo->info = $this->import_text($question['#']['info'][0]['#']['text']);
+            // The import should have the format in human readable form, so translate to machine readable format.
+            $qo->infoformat = $this->trans_format($question['#']['info'][0]['@']['format']);
+        }
         return $qo;
     }
 
@@ -1176,10 +1185,15 @@ public function writequestion($question) {
         // Categories are a special case.
         if ($question->qtype == 'category') {
             $categorypath = $this->writetext($question->category);
+            $categoryinfo = $this->writetext($question->info);
+            $infoformat = $this->format($question->infoformat);
             $expout .= "  <question type=\"category\">\n";
             $expout .= "    <category>\n";
-            $expout .= "        {$categorypath}\n";
+            $expout .= "        {$categorypath}";
             $expout .= "    </category>\n";
+            $expout .= "    <info {$infoformat}>\n";
+            $expout .= "        {$categoryinfo}";
+            $expout .= "    </info>\n";
             $expout .= "  </question>\n";
             return $expout;
         }

diff --git a/question/format/xml/tests/fixtures/categories_reverse_order.xml b/question/format/xml/tests/fixtures/categories_reverse_order.xml
@@ -0,0 +1,84 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<quiz>
+<!-- question: 0  -->
+  <question type="category">
+    <category>
+        <text>$course$/Sigma/Tau</text>
+    </category>
+    <info format="html">
+        <text>This is Tau category for test</text>
+    </info>
+  </question>
+
+<!-- question: 106  -->
+  <question type="essay">
+    <name>
+      <text>Tau Question</text>
+    </name>
+    <questiontext format="moodle_auto_format">
+      <text>Testing Tau Question</text>
+    </questiontext>
+    <generalfeedback format="moodle_auto_format">
+      <text></text>
+    </generalfeedback>
+    <defaultgrade>1.0000000</defaultgrade>
+    <penalty>0.0000000</penalty>
+    <hidden>0</hidden>
+    <responseformat>editor</responseformat>
+    <responserequired>1</responserequired>
+    <responsefieldlines>15</responsefieldlines>
+    <attachments>0</attachments>
+    <attachmentsrequired>0</attachmentsrequired>
+    <graderinfo format="html">
+      <text></text>
+    </graderinfo>
+    <responsetemplate format="html">
+      <text></text>
+    </responsetemplate>
+  </question>
+
+<!-- question: 0  -->
+  <question type="category">
+    <category>
+        <text>$course$/Sigma</text>
+    </category>
+    <info format="html">
+        <text>This is Sigma category for test</text>
+    </info>
+  </question>
+
+<!-- question: 105  -->
+  <question type="shortanswer">
+    <name>
+      <text>Sigma Question</text>
+    </name>
+    <questiontext format="moodle_auto_format">
+      <text>Testing Sigma Question</text>
+    </questiontext>
+    <generalfeedback format="moodle_auto_format">
+      <text></text>
+    </generalfeedback>
+    <defaultgrade>1.0000000</defaultgrade>
+    <penalty>0.3333333</penalty>
+    <hidden>0</hidden>
+    <usecase>0</usecase>
+    <answer fraction="100" format="moodle_auto_format">
+      <text>yes</text>
+      <feedback format="html">
+        <text></text>
+      </feedback>
+    </answer>
+    <answer fraction="0" format="moodle_auto_format">
+      <text>no</text>
+      <feedback format="html">
+        <text></text>
+      </feedback>
+    </answer>
+    <answer fraction="0" format="moodle_auto_format">
+      <text>may be</text>
+      <feedback format="html">
+        <text></text>
+      </feedback>
+    </answer>
+  </question>
+</quiz>