MDL-75359 reportbuilder: fixes for custom/user profile field elements.

Ensures cross-DB compatibility for all types of both custom and user
profile fields, specifically when they're filtered and/or aggregated.

Implement stress testing of both via appropriate report sources.

course/tests/reportbuilder/datasource/courses_test.php

@@ -19,7 +19,6 @@ declare(strict_types=1);
 namespace core_course\reportbuilder\datasource;
 
 use context_course;
-use core_customfield_generator;
 use core_reportbuilder_testcase;
 use core_reportbuilder_generator;
 use core_reportbuilder\local\filters\boolean_select;
@@ -426,13 +425,8 @@ class courses_test extends core_reportbuilder_testcase {
 
         $this->resetAfterTest();
 
-        /** @var core_customfield_generator $generator */
-        $generator = $this->getDataGenerator()->get_plugin_generator('core_customfield');
-        $customfieldcategory = $generator->create_category();
-        $generator->create_field(['categoryid' => $customfieldcategory->get('id'), 'shortname' => 'hi']);
-
         $category = $this->getDataGenerator()->create_category();
-        $course = $this->getDataGenerator()->create_course(['category' => $category->id, 'customfield_hi' => 'Hello']);
+        $course = $this->getDataGenerator()->create_course(['category' => $category->id]);
 
         $this->datasource_stress_test_columns(courses::class);
         $this->datasource_stress_test_columns_aggregation(courses::class);

reportbuilder/classes/local/helpers/custom_fields.php

@@ -142,6 +142,8 @@ class custom_fields {
      * @return column[]
      */
     public function get_columns(): array {
+        global $DB;
+
         $columns = [];
 
         $categorieswithfields = $this->handler->get_categories_with_fields();
@@ -151,34 +153,40 @@ class custom_fields {
                 $customdatatablealias = $this->get_table_alias($field);
 
                 $datacontroller = data_controller::create(0, null, $field);
+
                 $datafield = $datacontroller->datafield();
+                $datafieldsql = "{$customdatatablealias}.{$datafield}";
+
+                // Long text fields should be cast for Oracle, for aggregation support.
+                $columntype = $this->get_column_type($field, $datafield);
+                if ($columntype === column::TYPE_LONGTEXT && $DB->get_dbfamily() === 'oracle') {
+                    $datafieldsql = $DB->sql_order_by_text($datafieldsql, 1024);
+                }
 
                 // Select enough fields to re-create and format each custom field instance value.
-                $selectfields = "{$customdatatablealias}.{$datafield}, {$customdatatablealias}.id,
-                    {$customdatatablealias}.contextid";
+                $selectfields = "{$customdatatablealias}.id, {$customdatatablealias}.contextid";
                 if ($datafield === 'value') {
                     // We will take the format into account when displaying the individual values.
                     $selectfields .= ", {$customdatatablealias}.valueformat";
                 }
 
-                $columntype = $this->get_column_type($field, $datafield);
-
-                $newcolumn = (new column(
+                $columns[] = (new column(
                     'customfield_' . $field->get('shortname'),
                     new lang_string('customfieldcolumn', 'core_reportbuilder', $field->get_formatted_name()),
                     $this->entityname
                 ))
                     ->add_joins($this->get_joins())
                     ->add_join($this->get_table_join($field))
+                    ->add_field($datafieldsql, $datafield)
                     ->add_fields($selectfields)
                     ->set_type($columntype)
                     ->set_is_sortable($columntype !== column::TYPE_LONGTEXT)
-                    ->add_callback([$this, 'customfield_value'], $field)
+                    ->add_callback(static function($value, stdClass $row, field_controller $field): string {
+                        return (string) data_controller::create(0, $row, $field)->export_value();
+                    }, $field)
                     // Important. If the handler implements can_view() function, it will be called with parameter $instanceid=0.
                     // This means that per-instance access validation will be ignored.
                     ->set_is_available($this->handler->can_view($field, 0));
-
-                $columns[] = $newcolumn;
             }
         }
         return $columns;
@@ -223,6 +231,8 @@ class custom_fields {
      * @return filter[]
      */
     public function get_filters(): array {
+        global $DB;
+
         $filters = [];
 
         $categorieswithfields = $this->handler->get_categories_with_fields();
@@ -232,15 +242,20 @@ class custom_fields {
                 $customdatatablealias = $this->get_table_alias($field);
 
                 $datacontroller = data_controller::create(0, null, $field);
-                $datafield = $datacontroller->datafield();
-                $typeclass = $this->get_filter_class_type($datacontroller);
 
+                $datafield = $datacontroller->datafield();
+                $datafieldsql = "{$customdatatablealias}.{$datafield}";
+                if ($datafield === 'value') {
+                    $datafieldsql = $DB->sql_cast_to_char($datafieldsql);
+                }
+
+                $typeclass = $this->get_filter_class_type($datacontroller);
                 $filter = (new filter(
                     $typeclass,
                     'customfield_' . $field->get('shortname'),
                     new lang_string('customfieldcolumn', 'core_reportbuilder', $field->get_formatted_name()),
                     $this->entityname,
-                    "{$customdatatablealias}.{$datafield}"
+                    $datafieldsql
                 ))
                     ->add_joins($this->get_joins())
                     ->add_join($this->get_table_join($field));
@@ -298,17 +313,4 @@ class custom_fields {
 
         return $classtype;
     }
-
-    /**
-     * Format for custom fields value. We get the correct custom field value using export_value method.
-     *
-     * @param mixed $value Current value.
-     * @param stdClass $row Full row.
-     * @param field_controller $field Field controller object.
-     * @return mixed|null
-     */
-    public function customfield_value($value, stdClass $row, field_controller $field) {
-        $data = data_controller::create(0, (object)$row, $field);
-        return $data->export_value();
-    }
 }

reportbuilder/classes/local/helpers/user_profile_fields.php

@@ -157,13 +157,17 @@ class user_profile_fields {
         $columns = [];
         foreach ($this->userprofilefields as $profilefield) {
             $columntype = $this->get_user_field_type($profilefield->field->datatype);
-
             $columnfieldsql = $this->get_table_alias($profilefield) . '.data';
-            if ($DB->get_dbfamily() === 'oracle') {
+
+            // Numeric (checkbox/time) fields should be cast, as should all fields for Oracle, for aggregation support.
+            if ($columntype === column::TYPE_BOOLEAN || $columntype === column::TYPE_TIMESTAMP) {
+                $columnfieldsql = "CASE WHEN {$columnfieldsql} IS NULL THEN NULL ELSE " .
+                    $DB->sql_cast_char2int($columnfieldsql, true) . " END";
+            } else if ($DB->get_dbfamily() === 'oracle') {
                 $columnfieldsql = $DB->sql_order_by_text($columnfieldsql, 1024);
             }
 
-            $column = (new column(
+            $columns[] = (new column(
                 'profilefield_' . core_text::strtolower($profilefield->field->shortname),
                 new lang_string('customfieldcolumn', 'core_reportbuilder',
                     format_string($profilefield->field->name, true,
@@ -175,9 +179,15 @@ class user_profile_fields {
                 ->add_field($columnfieldsql, 'data')
                 ->set_type($columntype)
                 ->set_is_sortable($columntype !== column::TYPE_LONGTEXT)
-                ->add_callback([$this, 'format_profile_field'], $profilefield);
+                ->add_callback(static function($value, stdClass $row, profile_field_base $field): string {
+                    // Special handling of checkboxes, we want to display their boolean state rather than the input element itself.
+                    if (is_a($field, 'profile_field_checkbox')) {
+                        return format::boolean_as_text($value);
+                    }
 
-            $columns[] = $column;
+                    $field->data = $value;
+                    return (string) $field->display_data();
+                }, $profilefield);
         }
 
         return $columns;
@@ -274,22 +284,4 @@ class user_profile_fields {
         }
         return $customfieldtype;
     }
-
-    /**
-     * Formatter for a profile field. It formats the field according to its type.
-     *
-     * @param mixed $value
-     * @param stdClass $row
-     * @param profile_field_base $field
-     * @return string
-     */
-    public static function format_profile_field($value, stdClass $row, profile_field_base $field): string {
-        // Special handling of checkboxes, we want to display their boolean state rather than the input element itself.
-        if (is_a($field, 'profile_field_checkbox')) {
-            return format::boolean_as_text($value);
-        }
-
-        $field->data = $value;
-        return (string) $field->display_data();
-    }
 }

reportbuilder/tests/local/helpers/custom_fields_test.php

@@ -64,6 +64,9 @@ class custom_fields_test extends core_reportbuilder_testcase {
         $generator->create_field(
             ['categoryid' => $category->get('id'), 'type' => 'text', 'name' => 'Text', 'shortname' => 'text']);
 
+        $generator->create_field(
+            ['categoryid' => $category->get('id'), 'type' => 'textarea', 'name' => 'Textarea', 'shortname' => 'textarea']);
+
         $generator->create_field(
             ['categoryid' => $category->get('id'), 'type' => 'checkbox', 'name' => 'Checkbox', 'shortname' => 'checkbox']);
 
@@ -91,12 +94,13 @@ class custom_fields_test extends core_reportbuilder_testcase {
         $customfields = $this->generate_customfields();
         $columns = $customfields->get_columns();
 
-        $this->assertCount(4, $columns);
+        $this->assertCount(5, $columns);
         $this->assertContainsOnlyInstancesOf(column::class, $columns);
 
-        [$column0, $column1, $column2, $column3] = $columns;
-        $this->assertEqualsCanonicalizing(['Text', 'Checkbox', 'Date', 'Select'],
-            [$column0->get_title(), $column1->get_title(), $column2->get_title(), $column3->get_title()]);
+        [$column0, $column1, $column2, $column3, $column4] = $columns;
+        $this->assertEqualsCanonicalizing(['Text', 'Textarea', 'Checkbox', 'Date', 'Select'], [
+            $column0->get_title(), $column1->get_title(), $column2->get_title(), $column3->get_title(), $column4->get_title()
+        ]);
 
         $this->assertEquals(column::TYPE_TEXT, $column0->get_type());
         $this->assertEquals('course', $column0->get_entity_name());
@@ -144,12 +148,13 @@ class custom_fields_test extends core_reportbuilder_testcase {
         $customfields = $this->generate_customfields();
         $filters = $customfields->get_filters();
 
-        $this->assertCount(4, $filters);
+        $this->assertCount(5, $filters);
         $this->assertContainsOnlyInstancesOf(filter::class, $filters);
 
-        [$filter0, $filter1, $filter2, $filter3] = $filters;
-        $this->assertEqualsCanonicalizing(['Text', 'Checkbox', 'Date', 'Select'],
-            [$filter0->get_header(), $filter1->get_header(), $filter2->get_header(), $filter3->get_header()]);
+        [$filter0, $filter1, $filter2, $filter3, $filter4] = $filters;
+        $this->assertEqualsCanonicalizing(['Text', 'Textarea', 'Checkbox', 'Date', 'Select'], [
+            $filter0->get_header(), $filter1->get_header(), $filter2->get_header(), $filter3->get_header(), $filter4->get_header()
+        ]);
     }
 
     /**
@@ -162,6 +167,7 @@ class custom_fields_test extends core_reportbuilder_testcase {
 
         $course = $this->getDataGenerator()->create_course(['customfields' => [
             ['shortname' => 'text', 'value' => 'Hello'],
+            ['shortname' => 'textarea_editor', 'value' => ['text' => 'Goodbye', 'format' => FORMAT_MOODLE]],
             ['shortname' => 'checkbox', 'value' => true],
             ['shortname' => 'date', 'value' => 1669852800],
             ['shortname' => 'select', 'value' => 2],
@@ -174,6 +180,7 @@ class custom_fields_test extends core_reportbuilder_testcase {
         // Add user profile field columns to the report.
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:fullname']);
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:customfield_text']);
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:customfield_textarea']);
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:customfield_checkbox']);
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:customfield_date']);
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:customfield_select']);
@@ -183,6 +190,7 @@ class custom_fields_test extends core_reportbuilder_testcase {
         $this->assertEquals([
             $course->fullname,
             'Hello',
+            '<div class="text_to_html">Goodbye</div>',
             'Yes',
             userdate(1669852800),
             'Dog'
@@ -204,6 +212,14 @@ class custom_fields_test extends core_reportbuilder_testcase {
                 'course:customfield_text_operator' => text::IS_EQUAL_TO,
                 'course:customfield_text_value' => 'Goodbye',
             ], false],
+            'Filter by textarea custom field' => ['course:customfield_textarea', [
+                'course:customfield_textarea_operator' => text::IS_EQUAL_TO,
+                'course:customfield_textarea_value' => 'Goodbye',
+            ], true],
+            'Filter by textarea custom field (no match)' => ['course:customfield_textarea', [
+                'course:customfield_textarea_operator' => text::IS_EQUAL_TO,
+                'course:customfield_textarea_value' => 'Hello',
+            ], false],
             'Filter by checkbox custom field' => ['course:customfield_checkbox', [
                 'course:customfield_checkbox_operator' => boolean_select::CHECKED,
             ], true],
@@ -245,6 +261,7 @@ class custom_fields_test extends core_reportbuilder_testcase {
 
         $course = $this->getDataGenerator()->create_course(['customfields' => [
             ['shortname' => 'text', 'value' => 'Hello'],
+            ['shortname' => 'textarea_editor', 'value' => ['text' => 'Goodbye', 'format' => FORMAT_MOODLE]],
             ['shortname' => 'checkbox', 'value' => true],
             ['shortname' => 'date', 'value' => 1669852800],
             ['shortname' => 'select', 'value' => 2],
@@ -268,5 +285,31 @@ class custom_fields_test extends core_reportbuilder_testcase {
             $this->assertEmpty($content);
         }
     }
+
+    /**
+     * Stress test course datasource using custom fields
+     *
+     * In order to execute this test PHPUNIT_LONGTEST should be defined as true in phpunit.xml or directly in config.php
+     */
+    public function test_stress_datasource(): void {
+        if (!PHPUNIT_LONGTEST) {
+            $this->markTestSkipped('PHPUNIT_LONGTEST is not defined');
+        }
+
+        $this->resetAfterTest();
+
+        $this->generate_customfields();
+        $course = $this->getDataGenerator()->create_course(['customfields' => [
+            ['shortname' => 'text', 'value' => 'Hello'],
+            ['shortname' => 'textarea_editor', 'value' => ['text' => 'Goodbye', 'format' => FORMAT_MOODLE]],
+            ['shortname' => 'checkbox', 'value' => true],
+            ['shortname' => 'date', 'value' => 1669852800],
+            ['shortname' => 'select', 'value' => 2],
+        ]]);
+
+        $this->datasource_stress_test_columns(courses::class);
+        $this->datasource_stress_test_columns_aggregation(courses::class);
+        $this->datasource_stress_test_conditions(courses::class, 'course:idnumber');
+    }
 }
 

reportbuilder/tests/local/helpers/user_profile_fields_test.php

@@ -350,4 +350,31 @@ class user_profile_fields_test extends core_reportbuilder_testcase {
         $this->assertCount(1, $content);
         $this->assertEquals($expectmatchuser, reset($content[0]));
     }
+
+    /**
+     * Stress test user datasource using profile fields
+     *
+     * In order to execute this test PHPUNIT_LONGTEST should be defined as true in phpunit.xml or directly in config.php
+     */
+    public function test_stress_datasource(): void {
+        if (!PHPUNIT_LONGTEST) {
+            $this->markTestSkipped('PHPUNIT_LONGTEST is not defined');
+        }
+
+        $this->resetAfterTest();
+
+        $userprofilefields = $this->generate_userprofilefields();
+        $user = $this->getDataGenerator()->create_user([
+            'profile_field_checkbox' => true,
+            'profile_field_datetime' => '2021-12-09',
+            'profile_field_menu' => 'Dog',
+            'profile_field_Social' => '12345',
+            'profile_field_text' => 'Hello',
+            'profile_field_textarea' => 'Goodbye',
+        ]);
+
+        $this->datasource_stress_test_columns(users::class);
+        $this->datasource_stress_test_columns_aggregation(users::class);
+        $this->datasource_stress_test_conditions(users::class, 'user:idnumber');
+    }
 }

user/tests/reportbuilder/datasource/users_test.php

@@ -441,8 +441,7 @@ class users_test extends core_reportbuilder_testcase {
 
         $this->resetAfterTest();
 
-        $this->getDataGenerator()->create_custom_profile_field(['datatype' => 'text', 'name' => 'Hi', 'shortname' => 'hi']);
-        $user = $this->getDataGenerator()->create_user(['profile_field_hi' => 'Hello']);
+        $user = $this->getDataGenerator()->create_user();
 
         $this->datasource_stress_test_columns(users::class);
         $this->datasource_stress_test_columns_aggregation(users::class);