Merge branch 'MDL-83361' of https://github.com/paulholden/moodle

.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

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';

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
      *

reportbuilder/classes/local/aggregation/date.php

@@ -0,0 +1,91 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+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 <paulh@moodle.com>
+ * @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'));
+    }
+}

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';

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());
             }
 

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());
             }
 

reportbuilder/tests/local/aggregation/date_test.php

@@ -0,0 +1,88 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+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 <paulh@moodle.com>
+ * @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));
+    }
+}