From 4d18892676f1cbf5ee3eff6ca7c84b8b3eaa8768 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Tue, 27 Mar 2012 17:41:14 +0100 Subject: [PATCH] MDL-32220 question import: files sometimes stored in the wrong context. Sadly, this involves a small API change, but I don't believe anyone was using the argument I had to remove (because we were sometimes passing a wrong value, and there is not way to compute the right value at that point in the code.) Also sadly, the code to compute the context we are importing into is now rather spaghetti-like, but it works. --- question/format.php | 24 ++++++++++++++---------- question/format/aiken/format.php | 2 +- question/format/examview/format.php | 2 +- question/format/learnwise/format.php | 2 +- question/format/multianswer/format.php | 2 +- question/format/upgrade.txt | 8 ++++++++ question/format/webct/format.php | 2 +- question/format/xml/format.php | 2 +- 8 files changed, 28 insertions(+), 16 deletions(-) diff --git a/question/format.php b/question/format.php index 221db62e2da..2e3e3e2d4d4 100644 --- a/question/format.php +++ b/question/format.php @@ -128,6 +128,7 @@ class qformat_default { debugging('You shouldn\'t call setCategory after setQuestions'); } $this->category = $category; + $this->importcontext = get_context_instance_by_id($this->category->contextid); } /** @@ -311,9 +312,6 @@ class qformat_default { public function importprocess($category) { global $USER, $CFG, $DB, $OUTPUT; - $context = $category->context; - $this->importcontext = $context; - // reset the timer in case file upload was slow set_time_limit(0); @@ -325,7 +323,7 @@ class qformat_default { return false; } - if (!$questions = $this->readquestions($lines, $context)) { // Extract all the questions + if (!$questions = $this->readquestions($lines)) { // Extract all the questions echo $OUTPUT->notification(get_string('noquestionsinfile', 'question')); return false; } @@ -397,7 +395,7 @@ class qformat_default { } continue; } - $question->context = $context; + $question->context = $this->importcontext; $count++; @@ -415,13 +413,13 @@ class qformat_default { if (isset($question->questiontextfiles)) { foreach ($question->questiontextfiles as $file) { question_bank::get_qtype($question->qtype)->import_file( - $context, 'question', 'questiontext', $question->id, $file); + $this->importcontext, 'question', 'questiontext', $question->id, $file); } } if (isset($question->generalfeedbackfiles)) { foreach ($question->generalfeedbackfiles as $file) { question_bank::get_qtype($question->qtype)->import_file( - $context, 'question', 'generalfeedback', $question->id, $file); + $this->importcontext, 'question', 'generalfeedback', $question->id, $file); } } @@ -507,6 +505,7 @@ class qformat_default { } else { $context = get_context_instance_by_id($this->category->contextid); } + $this->importcontext = $context; // Now create any categories that need to be created. foreach ($catnames as $catname) { @@ -556,14 +555,19 @@ class qformat_default { * readquestion(). Questions are defined as anything * between blank lines. * + * NOTE this method used to take $context as a second argument. However, at + * the point where this method was called, it was impossible to know what + * context the quetsions were going to be saved into, so the value could be + * wrong. Also, none of the standard question formats were using this argument, + * so it was removed. See MDL-32220. + * * If your format does not use blank lines as a delimiter * then you will need to override this method. Even then * try to use readquestion for each question * @param array lines array of lines from readdata - * @param object $context * @return array array of question objects */ - protected function readquestions($lines, $context) { + protected function readquestions($lines) { $questions = array(); $currentquestion = array(); @@ -583,7 +587,7 @@ class qformat_default { } if (!empty($currentquestion)) { // There may be a final question - if ($question = $this->readquestion($currentquestion, $context)) { + if ($question = $this->readquestion($currentquestion)) { $questions[] = $question; } } diff --git a/question/format/aiken/format.php b/question/format/aiken/format.php index 06afee43e8f..4e64c2f6a93 100644 --- a/question/format/aiken/format.php +++ b/question/format/aiken/format.php @@ -58,7 +58,7 @@ class qformat_aiken extends qformat_default { return true; } - public function readquestions($lines, $context) { + public function readquestions($lines) { $questions = array(); $question = $this->defaultquestion(); $endchar = chr(13); diff --git a/question/format/examview/format.php b/question/format/examview/format.php index ba51cbc0c0c..dc0481a20cd 100644 --- a/question/format/examview/format.php +++ b/question/format/examview/format.php @@ -149,7 +149,7 @@ class qformat_examview extends qformat_default { return str_replace('’', "'", $text); } - protected function readquestions($lines, $context) { + protected function readquestions($lines) { /// Parses an array of lines into an array of questions, /// where each item is a question object as defined by /// readquestion(). diff --git a/question/format/learnwise/format.php b/question/format/learnwise/format.php index d625ae8b7bb..fa804756af6 100644 --- a/question/format/learnwise/format.php +++ b/question/format/learnwise/format.php @@ -46,7 +46,7 @@ class qformat_learnwise extends qformat_default { return true; } - protected function readquestions($lines, $context) { + protected function readquestions($lines) { $questions = array(); $currentquestion = array(); diff --git a/question/format/multianswer/format.php b/question/format/multianswer/format.php index d8a94045404..45179d8d9f6 100644 --- a/question/format/multianswer/format.php +++ b/question/format/multianswer/format.php @@ -40,7 +40,7 @@ class qformat_multianswer extends qformat_default { return true; } - protected function readquestions($lines, $context) { + protected function readquestions($lines) { // For this class the method has been simplified as // there can never be more than one question for a // multianswer import diff --git a/question/format/upgrade.txt b/question/format/upgrade.txt index 595dbff7f50..b31a7a08ad8 100644 --- a/question/format/upgrade.txt +++ b/question/format/upgrade.txt @@ -1,5 +1,13 @@ This files describes API changes for question import/export format plugins. +=== 2.1.5 / 2.2.3 / 2.3 === + +* The readquestions method used to take a second argument $context. However, at + the point where this method was called, it was impossible to know what + context the quetsions were going to be saved into, so the value could be + wrong. Also, none of the standard question formats were using this argument, + so it was removed. See MDL-32220. + === 2.2 === * The plugin name used to be defined in a string called the same thing as the diff --git a/question/format/webct/format.php b/question/format/webct/format.php index b00063f56d9..c599ca8bbf9 100644 --- a/question/format/webct/format.php +++ b/question/format/webct/format.php @@ -172,7 +172,7 @@ class qformat_webct extends qformat_default { return true; } - protected function readquestions($lines, $context) { + protected function readquestions($lines) { $webctnumberregex = '[+-]?([0-9]+(\\.[0-9]*)?|\\.[0-9]+)((e|E|\\*10\\*\\*)([+-]?[0-9]+|\\([+-]?[0-9]+\\)))?'; diff --git a/question/format/xml/format.php b/question/format/xml/format.php index d7508641ca9..740279f057f 100644 --- a/question/format/xml/format.php +++ b/question/format/xml/format.php @@ -889,7 +889,7 @@ class qformat_xml extends qformat_default { * @param stdClass $context * @return array (of objects) question objects. */ - protected function readquestions($lines, $context) { + protected function readquestions($lines) { // We just need it as one big string $text = implode($lines, ' '); unset($lines); -- 2.43.0