Skip to content

Commit

Permalink
MDL-48660 Availability: filter_user_list() should respect 'view hidden'
Browse files Browse the repository at this point in the history 
Updated filter_user_list and get_user_list_sql to account for
the viewhiddenactivities and viewhiddensections capabilities.
  • Loading branch information
@sammarshallou
sammarshallou committed Jan 27, 2015
1 parent da0ef2e commit 6b40e5b
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 8 deletions.
42 changes: 39 additions & 3 deletions availability/classes/info.php
View file
Expand Up @@ -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.
*
Expand All @@ -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));
}

/**
Expand Down
4 changes: 4 additions & 0 deletions availability/classes/info_module.php
View file
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions availability/classes/info_section.php
View file
Expand Up @@ -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,
Expand Down
13 changes: 11 additions & 2 deletions availability/classes/tree_node.php
View file
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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.
*
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions availability/tests/fixtures/mock_info.php
View file
Expand Up @@ -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) {
}

Expand Down
31 changes: 28 additions & 3 deletions availability/tests/info_test.php
View file
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}

0 comments on commit 6b40e5b

Please sign in to comment.