From 2daffca554e845b0555e4adecdf17bce925a013d Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Thu, 24 Feb 2011 17:47:51 +0000 Subject: [PATCH] MDL-20636 Fix 30 TODOs --- lang/en/question.php | 2 + lib/questionlib.php | 16 +++---- mod/quiz/comment.php | 1 - mod/quiz/lang/en/quiz.php | 2 - mod/quiz/lib.php | 7 ++- mod/quiz/locallib.php | 39 ----------------- mod/quiz/report/attemptsreport.php | 8 ++-- .../report/overview/overviewsettings_form.php | 5 +-- mod/quiz/report/overview/report.php | 3 +- mod/quiz/report/reportlib.php | 16 ++++--- mod/quiz/report/responses/report.php | 3 +- .../responses/responsessettings_form.php | 5 +-- mod/quiz/reviewquestion.php | 8 ++-- mod/quiz/view.php | 1 - question/behaviour/behaviourbase.php | 8 ++-- .../simpletest/testmissingbehaviour.php | 6 +-- question/behaviour/opaque/behaviour.php | 2 +- question/editlib.php | 20 ++++----- question/engine/bank.php | 26 +++++------ question/engine/datalib.php | 4 +- question/engine/lib.php | 43 +++++++++++-------- question/engine/renderer.php | 3 ++ question/engine/simpletest/helpers.php | 2 +- .../engine/simpletest/testquestionattempt.php | 10 ++--- .../testquestionattemptstepiterator.php | 2 +- question/format.php | 16 ------- question/format/qti_two/format.php | 9 ++-- question/import.php | 3 -- .../backup_qtype_calculated_plugin.class.php | 8 ---- question/type/calculated/questiontype.php | 5 +-- .../type/calculatedmulti/questiontype.php | 2 - .../type/calculatedsimple/questiontype.php | 5 +-- .../simpletest/testmissingtype.php | 2 +- .../backup_qtype_multichoice_plugin.class.php | 7 --- question/type/multichoice/question.php | 10 ++--- question/type/multichoice/styles.css | 1 - .../backup_qtype_numerical_plugin.class.php | 8 +--- question/type/numerical/questiontype.php | 2 +- question/type/questionbase.php | 5 +-- question/type/random/questiontype.php | 10 +++-- question/type/randomsamatch/questiontype.php | 32 +++----------- question/type/rendererbase.php | 5 ++- 42 files changed, 139 insertions(+), 233 deletions(-) diff --git a/lang/en/question.php b/lang/en/question.php index fbfb40522d3..bacdbaebe4e 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -178,6 +178,7 @@ $string['movedquestionsandcategories'] = 'Moved questions and question categorie $string['movelinksonly'] = 'Just change where links point to, do not move or copy files.'; $string['moveq'] = 'Move question(s)'; $string['moveqtoanothercontext'] = 'Move question to another context.'; +$string['moveto'] = 'Move to >>'; $string['movingcategory'] = 'Moving category'; $string['movingcategoryandfiles'] = 'Are you sure you want to move category {$a->name} and all child categories to context for "{$a->contextto}"?
We have detected {$a->urlcount} files linked from questions in {$a->fromareaname}, would you like to copy or move these to {$a->toareaname}?'; $string['movingcategorynofiles'] = 'Are you sure you want to move category "{$a->name}" and all child categories to context for "{$a->contextto}"?'; @@ -347,5 +348,6 @@ $string['submitted'] = 'Submit: {$a}'; $string['unknownquestion'] = 'Unknown question: {$a}.'; $string['unknownquestioncatregory'] = 'Unknown question category: {$a}.'; $string['whethercorrect'] = 'Whether correct'; +$string['withselected'] = 'With selected'; $string['xoutofmax'] = '{$a->mark} out of {$a->max}'; $string['yougotnright'] = 'You have correctly selected {$a->num}.'; diff --git a/lib/questionlib.php b/lib/questionlib.php index 410d1e41a0e..2292117af18 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -777,7 +777,7 @@ function question_load_questions($questionids, $extrafields = '', $join = '') { */ function _tidy_question($question, $loadtags = false) { global $CFG; - if (question_bank::is_qtype_installed($question->qtype)) { + if (!question_bank::is_qtype_installed($question->qtype)) { $question->questiontext = html_writer::tag('p', get_string('warningmissingtype', 'qtype_missingtype')) . $question->questiontext; } @@ -1153,18 +1153,18 @@ function question_add_tops($categories, $pcontexts){ } /** - * Returns a comma separated list of ids of the category and all subcategories + * @return array of question category ids of the category and all subcategories. */ function question_categorylist($categoryid) { global $DB; - // returns a comma separated list of ids of the category and all subcategories - $categorylist = $categoryid; - if ($subcategories = $DB->get_records('question_categories', array('parent' => $categoryid), 'sortorder ASC', 'id, 1')) { - foreach ($subcategories as $subcategory) { - $categorylist .= ','. question_categorylist($subcategory->id); - } + $subcategories = $DB->get_records('question_categories', array('parent' => $categoryid), 'sortorder ASC', 'id, 1'); + + $categorylist = array($categoryid); + foreach ($subcategories as $subcategory) { + $categorylist = array_merge($categorylist, question_categorylist($subcategory->id)); } + return $categorylist; } diff --git a/mod/quiz/comment.php b/mod/quiz/comment.php index e752fae14cd..1f5271c49c8 100644 --- a/mod/quiz/comment.php +++ b/mod/quiz/comment.php @@ -57,7 +57,6 @@ echo $OUTPUT->heading(format_string($attemptobj->get_question_name($slot))); // Process any data that was submitted. if (data_submitted() && confirm_sesskey()) { if (optional_param('submit', false, PARAM_BOOL)) { - // TODO better error handling. $transaction = $DB->start_delegated_transaction(); $attemptobj->process_all_actions(time()); $transaction->allow_commit(); diff --git a/mod/quiz/lang/en/quiz.php b/mod/quiz/lang/en/quiz.php index 604f84128f6..4731b4f7f65 100644 --- a/mod/quiz/lang/en/quiz.php +++ b/mod/quiz/lang/en/quiz.php @@ -420,7 +420,6 @@ $string['modulename'] = 'Quiz'; $string['modulename_help'] = 'The quiz module enables the teacher to design and set quizzes consisting of multiple choice, true-false, matching and other question types. Each attempt is automatically marked, and the teacher can choose whether to give feedback and/or show correct answers.'; $string['modulenameplural'] = 'Quizzes'; $string['moveselectedonpage'] = 'Move selected questions to page: {$a}'; -$string['moveto'] = 'Move to >>'; $string['multianswer'] = 'Embedded answers (cloze)'; $string['multichoice'] = 'Multiple choice'; $string['multipleanswers'] = 'Choose at least one answer.'; @@ -789,7 +788,6 @@ $string['warningmissingtype'] = 'This question is of a type that has not been $string['wheregrade'] = 'Where\'s my grade?'; $string['wildcard'] = 'Wild card'; $string['windowclosing'] = 'This window will close shortly.'; -$string['withselected'] = 'With selected'; $string['withsummary'] = 'with summary statistics'; $string['wronguse'] = 'You can not use this page like that'; $string['xhtml'] = 'XHTML'; diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index a3dc8536a0d..0d0871b4e36 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -305,9 +305,8 @@ function quiz_update_effective_access($quiz, $userid) { function quiz_delete_all_attempts($quiz) { global $CFG, $DB; require_once($CFG->libdir . '/questionlib.php'); - // TODO remove $CFG->prefix - question_engine::delete_questions_usage_by_activities("{$CFG->prefix}question_usages.id IN ( - SELECT uniqueid FROM {$CFG->prefix}quiz_attempts WHERE quiz = $quizid)"); + question_engine::delete_questions_usage_by_activities("{question_usages}.id IN ( + SELECT uniqueid FROM {quiz_attempts} WHERE quiz = :quizid)", array('quizid' => $quizid)); $DB->delete_records('quiz_attempts', array('quiz' => $quiz->id)); $DB->delete_records('quiz_grades', array('quiz' => $quiz->id)); } @@ -1232,7 +1231,7 @@ function quiz_reset_userdata($data) { SELECT uniqueid FROM {quiz_attempts} quiza JOIN {quiz} quiz ON quiza.quiz = quiz.id - WHERE quiz.course = ?)', array($data->courseid)); + WHERE quiz.course = :courseid)', array('courseid' => $data->courseid)); $DB->delete_records_select('quiz_attempts', 'quiz IN (SELECT id FROM {quiz} WHERE course = ?)', array($data->courseid)); diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 76b2630c58c..bef0e2b2bf9 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -217,21 +217,6 @@ function quiz_has_attempts($quizid) { /// Functions to do with quiz layout and pages //////////////////////////////// -/** - * Returns a comma separated list of question ids for the current page - * - * @param string $layout the string representing the quiz layout. Each page is represented as a - * comma separated list of question ids and 0 indicating page breaks. - * So 5,2,0,3,0 means questions 5 and 2 on page 1 and question 3 on page 2 - * @param int $page the number of the current page. - * @return string comma separated list of question ids - */ -function quiz_questions_on_page($layout, $page) { - $pages = explode(',0', $layout); - // TODO is this still used. - return trim($pages[$page], ','); -} - /** * Returns a comma separated list of question ids for the quiz * @@ -267,7 +252,6 @@ function quiz_number_of_pages($layout) { * @return int The number of questions in the quiz. */ function quiz_number_of_questions_in_quiz($layout) { - // TODO is this still used. $layout = quiz_questions_in_quiz(quiz_clean_layout($layout)); $count = substr_count($layout, ','); if ($layout !== '') { @@ -276,29 +260,6 @@ function quiz_number_of_questions_in_quiz($layout) { return $count; } -/** - * Returns the first question number for the current quiz page - * - * @param string $quizlayout The string representing the layout for the whole quiz - * @param string $pagelayout The string representing the layout for the current page - * @return int the number of the first question - */ -function quiz_first_questionnumber($quizlayout, $pagelayout) { - // TODO is this still used. - // this works by finding all the questions from the quizlayout that - // come before the current page and then adding up their lengths. - global $CFG, $DB; - $start = strpos($quizlayout, ','.$pagelayout.',')-2; - if ($start > 0) { - $prevlist = substr($quizlayout, 0, $start); - list($usql, $params) = $DB->get_in_or_equal(explode(',', $prevlist)); - return $DB->get_field_sql("SELECT sum(length)+1 FROM {question} - WHERE id $usql", $params); - } else { - return 1; - } -} - /** * Re-paginates the quiz layout * diff --git a/mod/quiz/report/attemptsreport.php b/mod/quiz/report/attemptsreport.php index b0e2226ee09..55e0b5c7e82 100644 --- a/mod/quiz/report/attemptsreport.php +++ b/mod/quiz/report/attemptsreport.php @@ -342,8 +342,10 @@ abstract class quiz_attempt_report extends quiz_default_report { * Empty means all users. */ protected function delete_selected_attempts($quiz, $cm, $attemptids, $allowed) { + global $DB; + foreach ($attemptids as $attemptid) { - $attempt = get_record('quiz_attempts', 'id', $attemptid); + $attempt = $DB->get_record('quiz_attempts', array('id' => $attemptid)); if (!$attempt || $attempt->quiz != $quiz->id || $attempt->preview != 0) { // Ensure the attempt exists, and belongs to this quiz. If not skip. continue; @@ -610,8 +612,8 @@ abstract class quiz_attempt_report_table extends table_sql { $this->sql->fields .= ",\n$fields"; $this->sql->from .= "\nLEFT JOIN $inlineview ON " . - "$alias.questionusageid = quiza.uniqueid AND $alias.slot = $slot"; - // TODO params + "$alias.questionusageid = quiza.uniqueid AND $alias.slot = :{$alias}slot"; + $this->sql->params[$alias . 'slot'] = $slot; } /** diff --git a/mod/quiz/report/overview/overviewsettings_form.php b/mod/quiz/report/overview/overviewsettings_form.php index 1f1a2bb92dc..9dff2e56e7f 100644 --- a/mod/quiz/report/overview/overviewsettings_form.php +++ b/mod/quiz/report/overview/overviewsettings_form.php @@ -38,8 +38,7 @@ require_once($CFG->libdir . '/formslib.php'); */ class mod_quiz_report_overview_settings extends moodleform { - function definition() { - global $COURSE; // TODO get rid of this. + public function definition() { $mform = $this->_form; $mform->addElement('header', 'preferencespage', get_string('preferencespage', 'quiz_overview')); @@ -60,7 +59,7 @@ class mod_quiz_report_overview_settings extends moodleform { if (!$this->_customdata['currentgroup']) { $options[QUIZ_REPORT_ATTEMPTS_ALL] = get_string('optallattempts','quiz_overview'); } - if ($this->_customdata['currentgroup'] || $COURSE->id != SITEID) { + if ($this->_customdata['currentgroup'] || !is_inside_frontpage($this->_customdata['context'])) { $options[QUIZ_REPORT_ATTEMPTS_ALL_STUDENTS] = get_string('optallstudents','quiz_overview', $studentsstring); $options[QUIZ_REPORT_ATTEMPTS_STUDENTS_WITH] = get_string('optattemptsonly','quiz_overview', $studentsstring); diff --git a/mod/quiz/report/overview/report.php b/mod/quiz/report/overview/report.php index 0323ebc0a0e..59ea18a3258 100644 --- a/mod/quiz/report/overview/report.php +++ b/mod/quiz/report/overview/report.php @@ -58,7 +58,8 @@ class quiz_overview_report extends quiz_attempt_report { $qmsubselect = quiz_report_qm_filter_select($quiz); $mform = new mod_quiz_report_overview_settings($reporturl, - array('qmsubselect' => $qmsubselect, 'quiz' => $quiz, 'currentgroup' => $currentgroup, 'context'=>$this->context)); + array('qmsubselect' => $qmsubselect, 'quiz' => $quiz, + 'currentgroup' => $currentgroup, 'context' => $this->context)); if ($fromform = $mform->get_data()) { $regradeall = false; diff --git a/mod/quiz/report/reportlib.php b/mod/quiz/report/reportlib.php index 93da23fbda2..2006671667a 100644 --- a/mod/quiz/report/reportlib.php +++ b/mod/quiz/report/reportlib.php @@ -188,15 +188,15 @@ function quiz_report_grade_bands($bandwidth, $bands, $quizid, $userids = array() global $DB; if ($userids) { - list($usql, $params) = $DB->get_in_or_equal($userids); + list($usql, $params) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED, 'u000000'); $usql = "qg.userid $usql AND"; } else { - $usql =''; + $usql = ''; $params = array(); } $sql = " SELECT - FLOOR(qg.grade/$bandwidth) AS band, + FLOOR(qg.grade / :bandwidth1) AS band, COUNT(1) AS num FROM {quiz_grades} qg @@ -204,15 +204,17 @@ JOIN {quiz} q ON qg.quiz = q.id WHERE $usql - qg.quiz = ? + qg.quiz = :quizid GROUP BY - FLOOR(qg.grade/$bandwidth) + FLOOR(qg.grade / :bandwidth2) ORDER BY band"; -// TODO bandiwth with a placeholder. - $params[] = $quizid; + + $params['quizid'] = $quizid; + $params['bandwidth1'] = $bandwidth; + $params['bandwidth2'] = $bandwidth; $data = $DB->get_records_sql_menu($sql, $params); diff --git a/mod/quiz/report/responses/report.php b/mod/quiz/report/responses/report.php index 0783dda79a6..4ee74565371 100644 --- a/mod/quiz/report/responses/report.php +++ b/mod/quiz/report/responses/report.php @@ -66,7 +66,8 @@ class quiz_responses_report extends quiz_attempt_report { $qmsubselect = quiz_report_qm_filter_select($quiz); $mform = new mod_quiz_report_responses_settings($reporturl, - array('qmsubselect' => $qmsubselect, 'quiz' => $quiz, 'currentgroup' => $currentgroup)); + array('qmsubselect' => $qmsubselect, 'quiz' => $quiz, + 'currentgroup' => $currentgroup, 'context' => $this->context)); if ($fromform = $mform->get_data()) { $attemptsmode = $fromform->attemptsmode; diff --git a/mod/quiz/report/responses/responsessettings_form.php b/mod/quiz/report/responses/responsessettings_form.php index 59181197e09..b36c3a8e512 100644 --- a/mod/quiz/report/responses/responsessettings_form.php +++ b/mod/quiz/report/responses/responsessettings_form.php @@ -38,8 +38,7 @@ require_once($CFG->libdir . '/formslib.php'); */ class mod_quiz_report_responses_settings extends moodleform { - function definition() { - global $COURSE; // TODO get rid of this. + public function definition() { $mform = $this->_form; $mform->addElement('header', 'preferencespage', get_string('preferencespage', 'quiz_overview')); @@ -60,7 +59,7 @@ class mod_quiz_report_responses_settings extends moodleform { if (!$this->_customdata['currentgroup']) { $options[QUIZ_REPORT_ATTEMPTS_ALL] = get_string('optallattempts','quiz_overview'); } - if ($this->_customdata['currentgroup'] || $COURSE->id != SITEID) { + if ($this->_customdata['currentgroup'] || !is_inside_frontpage($this->_customdata['context'])) { $options[QUIZ_REPORT_ATTEMPTS_ALL_STUDENTS] = get_string('optallstudents','quiz_overview', $studentsstring); $options[QUIZ_REPORT_ATTEMPTS_STUDENTS_WITH] = get_string('optattemptsonly','quiz_overview', $studentsstring); diff --git a/mod/quiz/reviewquestion.php b/mod/quiz/reviewquestion.php index 8b5eb308219..4b6483bd608 100644 --- a/mod/quiz/reviewquestion.php +++ b/mod/quiz/reviewquestion.php @@ -48,21 +48,21 @@ $attemptobj = quiz_attempt::create($attemptid); require_login($attemptobj->get_courseid(), false, $attemptobj->get_cm()); $attemptobj->check_review_capability(); +echo $OUTPUT->header(); + // Check permissions. if ($attemptobj->is_own_attempt()) { if (!$attemptobj->is_finished()) { - echo $OUTPUT->header(); echo $OUTPUT->notification(get_string('cannotreviewopen', 'quiz')); echo $OUTPUT->close_window_button(); echo $OUTPUT->footer(); - die; + die(); } else if (!$options->responses) { $accessmanager = $attemptobj->get_access_manager(time()); - echo $OUTPUT->header(); echo $OUTPUT->notification($accessmanager->cannot_review_message($attemptobj->get_review_options())); echo $OUTPUT->close_window_button(); echo $OUTPUT->footer(); - die; + die(); } } else if (!$attemptobj->is_review_allowed()) { diff --git a/mod/quiz/view.php b/mod/quiz/view.php index 2dc79afb191..5fa34820ed0 100644 --- a/mod/quiz/view.php +++ b/mod/quiz/view.php @@ -118,7 +118,6 @@ if (isguestuser()) { } // If they are not enrolled in this course in a good enough role, tell them to enrol. -// TODO, review this. if (!($canattempt || $canpreview || $canreviewmine)) { echo $OUTPUT->box('

' . 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(); }