MDL-3030 quiz overdue handling: test state, not timefinish where applicable.

mod/quiz/accessmanager.php

@@ -462,8 +462,8 @@ class quiz_access_manager {
      */
     public function make_review_link($attempt, $reviewoptions, $output) {
 
-        // If review of responses is not allowed, or the attempt is still open, don't link.
+        // If the attempt is still open, don't link.
-        if (!$attempt->timefinish) {
+        if (in_array($attempt->state, array(quiz_attempt::IN_PROGRESS, quiz_attempt::OVERDUE))) {
             return $output->no_review_message('');
         }
 

mod/quiz/attemptlib.php

@@ -513,16 +513,7 @@ class quiz_attempt {
      * @return string the human-readable state name.
      */
     public static function state_name($state) {
-        switch ($state) {
+        return quiz_attempt_state_name($state);
-            case quiz_attempt::IN_PROGRESS:
-                return get_string('stateinprogress', 'quiz');
-            case quiz_attempt::OVERDUE:
-                return get_string('stateoverdue', 'quiz');
-            case quiz_attempt::FINISHED:
-                return get_string('statefinished', 'quiz');
-            case quiz_attempt::ABANDONED:
-                return get_string('stateabandoned', 'quiz');
-        }
     }
 
     private function determine_layout() {
@@ -1321,7 +1312,7 @@ class quiz_attempt {
             $DB->update_record('quiz_attempts', $this->attempt);
         }
 
-        if (!$this->is_preview() && $this->attempt->timefinish) {
+        if (!$this->is_preview() && $this->attempt->state == quiz_attempt::FINISHED) {
             quiz_save_best_grade($this->get_quiz(), $this->get_userid());
         }
 
@@ -1435,7 +1426,8 @@ class quiz_attempt {
         }
 
         if ($event == 'quiz_attempt_submitted') {
-            $eventdata->timefinish = $timestamp; // Backwards compatibility.
+            // Backwards compatibility for this event type. $eventdata->timestamp is now preferred.
+            $eventdata->timefinish = $timestamp;
         }
 
         events_trigger($event, $eventdata);

mod/quiz/lib.php

@@ -402,7 +402,6 @@ function quiz_user_outline($course, $user, $mod, $quiz) {
  * Print a detailed representation of what a  user has done with
  * a given particular instance of this module, for user activity reports.
  *
- * @global object
  * @param object $course
  * @param object $user
  * @param object $mod
@@ -411,7 +410,8 @@ function quiz_user_outline($course, $user, $mod, $quiz) {
  */
 function quiz_user_complete($course, $user, $mod, $quiz) {
     global $DB, $CFG, $OUTPUT;
-    require_once("$CFG->libdir/gradelib.php");
+    require_once($CFG->libdir . '/gradelib.php');
+    require_once($CFG->libdir . '/mod/quiz/locallib.php');
 
     $grades = grade_get_grades($course->id, 'mod', 'quiz', $quiz->id, $user->id);
     if (!empty($grades->items[0]->grades)) {
@@ -426,8 +426,8 @@ function quiz_user_complete($course, $user, $mod, $quiz) {
             array('userid' => $user->id, 'quiz' => $quiz->id), 'attempt')) {
         foreach ($attempts as $attempt) {
             echo get_string('attempt', 'quiz').' '.$attempt->attempt.': ';
-            if ($attempt->timefinish == 0) {
+            if ($attempt->state != quiz_attempt::FINISHED) {
-                print_string('unfinished');
+                echo quiz_attempt_state_name($attempt->state);
             } else {
                 echo quiz_format_grade($quiz, $attempt->sumgrades) . '/' .
                         quiz_format_grade($quiz, $quiz->sumgrades);
@@ -445,6 +445,7 @@ function quiz_user_complete($course, $user, $mod, $quiz) {
  * Quiz periodic clean-up tasks.
  */
 function quiz_cron() {
+    global $CFG;
 
     // Run cron for our sub-plugin types.
     cron_execute_plugin_type('quiz', 'quiz reports');

mod/quiz/locallib.php

@@ -105,6 +105,7 @@ function quiz_create_attempt($quiz, $attemptnumber, $lastattempt, $timenow, $isp
     $attempt->timestart = $timenow;
     $attempt->timefinish = 0;
     $attempt->timemodified = $timenow;
+    $attempt->state = quiz_attempt::IN_PROGRESS;
 
     // If this is a preview, mark it as such.
     if ($ispreview) {
@@ -445,8 +446,9 @@ function quiz_update_all_attempt_sumgrades($quiz) {
                 sumgrades = (
                     {$dm->sum_usage_marks_subquery('uniqueid')}
                 )
-            WHERE quiz = :quizid AND timefinish <> 0";
+            WHERE quiz = :quizid AND state = :finishedstate";
-    $DB->execute($sql, array('timenow' => $timenow, 'quizid' => $quiz->id));
+    $DB->execute($sql, array('timenow' => $timenow, 'quizid' => $quiz->id,
+            'finishedstate' => quiz_attempt::FINISHED));
 }
 
 /**
@@ -613,7 +615,7 @@ function quiz_update_all_final_grades($quiz) {
         return;
     }
 
-    $param = array('iquizid' => $quiz->id);
+    $param = array('iquizid' => $quiz->id, 'istatefinished' => quiz_attempt::FINISHED);
     $firstlastattemptjoin = "JOIN (
             SELECT
                 iquiza.userid,
@@ -623,7 +625,7 @@ function quiz_update_all_final_grades($quiz) {
             FROM {quiz_attempts} iquiza
 
             WHERE
-                iquiza.timefinish <> 0 AND
+                iquiza.state = :istatefinished
                 iquiza.preview = 0 AND
                 iquiza.quiz = :iquizid
 
@@ -670,13 +672,15 @@ function quiz_update_all_final_grades($quiz) {
     $param['quizid2'] = $quiz->id;
     $param['quizid3'] = $quiz->id;
     $param['quizid4'] = $quiz->id;
+    $param['statefinished'] = quiz_attempt::FINISHED;
+    $param['statefinished2'] = quiz_attempt::FINISHED;
     $finalgradesubquery = "
             SELECT quiza.userid, $finalgrade AS newgrade
             FROM {quiz_attempts} quiza
             $join
             WHERE
                 $where
-                quiza.timefinish <> 0 AND
+                quiza.quiza.state = :statefinished AND
                 quiza.preview = 0 AND
                 quiza.quiz = :quizid3
             GROUP BY quiza.userid";
@@ -692,7 +696,7 @@ function quiz_update_all_final_grades($quiz) {
                 SELECT DISTINCT userid
                 FROM {quiz_attempts} quiza2
                 WHERE
-                    quiza2.timefinish <> 0 AND
+                    quiza2.quiza.state = :statefinished2 AND
                     quiza2.preview = 0 AND
                     quiza2.quiz = :quizid2
             ) users
@@ -818,6 +822,25 @@ function quiz_get_overdue_handling_options() {
     );
 }
 
+/**
+ * @param string $state one of the state constants like IN_PROGRESS.
+ * @return string the human-readable state name.
+ */
+function quiz_attempt_state_name($state) {
+    switch ($state) {
+        case quiz_attempt::IN_PROGRESS:
+            return get_string('stateinprogress', 'quiz');
+        case quiz_attempt::OVERDUE:
+            return get_string('stateoverdue', 'quiz');
+        case quiz_attempt::FINISHED:
+            return get_string('statefinished', 'quiz');
+        case quiz_attempt::ABANDONED:
+            return get_string('stateabandoned', 'quiz');
+        default:
+            throw new coding_exception('Unknown quiz attempt state.');
+    }
+}
+
 /// Other quiz functions ////////////////////////////////////////////////////
 
 /**
@@ -950,14 +973,15 @@ function quiz_get_flag_option($attempt, $context) {
 }
 
 /**
- * Work out what state this quiz attempt is in.
+ * Work out what state this quiz attempt is in - in the sense used by
+ * quiz_get_review_options, not in the sense of $attempt->state.
  * @param object $quiz the quiz settings
  * @param object $attempt the quiz_attempt database row.
  * @return int one of the mod_quiz_display_options::DURING,
  *      IMMEDIATELY_AFTER, LATER_WHILE_OPEN or AFTER_CLOSE constants.
  */
 function quiz_attempt_state($quiz, $attempt) {
-    if ($attempt->timefinish == 0) {
+    if ($attempt->state != quiz_attempt::FINISHED) {
         return mod_quiz_display_options::DURING;
     } else if (time() < $attempt->timefinish + 120) {
         return mod_quiz_display_options::IMMEDIATELY_AFTER;
@@ -989,7 +1013,7 @@ function quiz_get_review_options($quiz, $attempt, $context) {
     }
 
     // Show a link to the comment box only for closed attempts
-    if (!empty($attempt->id) && $attempt->timefinish && !$attempt->preview &&
+    if (!empty($attempt->id) && $attempt->state == quiz_attempt::FINISHED && !$attempt->preview &&
             !is_null($context) && has_capability('mod/quiz:grade', $context)) {
         $options->manualcomment = question_display_options::VISIBLE;
         $options->manualcommentlink = new moodle_url('/mod/quiz/comment.php',
@@ -1517,14 +1541,17 @@ class mod_quiz_display_options extends question_display_options {
 class qubaids_for_quiz extends qubaid_join {
     public function __construct($quizid, $includepreviews = true, $onlyfinished = false) {
         $where = 'quiza.quiz = :quizaquiz';
+        $params = array('quizaquiz' => $quizid);
+
         if (!$includepreviews) {
             $where .= ' AND preview = 0';
         }
+
         if ($onlyfinished) {
-            $where .= ' AND timefinish <> 0';
+            $where .= ' AND state == :statefinished';
+            $params['statefinished'] = quiz_attempt::FINISHED;
         }
 
-        parent::__construct('{quiz_attempts} quiza', 'quiza.uniqueid', $where,
+        parent::__construct('{quiz_attempts} quiza', 'quiza.uniqueid', $where, $params);
-                array('quizaquiz' => $quizid));
     }
 }

mod/quiz/report/attemptsreport_table.php

@@ -202,7 +202,7 @@ abstract class quiz_attempts_report_table extends table_sql {
      * @return string HTML content to go inside the td.
      */
     public function col_feedbacktext($attempt) {
-        if (!$attempt->timefinish) {
+        if ($attempt->state != quiz_attempt::FINISHED) {
             return '-';
         }
 
@@ -371,10 +371,7 @@ abstract class quiz_attempts_report_table extends table_sql {
                 CASE WHEN quiza.timefinish = 0 THEN null
                      WHEN quiza.timefinish > quiza.timestart THEN quiza.timefinish - quiza.timestart
                      ELSE 0 END AS duration';
-            // To explain that last bit, in MySQL, qa.timestart and qa.timefinish
+            // To explain that last bit, timefinish can be non-zero and less
-            // are unsigned. Since MySQL 5.5.5, when they introduced strict mode,
-            // subtracting a larger unsigned int from a smaller one gave an error.
-            // Therefore, we avoid doing that. timefinish can be non-zero and less
             // than timestart when you have two load-balanced servers with very
             // badly synchronised clocks, and a student does a really quick attempt.
 

mod/quiz/report/grading/report.php

@@ -162,8 +162,8 @@ class quiz_grading_report extends quiz_default_report {
 
         $where = "quiza.quiz = :mangrquizid AND
                 quiza.preview = 0 AND
-                quiza.timefinish <> 0";
+                quiza.state = :statefinished";
-        $params = array('mangrquizid' => $this->cm->instance);
+        $params = array('mangrquizid' => $this->cm->instance, 'statefinished' => quiz_attempt::FINISHED);
 
         $currentgroup = groups_get_activity_group($this->cm, true);
         if ($currentgroup) {
@@ -187,13 +187,14 @@ class quiz_grading_report extends quiz_default_report {
         global $DB;
 
         list($asql, $params) = $DB->get_in_or_equal($qubaids);
+        $params[] = quiz_attempt::FINISHED;
         $params[] = $this->quiz->id;
 
         $attemptsbyid = $DB->get_records_sql("
                 SELECT quiza.*, u.firstname, u.lastname, u.idnumber
                 FROM {quiz_attempts} quiza
                 JOIN {user} u ON u.id = quiza.userid
-                WHERE quiza.uniqueid $asql AND quiza.timefinish <> 0 AND quiza.quiz = ?",
+                WHERE quiza.uniqueid $asql AND quiza.state == ? AND quiza.quiz = ?",
                 $params);
 
         $attempts = array();

mod/quiz/report/overview/overview_table.php

@@ -180,7 +180,7 @@ class quiz_overview_table extends quiz_attempts_report_table {
     }
 
     public function col_sumgrades($attempt) {
-        if (!$attempt->timefinish) {
+        if ($attempt->state != quiz_attempt::FINISHED) {
             return '-';
         }
 

mod/quiz/report/overview/report.php

@@ -321,7 +321,7 @@ class quiz_overview_report extends quiz_attempts_report {
             $slots = $quba->get_slots();
         }
 
-        $finished = $attempt->timefinish > 0;
+        $finished = $attempt->state == quiz_attempt::FINISHED;
         foreach ($slots as $slot) {
             $qqr = new stdClass();
             $qqr->oldfraction = $quba->get_question_fraction($slot);

mod/quiz/report/responses/responses_table.php

@@ -63,7 +63,7 @@ class quiz_responses_table extends quiz_attempts_report_table {
     }
 
     public function col_sumgrades($attempt) {
-        if (!$attempt->timefinish) {
+        if ($attempt->state == quiz_attempt::FINISHED) {
             return '-';
         }
 

mod/quiz/review.php

@@ -184,7 +184,7 @@ if (!empty($overtime)) {
 $grade = quiz_rescale_grade($attempt->sumgrades, $quiz, false);
 if ($options->marks >= question_display_options::MARK_AND_MAX && quiz_has_grades($quiz)) {
 
-    if (!$attempt->timefinish) {
+    if ($attempt->state != quiz_attempt::FINISHED) {
         $summarydata['grade'] = array(
             'title'   => get_string('grade', 'quiz'),
             'content' => get_string('attemptstillinprogress', 'quiz'),