From 341b7245df3c53e738b689e80f1794fc172f9d28 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 29 Jan 2021 20:47:57 +0000 Subject: [PATCH] MDL-70741 capability overview: avoid errors from orphan contexts --- admin/tool/capability/locallib.php | 13 ++- admin/tool/capability/tests/locallib_test.php | 88 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 admin/tool/capability/tests/locallib_test.php diff --git a/admin/tool/capability/locallib.php b/admin/tool/capability/locallib.php index b27a7ce3461..0e51ed67b0f 100644 --- a/admin/tool/capability/locallib.php +++ b/admin/tool/capability/locallib.php @@ -53,7 +53,12 @@ function tool_capability_calculate_role_data($capability, array $roles) { $sql = 'SELECT DISTINCT con.path, 1 FROM {context} con JOIN {role_capabilities} rc ON rc.contextid = con.id - WHERE capability = ? '.$sqlroletest; + WHERE capability = ?' . + $sqlroletest . + // Context path should never be null, but can happen in old database with + // bad data (e.g. a course_module where the corresponding course no longer exists). + // We need to leave these out of the report to prevent errors. + ' AND con.path IS NOT NULL'; $relevantpaths = $DB->get_records_sql_menu($sql, $params); $requiredcontexts = array($systemcontext->id); foreach ($relevantpaths as $path => $notused) { @@ -88,6 +93,10 @@ function tool_capability_calculate_role_data($capability, array $roles) { // Put the role capabilities into the context tree. foreach ($rolecaps as $rolecap) { + if (!isset($contexts[$rolecap->contextid])) { + // Skip capabilities in orphaned contexts that are not in the tree. + continue; + } $contexts[$rolecap->contextid]->rolecapabilities[$rolecap->roleid] = $rolecap->permission; } @@ -99,4 +108,4 @@ function tool_capability_calculate_role_data($capability, array $roles) { } return $contexts; -} \ No newline at end of file +} diff --git a/admin/tool/capability/tests/locallib_test.php b/admin/tool/capability/tests/locallib_test.php new file mode 100644 index 00000000000..cf4df06a493 --- /dev/null +++ b/admin/tool/capability/tests/locallib_test.php @@ -0,0 +1,88 @@ +. + +/** + * Tests for the capability overview helper functions. + * + * @package tool_capability + * @copyright 2021 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ + +defined('MOODLE_INTERNAL') || die(); +global $CFG; +require_once($CFG->dirroot . '/' . $CFG->admin . '/tool/capability/locallib.php'); + + +/** + * Tests for the capability overview helper functions. + */ +class tool_capability_locallib_testcase extends advanced_testcase { + + /** + * Test the function that gets the data - simple case. + */ + public function test_tool_capability_calculate_role_data() { + global $DB; + + $data = tool_capability_calculate_role_data('mod/quiz:attempt', get_all_roles()); + + $systcontext = context_system::instance(); + $studentroleid = $DB->get_field('role', 'id', ['shortname' => 'student']); + + $this->assertArrayHasKey($systcontext->id, $data); + $this->assertCount(1, $data); + foreach ($data[$systcontext->id]->rolecapabilities as $roleid => $permission) { + if ($roleid == $studentroleid) { + $this->assertEquals(CAP_ALLOW, $permission); + } else { + $this->assertEquals(CAP_INHERIT, $permission); + } + } + } + + /** + * Test the function that gets the data - simple case. + */ + public function test_tool_capability_calculate_role_data_orphan_contexts() { + global $DB; + $this->resetAfterTest(); + + // This simulates a situation that seems to happen sometimes, where + // we end up with contexts with path = NULL in the database. + $systcontext = context_system::instance(); + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $coursecontext = context_course::instance($course->id); + $studentroleid = $DB->get_field('role', 'id', ['shortname' => 'student']); + role_change_permission($studentroleid, $coursecontext, 'mod/quiz:attempt', CAP_PREVENT); + // This is where we simulate the breakage. + $DB->set_field('context', 'path', null, ['id' => $coursecontext->id]); + + // Now call the function. We mainly just want to know there is no exception. + $data = tool_capability_calculate_role_data('mod/quiz:attempt', get_all_roles()); + + $this->assertArrayHasKey($systcontext->id, $data); + $this->assertCount(1, $data); + foreach ($data[$systcontext->id]->rolecapabilities as $roleid => $permission) { + if ($roleid == $studentroleid) { + $this->assertEquals(CAP_ALLOW, $permission); + } else { + $this->assertEquals(CAP_INHERIT, $permission); + } + } + } +}