diff --git a/.upgradenotes/MDL-83361-2024101618221379.yml b/.upgradenotes/MDL-83361-2024101618221379.yml new file mode 100644 index 00000000000..992a5246f90 --- /dev/null +++ b/.upgradenotes/MDL-83361-2024101618221379.yml @@ -0,0 +1,12 @@ +issueNumber: MDL-83361 +notes: + core_reportbuilder: + - message: >- + The base aggregation class has a new `column_groupby` method, to be + implemented in aggregation types to determime whether report tables + should group by the fields of the aggregated column + type: improved + - message: >- + There is a new `date` aggregation type, that can be applied in custom + and system reports + type: improved diff --git a/lang/en/reportbuilder.php b/lang/en/reportbuilder.php index 760344e4049..32d6cbacfc3 100644 --- a/lang/en/reportbuilder.php +++ b/lang/en/reportbuilder.php @@ -31,6 +31,7 @@ $string['aggregatecolumn'] = 'Aggregate column \'{$a}\''; $string['aggregationavg'] = 'Average'; $string['aggregationcount'] = 'Count'; $string['aggregationcountdistinct'] = 'Count distinct'; +$string['aggregationdate'] = 'Date'; $string['aggregationgroupconcat'] = 'Comma separated values'; $string['aggregationgroupconcatdistinct'] = 'Comma separated distinct values'; $string['aggregationmax'] = 'Maximum'; diff --git a/reportbuilder/classes/local/aggregation/base.php b/reportbuilder/classes/local/aggregation/base.php index 77409aa35f0..2a93def98ca 100644 --- a/reportbuilder/classes/local/aggregation/base.php +++ b/reportbuilder/classes/local/aggregation/base.php @@ -18,7 +18,7 @@ declare(strict_types=1); namespace core_reportbuilder\local\aggregation; -use lang_string; +use core\lang_string; use core_reportbuilder\local\report\column; /** @@ -118,6 +118,16 @@ abstract class base { */ abstract public static function get_field_sql(string $field, int $columntype): string; + /** + * Whether the aggregation method is applied to a column, this method determines whether the report table should + * group by the column fields or not + * + * @return bool + */ + public static function column_groupby(): bool { + return false; + } + /** * Return formatted value for column when applying aggregation, by default executing all callbacks on the value * diff --git a/reportbuilder/classes/local/aggregation/date.php b/reportbuilder/classes/local/aggregation/date.php new file mode 100644 index 00000000000..1b51b84206b --- /dev/null +++ b/reportbuilder/classes/local/aggregation/date.php @@ -0,0 +1,91 @@ +. + +declare(strict_types=1); + +namespace core_reportbuilder\local\aggregation; + +use core\{clock, di}; +use core\lang_string; +use core_reportbuilder\local\helpers\format; +use core_reportbuilder\local\report\column; + +/** + * Column date aggregation type + * + * @package core_reportbuilder + * @copyright 2024 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class date extends base { + + /** + * Return aggregation name + * + * @return lang_string + */ + public static function get_name(): lang_string { + return new lang_string('aggregationdate', 'core_reportbuilder'); + } + + /** + * This aggregation can be performed on timestamp columns + * + * @param int $columntype + * @return bool + */ + public static function compatible(int $columntype): bool { + return $columntype === column::TYPE_TIMESTAMP; + } + + /** + * Return the aggregated field SQL + * + * @param string $field + * @param int $columntype + * @return string + */ + public static function get_field_sql(string $field, int $columntype): string { + global $DB; + + // Apply timezone offset for current user. + $datenow = di::get(clock::class)->now(); + + return "({$field} + " . $datenow->getOffset() . ") - " . $DB->sql_modulo($field, DAYSECS); + } + + /** + * When applied to a column, we should group by its fields + * + * @return bool + */ + public static function column_groupby(): bool { + return true; + } + + /** + * Return formatted value for column when applying aggregation + * + * @param mixed $value + * @param array $values + * @param array $callbacks + * @param int $columntype + * @return string + */ + public static function format_value($value, array $values, array $callbacks, int $columntype): string { + return format::userdate($value, (object) [], get_string('strftimedaydate', 'core_langconfig')); + } +} diff --git a/reportbuilder/classes/local/report/column.php b/reportbuilder/classes/local/report/column.php index f0d96f85aff..1eef3a2cf7b 100644 --- a/reportbuilder/classes/local/report/column.php +++ b/reportbuilder/classes/local/report/column.php @@ -18,8 +18,8 @@ declare(strict_types=1); namespace core_reportbuilder\local\report; -use coding_exception; -use lang_string; +use core\exception\coding_exception; +use core\lang_string; use core_reportbuilder\local\helpers\{aggregation, database, join_trait}; use core_reportbuilder\local\aggregation\base; use core_reportbuilder\local\models\column as column_model; @@ -359,8 +359,7 @@ final class column { $field = reset($fieldsalias); // 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()); + $aggregationfieldsql = $this->get_field_aggregation_sql($fieldsaliassql); $fields = ["{$aggregationfieldsql} AS {$field['alias']}"]; } else { @@ -372,6 +371,22 @@ final class column { return array_values($fields); } + /** + * Return aggregated field SQL for the column + * + * @param string[] $sqlfields + * @return string + * @throws coding_exception + */ + private function get_field_aggregation_sql(array $sqlfields): string { + if (empty($this->aggregation)) { + throw new coding_exception('Column aggregation is undefined'); + } + + $columnfieldsql = $this->aggregation::get_column_field_sql($sqlfields); + return $this->aggregation::get_field_sql($columnfieldsql, $this->get_type()); + } + /** * Return column parameters, prefixed by the current index to allow the column to be added multiple times to a report * @@ -421,13 +436,20 @@ final class column { public function get_groupby_sql(): array { global $DB; + $fieldsalias = $this->get_fields_sql_alias(); + + // To ensure cross-platform support for column aggregation, where the aggregation should also be grouped, we need + // to generate SQL from column fields and use it to generate aggregation SQL. + if (!empty($this->aggregation) && $this->aggregation::column_groupby()) { + $fieldsaliassql = array_column($fieldsalias, 'sql'); + return [$this->get_field_aggregation_sql($fieldsaliassql)]; + } + // Return defined value if it's already been set during column definition. if (!empty($this->groupbysql)) { return [$this->groupbysql]; } - $fieldsalias = $this->get_fields_sql_alias(); - // Note that we can reference field aliases in GROUP BY only in MySQL/Postgres. $usealias = in_array($DB->get_dbfamily(), ['mysql', 'postgres']); $columnname = $usealias ? 'alias' : 'sql'; diff --git a/reportbuilder/classes/table/custom_report_table.php b/reportbuilder/classes/table/custom_report_table.php index 9ac51c8395f..14904761eb2 100644 --- a/reportbuilder/classes/table/custom_report_table.php +++ b/reportbuilder/classes/table/custom_report_table.php @@ -93,10 +93,7 @@ class custom_report_table extends base_report_table { } // If we are aggregating any columns, we should group by the remaining ones. - $aggregatedcolumns = array_filter($columns, static function(column $column): bool { - return !empty($column->get_aggregation()); - }); - + $aggregatedcolumns = array_filter($columns, fn(column $column): bool => !empty($column->get_aggregation())); $hasaggregatedcolumns = !empty($aggregatedcolumns); // Also take account of the report setting to show unique rows (only if no columns are being aggregated). @@ -108,7 +105,8 @@ class custom_report_table extends base_report_table { $columnheaders[$column->get_column_alias()] = $columnheading !== '' ? $columnheading : $column->get_title(); // We need to determine for each column whether we should group by its fields, to support aggregation. - if ($showuniquerows || ($hasaggregatedcolumns && empty($column->get_aggregation()))) { + $columnaggregation = $column->get_aggregation(); + if ($showuniquerows || ($hasaggregatedcolumns && (empty($columnaggregation) || $columnaggregation::column_groupby()))) { $groupby = array_merge($groupby, $column->get_groupby_sql()); } diff --git a/reportbuilder/classes/table/system_report_table.php b/reportbuilder/classes/table/system_report_table.php index 424fb97851d..04c1df8b312 100644 --- a/reportbuilder/classes/table/system_report_table.php +++ b/reportbuilder/classes/table/system_report_table.php @@ -103,10 +103,7 @@ class system_report_table extends base_report_table { } // If we are aggregating any columns, we should group by the remaining ones. - $aggregatedcolumns = array_filter($columns, static function(column $column): bool { - return !empty($column->get_aggregation()); - }); - + $aggregatedcolumns = array_filter($columns, fn(column $column): bool => !empty($column->get_aggregation())); $hasaggregatedcolumns = !empty($aggregatedcolumns); if ($hasaggregatedcolumns) { $groupby = $fields; @@ -132,7 +129,8 @@ class system_report_table extends base_report_table { } // We need to determine for each column whether we should group by its fields, to support aggregation. - if ($hasaggregatedcolumns && empty($column->get_aggregation())) { + $columnaggregation = $column->get_aggregation(); + if ($hasaggregatedcolumns && (empty($columnaggregation) || $columnaggregation::column_groupby())) { $groupby = array_merge($groupby, $column->get_groupby_sql()); } diff --git a/reportbuilder/tests/local/aggregation/date_test.php b/reportbuilder/tests/local/aggregation/date_test.php new file mode 100644 index 00000000000..fd25947960c --- /dev/null +++ b/reportbuilder/tests/local/aggregation/date_test.php @@ -0,0 +1,88 @@ +. + +declare(strict_types=1); + +namespace core_reportbuilder\local\aggregation; + +use core_reportbuilder_testcase; +use core_reportbuilder_generator; +use core_user\reportbuilder\datasource\users; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once("{$CFG->dirroot}/reportbuilder/tests/helpers.php"); + +/** + * Unit tests for date aggregation + * + * @package core_reportbuilder + * @covers \core_reportbuilder\local\aggregation\date + * @copyright 2024 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class date_test extends core_reportbuilder_testcase { + + /** + * Mock the clock + */ + protected function setUp(): void { + parent::setUp(); + + $this->setTimezone('Europe/London', 'Europe/London'); + $this->mock_clock_with_frozen(1609495200); + } + + /** + * Test aggregation when applied to column + */ + public function test_column_aggregation(): void { + $this->resetAfterTest(); + + // Test subjects. + $this->getDataGenerator()->create_user(['firstname' => 'Andy', 'lastaccess' => 1622523600]); + $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastaccess' => 1622530800]); + $this->getDataGenerator()->create_user(['firstname' => 'Charlie', 'lastaccess' => 1622624400]); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'Users', 'source' => users::class, 'default' => 0]); + + // Report columns, aggregated/sorted by user lastaccess. + $generator->create_column([ + 'reportid' => $report->get('id'), + 'uniqueidentifier' => 'user:lastaccess', + 'aggregation' => date::get_class_name(), + 'sortenabled' => 1, + 'sortdirection' => SORT_ASC, + ]); + + // Date aggregation requires other columns to be aggregated to make any sense. + $generator->create_column([ + 'reportid' => $report->get('id'), + 'uniqueidentifier' => 'user:firstname', + 'aggregation' => groupconcat::get_class_name(), + ]); + + $content = $this->get_custom_report_content($report->get('id')); + $this->assertEquals([ + ['', 'Admin'], + ['Tuesday, 1 June 2021', 'Andy, Bob'], + ['Wednesday, 2 June 2021', 'Charlie'], + ], array_map('array_values', $content)); + } +}