From 4e8013e418d7ed1f87ce35bf2db30ca32d9685cc Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Tue, 2 Aug 2022 17:21:04 +0200 Subject: [PATCH 1/2] MDL-75358 reportbuilder: display nulls as empty cells for numeric/bool --- reportbuilder/classes/local/helpers/format.php | 12 +++++++++--- reportbuilder/classes/local/report/column.php | 4 ++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/reportbuilder/classes/local/helpers/format.php b/reportbuilder/classes/local/helpers/format.php index 2662cd5bee8..722856acecd 100644 --- a/reportbuilder/classes/local/helpers/format.php +++ b/reportbuilder/classes/local/helpers/format.php @@ -35,7 +35,7 @@ class format { /** * Returns formatted date. * - * @param int $value Unix timestamp + * @param int|null $value Unix timestamp * @param stdClass $row * @param string|null $format Format string for strftime * @return string @@ -47,20 +47,26 @@ class format { /** * Returns yes/no string depending on the given value * - * @param bool $value + * @param bool|null $value * @return string */ public static function boolean_as_text($value): string { + if ($value === null) { + return ''; + } return (bool) $value ? get_string('yes') : get_string('no'); } /** * Returns float value as a percentage * - * @param float $value + * @param float|null $value * @return string */ public static function percent($value): string { + if ($value === null) { + return ''; + } return get_string('percents', 'moodle', format_float((float) $value)); } } diff --git a/reportbuilder/classes/local/report/column.php b/reportbuilder/classes/local/report/column.php index d0177c3e4e6..159c80caeeb 100644 --- a/reportbuilder/classes/local/report/column.php +++ b/reportbuilder/classes/local/report/column.php @@ -481,6 +481,7 @@ final class column { * The type of the $value parameter passed to the callback is determined by calling {@see set_type}, however note that * if the column is part of a report source and can be aggregated using one of the "Group concatenation" methods then the * type should be omitted if it's not string + * For entities that can to be left joined to a report, the first argument to their column callbacks must be nullable. * * function($value, stdClass $row[, $additionalarguments]): string * @@ -649,6 +650,9 @@ final class column { */ private function get_default_value(array $values) { $value = reset($values); + if ($value === null) { + return $value; + } // Ensure default value is cast to it's strict type. switch ($this->get_type()) { From be60921e145a1c1478743472c5a017d1bb80b406 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Tue, 2 Aug 2022 18:26:37 +0200 Subject: [PATCH 2/2] MDL-75358 reportbuilder: display nulls as empty cells in aggregation --- reportbuilder/classes/local/aggregation/avg.php | 3 +++ reportbuilder/classes/local/aggregation/groupconcat.php | 6 +++++- reportbuilder/classes/local/aggregation/percent.php | 3 +++ reportbuilder/classes/local/aggregation/sum.php | 3 +++ .../tests/local/helpers/user_profile_fields_test.php | 2 +- 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/reportbuilder/classes/local/aggregation/avg.php b/reportbuilder/classes/local/aggregation/avg.php index 16ff16a9c4b..d929ed3b6e7 100644 --- a/reportbuilder/classes/local/aggregation/avg.php +++ b/reportbuilder/classes/local/aggregation/avg.php @@ -73,6 +73,9 @@ class avg extends base { * @return mixed */ public static function format_value($value, array $values, array $callbacks) { + if (reset($values) === null) { + return null; + } return format_float((float) reset($values), 1); } } diff --git a/reportbuilder/classes/local/aggregation/groupconcat.php b/reportbuilder/classes/local/aggregation/groupconcat.php index 77c27b579f4..04d29d1634d 100644 --- a/reportbuilder/classes/local/aggregation/groupconcat.php +++ b/reportbuilder/classes/local/aggregation/groupconcat.php @@ -110,6 +110,10 @@ class groupconcat extends base { * @return mixed */ public static function format_value($value, array $values, array $callbacks) { + $firstvalue = reset($values); + if ($firstvalue === null) { + return ''; + } $formattedvalues = []; // Store original names of all values that would be present without aggregation. @@ -117,7 +121,7 @@ class groupconcat extends base { $valuenamescount = count($valuenames); // Loop over each extracted value from the concatenated string. - $values = explode(self::FIELD_VALUE_DELIMETER, (string) reset($values)); + $values = explode(self::FIELD_VALUE_DELIMETER, (string)$firstvalue); foreach ($values as $value) { // Ensure we have equal number of value names/data, account for truncation by DB. diff --git a/reportbuilder/classes/local/aggregation/percent.php b/reportbuilder/classes/local/aggregation/percent.php index 29b9878196c..d4c84c35f31 100644 --- a/reportbuilder/classes/local/aggregation/percent.php +++ b/reportbuilder/classes/local/aggregation/percent.php @@ -72,6 +72,9 @@ class percent extends base { * @return mixed */ public static function format_value($value, array $values, array $callbacks) { + if (reset($values) === null) { + return ''; + } return format::percent(reset($values)); } } diff --git a/reportbuilder/classes/local/aggregation/sum.php b/reportbuilder/classes/local/aggregation/sum.php index 119fb52f508..90fa320c2c4 100644 --- a/reportbuilder/classes/local/aggregation/sum.php +++ b/reportbuilder/classes/local/aggregation/sum.php @@ -73,6 +73,9 @@ class sum extends base { * @return mixed */ public static function format_value($value, array $values, array $callbacks) { + if (reset($values) === null) { + return ''; + } return (int) reset($values); } } diff --git a/reportbuilder/tests/local/helpers/user_profile_fields_test.php b/reportbuilder/tests/local/helpers/user_profile_fields_test.php index 13af297915e..64f0cd27e1b 100644 --- a/reportbuilder/tests/local/helpers/user_profile_fields_test.php +++ b/reportbuilder/tests/local/helpers/user_profile_fields_test.php @@ -206,7 +206,7 @@ class user_profile_fields_test extends core_reportbuilder_testcase { $this->assertEquals([ [ 'c0_firstname' => 'Admin', - 'c1_data' => 'No', + 'c1_data' => '', 'c2_data' => 'Not set', 'c3_data' => '', 'c4_data' => '',