moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-04 16:36:37 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDL-77946 access: fix category sort order of capability contexts.

Browse source
This commit is contained in:
Paul Holden 2023-04-18 10:23:01 +01:00
parent 5d320dd7d1
commit ebcc7e58ce
No known key found for this signature in database
GPG key ID: A81A96D6045F6164
2 changed files with 17 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

13
lib/accesslib.php
View file

@ -4266,21 +4266,21 @@ function get_user_capability_contexts(string $capability, bool $getcategories, $
$fieldlist = \core\access\get_user_capability_course_helper::map_fieldnames($categoryfieldsexceptid); $fieldlist = \core\access\get_user_capability_course_helper::map_fieldnames($categoryfieldsexceptid);
if ($categoryorderby) { if ($categoryorderby) {
$fields = explode(',', $categoryorderby); $fields = explode(',', $categoryorderby);
$orderby = ''; $categoryorderby = '';
foreach ($fields as $field) { foreach ($fields as $field) {
if ($orderby) { if ($categoryorderby) {
$orderby .= ','; $categoryorderby .= ',';
} }
$orderby .= 'c.'.$field; $categoryorderby .= 'c.'.$field;
} }
$orderby = 'ORDER BY '.$orderby; $categoryorderby = 'ORDER BY '.$categoryorderby;
} }
$rs = $DB->get_recordset_sql(" $rs = $DB->get_recordset_sql("
SELECT c.id $fieldlist SELECT c.id $fieldlist
FROM {course_categories} c FROM {course_categories} c
JOIN {context} x ON c.id = x.instanceid AND x.contextlevel = ? JOIN {context} x ON c.id = x.instanceid AND x.contextlevel = ?
$contextlimitsql $contextlimitsql
$orderby", array_merge([CONTEXT_COURSECAT], $contextlimitparams)); $categoryorderby", array_merge([CONTEXT_COURSECAT], $contextlimitparams));
$basedlimit = $limit; $basedlimit = $limit;
foreach ($rs as $category) { foreach ($rs as $category) {
$categories[] = $category; $categories[] = $category;
@ -4289,6 +4289,7 @@ function get_user_capability_contexts(string $capability, bool $getcategories, $
break; break;
} }
} }
$rs->close();
} }
$courses = []; $courses = [];

18
lib/tests/accesslib_test.php
View file

@ -2468,6 +2468,8 @@ class accesslib_test extends advanced_testcase {
/** /**
* Tests get_user_capability_contexts() which checks a capability across all courses and categories. * Tests get_user_capability_contexts() which checks a capability across all courses and categories.
* Testing for categories only because courses results are covered by test_get_user_capability_course. * Testing for categories only because courses results are covered by test_get_user_capability_course.
*
* @covers ::get_user_capability_contexts
*/ */
public function test_get_user_capability_contexts() { public function test_get_user_capability_contexts() {
$this->resetAfterTest(); $this->resetAfterTest();
@ -2495,9 +2497,9 @@ class accesslib_test extends advanced_testcase {
assign_capability($cap, CAP_PROHIBIT, $prohibitroleid, $systemcontext->id); assign_capability($cap, CAP_PROHIBIT, $prohibitroleid, $systemcontext->id);
// Create three categories (two of them nested). // Create three categories (two of them nested).
$cat1 = $generator->create_category(); $cat1 = $generator->create_category(['name' => 'Aardvarks']);
$cat2 = $generator->create_category(); $cat2 = $generator->create_category(['name' => 'Badgers']);
$cat3 = $generator->create_category(['parent' => $cat1->id]); $cat3 = $generator->create_category(['parent' => $cat1->id, 'name' => 'Cheetahs']);
// Category overrides: in cat 1, empty role is allowed; in cat 2, empty role is prevented. // Category overrides: in cat 1, empty role is allowed; in cat 2, empty role is prevented.
assign_capability($cap, CAP_ALLOW, $emptyroleid, assign_capability($cap, CAP_ALLOW, $emptyroleid,
@ -2518,7 +2520,7 @@ class accesslib_test extends advanced_testcase {
$u1 = $generator->create_user(); $u1 = $generator->create_user();
// It returns false (annoyingly) if there are no course categories. // It returns false (annoyingly) if there are no course categories.
list($categories, $courses) = get_user_capability_contexts($cap, true, $u1->id, true, '', '', '', 'id'); list($categories, $courses) = get_user_capability_contexts($cap, true, $u1->id);
$this->assertFalse($categories); $this->assertFalse($categories);
// User 2 has allow role (system wide). // User 2 has allow role (system wide).
@ -2526,7 +2528,7 @@ class accesslib_test extends advanced_testcase {
role_assign($allowroleid, $u2->id, $systemcontext->id); role_assign($allowroleid, $u2->id, $systemcontext->id);
// Should get $defaultcategory only. cat2 is prohibited; cat1 is prevented, so cat3 is not allowed. // Should get $defaultcategory only. cat2 is prohibited; cat1 is prevented, so cat3 is not allowed.
list($categories, $courses) = get_user_capability_contexts($cap, true, $u2->id, true, '', '', '', 'id'); list($categories, $courses) = get_user_capability_contexts($cap, true, $u2->id);
// Using same assert_course_ids helper even when we are checking course category ids. // Using same assert_course_ids helper even when we are checking course category ids.
$this->assert_course_ids([$defaultcategoryid], $categories); $this->assert_course_ids([$defaultcategoryid], $categories);
@ -2534,8 +2536,8 @@ class accesslib_test extends advanced_testcase {
$u3 = $generator->create_user(); $u3 = $generator->create_user();
role_assign($emptyroleid, $u3->id, $systemcontext->id); role_assign($emptyroleid, $u3->id, $systemcontext->id);
// Should get cat1 and cat3. cat2 is prohibited; no access to system level. // Should get cat1 and cat3. cat2 is prohibited; no access to system level. Sorted by category name.
list($categories, $courses) = get_user_capability_contexts($cap, true, $u3->id, true, '', '', '', 'id'); list($categories, $courses) = get_user_capability_contexts($cap, true, $u3->id, true, '', '', '', 'name');
$this->assert_course_ids([$cat1->id, $cat3->id], $categories); $this->assert_course_ids([$cat1->id, $cat3->id], $categories);
// User 4 has prohibit role (system wide). // User 4 has prohibit role (system wide).
@ -2544,7 +2546,7 @@ class accesslib_test extends advanced_testcase {
// Should not get any, because all of them are prohibited at system level. // Should not get any, because all of them are prohibited at system level.
// Even if we try to allow an specific category. // Even if we try to allow an specific category.
list($categories, $courses) = get_user_capability_contexts($cap, true, $u4->id, true, '', '', '', 'id'); list($categories, $courses) = get_user_capability_contexts($cap, true, $u4->id);
$this->assertFalse($categories); $this->assertFalse($categories);
} }