MDL-44492 quiz reports: make show only graded attempt work.
authorTim Hunt <T.J.Hunt@open.ac.uk>
Tue, 1 Jul 2014 16:43:18 +0000 (17:43 +0100)
committerTim Hunt <T.J.Hunt@open.ac.uk>
Tue, 8 Jul 2014 12:31:49 +0000 (13:31 +0100)
It needs to only affect finished attempts, and the wording needs to be
modified to make it clear that is what it does.

mod/quiz/lang/en/quiz.php
mod/quiz/report/attemptsreport_form.php
mod/quiz/report/attemptsreport_options.php
mod/quiz/report/attemptsreport_table.php
mod/quiz/report/overview/lang/en/quiz_overview.php
mod/quiz/report/overview/overview_form.php
mod/quiz/report/overview/overview_options.php
mod/quiz/report/overview/report.php
mod/quiz/report/reportlib.php
mod/quiz/tests/reportlib_test.php

index 18cc1e6..2ebae5f 100644 (file)
@@ -709,6 +709,7 @@ $string['reportregrade'] = 'Regrade attempts';
 $string['reportresponses'] = 'Detailed responses';
 $string['reports'] = 'Reports';
 $string['reportshowonly'] = 'Show only attempts';
+$string['reportshowonlyfinished'] = 'Show at most one finished attempt per user ({$a})';
 $string['reportsimplestat'] = 'Simple statistics';
 $string['reportusersall'] = 'all users who have attempted the quiz';
 $string['reportuserswith'] = 'enrolled users who have attempted the quiz';
index c62a706..c468f6a 100644 (file)
@@ -89,8 +89,8 @@ abstract class mod_quiz_attempts_report_form extends moodleform {
             $gm = html_writer::tag('span',
                     quiz_get_grading_option_name($this->_customdata['quiz']->grademethod),
                     array('class' => 'highlight'));
-            $mform->addElement('advcheckbox', 'onlygraded', get_string('reportshowonly', 'quiz'),
-                    get_string('optonlygradedattempts', 'quiz_overview', $gm));
+            $mform->addElement('advcheckbox', 'onlygraded', '',
+                    get_string('reportshowonlyfinished', 'quiz', $gm));
             $mform->disabledIf('onlygraded', 'attempts', 'eq', quiz_attempts_report::ENROLLED_WITHOUT);
             $mform->disabledIf('onlygraded', 'statefinished', 'notchecked');
         }
index 06fd29c..839872f 100644 (file)
@@ -272,12 +272,20 @@ class mod_quiz_attempts_report_options {
             $this->onlygraded = false;
         }
 
-        if ($this->onlygraded) {
-            $this->states = array(quiz_attempt::FINISHED);
+        if (!$this->is_showing_finished_attempts()) {
+            $this->onlygraded = false;
         }
 
         if ($this->pagesize < 1) {
             $this->pagesize = quiz_attempts_report::DEFAULT_PAGE_SIZE;
         }
     }
+
+    /**
+     * Whether the options are such that finished attempts are being shown.
+     * @return boolean
+     */
+    protected function is_showing_finished_attempts() {
+        return $this->states === null || in_array(quiz_attempt::FINISHED, $this->states);
+    }
 }
index 61f3247..ab4ecb5 100644 (file)
@@ -425,7 +425,8 @@ abstract class quiz_attempts_report_table extends table_sql {
         $params = array('quizid' => $this->quiz->id);
 
         if ($this->qmsubselect && $this->options->onlygraded) {
-            $from .= " AND $this->qmsubselect";
+            $from .= " AND (quiza.state <> :finishedstate OR $this->qmsubselect)";
+            $params['finishedstate'] = quiz_attempt::FINISHED;
         }
 
         switch ($this->options->attempts) {
index e8bb6dc..dc188c2 100644 (file)
@@ -41,7 +41,6 @@ $string['optallattempts'] = 'all attempts';
 $string['optallstudents'] = 'all {$a} who have or have not attempted the quiz';
 $string['optattemptsonly'] = '{$a} who have attempted the quiz';
 $string['optnoattemptsonly'] = '{$a} who have not attempted the quiz';
-$string['optonlygradedattempts'] = 'that are graded for each user ({$a})';
 $string['optonlyregradedattempts'] = 'that have been regraded / are marked as needing regrading';
 $string['overview'] = 'Grades';
 $string['overviewdownload'] = 'Overview download';
index e03bab9..c3aed74 100644 (file)
@@ -38,7 +38,7 @@ class quiz_overview_settings_form extends mod_quiz_attempts_report_form {
 
     protected function other_attempt_fields(MoodleQuickForm $mform) {
         if (has_capability('mod/quiz:regrade', $this->_customdata['context'])) {
-            $mform->addElement('advcheckbox', 'onlyregraded', '',
+            $mform->addElement('advcheckbox', 'onlyregraded', get_string('reportshowonly', 'quiz'),
                     get_string('optonlyregradedattempts', 'quiz_overview'));
             $mform->disabledIf('onlyregraded', 'attempts', 'eq', quiz_attempts_report::ENROLLED_WITHOUT);
         }
index 57c1240..57c995a 100644 (file)
@@ -88,6 +88,10 @@ class quiz_overview_options extends mod_quiz_attempts_report_options {
     public function resolve_dependencies() {
         parent::resolve_dependencies();
 
+        if ($this->attempts == quiz_attempts_report::ENROLLED_WITHOUT) {
+            $this->onlyregraded = false;
+        }
+
         if (!$this->usercanseegrades) {
             $this->slotmarks = false;
         }
index d60da0b..5cb1145 100644 (file)
@@ -121,13 +121,6 @@ class quiz_overview_report extends quiz_attempts_report {
             // Construct the SQL.
             $fields = $DB->sql_concat('u.id', "'#'", 'COALESCE(quiza.attempt, 0)') .
                     ' AS uniqueid, ';
-            if ($this->qmsubselect) {
-                $fields .=
-                    "(CASE " .
-                    "   WHEN {$this->qmsubselect} THEN 1" .
-                    "   ELSE 0 " .
-                    "END) AS gradedattempt, ";
-            }
 
             list($fields, $from, $where, $params) = $table->base_sql($allowed);
 
index a998634..0bb3b5b 100644 (file)
@@ -156,27 +156,33 @@ function quiz_report_qm_filter_select($quiz, $quizattemptsalias = 'quiza') {
 function quiz_report_grade_method_sql($grademethod, $quizattemptsalias = 'quiza') {
     switch ($grademethod) {
         case QUIZ_GRADEHIGHEST :
-            return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2
+            return "($quizattemptsalias.state = 'finished' AND NOT EXISTS (
+                           SELECT 1 FROM {quiz_attempts} qa2
                             WHERE qa2.quiz = $quizattemptsalias.quiz AND
-                                qa2.userid = $quizattemptsalias.userid AND (
+                                qa2.userid = $quizattemptsalias.userid AND
+                                 qa2.state = 'finished' AND (
                 COALESCE(qa2.sumgrades, 0) > COALESCE($quizattemptsalias.sumgrades, 0) OR
                (COALESCE(qa2.sumgrades, 0) = COALESCE($quizattemptsalias.sumgrades, 0) AND qa2.attempt < $quizattemptsalias.attempt)
-                                ))";
+                                )))";
 
         case QUIZ_GRADEAVERAGE :
             return '';
 
         case QUIZ_ATTEMPTFIRST :
-            return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2
+            return "($quizattemptsalias.state = 'finished' AND NOT EXISTS (
+                           SELECT 1 FROM {quiz_attempts} qa2
                             WHERE qa2.quiz = $quizattemptsalias.quiz AND
                                 qa2.userid = $quizattemptsalias.userid AND
-                               qa2.attempt < $quizattemptsalias.attempt)";
+                                 qa2.state = 'finished' AND
+                               qa2.attempt < $quizattemptsalias.attempt))";
 
         case QUIZ_ATTEMPTLAST :
-            return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2
+            return "($quizattemptsalias.state = 'finished' AND NOT EXISTS (
+                           SELECT 1 FROM {quiz_attempts} qa2
                             WHERE qa2.quiz = $quizattemptsalias.quiz AND
                                 qa2.userid = $quizattemptsalias.userid AND
-                               qa2.attempt > $quizattemptsalias.attempt)";
+                                 qa2.state = 'finished' AND
+                               qa2.attempt > $quizattemptsalias.attempt))";
     }
 }
 
index de4b6a6..9986d30 100644 (file)
@@ -95,16 +95,17 @@ class mod_quiz_reportlib_testcase extends advanced_testcase {
         $fakeattempt->userid = 123;
         $fakeattempt->quiz = 456;
         $fakeattempt->layout = '1,2,0,3,4,0,5';
+        $fakeattempt->state = quiz_attempt::FINISHED;
 
         // We intentionally insert these in a funny order, to test the SQL better.
         // The test data is:
-        // id | quizid | user | attempt | sumgrades
-        // ----------------------------------------
-        // 4  | 456    | 123  | 1       | 30
-        // 2  | 456    | 123  | 2       | 50
-        // 1  | 456    | 123  | 3       | 50
-        // 3  | 456    | 123  | 4       | null
-        // 5  | 456    | 1    | 1       | 100
+        // id | quizid | user | attempt | sumgrades | state
+        // ---------------------------------------------------
+        // 4  | 456    | 123  | 1       | 30        | finished
+        // 2  | 456    | 123  | 2       | 50        | finished
+        // 1  | 456    | 123  | 3       | 50        | finished
+        // 3  | 456    | 123  | 4       | null      | inprogress
+        // 5  | 456    | 1    | 1       | 100       | finished
         // layout is only given because it has a not-null constraint.
         // uniqueid values are meaningless, but that column has a unique constraint.
 
@@ -121,11 +122,13 @@ class mod_quiz_reportlib_testcase extends advanced_testcase {
         $fakeattempt->attempt = 4;
         $fakeattempt->sumgrades = null;
         $fakeattempt->uniqueid = 39;
+        $fakeattempt->state = quiz_attempt::IN_PROGRESS;
         $DB->insert_record('quiz_attempts', $fakeattempt);
 
         $fakeattempt->attempt = 1;
         $fakeattempt->sumgrades = 30;
         $fakeattempt->uniqueid = 52;
+        $fakeattempt->state = quiz_attempt::FINISHED;
         $DB->insert_record('quiz_attempts', $fakeattempt);
 
         $fakeattempt->attempt = 1;
@@ -151,7 +154,7 @@ class mod_quiz_reportlib_testcase extends advanced_testcase {
                 . quiz_report_qm_filter_select($quiz), array(123, 456));
         $this->assertEquals(1, count($lastattempt));
         $lastattempt = reset($lastattempt);
-        $this->assertEquals(4, $lastattempt->attempt);
+        $this->assertEquals(3, $lastattempt->attempt);
 
         $quiz->attempts = 0;
         $quiz->grademethod = QUIZ_GRADEHIGHEST;