MDL-48660 Availability: filter_user_list() should respect 'view hidden'
authorsam marshall <s.marshall@open.ac.uk>
Mon, 5 Jan 2015 15:46:09 +0000 (15:46 +0000)
committersam marshall <s.marshall@open.ac.uk>
Tue, 27 Jan 2015 10:59:05 +0000 (10:59 +0000)
Updated filter_user_list and get_user_list_sql to account for
the viewhiddenactivities and viewhiddensections capabilities.

availability/classes/info.php
availability/classes/info_module.php
availability/classes/info_section.php
availability/classes/tree_node.php
availability/tests/fixtures/mock_info.php
availability/tests/info_test.php

index fe0735b..95363c9 100644 (file)
@@ -608,17 +608,39 @@ abstract class info {
         }
         $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 @@ abstract class info {
         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));
     }
 
     /**
index 8340345..5b1c8d4 100644 (file)
@@ -109,6 +109,10 @@ class info_module extends info {
         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) {
index 4f98522..767891b 100644 (file)
@@ -56,6 +56,10 @@ class info_section extends info {
         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,
index a8471a0..0c3e0e4 100644 (file)
@@ -139,7 +139,8 @@ abstract class tree_node {
 
     /**
      * 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 @@ abstract class tree_node {
      * 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 @@ abstract class tree_node {
      * 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 @@ abstract class tree_node {
      *
      * 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
index f995c41..d7ef76e 100644 (file)
@@ -60,6 +60,10 @@ class mock_info extends info {
         return \context_course::instance($this->get_course()->id);
     }
 
+    protected function get_view_hidden_capability() {
+        return 'moodle/course:viewhiddensections';
+    }
+
     protected function set_in_database($availability) {
     }
 
index d198df8..13091ec 100644 (file)
@@ -407,11 +407,14 @@ class info_testcase extends \advanced_testcase {
         $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 @@ class info_testcase extends \advanced_testcase {
         $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);
     }
 }