diff --git a/.upgradenotes/MDL-84537-2025021416101901.yml b/.upgradenotes/MDL-84537-2025021416101901.yml new file mode 100644 index 00000000000..576bb31ca28 --- /dev/null +++ b/.upgradenotes/MDL-84537-2025021416101901.yml @@ -0,0 +1,18 @@ +issueNumber: MDL-84537 +notes: + core_reportbuilder: + - message: >- + Aggregation types can access passed options set via the base class + constructor in the `$this->options[]` class property. As such, their + `format_value` method is no longer static and is always called from an + instantiated class instance + type: changed + - message: >- + New `$options` argument added to the + `column::set_aggregation` method for system reports, to set aggregation + type-specific options + + + Report entities can call new `column::set_aggregation_options` to + achieve the same + type: changed diff --git a/reportbuilder/classes/datasource.php b/reportbuilder/classes/datasource.php index 3aec964d30b..c3c32d14d61 100644 --- a/reportbuilder/classes/datasource.php +++ b/reportbuilder/classes/datasource.php @@ -134,11 +134,13 @@ abstract class datasource extends base { " {$instance->get_is_deprecated_message()}", DEBUG_DEVELOPER); } + $columnaggregation = $column->get('aggregation'); + // We should clone the report column to ensure if it's added twice to a report, each operates independently. $this->activecolumns['values'][] = clone $instance ->set_index($index) ->set_persistent($column) - ->set_aggregation($column->get('aggregation')); + ->set_aggregation($columnaggregation, $instance->get_aggregation_options($columnaggregation)); } } diff --git a/reportbuilder/classes/local/aggregation/avg.php b/reportbuilder/classes/local/aggregation/avg.php index ed126b92d88..788792390a7 100644 --- a/reportbuilder/classes/local/aggregation/avg.php +++ b/reportbuilder/classes/local/aggregation/avg.php @@ -85,7 +85,7 @@ class avg extends base { * @param int $columntype * @return mixed */ - public static function format_value($value, array $values, array $callbacks, int $columntype) { + public function format_value($value, array $values, array $callbacks, int $columntype) { if (reset($values) === null) { return null; } diff --git a/reportbuilder/classes/local/aggregation/base.php b/reportbuilder/classes/local/aggregation/base.php index 79c3b4ce024..a3b2059c017 100644 --- a/reportbuilder/classes/local/aggregation/base.php +++ b/reportbuilder/classes/local/aggregation/base.php @@ -30,6 +30,18 @@ use core_reportbuilder\local\report\column; */ abstract class base { + /** + * Constructor + * + * @param array $options Aggregation type specific options + */ + public function __construct( + /** @var array Aggregation type specific options */ + protected readonly array $options = [], + ) { + + } + /** * Return the class name of the aggregation type * @@ -153,7 +165,7 @@ abstract class base { * @param int $columntype The original type of the column, to ensure it is preserved for callbacks * @return mixed */ - public static function format_value($value, array $values, array $callbacks, int $columntype) { + public function format_value($value, array $values, array $callbacks, int $columntype) { foreach ($callbacks as $callback) { [$callable, $arguments] = $callback; $value = ($callable)($value, (object) $values, $arguments, static::get_class_name()); diff --git a/reportbuilder/classes/local/aggregation/count.php b/reportbuilder/classes/local/aggregation/count.php index f2d4ab5e606..d7111464887 100644 --- a/reportbuilder/classes/local/aggregation/count.php +++ b/reportbuilder/classes/local/aggregation/count.php @@ -79,7 +79,7 @@ class count extends base { * @param int $columntype * @return int */ - public static function format_value($value, array $values, array $callbacks, int $columntype): int { + public function format_value($value, array $values, array $callbacks, int $columntype): int { return (int) reset($values); } } diff --git a/reportbuilder/classes/local/aggregation/countdistinct.php b/reportbuilder/classes/local/aggregation/countdistinct.php index 0b4a834ef88..944a034a597 100644 --- a/reportbuilder/classes/local/aggregation/countdistinct.php +++ b/reportbuilder/classes/local/aggregation/countdistinct.php @@ -93,7 +93,7 @@ class countdistinct extends base { * @param int $columntype * @return int */ - public static function format_value($value, array $values, array $callbacks, int $columntype): int { + public function format_value($value, array $values, array $callbacks, int $columntype): int { return (int) reset($values); } } diff --git a/reportbuilder/classes/local/aggregation/date.php b/reportbuilder/classes/local/aggregation/date.php index 294cd9419d3..17200af5472 100644 --- a/reportbuilder/classes/local/aggregation/date.php +++ b/reportbuilder/classes/local/aggregation/date.php @@ -93,7 +93,7 @@ class date extends base { * @param int $columntype * @return string */ - public static function format_value($value, array $values, array $callbacks, int $columntype): string { + public 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/aggregation/groupconcat.php b/reportbuilder/classes/local/aggregation/groupconcat.php index 8a76fc5e997..d84c4c0655c 100644 --- a/reportbuilder/classes/local/aggregation/groupconcat.php +++ b/reportbuilder/classes/local/aggregation/groupconcat.php @@ -100,7 +100,7 @@ class groupconcat extends base { * @param int $columntype * @return mixed */ - public static function format_value($value, array $values, array $callbacks, int $columntype) { + public function format_value($value, array $values, array $callbacks, int $columntype) { $firstvalue = reset($values); if ($firstvalue === null) { return ''; diff --git a/reportbuilder/classes/local/aggregation/percent.php b/reportbuilder/classes/local/aggregation/percent.php index a97305941e6..fd0c83acb62 100644 --- a/reportbuilder/classes/local/aggregation/percent.php +++ b/reportbuilder/classes/local/aggregation/percent.php @@ -82,7 +82,7 @@ class percent extends base { * @param int $columntype * @return string */ - public static function format_value($value, array $values, array $callbacks, int $columntype): string { + public function format_value($value, array $values, array $callbacks, int $columntype): string { if (reset($values) === null) { return ''; } diff --git a/reportbuilder/classes/local/aggregation/sum.php b/reportbuilder/classes/local/aggregation/sum.php index 6395212803d..93d35c395b0 100644 --- a/reportbuilder/classes/local/aggregation/sum.php +++ b/reportbuilder/classes/local/aggregation/sum.php @@ -88,7 +88,7 @@ class sum extends base { * @param int $columntype * @return mixed */ - public static function format_value($value, array $values, array $callbacks, int $columntype) { + public function format_value($value, array $values, array $callbacks, int $columntype) { $firstvalue = reset($values); if ($firstvalue === null) { return null; diff --git a/reportbuilder/classes/local/report/column.php b/reportbuilder/classes/local/report/column.php index 81cc070a9d5..bc593a0cc51 100644 --- a/reportbuilder/classes/local/report/column.php +++ b/reportbuilder/classes/local/report/column.php @@ -76,7 +76,10 @@ final class column { private $callbacks = []; /** @var base|null $aggregation Aggregation type to apply to column */ - private $aggregation = null; + private base|null $aggregation = null; + + /** @var array[] $aggregationoptions Aggregation type options */ + private array $aggregationoptions = []; /** @var array $disabledaggregation Aggregation types explicitly disabled */ private $disabledaggregation = []; @@ -358,7 +361,7 @@ final class column { public function get_fields(): array { $fieldsalias = $this->get_fields_sql_alias(); - if (!empty($this->aggregation)) { + if ($this->aggregation !== null) { $fieldsaliassql = array_column($fieldsalias, 'sql'); $field = reset($fieldsalias); @@ -383,7 +386,7 @@ final class column { * @throws coding_exception */ private function get_field_aggregation_sql(array $sqlfields): string { - if (empty($this->aggregation)) { + if ($this->aggregation === null) { throw new coding_exception('Column aggregation is undefined'); } @@ -447,7 +450,7 @@ final class column { // 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()) { + if ($this->aggregation !== null && $this->aggregation::column_groupby()) { if ($usealias) { $this->set_groupby_sql($this->get_column_alias()); } else { @@ -504,18 +507,25 @@ final class column { * Set column aggregation type * * @param string|null $aggregation Type of aggregation, e.g. 'sum', 'count', etc + * @param array|null $options Aggregation type options * @return self * @throws coding_exception For invalid aggregation type, or one that is incompatible with column type */ - public function set_aggregation(?string $aggregation): self { - if (!empty($aggregation)) { - $aggregation = aggregation::get_full_classpath($aggregation); - if (!aggregation::valid($aggregation) || !$aggregation::compatible($this->get_type())) { + public function set_aggregation(?string $aggregation, ?array $options = null): self { + if ((string) $aggregation !== '') { + + // Convert aggregation to full class instance for internal storage. + $aggregationclasspath = aggregation::get_full_classpath($aggregation); + if (!aggregation::valid($aggregationclasspath) || !$aggregationclasspath::compatible($this->get_type())) { throw new coding_exception('Invalid column aggregation', $aggregation); } + + $options ??= $this->get_aggregation_options($aggregation); + $this->aggregation = new $aggregationclasspath($options); + } else { + $this->aggregation = null; } - $this->aggregation = $aggregation; return $this; } @@ -524,10 +534,32 @@ final class column { * * @return base|null */ - public function get_aggregation(): ?string { + public function get_aggregation(): ?base { return $this->aggregation; } + /** + * Set options for the given aggregation type + * + * @param string $aggregation Type of aggregation, e.g. 'sum', 'count', etc + * @param array $options Aggregation type options + * @return self + */ + public function set_aggregation_options(string $aggregation, array $options): self { + $this->aggregationoptions[$aggregation] = $options; + return $this; + } + + /** + * Get options for the given aggregation type + * + * @param string|null $aggregation Type of aggregation, e.g. 'sum', 'count', etc + * @return array + */ + public function get_aggregation_options(?string $aggregation): array { + return $this->aggregationoptions[$aggregation] ?? []; + } + /** * Set disabled aggregation methods for the column. Typically only those methods suitable for the current column type are * available: {@see aggregation::get_column_aggregations}, however in some cases we may want to disable specific methods @@ -585,7 +617,7 @@ final class column { public function get_is_sortable(): bool { // Defer sortable status to aggregation type if column is being aggregated. - if (!empty($this->aggregation)) { + if ($this->aggregation !== null) { return $this->aggregation::sortable($this->issortable); } @@ -673,9 +705,9 @@ final class column { $values = $this->get_values($row); // If column is being aggregated then defer formatting to them, otherwise loop through all column callbacks. - if (!empty($this->aggregation)) { + if ($this->aggregation !== null) { $value = self::get_default_value($values, $this->aggregation::get_column_type($this->get_type())); - $value = $this->aggregation::format_value($value, $values, $this->callbacks, $this->get_type()); + $value = $this->aggregation->format_value($value, $values, $this->callbacks, $this->get_type()); } else { $value = self::get_default_value($values, $this->get_type()); foreach ($this->callbacks as $callback) {