MDL-61348 quiz reports: fix incorrect groups averages

the averages were wrong if a user had mulitple enrolments in a course,
which can happen with some enrolment plugins.

mod/quiz/report/overview/overview_table.php

@@ -92,42 +92,64 @@ public function build_table() {
         }
     }
 
-
     /**
-     * Add an average grade over the attempts of a set of users.
+     * Calculate the average overall and question scores for a set of attempts at the quiz.
+     *
      * @param string $label the title ot use for this row.
-     * @param \core\dml\sql_join $usersjoins (joins, wheres, params) for the users to average over.
+     * @param \core\dml\sql_join $usersjoins to indicate a set of users.
+     * @return array of table cells that make up the average row.
      */
-    protected function add_average_row($label, \core\dml\sql_join $usersjoins) {
+    public function compute_average_row($label, \core\dml\sql_join $usersjoins) {
         global $DB;
 
         list($fields, $from, $where, $params) = $this->base_sql($usersjoins);
         $record = $DB->get_record_sql("
-                SELECT AVG(quiza.sumgrades) AS grade, COUNT(quiza.sumgrades) AS numaveraged
-                  FROM $from
-                 WHERE $where", $params);
+                SELECT AVG(quizaouter.sumgrades) AS grade, COUNT(quizaouter.sumgrades) AS numaveraged
+                  FROM {quiz_attempts} quizaouter
+                  JOIN (
+                       SELECT DISTINCT quiza.id
+                         FROM $from
+                        WHERE $where
+                       ) relevant_attempt_ids ON quizaouter.id = relevant_attempt_ids.id
+                ", $params);
         $record->grade = quiz_rescale_grade($record->grade, $this->quiz, false);
-
         if ($this->is_downloading()) {
             $namekey = 'lastname';
         } else {
             $namekey = 'fullname';
         }
         $averagerow = array(
-            $namekey    => $label,
-            'sumgrades' => $this->format_average($record),
-            'feedbacktext'=> strip_tags(quiz_report_feedback_for_grade(
-                                        $record->grade, $this->quiz->id, $this->context))
+            $namekey       => $label,
+            'sumgrades'    => $this->format_average($record),
+            'feedbacktext' => strip_tags(quiz_report_feedback_for_grade(
+                                         $record->grade, $this->quiz->id, $this->context))
         );
 
         if ($this->options->slotmarks) {
             $dm = new question_engine_data_mapper();
-            $qubaids = new qubaid_join($from, 'quiza.uniqueid', $where, $params);
+            $qubaids = new qubaid_join("{quiz_attempts} quizaouter
+                  JOIN (
+                       SELECT DISTINCT quiza.id
+                         FROM $from
+                        WHERE $where
+                       ) relevant_attempt_ids ON quizaouter.id = relevant_attempt_ids.id",
+                    'quizaouter.uniqueid', '1 = 1', $params);
             $avggradebyq = $dm->load_average_marks($qubaids, array_keys($this->questions));
 
             $averagerow += $this->format_average_grade_for_questions($avggradebyq);
         }
 
+        return $averagerow;
+    }
+
+    /**
+     * Add an average grade row for a set of users.
+     *
+     * @param string $label the title ot use for this row.
+     * @param \core\dml\sql_join $usersjoins (joins, wheres, params) for the users to average over.
+     */
+    protected function add_average_row($label, \core\dml\sql_join $usersjoins) {
+        $averagerow = $this->compute_average_row($label, $usersjoins);
         $this->add_data_keyed($averagerow);
     }
 
@@ -163,7 +185,9 @@ protected function format_average_grade_for_questions($gradeaverages) {
 
     /**
      * Format an entry in an average row.
-     * @param object $record with fields grade and numaveraged
+     * @param object $record with fields grade and numaveraged.
+     * @param bool $question true if this is a question score, false if it is an overall score.
+     * @return string HTML fragment for an average score (with number of things included in the average).
      */
     protected function format_average($record, $question = false) {
         if (is_null($record->grade)) {

mod/quiz/report/overview/tests/report_test.php

@@ -89,7 +89,6 @@ public function test_report_sql($isdownloading) {
 
         // The test data.
         $timestamp = 1234567890;
-        $fields = array('quiz', 'userid', 'attempt', 'sumgrades', 'state');
         $attempts = array(
             array($quiz, $student1, 1, 0.0,  quiz_attempt::FINISHED),
             array($quiz, $student1, 2, 5.0,  quiz_attempt::FINISHED),
@@ -165,6 +164,8 @@ public function test_report_sql($isdownloading) {
         $reportoptions->states = array(quiz_attempt::IN_PROGRESS, quiz_attempt::OVERDUE, quiz_attempt::FINISHED);
 
         // Now do a minimal set-up of the table class.
+        $q->slot = 1;
+        $q->maxmark = 10;
         $table = new quiz_overview_table($quiz, $context, $qmsubselect, $reportoptions,
                 $empty, $studentsjoins, array(1 => $q), null);
         $table->download = $isdownloading; // Cannot call the is_downloading API, because it gives errors.
@@ -196,7 +197,16 @@ public function test_report_sql($isdownloading) {
         $this->assertArrayHasKey($student3->id . '#0', $table->rawdata);
         $this->assertEquals(0, $table->rawdata[$student3->id . '#0']->gradedattempt);
 
-        // Ensure that filtering by inital does not break it.
+        // Check the calculation of averages.
+        $averagerow = $table->compute_average_row('overallaverage', $studentsjoins);
+        $this->assertContains('75.00', $averagerow['sumgrades']);
+        $this->assertContains('75.00', $averagerow['qsgrade1']);
+        if (!$isdownloading) {
+            $this->assertContains('(2)', $averagerow['sumgrades']);
+            $this->assertContains('(2)', $averagerow['qsgrade1']);
+        }
+
+        // Ensure that filtering by initial does not break it.
         // This involves setting a private properly of the base class, which is
         // only really possible using reflection :-(.
         $reflectionobject = new ReflectionObject($table);