diff --git a/reportbuilder/classes/local/aggregation/base.php b/reportbuilder/classes/local/aggregation/base.php index ffb74d2d87e..f175133970b 100644 --- a/reportbuilder/classes/local/aggregation/base.php +++ b/reportbuilder/classes/local/aggregation/base.php @@ -76,6 +76,38 @@ abstract class base { 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 * diff --git a/reportbuilder/classes/local/aggregation/countdistinct.php b/reportbuilder/classes/local/aggregation/countdistinct.php index 7d936e0ebc8..ffcd20011e5 100644 --- a/reportbuilder/classes/local/aggregation/countdistinct.php +++ b/reportbuilder/classes/local/aggregation/countdistinct.php @@ -49,6 +49,20 @@ class countdistinct extends base { 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 * diff --git a/reportbuilder/classes/local/aggregation/groupconcat.php b/reportbuilder/classes/local/aggregation/groupconcat.php index 2c1412623fe..9d7daec241e 100644 --- a/reportbuilder/classes/local/aggregation/groupconcat.php +++ b/reportbuilder/classes/local/aggregation/groupconcat.php @@ -75,32 +75,11 @@ class groupconcat extends base { * @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); } /** diff --git a/reportbuilder/tests/local/aggregation/countdistinct_test.php b/reportbuilder/tests/local/aggregation/countdistinct_test.php index 201f7fca23a..bd46883f370 100644 --- a/reportbuilder/tests/local/aggregation/countdistinct_test.php +++ b/reportbuilder/tests/local/aggregation/countdistinct_test.php @@ -75,4 +75,30 @@ class countdistinct_test extends core_reportbuilder_testcase { ], ], $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); + } }