Permalink
Browse files

question import/export: MDL-22100 ' / etc. in category names confuse …

…the import/export.
  • Loading branch information...
1 parent 0b76235 commit 6ab47dd350d24775cd67dd0e9d32e0e1a66a2523 @timhunt timhunt committed Jul 2, 2010
Showing with 161 additions and 27 deletions.
  1. +73 −27 question/format.php
  2. +88 −0 question/simpletest/testimportexport.php
View
@@ -307,7 +307,7 @@ function importprocess() {
if ($this->catfromfile) {
// find/create category object
$catpath = $question->category;
- $newcategory = $this->create_category_path( $catpath, '/');
+ $newcategory = $this->create_category_path($catpath);
if (!empty($newcategory)) {
$this->category = $newcategory;
}
@@ -381,32 +381,33 @@ 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 string delimiter path delimiting character
* @param int courseid course to search for categories
* @return mixed category object or null if fails
*/
- function create_category_path($catpath, $delimiter='/') {
- $catpath = clean_param($catpath, PARAM_PATH);
- $catnames = explode($delimiter, $catpath);
+ function create_category_path($catpath) {
+ $catnames = $this->split_category_path($catpath);
$parent = 0;
$category = null;
// check for context id in path, it might not be there in pre 1.9 exports
$matchcount = preg_match('/^\$([a-z]+)\$$/', $catnames[0], $matches);
- if ($matchcount==1) {
+ if ($matchcount == 1) {
$contextid = $this->translator->string_to_context($matches[1]);
array_shift($catnames);
} else {
- $contextid = FALSE;
+ $contextid = false;
}
- if ($this->contextfromfile && ($contextid !== FALSE)){
+
+ if ($this->contextfromfile && $contextid !== false) {
$context = get_context_instance_by_id($contextid);
require_capability('moodle/question:add', $context);
} else {
$context = get_context_instance_by_id($this->category->contextid);
}
+
+ // Now create any categories that need to be created.
foreach ($catnames as $catname) {
- if ($category = get_record( 'question_categories', 'name', $catname, 'contextid', $context->id, 'parent', $parent)) {
+ if ($category = get_record('question_categories', 'name', $catname, 'contextid', $context->id, 'parent', $parent)) {
$parent = $category->id;
} else {
require_capability('moodle/question:managecategory', $context);
@@ -697,16 +698,16 @@ function exportprocess() {
if ($this->cattofile) {
if ($question->category != $trackcategory) {
$trackcategory = $question->category;
- $categoryname = $this->get_category_path($trackcategory, '/', $this->contexttofile);
+ $categoryname = $this->get_category_path($trackcategory, $this->contexttofile);
// create 'dummy' question for category export
$dummyquestion = new object;
$dummyquestion->qtype = 'category';
$dummyquestion->category = $categoryname;
- $dummyquestion->name = "switch category to $categoryname";
+ $dummyquestion->name = 'Switch category to ' . $categoryname;
$dummyquestion->id = 0;
$dummyquestion->questiontextformat = '';
- $expout .= $this->writequestion( $dummyquestion ) . "\n";
+ $expout .= $this->writequestion($dummyquestion) . "\n";
}
}
@@ -745,34 +746,79 @@ function exportprocess() {
/**
* get the category as a path (e.g., tom/dick/harry)
* @param int id the id of the most nested catgory
- * @param string delimiter the delimiter you want
* @return string the path
*/
- function get_category_path($id, $delimiter='/', $includecontext = true) {
- $path = '';
- if (!$firstcategory = get_record('question_categories','id',$id)) {
- error( "Error getting category record from db - " . $id );
+ function get_category_path($id, $includecontext = true) {
+
+ if (!$category = get_record('question_categories','id',$id)) {
+ error('Error getting category record from db - ' . $id);
}
- $category = $firstcategory;
$contextstring = $this->translator->context_to_string($category->contextid);
+
+ $pathsections = array();
do {
- $name = $category->name;
+ $pathsections[] = $category->name;
$id = $category->parent;
- if (!empty($path)) {
- $path = "{$name}{$delimiter}{$path}";
- }
- else {
- $path = $name;
- }
- } while ($category = get_record( 'question_categories','id',$id ));
+ } while ($category = get_record('question_categories', 'id', $id));
if ($includecontext){
- $path = '$'.$contextstring.'$'."{$delimiter}{$path}";
+ $pathsections[] = '$' . $contextstring . '$';
}
+
+ $path = $this->assemble_category_path(array_reverse($pathsections));
+
return $path;
}
/**
+ * Convert a list of category names, possibly preceeded by one of the
+ * context tokens like $course$, into a string representation of the
+ * category path.
+ *
+ * Names are separated by / delimiters. And /s in the name are replaced by //.
+ *
+ * To reverse the process and split the paths into names, use
+ * {@link split_category_path()}.
+ *
+ * @param array $names
+ * @return string
+ */
+ protected function assemble_category_path($names) {
+ $escapednames = array();
+ foreach ($names as $name) {
+ $escapedname = str_replace('/', '//', $name);
+ if (substr($escapedname, 0, 1) == '/') {
+ $escapedname = ' ' . $escapedname;
+ }
+ if (substr($escapedname, -1) == '/') {
+ $escapedname = $escapedname . ' ';
+ }
+ $escapednames[] = $escapedname;
+ }
+ return implode('/', $escapednames);
+ }
+
+ /**
+ * Convert a string, as returned by {@link assemble_category_path()},
+ * back into an array of category names.
+ *
+ * Each category name is cleaned by a call to clean_param(, PARAM_MULTILANG),
+ * which matches the cleaning in question/category_form.php. Not that this
+ * addslashes the names, ready for insertion into the database.
+ *
+ * @param string $path
+ * @return array of category names.
+ */
+ protected function split_category_path($path) {
+ $rawnames = preg_split('~(?<!/)/(?!/)~', $path);
+ $names = array();
+ foreach ($rawnames as $rawname) {
+ $names[] = clean_param(trim(str_replace('//', '/', $rawname)), PARAM_MULTILANG);
+ }
+ return $names;
+ }
+
+ /**
* Do an post-processing that may be required
* @return boolean success
*/
@@ -0,0 +1,88 @@
+<?php
+
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
+
+
+/**
+ * Unit tests for the question import and export system.
+ *
+ * @package core
+ * @subpackage questionbank
+ * @copyright 2010 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+require_once($CFG->libdir . '/questionlib.php');
+require_once($CFG->dirroot . '/question/format.php');
+
+
+/**
+ * Subclass to make it easier to test qformat_default.
+ *
+ * @copyright 2009 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class testable_qformat extends qformat_default {
+ public function assemble_category_path($names) {
+ return parent::assemble_category_path($names);
+ }
+
+ public function split_category_path($names) {
+ return parent::split_category_path($names);
+ }
+}
+
+
+/**
+ * Unit tests for the matching question definition class.
+ *
+ * @copyright 2009 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class qformat_default_test extends UnitTestCase {
+ public function test_assemble_category_path() {
+ $format = new testable_qformat();
+ $pathsections = array(
+ '$course$',
+ "Tim's questions",
+ "Tricky things like / // and so on",
+ 'Category name ending in /',
+ '/ and one that starts with one',
+ '<span lang="en" class="multilang">Matematically</span> <span lang="sv" class="multilang">Matematiskt (svenska)</span>'
+ );
+ $this->assertEqual('$course$/Tim\'s questions/Tricky things like // //// and so on/Category name ending in // / // and one that starts with one/<span lang="en" class="multilang">Matematically<//span> <span lang="sv" class="multilang">Matematiskt (svenska)<//span>',
+ $format->assemble_category_path($pathsections));
+ }
+
+ public function test_split_category_path() {
+ $format = new testable_qformat();
+ $path = '$course$/Tim\'s questions/Tricky things like // //// and so on/Category name ending in // / // and one that starts with one/<span lang="en" class="multilang">Matematically<//span> <span lang="sv" class="multilang">Matematiskt (svenska)<//span>';
+ $this->assertEqual(array(
+ '$course$',
+ "Tim\\'s questions",
+ "Tricky things like / // and so on",
+ 'Category name ending in /',
+ '/ and one that starts with one',
+ '<span lang=\"en\" class=\"multilang\">Matematically</span> <span lang=\"sv\" class=\"multilang\">Matematiskt (svenska)</span>'
+ ), $format->split_category_path($path));
+ }
+
+ public function test_split_category_path_cleans() {
+ $format = new testable_qformat();
+ $path = '<evil>Nasty <virus //> thing<//evil>';
+ $this->assertEqual(array('Nasty thing'), $format->split_category_path($path));
+ }
+}

0 comments on commit 6ab47dd

Please sign in to comment.