From b10c38a37b46872800bb9576d464fb10759179db Mon Sep 17 00:00:00 2001 From: tjhunt Date: Mon, 30 Jun 2008 16:56:49 +0000 Subject: [PATCH] MDL-15452 - ongoing - Put the OU quiz navigation improvements into the Moodle codebase * Javadoc comments for most of the code I committed on Friday. * Implement the get_question_status method. * Along the way, refactor duplicated logic between two of the question types, questionlib, and the new code - removing inconsistency while doing so. --- lang/en_utf8/quiz.php | 1 + lib/questionlib.php | 48 ++---- mod/quiz/attemptlib.php | 181 ++++++++++++++++++--- mod/quiz/summary.php | 12 +- question/type/questiontype.php | 15 +- question/type/shortanswer/questiontype.php | 29 +--- 6 files changed, 192 insertions(+), 94 deletions(-) diff --git a/lang/en_utf8/quiz.php b/lang/en_utf8/quiz.php index 97c2b84024..2953dfc302 100644 --- a/lang/en_utf8/quiz.php +++ b/lang/en_utf8/quiz.php @@ -215,6 +215,7 @@ $string['errorinquestion'] = 'Error in question'; $string['errormissingquestion'] = 'Error: The system is missing the question with id $a'; $string['errornotnumbers'] = 'Error - answers must be numeric'; $string['errorsdetected'] = '$a error(s) detected'; +$string['errorunexpectedevent'] = 'Unexpected event code $a->event found for question $a->questionid in attempt $a->attemptid.'; $string['essay'] = 'Essay'; $string['essayquestions'] = 'Questions'; $string['event1'] = 'Autosave'; diff --git a/lib/questionlib.php b/lib/questionlib.php index e813eea954..4bb34256a3 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -1154,38 +1154,20 @@ function question_extract_responses($questions, $formdata, $defaultevent=QUESTIO * @return string */ function question_get_feedback_image($fraction, $selected=true) { - global $CFG; + static $icons = array('correct' => 'tick_green', 'partiallycorrect' => 'tick_amber', + 'incorrect' => 'cross_red'); - if ($fraction >= 1.0) { - if ($selected) { - $feedbackimg = ''; - } else { - $feedbackimg = ''; - } - } else if ($fraction > 0.0 && $fraction < 1.0) { - if ($selected) { - $feedbackimg = ''; - } else { - $feedbackimg = ''; - } + if ($selected) { + $size = 'big'; } else { - if ($selected) { - $feedbackimg = ''; - } else { - $feedbackimg = ''; - } + $size = 'small'; } - return $feedbackimg; + $class = question_get_feedback_class($fraction); + return ''; } - /** * Returns the class name for question feedback. * @param float $fraction value representing the correctness of the user's @@ -1193,17 +1175,13 @@ function question_get_feedback_image($fraction, $selected=true) { * @return string */ function question_get_feedback_class($fraction) { - - global $CFG; - - if ($fraction >= 1.0) { - $class = 'correct'; - } else if ($fraction > 0.0 && $fraction < 1.0) { - $class = 'partiallycorrect'; + if ($fraction >= 1/1.01) { + return 'correct'; + } else if ($fraction > 0.0) { + return 'partiallycorrect'; } else { - $class = 'incorrect'; + return 'incorrect'; } - return $class; } diff --git a/mod/quiz/attemptlib.php b/mod/quiz/attemptlib.php index 7df4d43b7b..a521c72925 100644 --- a/mod/quiz/attemptlib.php +++ b/mod/quiz/attemptlib.php @@ -1,18 +1,17 @@ quiz = $quiz; $this->cm = $cm; @@ -68,35 +79,60 @@ class quiz { } // Simple getters ====================================================================== + /** @return integer the course id. */ public function get_courseid() { return $this->course->id; } + /** @return integer the quiz id. */ public function get_quizid() { return $this->quiz->id; } + /** @return object the row of the quiz table. */ public function get_quiz() { return $this->quiz; } + /** @return string the name of this quiz. */ public function get_quiz_name() { return $this->quiz->name; } + /** @return integer the course_module id. */ public function get_cmid() { return $this->cm->id; } + /** @return object the course_module object. */ public function get_cm() { return $this->cm; } + /** + * @return boolean wether the current user is someone who previews the quiz, + * rather than attempting it. + */ + public function is_preview_user() { + if (is_null($this->ispreviewuser)) { + $this->ispreviewuser = has_capability('mod/quiz:preview', $this->context); + } + return $this->ispreviewuser; + } + + /** + * @param integer $id the question id. + * @return object the question object with that id. + */ public function get_question($id) { $this->ensure_question_loaded($id); return $this->questions[$id]; } + /** + * @param integer $timenow the current time as a unix timestamp. + * @return object and instance of the quiz_access_manager class for this quiz at this time. + */ public function get_access_manager($timenow) { if (is_null($this->accessmanager)) { $this->accessmanager = new quiz_access_manager($this->quiz, $timenow, @@ -106,12 +142,19 @@ class quiz { } // URLs related to this attempt ======================================================== + /** + * @return string the URL of this quiz's view page. + */ public function view_url() { global $CFG; return $CFG->wwwroot . '/mod/quiz/view.php?id=' . $this->cm->id; } // Bits of content ===================================================================== + /** + * @return string the HTML snipped that needs to be supplied to print_header_simple + * as the $button parameter. + */ public function update_module_button() { if (has_capability('moodle/course:manageactivities', get_context_instance(CONTEXT_COURSE, $this->course->id))) { @@ -121,11 +164,17 @@ class quiz { } } + /** + * @param string $title the name of this particular quiz page. + * @return array the data that needs to be sent to print_header_simple as the $navigation + * parameter. + */ public function navigation($title) { return build_navigation($title, $this->cm); } // Private methods ===================================================================== + // Check that the definition of a particular question is loaded, and if not throw an exception. private function ensure_question_loaded($id) { if (!array_key_exists($id, $this->questions)) { throw new moodle_quiz_exception($this, 'questionnotloaded', $id); @@ -133,15 +182,24 @@ class quiz { } } +/** + * This class extends the quiz class to hold data about the state of a particular attempt, + * in addition to the data about the quiz. + */ class quiz_attempt extends quiz { // Fields initialised in the constructor. protected $attempt; // Fields set later if that data is needed. - protected $ispreview = null; protected $states = array(); // Constructor ========================================================================= + /** + * Constructor from just an attemptid. + * + * @param integer $attemptid the id of the attempt to load. We automatically load the + * associated quiz, course, etc. + */ function __construct($attemptid) { global $DB; if (!$this->attempt = quiz_load_attempt($attemptid)) { @@ -185,7 +243,7 @@ class quiz_attempt extends quiz { } /** - * Number the loaded quetsions. + * Number the loaded questions. * * At the moment, this assumes for simplicity that the loaded questions are contiguous. */ @@ -211,29 +269,31 @@ class quiz_attempt extends quiz { } // Simple getters ====================================================================== + /** @return integer the attempt id. */ public function get_attemptid() { return $this->attempt->id; } + /** @return object the row from the quiz_attempts table. */ public function get_attempt() { return $this->attempt; } + /** @return integer the id of the user this attempt belongs to. */ public function get_userid() { return $this->attempt->userid; } + /** @return boolean whether this attemp has been finished (true) or is still in progress (false). */ public function is_finished() { return $this->attempt->timefinish != 0; } - public function is_preview() { - if (is_null($this->ispreview)) { - $this->ispreview = has_capability('mod/quiz:preview', $this->context); - } - return $this->ispreview; - } - + /** + * Wrapper that calls quiz_get_reviewoptions with the appropriate arguments. + * + * @return object the review optoins for this user on this attempt. + */ public function get_review_options() { if (is_null($this->reviewoptions)) { $this->reviewoptions = quiz_get_reviewoptions($this->quiz, $this->attempt, $this->context); @@ -241,6 +301,13 @@ class quiz_attempt extends quiz { return $this->reviewoptions; } + /** + * Return the list of question ids for either a given page of the quiz, or for the + * whole quiz. + * + * @param mixed $page string 'all' or integer page number. + * @return array the reqested list of question ids. + */ public function get_question_ids($page = 'all') { if ($page == 'all') { $questionlist = quiz_questions_in_quiz($this->attempt->layout); @@ -250,19 +317,62 @@ class quiz_attempt extends quiz { return explode(',', $questionlist); } + /** + * Get a quiz_attempt_question_iterator for either a page of the quiz, or a whole quiz. + * You must have called load_questions with an appropriate argument first. + * + * @param mixed $page as for the @see{get_question_ids} method. + * @return quiz_attempt_question_iterator the requested iterator. + */ public function get_question_iterator($page = 'all') { return new quiz_attempt_question_iterator($this, $page); } + /** + * Return a summary of the current state of a question in this attempt. You must previously + * have called load_question_states to load the state data about this question. + * + * @param integer $questionid question id of a question that belongs to this quiz. + * @return string a brief string (that could be used as a CSS class name, for example) + * that describes the current state of a question in this attempt. Possible results are: + * open|saved|closed|correct|partiallycorrect|incorrect. + */ public function get_question_status($questionid) { - //TODO - return 'FROG'; + $this->ensure_state_loaded($questionid); + $state = $this->states[$questionid]; + switch ($state->event) { + case QUESTION_EVENTOPEN: + return 'open'; + + case QUESTION_EVENTSAVE: + case QUESTION_EVENTGRADE: + return 'saved'; + + case QUESTION_EVENTCLOSEANDGRADE: + case QUESTION_EVENTCLOSE: + case QUESTION_EVENTMANUALGRADE: + $options = quiz_get_renderoptions($this->quiz->review, $this->states[$questionid]); + if ($options->scores) { + return question_get_feedback_class($state->last_graded->raw_grade / + $this->questions[$questionid]->maxgrade); + } else { + return 'closed'; + } + + default: + $a = new stdClass; + $a->event = $state->event; + $a->questionid = $questionid; + $a->attemptid = $this->attempt->id; + throw new moodle_quiz_exception($this, 'errorunexpectedevent', $a); + } } /** - * Return the grade obtained on a particular question, if the user ispermitted to see it. + * Return the grade obtained on a particular question, if the user is permitted to see it. + * You must previously have called load_question_states to load the state data about this question. * - * @param integer $questionid + * @param integer $questionid question id of a question that belongs to this quiz. * @return string the formatted grade, to the number of decimal places specified by the quiz. */ public function get_question_score($questionid) { @@ -276,6 +386,13 @@ class quiz_attempt extends quiz { } // URLs related to this attempt ======================================================== + /** + * @param integer $page if specified, the URL of this particular page of the attempt, otherwise + * the URL will go to the first page. + * @param integer $question a question id. If set, will add a fragment to the URL + * to jump to a particuar question on the page. + * @return string the URL to continue this attempt. + */ public function attempt_url($page = 0, $question = false) { global $CFG; $fragment = ''; @@ -286,11 +403,23 @@ class quiz_attempt extends quiz { $this->cm->id . '$amp;page=' . $page . $fragment; } + /** + * @return string the URL of this quiz's summary page. + */ public function summary_url() { global $CFG; return $CFG->wwwroot . '/mod/quiz/summary.php?attempt=' . $this->attempt->id; } + /** + * @param integer $page if specified, the URL of this particular page of the attempt, otherwise + * the URL will go to the first page. + * @param integer $question a question id. If set, will add a fragment to the URL + * to jump to a particuar question on the page. + * @param boolean $showall if true, the URL will be to review the entire attempt on one page, + * and $page will be ignored. + * @return string the URL to review this attempt. + */ public function review_url($page = 0, $question = false, $showall = false) { global $CFG; $fragment = ''; @@ -309,6 +438,7 @@ class quiz_attempt extends quiz { // Private methods ===================================================================== + // Check that the state of a particular question is loaded, and if not throw an exception. private function ensure_state_loaded($id) { if (!array_key_exists($id, $this->states)) { throw new moodle_quiz_exception($this, 'statenotloaded', $id); @@ -316,15 +446,28 @@ class quiz_attempt extends quiz { } } +/** + * A PHP Iterator for conviniently looping over the questions in a quiz. The keys are the question + * numbers (with 'i' for descriptions) and the values are the question objects. + */ class quiz_attempt_question_iterator implements Iterator { - private $attemptobj; - private $questionids; + private $attemptobj; // Reference to the quiz_attempt object we provide access to. + private $questionids; // Array of the question ids within that attempt we are iterating over. + + /** + * Constructor. Normally, you don't want to call this directly. Instead call + * quiz_attempt::get_question_iterator + * + * @param quiz_attempt $attemptobj the quiz_attempt object we will be providing access to. + * @param mixed $page as for @see{quiz_attempt::get_question_iterator}. + */ public function __construct(quiz_attempt $attemptobj, $page = 'all') { $this->attemptobj = $attemptobj; $attemptobj->number_questions($page); $this->questionids = $attemptobj->get_question_ids($page); } + // Implementation of the Iterator interface ============================================ public function rewind() { reset($this->questionids); } diff --git a/mod/quiz/summary.php b/mod/quiz/summary.php index bbb45e548c..ddb0efe3df 100644 --- a/mod/quiz/summary.php +++ b/mod/quiz/summary.php @@ -30,11 +30,11 @@ if ($attemptobj->is_finished()) { /// Check access. $accessmanager = $attemptobj->get_access_manager(time()); $messages = $accessmanager->prevent_access(); -if (!$attemptobj->is_preview() && $messages) { +if (!$attemptobj->is_preview_user() && $messages) { print_error('attempterror', 'quiz', $attemptobj->view_url(), $accessmanager->print_messages($messages, true)); } -$accessmanager->do_password_check($attemptobj->is_preview()); +$accessmanager->do_password_check($attemptobj->is_preview_user()); /// Log this page view. add_to_log($attemptobj->get_courseid(), 'quiz', 'view summary', 'summary.php?attempt=' . $attemptobj->get_attemptid(), @@ -47,7 +47,7 @@ $attemptobj->load_question_states(); /// Print the page header require_js($CFG->wwwroot . '/mod/quiz/quiz.js'); $title = get_string('summaryofattempt', 'quiz'); -if ($accessmanager->securewindow_required($attemptobj->is_preview())) { +if ($accessmanager->securewindow_required($attemptobj->is_preview_user())) { $accessmanager->setup_secure_page($course->shortname.': '.format_string($quiz->name), $headtags); } else { print_header_simple(format_string($attemptobj->get_quiz_name()), '', @@ -55,14 +55,14 @@ if ($accessmanager->securewindow_required($attemptobj->is_preview())) { } /// Print tabs if they should be there. -if ($attemptobj->is_preview()) { +if ($attemptobj->is_preview_user()) { $currenttab = 'preview'; include('tabs.php'); } /// Print heading. print_heading(format_string($attemptobj->get_quiz_name())); -if ($attemptobj->is_preview()) { +if ($attemptobj->is_preview_user()) { print_restart_preview_button($quiz); } print_heading($title); @@ -107,7 +107,7 @@ echo "\n"; /// Finish the page $accessmanager->show_attempt_timer_if_needed($attemptobj->get_attempt(), time()); -if ($accessmanager->securewindow_required($attemptobj->is_preview())) { +if ($accessmanager->securewindow_required($attemptobj->is_preview_user())) { print_footer('empty'); } else { print_footer($course); diff --git a/question/type/questiontype.php b/question/type/questiontype.php index ed5d91b7b1..fa08a6f107 100644 --- a/question/type/questiontype.php +++ b/question/type/questiontype.php @@ -992,18 +992,9 @@ class default_questiontype { $grade->raw = round($state->last_graded->raw_grade, $cmoptions->decimalpoints); // let student know wether the answer was correct - echo '
'; - print_string('correct', 'quiz'); - } else if ($state->last_graded->raw_grade > 0) { - echo ' partiallycorrect">'; - print_string('partiallycorrect', 'quiz'); - } else { - echo ' incorrect">'; - print_string('incorrect', 'quiz'); - } - echo '
'; + $class = question_get_feedback_class($state->last_graded->raw_grade / + $question->maxgrade); + echo '
' . get_string($class, 'quiz') . '
'; echo '
'; // print grade for this submission diff --git a/question/type/shortanswer/questiontype.php b/question/type/shortanswer/questiontype.php index 78828205fe..a770a40667 100644 --- a/question/type/shortanswer/questiontype.php +++ b/question/type/shortanswer/questiontype.php @@ -405,28 +405,13 @@ class question_shortanswer_qtype extends default_questiontype { $grade->raw = round($state->last_graded->raw_grade, $cmoptions->decimalpoints); // let student know wether the answer was correct - echo '
'; - print_string('correct', 'quiz'); - } else if ($state->last_graded->raw_grade > 0) { - echo ' partiallycorrect">'; - print_string('partiallycorrect', 'quiz'); - // MDL-7496 - if ($correctanswer) { - echo ('
'); - print_string('correctansweris', 'quiz', s($correctanswer, true)); - echo ('
'); - } - } else { - echo ' incorrect">'; - // MDL-7496 - print_string('incorrect', 'quiz'); - if ($correctanswer) { - echo ('
'); - print_string('correctansweris', 'quiz', s($correctanswer, true)); - echo ('
'); - } + $class = question_get_feedback_class($state->last_graded->raw_grade / + $question->maxgrade); + echo '
' . get_string($class, 'quiz'); + if ($correctanswer && ($class == 'partiallycorrect' || $class == 'incorrect')) { + echo ('
'); + print_string('correctansweris', 'quiz', s($correctanswer, true)); + echo ('
'); } echo '
'; -- 2.39.5