Merge branch 'MDL-73493' of git://github.com/paulholden/moodle

reportbuilder/classes/local/filters/text.php

@@ -155,13 +155,15 @@ class text extends base {
                 $value = $DB->sql_like_escape($value);
                 $params[$name] = "%$value";
                 break;
-            case self::IS_EMPTY: // Note we also account for field not existing here.
-                $res = "COALESCE({$fieldsql}, '') = :{$name}";
-                $params[$name] = '';
+            case self::IS_EMPTY:
+                $paramempty = database::generate_param_name();
+                $res = "COALESCE({$fieldsql}, :{$paramempty}) = :{$name}";
+                $params[$paramempty] = $params[$name] = '';
                 break;
             case self::IS_NOT_EMPTY:
-                $res = "COALESCE({$fieldsql}, '') != :{$name}";
-                $params[$name] = '';
+                $paramempty = database::generate_param_name();
+                $res = "COALESCE({$fieldsql}, :{$paramempty}) != :{$name}";
+                $params[$paramempty] = $params[$name] = '';
                 break;
             default:
                 // Filter configuration is invalid. Ignore the filter.

reportbuilder/classes/local/helpers/user_profile_fields.php

@@ -23,6 +23,7 @@ use core_reportbuilder\local\filters\boolean_select;
 use core_reportbuilder\local\filters\date;
 use core_reportbuilder\local\filters\select;
 use core_reportbuilder\local\filters\text;
+use core_reportbuilder\local\helpers\database;
 use core_reportbuilder\local\report\column;
 use core_reportbuilder\local\report\filter;
 use lang_string;
@@ -155,24 +156,34 @@ class user_profile_fields {
         foreach ($this->userprofilefields as $profilefield) {
             $userinfotablealias = database::generate_alias();
             $field = "{$userinfotablealias}.data";
+            $params = [];
 
             switch ($profilefield->field->datatype) {
                 case 'checkbox':
                     $classname = boolean_select::class;
-                    $field = $DB->sql_cast_char2int($field);
+                    $fieldsql = "COALESCE(" . $DB->sql_cast_char2int($field, true) . ", 0)";
                     break;
                 case 'datetime':
                     $classname = date::class;
-                    $field = $DB->sql_cast_char2int($field);
+                    $fieldsql = $DB->sql_cast_char2int($field);
                     break;
                 case 'menu':
                     $classname = select::class;
+
+                    $emptyparam = database::generate_param_name();
+                    $fieldsql = "COALESCE(" . $DB->sql_compare_text($field, 255) . ", :{$emptyparam})";
+                    $params[$emptyparam] = '';
+
                     break;
                 case 'text':
                 case 'textarea':
                 default:
-                    $field = $DB->sql_compare_text($field, 255);
                     $classname = text::class;
+
+                    $emptyparam = database::generate_param_name();
+                    $fieldsql = "COALESCE(" . $DB->sql_compare_text($field, 255) . ", :{$emptyparam})";
+                    $params[$emptyparam] = '';
+
                     break;
             }
 
@@ -183,7 +194,8 @@ class user_profile_fields {
                     format_string($profilefield->field->name, true,
                         ['escape' => false, 'context' => context_system::instance()])),
                 $this->entityname,
-                $field
+                $fieldsql,
+                $params
             ))
                 ->add_joins($this->get_joins())
                 ->add_join("LEFT JOIN {user_info_data} {$userinfotablealias} " .

reportbuilder/tests/local/entities/user_test.php

@@ -19,12 +19,13 @@ declare(strict_types=1);
 namespace core_reportbuilder\local\entities;
 
 use advanced_testcase;
-use core_reportbuilder\local\filters\boolean_select;
-use core_reportbuilder\local\filters\date;
 use moodle_url;
 use core_reportbuilder\manager;
 use core_reportbuilder\testable_system_report_table;
 use core_reportbuilder\user_entity_report;
+use core_reportbuilder\local\filters\boolean_select;
+use core_reportbuilder\local\filters\date;
+use core_reportbuilder\local\filters\select;
 use core_reportbuilder\local\filters\text;
 use core_reportbuilder\local\helpers\user_filter_manager;
 
@@ -251,6 +252,60 @@ class user_test extends advanced_testcase {
         ], array_column($tablerows, 'fullname'));
     }
 
+    /**
+     * Data provider for {@see test_userprofilefield_filter_empty}
+     *
+     * @return array
+     */
+    public function userprofilefield_filter_empty_provider(): array {
+        return [
+            ['checkbox', 1, boolean_select::NOT_CHECKED],
+            ['text', 'Hello', text::IS_EMPTY],
+            ['text', 'Hello', text::IS_NOT_EQUAL_TO],
+            ['text', 'Hello', text::DOES_NOT_CONTAIN],
+            ['menu', 'one', select::NOT_EQUAL_TO, "one\ntwo"],
+        ];
+    }
+
+    /**
+     * Test filtering report by a user profile field with negated operators (contains the "empty" value appropriate to the field
+     * type, or is not set/null)
+     *
+     * @param string $datatype
+     * @param mixed $userfieldvalue
+     * @param int $operator
+     * @param string $datatypeparam1
+     *
+     * @dataProvider userprofilefield_filter_empty_provider
+     */
+    public function test_userprofilefield_filter_empty(string $datatype, $userfieldvalue, int $operator,
+            string $datatypeparam1 = ''): void {
+
+        $this->resetAfterTest();
+
+        $this->getDataGenerator()->create_custom_profile_field([
+            'datatype' => $datatype,
+            'shortname' => 'test',
+            'name' => 'My test field',
+            'param1' => $datatypeparam1,
+        ]);
+
+        // At this point, the custom profile field was created after the admin account, so the value will be not set/null.
+        $filtervalues = [
+            'user:profilefield_test_operator' => $operator,
+            'user:profilefield_test_value' => $userfieldvalue,
+        ];
+
+        // Create a user who does have the field set.
+        $user = $this->getDataGenerator()->create_user(['profile_field_test' => $userfieldvalue]);
+
+        $rows = $this->get_report_table_rows($filtervalues);
+
+        $usernames = array_column($rows, 'username');
+        $this->assertContains('admin', $usernames);
+        $this->assertNotContains($user->username, $usernames);
+    }
+
     /**
      * Data provider for {@see test_get_name_fields_select}
      *

reportbuilder/tests/local/filters/text_test.php

@@ -53,8 +53,6 @@ class text_test extends advanced_testcase {
             [text::STARTS_WITH, 'sunlight', false],
             [text::ENDS_WITH, 'looking for?', true],
             [text::ENDS_WITH, 'your heart', false],
-            [text::IS_EMPTY, null, false],
-            [text::IS_NOT_EMPTY, null, true],
         ];
     }
 
@@ -97,4 +95,60 @@ class text_test extends advanced_testcase {
             $this->assertNotContains($course->fullname, $fullnames);
         }
     }
+
+    /**
+     * Data provider for {@see test_get_sql_filter_empty}
+     *
+     * @return array
+     */
+    public function get_sql_filter_empty_provider(): array {
+        return [
+            [text::IS_EMPTY, null, true],
+            [text::IS_EMPTY, '', true],
+            [text::IS_EMPTY, 'hola', false],
+            [text::IS_NOT_EMPTY, null, false],
+            [text::IS_NOT_EMPTY, '', false],
+            [text::IS_NOT_EMPTY, 'hola', true],
+        ];
+    }
+
+    /**
+     * Test getting filter SQL using the {@see text::IS_EMPTY} and {@see text::IS_NOT_EMPTY} operators
+     *
+     * @param int $operator
+     * @param string|null $profilefieldvalue
+     * @param bool $expectmatch
+     *
+     * @dataProvider get_sql_filter_empty_provider
+     */
+    public function test_get_sql_filter_empty(int $operator, ?string $profilefieldvalue, bool $expectmatch): void {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // We are using the user.moodlenetprofile field because it is nullable.
+        $user = $this->getDataGenerator()->create_user([
+            'moodlenetprofile' => $profilefieldvalue,
+        ]);
+
+        $filter = new filter(
+            text::class,
+            'test',
+            new lang_string('user'),
+            'testentity',
+            'moodlenetprofile'
+        );
+
+        // Create instance of our filter, passing given operator.
+        [$select, $params] = text::create($filter)->get_sql_filter([
+            $filter->get_unique_identifier() . '_operator' => $operator,
+        ]);
+
+        $usernames = $DB->get_fieldset_select('user', 'username', $select, $params);
+        if ($expectmatch) {
+            $this->assertContains($user->username, $usernames);
+        } else {
+            $this->assertNotContains($user->username, $usernames);
+        }
+    }
 }