Merge branch 'MDL-86297-main' of https://github.com/PhMemmel/moodle
This commit is contained in:
@@ -19,6 +19,7 @@ namespace core_question\local\bank;
|
||||
use cm_info;
|
||||
use context;
|
||||
use context_course;
|
||||
use core\context_helper;
|
||||
use core\task\manager;
|
||||
use moodle_url;
|
||||
use stdClass;
|
||||
@@ -227,6 +228,16 @@ class question_bank_helper {
|
||||
$pluginssql = [];
|
||||
$params = [];
|
||||
|
||||
if ($getcategories || !empty($havingcap)) {
|
||||
$contextselect = ', ' . context_helper::get_preload_record_columns_sql('c');
|
||||
$contextsql = ' JOIN {context} c ON c.instanceid = cm.id AND c.contextlevel = ' . CONTEXT_MODULE . ' ';
|
||||
$contextgroupby = ', ' . implode(', ', array_keys(context_helper::get_preload_record_columns('c')));
|
||||
} else {
|
||||
$contextselect = '';
|
||||
$contextsql = '';
|
||||
$contextgroupby = '';
|
||||
}
|
||||
|
||||
// Build the SELECT portion of the SQL and include question category joins as required.
|
||||
if ($getcategories) {
|
||||
$concat = $DB->sql_concat('qc.id',
|
||||
@@ -237,8 +248,7 @@ class question_bank_helper {
|
||||
);
|
||||
$groupconcat = $DB->sql_group_concat($concat, self::CATEGORY_SEPARATOR);
|
||||
$select = "SELECT cm.id, cm.course, {$groupconcat} AS cats";
|
||||
$catsql = ' JOIN {context} c ON c.instanceid = cm.id AND c.contextlevel = ' . CONTEXT_MODULE .
|
||||
' JOIN {question_categories} qc ON qc.contextid = c.id AND qc.parent <> 0';
|
||||
$catsql = ' JOIN {question_categories} qc ON qc.contextid = c.id AND qc.parent <> 0';
|
||||
} else {
|
||||
$select = 'SELECT cm.id, cm.course';
|
||||
$catsql = '';
|
||||
@@ -296,22 +306,27 @@ class question_bank_helper {
|
||||
$orderbysql = '';
|
||||
}
|
||||
|
||||
$sql = "{$select}
|
||||
$sql = "{$select} {$contextselect}
|
||||
FROM {course_modules} cm
|
||||
JOIN {modules} m ON m.id = cm.module
|
||||
{$pluginssql}
|
||||
{$contextsql}
|
||||
{$catsql}
|
||||
WHERE 1=1 {$notincoursesql} {$incoursesql}
|
||||
GROUP BY cm.id, cm.course
|
||||
GROUP BY cm.id, cm.course {$contextgroupby}
|
||||
{$orderbysql}";
|
||||
|
||||
$rs = $DB->get_recordset_sql($sql, $params, limitnum: $limit);
|
||||
$limitforsql = $limit !== 0 && !empty($havingcap) ? 0 : $limit;
|
||||
$rs = $DB->get_recordset_sql($sql, $params, limitnum: $limitforsql);
|
||||
$banks = [];
|
||||
|
||||
foreach ($rs as $cm) {
|
||||
// If capabilities have been supplied as a method argument then ensure the viewing user has at least one of those
|
||||
// capabilities on the module itself.
|
||||
if (!empty($havingcap)) {
|
||||
// We can preload because we made sure that in case of capabilities being passed we have the context joined in the
|
||||
// SQL.
|
||||
context_helper::preload_from_record($cm);
|
||||
$context = \context_module::instance($cm->id);
|
||||
if (!(new question_edit_contexts($context))->have_one_cap($havingcap)) {
|
||||
continue;
|
||||
@@ -319,6 +334,9 @@ class question_bank_helper {
|
||||
}
|
||||
// Populate the raw record.
|
||||
$banks[] = self::get_formatted_bank($cm, $currentbankid, filtercontext: $filtercontext);
|
||||
if (!empty($limit) && count($banks) === $limit) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
$rs->close();
|
||||
|
||||
|
||||
@@ -160,6 +160,54 @@ final class question_bank_helper_test extends \advanced_testcase {
|
||||
$this->assertEquals(1, $count);
|
||||
}
|
||||
|
||||
/**
|
||||
* Tests if applying the limit and capability checks are interacting properly.
|
||||
*
|
||||
* @covers ::get_activity_instances_with_shareable_questions
|
||||
* @covers ::get_activity_instances_with_private_questions
|
||||
*/
|
||||
public function test_get_instances_with_limit_and_capabilities(): void {
|
||||
global $DB;
|
||||
$this->resetAfterTest();
|
||||
$course = $this->getDataGenerator()->create_course();
|
||||
$teacher = self::getDataGenerator()->create_user();
|
||||
self::setUser($teacher);
|
||||
$editingteacherroleid = $DB->get_record('role', ['shortname' => 'editingteacher'])->id;
|
||||
|
||||
$sharedmodgen = self::getDataGenerator()->get_plugin_generator('mod_qbank');
|
||||
// Create 20 question banks, and give the teacher permission to edit only in the last 5.
|
||||
for ($i = 0; $i < 20; $i++) {
|
||||
$sharedmod = $sharedmodgen->create_instance(['course' => $course]);
|
||||
if ($i >= 15) {
|
||||
role_assign($editingteacherroleid, $teacher->id, \context_module::instance($sharedmod->cmid));
|
||||
}
|
||||
}
|
||||
|
||||
// We now have created 20 banks. If the limit is below 20, we have to make sure that the code does NOT first apply a limit
|
||||
// of, for example, 15 and check capabilities afterward. This would mean we end up returning 0 qbanks.
|
||||
$sharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions(
|
||||
havingcap: ['moodle/question:add'],
|
||||
limit: 15
|
||||
);
|
||||
$this->assertCount(5, $sharedbanks);
|
||||
|
||||
// On the other hand, check if the limit parameter works at all and is being applied correctly.
|
||||
$sharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions(
|
||||
havingcap: ['moodle/question:add'],
|
||||
limit: 2
|
||||
);
|
||||
$this->assertCount(2, $sharedbanks);
|
||||
|
||||
$sharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions(limit: 10);
|
||||
$this->assertCount(10, $sharedbanks);
|
||||
|
||||
$sharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions(limit: 30);
|
||||
$this->assertCount(20, $sharedbanks);
|
||||
|
||||
$sharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions(limit: 0);
|
||||
$this->assertCount(20, $sharedbanks);
|
||||
}
|
||||
|
||||
/**
|
||||
* We should be able to filter sharable question bank instances by name.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user