MDL-74656 reportbuilder: ensure nulls preserved in column callbacks.

When concatenating column fields in order to perform aggregation on
them (e.g. group concatenation), we need to preserve all null values
in the data passed to each column callback.

Co-authored-by: Carlos Castillo <[email protected]>

reportbuilder/classes/local/aggregation/base.php

@@ -81,9 +81,14 @@ abstract class base {
      *
      * @param string[] $sqlfields
      * @param string $delimeter
+     * @param string $coalescechar
      * @return string
      */
-    final protected static function get_column_fields_concat(array $sqlfields, string $delimeter = ','): string {
+    final protected static function get_column_fields_concat(
+        array $sqlfields,
+        string $delimeter = ',',
+        string $coalescechar = ' '
+    ): string {
         global $DB;
 
         // We need to ensure all values are char.
@@ -95,8 +100,8 @@ abstract class base {
                 $sqlfield = $DB->sql_cast_to_char($sqlfield);
             }
 
-            // Coalesce all the SQL fields, to remove all nulls.
-            $concatfields[] = "COALESCE({$sqlfield}, ' ')";
+            // Coalesce all the SQL fields. Ensure cross-DB compatibility, and that we always get string data back.
+            $concatfields[] = "COALESCE({$sqlfield}, '{$coalescechar}')";
             $concatfields[] = "'{$delimeter}'";
         }
 

reportbuilder/classes/local/aggregation/groupconcat.php

@@ -34,6 +34,9 @@ class groupconcat extends base {
     /** @var string Character to use as a delimeter between column fields */
     protected const COLUMN_FIELD_DELIMETER = '<|>';
 
+    /** @var string Character to use a null coalesce value */
+    protected const COLUMN_NULL_COALESCE = '<^>';
+
     /** @var string Character to use as a delimeter between field values */
     protected const FIELD_VALUE_DELIMETER = '<,>';
 
@@ -79,7 +82,7 @@ class groupconcat extends base {
             return parent::get_column_field_sql($sqlfields);
         }
 
-        return self::get_column_fields_concat($sqlfields, self::COLUMN_FIELD_DELIMETER);
+        return self::get_column_fields_concat($sqlfields, self::COLUMN_FIELD_DELIMETER, self::COLUMN_NULL_COALESCE);
     }
 
     /**
@@ -124,7 +127,11 @@ class groupconcat extends base {
                 continue;
             }
 
-            $originalvalues = array_combine($valuenames, $valuedata);
+            // Re-construct original values, also ensuring any nulls contained within are restored.
+            $originalvalues = array_map(static function(string $value): ?string {
+                return $value === self::COLUMN_NULL_COALESCE ? null : $value;
+            }, array_combine($valuenames, $valuedata));
+
             $originalvalue = column::get_default_value($originalvalues, $columntype);
 
             // Once we've re-constructed each value, we can apply callbacks to it.

reportbuilder/tests/local/aggregation/groupconcat_test.php

@@ -18,6 +18,8 @@ declare(strict_types=1);
 
 namespace core_reportbuilder\local\aggregation;
 
+use core_badges_generator;
+use core_badges\reportbuilder\datasource\badges;
 use core_reportbuilder_testcase;
 use core_reportbuilder_generator;
 use core_user\reportbuilder\datasource\users;
@@ -141,4 +143,53 @@ class groupconcat_test extends core_reportbuilder_testcase {
             ],
         ], $content);
     }
+
+    /**
+     * Test aggregation when applied to column with callback that expects/handles null values
+     */
+    public function test_datasource_aggregate_column_callback_with_null(): void {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $userone = $this->getDataGenerator()->create_user(['description' => 'First user']);
+        $usertwo = $this->getDataGenerator()->create_user(['description' => 'Second user']);
+
+        /** @var core_badges_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_badges');
+
+        // Create course badge, issue to both users.
+        $badgeone = $generator->create_badge(['name' => 'First badge']);
+        $badgeone->issue($userone->id, true);
+        $badgeone->issue($usertwo->id, true);
+
+        // Create second badge, without issuing to anyone.
+        $badgetwo = $generator->create_badge(['name' => 'Second badge']);
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+        $report = $generator->create_report(['name' => 'Badges', 'source' => badges::class, 'default' => 0]);
+
+        // First column, sorted.
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'badge:name'])
+            ->set('sortenabled', true)
+            ->update();
+
+        // This is the column we'll aggregate.
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:description'])
+            ->set('aggregation', groupconcat::get_class_name())
+            ->update();
+
+        // Assert description column was aggregated, with callbacks accounting for null values.
+        $content = $this->get_custom_report_content($report->get('id'));
+        $this->assertEquals([
+            [
+                'c0_name' => $badgeone->name,
+                'c1_description' => "{$userone->description}, {$usertwo->description}",
+            ],
+            [
+                'c0_name' => $badgetwo->name,
+                'c1_description' => '',
+            ],
+        ], $content);
+    }
 }