diff --git a/reportbuilder/classes/local/aggregation/base.php b/reportbuilder/classes/local/aggregation/base.php index 719d16e6078..ffb74d2d87e 100644 --- a/reportbuilder/classes/local/aggregation/base.php +++ b/reportbuilder/classes/local/aggregation/base.php @@ -66,6 +66,16 @@ abstract class base { return $columnsortable; } + /** + * Return SQL suitable for using within {@see get_field_sql} for column fields, by default just the first one + * + * @param string[] $sqlfields + * @return string + */ + public static function get_column_field_sql(array $sqlfields): string { + return reset($sqlfields); + } + /** * Return the aggregated field SQL * @@ -76,12 +86,22 @@ abstract class base { abstract public static function get_field_sql(string $field, int $columntype): string; /** - * Return formatted value for column when applying aggregation + * Return formatted value for column when applying aggregation, by default executing all callbacks on the value + * + * Should be overridden in child classes that need to format the column value differently (e.g. 'sum' would just show + * a numeric count value) * * @param mixed $value * @param array $values - * @param array $callbacks + * @param array $callbacks Array of column callbacks, {@see column::add_callback} for definition * @return mixed */ - abstract public static function format_value($value, array $values, array $callbacks); + public static function format_value($value, array $values, array $callbacks) { + foreach ($callbacks as $callback) { + [$callable, $arguments] = $callback; + $value = ($callable)($value, (object) $values, $arguments); + } + + return $value; + } } diff --git a/reportbuilder/classes/local/aggregation/groupconcat.php b/reportbuilder/classes/local/aggregation/groupconcat.php index 4367aa1abad..0e490c47689 100644 --- a/reportbuilder/classes/local/aggregation/groupconcat.php +++ b/reportbuilder/classes/local/aggregation/groupconcat.php @@ -19,6 +19,7 @@ declare(strict_types=1); namespace core_reportbuilder\local\aggregation; use lang_string; +use core_reportbuilder\local\helpers\database; use core_reportbuilder\local\report\column; /** @@ -30,6 +31,9 @@ use core_reportbuilder\local\report\column; */ class groupconcat extends base { + /** @var string Character to use as a delimeter between column fields */ + private const DELIMETER = '|'; + /** * Return aggregation name * @@ -51,6 +55,51 @@ class groupconcat extends base { ]); } + /** + * We cannot sort this aggregation type + * + * @param bool $columnsortable + * @return bool + */ + public static function sortable(bool $columnsortable): bool { + return false; + } + + /** + * 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 { + 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::DELIMETER . "'"; + } + + // Slice off the last delimeter. + return $DB->sql_concat(...array_slice($concatfields, 0, -1)); + } + /** * Return the aggregated field SQL * @@ -61,11 +110,14 @@ class groupconcat extends base { public static function get_field_sql(string $field, int $columntype): string { global $DB; - return $DB->sql_group_concat($field); + $fieldsort = database::sql_group_concat_sort($field); + + return $DB->sql_group_concat($field, ', ', $fieldsort); } /** - * Return formatted value for column when applying aggregation + * Return formatted value for column when applying aggregation, note we need to split apart the concatenated string + * and apply callbacks to each concatenated value separately * * @param mixed $value * @param array $values @@ -73,6 +125,21 @@ class groupconcat extends base { * @return mixed */ public static function format_value($value, array $values, array $callbacks) { - return $value; + $formattedvalues = []; + + // Store original names of all values that would be present without aggregation. + $valuenames = array_keys($values); + $values = explode(', ', (string) reset($values)); + + // Loop over each extracted value from the concatenated string. + foreach ($values as $value) { + $originalvalue = array_combine($valuenames, explode(self::DELIMETER, $value)); + $originalfirstvalue = reset($originalvalue); + + // Once we've re-constructed each value, we can apply callbacks to it. + $formattedvalues[] = parent::format_value($originalfirstvalue, $originalvalue, $callbacks); + } + + return implode(', ', $formattedvalues); } } diff --git a/reportbuilder/classes/local/aggregation/groupconcatdistinct.php b/reportbuilder/classes/local/aggregation/groupconcatdistinct.php index 831a9175bf1..dd3f0963815 100644 --- a/reportbuilder/classes/local/aggregation/groupconcatdistinct.php +++ b/reportbuilder/classes/local/aggregation/groupconcatdistinct.php @@ -19,7 +19,7 @@ declare(strict_types=1); namespace core_reportbuilder\local\aggregation; use lang_string; -use core_reportbuilder\local\report\column; +use core_reportbuilder\local\helpers\database; /** * Column group concatenation distinct aggregation type @@ -28,7 +28,7 @@ use core_reportbuilder\local\report\column; * @copyright 2021 Paul Holden * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class groupconcatdistinct extends base { +class groupconcatdistinct extends groupconcat { /** * Return aggregation name @@ -53,9 +53,7 @@ class groupconcatdistinct extends base { 'postgres', ]); - return $dbsupportedtype && !in_array($columntype, [ - column::TYPE_TIMESTAMP, - ]); + return $dbsupportedtype && parent::compatible($columntype); } /** @@ -69,22 +67,15 @@ class groupconcatdistinct extends base { global $DB; // DB limitations mean we only support MySQL and Postgres, and each handle it differently. + $fieldsort = database::sql_group_concat_sort($field); if ($DB->get_dbfamily() === 'postgres') { - return "STRING_AGG(DISTINCT CAST({$field} AS VARCHAR), ', ')"; + if ($fieldsort !== '') { + $fieldsort = "ORDER BY {$fieldsort}"; + } + + return "STRING_AGG(DISTINCT CAST({$field} AS VARCHAR), ', ' {$fieldsort})"; } else { - return $DB->sql_group_concat("DISTINCT {$field}"); + return $DB->sql_group_concat("DISTINCT {$field}", ', ', $fieldsort); } } - - /** - * Return formatted value for column when applying aggregation - * - * @param mixed $value - * @param array $values - * @param array $callbacks - * @return mixed - */ - public static function format_value($value, array $values, array $callbacks) { - return $value; - } } diff --git a/reportbuilder/classes/local/aggregation/max.php b/reportbuilder/classes/local/aggregation/max.php index c801b9101e8..6dd1dcc6806 100644 --- a/reportbuilder/classes/local/aggregation/max.php +++ b/reportbuilder/classes/local/aggregation/max.php @@ -64,20 +64,4 @@ class max extends base { public static function get_field_sql(string $field, int $columntype): string { return "MAX({$field})"; } - - /** - * Return formatted value for column when applying aggregation - * - * @param mixed $value - * @param array $values - * @param array $callbacks - * @return mixed - */ - public static function format_value($value, array $values, array $callbacks) { - foreach ($callbacks as $callback) { - $value = ($callback[0])($value, (object) $values, $callback[1]); - } - - return $value; - } } diff --git a/reportbuilder/classes/local/aggregation/min.php b/reportbuilder/classes/local/aggregation/min.php index efde0020068..91608d7ad1c 100644 --- a/reportbuilder/classes/local/aggregation/min.php +++ b/reportbuilder/classes/local/aggregation/min.php @@ -64,20 +64,4 @@ class min extends base { public static function get_field_sql(string $field, int $columntype): string { return "MIN({$field})"; } - - /** - * Return formatted value for column when applying aggregation - * - * @param mixed $value - * @param array $values - * @param array $callbacks - * @return mixed - */ - public static function format_value($value, array $values, array $callbacks) { - foreach ($callbacks as $callback) { - $value = ($callback[0])($value, (object) $values, $callback[1]); - } - - return $value; - } } diff --git a/reportbuilder/classes/local/helpers/database.php b/reportbuilder/classes/local/helpers/database.php index e391504a44f..9bb3d082feb 100644 --- a/reportbuilder/classes/local/helpers/database.php +++ b/reportbuilder/classes/local/helpers/database.php @@ -19,6 +19,7 @@ declare(strict_types=1); namespace core_reportbuilder\local\helpers; use coding_exception; +use core_text; /** * Helper functions for DB manipulations @@ -74,4 +75,43 @@ class database { return true; } + + /** + * Generate SQL expression for sorting group concatenated fields + * + * @param string $field The original field or SQL expression + * @param string|null $sort A valid SQL ORDER BY to sort the concatenated fields, if omitted then $field will be used + * @return string + */ + public static function sql_group_concat_sort(string $field, string $sort = null): string { + global $DB; + + // Fallback to sorting by the specified field, unless it contains parameters which would be duplicated. + if ($sort === null && !preg_match('/[:?$]/', $field)) { + $fieldsort = $field; + } else { + $fieldsort = $sort; + } + + // Nothing to sort by. + if ($fieldsort === null) { + return ''; + } + + // If the sort specifies a direction, we need to handle that differently in Postgres. + if ($DB->get_dbfamily() === 'postgres') { + $fieldsortdirection = ''; + + preg_match('/(?ASC|DESC)?$/i', $fieldsort, $matches); + if (array_key_exists('direction', $matches)) { + $fieldsortdirection = $matches['direction']; + $fieldsort = core_text::substr($fieldsort, 0, -(core_text::strlen($fieldsortdirection))); + } + + // Cast sort, stick the direction on the end. + $fieldsort = "CAST({$fieldsort} AS VARCHAR) {$fieldsortdirection}"; + } + + return $fieldsort; + } } diff --git a/reportbuilder/classes/local/helpers/format.php b/reportbuilder/classes/local/helpers/format.php index 8eae420a6b8..8a0a2971764 100644 --- a/reportbuilder/classes/local/helpers/format.php +++ b/reportbuilder/classes/local/helpers/format.php @@ -21,7 +21,10 @@ namespace core_reportbuilder\local\helpers; use stdClass; /** - * Class containing helper methods for format columns data as callbacks. + * Class containing helper methods for formatting column data via callbacks + * + * Note that type hints for each $value argument are avoided to allow for these callbacks to be executed when columns are + * aggregated using one of the "Group concatenation" methods, where the value is typically stringified * * @package core_reportbuilder * @copyright 2021 Sara Arjona based on Alberto Lara Hernández code. @@ -37,8 +40,8 @@ class format { * @param string|null $format Format string for strftime * @return string */ - public static function userdate(int $value, stdClass $row, ?string $format = null): string { - return $value ? userdate($value, $format) : ''; + public static function userdate($value, stdClass $row, ?string $format = null): string { + return $value ? userdate((int) $value, $format) : ''; } /** @@ -47,8 +50,8 @@ class format { * @param bool $value * @return string */ - public static function boolean_as_text(bool $value): string { - return $value ? get_string('yes') : get_string('no'); + public static function boolean_as_text($value): string { + return (bool) $value ? get_string('yes') : get_string('no'); } /** @@ -57,7 +60,7 @@ class format { * @param float $value * @return string */ - public static function percent(float $value): string { - return sprintf('%.1f', $value) . '%'; + public static function percent($value): string { + return sprintf('%.1f%%', (float) $value); } } diff --git a/reportbuilder/classes/local/report/column.php b/reportbuilder/classes/local/report/column.php index f03fc980dfb..75f991f9c09 100644 --- a/reportbuilder/classes/local/report/column.php +++ b/reportbuilder/classes/local/report/column.php @@ -375,10 +375,15 @@ final class column { public function get_fields(): array { $fieldsalias = $this->get_fields_sql_alias(); - // We aggregate the first field only. if (!empty($this->aggregation)) { + $fieldsaliassql = array_column($fieldsalias, 'sql'); $field = reset($fieldsalias); - $fields = [$this->aggregation::get_field_sql($field['sql'], $this->get_type()) . " AS {$field['alias']}"]; + + // If aggregating the column, generate SQL from column fields and use it to generate aggregation SQL. + $columnfieldsql = $this->aggregation::get_column_field_sql($fieldsaliassql); + $aggregationfieldsql = $this->aggregation::get_field_sql($columnfieldsql, $this->get_type()); + + $fields = ["{$aggregationfieldsql} AS {$field['alias']}"]; } else { $fields = array_map(static function(array $field): string { return "{$field['sql']} AS {$field['alias']}"; diff --git a/reportbuilder/tests/behat/behat_reportbuilder.php b/reportbuilder/tests/behat/behat_reportbuilder.php index f869a044b0f..2c9c6a88401 100644 --- a/reportbuilder/tests/behat/behat_reportbuilder.php +++ b/reportbuilder/tests/behat/behat_reportbuilder.php @@ -19,7 +19,9 @@ declare(strict_types=1); // NOTE: no MOODLE_INTERNAL test here, this file may be required by behat before including /config.php. require_once(__DIR__ . '/../../../lib/behat/behat_base.php'); +use core_reportbuilder\local\aggregation\groupconcatdistinct; use core_reportbuilder\local\models\report; +use core_reportbuilder\local\report\column; /** * Behat step definitions for Report builder @@ -71,4 +73,30 @@ class behat_reportbuilder extends behat_base { ]), ]; } + + /** + * Set aggregation for given column in report editor (proxied so we can skip if aggregation type not available) + * + * @When I set the :column column aggregation to :aggregation + * + * @param string $column + * @param string $aggregation + * + * @throws \Moodle\BehatExtension\Exception\SkippedException + */ + public function i_set_the_column_aggregation_to(string $column, string $aggregation): void { + + // Skip if aggregation type unavailable. + $aggregationgroupconcatdistinct = (string) groupconcatdistinct::get_name(); + if ($aggregation === $aggregationgroupconcatdistinct && !groupconcatdistinct::compatible(column::TYPE_TEXT)) { + throw new \Moodle\BehatExtension\Exception\SkippedException("{$aggregationgroupconcatdistinct} not available"); + } + + $editlabel = get_string('aggregatecolumn', 'core_reportbuilder', $column); + + // Work around for MDL-72696, which treats all inplace_editable elements as text boxes and causes unpredictable + // behaviour in Chrome when it types characters into an inplace_editable of type select (it selects the wrong option). + $this->execute('behat_general::i_click_on', [$this->escape($editlabel), 'link']); + $this->execute('behat_forms::i_set_the_field_to', [$this->escape($editlabel), $this->escape($aggregation)]); + } } diff --git a/reportbuilder/tests/behat/columnaggregationeditor.feature b/reportbuilder/tests/behat/columnaggregationeditor.feature new file mode 100644 index 00000000000..a029d68005c --- /dev/null +++ b/reportbuilder/tests/behat/columnaggregationeditor.feature @@ -0,0 +1,96 @@ +@core_reportbuilder @javascript +Feature: Manage custom report columns aggregation + In order to manage the aggregation for columns of custom reports + As an admin + I need to select an aggregation for columns + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | confirmed | lastaccess | + | user01 | Bill | Richie | user01@example.com | 1 | ##2 days ago## | + | user02 | Ben | Richie | user02@example.com | 1 | ##3 days ago## | + | user03 | Bill | Richie | user03@example.com | 0 | ##3 days ago## | + + Scenario Outline: Aggregate a text column + Given the following "core_reportbuilder > Reports" exist: + | name | source | default | + | My report | core_user\reportbuilder\datasource\users | 0 | + And the following "core_reportbuilder > Columns" exist: + | report | uniqueidentifier | + | My report | user:lastname | + | My report | user:firstname | + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + And I change window size to "large" + When I set the "First name" column aggregation to "" + Then I should see "Aggregated column 'First name'" + And I should see "" in the "Richie" "table_row" + Examples: + | aggregation | output | + | Comma separated distinct values | Ben, Bill | + | Comma separated values | Ben, Bill, Bill | + | Count | 3 | + | Count distinct | 2 | + + Scenario Outline: Aggregate a text column containing multiple fields + Given the following "core_reportbuilder > Reports" exist: + | name | source | default | + | My report | core_user\reportbuilder\datasource\users | 0 | + And the following "core_reportbuilder > Columns" exist: + | report | uniqueidentifier | + | My report | user:lastname | + | My report | user:fullname | + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + And I change window size to "large" + When I set the "Full name" column aggregation to "" + Then I should see "Aggregated column 'Full name'" + And I should see "" in the "Richie" "table_row" + Examples: + | aggregation | output | + | Comma separated distinct values | Ben Richie, Bill Richie | + | Comma separated values | Ben Richie, Bill Richie, Bill Richie | + | Count | 3 | + | Count distinct | 2 | + + Scenario Outline: Aggregate a time column + Given the following "core_reportbuilder > Reports" exist: + | name | source | default | + | My report | core_user\reportbuilder\datasource\users | 0 | + And the following "core_reportbuilder > Columns" exist: + | report | uniqueidentifier | + | My report | user:lastname | + | My report | user:lastaccess | + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + And I change window size to "large" + When I set the "Last access" column aggregation to "" + Then I should see "Aggregated column 'Last access'" + And I should see "" in the "Richie" "table_row" + Examples: + | aggregation | output | + | Count | 3 | + | Count distinct | 2 | + | Maximum | ##2 days ago##%A, %d %B %Y## | + | Minimum | ##3 days ago##%A, %d %B %Y## | + + Scenario Outline: Aggregate a boolean column + Given the following "core_reportbuilder > Reports" exist: + | name | source | default | + | My report | core_user\reportbuilder\datasource\users | 0 | + And the following "core_reportbuilder > Columns" exist: + | report | uniqueidentifier | + | My report | user:lastname | + | My report | user:confirmed | + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + And I change window size to "large" + When I set the "Confirmed" column aggregation to "" + Then I should see "Aggregated column 'Confirmed'" + And I should see "" in the "Richie" "table_row" + Examples: + | aggregation | output | + | Comma separated distinct values | No, Yes | + | Comma separated values | No, Yes, Yes | + | Count | 3 | + | Count distinct | 2 | + | Maximum | Yes | + | Minimum | No | + | Percentage | 66.7% | + | Sum | 2 | diff --git a/reportbuilder/tests/local/aggregation/groupconcat_test.php b/reportbuilder/tests/local/aggregation/groupconcat_test.php index e8dde3931cb..f10ac6c85ba 100644 --- a/reportbuilder/tests/local/aggregation/groupconcat_test.php +++ b/reportbuilder/tests/local/aggregation/groupconcat_test.php @@ -45,8 +45,8 @@ class groupconcat_test extends core_reportbuilder_testcase { $this->resetAfterTest(); // Test subjects. - $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Apple']); $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Banana']); + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Apple']); $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Banana']); /** @var core_reportbuilder_generator $generator */ @@ -63,18 +63,56 @@ class groupconcat_test extends core_reportbuilder_testcase { ->set('aggregation', groupconcat::get_class_name()) ->update(); - [$firstrow, $secondrow] = $this->get_custom_report_content($report->get('id')); + // Assert lastname column was aggregated, and sorted predictably. + $content = $this->get_custom_report_content($report->get('id')); $this->assertEquals([ - 'c0_firstname' => 'Admin', - 'c1_lastname' => 'User', - ], $firstrow); + [ + 'c0_firstname' => 'Admin', + 'c1_lastname' => 'User', + ], + [ + 'c0_firstname' => 'Bob', + 'c1_lastname' => 'Apple, Banana, Banana', + ], + ], $content); + } - // Currently the aggregated field sort order is undefined. - $this->assertEquals('Bob', $secondrow['c0_firstname']); - $this->assertEqualsCanonicalizing([ - 'Apple', - 'Banana', - 'Banana', - ], explode(', ', $secondrow['c1_lastname'])); + /** + * Test aggregation when applied to column with callback + */ + public function test_column_aggregation_with_callback(): void { + $this->resetAfterTest(); + + // Test subjects. + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'confirmed' => 1]); + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'confirmed' => 0]); + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'confirmed' => 1]); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'Users', 'source' => users::class, 'default' => 0]); + + // First column, sorted. + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:firstname']) + ->set('sortenabled', true) + ->update(); + + // This is the column we'll aggregate. + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:confirmed']) + ->set('aggregation', groupconcat::get_class_name()) + ->update(); + + // Assert confirmed column was aggregated, and sorted predictably with callback applied. + $content = $this->get_custom_report_content($report->get('id')); + $this->assertEquals([ + [ + 'c0_firstname' => 'Admin', + 'c1_confirmed' => 'Yes', + ], + [ + 'c0_firstname' => 'Bob', + 'c1_confirmed' => 'No, Yes, Yes', + ], + ], $content); } } diff --git a/reportbuilder/tests/local/aggregation/groupconcatdistinct_test.php b/reportbuilder/tests/local/aggregation/groupconcatdistinct_test.php index 1f930720afd..680a5e37127 100644 --- a/reportbuilder/tests/local/aggregation/groupconcatdistinct_test.php +++ b/reportbuilder/tests/local/aggregation/groupconcatdistinct_test.php @@ -57,8 +57,8 @@ class groupconcatdistinct_test extends core_reportbuilder_testcase { $this->resetAfterTest(); // Test subjects. - $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Apple']); $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Banana']); + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Apple']); $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Banana']); /** @var core_reportbuilder_generator $generator */ @@ -75,17 +75,56 @@ class groupconcatdistinct_test extends core_reportbuilder_testcase { ->set('aggregation', groupconcatdistinct::get_class_name()) ->update(); - [$firstrow, $secondrow] = $this->get_custom_report_content($report->get('id')); + // Assert lastname column was aggregated, and sorted predictably. + $content = $this->get_custom_report_content($report->get('id')); $this->assertEquals([ - 'c0_firstname' => 'Admin', - 'c1_lastname' => 'User', - ], $firstrow); + [ + 'c0_firstname' => 'Admin', + 'c1_lastname' => 'User', + ], + [ + 'c0_firstname' => 'Bob', + 'c1_lastname' => 'Apple, Banana', + ], + ], $content); + } - // Currently the aggregated field sort order is undefined. - $this->assertEquals('Bob', $secondrow['c0_firstname']); - $this->assertEqualsCanonicalizing([ - 'Apple', - 'Banana', - ], explode(', ', $secondrow['c1_lastname'])); + /** + * Test aggregation when applied to column with callback + */ + public function test_column_aggregation_with_callback(): void { + $this->resetAfterTest(); + + // Test subjects. + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'confirmed' => 1]); + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'confirmed' => 0]); + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'confirmed' => 1]); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'Users', 'source' => users::class, 'default' => 0]); + + // First column, sorted. + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:firstname']) + ->set('sortenabled', true) + ->update(); + + // This is the column we'll aggregate. + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:confirmed']) + ->set('aggregation', groupconcatdistinct::get_class_name()) + ->update(); + + // Assert confirmed column was aggregated, and sorted predictably with callback applied. + $content = $this->get_custom_report_content($report->get('id')); + $this->assertEquals([ + [ + 'c0_firstname' => 'Admin', + 'c1_confirmed' => 'Yes', + ], + [ + 'c0_firstname' => 'Bob', + 'c1_confirmed' => 'No, Yes', + ], + ], $content); } }