This commit is contained in:
Huong Nguyen
2025-02-24 10:09:15 +07:00
14 changed files with 67 additions and 30 deletions
@@ -0,0 +1,8 @@
issueNumber: MDL-82913
notes:
core_reportbuilder:
- message: >-
When the `select` filter contains upto two options only then the
operator field is removed, switching to a simpler value selection field
only (this may affect your Behat scenarios)
type: changed
@@ -0,0 +1,7 @@
issueNumber: MDL-84213
notes:
core_reportbuilder:
- message: >-
The `select` filter type is now stricter in it's filtering, in that it
will now discard values that aren't present in available filter options
type: changed
@@ -149,7 +149,7 @@ final class roles_test extends core_reportbuilder_testcase {
], true],
'Filter role name (no match)' => ['role:name', [
'role:name_operator' => select::EQUAL_TO,
'role:name_value' => -1,
'role:name_value' => $DB->get_field('role', 'id', ['shortname' => 'teacher']),
], false],
'Filter role archetype' => ['role:archetype', [
'role:archetype_operator' => select::EQUAL_TO,
@@ -149,9 +149,9 @@ final class cohorts_test extends core_reportbuilder_testcase {
'cohort:context_operator' => select::EQUAL_TO,
'cohort:context_value' => system::instance()->id,
], true],
'Filter content (no match)' => ['cohort:context', [
'cohort:context_operator' => select::EQUAL_TO,
'cohort:context_value' => -1,
'Filter context (no match)' => ['cohort:context', [
'cohort:context_operator' => select::NOT_EQUAL_TO,
'cohort:context_value' => system::instance()->id,
], false],
'Filter name' => ['cohort:name', [
'cohort:name_operator' => text::IS_EQUAL_TO,
@@ -187,8 +187,8 @@ final class competencies_test extends core_reportbuilder_testcase {
'framework:scale_value' => '<SCALEID>',
], true],
'Framework scale (no match)' => ['framework:scale', [
'framework:scale_operator' => select::EQUAL_TO,
'framework:scale_value' => -1,
'framework:scale_operator' => select::NOT_EQUAL_TO,
'framework:scale_value' => '<SCALEID>',
], false],
'Framework visible' => ['framework:visible', [
'framework:visible_operator' => boolean_select::CHECKED,
@@ -194,7 +194,7 @@ final class categories_test extends core_reportbuilder_testcase {
], true],
'Filter role (no match)' => ['role:name', [
'role:name_operator' => select::EQUAL_TO,
'role:name_value' => -1,
'role:name_value' => $DB->get_field('role', 'id', ['shortname' => 'teacher']),
], false],
// User.
@@ -82,6 +82,7 @@ final class courses_test extends core_reportbuilder_testcase {
'category' => $category->id,
'fullname' => 'Cats',
'summary' => 'Course description',
'lang' => 'en',
'theme' => 'boost',
'tags' => ['Horses'],
]);
@@ -161,8 +162,8 @@ final class courses_test extends core_reportbuilder_testcase {
$this->assertEquals('Yes', $coursevisible);
$this->assertEquals('No groups', $coursegroupmode);
$this->assertEquals('No', $coursegroupmodeforce);
$this->assertEmpty($courselang);
$this->assertEmpty($coursecalendar);
$this->assertEquals('English (en)', $courselang);
$this->assertEquals('Do not force', $coursecalendar);
$this->assertEquals('Boost', $coursetheme);
$this->assertEquals('No', $coursecompletion);
$this->assertEmpty($coursedownload);
@@ -273,7 +274,7 @@ final class courses_test extends core_reportbuilder_testcase {
], true],
'Filter course format (no match)' => ['course:format', [
'course:format_operator' => select::EQUAL_TO,
'course:format_value' => 'weekly',
'course:format_value' => 'weeks',
], false],
'Filter course startdate' => ['course:startdate', [
'course:startdate_operator' => date::DATE_RANGE,
@@ -315,7 +316,7 @@ final class courses_test extends core_reportbuilder_testcase {
], true],
'Filter course lang (no match)' => ['course:lang', [
'course:lang_operator' => select::EQUAL_TO,
'course:lang_value' => 'de',
'course:lang_value' => '',
], false],
'Filter course calendartype' => ['course:calendartype', [
'course:calendartype_operator' => select::EQUAL_TO,
@@ -323,7 +324,7 @@ final class courses_test extends core_reportbuilder_testcase {
], true],
'Filter course calendartype (no match)' => ['course:calendartype', [
'course:calendartype_operator' => select::EQUAL_TO,
'course:calendartype_value' => 'hijri',
'course:calendartype_value' => '',
], false],
'Filter course theme' => ['course:theme', [
'course:theme_operator' => select::EQUAL_TO,
@@ -409,7 +409,7 @@ class course extends base {
* @return array
*/
public static function get_options_for_theme(): array {
return array_map(
return ['' => get_string('forceno')] + array_map(
fn(theme_config $theme) => $theme->get_theme_name(),
get_list_of_themes(),
);
@@ -421,7 +421,7 @@ class course extends base {
* @return array
*/
public static function get_options_for_lang(): array {
return get_string_manager()->get_list_of_translations();
return ['' => get_string('forceno')] + get_string_manager()->get_list_of_translations();
}
/**
@@ -430,7 +430,7 @@ class course extends base {
* @return array
*/
public static function get_options_for_calendartype(): array {
return \core_calendar\type_factory::get_list_of_calendar_types();
return ['' => get_string('forceno')] + \core_calendar\type_factory::get_list_of_calendar_types();
}
/**
@@ -452,7 +452,7 @@ class course extends base {
// If the column has corresponding filter, determine the value from its options.
$options = $this->get_options_for($fieldname);
if ($options !== null && array_key_exists($value, $options)) {
if ($options !== null && $value !== null && array_key_exists($value, $options)) {
return $options[$value];
}
@@ -364,7 +364,7 @@ class user extends base {
// If the column has corresponding filter, determine the value from its options.
$options = $this->get_options_for($fieldname);
if ($options !== null && array_key_exists($value, $options)) {
if ($options !== null && $value !== null && array_key_exists($value, $options)) {
return $options[$value];
}
+19 -4
View File
@@ -44,6 +44,9 @@ class select extends base {
/** @var int Not equal to */
public const NOT_EQUAL_TO = 2;
/** @var int Value to indicate "Any value" for the simplified filter options */
private const OPTION_ANY_VALUE = -124567;
/**
* Returns an array of comparison operators
*
@@ -65,7 +68,13 @@ class select extends base {
* @return array
*/
protected function get_select_options(): array {
return (array) $this->filter->get_options();
static $options = [];
if (!array_key_exists($this->name, $options)) {
$options[$this->name] = (array) $this->filter->get_options();
}
return $options[$this->name];
}
/**
@@ -91,7 +100,7 @@ class select extends base {
$element,
"{$this->name}_value",
get_string('filterfieldvalue', 'core_reportbuilder', $this->get_header()),
['' => $operators[self::ANY_VALUE]] + $options,
[self::OPTION_ANY_VALUE => $operators[self::ANY_VALUE]] + $options,
)->setHiddenLabel(true);
} else {
$elements = [];
@@ -129,13 +138,19 @@ class select extends base {
$name = database::generate_param_name();
$operator = (int) ($values["{$this->name}_operator"] ?? self::ANY_VALUE);
$value = $values["{$this->name}_value"] ?? '';
$value = (string) ($values["{$this->name}_value"] ?? self::OPTION_ANY_VALUE);
$fieldsql = $this->filter->get_field_sql();
$params = $this->filter->get_field_params();
// Get available options, if multidimensional then flatten the array.
$options = $this->get_select_options();
if (count($options) !== count($options, COUNT_RECURSIVE)) {
$options = array_merge(...array_values($options));
}
// Validate filter form values.
if ($value === '') {
if ($operator === self::ANY_VALUE || !array_key_exists($value, $options)) {
return ['', []];
}
@@ -43,8 +43,12 @@ final class select_test extends advanced_testcase {
[select::ANY_VALUE, null, true],
[select::EQUAL_TO, 'starwars', true],
[select::EQUAL_TO, 'mandalorian', false],
[select::EQUAL_TO, '', false],
[select::EQUAL_TO, 'invalid', true],
[select::NOT_EQUAL_TO, 'starwars', false],
[select::NOT_EQUAL_TO, 'mandalorian', true],
[select::NOT_EQUAL_TO, '', true],
[select::NOT_EQUAL_TO, 'invalid', true],
];
}
@@ -64,11 +68,11 @@ final class select_test extends advanced_testcase {
$course1 = $this->getDataGenerator()->create_course([
'fullname' => "May the course be with you",
'shortname' => 'starwars',
'idnumber' => 'starwars',
]);
$course2 = $this->getDataGenerator()->create_course([
'fullname' => "This is the course",
'shortname' => 'mandalorian',
'idnumber' => '',
]);
$filter = (new filter(
@@ -76,10 +80,11 @@ final class select_test extends advanced_testcase {
'test',
new lang_string('course'),
'testentity',
'shortname'
'idnumber'
))->set_options([
$course1->shortname => $course1->fullname,
$course2->shortname => $course2->fullname,
$course1->idnumber => $course1->fullname,
$course2->idnumber => $course2->fullname,
'mandalorian' => 'This is not the course you are looking for',
]);
// Create instance of our filter, passing given operator.
@@ -70,7 +70,7 @@ final class custom_fields_test extends core_reportbuilder_testcase {
$generator->create_field(
['categoryid' => $category->get('id'), 'type' => 'select', 'name' => 'Select', 'shortname' => 'select',
'configdata' => ['options' => "Cat\nDog", 'defaultvalue' => 'Cat']]);
'configdata' => ['options' => "Cat\nDog\nFish", 'defaultvalue' => 'Cat']]);
$generator->create_field(
['categoryid' => $category->get('id'), 'type' => 'number', 'name' => 'Number', 'shortname' => 'number',
@@ -50,7 +50,8 @@ final class user_profile_fields_test extends core_reportbuilder_testcase {
'defaultdata' => 0, 'visible' => PROFILE_VISIBLE_NONE]);
$this->getDataGenerator()->create_custom_profile_field([
'shortname' => 'menu', 'name' => 'Menu field', 'datatype' => 'menu', 'param1' => "Cat\nDog", 'defaultdata' => 'Cat']);
'shortname' => 'menu', 'name' => 'Menu field', 'datatype' => 'menu', 'param1' => "Cat\nDog\nFish",
'defaultdata' => 'Cat']);
$this->getDataGenerator()->create_custom_profile_field([
'shortname' => 'Social', 'name' => 'msn', 'datatype' => 'social', 'param1' => 'msn']);
@@ -386,8 +386,8 @@ final class users_test extends core_reportbuilder_testcase {
'user:lang_value' => 'en',
], true],
'Filter lang (no match)' => ['user:lang', [
'user:lang_operator' => select::EQUAL_TO,
'user:lang_value' => 'de',
'user:lang_operator' => select::NOT_EQUAL_TO,
'user:lang_value' => 'en',
], false],
'Filter timezone' => ['user:timezone', [
'user:timezone_operator' => select::EQUAL_TO,