From f4e8942755874e177f2d71e147c7b7aff52c1557 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Thu, 29 May 2025 15:09:12 +0100 Subject: [PATCH] MDL-85069 question: Services for question bank and category search This updates the core_question_search_shared_banks external function with additional parameters to make it applicable outside of the "Switch bank" dialogue in the quiz, with the corresponding changes to the question_banks_datasource Javascript module, and unit test coverage. It also adds a new \core_question\output\question_category_selector class for rendering a select list of question categories, migrating the logic used in qbank_managecategories to a core namespace. There is a fragment callback for rendering this via AJAX. --- .upgradenotes/MDL-85069-2025060213180503.yml | 19 + lang/en/question.php | 2 +- lib/questionlib.php | 2 +- .../build/question_banks_datasource.min.js | 2 +- .../question_banks_datasource.min.js.map | 2 +- question/amd/src/question_banks_datasource.js | 5 + .../bank/managecategories/classes/helper.php | 136 +----- .../classes/external/search_shared_banks.php | 64 ++- .../output/question_category_selector.php | 399 ++++++++++++++++++ question/lib.php | 13 + .../question_category_selector.mustache | 61 +++ .../external/search_shared_banks_test.php | 217 ++++++++++ version.php | 2 +- 13 files changed, 789 insertions(+), 135 deletions(-) create mode 100644 .upgradenotes/MDL-85069-2025060213180503.yml create mode 100644 question/classes/output/question_category_selector.php create mode 100644 question/templates/question_category_selector.mustache create mode 100644 question/tests/external/search_shared_banks_test.php diff --git a/.upgradenotes/MDL-85069-2025060213180503.yml b/.upgradenotes/MDL-85069-2025060213180503.yml new file mode 100644 index 00000000000..8bb8db3ef34 --- /dev/null +++ b/.upgradenotes/MDL-85069-2025060213180503.yml @@ -0,0 +1,19 @@ +issueNumber: MDL-85069 +notes: + core_question: + - message: > + `core_question_search_shared_banks` will now search all question banks, + not just those outside the current course. + + This makes the service usable in cases outside of the current "Switch + banks" UI, which require searching all banks on the site. + + It also makes the autocomplete in the "Switch banks" UI more consistent, + as it was previously excluding some of the banks listed in the UI + (Question banks in this course), but not others (Recently viewed + question banks). + + This change has also adds a 'requiredcapabilties' parameter to the + function, which accepts an list of abbreviated capabilities for + checking access against question banks before they are returned. + type: changed diff --git a/lang/en/question.php b/lang/en/question.php index 4c37c61c2e8..ac00e710591 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -267,7 +267,7 @@ $string['novirtualquestiontype'] = 'No virtual question type for question type { $string['numqas'] = 'No. question attempts'; $string['numquestions'] = 'No. questions'; $string['numquestionsandhidden'] = '{$a->numquestions} (+{$a->numhidden} hidden +{$a->numdraft} draft)'; -$string['otherquestionbank'] = 'Other question banks'; +$string['otherquestionbank'] = 'All question banks'; $string['otherquestionbankstoomany'] = 'More than {$a} results. You need to refine your search.'; $string['page-question-x'] = 'Any question page'; $string['page-question-edit'] = 'Question editing page'; diff --git a/lib/questionlib.php b/lib/questionlib.php index f8abcf56fb7..f689ee6b489 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -1276,7 +1276,7 @@ function question_default_export_filename($course, $category): string { * * @param int|stdClass|question_definition $questionorid object or id. * If an object is passed, it should include ->contextid and ->createdby. - * @param string $cap 'add', 'edit', 'view', 'use', 'move' or 'tag'. + * @param string $cap 'add', 'edit', 'view', 'use', 'move', or 'tag'. * @param int $notused no longer used. * @return bool this user has the capability $cap for this question $question? */ diff --git a/question/amd/build/question_banks_datasource.min.js b/question/amd/build/question_banks_datasource.min.js index 2ac75a6f2fa..a809ab5fcf2 100644 --- a/question/amd/build/question_banks_datasource.min.js +++ b/question/amd/build/question_banks_datasource.min.js @@ -6,6 +6,6 @@ define("core_question/question_banks_datasource",["exports","core/ajax","core/no * @copyright 2025 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 - */Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.default=void 0,_notification=(obj=_notification)&&obj.__esModule?obj:{default:obj};var _default={transport:function(selector,query,callback){const contextId=document.querySelector(selector).dataset.contextid;if(!contextId)throw new Error("The attribute data-contextid is required on "+selector);(0,_ajax.call)([{methodname:"core_question_search_shared_banks",args:{contextid:contextId,search:query}}])[0].then(callback).catch(_notification.default.exception)},processResults:(selector,results)=>results.sharedbanks};return _exports.default=_default,_exports.default})); + */Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.default=void 0,_notification=(obj=_notification)&&obj.__esModule?obj:{default:obj};var _default={transport:function(selector,query,callback){const element=document.querySelector(selector),contextId=element.dataset.contextid;let requiredcapabilities=["use"];if(element.dataset.requiredcapabilities&&(requiredcapabilities=JSON.parse(element.dataset.requiredcapabilities)),!contextId)throw new Error("The attribute data-contextid is required on "+selector);(0,_ajax.call)([{methodname:"core_question_search_shared_banks",args:{contextid:contextId,search:query,requiredcapabilities:requiredcapabilities}}])[0].then(callback).catch(_notification.default.exception)},processResults:(selector,results)=>results.sharedbanks};return _exports.default=_default,_exports.default})); //# sourceMappingURL=question_banks_datasource.min.js.map \ No newline at end of file diff --git a/question/amd/build/question_banks_datasource.min.js.map b/question/amd/build/question_banks_datasource.min.js.map index 3b9762a2c27..0c105ebae7b 100644 --- a/question/amd/build/question_banks_datasource.min.js.map +++ b/question/amd/build/question_banks_datasource.min.js.map @@ -1 +1 @@ -{"version":3,"file":"question_banks_datasource.min.js","sources":["../src/question_banks_datasource.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 * Autocomplete data source for shared question banks.\n *\n * @module core_question/question_banks_datasource\n * @copyright 2025 onwards Catalyst IT EU {@link https://catalyst-eu.net}\n * @author Mark Johnson \n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\n\nimport {call as fetchMany} from 'core/ajax';\nimport Notification from 'core/notification';\n\nexport default {\n\n transport: function(selector, query, callback) {\n const element = document.querySelector(selector);\n const contextId = element.dataset.contextid;\n\n if (!contextId) {\n throw new Error('The attribute data-contextid is required on ' + selector);\n }\n\n fetchMany([{\n methodname: 'core_question_search_shared_banks',\n args: {\n contextid: contextId,\n search: query,\n },\n }])[0]\n .then(callback)\n .catch(Notification.exception);\n },\n\n processResults: (selector, results) => {\n return results.sharedbanks;\n },\n};\n"],"names":["transport","selector","query","callback","contextId","document","querySelector","dataset","contextid","Error","methodname","args","search","then","catch","Notification","exception","processResults","results","sharedbanks"],"mappings":";;;;;;;;sKA2Be,CAEXA,UAAW,SAASC,SAAUC,MAAOC,gBAE3BC,UADUC,SAASC,cAAcL,UACbM,QAAQC,cAE7BJ,gBACK,IAAIK,MAAM,+CAAiDR,yBAG3D,CAAC,CACPS,WAAY,oCACZC,KAAM,CACFH,UAAWJ,UACXQ,OAAQV,UAEZ,GACHW,KAAKV,UACLW,MAAMC,sBAAaC,YAGxBC,eAAgB,CAAChB,SAAUiB,UAChBA,QAAQC"} \ No newline at end of file +{"version":3,"file":"question_banks_datasource.min.js","sources":["../src/question_banks_datasource.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 * Autocomplete data source for shared question banks.\n *\n * @module core_question/question_banks_datasource\n * @copyright 2025 onwards Catalyst IT EU {@link https://catalyst-eu.net}\n * @author Mark Johnson \n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\n\nimport {call as fetchMany} from 'core/ajax';\nimport Notification from 'core/notification';\n\nexport default {\n\n transport: function(selector, query, callback) {\n const element = document.querySelector(selector);\n const contextId = element.dataset.contextid;\n let requiredcapabilities = ['use'];\n if (element.dataset.requiredcapabilities) {\n requiredcapabilities = JSON.parse(element.dataset.requiredcapabilities);\n }\n\n if (!contextId) {\n throw new Error('The attribute data-contextid is required on ' + selector);\n }\n\n fetchMany([{\n methodname: 'core_question_search_shared_banks',\n args: {\n contextid: contextId,\n search: query,\n requiredcapabilities: requiredcapabilities,\n },\n }])[0]\n .then(callback)\n .catch(Notification.exception);\n },\n\n processResults: (selector, results) => {\n return results.sharedbanks;\n },\n};\n"],"names":["transport","selector","query","callback","element","document","querySelector","contextId","dataset","contextid","requiredcapabilities","JSON","parse","Error","methodname","args","search","then","catch","Notification","exception","processResults","results","sharedbanks"],"mappings":";;;;;;;;sKA2Be,CAEXA,UAAW,SAASC,SAAUC,MAAOC,gBAC3BC,QAAUC,SAASC,cAAcL,UACjCM,UAAYH,QAAQI,QAAQC,cAC9BC,qBAAuB,CAAC,UACxBN,QAAQI,QAAQE,uBAChBA,qBAAuBC,KAAKC,MAAMR,QAAQI,QAAQE,wBAGjDH,gBACK,IAAIM,MAAM,+CAAiDZ,yBAG3D,CAAC,CACPa,WAAY,oCACZC,KAAM,CACFN,UAAWF,UACXS,OAAQd,MACRQ,qBAAsBA,yBAE1B,GACHO,KAAKd,UACLe,MAAMC,sBAAaC,YAGxBC,eAAgB,CAACpB,SAAUqB,UAChBA,QAAQC"} \ No newline at end of file diff --git a/question/amd/src/question_banks_datasource.js b/question/amd/src/question_banks_datasource.js index fc47d89b802..629545eb1f5 100644 --- a/question/amd/src/question_banks_datasource.js +++ b/question/amd/src/question_banks_datasource.js @@ -30,6 +30,10 @@ export default { transport: function(selector, query, callback) { const element = document.querySelector(selector); const contextId = element.dataset.contextid; + let requiredcapabilities = ['use']; + if (element.dataset.requiredcapabilities) { + requiredcapabilities = JSON.parse(element.dataset.requiredcapabilities); + } if (!contextId) { throw new Error('The attribute data-contextid is required on ' + selector); @@ -40,6 +44,7 @@ export default { args: { contextid: contextId, search: query, + requiredcapabilities: requiredcapabilities, }, }])[0] .then(callback) diff --git a/question/bank/managecategories/classes/helper.php b/question/bank/managecategories/classes/helper.php index cffed3beaf0..affce3a6b47 100644 --- a/question/bank/managecategories/classes/helper.php +++ b/question/bank/managecategories/classes/helper.php @@ -23,6 +23,7 @@ require_once($CFG->libdir . "/questionlib.php"); use context; use core_question\category_manager; use core_question\local\bank\question_version_status; +use core_question\output\question_category_selector; use moodle_exception; use html_writer; @@ -304,46 +305,7 @@ class helper { bool $top = false, int $showallversions = 0, ): array { - global $DB; - - $contextids = explode(',', $contexts); - foreach ($contextids as $contextid) { - $context = context::instance_by_id($contextid); - if ($context->contextlevel === CONTEXT_MODULE) { - $validcontexts[] = $contextid; - } - } - if (empty($validcontexts)) { - return []; - } - - [$insql, $inparams] = $DB->get_in_or_equal($validcontexts); - - $topwhere = $top ? '' : 'AND c.parent <> 0'; - $statuscondition = "AND (qv.status = '" . question_version_status::QUESTION_STATUS_READY . "' " . - " OR qv.status = '" . question_version_status::QUESTION_STATUS_DRAFT . "' )"; - $substatuscondition = "AND v.status <> '" . question_version_status::QUESTION_STATUS_HIDDEN . "' "; - $sql = "SELECT c.*, - (SELECT COUNT(1) - FROM {question} q - JOIN {question_versions} qv ON qv.questionid = q.id - JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid - WHERE q.parent = '0' - $statuscondition - AND c.id = qbe.questioncategoryid - AND ({$showallversions} = 1 - OR (qv.version = (SELECT MAX(v.version) - FROM {question_versions} v - JOIN {question_bank_entries} be ON be.id = v.questionbankentryid - WHERE be.id = qbe.id $substatuscondition) - ) - ) - ) AS questioncount - FROM {question_categories} c - WHERE c.contextid {$insql} {$topwhere} - ORDER BY {$sortorder}"; - - return $DB->get_records_sql($sql, $inparams); + return (new question_category_selector())->get_categories_for_contexts($contexts, $sortorder, $top, $showallversions); } /** @@ -366,74 +328,14 @@ class helper { int $nochildrenof = -1, bool $escapecontextnames = true, ): array { - global $CFG; - $pcontexts = []; - foreach ($contexts as $context) { - if ($context->contextlevel !== CONTEXT_MODULE) { - continue; - } - $pcontexts[] = $context->id; - } - $contextslist = join(', ', $pcontexts); - - $categories = self::get_categories_for_contexts($contextslist, 'parent, sortorder, name ASC', $top); - - if ($top) { - $categories = self::question_fix_top_names($categories); - } - - $categories = self::question_add_context_in_key($categories); - $categories = self::add_indented_names($categories, $nochildrenof); - - // Sort cats out into different contexts. - $categoriesarray = []; - foreach ($pcontexts as $contextid) { - $context = \context::instance_by_id($contextid); - $contextstring = $context->get_context_name(true, true, $escapecontextnames); - foreach ($categories as $category) { - if ($category->contextid == $contextid) { - $cid = $category->id; - if ("{$currentcat},{$contextid}" != $cid || $currentcat == 0) { - $a = new \stdClass(); - $a->name = format_string( - $category->indentedname, - true, - ['context' => $context] - ); - if ($category->idnumber !== null && $category->idnumber !== '') { - $a->idnumber = s($category->idnumber); - } - if (!empty($category->questioncount)) { - $a->questioncount = $category->questioncount; - } - if (isset($a->idnumber) && isset($a->questioncount)) { - $formattedname = get_string('categorynamewithidnumberandcount', 'question', $a); - } else if (isset($a->idnumber)) { - $formattedname = get_string('categorynamewithidnumber', 'question', $a); - } else if (isset($a->questioncount)) { - $formattedname = get_string('categorynamewithcount', 'question', $a); - } else { - $formattedname = $a->name; - } - $categoriesarray[$contextstring][$cid] = $formattedname; - } - } - } - } - if ($popupform) { - $popupcats = []; - foreach ($categoriesarray as $contextstring => $optgroup) { - $group = []; - foreach ($optgroup as $key => $value) { - $key = str_replace($CFG->wwwroot, '', $key); - $group[$key] = $value; - } - $popupcats[] = [$contextstring => $group]; - } - return $popupcats; - } else { - return $categoriesarray; - } + return (new question_category_selector())->question_category_options( + $contexts, + $top, + $currentcat, + $popupform, + $nochildrenof, + $escapecontextnames, + ); } /** @@ -443,13 +345,7 @@ class helper { * @return array */ public static function question_add_context_in_key(array $categories): array { - $newcatarray = []; - foreach ($categories as $id => $category) { - $category->parent = "$category->parent,$category->contextid"; - $category->id = "$category->id,$category->contextid"; - $newcatarray["$id,$category->contextid"] = $category; - } - return $newcatarray; + return (new question_category_selector())->question_add_context_in_key($categories); } /** @@ -461,15 +357,7 @@ class helper { * @throws \coding_exception */ public static function question_fix_top_names(array $categories, bool $escape = true): array { - - foreach ($categories as $id => $category) { - if ($category->parent == 0) { - $context = \context::instance_by_id($category->contextid); - $categories[$id]->name = get_string('topfor', 'question', $context->get_context_name(false, false, $escape)); - } - } - - return $categories; + return (new question_category_selector())->question_fix_top_names($categories, $escape); } /** diff --git a/question/classes/external/search_shared_banks.php b/question/classes/external/search_shared_banks.php index 2b1418db37f..9293c2aaa23 100644 --- a/question/classes/external/search_shared_banks.php +++ b/question/classes/external/search_shared_banks.php @@ -16,6 +16,7 @@ namespace core_question\external; +use core\exception\coding_exception; use core_external\external_api; use core_external\external_function_parameters; use core_external\external_single_structure; @@ -49,34 +50,85 @@ class search_shared_banks extends external_api { public static function execute_parameters(): external_function_parameters { return new external_function_parameters( [ - 'contextid' => new external_value(PARAM_INT, 'The current context ID.'), + 'contextid' => new external_value(PARAM_INT, 'The current context ID for applying text filters to bank names.'), 'search' => new external_value(PARAM_TEXT, 'Search terms by which to filter the shared banks.', default: ''), + 'requiredcapabilities' => new external_multiple_structure( + new external_value(PARAM_TEXT, 'Capability'), + 'Array of abbreviated "moodle/question:" capabilities that the user must have at least one of in the context ' . + 'of each matching question bank. Valid options are "add", "managecategory", "flag", "config", "edit", ' . + '"view", "use", "move" or "tag". For "edit", "view", "use", "move" and "tag", the -all and -mine ' . + "suffixed versions of the capabilty will both be checked.", + VALUE_DEFAULT, + ['use'], + ), ] ); } + /** + * Expand a list of abbreviated capaibilities into an array of full capability strings. + * + * Each abbreviation must match a capability with the 'moodle/question:' prefix. Capabilities that have an "all" and "mine" + * variant will have both variants included in the returned array. + * + * These abbreviations are copied from {@see question_has_capability_on()} + * + * @param array $abbreviations Abbreviated capabilities. Must match capabilities with the 'moodle/question:' prefix. + * @return array The expanded capabilities + */ + protected static function expand_capabilities(array $abbreviations): array { + $capabilitieswithallandmine = ['edit', 'view', 'use', 'move', 'tag']; + $prefix = 'moodle/question:'; + $suffixes = ['all', 'mine']; + $capabilities = []; + foreach ($abbreviations as $abbreviation) { + if (in_array($abbreviation, $capabilitieswithallandmine)) { + foreach ($suffixes as $suffix) { + $capability = $prefix . $abbreviation . $suffix; + if (is_null(get_capability_info($capability))) { + throw new coding_exception("Capability {$capability} does not exist."); + } + $capabilities[] = $capability; + } + } else { + $capability = $prefix . $abbreviation; + if (is_null(get_capability_info($capability))) { + throw new coding_exception("Capability {$capability} does not exist."); + } + $capabilities[] = $capability; + } + } + return $capabilities; + } + /** * Return ID and formatted name of question banks accessible by the user, in courses other than the one $contextid is in. * * @param int $contextid Context ID of the current activity * @param string $search String to filter results by question bank name + * @param array $requiredcapabilities List of abbreviated capabilities to check, {@see self::expand_capabilities()} * @return array */ - public static function execute(int $contextid, string $search = ''): array { + public static function execute( + int $contextid, + string $search = '', + array $requiredcapabilities = ['use'], + ): array { [ 'contextid' => $contextid, 'search' => $search, + 'requiredcapabilities' => $requiredcapabilities, ] = self::validate_parameters(self::execute_parameters(), [ 'contextid' => $contextid, 'search' => $search, + 'requiredcapabilities' => $requiredcapabilities, ]); $modulecontext = context::instance_by_id($contextid); - $courseid = $modulecontext->get_parent_context()->instanceid; + self::validate_context($modulecontext); $sharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions( - notincourseids: [$courseid], - havingcap: ['moodle/question:useall', 'moodle/question:usemine'], + havingcap: self::expand_capabilities($requiredcapabilities), filtercontext: $modulecontext, search: $search, limit: self::MAX_RESULTS + 1, // Return up to 1 extra result, so we know there are more. @@ -112,7 +164,7 @@ class search_shared_banks extends external_api { return new external_single_structure([ 'sharedbanks' => new external_multiple_structure( new external_single_structure([ - 'value' => new external_value(PARAM_INT, 'Module ID of the shared bank.'), + 'value' => new external_value(PARAM_INT, 'Course Module ID of the shared bank.'), 'label' => new external_value(PARAM_TEXT, 'Formatted bank name'), ]), 'List of shared banks', diff --git a/question/classes/output/question_category_selector.php b/question/classes/output/question_category_selector.php new file mode 100644 index 00000000000..db064f30c2b --- /dev/null +++ b/question/classes/output/question_category_selector.php @@ -0,0 +1,399 @@ +. + +namespace core_question\output; + +use core\context; +use core\output\renderable; +use core\output\renderer_base; +use core\output\templatable; +use core_question\local\bank\question_version_status; + +/** + * A select menu of question categories. + * + * + * @package core_question + * @copyright 2025 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 question_category_selector implements renderable, templatable { + + /** + * Constructor. + * + * @param array $contexts + * @param bool $top + * @param string $currentcat + * @param string $selected + * @param int $nochildrenof + * @param bool $autocomplete + */ + public function __construct( + /** @var array The module contexts for the question banks to show category options for. */ + protected array $contexts = [], + /** @var bool If true, include top categories for each context in the options. */ + protected bool $top = false, + /** @var string The current category, to exclude from the list. */ + protected $currentcat = 0, + /** @var string The value of the initially selected option. */ + protected string $selected = "", + /** @var int If this matches the ID of a category in the list, don't include its children. */ + protected int $nochildrenof = -1, + /** @var bool If true, return options as a flattened array suitable for a list of autocomplete suggestions. */ + protected bool $autocomplete = false, + ) { + } + + /** + * Get all the category objects, including a count of the number of questions in that category, + * for all the categories in the lists $contexts. + * + * @param string $contexts comma separated list of contextids + * @param string $sortorder used as the ORDER BY clause in the select statement. + * @param bool $top Whether to return the top categories or not. + * @param int $showallversions 1 to show all versions not only the latest. + * @return array of category objects. + * @throws \dml_exception + */ + public function get_categories_for_contexts( + string $contexts, + string $sortorder = 'parent, sortorder, name ASC', + bool $top = false, + int $showallversions = 0, + ): array { + global $DB; + + $contextids = explode(',', $contexts); + foreach ($contextids as $contextid) { + $context = context::instance_by_id($contextid); + if ($context->contextlevel === CONTEXT_MODULE) { + $validcontexts[] = $contextid; + } + } + if (empty($validcontexts)) { + return []; + } + + [$insql, $inparams] = $DB->get_in_or_equal($validcontexts); + + $topwhere = $top ? '' : 'AND c.parent <> 0'; + $statuscondition = "AND (qv.status = '" . question_version_status::QUESTION_STATUS_READY . "' " . + " OR qv.status = '" . question_version_status::QUESTION_STATUS_DRAFT . "' )"; + $substatuscondition = "AND v.status <> '" . question_version_status::QUESTION_STATUS_HIDDEN . "' "; + $sql = "SELECT c.*, + (SELECT COUNT(1) + FROM {question} q + JOIN {question_versions} qv ON qv.questionid = q.id + JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid + WHERE q.parent = '0' + $statuscondition + AND c.id = qbe.questioncategoryid + AND ({$showallversions} = 1 + OR (qv.version = (SELECT MAX(v.version) + FROM {question_versions} v + JOIN {question_bank_entries} be ON be.id = v.questionbankentryid + WHERE be.id = qbe.id $substatuscondition) + ) + ) + ) AS questioncount + FROM {question_categories} c + WHERE c.contextid {$insql} {$topwhere} + ORDER BY {$sortorder}"; + + return $DB->get_records_sql($sql, $inparams); + } + + /** + * Output an array of question categories. + * + * @param array $contexts The list of contexts. + * @param bool $top Whether to return the top categories or not. + * @param int $currentcat The current category, to exclude from the list. + * @param bool $popupform Return each question bank's group in an additional nested array. + * @param int $nochildrenof Don't include children of this category + * @param bool $escapecontextnames Whether the returned name of the thing is to be HTML escaped or not. + * @return array + * @throws \coding_exception|\dml_exception + */ + public function question_category_options( + array $contexts, + bool $top = false, + int $currentcat = 0, + bool $popupform = false, + int $nochildrenof = -1, + bool $escapecontextnames = true, + ): array { + global $CFG; + $pcontexts = []; + foreach ($contexts as $context) { + if ($context->contextlevel !== CONTEXT_MODULE) { + continue; + } + $pcontexts[] = $context->id; + } + $contextslist = join(', ', $pcontexts); + + $categories = $this->get_categories_for_contexts($contextslist, 'parent, sortorder, name ASC', $top); + + if ($top) { + $categories = $this->question_fix_top_names($categories); + } + + $categories = $this->question_add_context_in_key($categories); + $categories = $this->add_indented_names($categories, $nochildrenof); + + // Sort cats out into different contexts. + $categoriesarray = []; + foreach ($pcontexts as $contextid) { + $context = context::instance_by_id($contextid); + $contextstring = $context->get_context_name(true, true, $escapecontextnames); + foreach ($categories as $category) { + if ($category->contextid == $contextid) { + $cid = $category->id; + if ("{$currentcat},{$contextid}" != $cid || $currentcat == 0) { + $a = new \stdClass(); + $a->name = format_string( + $category->indentedname, + true, + ['context' => $context] + ); + if ($category->idnumber !== null && $category->idnumber !== '') { + $a->idnumber = s($category->idnumber); + } + if (!empty($category->questioncount)) { + $a->questioncount = $category->questioncount; + } + if (isset($a->idnumber) && isset($a->questioncount)) { + $formattedname = get_string('categorynamewithidnumberandcount', 'question', $a); + } else if (isset($a->idnumber)) { + $formattedname = get_string('categorynamewithidnumber', 'question', $a); + } else if (isset($a->questioncount)) { + $formattedname = get_string('categorynamewithcount', 'question', $a); + } else { + $formattedname = $a->name; + } + $categoriesarray[$contextstring][$cid] = $formattedname; + } + } + } + } + if ($popupform) { + $popupcats = []; + foreach ($categoriesarray as $contextstring => $optgroup) { + $group = []; + foreach ($optgroup as $key => $value) { + $key = str_replace($CFG->wwwroot, '', $key); + $group[$key] = $value; + } + $popupcats[] = [$contextstring => $group]; + } + return $popupcats; + } else { + return $categoriesarray; + } + } + + /** + * Add context in categories key. + * + * @param array $categories The list of categories, keyed by ID. + * @return array The list with the context id added to each key, id, and parent attribute. + */ + public function question_add_context_in_key(array $categories): array { + $newcatarray = []; + foreach ($categories as $id => $category) { + $category->parent = "$category->parent,$category->contextid"; + $category->id = "$category->id,$category->contextid"; + $newcatarray["$id,$category->contextid"] = $category; + } + return $newcatarray; + } + + /** + * Finds top categories in the given categories hierarchy and replace their name with a proper localised string. + * + * @param array $categories An array of question categories. + * @param bool $escape Whether the returned name of the thing is to be HTML escaped or not. + * @return array The same question category list given to the function, with the top category names being translated. + * @throws \coding_exception + */ + public function question_fix_top_names(array $categories, bool $escape = true): array { + + foreach ($categories as $id => $category) { + if ($category->parent == 0) { + $context = context::instance_by_id($category->contextid); + $categories[$id]->name = get_string('topfor', 'question', $context->get_context_name(false, false, $escape)); + } + } + + return $categories; + } + + /** + * Format categories into an indented list reflecting the tree structure. + * + * @param array $categories An array of category objects, keyed by ID. + * @param int $nochildrenof If the category with this ID is in the list, don't include its children. + * @return array The formatted list of categories. + */ + public function add_indented_names(array $categories, int $nochildrenof = -1): array { + + // Add an array to each category to hold the child category ids. This array + // will be removed again by flatten_category_tree(). It should not be used + // outside these two functions. + foreach (array_keys($categories) as $id) { + $categories[$id]->childids = []; + } + + // Build the tree structure, and record which categories are top-level. + // We have to be careful, because the categories array may include published + // categories from other courses, but not their parents. + $toplevelcategoryids = []; + foreach (array_keys($categories) as $id) { + if ( + !empty($categories[$id]->parent) && + array_key_exists($categories[$id]->parent, $categories) + ) { + $categories[$categories[$id]->parent]->childids[] = $id; + } else { + $toplevelcategoryids[] = $id; + } + } + + // Flatten the tree to and add the indents. + $newcategories = []; + foreach ($toplevelcategoryids as $id) { + $newcategories = $newcategories + $this->flatten_category_tree( + $categories, + $id, + 0, + $nochildrenof, + ); + } + + return $newcategories; + } + + /** + * Only for the use of add_indented_names(). + * + * Recursively adds an indentedname field to each category, starting with the category + * with id $id, and dealing with that category and all its children, and + * return a new array, with those categories in the right order. + * + * @param array $categories an array of categories which has had childids + * fields added by flatten_category_tree(). Passed by reference for + * performance only. It is not modfied. + * @param int $id the category to start the indenting process from. + * @param int $depth the indent depth. Used in recursive calls. + * @param int $nochildrenof If the category with this ID is in the list, don't recur to its children. + * @return array a new array of categories, in the right order for the tree. + */ + public function flatten_category_tree(array &$categories, $id, int $depth = 0, int $nochildrenof = -1): array { + + // Indent the name of this category. + $newcategories = []; + $newcategories[$id] = $categories[$id]; + $newcategories[$id]->indentedname = str_repeat('   ', $depth) . + $categories[$id]->name; + + // Recursively indent the children. + foreach ($categories[$id]->childids as $childid) { + if ($childid != $nochildrenof) { + $newcategories = $newcategories + self::flatten_category_tree( + $categories, + $childid, + $depth + 1, + $nochildrenof, + ); + } + } + + // Remove the childids array that were temporarily added. + unset($newcategories[$id]->childids); + + return $newcategories; + } + + /** + * Context for the question_category_selector.mustache template. + * + * @param renderer_base $output + * @return array[] [ + * 'banks' => a 2-D array of question banks and categories, for a plain select list with optgroups. + * 'categories' => A flat list of categories, with bank names and disabled entries, for enhancing with an Autocomplete. + * ] + */ + public function export_for_template(renderer_base $output): array { + $categoriesarray = $this->question_category_options( + $this->contexts, + $this->top, + $this->currentcat, + false, + $this->nochildrenof, + false, + ); + + $bankoptgroups = []; + $categoryoptions = []; + if ($this->autocomplete) { + foreach ($categoriesarray as $bankname => $categories) { + $categoryoptions[] = [ + 'label' => $bankname, + 'value' => 0, + 'disabled' => true, + ]; + foreach ($categories as $idcontext => $category) { + $categoryoptions[] = [ + 'label' => $category, + 'value' => $idcontext, + 'selected' => $this->selected == $idcontext, + ]; + } + } + if (empty($selected) && isset($categoryoptions[1])) { + // Default to selecting the first category option. + $categoryoptions[1]['selected'] = 1; + } + } else { + foreach ($categoriesarray as $bankname => $categories) { + $bankoptgroups[] = [ + 'bankname' => $bankname, + 'categories' => array_map( + fn($idcontext, $category) => [ + 'idcontext' => $idcontext, + 'category' => $category, + 'selected' => $this->selected == $idcontext, + ], + array_keys($categories), + $categories, + ), + ]; + } + if (empty($selected) && isset($bankoptgroups[0]['categories'][0])) { + // Default to selecting the first category option. + $bankoptgroups[0]['categories'][0]['selected'] = 1; + } + } + + return [ + 'banks' => $bankoptgroups, + 'categories' => $categoryoptions, + ]; + } +} diff --git a/question/lib.php b/question/lib.php index d050c74168e..d5eef092518 100644 --- a/question/lib.php +++ b/question/lib.php @@ -86,3 +86,16 @@ function core_question_output_fragment_question_data(array $args): string { $questionbank->display_question_list(); return ob_get_clean(); } + +/** + * Render and return a category selector for the categories in a given question bank. + * + * @param array $args ['bankcmid' => Course module ID of the question bank] + * @return string The rendered selector. + */ +function core_question_output_fragment_category_selector(array $args): string { + global $OUTPUT; + $context = \core\context\module::instance($args['bankcmid']); + $selector = new \core_question\output\question_category_selector([$context], autocomplete: true); + return $OUTPUT->render($selector); +} diff --git a/question/templates/question_category_selector.mustache b/question/templates/question_category_selector.mustache new file mode 100644 index 00000000000..b1264824740 --- /dev/null +++ b/question/templates/question_category_selector.mustache @@ -0,0 +1,61 @@ +{{! + 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 . +}} +{{! + @template core_question/question_category_selector + + Template for core_question\output\question_category_selector + + Example context (json): + { + "categories": [ + { + "value": "0", + "disabled": true, + "label": "Bank name 1" + }, + { + "value": "123,456", + "selected": true, + "label": "Category 1" + }, + { + "value": "124,456", + "label": "Category 2" + } + ] + } +}} +
+ + +
diff --git a/question/tests/external/search_shared_banks_test.php b/question/tests/external/search_shared_banks_test.php new file mode 100644 index 00000000000..145f67466ec --- /dev/null +++ b/question/tests/external/search_shared_banks_test.php @@ -0,0 +1,217 @@ +. + +namespace core_question\external; + +use core\context\module; + +/** + * Unit tests for core_question\external\search_shared_banks + * + * @package core_question + * @copyright 2025 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 + * @covers \core_question\external\search_shared_banks + */ +final class search_shared_banks_test extends \advanced_testcase { + + /** + * Create a set of question banks across 3 courses. + * + * One bank belongs to a quiz, which is not searchable as it is not shared. + * One bank does not include the word "test" in its name, so will not match that search. + * One bank is on a course where the user has different permissions. + * One bank is on the same course as the quiz, so will not be returned unless banks on the same course are included. + * + * @return array + */ + protected function create_banks(): array { + $generator = $this->getDataGenerator(); + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + $qbankgenerator = $this->getDataGenerator()->get_plugin_generator('mod_qbank'); + + $course1 = $generator->create_course(); + $course2 = $generator->create_course(); + $course3 = $generator->create_course(); + $teacher = $generator->create_user(); + $generator->enrol_user($teacher->id, $course1->id, 'teacher'); + $generator->enrol_user($teacher->id, $course2->id, 'teacher'); + $generator->enrol_user($teacher->id, $course3->id, 'editingteacher'); + + $quiz = $quizgenerator->create_instance(['name' => 'Test quiz', 'course' => $course1->id]); + $qbank1 = $qbankgenerator->create_instance(['name' => 'Test qbank 1', 'course' => $course2->id]); + $qbank2 = $qbankgenerator->create_instance(['name' => 'Test qbank 2', 'course' => $course2->id]); + $qbank3 = $qbankgenerator->create_instance(['name' => 'Different name', 'course' => $course2->id]); + $qbank4 = $qbankgenerator->create_instance(['name' => 'Test qbank with different permissions', 'course' => $course3->id]); + + return [ + $course1, + $course2, + $course3, + $teacher, + module::instance($quiz->cmid), + $qbank1, + $qbank2, + $qbank3, + $qbank4, + ]; + } + + /** + * Call the function with no search string. All banks the user has permission to should be returned. + */ + public function test_empty_search(): void { + $this->resetAfterTest(); + [ + , + $course2, + $course3, + $teacher, + $quizcontext, + $qbank1, + $qbank2, + $qbank3, + $qbank4, + ] = $this->create_banks(); + + $this->setUser($teacher); + $result = search_shared_banks::execute($quizcontext->id); + + $this->assertEquals( + [ + [ + 'label' => "{$course2->shortname} - {$qbank1->name}", + 'value' => $qbank1->cmid, + ], + [ + 'label' => "{$course2->shortname} - {$qbank2->name}", + 'value' => $qbank2->cmid, + ], + [ + 'label' => "{$course2->shortname} - {$qbank3->name}", + 'value' => $qbank3->cmid, + ], + [ + 'label' => "{$course3->shortname} - {$qbank4->name}", + 'value' => $qbank4->cmid, + ], + ], + $result['sharedbanks'], + ); + } + + /** + * Call the function with a search string matching a subset of available banks. Only those matching should be returned. + */ + public function test_search(): void { + $this->resetAfterTest(); + [ + , + $course2, + $course3, + $teacher, + $quizcontext, + $qbank1, + $qbank2, + , + $qbank4, + ] = $this->create_banks(); + + $this->setUser($teacher); + $result = search_shared_banks::execute($quizcontext->id, 'Test'); + + $this->assertEquals( + [ + [ + 'label' => "{$course2->shortname} - {$qbank1->name}", + 'value' => $qbank1->cmid, + ], + [ + 'label' => "{$course2->shortname} - {$qbank2->name}", + 'value' => $qbank2->cmid, + ], + [ + 'label' => "{$course3->shortname} - {$qbank4->name}", + 'value' => $qbank4->cmid, + ], + ], + $result['sharedbanks'], + ); + } + + /** + * Call the function with a different capability. Only results where the user has that capability should be returned. + */ + public function test_search_different_capability(): void { + $this->resetAfterTest(); + [ + , + , + $course3, + $teacher, + $quizcontext, + , + , + , + $qbank4, + ] = $this->create_banks(); + + $this->setUser($teacher); + $result = search_shared_banks::execute($quizcontext->id, 'Test', ['edit']); + + $this->assertEquals( + [ + [ + 'label' => "{$course3->shortname} - {$qbank4->name}", + 'value' => $qbank4->cmid, + ], + ], + $result['sharedbanks'], + ); + } + + /** + * If there are more than the max number of results, a placeholder is returned at the end. + */ + public function test_search_max_results(): void { + $this->resetAfterTest(); + [ + , + $course2, + , + $teacher, + $quizcontext, + ] = $this->create_banks(); + + $qbankgenerator = $this->getDataGenerator()->get_plugin_generator('mod_qbank'); + for ($i = 1; $i <= search_shared_banks::MAX_RESULTS + 2; $i++) { + $qbankgenerator->create_instance(['name' => "Extra qbank {$i}", 'course' => $course2->id]); + } + + $this->setUser($teacher); + $result = search_shared_banks::execute($quizcontext->id, 'Extra'); + $this->assertCount(search_shared_banks::MAX_RESULTS + 1, $result['sharedbanks']); + $lastresult = end($result['sharedbanks']); + $this->assertEquals( + [ + 'label' => get_string('otherquestionbankstoomany', 'question', search_shared_banks::MAX_RESULTS), + 'value' => 0, + ], + $lastresult + ); + } +} diff --git a/version.php b/version.php index b95d01c26fa..c44a1f4d55c 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2025062900.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2025062900.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '5.1dev (Build: 20250629)'; // Human-friendly version name