' . get_string('youneedtoenrol', 'quiz') . "
\n\n" .
$OUTPUT->continue_button($CFG->wwwroot . '/course/view.php?id=' . $course->id) .
diff --git a/question/behaviour/behaviourbase.php b/question/behaviour/behaviourbase.php
index 1b366e7645c..da25319d274 100644
--- a/question/behaviour/behaviourbase.php
+++ b/question/behaviour/behaviourbase.php
@@ -118,7 +118,7 @@ abstract class question_behaviour {
*/
public function render(question_display_options $options, $number,
core_question_renderer $qoutput, qtype_renderer $qtoutput) {
- $behaviouroutput = $this->get_renderer();
+ $behaviouroutput = $this->get_renderer($qoutput->get_page());
$options = clone($options);
$this->adjust_display_options($options);
return $qoutput->question($this->qa, $behaviouroutput, $qtoutput, $options, $number);
@@ -139,11 +139,11 @@ abstract class question_behaviour {
}
/**
+ * @param moodle_page $page the page to render for.
* @return qbehaviour_renderer get the appropriate renderer to use for this model.
*/
- public function get_renderer() {
- global $PAGE;
- return $PAGE->get_renderer(get_class($this));
+ public function get_renderer(moodle_page $page) {
+ return $page->get_renderer(get_class($this));
}
/**
diff --git a/question/behaviour/missing/simpletest/testmissingbehaviour.php b/question/behaviour/missing/simpletest/testmissingbehaviour.php
index 33a1becb8bf..14527f66e87 100644
--- a/question/behaviour/missing/simpletest/testmissingbehaviour.php
+++ b/question/behaviour/missing/simpletest/testmissingbehaviour.php
@@ -40,21 +40,21 @@ require_once(dirname(__FILE__) . '/../behaviour.php');
*/
class qbehaviour_missing_test extends UnitTestCase {
public function test_missing_cannot_start() {
- $qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0, 0);
+ $qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0);
$behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
$this->expectException();
$behaviour->init_first_step(new question_attempt_step(array()));
}
public function test_missing_cannot_process() {
- $qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0, 0);
+ $qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0);
$behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
$this->expectException();
$behaviour->process_action(new question_attempt_pending_step(array()));
}
public function test_missing_cannot_get_min_grade() {
- $qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0, 0);
+ $qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0);
$behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
$this->expectException();
$behaviour->get_min_fraction();
diff --git a/question/behaviour/opaque/behaviour.php b/question/behaviour/opaque/behaviour.php
index c334433af5c..34348f77257 100644
--- a/question/behaviour/opaque/behaviour.php
+++ b/question/behaviour/opaque/behaviour.php
@@ -156,7 +156,7 @@ class qbehaviour_opaque extends question_behaviour {
$opaquestate = qtype_opaque_update_state($this->qa, $pendingstep);
} catch (SoapFault $sf) {
print_object($sf);
- return question_attempt::DISCARD; // TODO
+ return question_attempt::DISCARD; // TODO better Opaque error handling.
}
if ($opaquestate->resultssequencenumber != $this->qa->get_num_steps()) {
diff --git a/question/editlib.php b/question/editlib.php
index b11aced5539..4cc811ad19c 100644
--- a/question/editlib.php
+++ b/question/editlib.php
@@ -69,17 +69,16 @@ function get_questions_category( $category, $noparent=false, $recurse=true, $exp
$npsql = " and parent='0' ";
}
- // get (list) of categories
+ // Get list of categories
if ($recurse) {
$categorylist = question_categorylist($category->id);
- }
- else {
- $categorylist = $category->id;
+ } else {
+ $categorylist = array($category->id);
}
// get the list of questions for the category
- list ($usql, $params) = $DB->get_in_or_equal(explode(',', $categorylist));
- if ($questions = $DB->get_records_select("question","category $usql $npsql", $params, "qtype, name ASC")) {
+ list($usql, $params) = $DB->get_in_or_equal($categorylist);
+ if ($questions = $DB->get_records_select('question', "category $usql $npsql", $params, 'qtype, name')) {
// iterate through questions, getting stuff we need
foreach($questions as $question) {
@@ -1103,7 +1102,7 @@ class question_bank_view {
}
if ($recurse) {
- $categoryids = explode(',', question_categorylist($category->id));
+ $categoryids = question_categorylist($category->id);
} else {
$categoryids = array($category->id);
}
@@ -1302,7 +1301,7 @@ class question_bank_view {
$cmoptions->hasattempts = !empty($this->quizhasattempts);
$strselectall = get_string('selectall');
- $strselectnone = get_string('selectnone');
+ $strselectnone = get_string('deselectall');
$strdelete = get_string('delete');
list($categoryid, $contextid) = explode(',', $categoryandcontext);
@@ -1374,7 +1373,7 @@ class question_bank_view {
}
if ($canmoveall && count($addcontexts)) {
- echo '\n";
+ echo '\n";
question_category_select_menu($addcontexts, false, 0, "$category->id,$category->contextid");
}
@@ -1561,9 +1560,6 @@ class question_bank_view {
function question_edit_setup($edittab, $baseurl, $requirecmid = false, $requirecourseid = true) {
global $DB, $PAGE;
- //$thispageurl is used to construct urls for all question edit pages we link to from this page. It contains an array
- //of parameters that are passed from page to page.
-// $thispageurl = new moodle_url($PAGE->url); //TODO: this looks dumb, because this method is called BEFORE $PAGE->set_page() !!!!
$thispageurl = new moodle_url($baseurl);
$thispageurl->remove_all_params(); // We are going to explicity add back everything important - this avoids unwanted params from being retained.
diff --git a/question/engine/bank.php b/question/engine/bank.php
index c1e90606aa1..e01240cac1a 100644
--- a/question/engine/bank.php
+++ b/question/engine/bank.php
@@ -139,8 +139,9 @@ abstract class question_bank {
foreach (get_plugin_list('qtype') as $plugin => $notused) {
try {
$qtypes[$plugin] = self::get_qtype($plugin);
- } catch (Exception $e) {
- // TODO ingore, but review this later.
+ } catch (coding_exception $e) {
+ // Catching coding_exceptions here means that incompatible
+ // question types do not cause the rest of Moodle to break.
}
}
return $qtypes;
@@ -317,27 +318,26 @@ abstract class question_bank {
class question_finder {
/**
* Get the ids of all the questions in a list of categoryies.
- * @param int|string|array $categoryids either a categoryid, or a comma-separated list
+ * @param array $categoryids either a categoryid, or a comma-separated list
* category ids, or an array of them.
- * @param string $extraconditions extra conditions to AND with the rest of the where clause.
+ * @param string $extraconditions extra conditions to AND with the rest of
+ * the where clause. Must use named parameters.
+ * @param array $extraparams any parameters used by $extraconditions.
* @return array questionid => questionid.
*/
- public function get_questions_from_categories($categoryids, $extraconditions) {
+ public function get_questions_from_categories($categoryids, $extraconditions, $extraparams = array()) {
global $DB;
- if (is_array($categoryids)) {
- $categoryids = implode(',', $categoryids);
- }
+ list($qcsql, $qcparams) = $DB->get_in_or_equal($categoryids, SQL_PARAMS_NAMED, 'qc0000');
if ($extraconditions) {
$extraconditions = ' AND (' . $extraconditions . ')';
}
- // TODO switch to using $DB->in_or_equal.
- $questionids = $DB->get_records_select_menu('question',
- "category IN ($categoryids)
+
+ return $DB->get_records_select_menu('question',
+ "category $qcsql
AND parent = 0
AND hidden = 0
- $extraconditions", array(), '', 'id,id AS id2');
- return $questionids;
+ $extraconditions", $qcparams + $extraparams, '', 'id,id AS id2');
}
}
diff --git a/question/engine/datalib.php b/question/engine/datalib.php
index c20370a3ea6..a4af365afe0 100644
--- a/question/engine/datalib.php
+++ b/question/engine/datalib.php
@@ -747,7 +747,8 @@ ORDER BY
* this method.
*
* @param string $qubaid SQL fragment that controls which usage is summed.
- * This might be the name of a column in the outer query.
+ * This will normally be the name of a column in the outer query. Not that this
+ * SQL fragment must not contain any placeholders.
* @return string SQL code for the subquery.
*/
public function sum_usage_marks_subquery($qubaid) {
@@ -763,7 +764,6 @@ ORDER BY
JOIN {question_attempt_steps} qas ON qas.id = lateststepid.latestid
WHERE qa.questionusageid = $qubaid
HAVING COUNT(CASE WHEN qas.state = 'needsgrading' AND qa.maxmark > 0 THEN 1 ELSE NULL END) = 0";
- // TODO handle $qubaid with placeholders.
}
public function question_attempt_latest_state_view($alias) {
diff --git a/question/engine/lib.php b/question/engine/lib.php
index 4b6cf6401af..2b2d7a7628f 100644
--- a/question/engine/lib.php
+++ b/question/engine/lib.php
@@ -102,8 +102,11 @@ abstract class question_engine {
}
/**
- * Delete a {@link question_usage_by_activity} from the database, based on its id.
- * @param int $qubaid the id of the usage to delete.
+ * Delete {@link question_usage_by_activity}s from the database that match
+ * an arbitrary SQL where clause.
+ * @param string $where a where clause. Becuase of MySQL limitations, you
+ * must refer to {question_usages}.id in full like that.
+ * @param array $params values to substitute for placeholders in $where.
*/
public static function delete_questions_usage_by_activities($where, $params) {
$dm = new question_engine_data_mapper();
@@ -728,8 +731,7 @@ class question_usage_by_activity {
* @return int the number used to identify this question within this usage.
*/
public function add_question(question_definition $question, $maxmark = null) {
- $qa = new question_attempt($question, $this->get_id(),
- $this->context->id, $this->observer, $maxmark);
+ $qa = new question_attempt($question, $this->get_id(), $this->observer, $maxmark);
if (count($this->questionattempts) == 0) {
$this->questionattempts[1] = $qa;
} else {
@@ -1165,7 +1167,7 @@ class question_usage_by_activity {
$this->observer->notify_delete_attempt_steps($oldqa);
$newqa = new question_attempt($oldqa->get_question(), $oldqa->get_usage_id(),
- $this->context->id, $this->observer, $newmaxmark);
+ $this->observer, $newmaxmark);
$newqa->set_database_id($oldqa->get_database_id());
$newqa->regrade($oldqa, $finished);
@@ -1321,10 +1323,6 @@ class question_attempt {
/** @var integer|string the id of the question_usage_by_activity we belong to. */
protected $usageid;
- /** @var integer the id of the context this question_attempt belongs to. */
- // TODO I don't think this is actually needed.
- protected $owningcontextid = null;
-
/** @var integer the number used to identify this question_attempt within the usage. */
protected $slot = null;
@@ -1395,11 +1393,10 @@ class question_attempt {
* @param number $maxmark the maximum grade for this question_attempt. If not
* passed, $question->defaultmark is used.
*/
- public function __construct(question_definition $question, $usageid, $owningcontextid,
+ public function __construct(question_definition $question, $usageid,
question_usage_observer $observer = null, $maxmark = null) {
$this->question = $question;
$this->usageid = $usageid;
- $this->owningcontextid = $owningcontextid;
if (is_null($observer)) {
$observer = new question_usage_null_observer();
}
@@ -1830,10 +1827,13 @@ class question_attempt {
* @param string|null $number The question number to display.
* @return string HTML fragment representing the question.
*/
- public function render($options, $number) {
- global $PAGE;
- $qoutput = $PAGE->get_renderer('core', 'question');
- $qtoutput = $this->question->get_renderer();
+ public function render($options, $number, $page = null) {
+ if (is_null($page)) {
+ global $PAGE;
+ $page = $PAGE;
+ }
+ $qoutput = $page->get_renderer('core', 'question');
+ $qtoutput = $this->question->get_renderer($page);
return $this->behaviour->render($options, $number, $qoutput, $qtoutput);
}
@@ -1842,9 +1842,14 @@ class question_attempt {
* attempt is displayed in the body.
* @return string HTML fragment.
*/
- public function render_head_html() {
- return $this->question->get_renderer()->head_code($this) .
- $this->behaviour->get_renderer()->head_code($this);
+ public function render_head_html($page = null) {
+ if (is_null($page)) {
+ global $PAGE;
+ $page = $PAGE;
+ }
+ // TODO go via behaviour.
+ return $this->question->get_renderer($page)->head_code($this) .
+ $this->behaviour->get_renderer($page)->head_code($this);
}
/**
@@ -2221,7 +2226,7 @@ class question_attempt {
}
$qa = new question_attempt($question, $record->questionusageid,
- $record->contextid, null, $record->maxmark + 0);
+ null, $record->maxmark + 0);
$qa->set_database_id($record->questionattemptid);
$qa->set_number_in_usage($record->slot);
$qa->minfraction = $record->minfraction + 0;
diff --git a/question/engine/renderer.php b/question/engine/renderer.php
index c076d50fcc6..b869bab5d25 100644
--- a/question/engine/renderer.php
+++ b/question/engine/renderer.php
@@ -37,6 +37,9 @@ defined('MOODLE_INTERNAL') || die();
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class core_question_renderer extends plugin_renderer_base {
+ public function get_page() {
+ return $this->page;
+ }
/**
* Generate the display of a question in a particular state, and with certain
diff --git a/question/engine/simpletest/helpers.php b/question/engine/simpletest/helpers.php
index bc8999562f0..7e2b9c66880 100644
--- a/question/engine/simpletest/helpers.php
+++ b/question/engine/simpletest/helpers.php
@@ -70,7 +70,7 @@ class test_question_maker {
* @return question_attempt the question attempt.
*/
public function get_a_qa($question, $maxmark = 3) {
- return new question_attempt($question, 13, 0, null, $maxmark);
+ return new question_attempt($question, 13, null, $maxmark);
}
/**
diff --git a/question/engine/simpletest/testquestionattempt.php b/question/engine/simpletest/testquestionattempt.php
index 3e30eeb8394..ea11aca2861 100644
--- a/question/engine/simpletest/testquestionattempt.php
+++ b/question/engine/simpletest/testquestionattempt.php
@@ -51,7 +51,7 @@ class question_attempt_test extends UnitTestCase {
$this->question = test_question_maker::make_a_description_question();
$this->question->defaultmark = 3;
$this->usageid = 13;
- $this->qa = new question_attempt($this->question, $this->usageid, 0);
+ $this->qa = new question_attempt($this->question, $this->usageid);
}
public function tearDown() {
@@ -61,13 +61,13 @@ class question_attempt_test extends UnitTestCase {
}
public function test_constructor_sets_maxmark() {
- $qa = new question_attempt($this->question, $this->usageid, 0);
+ $qa = new question_attempt($this->question, $this->usageid);
$this->assertIdentical($this->question, $qa->get_question());
$this->assertEqual(3, $qa->get_max_mark());
}
public function test_maxmark_beats_default_mark() {
- $qa = new question_attempt($this->question, $this->usageid, 0, null, 2);
+ $qa = new question_attempt($this->question, $this->usageid, null, 2);
$this->assertEqual(2, $qa->get_max_mark());
}
@@ -143,7 +143,7 @@ class question_attempt_with_steps_test extends UnitTestCase {
public function setUp() {
$this->question = test_question_maker::make_a_description_question();
- $this->qa = new testable_question_attempt($this->question, 0, 0, null, 2);
+ $this->qa = new testable_question_attempt($this->question, 0, null, 2);
for ($i = 0; $i < 3; $i++) {
$step = new question_attempt_step(array('i' => $i));
$this->qa->add_step($step);
@@ -247,7 +247,7 @@ class question_attempt_with_steps_test extends UnitTestCase {
}
public function test_cannot_get_min_fraction_before_start() {
- $qa = new question_attempt($this->question, null, 0);
+ $qa = new question_attempt($this->question, 0);
$this->expectException();
$qa->get_min_fraction();
}
diff --git a/question/engine/simpletest/testquestionattemptstepiterator.php b/question/engine/simpletest/testquestionattemptstepiterator.php
index 6f0cd287d20..a7ec5b2cd21 100644
--- a/question/engine/simpletest/testquestionattemptstepiterator.php
+++ b/question/engine/simpletest/testquestionattemptstepiterator.php
@@ -43,7 +43,7 @@ class question_attempt_step_iterator_test extends UnitTestCase {
public function setUp() {
$question = test_question_maker::make_a_description_question();
- $this->qa = new testable_question_attempt($question, 0, 0);
+ $this->qa = new testable_question_attempt($question, 0);
for ($i = 0; $i < 3; $i++) {
$step = new question_attempt_step(array('i' => $i));
$this->qa->add_step($step);
diff --git a/question/format.php b/question/format.php
index 514d70581c0..655ae4c73f5 100644
--- a/question/format.php
+++ b/question/format.php
@@ -838,22 +838,6 @@ class qformat_default {
return NULL;
}
- /**
- * get directory into which export is going
- * @return string file path
- */
- function question_get_export_dir() {
- // TODO is this still used.
- global $USER;
- if ($this->canaccessbackupdata) {
- $dirname = get_string('exportfilename', 'question');
- $path = $this->course->id . '/backupdata/' . $dirname; // backupdata is protected directory
- } else {
- $path = 'temp/questionexport/' . $USER->id;
- }
- return $path;
- }
-
/**
* Convert the question text to plain text, so it can safely be displayed
* during import to let the user see roughly what is going on.
diff --git a/question/format/qti_two/format.php b/question/format/qti_two/format.php
index 1e6238a5c39..cf578876a75 100644
--- a/question/format/qti_two/format.php
+++ b/question/format/qti_two/format.php
@@ -261,14 +261,15 @@ function handle_questions_media(&$questions, $path, $courseid) {
*/
function exportprocess() {
- global $CFG, $OUTPUT;
+ global $CFG, $OUTPUT, $USER;
$courseid = $this->course->id;
+ $path = 'temp/qformat_qti_two/' . $USER->id . '/' . $this->filename;
// create a directory for the exports (if not already existing)
- if (!$export_dir = make_upload_directory($this->question_get_export_dir().'/'.$this->filename)) {
- print_error('cannotcreatepath', 'question', '', $export_dir);
+ if (!make_upload_directory($path)) {
+ throw new moodle_exception('cannotcreatepath', 'question', '', $path);
}
- $path = $CFG->dataroot.'/'.$this->question_get_export_dir().'/'.$this->filename;
+ $path = $CFG->dataroot . '/' . $path;
// get the questions (from database) in this category
$questions = get_questions_category( $this->category );
diff --git a/question/import.php b/question/import.php
index 357932d2453..673257ad98d 100644
--- a/question/import.php
+++ b/question/import.php
@@ -115,19 +115,16 @@ if ($form = $import_form->get_data()) {
// Do anything before that we need to
if (!$qformat->importpreprocess()) {
- //TODO: need more detailed error info
print_error('cannotimport', '', $thispageurl->out());
}
// Process the uploaded file
if (!$qformat->importprocess($category)) {
- //TODO: need more detailed error info
print_error('cannotimport', '', $thispageurl->out());
}
// In case anything needs to be done after
if (!$qformat->importpostprocess()) {
- //TODO: need more detailed error info
print_error('cannotimport', '', $thispageurl->out());
}
diff --git a/question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php b/question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php
index 24420015f95..cad5eee7777 100644
--- a/question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php
+++ b/question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php
@@ -98,15 +98,7 @@ class backup_qtype_calculated_plugin extends backup_qtype_plugin {
* files to be processed both in backup and restore.
*/
public static function get_qtype_fileareas() {
- // TODO: Discuss. Commented below are the "in theory" correct
- // mappings for those fileareas. Instead we are using question for
- // them, that will cause problems in the future if we want to change
- // any of them to be 1..n (i.e. we should be always pointing to own id)
return array(
- //'instruction' => 'question_numerical_option',
- //'correctfeedback' => 'question_calculated_option',
- //'partiallycorrectfeedback' => 'question_calculated_option',
- //'incorrectfeedback' => 'question_calculated_option');
'instruction' => 'question_created',
'correctfeedback' => 'question_created',
'partiallycorrectfeedback' => 'question_created',
diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php
index e6022549420..bc538f8e3d2 100644
--- a/question/type/calculated/questiontype.php
+++ b/question/type/calculated/questiontype.php
@@ -2145,15 +2145,14 @@ class question_calculated_qtype extends question_type {
}
return true;
} else if ($filearea == 'instruction') {
- // TODO: should it be display all the time like questiontext?
- // check if question id exists
+ // Displayed all the time like the question text. Check if question id exists
if ($itemid != $question->id) {
return false;
} else {
return true;
}
} else if (in_array($filearea, array('correctfeedback', 'partiallycorrectfeedback', 'incorrectfeedback'))) {
- // TODO: calculated type doesn't display question feedback yet
+ // Note: calculated type doesn't display question feedback yet
return false;
} else {
return parent::check_file_access($question, $state, $options, $contextid, $component,
diff --git a/question/type/calculatedmulti/questiontype.php b/question/type/calculatedmulti/questiontype.php
index 0abd637f0de..6d998d09ba9 100644
--- a/question/type/calculatedmulti/questiontype.php
+++ b/question/type/calculatedmulti/questiontype.php
@@ -107,8 +107,6 @@ class question_calculatedmulti_qtype extends question_calculated_qtype {
// Save the units.
$virtualqtype = $this->get_virtual_qtype($question);
- // TODO: What is this?
- // $result = $virtualqtype->save_numerical_units($question);
if (isset($result->error)) {
return $result;
} else {
diff --git a/question/type/calculatedsimple/questiontype.php b/question/type/calculatedsimple/questiontype.php
index 380ed933700..687c77fd39a 100644
--- a/question/type/calculatedsimple/questiontype.php
+++ b/question/type/calculatedsimple/questiontype.php
@@ -378,7 +378,7 @@ class question_calculatedsimple_qtype extends question_calculated_qtype {
$itemid = reset($args);
if ($component == 'question' && $filearea == 'answerfeedback') {
- // check if answer id exists
+ // Check if answer id exists
$result = $options->feedback && array_key_exists($itemid, $question->options->answers);
if (!$result) {
return false;
@@ -389,8 +389,7 @@ class question_calculatedsimple_qtype extends question_calculated_qtype {
}
return true;
} else if ($filearea == 'instruction') {
- // TODO: should it be display all the time like questiontext?
- // check if question id exists
+ // Displayed all the time like the question text. Check if question id exists
if ($itemid != $question->id) {
return false;
} else {
diff --git a/question/type/missingtype/simpletest/testmissingtype.php b/question/type/missingtype/simpletest/testmissingtype.php
index 56261e8f366..a257c19f237 100644
--- a/question/type/missingtype/simpletest/testmissingtype.php
+++ b/question/type/missingtype/simpletest/testmissingtype.php
@@ -95,7 +95,7 @@ class qtype_missing_test extends UnitTestCase {
public function test_render_missing() {
$questiondata = $this->get_unknown_questiondata();
$q = question_bank::make_question($questiondata);
- $qa = new testable_question_attempt($q, 0, 0);
+ $qa = new testable_question_attempt($q, 0);
$step = new question_attempt_step(array('answer' => 'frog'));
$step->set_state(question_state::$todo);
diff --git a/question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php b/question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php
index 9d5a760ae15..0f6ba149c30 100644
--- a/question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php
+++ b/question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php
@@ -76,14 +76,7 @@ class backup_qtype_multichoice_plugin extends backup_qtype_plugin {
* files to be processed both in backup and restore.
*/
public static function get_qtype_fileareas() {
- // TODO: Discuss. Commented below are the "in theory" correct
- // mappings for those fileareas. Instead we are using question for
- // them, that will cause problems in the future if we want to change
- // any of them to be 1..n (i.e. we should be always pointing to own id)
return array(
- //'correctfeedback' => 'question_multichoice',
- //'partiallycorrectfeedback' => 'question_multichoice',
- //'incorrectfeedback' => 'question_multichoice');
'correctfeedback' => 'question_created',
'partiallycorrectfeedback' => 'question_created',
'incorrectfeedback' => 'question_created');
diff --git a/question/type/multichoice/question.php b/question/type/multichoice/question.php
index 3f929f10cd2..6d495f37dca 100644
--- a/question/type/multichoice/question.php
+++ b/question/type/multichoice/question.php
@@ -129,9 +129,8 @@ abstract class qtype_multichoice_base extends question_graded_automatically {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_multichoice_single_question extends qtype_multichoice_base {
- public function get_renderer() {
- global $PAGE; // TODO get rid of this global.
- return $PAGE->get_renderer('qtype_multichoice', 'single');
+ public function get_renderer(moodle_page $page) {
+ return $page->get_renderer('qtype_multichoice', 'single');
}
public function get_min_fraction() {
@@ -228,9 +227,8 @@ class qtype_multichoice_single_question extends qtype_multichoice_base {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_multichoice_multi_question extends qtype_multichoice_base {
- public function get_renderer() {
- global $PAGE; // TODO get rid of this global.
- return $PAGE->get_renderer('qtype_multichoice', 'multi');
+ public function get_renderer(moodle_page $page) {
+ return $page->get_renderer('qtype_multichoice', 'multi');
}
public function get_min_fraction() {
diff --git a/question/type/multichoice/styles.css b/question/type/multichoice/styles.css
index 311d9cb8c76..a9a2936eac1 100644
--- a/question/type/multichoice/styles.css
+++ b/question/type/multichoice/styles.css
@@ -1,4 +1,3 @@
-/* TODO */
.que.multichoice .answer .specificfeedback {
display: inline;
padding: 0 0.7em;
diff --git a/question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php b/question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php
index fdf077c8ccc..468d9e15bb8 100644
--- a/question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php
+++ b/question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php
@@ -82,12 +82,6 @@ class backup_qtype_numerical_plugin extends backup_qtype_plugin {
* files to be processed both in backup and restore.
*/
public static function get_qtype_fileareas() {
- // TODO: Discuss. Commented below are the "in theory" correct
- // mappings for those fileareas. Instead we are using question for
- // them, that will cause problems in the future if we want to change
- // any of them to be 1..n (i.e. we should be always pointing to own id)
- return array(
- //'instruction' => 'question_numerical_option');
- 'instruction' => 'question_created');
+ return array('instruction' => 'question_created');
}
}
diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php
index 587a9c9ea09..1393861a21c 100644
--- a/question/type/numerical/questiontype.php
+++ b/question/type/numerical/questiontype.php
@@ -1067,7 +1067,7 @@ class question_numerical_qtype extends qtype_shortanswer {
$mform->setType('unitpenalty', PARAM_NUMBER);
$mform->setDefault('unitpenalty', 0.1);
$mform->setDefault('unitgradingtypes', 1);
- $mform->addHelpButton('penaltygrp', 'unitpenalty', 'qtype_numerical'); // TODO help did not exist before MDL-21695
+ $mform->addHelpButton('penaltygrp', 'unitpenalty', 'qtype_numerical');
$mform->setDefault('unitsleft', 0);
$mform->setType('instructions', PARAM_RAW);
$mform->addHelpButton('instructions', 'numericalinstructions', 'qtype_numerical');
diff --git a/question/type/questionbase.php b/question/type/questionbase.php
index 49cef95d50b..4636840223d 100644
--- a/question/type/questionbase.php
+++ b/question/type/questionbase.php
@@ -204,9 +204,8 @@ abstract class question_definition {
/**
* @return qtype_renderer the renderer to use for outputting this question.
*/
- public function get_renderer() {
- global $PAGE; // TODO get rid of this global.
- return $PAGE->get_renderer('qtype_' . $this->qtype->name());
+ public function get_renderer(moodle_page $page) {
+ return $page->get_renderer($this->qtype->plugin_name());
}
/**
diff --git a/question/type/random/questiontype.php b/question/type/random/questiontype.php
index fc80e3cf0fc..52b4f8fd21f 100644
--- a/question/type/random/questiontype.php
+++ b/question/type/random/questiontype.php
@@ -80,14 +80,16 @@ class qtype_random extends question_type {
if ($question->questiontext) {
$categorylist = question_categorylist($question->category);
} else {
- $categorylist = $question->category;
+ $categorylist = array($question->category);
}
+ list($qcsql, $qcparams) = $DB->get_in_or_equal($categorylist);
+ // TODO use in_or_equal for $otherquestionsinuse and $this->manualqtypes
return $DB->record_exists_select('question',
- "category IN ($categorylist)
+ "category $qcsql
AND parent = 0
AND hidden = 0
AND id NOT IN ($otherquestionsinuse)
- AND qtype IN ($this->manualqtypes)");
+ AND qtype IN ($this->manualqtypes)", $qcparams);
}
/**
@@ -186,7 +188,7 @@ class qtype_random extends question_type {
if ($subcategories) {
$categoryids = question_categorylist($categoryid);
} else {
- $categoryids = $categoryid;
+ $categoryids = array($categoryid);
}
$questionids = question_bank::get_finder()->get_questions_from_categories(
diff --git a/question/type/randomsamatch/questiontype.php b/question/type/randomsamatch/questiontype.php
index 39e884afe5d..7a712f69001 100644
--- a/question/type/randomsamatch/questiontype.php
+++ b/question/type/randomsamatch/questiontype.php
@@ -109,36 +109,18 @@ class question_randomsamatch_qtype extends qtype_match {
// recurse into subcategories
$categorylist = question_categorylist($question->category);
} else {
- $categorylist = $question->category;
+ $categorylist = array($question->category);
}
$saquestions = $this->get_sa_candidates($categorylist, $cmoptions->questionsinuse);
$count = count($saquestions);
$wanted = $question->options->choose;
- $errorstr = '';
- if ($count < $wanted && has_coursecontact_role($USER->id)) { //TODO: this teacher test is far from optimal
- if ($count >= 2) {
- $errorstr = "Error: could not get enough Short-Answer questions!
- Got $count Short-Answer questions, but wanted $wanted.
- Reducing number to choose from to $count!";
- $wanted = $question->options->choose = $count;
- } else {
- $errorstr = "Error: could not get enough Short-Answer questions!
- This can happen if all available Short-Answer questions are already
- taken up by other Random questions or Random Short-Answer question.
- Another possible cause for this error is that Short-Answer
- questions were deleted after this Random Short-Answer question was
- created.";
- }
- echo $OUTPUT->notification($errorstr);
- $errorstr = '' . $errorstr . '';
- }
if ($count < $wanted) {
- $question->questiontext = "$errorstr
Insufficient selection options are
- available for this question, therefore it is not available in this
- quiz. Please inform your teacher.";
+ $question->questiontext = "Insufficient selection options are
+ available for this question, therefore it is not available in this
+ quiz. Please inform your teacher.";
// Treat this as a description from this point on
$question->qtype = DESCRIPTION;
return true;
@@ -263,9 +245,9 @@ class question_randomsamatch_qtype extends qtype_match {
return $response;
}
- function get_sa_candidates($categorylist, $questionsinuse=0) {
+ function get_sa_candidates($categorylist, $questionsinuse = 0) {
global $DB;
- list ($usql, $params) = $DB->get_in_or_equal(explode(',', $categorylist));
+ list ($usql, $params) = $DB->get_in_or_equal($categorylist);
list ($ques_usql, $ques_params) = $DB->get_in_or_equal(explode(',', $questionsinuse), SQL_PARAMS_QM, null, false);
$params = array_merge($params, $ques_params);
return $DB->get_records_select('question',
@@ -310,7 +292,7 @@ class question_randomsamatch_qtype extends qtype_match {
// recurse into subcategories
$categorylist = question_categorylist($question->category);
} else {
- $categorylist = $question->category;
+ $categorylist = array($question->category);
}
$question->options->subquestions = $this->get_sa_candidates($categorylist);
diff --git a/question/type/rendererbase.php b/question/type/rendererbase.php
index 7d336e68018..2354c8e8398 100644
--- a/question/type/rendererbase.php
+++ b/question/type/rendererbase.php
@@ -187,7 +187,10 @@ abstract class qtype_renderer extends plugin_renderer_base {
* @return string HTML fragment.
*/
public function head_code(question_attempt $qa) {
- // TODO I think we can get rid of this, but what about Opaque?
+ // This method is used by the Opaque question type. The remote question
+ // engine can send back arbitrary CSS that we have to link to in the
+ // page header. If it was not for that, we might be able to eliminate
+ // this method and load the required CSS and JS some other way.
$qa->get_question()->qtype->find_standard_scripts();
}