From a6866c7e9ff4c2b750df49ac4ec051687aaab8cc Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Thu, 7 Jun 2012 19:17:07 +0100 Subject: [PATCH] MDL-32783 quiz overdue handling: enforce end of grace period The summary page was not enforcing the end of the grace period. If the user had the summary page open, then they coudl still stubmit after the end of the grace period. Also, the editing form was not validating that the quiz grace period was greater than quiz|graceperiodmin in the quiz configuration, and that should be checked, so I added it. I fear that processattempt.php is becoming very spaghetti-like with all the timing rules, it really needs to be refactored, but not 2 weeks before the 2.3 release. (When refactoring, we really need unit tests for this.) --- mod/quiz/lang/en/quiz.php | 1 + mod/quiz/mod_form.php | 8 ++++++++ mod/quiz/processattempt.php | 24 ++++++++++++++++++++---- mod/quiz/summary.php | 13 ++++++++----- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/mod/quiz/lang/en/quiz.php b/mod/quiz/lang/en/quiz.php index dd4645fa33d..67587d07513 100644 --- a/mod/quiz/lang/en/quiz.php +++ b/mod/quiz/lang/en/quiz.php @@ -335,6 +335,7 @@ $string['graceperiod_desc'] = 'If what to do when time expires is set to \'Allow $string['graceperiod_help'] = 'If what to do when time expires is set to \'Allow a grace period to submit, but not change any responses\', the amount of extra time that is allowed.'; $string['graceperiodmin'] = 'Last submission grace period'; $string['graceperiodmin_desc'] = 'There is a potential problem right at the end of the quiz. On the one hand, we want to let students continue working right up until the last second - with the help of the timer that automatically submits the quiz when time runs out. On the other hand, the server may then be overloaded, and take some time to get to process the responses. Therefore, we will accept responses for up to this many seconds after time expires, so they are not penalised for the server being slow. However, the student could cheat and get this many seconds to answer the quiz. You have to make a trade-off based on how much you trust the performance of your server during quizzes.'; +$string['graceperiodtoosmall'] = 'The grace period must be at more than {$a}.'; $string['grade'] = 'Grade'; $string['gradeall'] = 'Grade all'; $string['gradeaverage'] = 'Average grade'; diff --git a/mod/quiz/mod_form.php b/mod/quiz/mod_form.php index 94336a65852..0b101ece09d 100644 --- a/mod/quiz/mod_form.php +++ b/mod/quiz/mod_form.php @@ -502,6 +502,14 @@ class mod_quiz_mod_form extends moodleform_mod { $errors['timeclose'] = get_string('closebeforeopen', 'quiz'); } + // Check that the grace period is not too short. + if ($data['overduehandling'] == 'graceperiod') { + $graceperiodmin = get_config('quiz', 'graceperiodmin'); + if ($data['graceperiod'] <= $graceperiodmin) { + $errors['graceperiod'] = get_string('graceperiodtoosmall', 'quiz', format_time($graceperiodmin)); + } + } + // Check the boundary value is a number or a percentage, and in range. $i = 0; while (!empty($data['feedbackboundaries'][$i] )) { diff --git a/mod/quiz/processattempt.php b/mod/quiz/processattempt.php index 6d1fc60c4b9..f355eb90ad4 100644 --- a/mod/quiz/processattempt.php +++ b/mod/quiz/processattempt.php @@ -65,12 +65,14 @@ if ($page == -1) { // If there is only a very small amount of time left, there is no point trying // to show the student another page of the quiz. Just finish now. +$graceperiodmin = null; $accessmanager = $attemptobj->get_access_manager($timenow); $timeleft = $accessmanager->get_time_left($attemptobj->get_attempt(), $timenow); $toolate = false; if ($timeleft !== false && $timeleft < QUIZ_MIN_TIME_TO_CONTINUE) { $timeup = true; - if ($timeleft < -get_config('quiz', 'graceperiodmin')) { + $graceperiodmin = get_config('quiz', 'graceperiodmin'); + if ($timeleft < -$graceperiodmin) { $toolate = true; } } @@ -97,9 +99,19 @@ if ($attemptobj->is_finished()) { // If time is running out, trigger the appropriate action. $becomingoverdue = false; +$becomingabandoned = false; if ($timeup) { - if ($attemptobj->get_quiz()->overduehandling == 'graceperiod') { - $becomingoverdue = true; + if ($attemptobj->get_quiz()->overduehandling == 'graceperiod') {# + if (is_null($graceperiodmin)) { + $graceperiodmin = get_config('quiz', 'graceperiodmin'); + } + if ($timeleft < -$attemptobj->get_quiz()->graceperiod - $graceperiodmin) { + // Grace period has run out. + $finishattempt = true; + $becomingabandoned = true; + } else { + $becomingoverdue = true; + } } else { $finishattempt = true; } @@ -150,7 +162,11 @@ add_to_log($attemptobj->get_courseid(), 'quiz', 'close attempt', // Update the quiz attempt record. try { - $attemptobj->process_finish($timenow, !$toolate); + if ($becomingabandoned) { + $attemptobj->process_abandon($timenow, true); + } else { + $attemptobj->process_finish($timenow, !$toolate); + } } catch (question_out_of_sequence_exception $e) { print_error('submissionoutofsequencefriendlymessage', 'question', diff --git a/mod/quiz/summary.php b/mod/quiz/summary.php index 0b1cdb0e762..303638f013c 100644 --- a/mod/quiz/summary.php +++ b/mod/quiz/summary.php @@ -46,11 +46,6 @@ if (!$attemptobj->is_preview_user()) { $attemptobj->require_capability('mod/quiz:attempt'); } -// If the attempt is already closed, redirect them to the review page. -if ($attemptobj->is_finished()) { - redirect($attemptobj->review_url()); -} - if ($attemptobj->is_preview_user()) { navigation_node::override_active_url($attemptobj->start_attempt_url()); } @@ -69,6 +64,14 @@ if ($accessmanager->is_preflight_check_required($attemptobj->get_attemptid())) { $displayoptions = $attemptobj->get_display_options(false); +// If the attempt is now overdue, or abandoned, deal with that. +$attemptobj->handle_if_time_expired(time(), true); + +// If the attempt is already closed, redirect them to the review page. +if ($attemptobj->is_finished()) { + redirect($attemptobj->review_url()); +} + // Log this page view. add_to_log($attemptobj->get_courseid(), 'quiz', 'view summary', 'summary.php?attempt=' . $attemptobj->get_attemptid(),