mirror of
https://github.com/moodle/moodle.git
synced 2025-08-05 00:46:50 +02:00
MDL-73493 reportbuilder: correct handling of empty string in filters.
This is specifically for Oracle, which treats empty strings and NULL in an inconsistent manner unless passed as query parameters. Increase test coverage of the same. Co-authored-by: Carlos Castillo <carlos.castillo@moodle.com>
This commit is contained in:
Paul Holden
parent
3d8620854a
commit
628541b5e1
4 changed files with 136 additions and 14 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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,25 +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("COALESCE({$field}, 0)");
|
||||
$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;
|
||||
$field = "COALESCE({$field}, '')";
|
||||
|
||||
$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("COALESCE({$field}, '')", 255);
|
||||
$classname = text::class;
|
||||
|
||||
$emptyparam = database::generate_param_name();
|
||||
$fieldsql = "COALESCE(" . $DB->sql_compare_text($field, 255) . ", :{$emptyparam})";
|
||||
$params[$emptyparam] = '';
|
||||
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
@ -184,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} " .
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
*
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue