diff --git a/lib/accesslib.php b/lib/accesslib.php index 1000d93aa94..376e512c5ca 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -4266,21 +4266,21 @@ function get_user_capability_contexts(string $capability, bool $getcategories, $ $fieldlist = \core\access\get_user_capability_course_helper::map_fieldnames($categoryfieldsexceptid); if ($categoryorderby) { $fields = explode(',', $categoryorderby); - $orderby = ''; + $categoryorderby = ''; foreach ($fields as $field) { - if ($orderby) { - $orderby .= ','; + if ($categoryorderby) { + $categoryorderby .= ','; } - $orderby .= 'c.'.$field; + $categoryorderby .= 'c.'.$field; } - $orderby = 'ORDER BY '.$orderby; + $categoryorderby = 'ORDER BY '.$categoryorderby; } $rs = $DB->get_recordset_sql(" SELECT c.id $fieldlist FROM {course_categories} c JOIN {context} x ON c.id = x.instanceid AND x.contextlevel = ? $contextlimitsql - $orderby", array_merge([CONTEXT_COURSECAT], $contextlimitparams)); + $categoryorderby", array_merge([CONTEXT_COURSECAT], $contextlimitparams)); $basedlimit = $limit; foreach ($rs as $category) { $categories[] = $category; @@ -4289,6 +4289,7 @@ function get_user_capability_contexts(string $capability, bool $getcategories, $ break; } } + $rs->close(); } $courses = []; diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index 220a1a7bc6a..9d23ca8c4fa 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -2468,6 +2468,8 @@ class accesslib_test extends advanced_testcase { /** * 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. + * + * @covers ::get_user_capability_contexts */ public function test_get_user_capability_contexts() { $this->resetAfterTest(); @@ -2495,9 +2497,9 @@ class accesslib_test extends advanced_testcase { assign_capability($cap, CAP_PROHIBIT, $prohibitroleid, $systemcontext->id); // Create three categories (two of them nested). - $cat1 = $generator->create_category(); - $cat2 = $generator->create_category(); - $cat3 = $generator->create_category(['parent' => $cat1->id]); + $cat1 = $generator->create_category(['name' => 'Aardvarks']); + $cat2 = $generator->create_category(['name' => 'Badgers']); + $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. assign_capability($cap, CAP_ALLOW, $emptyroleid, @@ -2518,7 +2520,7 @@ class accesslib_test extends advanced_testcase { $u1 = $generator->create_user(); // 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); // User 2 has allow role (system wide). @@ -2526,7 +2528,7 @@ class accesslib_test extends advanced_testcase { role_assign($allowroleid, $u2->id, $systemcontext->id); // 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. $this->assert_course_ids([$defaultcategoryid], $categories); @@ -2534,8 +2536,8 @@ class accesslib_test extends advanced_testcase { $u3 = $generator->create_user(); role_assign($emptyroleid, $u3->id, $systemcontext->id); - // Should get cat1 and cat3. cat2 is prohibited; no access to system level. - list($categories, $courses) = get_user_capability_contexts($cap, true, $u3->id, true, '', '', '', 'id'); + // 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, '', '', '', 'name'); $this->assert_course_ids([$cat1->id, $cat3->id], $categories); // 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. // 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); }