diff --git a/question/editlib.php b/question/editlib.php index da255f515dd..d0dfbf87c74 100644 --- 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 index d0091aaa6a3..da31797397f 100644 --- 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 @@ class qformat_default { $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 @@ class qformat_default { 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 @@ class qformat_default { } // 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 @@ class qformat_default { 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 @@ class qformat_default { $count++; - echo "
{$count}. ".$this->format_question_text($question)."
"; + if ($this->displayprogress) { + echo "{$count}. " . $this->format_question_text($question) . "
"; + } $question->category = $this->category->id; $question->stamp = make_unique_id_code(); // Set the unique code (not to be changed) @@ -502,10 +521,10 @@ class qformat_default { * 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 @@ class qformat_default { $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 @@ class qformat_default { // 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 @@ class qformat_default { 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 @@ class qformat_default { // 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 @@ class qformat_default { /** * 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 index 720ef49cfb7..2817dc46fda 100644 --- 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 index 48dd14cebbb..70760e6b8a6 100644 --- 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: +TestingAlphaQuestion
'], + 'generalfeedback' => ['format' => '1', 'text' => ''], + 'correctanswer' => '1', + 'feedbacktrue' => ['format' => '1', 'text' => ''], + 'feedbackfalse' => ['format' => '1', 'text' => ''], + 'penalty' => '1']); + $qformat->setCategory($category); + + $xml = preg_replace('/()/', '', $qformat->exportprocess()); + $file = preg_replace('/()/', '', + file_get_contents(__DIR__ . '/fixtures/export_category.xml')); + $this->assert_same_xml($file, $xml); + } + + /** + * Check exporting nested categories. + */ + public function test_export_nested_categories() { + $this->resetAfterTest(true); + $course = $this->getDataGenerator()->create_course(); + $this->setAdminUser(); + $generator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $qformat = $this->create_qformat('nested_categories.zml', $course); + + $categorydelta = $generator->create_question_category([ + 'name' => 'Delta', + 'contextid' => '2', + 'info' => 'This is Delta category for test', + 'infoformat' => '2', + 'stamp' => make_unique_id_code(), + 'parent' => '0', + 'sortorder' => '999']); + $categoryepsilon = $generator->create_question_category([ + 'name' => 'Epsilon', + 'contextid' => '2', + 'info' => 'This is Epsilon category for test', + 'infoformat' => '4', + 'stamp' => make_unique_id_code(), + 'parent' => $categorydelta->id, + 'sortorder' => '999']); + $categoryzeta = $generator->create_question_category([ + 'name' => 'Zeta', + 'contextid' => '2', + 'info' => 'This is Zeta category for test', + 'infoformat' => '0', + 'stamp' => make_unique_id_code(), + 'parent' => $categoryepsilon->id, + 'sortorder' => '999']); + $question = $generator->create_question('truefalse', null, [ + 'category' => $categoryzeta->id, + 'name' => 'Zeta Question', + 'questiontext' => [ + 'format' => '1', + 'text' => 'Testing Zeta Question
'], + 'generalfeedback' => ['format' => '1', 'text' => ''], + 'correctanswer' => '1', + 'feedbacktrue' => ['format' => '1', 'text' => ''], + 'feedbackfalse' => ['format' => '1', 'text' => ''], + 'penalty' => '1']); + $qformat->setCategory($categorydelta); + $qformat->setCategory($categoryepsilon); + $qformat->setCategory($categoryzeta); + + $xml = preg_replace('/()/', '', $qformat->exportprocess()); + $file = preg_replace('/()/', '', + file_get_contents(__DIR__ . '/fixtures/nested_categories.xml')); + $this->assert_same_xml($file, $xml); + } + + /** + * Check exporting nested categories contain the right questions. + */ + public function test_export_nested_categories_with_questions() { + $this->resetAfterTest(true); + $course = $this->getDataGenerator()->create_course(); + $this->setAdminUser(); + $generator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $qformat = $this->create_qformat('nested_categories_with_questions.xml', $course); + + $categoryiota = $generator->create_question_category([ + 'name' => 'Iota', + 'contextid' => '2', + 'info' => 'This is Iota category for test', + 'infoformat' => '2', + 'stamp' => make_unique_id_code(), + 'parent' => '0', + 'sortorder' => '999']); + $iotaquestion = $generator->create_question('truefalse', null, [ + 'category' => $categoryiota->id, + 'name' => 'Iota Question', + 'questiontext' => [ + 'format' => '1', + 'text' => 'Testing Iota Question
'], + 'generalfeedback' => ['format' => '1', 'text' => ''], + 'correctanswer' => '1', + 'feedbacktrue' => ['format' => '1', 'text' => ''], + 'feedbackfalse' => ['format' => '1', 'text' => ''], + 'penalty' => '1']); + $categorykappa = $generator->create_question_category([ + 'name' => 'Kappa', + 'contextid' => '2', + 'info' => 'This is Kappa category for test', + 'infoformat' => '4', + 'stamp' => make_unique_id_code(), + 'parent' => $categoryiota->id, + 'sortorder' => '999']); + $kappaquestion = $generator->create_question('essay', null, [ + 'category' => $categorykappa->id, + 'name' => 'Kappa Essay Question', + 'questiontext' => ['text' => 'Testing Kappa Essay Question'], + 'generalfeedback' => '', + 'responseformat' => 'editor', + 'responserequired' => 1, + 'responsefieldlines' => 10, + 'attachments' => 0, + 'attachmentsrequired' => 0, + 'graderinfo' => ['format' => '1', 'text' => ''], + 'responsetemplate' => ['format' => '1', 'text' => ''], + ]); + $kappaquestion1 = $generator->create_question('truefalse', null, [ + 'category' => $categorykappa->id, + 'name' => 'Kappa Question', + 'questiontext' => [ + 'format' => '1', + 'text' => 'Testing Kappa Question
'], + 'generalfeedback' => ['format' => '1', 'text' => ''], + 'correctanswer' => '1', + 'feedbacktrue' => ['format' => '1', 'text' => ''], + 'feedbackfalse' => ['format' => '1', 'text' => ''], + 'penalty' => '1']); + $categorylambda = $generator->create_question_category([ + 'name' => 'Lambda', + 'contextid' => '2', + 'info' => 'This is Lambda category for test', + 'infoformat' => '0', + 'stamp' => make_unique_id_code(), + 'parent' => $categorykappa->id, + 'sortorder' => '999']); + $lambdaquestion = $generator->create_question('truefalse', null, [ + 'category' => $categorylambda->id, + 'name' => 'Lambda Question', + 'questiontext' => [ + 'format' => '1', + 'text' => 'Testing Lambda Question
'], + 'generalfeedback' => ['format' => '1', 'text' => ''], + 'correctanswer' => '1', + 'feedbacktrue' => ['format' => '1', 'text' => ''], + 'feedbackfalse' => ['format' => '1', 'text' => ''], + 'penalty' => '1']); + $categorymu = $generator->create_question_category([ + 'name' => 'Mu', + 'contextid' => '2', + 'info' => 'This is Mu category for test', + 'infoformat' => '0', + 'stamp' => make_unique_id_code(), + 'parent' => $categoryiota->id, + 'sortorder' => '999']); + $muquestion = $generator->create_question('truefalse', null, [ + 'category' => $categorymu->id, + 'name' => 'Mu Question', + 'questiontext' => [ + 'format' => '1', + 'text' => 'Testing Mu Question
'], + 'generalfeedback' => ['format' => '1', 'text' => ''], + 'correctanswer' => '1', + 'feedbacktrue' => ['format' => '1', 'text' => ''], + 'feedbackfalse' => ['format' => '1', 'text' => ''], + 'penalty' => '1']); + $qformat->setCategory($categoryiota); + + $xml = preg_replace('/()/', '', $qformat->exportprocess()); + $file = preg_replace('/()/', '', + file_get_contents(__DIR__ . '/fixtures/nested_categories_with_questions.xml')); + $this->assert_same_xml($file, $xml); + } +} diff --git a/question/format/xml/tests/xmlformat_test.php b/question/format/xml/tests/xmlformat_test.php index 5b923a0d48f..6732cbc9545 100644 --- a/question/format/xml/tests/xmlformat_test.php +++ b/question/format/xml/tests/xmlformat_test.php @@ -1583,4 +1583,40 @@ END; $this->assertEquals('/myfolder/', $file->filepath); $this->assertEquals(6, $file->size); } + + public function test_create_dummy_question() { + + $testobject = new mock_qformat_xml(); + $categoryname = 'name1'; + $categoryinfo = new stdClass(); + $categoryinfo->info = 'info1'; + $categoryinfo->infoformat = 'infoformat1'; + $dummyquestion = $testobject->mock_create_dummy_question_representing_category($categoryname, $categoryinfo); + + $this->assertEquals('category', $dummyquestion->qtype); + $this->assertEquals($categoryname, $dummyquestion->category); + $this->assertEquals($categoryinfo->info, $dummyquestion->info); + $this->assertEquals($categoryinfo->infoformat, $dummyquestion->infoformat); + $this->assertEquals('Switch category to ' . $categoryname, $dummyquestion->name); + $this->assertEquals(0, $dummyquestion->id); + $this->assertEquals('', $dummyquestion->questiontextformat); + $this->assertEquals(0, $dummyquestion->contextid); + } +} + +/** + * Class mock_qformat_xml exists only to enable testing of the create dummy question category. + * @package qformat_xml + * @copyright 2018 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mock_qformat_xml extends qformat_xml { + /** + * Make public an otherwise protected function. + * @param string $categoryname the name of the category + * @param object $categoryinfo description of the category + */ + public function mock_create_dummy_question_representing_category(string $categoryname, $categoryinfo) { + return $this->create_dummy_question_representing_category($categoryname, $categoryinfo); + } }