From 56e82d993d6de7ab45649b23e1e142f89f5657c8 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Wed, 9 Feb 2011 20:33:51 +0000 Subject: [PATCH] MDL-20636 Images in questions now work in quiz attempts. --- lib/questionlib.php | 27 ++-------- mod/quiz/attempt.php | 2 +- mod/quiz/attemptlib.php | 52 +++++++++++++------ mod/quiz/lib.php | 11 ++-- mod/quiz/locallib.php | 23 -------- mod/quiz/processattempt.php | 2 +- question/engine/bank.php | 6 ++- question/engine/lib.php | 3 +- .../ddwtos/simpletest/testquestiontype.php | 1 + .../gapselect/simpletest/testquestiontype.php | 1 + .../match/simpletest/testquestiontype.php | 4 ++ .../simpletest/testmissingtype.php | 1 + question/type/questionbase.php | 3 ++ question/type/questiontype.php | 1 + 14 files changed, 65 insertions(+), 72 deletions(-) diff --git a/lib/questionlib.php b/lib/questionlib.php index f8f6b29e0eeb1..cf520359e3af5 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -797,8 +797,10 @@ function question_preload_questions($questionids, $extrafields = '', $join = '', } list($questionidcondition, $params) = $DB->get_in_or_equal( $questionids, SQL_PARAMS_NAMED, 'qid0000'); - $sql = 'SELECT q.*' . $extrafields . ' FROM {question} q' . $join . - ' WHERE q.id ' . $questionidcondition; + $sql = 'SELECT q.*, qc.contextid' . $extrafields . ' FROM {question} q + JOIN {question_categories} qc ON q.category = qc.id' . + $join . + ' WHERE q.id ' . $questionidcondition; // Load the questions if (!$questions = $DB->get_records_sql($sql, $extraparams + $params)) { @@ -1863,27 +1865,6 @@ function question_pluginfile($course, $context, $component, $filearea, $args, $f } } -/** - * TODO delete this. Replaced by $quba->check_file_access. - * - * Final test for whether a studnet should be allowed to see a particular file. - * This delegates the decision to the question type plugin. - * - * @param object $question The question to be rendered. - * @param object $state The state to render the question in. - * @param object $options An object specifying the rendering options. - * @param string $component the name of the component we are serving files for. - * @param string $filearea the name of the file area. - * @param array $args the remaining bits of the file path. - * @param bool $forcedownload whether the user must be forced to download the file. - */ -function question_check_file_access($question, $state, $options, $contextid, $component, - $filearea, $args, $forcedownload) { - global $QTYPES; - return $QTYPES[$question->qtype]->check_file_access($question, $state, $options, $contextid, $component, - $filearea, $args, $forcedownload); -} - /** * Create url for question export * diff --git a/mod/quiz/attempt.php b/mod/quiz/attempt.php index bb1118849434d..e131e516f7008 100644 --- a/mod/quiz/attempt.php +++ b/mod/quiz/attempt.php @@ -157,7 +157,7 @@ // Some hidden fields to trach what is going on. echo ''; -echo ''; +echo ''; echo ''; echo ''; echo ''; diff --git a/mod/quiz/attemptlib.php b/mod/quiz/attemptlib.php index 9c29e3130065f..1578623be8a67 100644 --- a/mod/quiz/attemptlib.php +++ b/mod/quiz/attemptlib.php @@ -375,15 +375,19 @@ function __construct($attempt, $quiz, $cm, $course) { } /** - * Static function to create a new quiz_attempt object given an attemptid. - * - * @param integer $attemptid the attempt id. - * @return quiz_attempt the new quiz_attempt object + * Used by {create()} and {create_from_usage_id()}. + * @param array $conditions passed to $DB->get_record('quiz_attempts', $conditions). */ - static public function create($attemptid) { + static protected function create_helper($conditions) { global $DB; - if (!$attempt = quiz_load_attempt($attemptid)) { +// TODO deal with the issue that makes this necessary. +// if (!$DB->record_exists('question_sessions', array('attemptid' => $attempt->uniqueid))) { +// // this attempt has not yet been upgraded to the new model +// quiz_upgrade_states($attempt); +// } + + if (!$attempt = $DB->get_record('quiz_attempts', $conditions)) { throw new moodle_exception('invalidattemptid', 'quiz'); } if (!$quiz = $DB->get_record('quiz', array('id' => $attempt->quiz))) { @@ -402,6 +406,26 @@ static public function create($attemptid) { return new quiz_attempt($attempt, $quiz, $cm, $course); } + /** + * Static function to create a new quiz_attempt object given an attemptid. + * + * @param integer $attemptid the attempt id. + * @return quiz_attempt the new quiz_attempt object + */ + static public function create($attemptid) { + return self::create_helper(array('id' => $attemptid)); + } + + /** + * Static function to create a new quiz_attempt object given a usage id. + * + * @param integer $usageid the attempt usage id. + * @return quiz_attempt the new quiz_attempt object + */ + static public function create_from_usage_id($usageid) { + return self::create_helper(array('uniqueid' => $usageid)); + } + private function determine_layout() { $this->pagelayout = array(); @@ -637,7 +661,6 @@ public function is_last_page($page) { * @return array the reqested list of question ids. */ public function get_slots($page = 'all') { - // TODO rename to get_slots if ($page === 'all') { $numbers = array(); foreach ($this->pagelayout as $numbersonpage) { @@ -757,7 +780,7 @@ public function start_attempt_url() { * this page to be output as only a fragment. * @return string the URL to continue this attempt. */ - public function attempt_url($slot = 0, $page = -1, $thispage = -1) { + public function attempt_url($slot = null, $page = -1, $thispage = -1) { return $this->page_and_question_url('attempt', $slot, $page, false, $thispage); } @@ -785,7 +808,7 @@ public function processattempt_url() { * this page to be output as only a fragment. * @return string the URL to review this attempt. */ - public function review_url($slot = 0, $page = -1, $showall = false, $thispage = -1) { + public function review_url($slot = null, $page = -1, $showall = false, $thispage = -1) { return $this->page_and_question_url('review', $slot, $page, $showall, $thispage); } @@ -889,11 +912,10 @@ public function render_question_for_commenting($slot) { * @param string $thispageurl the URL of the page this question is being printed on. * @return string HTML for the question in its current state. */ - public function check_file_access($questionid, $reviewing, $contextid, $component, + public function check_file_access($slot, $reviewing, $contextid, $component, $filearea, $args, $forcedownload) { - return question_check_file_access($this->questions[$questionid], - $this->get_question_state($questionid), $this->get_display_options($reviewing), - $contextid, $component, $filearea, $args, $forcedownload); + return $this->quba->check_file_access($slot, $this->get_display_options($reviewing), + $component, $filearea, $args, $forcedownload); } /** @@ -1030,7 +1052,7 @@ public function question_print_comment_fields($slot, $prefix) { protected function page_and_question_url($script, $slot, $page, $showall, $thispage) { // Fix up $page if ($page == -1) { - if ($slot && !$showall) { + if (!is_null($slot) && !$showall) { $page = $this->quba->get_question($slot)->_page; } else { $page = 0; @@ -1043,7 +1065,7 @@ protected function page_and_question_url($script, $slot, $page, $showall, $thisp // Add a fragment to scroll down to the question. $fragment = ''; - if ($slot) { + if (!is_null($slot)) { if ($slot == reset($this->pagelayout[$page])) { // First question on page, go to top. $fragment = '#'; diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 5e2be4629d94d..be6909692a9c6 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -1608,16 +1608,13 @@ function quiz_pluginfile($course, $cm, $context, $filearea, $args, $forcedownloa * @param bool $forcedownload whether the user must be forced to download the file. * @return bool false if file not found, does not return if found - justsend the file */ -function quiz_question_pluginfile($course, $context, $component, - $filearea, $attemptid, $questionid, $args, $forcedownload) { +function mod_quiz_question_pluginfile($course, $context, $component, + $filearea, $qubaid, $slot, $args, $forcedownload) { global $USER, $CFG; require_once($CFG->dirroot . '/mod/quiz/locallib.php'); - $attemptobj = quiz_attempt::create($attemptid); + $attemptobj = quiz_attempt::create_from_usage_id($qubaid); require_login($attemptobj->get_courseid(), false, $attemptobj->get_cm()); - $questionids = array($questionid); - $attemptobj->load_questions($questionids); - $attemptobj->load_question_states($questionids); if ($attemptobj->is_own_attempt() && !$attemptobj->is_finished()) { // In the middle of an attempt. @@ -1632,7 +1629,7 @@ function quiz_question_pluginfile($course, $context, $component, $isreviewing = true; } - if (!$attemptobj->check_file_access($questionid, $isreviewing, $context->id, + if (!$attemptobj->check_file_access($slot, $isreviewing, $context->id, $component, $filearea, $args, $forcedownload)) { send_file_not_found(); } diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index a7be977932015..20093ecc81f75 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -157,29 +157,6 @@ function quiz_get_latest_attempt_by_user($quizid, $userid) { } } -/** - * Load an attempt by id. You need to use this method instead of $DB->get_record, because - * of some ancient history to do with the upgrade from Moodle 1.4 to 1.5, See the comment - * after CREATE TABLE `prefix_quiz_newest_states` in mod/quiz/db/mysql.php. - * - * @param integer $attemptid the id of the attempt to load. - */ -function quiz_load_attempt($attemptid) { - global $DB; - $attempt = $DB->get_record('quiz_attempts', array('id' => $attemptid)); - if (!$attempt) { - return false; - } - - // TODO deal with the issue that makes this necessary. -// if (!$DB->record_exists('question_sessions', array('attemptid' => $attempt->uniqueid))) { -// /// this attempt has not yet been upgraded to the new model -// quiz_upgrade_states($attempt); -// } - - return $attempt; -} - /** * Delete a quiz attempt. * @param mixed $attempt an integer attempt id or an attempt object (row of the quiz_attempts table). diff --git a/mod/quiz/processattempt.php b/mod/quiz/processattempt.php index f4789a5ea49eb..b1b052c673582 100644 --- a/mod/quiz/processattempt.php +++ b/mod/quiz/processattempt.php @@ -60,7 +60,7 @@ } else { $nexturl = $attemptobj->attempt_url(0, $page); if ($scrollpos !== '') { - $nexturl .= '&scrollpos=' . ((int) $scrollpos); + $nexturl->param('scrollpos', $scrollpos); } } diff --git a/question/engine/bank.php b/question/engine/bank.php index 2b8548f52622b..1c1cb66b6b321 100644 --- a/question/engine/bank.php +++ b/question/engine/bank.php @@ -145,7 +145,11 @@ public static function load_question($questionid, $allowshuffle = true) { return self::return_test_question_data($questionid); } - $questiondata = $DB->get_record('question', array('id' => $questionid), '*', MUST_EXIST); + $questiondata = $DB->get_record_sql(' + SELECT q.*, qc.contextid + FROM {question} q + JOIN {question_categories} qc ON q.category = qc.id + WHERE q.id = :id', array('id' => $questionid), MUST_EXIST); get_question_options($questiondata); if (!$allowshuffle) { $questiondata->options->shuffleanswers = false; diff --git a/question/engine/lib.php b/question/engine/lib.php index 747e79cdbf685..2293f433a36b7 100644 --- a/question/engine/lib.php +++ b/question/engine/lib.php @@ -1313,6 +1313,7 @@ class question_attempt { 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. */ @@ -1808,7 +1809,7 @@ public function summarise_action(question_attempt_step $step) { */ public function rewrite_pluginfile_urls($text, $component, $filearea, $itemid) { return question_rewrite_question_urls($text, - 'pluginfile.php', $this->owningcontextid, $component, $filearea, + 'pluginfile.php', $this->question->contextid, $component, $filearea, array($this->get_usage_id(), $this->get_slot()), $itemid); } diff --git a/question/type/ddwtos/simpletest/testquestiontype.php b/question/type/ddwtos/simpletest/testquestiontype.php index f8bfa87e9c6f0..4ef6567c5b5cb 100644 --- a/question/type/ddwtos/simpletest/testquestiontype.php +++ b/question/type/ddwtos/simpletest/testquestiontype.php @@ -62,6 +62,7 @@ protected function get_test_question_data() { $dd = new stdClass; $dd->id = 0; $dd->category = 0; + $dd->contextid = 0; $dd->parent = 0; $dd->questiontextformat = FORMAT_HTML; $dd->generalfeedbackformat = FORMAT_HTML; diff --git a/question/type/gapselect/simpletest/testquestiontype.php b/question/type/gapselect/simpletest/testquestiontype.php index 4f9465e328102..7860664e606f3 100644 --- a/question/type/gapselect/simpletest/testquestiontype.php +++ b/question/type/gapselect/simpletest/testquestiontype.php @@ -64,6 +64,7 @@ protected function get_test_question_data() { $gapselect = new stdClass; $gapselect->id = 0; $gapselect->category = 0; + $gapselect->contextid = 0; $gapselect->parent = 0; $gapselect->questiontextformat = FORMAT_HTML; $gapselect->generalfeedbackformat = FORMAT_HTML; diff --git a/question/type/match/simpletest/testquestiontype.php b/question/type/match/simpletest/testquestiontype.php index 4792fd50f138b..0ab03012331ea 100644 --- a/question/type/match/simpletest/testquestiontype.php +++ b/question/type/match/simpletest/testquestiontype.php @@ -24,8 +24,11 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ + +require_once($CFG->dirroot . '/question/engine/simpletest/helpers.php'); require_once($CFG->dirroot . '/question/type/match/questiontype.php'); + /** * Unit tests for the matching question definition class. * @@ -50,6 +53,7 @@ protected function get_test_question_data() { $q->id = 0; $q->name = 'Matching question'; $q->category = 0; + $q->contextid = 0; $q->parent = 0; $q->questiontext = 'Classify the animals.'; $q->questiontextformat = FORMAT_HTML; diff --git a/question/type/missingtype/simpletest/testmissingtype.php b/question/type/missingtype/simpletest/testmissingtype.php index d3f9d87c7358e..a85d82db1c488 100644 --- a/question/type/missingtype/simpletest/testmissingtype.php +++ b/question/type/missingtype/simpletest/testmissingtype.php @@ -35,6 +35,7 @@ protected function get_unknown_questiondata() { $questiondata = new stdClass; $questiondata->id = 0; $questiondata->category = 0; + $questiondata->contextid = 0; $questiondata->parent = 0; $questiondata->name = 'Test'; $questiondata->questiontext = 'This is the question text.'; diff --git a/question/type/questionbase.php b/question/type/questionbase.php index 9ed5007da045d..5bd83ffea6199 100644 --- a/question/type/questionbase.php +++ b/question/type/questionbase.php @@ -45,6 +45,9 @@ abstract class question_definition { /** @var integer question category id. */ public $category; + /** @var integer question category id. */ + public $contextid; + /** @var integer parent question id. */ public $parent = 0; diff --git a/question/type/questiontype.php b/question/type/questiontype.php index f67165f5ce3b6..20a94c6670a26 100644 --- a/question/type/questiontype.php +++ b/question/type/questiontype.php @@ -642,6 +642,7 @@ protected function make_question_instance($questiondata) { protected function initialise_question_instance(question_definition $question, $questiondata) { $question->id = $questiondata->id; $question->category = $questiondata->category; + $question->contextid = $questiondata->contextid; $question->parent = $questiondata->parent; $question->qtype = $this; $question->name = $questiondata->name;