From 6b40e5b2ecefd0fb400e393b012f1e1ed595c7a9 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Mon, 5 Jan 2015 15:46:09 +0000 Subject: [PATCH] MDL-48660 Availability: filter_user_list() should respect 'view hidden' Updated filter_user_list and get_user_list_sql to account for the viewhiddenactivities and viewhiddensections capabilities. --- availability/classes/info.php | 42 +++++++++++++++++++++-- availability/classes/info_module.php | 4 +++ availability/classes/info_section.php | 4 +++ availability/classes/tree_node.php | 13 +++++-- availability/tests/fixtures/mock_info.php | 4 +++ availability/tests/info_test.php | 31 +++++++++++++++-- 6 files changed, 90 insertions(+), 8 deletions(-) diff --git a/availability/classes/info.php b/availability/classes/info.php index fe0735be511ee..95363c9653e0d 100644 --- a/availability/classes/info.php +++ b/availability/classes/info.php @@ -608,17 +608,39 @@ public function filter_user_list(array $users) { } $tree = $this->get_availability_tree(); $checker = new capability_checker($this->get_context()); + + // Filter using availability tree. $this->modinfo = get_fast_modinfo($this->get_course()); - $result = $tree->filter_user_list($users, false, $this, $checker); + $filtered = $tree->filter_user_list($users, false, $this, $checker); $this->modinfo = null; + + // Include users in the result if they're either in the filtered list, + // or they have viewhidden. This logic preserves ordering of the + // passed users array. + $result = array(); + $canviewhidden = $checker->get_users_by_capability($this->get_view_hidden_capability()); + foreach ($users as $userid => $data) { + if (array_key_exists($userid, $filtered) || array_key_exists($userid, $canviewhidden)) { + $result[$userid] = $users[$userid]; + } + } + return $result; } + /** + * Gets the capability used to view hidden activities/sections (as + * appropriate). + * + * @return string Name of capability used to view hidden items of this type + */ + protected abstract function get_view_hidden_capability(); + /** * Obtains SQL that returns a list of enrolled users that has been filtered * by the conditions applied in the availability API, similar to calling * get_enrolled_users and then filter_user_list. As for filter_user_list, - * this ONLY filteres out users with conditions that are marked as applying + * this ONLY filters out users with conditions that are marked as applying * to user lists. For example, group conditions are included but date * conditions are not included. * @@ -640,8 +662,22 @@ public function get_user_list_sql($onlyactive) { if (is_null($this->availability) || !$CFG->enableavailability) { return array('', array()); } + + // Get SQL for the availability filter. $tree = $this->get_availability_tree(); - return $tree->get_user_list_sql(false, $this, $onlyactive); + list ($filtersql, $filterparams) = $tree->get_user_list_sql(false, $this, $onlyactive); + if ($filtersql === '') { + // No restrictions, so return empty query. + return array('', array()); + } + + // Get SQL for the view hidden list. + list ($viewhiddensql, $viewhiddenparams) = get_enrolled_sql( + $this->get_context(), $this->get_view_hidden_capability(), 0, $onlyactive); + + // Result is a union of the two. + return array('(' . $filtersql . ') UNION (' . $viewhiddensql . ')', + array_merge($filterparams, $viewhiddenparams)); } /** diff --git a/availability/classes/info_module.php b/availability/classes/info_module.php index 83403455529c3..5b1c8d4f358de 100644 --- a/availability/classes/info_module.php +++ b/availability/classes/info_module.php @@ -109,6 +109,10 @@ public function filter_user_list(array $users) { return parent::filter_user_list($filtered); } + protected function get_view_hidden_capability() { + return 'moodle/course:viewhiddenactivities'; + } + public function get_user_list_sql($onlyactive = true) { global $CFG, $DB; if (!$CFG->enableavailability) { diff --git a/availability/classes/info_section.php b/availability/classes/info_section.php index 4f98522c87997..767891b71f4e9 100644 --- a/availability/classes/info_section.php +++ b/availability/classes/info_section.php @@ -56,6 +56,10 @@ public function get_context() { return \context_course::instance($this->get_course()->id); } + protected function get_view_hidden_capability() { + return 'moodle/course:viewhiddensections'; + } + protected function set_in_database($availability) { global $DB; $DB->set_field('course_sections', 'availability', $availability, diff --git a/availability/classes/tree_node.php b/availability/classes/tree_node.php index a8471a01900b1..0c3e0e47a9ca4 100644 --- a/availability/classes/tree_node.php +++ b/availability/classes/tree_node.php @@ -139,7 +139,8 @@ public function is_applied_to_user_lists() { /** * Tests this condition against a user list. Users who do not meet the - * condition will be removed from the list. + * condition will be removed from the list, unless they have the ability + * to view hidden activities/sections. * * This function must be implemented if is_applied_to_user_lists returns * true. Otherwise it will not be called. @@ -150,6 +151,10 @@ public function is_applied_to_user_lists() { * Within this function, if you need to check capabilities, please use * the provided checker which caches results where possible. * + * Conditions do not need to check the viewhiddenactivities or + * viewhiddensections capabilities. These are handled by + * core_availability\info::filter_user_list. + * * @param array $users Array of userid => object * @param bool $not True if this condition is applying in negative mode * @param \core_availability\info $info Item we're checking @@ -167,7 +172,7 @@ public function filter_user_list(array $users, $not, * Obtains SQL that returns a list of enrolled users that has been filtered * by the conditions applied in the availability API, similar to calling * get_enrolled_users and then filter_user_list. As for filter_user_list, - * this ONLY filteres out users with conditions that are marked as applying + * this ONLY filters out users with conditions that are marked as applying * to user lists. For example, group conditions are included but date * conditions are not included. * @@ -180,6 +185,10 @@ public function filter_user_list(array $users, $not, * * If there are no conditions, the returned result is array('', array()). * + * Conditions do not need to check the viewhiddenactivities or + * viewhiddensections capabilities. These are handled by + * core_availability\info::get_user_list_sql. + * * @param bool $not True if this condition is applying in negative mode * @param \core_availability\info $info Item we're checking * @param bool $onlyactive If true, only returns active enrolments diff --git a/availability/tests/fixtures/mock_info.php b/availability/tests/fixtures/mock_info.php index f995c41482e6f..d7ef76e39d109 100644 --- a/availability/tests/fixtures/mock_info.php +++ b/availability/tests/fixtures/mock_info.php @@ -60,6 +60,10 @@ public function get_context() { return \context_course::instance($this->get_course()->id); } + protected function get_view_hidden_capability() { + return 'moodle/course:viewhiddensections'; + } + protected function set_in_database($availability) { } diff --git a/availability/tests/info_test.php b/availability/tests/info_test.php index d198df834a050..13091ecd72d00 100644 --- a/availability/tests/info_test.php +++ b/availability/tests/info_test.php @@ -407,11 +407,14 @@ public function test_filter_user_list() { $u1 = $generator->create_user(); $u2 = $generator->create_user(); $u3 = $generator->create_user(); + $studentroleid = $DB->get_field('role', 'id', array('shortname' => 'student'), MUST_EXIST); $allusers = array($u1->id => $u1, $u2->id => $u2, $u3->id => $u3); - $generator->enrol_user($u1->id, $course->id); - $generator->enrol_user($u2->id, $course->id); - $generator->enrol_user($u3->id, $course->id); + $generator->enrol_user($u1->id, $course->id, $studentroleid); + $generator->enrol_user($u2->id, $course->id, $studentroleid); + $generator->enrol_user($u3->id, $course->id, $studentroleid); + // Page 2 allows access to users 2 and 3, while section 2 allows access + // to users 1 and 2. $pagegen = $generator->get_plugin_generator('mod_page'); $page = $pagegen->create_instance(array('course' => $course)); $page2 = $pagegen->create_instance(array('course' => $course, @@ -483,5 +486,27 @@ public function test_filter_user_list() { $result = $DB->get_fieldset_sql($sql, $params); sort($result); $this->assertEquals($expected, $result); + + // If the students have viewhiddenactivities, they get past the module + // restriction. + role_change_permission($studentroleid, context_module::instance($page2->cmid), + 'moodle/course:viewhiddenactivities', CAP_ALLOW); + $expected = array($u1->id, $u2->id); + $this->assertEquals($expected, array_keys($info->filter_user_list($allusers))); + list ($sql, $params) = $info->get_user_list_sql(true); + $result = $DB->get_fieldset_sql($sql, $params); + sort($result); + $this->assertEquals($expected, $result); + + // If they have viewhiddensections, they also get past the section + // restriction. + role_change_permission($studentroleid, context_course::instance($course->id), + 'moodle/course:viewhiddensections', CAP_ALLOW); + $expected = array($u1->id, $u2->id, $u3->id); + $this->assertEquals($expected, array_keys($info->filter_user_list($allusers))); + list ($sql, $params) = $info->get_user_list_sql(true); + $result = $DB->get_fieldset_sql($sql, $params); + sort($result); + $this->assertEquals($expected, $result); } }