Skip to content

Commit

Permalink
MDL-32220 question import: files sometimes stored in the wrong context.
Browse files Browse the repository at this point in the history
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.

Conflicts:

	question/format/examview/format.php
	question/format/learnwise/format.php
	question/format/webct/format.php

Conflicts:

	question/format/upgrade.txt
  • Loading branch information
timhunt committed Mar 29, 2012
1 parent c54172b commit 64f4979
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 13 deletions.
24 changes: 14 additions & 10 deletions question/format.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ public function setCategory($category) {
debugging('You shouldn\'t call setCategory after setQuestions');
}
$this->category = $category;
$this->importcontext = get_context_instance_by_id($this->category->contextid);
}

/**
Expand Down Expand Up @@ -311,9 +312,6 @@ public function importpreprocess() {
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);

Expand All @@ -325,7 +323,7 @@ public function importprocess($category) {
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;
}
Expand Down Expand Up @@ -397,7 +395,7 @@ public function importprocess($category) {
}
continue;
}
$question->context = $context;
$question->context = $this->importcontext;

$count++;

Expand All @@ -415,13 +413,13 @@ public function importprocess($category) {
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);
}
}

Expand Down Expand Up @@ -507,6 +505,7 @@ protected function create_category_path($catpath) {
} 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) {
Expand Down Expand Up @@ -556,14 +555,19 @@ protected function readdata($filename) {
* 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();
Expand All @@ -583,7 +587,7 @@ protected function readquestions($lines, $context) {
}

if (!empty($currentquestion)) { // There may be a final question
if ($question = $this->readquestion($currentquestion, $context)) {
if ($question = $this->readquestion($currentquestion)) {
$questions[] = $question;
}
}
Expand Down
2 changes: 1 addition & 1 deletion question/format/examview/format.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ function cleanUnicode($text) {
return str_replace('’', "'", $text);
}

function readquestions($lines) {
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().
Expand Down
2 changes: 1 addition & 1 deletion question/format/learnwise/format.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function provide_import() {
return true;
}

function readquestions($lines) {
protected function readquestions($lines) {
$questions = array();
$currentquestion = array();

Expand Down
9 changes: 9 additions & 0 deletions question/format/upgrade.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
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 changes: 1 addition & 1 deletion question/format/webct/format.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ function provide_import() {
return true;
}

function readquestions ($lines) {
protected function readquestions($lines) {
$webctnumberregex =
'[+-]?([0-9]+(\\.[0-9]*)?|\\.[0-9]+)((e|E|\\*10\\*\\*)([+-]?[0-9]+|\\([+-]?[0-9]+\\)))?';

Expand Down

0 comments on commit 64f4979

Please sign in to comment.