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

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 (a0ef4bb) as well as improving validation to ensure only present options can be used for filtering.
paulholden · Jan 31, 2025 · acc654e · acc654e
1 parent 07881a5
commit acc654e
Show file tree

Hide file tree

Showing 14 changed files with 67 additions and 30 deletions.
diff --git a/.upgradenotes/MDL-82913-2025011510021468.yml b/.upgradenotes/MDL-82913-2025011510021468.yml
@@ -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
diff --git a/.upgradenotes/MDL-84213-2025011422342259.yml b/.upgradenotes/MDL-84213-2025011422342259.yml
@@ -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
diff --git a/admin/roles/tests/reportbuilder/datasource/roles_test.php b/admin/roles/tests/reportbuilder/datasource/roles_test.php
@@ -149,7 +149,7 @@ public static function datasource_filters_provider(): array {
             ], 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,

diff --git a/cohort/tests/reportbuilder/datasource/cohorts_test.php b/cohort/tests/reportbuilder/datasource/cohorts_test.php
@@ -149,9 +149,9 @@ public static function datasource_filters_provider(): array {
                 '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,

diff --git a/competency/tests/reportbuilder/datasource/competencies_test.php b/competency/tests/reportbuilder/datasource/competencies_test.php
@@ -187,8 +187,8 @@ public static function datasource_filters_provider(): array {
                 '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,

diff --git a/course/tests/reportbuilder/datasource/categories_test.php b/course/tests/reportbuilder/datasource/categories_test.php
@@ -194,7 +194,7 @@ public static function datasource_filters_provider(): array {
             ], 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.

diff --git a/course/tests/reportbuilder/datasource/courses_test.php b/course/tests/reportbuilder/datasource/courses_test.php
@@ -82,6 +82,7 @@ public function test_datasource_non_default_columns(): void {
             'category' => $category->id,
             'fullname' => 'Cats',
             'summary' => 'Course description',
+            'lang' => 'en',
             'theme' => 'boost',
             'tags' => ['Horses'],
         ]);
@@ -161,8 +162,8 @@ public function test_datasource_non_default_columns(): void {
         $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 @@ public static function datasource_filters_provider(): array {
             ], 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,15 +316,15 @@ public static function datasource_filters_provider(): array {
             ], 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,
                 'course:calendartype_value' => 'gregorian',
             ], 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,

diff --git a/reportbuilder/classes/local/entities/course.php b/reportbuilder/classes/local/entities/course.php
@@ -409,7 +409,7 @@ public static function get_options_for_format(): array {
      * @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 @@ public static function get_options_for_theme(): array {
      * @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 @@ public static function get_options_for_lang(): array {
      * @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 @@ public function format($value, stdClass $row, string $fieldname): string {
 
         // 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];
         }
 

diff --git a/reportbuilder/classes/local/entities/user.php b/reportbuilder/classes/local/entities/user.php
@@ -364,7 +364,7 @@ public function format($value, stdClass $row, string $fieldname): string {
 
         // 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];
         }
 

diff --git a/reportbuilder/classes/local/filters/select.php b/reportbuilder/classes/local/filters/select.php
@@ -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 @@ protected function get_operators(): array {
      * @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 @@ public function setup_form(MoodleQuickForm $mform): void {
                 $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 @@ public function get_sql_filter(array $values): array {
         $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 ['', []];
         }
 

diff --git a/reportbuilder/tests/local/filters/select_test.php b/reportbuilder/tests/local/filters/select_test.php
@@ -43,8 +43,12 @@ public static function get_sql_filter_simple_provider(): array {
             [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,22 +68,23 @@ public function test_get_sql_filter_simple(int $operator, ?string $value, bool $
 
         $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(
             select::class,
             '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.

diff --git a/reportbuilder/tests/local/helpers/custom_fields_test.php b/reportbuilder/tests/local/helpers/custom_fields_test.php
@@ -70,7 +70,7 @@ private function generate_customfields(): custom_fields {
 
         $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',

diff --git a/reportbuilder/tests/local/helpers/user_profile_fields_test.php b/reportbuilder/tests/local/helpers/user_profile_fields_test.php
@@ -50,7 +50,8 @@ private function generate_userprofilefields(): user_profile_fields {
                 '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']);

diff --git a/user/tests/reportbuilder/datasource/users_test.php b/user/tests/reportbuilder/datasource/users_test.php
@@ -386,8 +386,8 @@ public static function datasource_filters_provider(): array {
                 '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,