From 20905a14813ec5eb9a3e6fcca22d1f90c5ebffbc Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 18 Aug 2020 09:01:14 +0100 Subject: [PATCH] MDL-70159 tool_capability: order capabilities by their name. --- admin/tool/capability/index.php | 2 +- lib/accesslib.php | 103 ++++++++++++++----------------- lib/tests/accesslib_test.php | 52 ++++++++++++++++ lib/tests/customcontext_test.php | 6 +- lib/upgrade.txt | 2 + 5 files changed, 105 insertions(+), 60 deletions(-) diff --git a/admin/tool/capability/index.php b/admin/tool/capability/index.php index 7f01576126f..1f392fab5e4 100644 --- a/admin/tool/capability/index.php +++ b/admin/tool/capability/index.php @@ -40,7 +40,7 @@ admin_externalpage_setup('toolcapability'); // Prepare the list of capabilities to choose from. $capabilitychoices = array(); -foreach ($context->get_capabilities() as $cap) { +foreach ($context->get_capabilities('name') as $cap) { $capabilitychoices[$cap->name] = $cap->name . ': ' . get_capability_string($cap->name); } diff --git a/lib/accesslib.php b/lib/accesslib.php index 46652ef487e..63e43412c02 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -4873,6 +4873,9 @@ function role_change_permission($roleid, $context, $capname, $permission) { */ abstract class context extends stdClass implements IteratorAggregate { + /** @var string Default sorting of capabilities in {@see get_capabilities} */ + protected const DEFAULT_CAPABILITY_SORT = 'contextlevel, component, name'; + /** * The context id * Can be accessed publicly through $context->id @@ -5559,9 +5562,10 @@ abstract class context extends stdClass implements IteratorAggregate { /** * Returns array of relevant context capability records. * + * @param string $sort SQL order by snippet for sorting returned capabilities sensibly for display * @return array */ - public abstract function get_capabilities(); + public abstract function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT); /** * Recursive function which, given a context, find all its children context ids. @@ -6186,8 +6190,10 @@ class context_helper extends context { /** * not used + * + * @param string $sort */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { } } @@ -6247,18 +6253,13 @@ class context_system extends context { /** * Returns array of relevant context capability records. * + * @param string $sort * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { global $DB; - $sort = 'ORDER BY contextlevel,component,name'; // To group them sensibly for display - - $params = array(); - $sql = "SELECT * - FROM {capabilities}"; - - return $DB->get_records_sql($sql.' '.$sort, $params); + return $DB->get_records('capabilities', [], $sort); } /** @@ -6522,21 +6523,17 @@ class context_user extends context { /** * Returns array of relevant context capability records. * + * @param string $sort * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { global $DB; - $sort = 'ORDER BY contextlevel,component,name'; // To group them sensibly for display - $extracaps = array('moodle/grade:viewall'); list($extra, $params) = $DB->get_in_or_equal($extracaps, SQL_PARAMS_NAMED, 'cap'); - $sql = "SELECT * - FROM {capabilities} - WHERE contextlevel = ".CONTEXT_USER." - OR name $extra"; - return $records = $DB->get_records_sql($sql.' '.$sort, $params); + return $DB->get_records_select('capabilities', "contextlevel = :level OR name {$extra}", + $params + ['level' => CONTEXT_USER], $sort); } /** @@ -6702,19 +6699,18 @@ class context_coursecat extends context { /** * Returns array of relevant context capability records. * + * @param string $sort * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { global $DB; - $sort = 'ORDER BY contextlevel,component,name'; // To group them sensibly for display - - $params = array(); - $sql = "SELECT * - FROM {capabilities} - WHERE contextlevel IN (".CONTEXT_COURSECAT.",".CONTEXT_COURSE.",".CONTEXT_MODULE.",".CONTEXT_BLOCK.")"; - - return $DB->get_records_sql($sql.' '.$sort, $params); + return $DB->get_records_list('capabilities', 'contextlevel', [ + CONTEXT_COURSECAT, + CONTEXT_COURSE, + CONTEXT_MODULE, + CONTEXT_BLOCK, + ], $sort); } /** @@ -6946,19 +6942,17 @@ class context_course extends context { /** * Returns array of relevant context capability records. * + * @param string $sort * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { global $DB; - $sort = 'ORDER BY contextlevel,component,name'; // To group them sensibly for display - - $params = array(); - $sql = "SELECT * - FROM {capabilities} - WHERE contextlevel IN (".CONTEXT_COURSE.",".CONTEXT_MODULE.",".CONTEXT_BLOCK.")"; - - return $DB->get_records_sql($sql.' '.$sort, $params); + return $DB->get_records_list('capabilities', 'contextlevel', [ + CONTEXT_COURSE, + CONTEXT_MODULE, + CONTEXT_BLOCK, + ], $sort); } /** @@ -7171,13 +7165,12 @@ class context_module extends context { /** * Returns array of relevant context capability records. * + * @param string $sort * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { global $DB, $CFG; - $sort = 'ORDER BY contextlevel,component,name'; // To group them sensibly for display - $cm = $DB->get_record('course_modules', array('id'=>$this->_instanceid)); $module = $DB->get_record('modules', array('id'=>$cm->module)); @@ -7251,9 +7244,10 @@ class context_module extends context { WHERE (contextlevel = ".CONTEXT_MODULE." AND component {$notcompsql} AND {$notlikesql}) - $extra"; + $extra + ORDER BY $sort"; - return $DB->get_records_sql($sql.' '.$sort, $params); + return $DB->get_records_sql($sql, $params); } /** @@ -7440,31 +7434,28 @@ class context_block extends context { /** * Returns array of relevant context capability records. * + * @param string $sort * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { global $DB; - $sort = 'ORDER BY contextlevel,component,name'; // To group them sensibly for display - - $params = array(); $bi = $DB->get_record('block_instances', array('id' => $this->_instanceid)); - $extra = ''; + $select = '(contextlevel = :level AND component = :component)'; + $params = [ + 'level' => CONTEXT_BLOCK, + 'component' => 'block_' . $bi->blockname, + ]; + $extracaps = block_method_result($bi->blockname, 'get_extra_capabilities'); if ($extracaps) { - list($extra, $params) = $DB->get_in_or_equal($extracaps, SQL_PARAMS_NAMED, 'cap'); - $extra = "OR name $extra"; + list($extra, $extraparams) = $DB->get_in_or_equal($extracaps, SQL_PARAMS_NAMED, 'cap'); + $select .= " OR name $extra"; + $params = array_merge($params, $extraparams); } - $sql = "SELECT * - FROM {capabilities} - WHERE (contextlevel = ".CONTEXT_BLOCK." - AND component = :component) - $extra"; - $params['component'] = 'block_' . $bi->blockname; - - return $DB->get_records_sql($sql.' '.$sort, $params); + return $DB->get_records_select('capabilities', $select, $params, $sort); } /** diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index a0f549cbf80..a36a6e4117b 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -3643,6 +3643,58 @@ class core_accesslib_testcase extends advanced_testcase { $this->assert_capability_list_contains($expectedcapabilities, $actual); } + /** + * Test that {@see context_block::get_capabilities} returns capabilities relevant to blocks + */ + public function test_context_block_caps_returned_by_get_capabilities_block_context(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $block = $this->getDataGenerator()->create_block('online_users', [ + 'parentcontextid' => context_course::instance($course->id)->id, + ]); + + $capabilities = context_block::instance($block->id)->get_capabilities(); + + // Just test a few representative capabilities. + $expected = ['block/online_users:addinstance', 'moodle/block:edit', 'moodle/block:view']; + $this->assert_capability_list_contains($expected, $capabilities); + + // Now test with different sorting. + $capabilitiesbyname = context_block::instance($block->id)->get_capabilities('riskbitmask'); + + $capabilitynames = array_column($capabilities, 'name'); + $capabilitynamesordered = array_column($capabilitiesbyname, 'name'); + + // Each array should contain the same data, ordered differently. + $this->assertEqualsCanonicalizing($capabilitynames, $capabilitynamesordered); + $this->assertNotSame($capabilitynames, $capabilitynamesordered); + } + + /** + * Test that {@see context_user::get_capabilities} returns capabilities relevant to users + */ + public function test_context_user_caps_returned_by_get_capabilities_user_context(): void { + $this->resetAfterTest(); + + $user = $this->getDataGenerator()->create_user(); + $capabilities = context_user::instance($user->id)->get_capabilities(); + + // Just test a few representative capabilities. + $expected = ['moodle/user:editmessageprofile', 'moodle/user:editprofile', 'moodle/user:viewalldetails']; + $this->assert_capability_list_contains($expected, $capabilities); + + // Now test with different sorting. + $capabilitiesbyname = context_user::instance($user->id)->get_capabilities('name'); + + $capabilitynames = array_column($capabilities, 'name'); + $capabilitynamesordered = array_column($capabilitiesbyname, 'name'); + + // Each array should contain the same data, ordered differently. + $this->assertEqualsCanonicalizing($capabilitynames, $capabilitynamesordered); + $this->assertNotSame($capabilitynames, $capabilitynamesordered); + } + /** * Test updating of role capabilities during upgrade */ diff --git a/lib/tests/customcontext_test.php b/lib/tests/customcontext_test.php index 7dcd228f7f2..8a3077b3098 100644 --- a/lib/tests/customcontext_test.php +++ b/lib/tests/customcontext_test.php @@ -45,7 +45,7 @@ class context_bogus1 extends context { * * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { return array(); } } @@ -69,7 +69,7 @@ class context_bogus2 extends context { * * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { return array(); } } @@ -93,7 +93,7 @@ class context_bogus3 extends context { * * @return array */ - public function get_capabilities() { + public function get_capabilities(string $sort = self::DEFAULT_CAPABILITY_SORT) { return array(); } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 9f0d6165360..5b8e70e4aa8 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -14,6 +14,8 @@ information provided here is intended especially for developers. * Behat timeout constants behat_base::TIMEOUT, EXTENDED_TIMEOUT, and REDUCED_TIMEOUT, which were deprecated in 3.7, have been removed. * \core_table\local\filter\filterset::JOINTYPE_DEFAULT is being changed from 1 (ANY) to 2 (ALL). Filterset implementations can override the default filterset join type by overriding \core_table\local\filter\filterset::get_join_type() instead. +* A new optional parameter `$sort` has been added to all `$context->get_capabilities()` methods to be able to define order of + returned capability array. === 3.10 === * PHPUnit has been upgraded to 8.5. That comes with a few changes: