L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-82463 reportbuilder: change non-numeric entity column types.

Browse Source 
Where columns were previously of type `TYPE_INTEGER` or `TYPE_FLOAT`
but did not provide numeric data on output, we should change their
type to `TYPE_TEXT` (i.e. the default) to ensure that future work on
numeric aggregation doesn't affect them.
This commit is contained in:
Paul Holden
2024-07-12 17:17:57 +01:00
parent fd487cd3f2
commit a3c386af93
10 changed files with 20 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
admin/classes/reportbuilder/local/entities/task_log.php
+3 -9
View File
@@ -211,11 +211,8 @@ class task_log extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_field("{$tablealias}.pid")
->set_is_sortable(true)
// Although this is an integer column, it doesn't make sense to perform numeric aggregation on it.
->set_disabled_aggregation(['avg', 'count', 'countdistinct', 'max', 'min', 'sum']);
->set_is_sortable(true);
// Database column.
$columns[] = (new column(
@@ -224,17 +221,14 @@ class task_log extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_fields("{$tablealias}.dbreads, {$tablealias}.dbwrites")
->set_is_sortable(true, ["{$tablealias}.dbreads", "{$tablealias}.dbwrites"])
->add_callback(static function(int $value, stdClass $row): string {
->add_callback(static function($value, stdClass $row): string {
$output = '';
$output .= \html_writer::div(get_string('task_stats:dbreads', 'admin', $row->dbreads));
$output .= \html_writer::div(get_string('task_stats:dbwrites', 'admin', $row->dbwrites));
return $output;
})
// Although this is an integer column, it doesn't make sense to perform numeric aggregation on it.
->set_disabled_aggregation(['avg', 'count', 'countdistinct', 'max', 'min', 'sum']);
});
// Database reads column.
$columns[] = (new column(
admin/roles/classes/reportbuilder/local/entities/role_assignment.php
-2
View File
@@ -102,7 +102,6 @@ class role_assignment extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_TEXT)
->add_field("{$raalias}.component")
->set_is_sortable(true);
@@ -113,7 +112,6 @@ class role_assignment extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_field("{$raalias}.itemid")
->set_is_sortable(true);
badges/classes/reportbuilder/local/entities/badge.php
+4 -6
View File
@@ -175,13 +175,11 @@ class badge extends base {
->add_join("LEFT JOIN {context} {$contextalias}
ON {$contextalias}.contextlevel = " . CONTEXT_COURSE . "
AND {$contextalias}.instanceid = {$badgealias}.courseid")
->set_type(column::TYPE_INTEGER)
->add_fields("{$badgealias}.id, {$badgealias}.type, {$badgealias}.courseid")
->add_field($DB->sql_cast_to_char("{$badgealias}.imagecaption"), 'imagecaption')
->add_fields(context_helper::get_preload_record_columns_sql($contextalias))
->set_disabled_aggregation_all()
->add_callback(static function(?int $badgeid, stdClass $badge): string {
if (!$badgeid) {
->add_callback(static function($value, stdClass $badge): string {
if ($badge->id === null) {
return '';
}
if ($badge->type == BADGE_TYPE_SITE) {
@@ -191,7 +189,7 @@ class badge extends base {
$context = context_course::instance($badge->courseid);
}
$badgeimage = moodle_url::make_pluginfile_url($context->id, 'badges', 'badgeimage', $badgeid, '/', 'f2');
$badgeimage = moodle_url::make_pluginfile_url($context->id, 'badges', 'badgeimage', $badge->id, '/', 'f2');
return html_writer::img($badgeimage, $badge->imagecaption);
});
@@ -207,7 +205,7 @@ class badge extends base {
->set_is_sortable(true)
->add_callback(static function($language): string {
$languages = get_string_manager()->get_list_of_languages();
return $languages[$language] ?? $language ?? '';
return (string) ($languages[$language] ?? $language);
});
// Version.
comment/classes/reportbuilder/local/entities/comment.php
+1 -3
View File
@@ -193,10 +193,8 @@ class comment extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_fields("{$commentalias}.itemid")
->set_is_sortable(true)
->set_disabled_aggregation_all();
->set_is_sortable(true);
// Time created.
$columns[] = (new column(
files/classes/reportbuilder/local/entities/file.php
+1 -3
View File
@@ -306,10 +306,8 @@ class file extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_fields("{$filesalias}.itemid")
->set_is_sortable(true)
->set_disabled_aggregation_all();
->set_is_sortable(true);
// Time created.
$columns[] = (new column(
group/classes/reportbuilder/local/entities/group.php
+3 -9
View File
@@ -192,12 +192,9 @@ class group extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_fields("{$groupsalias}.visibility")
->set_is_sortable(true)
// It doesn't make sense to offer integer aggregation methods for this column.
->set_disabled_aggregation(['avg', 'max', 'min', 'sum'])
->set_callback(static function(?int $visibility): string {
->set_callback(static function(?string $visibility): string {
if ($visibility === null) {
return '';
}
@@ -209,7 +206,7 @@ class group extends base {
GROUPS_VISIBILITY_NONE => new lang_string('visibilitynone', 'core_group'),
];
return (string) ($options[$visibility] ?? $visibility);
return (string) ($options[(int) $visibility] ?? $visibility);
});
// Participation column.
@@ -231,12 +228,9 @@ class group extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_fields("{$groupsalias}.picture, {$groupsalias}.id, {$contextalias}.id AS contextid")
->set_is_sortable(false)
// It doesn't make sense to offer integer aggregation methods for this column.
->set_disabled_aggregation(['avg', 'max', 'min', 'sum'])
->set_callback(static function ($picture, stdClass $group): string {
->set_callback(static function($value, stdClass $group): string {
if (empty($group->picture)) {
return '';
}
lib/classes/reportbuilder/local/entities/context.php
+2 -5
View File
@@ -136,17 +136,14 @@ class context extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_fields("{$contextalias}.contextlevel")
->set_is_sortable(true)
// It doesn't make sense to offer integer aggregation methods for this column.
->set_disabled_aggregation(['avg', 'max', 'min', 'sum'])
->add_callback(static function(?int $level): string {
->add_callback(static function(?string $level): string {
if ($level === null) {
return '';
}
return context_helper::get_level_name($level);
return context_helper::get_level_name((int) $level);
});
// Path.
reportbuilder/classes/local/entities/user.php
+1 -4
View File
@@ -282,11 +282,8 @@ class user extends base {
))
->add_joins($this->get_joins())
->add_fields($userpictureselect)
->set_type(column::TYPE_INTEGER)
->set_is_sortable($this->is_sortable('picture'))
// It doesn't make sense to offer integer aggregation methods for this column.
->set_disabled_aggregation(['avg', 'max', 'min', 'sum'])
->add_callback(static function ($value, stdClass $row): string {
->add_callback(static function($value, stdClass $row): string {
global $OUTPUT;
return !empty($row->id) ? $OUTPUT->user_picture($row, ['link' => false, 'alttext' => false]) : '';
reportbuilder/classes/local/report/column.php
+4 -3
View File
@@ -213,7 +213,8 @@ final class column {
* Set the column type, if not called then the type will be assumed to be {@see TYPE_TEXT}
*
* The type of a column is used to cast the first column field passed to any callbacks {@see add_callback} as well as the
* aggregation options available for the column
* aggregation options available for the column. It should represent how the column content is returned from callbacks
*
*
* @param int $type
* @return self
@@ -643,11 +644,11 @@ final class column {
*/
public function format_value(array $row) {
$values = $this->get_values($row);
$value = self::get_default_value($values, $this->type);
$value = self::get_default_value($values, $this->get_type());
// If column is being aggregated then defer formatting to them, otherwise loop through all column callbacks.
if (!empty($this->aggregation)) {
$value = $this->aggregation::format_value($value, $values, $this->callbacks, $this->type);
$value = $this->aggregation::format_value($value, $values, $this->callbacks, $this->get_type());
} else {
foreach ($this->callbacks as $callback) {
[$callable, $arguments] = $callback;
tag/classes/reportbuilder/local/entities/instance.php
+1 -3
View File
@@ -184,10 +184,8 @@ class instance extends base {
$this->get_entity_name()
))
->add_joins($this->get_joins())
->set_type(column::TYPE_INTEGER)
->add_fields("{$instancealias}.itemid")
->set_is_sortable(true)
->set_disabled_aggregation_all();
->set_is_sortable(true);
// Time created.
$columns[] = (new column(