diff --git a/mod/quiz/classes/access_manager.php b/mod/quiz/classes/access_manager.php index 577380e0869..b7f29824eee 100644 --- a/mod/quiz/classes/access_manager.php +++ b/mod/quiz/classes/access_manager.php @@ -556,7 +556,7 @@ class access_manager { $this->quizobj->get_quiz(), $when); if (!$reviewoptions->attempt) { - return $output->no_review_message($this->quizobj->cannot_review_message($when, true)); + return $output->no_review_message($this->quizobj->cannot_review_message($when, true, $attempt->timefinish)); } else { return $output->review_link($this->quizobj->review_url($attempt->id), diff --git a/mod/quiz/classes/quiz_attempt.php b/mod/quiz/classes/quiz_attempt.php index 01496d5db3c..eab5e36fbc8 100644 --- a/mod/quiz/classes/quiz_attempt.php +++ b/mod/quiz/classes/quiz_attempt.php @@ -61,6 +61,9 @@ class quiz_attempt { /** @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; + /** @var int amount of time considered 'immedately after the attempt', in seconds. */ + const IMMEDIATELY_AFTER_PERIOD = 2 * MINSECS; + /** @var quiz_settings object containing the quiz settings. */ protected $quizobj; @@ -1226,7 +1229,7 @@ class quiz_attempt { */ public function cannot_review_message($short = false) { return $this->quizobj->cannot_review_message( - $this->get_attempt_state(), $short); + $this->get_attempt_state(), $short, $this->attempt->timefinish); } /** diff --git a/mod/quiz/classes/quiz_settings.php b/mod/quiz/classes/quiz_settings.php index 4aaa1130388..48ab0d6575a 100644 --- a/mod/quiz/classes/quiz_settings.php +++ b/mod/quiz/classes/quiz_settings.php @@ -497,9 +497,15 @@ class quiz_settings { * @param int $when One of the display_options::DURING, * IMMEDIATELY_AFTER, LATER_WHILE_OPEN or AFTER_CLOSE constants. * @param bool $short if true, return a shorter string. + * @param int|null $attemptsubmittime time this attempt was submitted. (Optional, but should be given.) * @return string an appropraite message. */ - public function cannot_review_message($when, $short = false) { + public function cannot_review_message($when, $short = false, int $attemptsubmittime = null) { + + if ($attemptsubmittime === null) { + debugging('It is recommended that you pass $attemptsubmittime to cannot_review_message', DEBUG_DEVELOPER); + $attemptsubmittime = time(); // This will be approximately right, which is enough for the one place were it is used. + } if ($short) { $langstrsuffix = 'short'; @@ -509,17 +515,30 @@ class quiz_settings { $dateformat = ''; } - if ($when == display_options::DURING || - $when == display_options::IMMEDIATELY_AFTER) { - return ''; + $reviewfrom = 0; + switch ($when) { + case display_options::DURING: + return ''; + + case display_options::IMMEDIATELY_AFTER: + if ($this->quiz->reviewattempt & display_options::LATER_WHILE_OPEN) { + $reviewfrom = $attemptsubmittime + quiz_attempt::IMMEDIATELY_AFTER_PERIOD; + break; + } + // Fall through. + + case display_options::LATER_WHILE_OPEN: + if ($this->quiz->timeclose && ($this->quiz->reviewattempt & display_options::AFTER_CLOSE)) { + $reviewfrom = $this->quiz->timeclose; + break; + } + } + + if ($reviewfrom) { + return get_string('noreviewuntil' . $langstrsuffix, 'quiz', + userdate($reviewfrom, $dateformat)); } else { - if ($when == display_options::LATER_WHILE_OPEN && $this->quiz->timeclose && - $this->quiz->reviewattempt & display_options::AFTER_CLOSE) { - return get_string('noreviewuntil' . $langstrsuffix, 'quiz', - userdate($this->quiz->timeclose, $dateformat)); - } else { - return get_string('noreview' . $langstrsuffix, 'quiz'); - } + return get_string('noreview' . $langstrsuffix, 'quiz'); } } diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index eeb4587a10b..6bbed0659d4 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -1125,7 +1125,7 @@ function quiz_attempt_state($quiz, $attempt) { return display_options::DURING; } else if ($quiz->timeclose && time() >= $quiz->timeclose) { return display_options::AFTER_CLOSE; - } else if (time() < $attempt->timefinish + 120) { + } else if (time() < $attempt->timefinish + quiz_attempt::IMMEDIATELY_AFTER_PERIOD) { return display_options::IMMEDIATELY_AFTER; } else { return display_options::LATER_WHILE_OPEN; diff --git a/mod/quiz/tests/quizobj_test.php b/mod/quiz/tests/quizobj_test.php index ce6e4fb2f21..d9934a94713 100644 --- a/mod/quiz/tests/quizobj_test.php +++ b/mod/quiz/tests/quizobj_test.php @@ -16,8 +16,10 @@ namespace mod_quiz; +use basic_testcase; use mod_quiz\question\display_options; use mod_quiz\quiz_settings; +use stdClass; defined('MOODLE_INTERNAL') || die(); @@ -30,33 +32,99 @@ require_once($CFG->dirroot . '/mod/quiz/locallib.php'); * @package mod_quiz * @copyright 2008 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \mod_quiz\quiz_settings */ -class quizobj_test extends \basic_testcase { - public function test_cannot_review_message() { - $quiz = new \stdClass(); - $quiz->reviewattempt = 0x10010; - $quiz->timeclose = 0; - $quiz->attempts = 0; +class quizobj_test extends basic_testcase { + /** + * Test cases for {@see test_cannot_review_message()}. + * + * @return array[] + */ + public static function cannot_review_message_testcases(): array { + return [ + // Review Time close + // Later close quiz attempt When Expected + // Quiz with no close date. + [false, false, null, null, display_options::DURING, ''], + [false, false, null, -60, display_options::IMMEDIATELY_AFTER, 'noreview'], + [false, false, null, -180, display_options::LATER_WHILE_OPEN, 'noreview'], + [false, false, null, -180, display_options::AFTER_CLOSE, 'noreview'], + [false, true, null, null, display_options::DURING, ''], + [false, true, null, -60, display_options::IMMEDIATELY_AFTER, 'noreview'], + [false, true, null, -180, display_options::LATER_WHILE_OPEN, 'noreview'], + [false, true, null, -180, display_options::AFTER_CLOSE, 'noreview'], + // Quiz with a close in the future date, review only after close. + [false, true, 300, null, display_options::DURING, ''], + [false, true, 300, -60, display_options::IMMEDIATELY_AFTER, 300], + [false, true, 300, -180, display_options::LATER_WHILE_OPEN, 300], + // Quiz with a close in the future date, review later while open, or after close. + [true, true, 300, null, display_options::DURING, ''], + [true, true, 300, -60, display_options::IMMEDIATELY_AFTER, 60], + [true, false, 300, -60, display_options::IMMEDIATELY_AFTER, 60], + // Quiz with no closer date, review later while open. + [true, false, 300, null, display_options::DURING, ''], + [true, false, 300, -60, display_options::IMMEDIATELY_AFTER, 60], + ]; + } - $cm = new \stdClass(); + /** + * Unit test for {@see quiz_settings::cannot_review_message()}. + * + * @dataProvider cannot_review_message_testcases + * @param bool $reviewlater whether the quiz allows reivew 'later while the quiz is still open'. + * @param bool $reviewafterclose whether the quiz allows rievew 'after the quiz is closed'. + * @param int|null $quizcloseoffset quiz close date, relative to now. Null means not set. + * @param int|null $attemptsubmitoffset quiz attempt sumbite time relative to now. Null means not submitted yet. + * @param int $attemptstate current state of the attempt, one of the display_options constants. + * @param string|int $expectation expected result: '' means '', 'noreview' means noreview lang string, + * int means noreviewuntil with that time relative to now. + */ + public function test_cannot_review_message( + bool $reviewlater, + bool $reviewafterclose, + ?int $quizcloseoffset, + ?int $attemptsubmitoffset, + int $attemptstate, + string|int $expectation + ): void { + $quiz = new stdClass(); + $now = time(); + + $cm = new stdClass(); $cm->id = 123; - $quizobj = new quiz_settings($quiz, $cm, new \stdClass(), false); + // Prepare quiz settings. + $quiz->reviewattempt = display_options::DURING; + if ($reviewlater) { + $quiz->reviewattempt |= display_options::LATER_WHILE_OPEN; + } + if ($reviewafterclose) { + $quiz->reviewattempt |= display_options::AFTER_CLOSE; + } + $quiz->attempts = 0; - $this->assertEquals('', - $quizobj->cannot_review_message(display_options::DURING)); - $this->assertEquals('', - $quizobj->cannot_review_message(display_options::IMMEDIATELY_AFTER)); - $this->assertEquals(get_string('noreview', 'quiz'), - $quizobj->cannot_review_message(display_options::LATER_WHILE_OPEN)); - $this->assertEquals(get_string('noreview', 'quiz'), - $quizobj->cannot_review_message(display_options::AFTER_CLOSE)); + if ($quizcloseoffset === null) { + $quiz->timeclose = 0; + } else { + $quiz->timeclose = $now + $quizcloseoffset; + } + if ($attemptsubmitoffset === null) { + $submittime = 0; + } else { + $submittime = $now + $attemptsubmitoffset; + } - $closetime = time() + 10000; - $quiz->timeclose = $closetime; - $quizobj = new quiz_settings($quiz, $cm, new \stdClass(), false); + $quizobj = new quiz_settings($quiz, $cm, new stdClass(), false); - $this->assertEquals(get_string('noreviewuntil', 'quiz', userdate($closetime)), - $quizobj->cannot_review_message(display_options::LATER_WHILE_OPEN)); + // Prepare expected message. + if ($expectation === 'noreview') { + $expectation = get_string('noreview', 'quiz'); + } else if (is_int($expectation)) { + $expectation = get_string('noreviewuntil', 'quiz', userdate($now + $expectation)); + } + + // Test. + $this->assertEquals($expectation, + $quizobj->cannot_review_message($attemptstate, false, $submittime)); } } diff --git a/mod/quiz/upgrade.txt b/mod/quiz/upgrade.txt index ba58d8552ec..00504f104e0 100644 --- a/mod/quiz/upgrade.txt +++ b/mod/quiz/upgrade.txt @@ -4,6 +4,8 @@ This files describes API changes in the quiz code. * A quiz_structure_modified callback has been added for quiz_ plugins, called from grade_calculator::recompute_quiz_sumgrades(). Plugins can implement this by creating a `quiz_structure_modified` class in their namespace with a static `callback` method, see quiz_statistics as an example. +* quiz_settings::no_review_message now takes a new argument $attemptsubmittime for the time when the quiz attempt was + submitted. It is strongly recommended that you always pass that. === 4.3 ===