MDL-84129 reportbuilder: improve handling of alternate name fields.

It's possible the first returned field contains a null value, while subsequent fields may be non-null - in this case we should still populate the fullname in column callback.
paulholden · Jan 13, 2025 · d24b3b0 · d24b3b0
1 parent f4f1666
commit d24b3b0
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 7 deletions.
diff --git a/reportbuilder/classes/local/entities/user.php b/reportbuilder/classes/local/entities/user.php
@@ -203,10 +203,11 @@ protected function get_all_columns(): array {
         ))
             ->add_joins($this->get_joins())
             ->add_fields($fullnameselect)
-            ->set_type(column::TYPE_TEXT)
             ->set_is_sortable($this->is_sortable('fullname'), $fullnamesort)
-            ->add_callback(static function(?string $value, stdClass $row) use ($viewfullnames): string {
-                if ($value === null) {
+            ->add_callback(static function($value, stdClass $row) use ($viewfullnames): string {
+
+                // Ensure we have at least one field present.
+                if (count(array_filter((array) $row, fn($field) => $field !== null)) === 0) {
                     return '';
                 }
 
@@ -234,12 +235,12 @@ protected function get_all_columns(): array {
                 ->add_joins($this->get_joins())
                 ->add_fields($fullnameselect)
                 ->add_field("{$usertablealias}.id")
-                ->set_type(column::TYPE_TEXT)
                 ->set_is_sortable($this->is_sortable($fullnamefield), $fullnamesort)
-                ->add_callback(static function(?string $value, stdClass $row) use ($fullnamefield, $viewfullnames): string {
+                ->add_callback(static function($value, stdClass $row) use ($fullnamefield, $viewfullnames): string {
                     global $OUTPUT;
 
-                    if ($value === null) {
+                    // Ensure we have at least one field present.
+                    if (count(array_filter((array) $row, fn($field) => $field !== null)) === 0) {
                         return '';
                     }
 
@@ -263,7 +264,7 @@ protected function get_all_columns(): array {
                             fullname($row, $viewfullnames));
                     }
 
-                    return $value;
+                    return (string) $value;
                 });
 
             // Picture fields need some more data.

diff --git a/user/tests/reportbuilder/datasource/users_test.php b/user/tests/reportbuilder/datasource/users_test.php
@@ -209,6 +209,56 @@ public function test_datasource_non_default_columns(): void {
         $this->assertEquals($cohort->name, $cohortname);
     }
 
+    /**
+     * Test fullname columns when alternative fullname format is configured
+     */
+    public function test_datasource_alternative_fullname_columns(): void {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        set_config('alternativefullnameformat', '(alternatename) firstname lastname');
+
+        $this->getDataGenerator()->create_user(['firstname' => 'John', 'lastname' => 'Smith', 'alternatename' => 'JS']);
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+        $report = $generator->create_report(['name' => 'Users', 'source' => users::class, 'default' => 0]);
+
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname', 'sortenabled' => 1]);
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullnamewithlink']);
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullnamewithpicture']);
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullnamewithpicturelink']);
+
+        $content = $this->get_custom_report_content($report->get('id'));
+        $this->assertCount(2, $content);
+
+        // Admin row.
+        [
+            $fullname,
+            $fullnamewithlink,
+            $fullnamewithpicture,
+            $fullnamewithpicturelink
+        ] = array_values($content[0]);
+
+        $this->assertEquals('Admin User', $fullname);
+        $this->assertStringContainsString('Admin User', $fullnamewithlink);
+        $this->assertStringContainsString('Admin User', $fullnamewithpicture);
+        $this->assertStringContainsString('Admin User', $fullnamewithpicturelink);
+
+        // User row.
+        [
+            $fullname,
+            $fullnamewithlink,
+            $fullnamewithpicture,
+            $fullnamewithpicturelink
+        ] = array_values($content[1]);
+
+        $this->assertEquals('(JS) John Smith', $fullname);
+        $this->assertStringContainsString('(JS) John Smith', $fullnamewithlink);
+        $this->assertStringContainsString('(JS) John Smith', $fullnamewithpicture);
+        $this->assertStringContainsString('(JS) John Smith', $fullnamewithpicturelink);
+    }
+
     /**
      * Data provider for {@see test_datasource_filters}
      *