MDL-35453 quiz reports: don't order by id.

We were using ORDER BY id even though there was a perfectly good
attempt column to order the attempts by one user at one quiz.

Also, Oracle was complaining:
Debug info: ORA-01799: a column may not be outer-joined to a subquery

mod/quiz/report/overview/report.php

@@ -526,10 +526,11 @@ class quiz_overview_report extends quiz_attempts_report {
      */
     protected function has_regraded_questions($from, $where, $params) {
         global $DB;
-        $qubaids = new qubaid_join($from, 'uniqueid', $where, $params);
+        return $DB->record_exists_sql("
-        return $DB->record_exists_select('quiz_overview_regrades',
+                SELECT 1
-                'questionusageid ' . $qubaids->usage_id_in(),
+                  FROM {$from}
-                $qubaids->usage_id_in_params());
+                  JOIN {quiz_overview_regrades} qor ON qor.questionusageid = quiza.uniqueid
+                 WHERE {$where}", $params);
     }
 
     /**

mod/quiz/report/reportlib.php

@@ -156,35 +156,27 @@ function quiz_report_qm_filter_select($quiz, $quizattemptsalias = 'quiza') {
 function quiz_report_grade_method_sql($grademethod, $quizattemptsalias = 'quiza') {
     switch ($grademethod) {
         case QUIZ_GRADEHIGHEST :
-            return "$quizattemptsalias.id = (
+            return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2
-                            SELECT MIN(qa2.id)
-                            FROM {quiz_attempts} qa2
                             WHERE qa2.quiz = $quizattemptsalias.quiz AND
-                                qa2.userid = $quizattemptsalias.userid AND
+                                qa2.userid = $quizattemptsalias.userid AND (
-                                COALESCE(qa2.sumgrades, 0) = (
+                COALESCE(qa2.sumgrades, 0) > COALESCE($quizattemptsalias.sumgrades, 0) OR
-                                    SELECT MAX(COALESCE(qa3.sumgrades, 0))
+               (COALESCE(qa2.sumgrades, 0) = COALESCE($quizattemptsalias.sumgrades, 0) AND qa2.attempt < $quizattemptsalias.attempt)
-                                    FROM {quiz_attempts} qa3
+                                ))";
-                                    WHERE qa3.quiz = $quizattemptsalias.quiz AND
-                                        qa3.userid = $quizattemptsalias.userid
-                                )
-                            )";
 
         case QUIZ_GRADEAVERAGE :
             return '';
 
         case QUIZ_ATTEMPTFIRST :
-            return "$quizattemptsalias.id = (
+            return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2
-                            SELECT MIN(qa2.id)
-                            FROM {quiz_attempts} qa2
                             WHERE qa2.quiz = $quizattemptsalias.quiz AND
-                                qa2.userid = $quizattemptsalias.userid)";
+                                qa2.userid = $quizattemptsalias.userid AND
+                               qa2.attempt < $quizattemptsalias.attempt)";
 
         case QUIZ_ATTEMPTLAST :
-            return "$quizattemptsalias.id = (
+            return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2
-                            SELECT MAX(qa2.id)
-                            FROM {quiz_attempts} qa2
                             WHERE qa2.quiz = $quizattemptsalias.quiz AND
-                                qa2.userid = $quizattemptsalias.userid)";
+                                qa2.userid = $quizattemptsalias.userid AND
+                               qa2.attempt > $quizattemptsalias.attempt)";
     }
 }
 

mod/quiz/tests/reportlib_test.php

@@ -36,7 +36,7 @@ require_once($CFG->dirroot . '/mod/quiz/report/reportlib.php');
  * @copyright 2008 Jamie Pratt me@jamiep.org
  * @license   http://www.gnu.org/copyleft/gpl.html GNU Public License
  */
-class mod_quiz_reportlib_testcase extends basic_testcase {
+class mod_quiz_reportlib_testcase extends advanced_testcase {
     public function test_quiz_report_index_by_keys() {
         $datum = array();
         $object = new stdClass();
@@ -73,4 +73,93 @@ class mod_quiz_reportlib_testcase extends basic_testcase {
         $this->assertEquals('-',
             quiz_report_scale_summarks_as_percentage('-', $quiz, true));
     }
+
+    public function test_quiz_report_qm_filter_select_only_one_attempt_allowed() {
+        $quiz = new stdClass();
+        $quiz->attempts = 1;
+        $this->assertSame('', quiz_report_qm_filter_select($quiz));
+    }
+
+    public function test_quiz_report_qm_filter_select_average() {
+        $quiz = new stdClass();
+        $quiz->attempts = 10;
+        $quiz->grademethod = QUIZ_GRADEAVERAGE;
+        $this->assertSame('', quiz_report_qm_filter_select($quiz));
+    }
+
+    public function test_quiz_report_qm_filter_select_first_last_best() {
+        global $DB;
+        $this->resetAfterTest();
+
+        $fakeattempt = new stdClass();
+        $fakeattempt->userid = 123;
+        $fakeattempt->quiz = 456;
+        $fakeattempt->layout = '1,2,0,3,4,0,5';
+
+        // 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
+        // layout is only given because it has a not-null constraint.
+        // uniqueid values are meaningless, but that column has a unique constraint.
+
+        $fakeattempt->attempt = 3;
+        $fakeattempt->sumgrades = 50;
+        $fakeattempt->uniqueid = 13;
+        $DB->insert_record('quiz_attempts', $fakeattempt);
+
+        $fakeattempt->attempt = 2;
+        $fakeattempt->sumgrades = 50;
+        $fakeattempt->uniqueid = 26;
+        $DB->insert_record('quiz_attempts', $fakeattempt);
+
+        $fakeattempt->attempt = 4;
+        $fakeattempt->sumgrades = null;
+        $fakeattempt->uniqueid = 39;
+        $DB->insert_record('quiz_attempts', $fakeattempt);
+
+        $fakeattempt->attempt = 1;
+        $fakeattempt->sumgrades = 30;
+        $fakeattempt->uniqueid = 52;
+        $DB->insert_record('quiz_attempts', $fakeattempt);
+
+        $fakeattempt->attempt = 1;
+        $fakeattempt->userid = 1;
+        $fakeattempt->sumgrades = 100;
+        $fakeattempt->uniqueid = 65;
+        $DB->insert_record('quiz_attempts', $fakeattempt);
+
+        $quiz = new stdClass();
+        $quiz->attempts = 10;
+
+        $quiz->grademethod = QUIZ_ATTEMPTFIRST;
+        $firstattempt = $DB->get_records_sql("
+                SELECT * FROM {quiz_attempts} quiza WHERE userid = ? AND quiz = ? AND "
+                        . quiz_report_qm_filter_select($quiz), array(123, 456));
+        $this->assertEquals(1, count($firstattempt));
+        $firstattempt = reset($firstattempt);
+        $this->assertEquals(1, $firstattempt->attempt);
+
+        $quiz->grademethod = QUIZ_ATTEMPTLAST;
+        $lastattempt = $DB->get_records_sql("
+                SELECT * FROM {quiz_attempts} quiza WHERE userid = ? AND quiz = ? AND "
+                . quiz_report_qm_filter_select($quiz), array(123, 456));
+        $this->assertEquals(1, count($lastattempt));
+        $lastattempt = reset($lastattempt);
+        $this->assertEquals(4, $lastattempt->attempt);
+
+        $quiz->attempts = 0;
+        $quiz->grademethod = QUIZ_GRADEHIGHEST;
+        $bestattempt = $DB->get_records_sql("
+                SELECT * FROM {quiz_attempts} qa_alias WHERE userid = ? AND quiz = ? AND "
+                . quiz_report_qm_filter_select($quiz, 'qa_alias'), array(123, 456));
+        $this->assertEquals(1, count($bestattempt));
+        $bestattempt = reset($bestattempt);
+        $this->assertEquals(2, $bestattempt->attempt);
+    }
 }