From 097dbfe11a4598031d4f87b3fd5208e61a7ca82f Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Sun, 22 Jun 2014 11:03:03 +0100 Subject: [PATCH] MDL-46093 quiz review should default to showing everything on one page ... as long as the quiz is not too big. At the moment, that limit is set to 50 questions. --- mod/quiz/attemptlib.php | 56 +++++-- mod/quiz/review.php | 11 +- mod/quiz/tests/attempt_test.php | 289 ++++++++++++++++++++++++++++++++ 3 files changed, 337 insertions(+), 19 deletions(-) create mode 100644 mod/quiz/tests/attempt_test.php 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)); + } +}