diff --git a/mod/quiz/attemptlib.php b/mod/quiz/attemptlib.php index 237ce19484d..7f5868ffc71 100644 --- a/mod/quiz/attemptlib.php +++ b/mod/quiz/attemptlib.php @@ -433,6 +433,9 @@ class quiz_attempt { /** @var string to identify the abandoned state. */ const ABANDONED = 'abandoned'; + /** @var int maximum number of slots in the quiz for the review page to default to show all. */ + const MAX_SLOTS_FOR_DEFAULT_REVIEW_SHOW_ALL = 50; + // Basic data. protected $quizobj; protected $attempt; @@ -522,7 +525,10 @@ class quiz_attempt { return quiz_attempt_state_name($state); } - private function determine_layout() { + /** + * Parse attempt->layout to populate the other arrays the represent the layout. + */ + protected function determine_layout() { $this->pagelayout = array(); // Break up the layout string into pages. @@ -544,15 +550,16 @@ class quiz_attempt { } } - // Number the questions. - private function number_questions() { + /** + * Work out the number to display for each question/slot. + */ + protected function number_questions() { $number = 1; foreach ($this->pagelayout as $page => $slots) { foreach ($slots as $slot) { - $question = $this->quba->get_question($slot); - if ($question->length > 0) { + if ($length = $this->is_real_question($slot)) { $this->questionnumbers[$slot] = $number; - $number += $question->length; + $number += $length; } else { $this->questionnumbers[$slot] = get_string('infoshort', 'quiz'); } @@ -934,10 +941,11 @@ class quiz_attempt { /** * Is a particular question in this attempt a real question, or something like a description. * @param int $slot the number used to identify this question within this attempt. - * @return bool whether that question is a real question. + * @return int whether that question is a real question. Actually returns the + * question length, which could theoretically be greater than one. */ public function is_real_question($slot) { - return $this->quba->get_question($slot)->length != 0; + return $this->quba->get_question($slot)->length; } /** @@ -1135,16 +1143,25 @@ class quiz_attempt { * @param int $slot indicates which question to link to. * @param int $page if specified, the URL of this particular page of the attempt, otherwise * the URL will go to the first page. If -1, deduce $page from $slot. - * @param bool $showall if true, the URL will be to review the entire attempt on one page, - * and $page will be ignored. + * @param bool|null $showall if true, the URL will be to review the entire attempt on one page, + * and $page will be ignored. If null, a sensible default will be chosen. * @param int $thispage if not -1, the current page. Will cause links to other things on * this page to be output as only a fragment. * @return string the URL to review this attempt. */ - public function review_url($slot = null, $page = -1, $showall = false, $thispage = -1) { + public function review_url($slot = null, $page = -1, $showall = null, $thispage = -1) { return $this->page_and_question_url('review', $slot, $page, $showall, $thispage); } + /** + * By default, should this script show all questions on one page for this attempt? + * @param string $script the script name, e.g. 'attempt', 'summary', 'review'. + * @return whether show all on one page should be on by default. + */ + public function get_default_show_all($script) { + return $script == 'review' && count($this->questionpages) < self::MAX_SLOTS_FOR_DEFAULT_REVIEW_SHOW_ALL; + } + // Bits of content ========================================================= /** @@ -1572,15 +1589,22 @@ class quiz_attempt { * 0 to just use the $page parameter. * @param int $page -1 to look up the page number from the slot, otherwise * the page number to go to. - * @param bool $showall if true, return a URL with showall=1, and not page number + * @param bool|null $showall if true, return a URL with showall=1, and not page number. + * if null, then an intelligent default will be chosen. * @param int $thispage the page we are currently on. Links to questions on this * page will just be a fragment #q123. -1 to disable this. * @return The requested URL. */ protected function page_and_question_url($script, $slot, $page, $showall, $thispage) { + + $defaultshowall = $this->get_default_show_all($script); + if ($showall === null && ($page == 0 || $page == -1)) { + $showall = $defaultshowall; + } + // Fix up $page. if ($page == -1) { - if (!is_null($slot) && !$showall) { + if ($slot !== null && !$showall) { $page = $this->get_question_page($slot); } else { $page = 0; @@ -1593,7 +1617,7 @@ class quiz_attempt { // Add a fragment to scroll down to the question. $fragment = ''; - if (!is_null($slot)) { + if ($slot !== null) { if ($slot == reset($this->pagelayout[$page])) { // First question on page, go to top. $fragment = '#'; @@ -1609,8 +1633,8 @@ class quiz_attempt { } else { $url = new moodle_url('/mod/quiz/' . $script . '.php' . $fragment, array('attempt' => $this->attempt->id)); - if ($showall) { - $url->param('showall', 1); + if ($page == 0 && $showall != $defaultshowall) { + $url->param('showall', (int) $showall); } else if ($page > 0) { $url->param('page', $page); } diff --git a/mod/quiz/review.php b/mod/quiz/review.php index 4c2deb7fc1f..b9237998bcf 100644 --- a/mod/quiz/review.php +++ b/mod/quiz/review.php @@ -32,13 +32,12 @@ require_once($CFG->dirroot . '/mod/quiz/report/reportlib.php'); $attemptid = required_param('attempt', PARAM_INT); $page = optional_param('page', 0, PARAM_INT); -$showall = optional_param('showall', 0, PARAM_BOOL); +$showall = optional_param('showall', null, PARAM_BOOL); $url = new moodle_url('/mod/quiz/review.php', array('attempt'=>$attemptid)); if ($page !== 0) { $url->param('page', $page); -} -if ($showall !== 0) { +} else if ($showall) { $url->param('showall', $showall); } $PAGE->set_url($url); @@ -46,6 +45,12 @@ $PAGE->set_url($url); $attemptobj = quiz_attempt::create($attemptid); $page = $attemptobj->force_page_number_into_range($page); +// Now we can validate the params better, re-genrate the page URL. +if ($showall === null) { + $showall = $page == 0 && $attemptobj->get_default_show_all('review'); +} +$PAGE->set_url($attemptobj->review_url(null, $page, $showall)); + // Check login. require_login($attemptobj->get_course(), false, $attemptobj->get_cm()); $attemptobj->check_review_capability(); diff --git a/mod/quiz/tests/attempt_test.php b/mod/quiz/tests/attempt_test.php new file mode 100644 index 00000000000..209861846aa --- /dev/null +++ b/mod/quiz/tests/attempt_test.php @@ -0,0 +1,289 @@ +. + +/** + * Tests for the quiz_attempt class. + * + * @package mod_quiz + * @category test + * @copyright 2014 Tim Hunt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/mod/quiz/locallib.php'); + + +/** + * Subclass of quiz_attempt to allow faking of the page layout. + * + * @copyright 2014 Tim Hunt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mod_quiz_attempt_testable extends quiz_attempt { + /** @var array list of slots to treat as if they contain descriptions in the fake layout. */ + protected $infos = array(); + + /** + * Set a fake page layout. Used when we test URL generation. + * @param int $id assumed attempt id. + * @param string $layout layout to set. Like quiz attempt.layout. E.g. '1,2,0,3,4,0,'. + * @param array $infos slot numbers which contain 'descriptions', or other non-questions. + * @return quiz_attempt attempt object for use in unit tests. + */ + public static function setup_fake_attempt_layout($id, $layout, $infos = array()) { + $attempt = new stdClass(); + $attempt->id = $id; + $attempt->layout = $layout; + + $course = new stdClass(); + $quiz = new stdClass(); + $cm = new stdClass(); + $cm->id = 0; + + $attemptobj = new self($attempt, $quiz, $cm, $course, false); + + $attemptobj->infos = $infos; + $attemptobj->determine_layout(); + $attemptobj->number_questions(); + + return $attemptobj; + } + + public function is_real_question($slot) { + return !in_array($slot, $this->infos); + } +} + + +/** + * Tests for the quiz_attempt class. + * + * @copyright 2014 Tim Hunt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mod_quiz_attempt_testcase extends basic_testcase { + /** + * Test the functions quiz_update_open_attempts() and get_list_of_overdue_attempts() + */ + public function test_attempt_url() { + $attempt = mod_quiz_attempt_testable::setup_fake_attempt_layout( + 123, '1,2,0,3,4,0,5,6,0'); + + // Attempt pages. + $this->assertEquals(new moodle_url( + '/mod/quiz/attempt.php?attempt=123'), + $attempt->attempt_url()); + + $this->assertEquals(new moodle_url( + '/mod/quiz/attempt.php?attempt=123&page=2'), + $attempt->attempt_url(null, 2)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/attempt.php?attempt=123&page=1#'), + $attempt->attempt_url(3)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/attempt.php?attempt=123&page=1#q4'), + $attempt->attempt_url(4)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->attempt_url(null, 2, 2)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->attempt_url(3, -1, 1)); + + $this->assertEquals(new moodle_url( + '#q4'), + $attempt->attempt_url(4, -1, 1)); + + // Summary page. + $this->assertEquals(new moodle_url( + '/mod/quiz/summary.php?attempt=123'), + $attempt->summary_url()); + + // Review page. + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123'), + $attempt->review_url()); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&page=2'), + $attempt->review_url(null, 2)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&page=1'), + $attempt->review_url(3, -1, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&page=1#q4'), + $attempt->review_url(4, -1, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123'), + $attempt->review_url(null, 2, true)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123'), + $attempt->review_url(1, -1, true)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&page=2'), + $attempt->review_url(null, 2, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&showall=0'), + $attempt->review_url(null, 0, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&showall=0'), + $attempt->review_url(1, -1, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&page=1'), + $attempt->review_url(3, -1, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&page=2'), + $attempt->review_url(null, 2)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(null, -1, null, 0)); + + $this->assertEquals(new moodle_url( + '#q3'), + $attempt->review_url(3, -1, null, 0)); + + $this->assertEquals(new moodle_url( + '#q4'), + $attempt->review_url(4, -1, null, 0)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(null, 2, true, 0)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(1, -1, true, 0)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&page=2'), + $attempt->review_url(null, 2, false, 0)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(null, 0, false, 0)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(1, -1, false, 0)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=123&page=1#'), + $attempt->review_url(3, -1, false, 0)); + + // Review with more than 50 questions in the quiz. + $attempt = mod_quiz_attempt_testable::setup_fake_attempt_layout( + 124, '1,2,3,4,5,6,7,8,9,10,0,11,12,13,14,15,16,17,18,19,20,0,' . + '21,22,23,24,25,26,27,28,29,30,0,31,32,33,34,35,36,37,38,39,40,0,' . + '41,42,43,44,45,46,47,48,49,50,0,51,52,53,54,55,56,57,58,59,60,0'); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124'), + $attempt->review_url()); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=2'), + $attempt->review_url(null, 2)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=1'), + $attempt->review_url(11, -1, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=1#q12'), + $attempt->review_url(12, -1, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&showall=1'), + $attempt->review_url(null, 2, true)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&showall=1'), + $attempt->review_url(1, -1, true)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=2'), + $attempt->review_url(null, 2, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124'), + $attempt->review_url(null, 0, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=1'), + $attempt->review_url(11, -1, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=1#q12'), + $attempt->review_url(12, -1, false)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=2'), + $attempt->review_url(null, 2)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(null, -1, null, 0)); + + $this->assertEquals(new moodle_url( + '#q3'), + $attempt->review_url(3, -1, null, 0)); + + $this->assertEquals(new moodle_url( + '#q4'), + $attempt->review_url(4, -1, null, 0)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(null, 2, true, 0)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(1, -1, true, 0)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=2'), + $attempt->review_url(null, 2, false, 0)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(null, 0, false, 0)); + + $this->assertEquals(new moodle_url( + '#'), + $attempt->review_url(1, -1, false, 0)); + + $this->assertEquals(new moodle_url( + '/mod/quiz/review.php?attempt=124&page=1#'), + $attempt->review_url(11, -1, false, 0)); + } +}