MDL-75191 reportbuilder: use column callbacks in avg/sum aggregation.

When aggregating a column using either method, for numeric types we
should execute column callbacks to ensure consistent formatting.
This commit is contained in:
Paul Holden 2022-07-07 12:55:38 +01:00
parent 17ee072693
commit 4b8f076560
5 changed files with 114 additions and 7 deletions

View File

@ -67,13 +67,19 @@ class avg extends base {
/**
* Return formatted value for column when applying aggregation
*
* For boolean columns we return the average of the values (0..1), numeric columns execute original callbacks if present
*
* @param mixed $value
* @param array $values
* @param array $callbacks
* @param int $columntype
* @return string
* @return mixed
*/
public static function format_value($value, array $values, array $callbacks, int $columntype): string {
return format_float((float) reset($values), 1);
public static function format_value($value, array $values, array $callbacks, int $columntype) {
if ($columntype === column::TYPE_BOOLEAN || empty($callbacks)) {
return format_float((float) reset($values), 1);
}
return parent::format_value($value, $values, $callbacks, $columntype);
}
}

View File

@ -67,13 +67,20 @@ class sum extends base {
/**
* Return formatted value for column when applying aggregation
*
* For boolean columns we return the sum of the true values, numeric columns execute original callbacks if present
*
* @param mixed $value
* @param array $values
* @param array $callbacks
* @param int $columntype
* @return int
* @return mixed
*/
public static function format_value($value, array $values, array $callbacks, int $columntype): int {
return (int) reset($values);
public static function format_value($value, array $values, array $callbacks, int $columntype) {
if ($columntype === column::TYPE_BOOLEAN || empty($callbacks)) {
$decimalpoints = (int) ($columntype === column::TYPE_FLOAT);
return format_float((float) reset($values), $decimalpoints);
}
return parent::format_value($value, $values, $callbacks, $columntype);
}
}

View File

@ -479,7 +479,7 @@ final class column {
* fields, and $additionalarguments are those passed on from this method):
*
* The type of the $value parameter passed to the callback is determined by calling {@see set_type}, this type is preserved
* if the column is part of a report source and is aggregated using one of the "Group concatenation" methods
* if the column is part of a report source and is being aggregated
*
* function($value, stdClass $row[, $additionalarguments]): string
*

View File

@ -20,6 +20,8 @@ namespace core_reportbuilder\local\aggregation;
use core_reportbuilder_testcase;
use core_reportbuilder_generator;
use core_reportbuilder\manager;
use core_reportbuilder\local\report\column;
use core_user\reportbuilder\datasource\users;
defined('MOODLE_INTERNAL') || die();
@ -73,4 +75,49 @@ class avg_test extends core_reportbuilder_testcase {
],
], $content);
}
/**
* 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', 'suspended' => 1]);
$this->getDataGenerator()->create_user(['firstname' => 'Bob', 'suspended' => 0]);
/** @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:suspended'])
->set('aggregation', avg::get_class_name())
->update();
// Set callback to format the column (hack column definition to ensure callbacks are executed).
$instance = manager::get_report_from_persistent($report);
$instance->get_column('user:suspended')
->set_type(column::TYPE_FLOAT)
->set_callback(static function(float $value): string {
return number_format($value, 1) . ' suspended';
});
$content = $this->get_custom_report_content($report->get('id'));
$this->assertEquals([
[
'c0_firstname' => 'Admin',
'c1_suspended' => '0.0 suspended',
],
[
'c0_firstname' => 'Bob',
'c1_suspended' => '0.5 suspended',
],
], $content);
}
}

View File

@ -20,6 +20,8 @@ namespace core_reportbuilder\local\aggregation;
use core_reportbuilder_testcase;
use core_reportbuilder_generator;
use core_reportbuilder\manager;
use core_reportbuilder\local\report\column;
use core_user\reportbuilder\datasource\users;
defined('MOODLE_INTERNAL') || die();
@ -74,4 +76,49 @@ class sum_test extends core_reportbuilder_testcase {
],
], $content);
}
/**
* 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', 'suspended' => 1]);
$this->getDataGenerator()->create_user(['firstname' => 'Bob', 'suspended' => 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:suspended'])
->set('aggregation', sum::get_class_name())
->update();
// Set callback to format the column (hack column definition to ensure callbacks are executed).
$instance = manager::get_report_from_persistent($report);
$instance->get_column('user:suspended')
->set_type(column::TYPE_INTEGER)
->set_callback(static function(int $value): string {
return "{$value} suspended";
});
$content = $this->get_custom_report_content($report->get('id'));
$this->assertEquals([
[
'c0_firstname' => 'Admin',
'c1_suspended' => '0 suspended',
],
[
'c0_firstname' => 'Bob',
'c1_suspended' => '2 suspended',
],
], $content);
}
}