From 1613a98102aaaba2f60216d77da02d453fdc2196 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Thu, 5 Dec 2024 09:30:56 +0000 Subject: [PATCH 1/2] MDL-83859 core_question: Update filter condition API This corrects some definitions of the methods in the base condition class to make things more obvious to developers implementing new filters. Previously if your filter wanted to use the default `core/datafilter/filtertype` class, you still had to implement `get_filter_class` to return `null`, since it was declared as abstract. This change defines it as returning `null` by default, so this is no longer necessary. Also, this removes the default definitions for `get_condition_key` and `build_query_from_functions`, and declares them abstract. Currently it is necessary to override these to implement a functional filter so it doesn't make sense to have a useless default definition. This will not cause any breakages with existing filters. All filters must already be defining the methods that are now abstract, otherwise they will not function. Any filter that is now overriding `get_filter_class` to return `null` will continue to work as before, even though this is no longer necessary. --- .upgradenotes/MDL-83859-2024120511024767.yml | 20 +++++++++++++++ question/classes/local/bank/condition.php | 27 +++++++++++--------- 2 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 .upgradenotes/MDL-83859-2024120511024767.yml diff --git a/.upgradenotes/MDL-83859-2024120511024767.yml b/.upgradenotes/MDL-83859-2024120511024767.yml new file mode 100644 index 00000000000..7bc3bb9a7d9 --- /dev/null +++ b/.upgradenotes/MDL-83859-2024120511024767.yml @@ -0,0 +1,20 @@ +issueNumber: MDL-83859 +notes: + core_question: + - message: > + The definition of the abstract `core_question\local\bank\condition` + class has changed to make it clearer which methods are required + in child classes. + + The `get_filter_class` method is no longer declared as abstract, and + will return `null` by default to use the base + `core/datafilter/filtertype` class. If you have defined this method + to return `null` in your own class, it will continue to work, but + it is no longer necessary. + + `build_query_from_filter` and `get_condition_key` are now declared as + abstract, since all filter condition classes must define these + (as well as existing abstract methods) to function. Again, exsiting + child classes will continue to work if they did before, as they + already needed these methods. + type: changed diff --git a/question/classes/local/bank/condition.php b/question/classes/local/bank/condition.php index c337613fda7..549446a1d52 100644 --- a/question/classes/local/bank/condition.php +++ b/question/classes/local/bank/condition.php @@ -27,7 +27,6 @@ use core\output\datafilter; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ abstract class condition { - /** @var int The default filter type */ const JOINTYPE_DEFAULT = datafilter::JOINTYPE_ANY; @@ -48,11 +47,16 @@ abstract class condition { abstract public function get_title(); /** - * Return filter class associated with this condition + * Return the Javascript filter class to provide the UI for this condition. * - * @return string filter class + * If left as null, this will use the default core/datafilter/filtertype class. Otherwise, override it to return + * the full path to the Javascript module path for the class. + * + * @return ?string filter class */ - abstract public function get_filter_class(); + public function get_filter_class() { + return null; + } /** * Extract the required filter from the provided question bank view. @@ -140,9 +144,7 @@ abstract class condition { * * @return string */ - public static function get_condition_key() { - return ''; - } + abstract public static function get_condition_key(); /** * Return an SQL fragment to be ANDed into the WHERE clause to filter which questions are shown. @@ -211,12 +213,13 @@ abstract class condition { } /** - * Build query from filter value + * Return the SQL WHERE condition and parameters to be ANDed with other filter conditions. + * + * The $filter parameter recieves an array with a 'values' key, containing an array of the filter values selected, + * and a 'jointype' key containing the selected join type. * * @param array $filter filter properties - * @return array where sql and params + * @return array ['SQL where condition', ['param1' => 'value1', 'param2' => 'value2', ...]] */ - public static function build_query_from_filter(array $filter): array { - return ['', []]; - } + abstract public static function build_query_from_filter(array $filter): array; } From e46a0c33acd1d455ca433efff8aad623ce4bf4db Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 3 Dec 2024 13:25:50 +0000 Subject: [PATCH 2/2] MDL-83859 qbank_viewquestiontype: Add type filter This adds a new question bank filter for filtering by question type. The filter can be applied with one or multiple question types, and can either include or exclude the selected types. --- .../build/datafilter/filtertype/type.min.js | 11 ++ .../datafilter/filtertype/type.min.js.map | 1 + .../amd/src/datafilter/filtertype/type.js | 39 ++++++ .../classes/plugin_feature.php | 12 +- .../classes/type_condition.php | 112 ++++++++++++++++++ .../lang/en/qbank_viewquestiontype.php | 1 + .../filter_condition_question_type.feature | 45 +++++++ 7 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 question/bank/viewquestiontype/amd/build/datafilter/filtertype/type.min.js create mode 100644 question/bank/viewquestiontype/amd/build/datafilter/filtertype/type.min.js.map create mode 100644 question/bank/viewquestiontype/amd/src/datafilter/filtertype/type.js create mode 100644 question/bank/viewquestiontype/classes/type_condition.php create mode 100644 question/bank/viewquestiontype/tests/behat/filter_condition_question_type.feature diff --git a/question/bank/viewquestiontype/amd/build/datafilter/filtertype/type.min.js b/question/bank/viewquestiontype/amd/build/datafilter/filtertype/type.min.js new file mode 100644 index 00000000000..02afdc7be14 --- /dev/null +++ b/question/bank/viewquestiontype/amd/build/datafilter/filtertype/type.min.js @@ -0,0 +1,11 @@ +define("qbank_viewquestiontype/datafilter/filtertype/type",["exports","core/datafilter/filtertype"],(function(_exports,_filtertype){var obj; +/** + * Filter managing hidden questions. + * + * @module qbank_viewquestiontype/datafilter/filtertypes/type + * @author Mark Johnson + * @copyright 2024 Catalyst IT Europe Ltd + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.default=void 0,_filtertype=(obj=_filtertype)&&obj.__esModule?obj:{default:obj};class _default extends _filtertype.default{get values(){return this.rawValues}}return _exports.default=_default,_exports.default})); + +//# sourceMappingURL=type.min.js.map \ No newline at end of file diff --git a/question/bank/viewquestiontype/amd/build/datafilter/filtertype/type.min.js.map b/question/bank/viewquestiontype/amd/build/datafilter/filtertype/type.min.js.map new file mode 100644 index 00000000000..b63354db168 --- /dev/null +++ b/question/bank/viewquestiontype/amd/build/datafilter/filtertype/type.min.js.map @@ -0,0 +1 @@ +{"version":3,"file":"type.min.js","sources":["../../../src/datafilter/filtertype/type.js"],"sourcesContent":["// This file is part of Moodle - http://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * Filter managing hidden questions.\n *\n * @module qbank_viewquestiontype/datafilter/filtertypes/type\n * @author Mark Johnson \n * @copyright 2024 Catalyst IT Europe Ltd\n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\n\nimport Filter from 'core/datafilter/filtertype';\n\nexport default class extends Filter {\n /**\n * Get the list of values for this filter type.\n *\n * Overrides the default behaviour of running parseInt on the raw values, since we have textual\n * plugin identifiers and not numeric IDs.\n *\n * @returns {Array}\n */\n get values() {\n return this.rawValues;\n }\n}\n"],"names":["Filter","values","this","rawValues"],"mappings":";;;;;;;;4KA0B6BA,oBASrBC,oBACOC,KAAKC"} \ No newline at end of file diff --git a/question/bank/viewquestiontype/amd/src/datafilter/filtertype/type.js b/question/bank/viewquestiontype/amd/src/datafilter/filtertype/type.js new file mode 100644 index 00000000000..da9547111ae --- /dev/null +++ b/question/bank/viewquestiontype/amd/src/datafilter/filtertype/type.js @@ -0,0 +1,39 @@ +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +/** + * Filter managing hidden questions. + * + * @module qbank_viewquestiontype/datafilter/filtertypes/type + * @author Mark Johnson + * @copyright 2024 Catalyst IT Europe Ltd + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +import Filter from 'core/datafilter/filtertype'; + +export default class extends Filter { + /** + * Get the list of values for this filter type. + * + * Overrides the default behaviour of running parseInt on the raw values, since we have textual + * plugin identifiers and not numeric IDs. + * + * @returns {Array} + */ + get values() { + return this.rawValues; + } +} diff --git a/question/bank/viewquestiontype/classes/plugin_feature.php b/question/bank/viewquestiontype/classes/plugin_feature.php index 15b23bfb887..0894b43da68 100644 --- a/question/bank/viewquestiontype/classes/plugin_feature.php +++ b/question/bank/viewquestiontype/classes/plugin_feature.php @@ -17,6 +17,7 @@ namespace qbank_viewquestiontype; use core_question\local\bank\plugin_features_base; +use core_question\local\bank\view; /** * Class plugin_feature is the entrypoint for the columns. @@ -27,10 +28,17 @@ use core_question\local\bank\plugin_features_base; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class plugin_feature extends plugin_features_base { - + #[\Override] public function get_question_columns($qbank): array { return [ - new question_type_column($qbank) + new question_type_column($qbank), + ]; + } + + #[\Override] + public function get_question_filters(?view $qbank = null): array { + return [ + new type_condition($qbank), ]; } } diff --git a/question/bank/viewquestiontype/classes/type_condition.php b/question/bank/viewquestiontype/classes/type_condition.php new file mode 100644 index 00000000000..c6e49d99b4e --- /dev/null +++ b/question/bank/viewquestiontype/classes/type_condition.php @@ -0,0 +1,112 @@ +. + +namespace qbank_viewquestiontype; + +use question_bank; +use core\output\datafilter; +use core_question\local\bank\condition; + +/** + * Filter condition for question type + * + * @package qbank_viewquestiontype + * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class type_condition extends condition { + #[\Override] + public static function get_condition_key() { + return 'qtype'; + } + + #[\Override] + public function get_title() { + return get_string('typefilter', 'qbank_viewquestiontype'); + } + + #[\Override] + public function get_filter_class() { + return 'qbank_viewquestiontype/datafilter/filtertype/type'; + } + + #[\Override] + public function allow_custom() { + return false; + } + + /** + * Get the list of available joins for the filter. + * + * Questions can be ANY or NONE of the selected types. Since questions cannot have multiple types, + * allowing "ALL" does not make sense here. + * + * @return array + */ + public function get_join_list(): array { + return [ + datafilter::JOINTYPE_NONE, + datafilter::JOINTYPE_ANY, + ]; + } + + /** + * Build a list of the available question types. + * + * @return array + */ + public function get_initial_values(): array { + $types = question_bank::get_all_qtypes(); + $values = []; + foreach ($types as $plugin => $type) { + $values[] = [ + 'value' => $plugin, + 'title' => get_string('pluginname', 'qtype_' . $plugin), + ]; + } + usort($values, fn($a, $b) => $a['title'] <=> $b['title']); + return $values; + } + + /** + * Build a WHERE condition to filter the q.qtype column by the selected question types. + * + * @param array $filter + * @return array + * @throws \coding_exception + * @throws \dml_exception + */ + public static function build_query_from_filter(array $filter): array { + global $DB; + + // Remove empty string. + $filter['values'] = array_filter($filter['values']); + + $selectedtypes = $filter['values'] ?? []; + + $params = []; + $where = ''; + $jointype = $filter['jointype'] ?? self::JOINTYPE_DEFAULT; + if ($selectedtypes) { + // If we are matching NONE rather than ANY, exclude the selected types instead. + $equal = !($jointype === datafilter::JOINTYPE_NONE); + [$typesql, $params] = $DB->get_in_or_equal($selectedtypes, SQL_PARAMS_NAMED, 'param', $equal); + $where = "q.qtype $typesql"; + } + return [$where, $params]; + } +} diff --git a/question/bank/viewquestiontype/lang/en/qbank_viewquestiontype.php b/question/bank/viewquestiontype/lang/en/qbank_viewquestiontype.php index 735ef5130b4..812e903c58f 100644 --- a/question/bank/viewquestiontype/lang/en/qbank_viewquestiontype.php +++ b/question/bank/viewquestiontype/lang/en/qbank_viewquestiontype.php @@ -23,5 +23,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +$string['typefilter'] = 'Type'; $string['pluginname'] = 'View question type'; $string['privacy:metadata'] = 'The View question type question bank plugin does not store any personal data.'; diff --git a/question/bank/viewquestiontype/tests/behat/filter_condition_question_type.feature b/question/bank/viewquestiontype/tests/behat/filter_condition_question_type.feature new file mode 100644 index 00000000000..bb86deb6d7c --- /dev/null +++ b/question/bank/viewquestiontype/tests/behat/filter_condition_question_type.feature @@ -0,0 +1,45 @@ +@qbank @qbank_viewquestiontype @javascript +Feature: Filter questions by type + As a teacher + In order to organise my questions + I want to filter the list of questions by type + + Background: + Given the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | + | qbank | Qbank 1 | Question bank 1 | C1 | qbank1 | + And the following "question categories" exist: + | contextlevel | reference | name | + | Activity module | qbank1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions | truefalse | First question | Answer the first question | + | Test questions | numerical | Second question | Answer the second question | + | Test questions | essay | Third question | Answer the third question | + + Scenario: Filter by a single type + Given I am on the "Qbank 1" "core_question > question bank" page logged in as "admin" + When I apply question bank filter "Type" with value "True/False" + Then I should see "First question" + And I should not see "Second question" + And I should not see "Third question" + + Scenario: Filter by multiple types + Given I am on the "Qbank 1" "core_question > question bank" page logged in as "admin" + When I apply question bank filter "Type" with value "True/False, Essay" + Then I should see "First question" + And I should not see "Second question" + And I should see "Third question" + + Scenario: Exclude types by filter + Given I am on the "Qbank 1" "core_question > question bank" page logged in as "admin" + When I add question bank filter "Type" + And I set the field "Type" to "True/False, Essay" + And I set the field "Match" in the "Filter 3" "fieldset" to "None" + And I press "Apply filters" + Then I should not see "First question" + And I should see "Second question" + And I should not see "Third question"