From 62600aaac63e3113dd3859acf84f5248afdebb72 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Wed, 28 Oct 2015 12:48:02 +0800 Subject: [PATCH] MDL-51416 enrol: exclude suspended users from groups when syncing --- enrol/cohort/locallib.php | 2 +- enrol/meta/locallib.php | 13 +++++++++---- enrol/meta/tests/plugin_test.php | 10 +++++++++- group/lib.php | 20 +++++++++++++++----- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/enrol/cohort/locallib.php b/enrol/cohort/locallib.php index fc8b4bf..fe92176 100644 --- a/enrol/cohort/locallib.php +++ b/enrol/cohort/locallib.php @@ -286,7 +286,7 @@ function enrol_cohort_sync(progress_trace $trace, $courseid = NULL) { // Finally sync groups. - $affectedusers = groups_sync_with_enrolment('cohort', $courseid); + $affectedusers = groups_sync_with_enrolment('cohort', $courseid, 'customint2', true); foreach ($affectedusers['removed'] as $gm) { $trace->output("removing user from group: $gm->userid ==> $gm->courseid - $gm->groupname", 1); } diff --git a/enrol/meta/locallib.php b/enrol/meta/locallib.php index a7af91c..f361ff1 100644 --- a/enrol/meta/locallib.php +++ b/enrol/meta/locallib.php @@ -173,13 +173,17 @@ class enrol_meta_handler { $ue->userid = $userid; $ue->enrolid = $instance->id; $ue->status = $parentstatus; - if ($instance->customint2) { - groups_add_member($instance->customint2, $userid, 'enrol_meta', $instance->id); - } } $unenrolaction = $plugin->get_config('unenrolaction', ENROL_EXT_REMOVED_SUSPENDNOROLES); + // Make sure user group membership is correct. + if ($instance->customint2) { + // In case + $onlyactiveusers = $unenrolaction != ENROL_EXT_REMOVED_SUSPEND; + groups_sync_with_enrolment('meta', $instance->courseid, 'customint2', $onlyactiveusers, $userid); + } + // Only active users in enabled instances are supposed to have roles (we can reassign the roles any time later). if ($ue->status != ENROL_USER_ACTIVE or $instance->status != ENROL_INSTANCE_ENABLED or ($parenttimeend and $parenttimeend < time()) or ($parenttimestart > time())) { @@ -592,7 +596,8 @@ function enrol_meta_sync($courseid = NULL, $verbose = false) { } // Finally sync groups. - $affectedusers = groups_sync_with_enrolment('meta', $courseid); + $onlyactiveusers = $unenrolaction != ENROL_EXT_REMOVED_SUSPEND; + $affectedusers = groups_sync_with_enrolment('meta', $courseid, 'customint2', $onlyactiveusers); if ($verbose) { foreach ($affectedusers['removed'] as $gm) { mtrace("removing user from group: $gm->userid ==> $gm->courseid - $gm->groupname", 1); diff --git a/enrol/meta/tests/plugin_test.php b/enrol/meta/tests/plugin_test.php index 9f0fb8a..852ab1c 100644 --- a/enrol/meta/tests/plugin_test.php +++ b/enrol/meta/tests/plugin_test.php @@ -519,7 +519,7 @@ class enrol_meta_plugin_testcase extends advanced_testcase { $this->assertFalse(groups_is_member($group32->id, $user1->id)); $this->assertFalse(is_enrolled(context_course::instance($course3->id), $user1)); - set_config('unenrolaction', ENROL_EXT_REMOVED_SUSPENDNOROLES, 'enrol_meta'); + set_config('unenrolaction', ENROL_EXT_REMOVED_SUSPEND, 'enrol_meta'); // When user is unenrolled in this case, he is still a member of a group (but enrolment is suspended). enrol_get_plugin('manual')->unenrol_user($manualenrol1, $user4->id); @@ -530,6 +530,14 @@ class enrol_meta_plugin_testcase extends advanced_testcase { $this->assertTrue(groups_is_member($group31->id, $user4->id)); $this->assertTrue(is_enrolled(context_course::instance($course3->id), $user4)); $this->assertFalse(is_enrolled(context_course::instance($course3->id), $user4, '', true)); + + set_config('unenrolaction', ENROL_EXT_REMOVED_SUSPENDNOROLES, 'enrol_meta'); + + // When user is unenrolled from parent course in this case, he is still enrolled in this course (suspended) but not a member of a group. + enrol_meta_sync(null, false); + $this->assertFalse(groups_is_member($group31->id, $user4->id)); + $this->assertTrue(is_enrolled(context_course::instance($course3->id), $user4)); + $this->assertFalse(is_enrolled(context_course::instance($course3->id), $user4, '', true)); } /** diff --git a/group/lib.php b/group/lib.php index 27ee1c3..b88be07 100644 --- a/group/lib.php +++ b/group/lib.php @@ -1031,16 +1031,26 @@ function groups_calculate_role_people($rs, $context) { * @param string $enrolname name of enrolment method without prefix * @param int $courseid course id where sync needs to be performed (0 for all courses) * @param string $gidfield name of the field in 'enrol' table that stores group id + * @param bool $onlyactiveusers only users with active enrolments must be added to the group + * @param int $userid individual user that needs to be checked. This parameter was added in Moodle 2.9.3, 2.8.9 and 2.7.11 * @return array Returns the list of removed and added users. Each record contains fields: * userid, enrolid, courseid, groupid, groupname */ -function groups_sync_with_enrolment($enrolname, $courseid = 0, $gidfield = 'customint2') { +function groups_sync_with_enrolment($enrolname, $courseid = 0, $gidfield = 'customint2', $onlyactiveusers = false, $userid = 0) { global $DB; $onecourse = $courseid ? "AND e.courseid = :courseid" : ""; + $oneuser = $userid ? "AND ue.userid = :userid" :""; + $active = $onlyactiveusers ? "AND e.status = :enabled AND ue.status = :active AND (ue.timeend = 0 OR ue.timeend >= :curtime1) AND ue.timestart <= :curtime2" : ""; + $time = time(); $params = array( 'enrolname' => $enrolname, 'component' => 'enrol_'.$enrolname, - 'courseid' => $courseid + 'courseid' => $courseid, + 'active' => ENROL_USER_ACTIVE, + 'enabled' => ENROL_INSTANCE_ENABLED, + 'curtime1' => $time, + 'curtime2' => $time, + 'userid' => $userid ); $affectedusers = array( @@ -1053,8 +1063,8 @@ function groups_sync_with_enrolment($enrolname, $courseid = 0, $gidfield = 'cust FROM {groups_members} gm JOIN {groups} g ON (g.id = gm.groupid) JOIN {enrol} e ON (e.enrol = :enrolname AND e.courseid = g.courseid $onecourse) - JOIN {user_enrolments} ue ON (ue.userid = gm.userid AND ue.enrolid = e.id) - WHERE gm.component=:component AND gm.itemid = e.id AND g.id <> e.{$gidfield}"; + JOIN {user_enrolments} ue ON (ue.userid = gm.userid AND ue.enrolid = e.id $oneuser) + WHERE gm.component=:component AND gm.itemid = e.id AND NOT (g.id = e.{$gidfield} {$active})"; $rs = $DB->get_recordset_sql($sql, $params); foreach ($rs as $gm) { @@ -1070,7 +1080,7 @@ function groups_sync_with_enrolment($enrolname, $courseid = 0, $gidfield = 'cust JOIN {groups} g ON (g.courseid = e.courseid AND g.id = e.{$gidfield}) JOIN {user} u ON (u.id = ue.userid AND u.deleted = 0) LEFT JOIN {groups_members} gm ON (gm.groupid = g.id AND gm.userid = ue.userid) - WHERE gm.id IS NULL"; + WHERE gm.id IS NULL {$oneuser} {$active}"; $rs = $DB->get_recordset_sql($sql, $params); foreach ($rs as $ue) { -- 1.9.1