MDL-73706 reportbuilder: fix count for columns with multiple fields.

When using the "Count distinct" aggregation type on a column that
selects multiple fields, we should account for each of them in the
returned SQL.

Move helper method to facilitate this to the base aggregation class
so it can be re-used between all types.

reportbuilder/classes/local/aggregation/base.php

@@ -76,6 +76,38 @@ public static function get_column_field_sql(array $sqlfields): string {
         return reset($sqlfields);
     }
 
+    /**
+     * Helper method for concatenating given fields for a column, so they are suitable for aggregation
+     *
+     * @param string[] $sqlfields
+     * @param string $delimeter
+     * @return string
+     */
+    final protected static function get_column_fields_concat(array $sqlfields, string $delimeter = ','): string {
+        global $DB;
+
+        $concatfields = [];
+        foreach ($sqlfields as $sqlfield) {
+
+            // We need to ensure all values are char (this ought to be done in the DML drivers, see MDL-72184).
+            switch ($DB->get_dbfamily()) {
+                case 'postgres' :
+                    $sqlfield = "CAST({$sqlfield} AS VARCHAR)";
+                    break;
+                case 'oracle' :
+                    $sqlfield = "TO_CHAR({$sqlfield})";
+                    break;
+            }
+
+            // Coalesce all the SQL fields, to remove all nulls.
+            $concatfields[] = "COALESCE({$sqlfield}, ' ')";
+            $concatfields[] = "'{$delimeter}'";
+        }
+
+        // Slice off the last delimeter.
+        return $DB->sql_concat(...array_slice($concatfields, 0, -1));
+    }
+
     /**
      * Return the aggregated field SQL
      *

reportbuilder/classes/local/aggregation/countdistinct.php

@@ -49,6 +49,20 @@ public static function compatible(int $columntype): bool {
         return true;
     }
 
+    /**
+     * Override base method to ensure all SQL fields are concatenated together if there are multiple
+     *
+     * @param array $sqlfields
+     * @return string
+     */
+    public static function get_column_field_sql(array $sqlfields): string {
+        if (count($sqlfields) === 1) {
+            return parent::get_column_field_sql($sqlfields);
+        }
+
+        return self::get_column_fields_concat($sqlfields);
+    }
+
     /**
      * Return the aggregated field SQL
      *

reportbuilder/classes/local/aggregation/groupconcat.php

@@ -75,32 +75,11 @@ public static function sortable(bool $columnsortable): bool {
      * @return string
      */
     public static function get_column_field_sql(array $sqlfields): string {
-        global $DB;
-
         if (count($sqlfields) === 1) {
             return parent::get_column_field_sql($sqlfields);
         }
 
-        // Coalesce all the SQL fields, to remove all nulls.
-        $concatfields = [];
-        foreach ($sqlfields as $sqlfield) {
-
-            // We need to ensure all values are char (this ought to be done in the DML drivers, see MDL-72184).
-            switch ($DB->get_dbfamily()) {
-                case 'postgres' :
-                    $sqlfield = "CAST({$sqlfield} AS VARCHAR)";
-                break;
-                case 'oracle' :
-                    $sqlfield = "TO_CHAR({$sqlfield})";
-                break;
-            }
-
-            $concatfields[] = "COALESCE({$sqlfield}, ' ')";
-            $concatfields[] = "'" . self::COLUMN_FIELD_DELIMETER . "'";
-        }
-
-        // Slice off the last delimeter.
-        return $DB->sql_concat(...array_slice($concatfields, 0, -1));
+        return self::get_column_fields_concat($sqlfields, self::COLUMN_FIELD_DELIMETER);
     }
 
     /**

reportbuilder/tests/local/aggregation/countdistinct_test.php

@@ -75,4 +75,30 @@ public function test_column_aggregation(): void {
             ],
         ], $content);
     }
+
+    /**
+     * Test aggregation when applied to column with multiple fields
+     */
+    public function test_column_aggregation_multiple_fields(): void {
+        $this->resetAfterTest();
+
+        // Create a user with the same firstname as existing admin.
+        $this->getDataGenerator()->create_user(['firstname' => 'Admin', 'lastname' => 'Test']);
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+        $report = $generator->create_report(['name' => 'Users', 'source' => users::class, 'default' => 0]);
+
+        // This is the column we'll aggregate.
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname'])
+            ->set('aggregation', countdistinct::get_class_name())
+            ->update();
+
+        $content = $this->get_custom_report_content($report->get('id'));
+        $this->assertCount(1, $content);
+
+        // There are two distinct fullnames ("Admin User" & "Admin Test").
+        $countdistinct = reset($content[0]);
+        $this->assertEquals(2, $countdistinct);
+    }
 }