Skip to content

Commit

Permalink
MDL-43721 Assign + groups: Improve performance of assign grading table
Browse files Browse the repository at this point in the history
Add a function to the groups lib to filter a list of users down
to the ones who can see the module. Required because calling
groups_course_module_visible() for a list of users is too slow
and we shouldn't spread group logic outside of grouplib.php.

Using it in the assign grading table reduces DB queries from 6198/1 to 256/3.

This is 12secs down to 2.5secs.

Conflicts:
	mod/assign/locallib.php
  • Loading branch information
Damyon Wiese committed Mar 6, 2014
1 parent 6a5effb commit 32c9c25
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 10 deletions.
71 changes: 71 additions & 0 deletions lib/grouplib.php
Expand Up @@ -790,6 +790,77 @@ function groups_get_activity_allowed_groups($cm,$userid=0) {
}
}

/**
* Filter a user list and return only the users that can see the course module based on
* groups/permissions etc. It is assumed that the users are pre-filtered to those who are enrolled in the course.
*
* @category group
* @param stdClass $cm The course module
* @param array $users An array of users, indexed by userid
* @return array A filtered list of users that can see the module, indexed by userid.
*/
function groups_filter_users_by_course_module_visible($cm, $users) {
global $CFG, $DB;

if (empty($CFG->enablegroupmembersonly)) {
return $users;
}
if (empty($cm->groupmembersonly)) {
return $users;
}
list($usql, $uparams) = $DB->get_in_or_equal(array_keys($users), SQL_PARAMS_NAMED, 'userid', true);

// Group membership sub-query.
if ($cm->groupingid) {
// Find out if member of any group in selected activity grouping.
$igsql = "SELECT gm.userid
FROM {groups_members} gm
LEFT JOIN {groupings_groups} gg
ON gm.groupid = gg.groupid
WHERE gm.userid $usql AND gg.groupingid = :groupingid";
$igparams = array_merge($uparams, array('groupingid' => $cm->groupingid));

} else {
// No grouping used - check all groups in course.
$igsql = "SELECT gm.userid
FROM {groups_members} gm
LEFT JOIN {groups} g
ON gm.groupid = g.id
WHERE gm.userid $usql AND g.courseid = :courseid";
$igparams = array_merge($uparams, array('courseid' => $cm->course));
}

$context = context_module::instance($cm->id);

// Get the list of users in a valid group.
$usersingroup = $DB->get_records_sql($igsql, $igparams);
if (!$usersingroup) {
$usersingroup = array();
} else {
$usersingroup = array_keys($usersingroup);
}

// Get the list of users who can access all groups.
list($accessallgroupssql, $accessallgroupsparams) = get_enrolled_sql($context, 'moodle/site:accessallgroups');

$userswithaccessallgroups = $DB->get_records_sql($accessallgroupssql, $accessallgroupsparams);
if (!$userswithaccessallgroups) {
$userswithaccessallgroups = array();
} else {
$userswithaccessallgroups = array_keys($userswithaccessallgroups);
}

// Explaining this array mangling:
// $users is an array of $user[$userid] => stdClass etc
// $userswithaccessallgroups is an array of $userswithaccessallgroups[random int] = $userid
// $usersingroup is an array of $usersingroup[random int] = $userid
// so - the inner array_merge combines the values of the 2 arrays disregarding the keys (because they are ints)
// this is then flipped so the values become the keys (and the values are now nonsense)
// this is then intersected with the users array only by looking at the keys - this
// returns only the users from the original array that had a value in one of the two $usersxxx lists.
return array_intersect_key($users, array_flip(array_merge($userswithaccessallgroups, $usersingroup)));
}

/**
* Determine if a course module is currently visible to a user
*
Expand Down
28 changes: 18 additions & 10 deletions mod/assign/locallib.php
Expand Up @@ -114,6 +114,9 @@ class assign {
/** @var string modulenameplural prevents excessive calls to get_string */
private static $modulenameplural = null;

/** @var array cached list of participants for this assignment. The cache key will be group, showactive and the context id */
private $participants = array();

/**
* Constructor for the base assign class.
*
Expand Down Expand Up @@ -1243,20 +1246,25 @@ class="quickgrade"/>';
* @return array List of user records
*/
public function list_participants($currentgroup, $idsonly) {
if ($idsonly) {
$users = get_enrolled_users($this->context, 'mod/assign:submit', $currentgroup, 'u.id');
} else {
$users = get_enrolled_users($this->context, 'mod/assign:submit', $currentgroup);
$key = $this->context->id . '-' . $currentgroup;
if (!isset($this->participants[$key])) {
$users = get_enrolled_users($this->context, 'mod/assign:submit', $currentgroup, 'u.*');

$cm = $this->get_course_module();
$users = groups_filter_users_by_course_module_visible($cm, $users);

$this->participants[$key] = $users;
}

$cm = $this->get_course_module();
foreach ($users as $userid => $user) {
if (!groups_course_module_visible($cm, $userid)) {
unset($users[$userid]);
if ($idsonly) {
$idslist = array();
foreach ($this->participants[$key] as $id => $user) {
$idslist[$id] = new stdClass();
$idslist[$id]->id = $id;
}
return $idslist;
}

return $users;
return $this->participants[$key];
}

/**
Expand Down

0 comments on commit 32c9c25

Please sign in to comment.