L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-84213 reportbuilder: add course "do not force" field options.

Browse Source 
Where course entity select elements theme, language and calendar are
defined we should prepend with "Do not force" in order to match the
interface when editing the same fields.

The select filter has been updated to ensure it supports empty values
when switched to simplified version (a0ef4bb8) as well as improving
validation to ensure only present options can be used for filtering.
This commit is contained in:
Paul Holden
2025-01-14 21:06:54 +00:00
parent 07881a5772
commit acc654e8ce
14 changed files with 67 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.upgradenotes/MDL-82913-2025011510021468.yml
+8
View File
@@ -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
.upgradenotes/MDL-84213-2025011422342259.yml
+7
View File
@@ -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
admin/roles/tests/reportbuilder/datasource/roles_test.php
+1 -1
View File
@@ -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,
cohort/tests/reportbuilder/datasource/cohorts_test.php
+3 -3
View File
@@ -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,
competency/tests/reportbuilder/datasource/competencies_test.php
+2 -2
View File
@@ -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,
course/tests/reportbuilder/datasource/categories_test.php
+1 -1
View File
@@ -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.
course/tests/reportbuilder/datasource/courses_test.php
+6 -5
View File
@@ -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,
reportbuilder/classes/local/entities/course.php
+4 -4
View File
@@ -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];
}
reportbuilder/classes/local/entities/user.php
+1 -1
View File
@@ -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];
}
reportbuilder/classes/local/filters/select.php
+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 ['', []];
}
reportbuilder/tests/local/filters/select_test.php
+10 -5
View File
@@ -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.
reportbuilder/tests/local/helpers/custom_fields_test.php
+1 -1
View File
@@ -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',
reportbuilder/tests/local/helpers/user_profile_fields_test.php
+2 -1
View File
@@ -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']);
user/tests/reportbuilder/datasource/users_test.php
+2 -2
View File
@@ -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,