From 81d833adaa4ac66178b52d0bc4d71166bb7670ba Mon Sep 17 00:00:00 2001 From: tjhunt Date: Thu, 4 Sep 2008 06:37:43 +0000 Subject: [PATCH] MDL-15540 Navigation panel did not work on the review page for multi-page quizzes because of the way states were loaded. Refactor. --- lib/questionlib.php | 189 +++++++++++++++++++++++++--------------- mod/quiz/attemptlib.php | 23 +++-- question/preview.php | 3 +- 3 files changed, 138 insertions(+), 77 deletions(-) diff --git a/lib/questionlib.php b/lib/questionlib.php index b7d3bf51a2..9f7c6614e2 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -885,63 +885,77 @@ function get_question_options(&$questions) { } /** -* Loads the most recent state of each question session from the database -* or create new one. -* -* For each question the most recent session state for the current attempt -* is loaded from the question_states table and the question type specific data and -* responses are added by calling {@link restore_question_state()} which in turn -* calls {@link restore_session_and_responses()} for each question. -* If no states exist for the question instance an empty state object is -* created representing the start of a session and empty question -* type specific information and responses are created by calling -* {@link create_session_and_responses()}. -* -* @return array An array of state objects representing the most recent -* states of the question sessions. -* @param array $questions The questions for which sessions are to be restored or -* created. -* @param object $cmoptions -* @param object $attempt The attempt for which the question sessions are -* to be restored or created. -* @param mixed either the id of a previous attempt, if this attmpt is -* building on a previous one, or false for a clean attempt. -*/ -function get_question_states(&$questions, $cmoptions, $attempt, $lastattemptid = false) { - global $CFG, $QTYPES, $DB; - - // get the question ids - $ids = array_keys($questions); - $questionlist = implode(',', $ids); + * Load the basic state information for + * + * @param integer $attemptid the attempt id to load the states for. + * @return array an array of state data from the database, you will subsequently + * need to call question_load_states to get fully loaded states that can be + * used by the question types. The states here should be sufficient for + * basic tasks like rendering navigation. + */ +function question_preload_states($attemptid) { + global $DB; - // The question field must be listed first so that it is used as the + // The questionid field must be listed first so that it is used as the // array index in the array returned by $DB->get_records_sql $statefields = 'n.questionid as question, s.*, n.sumpenalty, n.manualcomment, n.flagged, n.id as questionsessionid'; + // Load the newest states for the questions $sql = "SELECT $statefields FROM {question_states} s, {question_sessions} n - WHERE s.id = n.newest - AND n.attemptid = ? - AND n.questionid IN ($questionlist)"; - $states = $DB->get_records_sql($sql, array($attempt->uniqueid)); + WHERE s.id = n.newest AND n.attemptid = ?"; + $states = $DB->get_records_sql($sql, array($attemptid)); + if (!$states) { + return false; + } // Load the newest graded states for the questions $sql = "SELECT $statefields FROM {question_states} s, {question_sessions} n - WHERE s.id = n.newgraded - AND n.attemptid = ? - AND n.questionid IN ($questionlist)"; - $gradedstates = $DB->get_records_sql($sql, array($attempt->uniqueid)); + WHERE s.id = n.newgraded AND n.attemptid = ?"; + $gradedstates = $DB->get_records_sql($sql, array($attemptid)); + + // Hook the two together. + foreach ($states as $questionid => $state) { + $states[$questionid]->_partiallyloaded = true; + if ($gradedstates[$questionid]) { + $gradedstates[$questionid]->_partiallyloaded = true; + $states[$questionid]->last_graded = $gradedstates[$questionid]; + } else { + $states[$questionid]->last_graded = clone($states[$questionid]); + } + } + + return $states; +} + +/** + * Finish loading the question states that were extracted from the database with + * question_preload_states, creating new states for any question where there + * is not a state in the database. + * + * @param array $questions the questions to load state for. + * @param array $states the partially loaded states. + * @param object $cmoptions options from the module we are loading the states for. E.g. $quiz. + * @param object $attempt The attempt for which the question sessions are + * to be restored or created. + * @param mixed either the id of a previous attempt, if this attmpt is + * building on a previous one, or false for a clean attempt. + * @return array the fully loaded states. + */ +function question_load_states(&$questions, &$states, $cmoptions, $attempt, $lastattemptid = false) { + global $QTYPES, $DB; // loop through all questions and set the last_graded states - foreach ($ids as $i) { - if (isset($states[$i])) { - restore_question_state($questions[$i], $states[$i]); - if (isset($gradedstates[$i])) { - restore_question_state($questions[$i], $gradedstates[$i]); - $states[$i]->last_graded = $gradedstates[$i]; - } else { - $states[$i]->last_graded = clone($states[$i]); + foreach (array_keys($questions) as $qid) { + if (isset($states[$qid])) { + restore_question_state($questions[$qid], $states[$qid]); + if (isset($states[$qid]->_partiallyloaded)) { + unset($states[$qid]->_partiallyloaded); + } + if (isset($states[$qid]->lastgraded->_partiallyloaded)) { + restore_question_state($questions[$qid], $states[$qid]->lastgraded); + unset($states[$qid]->lastgraded->_partiallyloaded); } } else { // If the new attempt is to be based on a previous attempt get it and clean things @@ -957,36 +971,36 @@ function get_question_states(&$questions, $cmoptions, $attempt, $lastattemptid = WHERE s.id = n.newgraded AND n.attemptid = ? AND n.questionid = ?"; - if (!$laststate = $DB->get_record_sql($sql, array($lastattemptid, $i))) { + if (!$laststate = $DB->get_record_sql($sql, array($lastattemptid, $qid))) { // Only restore previous responses that have been graded continue; } // Restore the state so that the responses will be restored - restore_question_state($questions[$i], $laststate); - $states[$i] = clone($laststate); - unset($states[$i]->id); + restore_question_state($questions[$qid], $laststate); + $states[$qid] = clone($laststate); + unset($states[$qid]->id); } else { // create a new empty state - $states[$i] = new object; - $states[$i]->question = $i; - $states[$i]->responses = array('' => ''); - $states[$i]->raw_grade = 0; + $states[$qid] = new object; + $states[$qid]->question = $qid; + $states[$qid]->responses = array('' => ''); + $states[$qid]->raw_grade = 0; } // now fill/overide initial values - $states[$i]->attempt = $attempt->uniqueid; - $states[$i]->seq_number = 0; - $states[$i]->timestamp = $attempt->timestart; - $states[$i]->event = ($attempt->timefinish) ? QUESTION_EVENTCLOSE : QUESTION_EVENTOPEN; - $states[$i]->grade = 0; - $states[$i]->penalty = 0; - $states[$i]->sumpenalty = 0; - $states[$i]->manualcomment = ''; - $states[$i]->flagged = 0; + $states[$qid]->attempt = $attempt->uniqueid; + $states[$qid]->seq_number = 0; + $states[$qid]->timestamp = $attempt->timestart; + $states[$qid]->event = ($attempt->timefinish) ? QUESTION_EVENTCLOSE : QUESTION_EVENTOPEN; + $states[$qid]->grade = 0; + $states[$qid]->penalty = 0; + $states[$qid]->sumpenalty = 0; + $states[$qid]->manualcomment = ''; + $states[$qid]->flagged = 0; // Prevent further changes to the session from incrementing the // sequence number - $states[$i]->changed = true; + $states[$qid]->changed = true; if ($lastattemptid) { // prepare the previous responses for new processing @@ -996,26 +1010,65 @@ function get_question_states(&$questions, $cmoptions, $attempt, $lastattemptid = $action->event = QUESTION_EVENTSAVE; //emulate save of questions from all pages MDL-7631 // Process these responses ... - question_process_responses($questions[$i], $states[$i], $action, $cmoptions, $attempt); + question_process_responses($questions[$qid], $states[$qid], $action, $cmoptions, $attempt); // Fix for Bug #5506: When each attempt is built on the last one, // preserve the options from any previous attempt. if ( isset($laststate->options) ) { - $states[$i]->options = $laststate->options; + $states[$qid]->options = $laststate->options; } } else { // Create the empty question type specific information - if (!$QTYPES[$questions[$i]->qtype]->create_session_and_responses( - $questions[$i], $states[$i], $cmoptions, $attempt)) { + if (!$QTYPES[$questions[$qid]->qtype]->create_session_and_responses( + $questions[$qid], $states[$qid], $cmoptions, $attempt)) { return false; } } - $states[$i]->last_graded = clone($states[$i]); + $states[$qid]->last_graded = clone($states[$qid]); } } return $states; } +/** +* Loads the most recent state of each question session from the database +* or create new one. +* +* For each question the most recent session state for the current attempt +* is loaded from the question_states table and the question type specific data and +* responses are added by calling {@link restore_question_state()} which in turn +* calls {@link restore_session_and_responses()} for each question. +* If no states exist for the question instance an empty state object is +* created representing the start of a session and empty question +* type specific information and responses are created by calling +* {@link create_session_and_responses()}. +* +* @return array An array of state objects representing the most recent +* states of the question sessions. +* @param array $questions The questions for which sessions are to be restored or +* created. +* @param object $cmoptions +* @param object $attempt The attempt for which the question sessions are +* to be restored or created. +* @param mixed either the id of a previous attempt, if this attmpt is +* building on a previous one, or false for a clean attempt. +*/ +function get_question_states(&$questions, $cmoptions, $attempt, $lastattemptid = false) { + // get the question ids + $ids = array_keys($questions); + + // Preload the states. + $states = question_preload_states($attempt->uniqueid); + if (!$states) { + $states = array(); + } + + // Then finish the job. + question_load_states($questions, $states, $cmoptions, $attempt, $lastattemptid); + + return $states; +} + /** * Load a particular previous state of a question. * diff --git a/mod/quiz/attemptlib.php b/mod/quiz/attemptlib.php index 9a91a7d329..8cc44d2265 100644 --- a/mod/quiz/attemptlib.php +++ b/mod/quiz/attemptlib.php @@ -74,7 +74,7 @@ class quiz { $this->number_questions(); } - /** + /** * Load some or all of the queestions for this quiz. * * @param array $questionids question ids of the questions to load. null for all. @@ -396,6 +396,7 @@ class quiz_attempt extends quiz { } parent::__construct($quiz, $cm, $course); $this->preload_questions(); + $this->preload_question_states(); } // Functions for loading more data ===================================================== @@ -413,10 +414,21 @@ class quiz_attempt extends quiz { $this->ensure_question_loaded($id); $questionstoprocess[$id] = $this->questions[$id]; } - if (!$newstates = get_question_states($questionstoprocess, $this->quiz, $this->attempt)) { + if (!$newstates = question_load_states($questionstoprocess, $this->states, + $this->quiz, $this->attempt)) { throw new moodle_quiz_exception($this, 'cannotrestore'); } - $this->states = $newstates + $this->states; + } + + + public function preload_question_states() { + if (empty($this->questionids)) { + throw new moodle_quiz_exception($this, 'noquestions', $this->edit_url()); + } + $this->states = question_preload_states($this->attempt->uniqueid); + if (!$this->states) { + $this->states = array(); + } } public function load_specific_question_state($questionid, $stateid) { @@ -520,7 +532,6 @@ class quiz_attempt extends quiz { * open|saved|closed|correct|partiallycorrect|incorrect. */ public function get_question_status($questionid) { - $this->ensure_state_loaded($questionid); $state = $this->states[$questionid]; switch ($state->event) { case QUESTION_EVENTOPEN: @@ -555,7 +566,6 @@ class quiz_attempt extends quiz { * @return boolean whether this question hss been flagged by the attempter. */ public function is_question_flagged($questionid) { - $this->ensure_state_loaded($questionid); $state = $this->states[$questionid]; return $state->flagged; } @@ -568,7 +578,6 @@ class quiz_attempt extends quiz { * @return string the formatted grade, to the number of decimal places specified by the quiz. */ public function get_question_score($questionid) { - $this->ensure_state_loaded($questionid); $options = $this->get_render_options($this->states[$questionid]); if ($options->scores) { return quiz_format_grade($this->quiz, $this->states[$questionid]->last_graded->grade); @@ -684,7 +693,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)) { + if (!array_key_exists($id, $this->states) || isset($this->states[$id]->_partiallyloaded)) { throw new moodle_quiz_exception($this, 'statenotloaded', $id); } } diff --git a/question/preview.php b/question/preview.php index 1cbdf50ba7..db46d516da 100644 --- a/question/preview.php +++ b/question/preview.php @@ -134,8 +134,7 @@ // Record the question id in the moodle session $SESSION->quizpreview->questionid = $id; // Create an empty session for the question - if (!$newstates = - get_question_states($questions, $quiz, $attempt)) { + if (!$newstates = get_question_states($questions, $quiz, $attempt)) { print_error('newattemptfail', 'quiz'); } $SESSION->quizpreview->states = array($newstates); -- 2.39.5