From 7b96ffb0449b6f1b458a154fa262fe9e292ecf5f Mon Sep 17 00:00:00 2001 From: Niclas Tollstorff Date: Thu, 11 Feb 2016 10:23:22 +0800 Subject: [PATCH 1/3] MDL-53072 group: added 'only active' option when auto-creating groups --- group/autogroup.php | 8 +++++++- group/autogroup_form.php | 9 ++++++++- group/lib.php | 5 +++-- lang/en/group.php | 2 ++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/group/autogroup.php b/group/autogroup.php index 8270ff326cc..e9921352f76 100644 --- a/group/autogroup.php +++ b/group/autogroup.php @@ -95,7 +95,13 @@ if ($editform->is_cancelled()) { if ($data->groupid) { $source['groupid'] = $data->groupid; } - $users = groups_get_potential_members($data->courseid, $data->roleid, $source, $orderby, !empty($data->notingroup)); + + $onlyactive = true; + if (!$data->includeonlyactiveenrol && has_capability('moodle/course:viewsuspendedusers', $context)) { + $onlyactive = false; + } + + $users = groups_get_potential_members($data->courseid, $data->roleid, $source, $orderby, !empty($data->notingroup), $onlyactive); $usercnt = count($users); if ($data->allocateby == 'random') { diff --git a/group/autogroup_form.php b/group/autogroup_form.php index a04da015a47..8e17c173878 100644 --- a/group/autogroup_form.php +++ b/group/autogroup_form.php @@ -82,7 +82,8 @@ class autogroup_form extends moodleform { $mform->setDefault('roleid', $student->id); } - if ($cohorts = cohort_get_available_cohorts(context_course::instance($COURSE->id), COHORT_WITH_ENROLLED_MEMBERS_ONLY, 0, 0)) { + $coursecontext = context_course::instance($COURSE->id); + if ($cohorts = cohort_get_available_cohorts($coursecontext, COHORT_WITH_ENROLLED_MEMBERS_ONLY)) { $options = array(0 => get_string('anycohort', 'cohort')); foreach ($cohorts as $c) { $options[$c->id] = format_string($c->name, true, context::instance_by_id($c->contextid)); @@ -140,6 +141,12 @@ class autogroup_form extends moodleform { $mform->disabledIf('notingroup', 'groupingid', 'neq', 0); $mform->disabledIf('notingroup', 'groupid', 'neq', 0); + if (has_capability('moodle/course:viewsuspendedusers', $coursecontext)) { + $mform->addElement('checkbox', 'includeonlyactiveenrol', get_string('includeonlyactiveenrol', 'group'), ''); + $mform->addHelpButton('includeonlyactiveenrol', 'includeonlyactiveenrol', 'group'); + $mform->setDefault('includeonlyactiveenrol', true); + } + $mform->addElement('header', 'groupinghdr', get_string('grouping', 'group')); $options = array('0' => get_string('nogrouping', 'group'), diff --git a/group/lib.php b/group/lib.php index 3178e7b2f79..3af8deb5b3a 100644 --- a/group/lib.php +++ b/group/lib.php @@ -717,16 +717,17 @@ function groups_get_possible_roles($context) { * @param mixed $source restrict to cohort, grouping or group id * @param string $orderby The column to sort users by * @param int $notingroup restrict to users not in existing groups + * @param bool $onlyactiveenrolments restrict to users who have an active enrolment in the course * @return array An array of the users */ function groups_get_potential_members($courseid, $roleid = null, $source = null, $orderby = 'lastname ASC, firstname ASC', - $notingroup = null) { + $notingroup = null, $onlyactiveenrolments = true) { global $DB; $context = context_course::instance($courseid); - list($esql, $params) = get_enrolled_sql($context); + list($esql, $params) = get_enrolled_sql($context, '', 0, $onlyactiveenrolments); $notingroupsql = ""; if ($notingroup) { diff --git a/lang/en/group.php b/lang/en/group.php index 17007edf5dd..464ee741061 100644 --- a/lang/en/group.php +++ b/lang/en/group.php @@ -137,6 +137,8 @@ $string['importgroups_help'] = 'Groups may be imported via text file. The format * Required fieldname is groupname * Optional fieldnames are description, enrolmentkey, picture, hidepicture'; $string['importgroups_link'] = 'group/import'; +$string['includeonlyactiveenrol'] = 'Include only active enrolments'; +$string['includeonlyactiveenrol_help'] = 'This setting determines if only active enrolled users will be included in the groups. If enabled, suspended users will not be included in groups.'; $string['javascriptrequired'] = 'This page requires JavaScript to be enabled.'; $string['members'] = 'Members per group'; $string['membersofselectedgroup'] = 'Members of:'; From 4cb32a243da7fdbf49fa64b58b7ec8b191ea86ca Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Wed, 3 Dec 2014 23:39:19 -0800 Subject: [PATCH 2/3] MDL-53072 group: added related behat tests --- group/tests/behat/auto_creation.feature | 50 ++++++++++++++++--------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/group/tests/behat/auto_creation.feature b/group/tests/behat/auto_creation.feature index f47db8e920f..d6754565558 100644 --- a/group/tests/behat/auto_creation.feature +++ b/group/tests/behat/auto_creation.feature @@ -11,7 +11,6 @@ Feature: Automatic creation of groups And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@example.com | - | student0 | Student | 0 | student0@example.com | | student1 | Student | 1 | student1@example.com | | student2 | Student | 2 | student2@example.com | | student3 | Student | 3 | student3@example.com | @@ -21,19 +20,22 @@ Feature: Automatic creation of groups | student7 | Student | 7 | student7@example.com | | student8 | Student | 8 | student8@example.com | | student9 | Student | 9 | student9@example.com | + | student10 | Student | 10 | student10@example.com | + | suspendedstudent11 | Suspended student | 11 | suspendedstudent11@example.com | And the following "course enrolments" exist: - | user | course | role | - | teacher1 | C1 | editingteacher | - | student0 | C1 | student | - | student1 | C1 | student | - | student2 | C1 | student | - | student3 | C1 | student | - | student4 | C1 | student | - | student5 | C1 | student | - | student6 | C1 | student | - | student7 | C1 | student | - | student8 | C1 | student | - | student9 | C1 | student | + | user | course | role | status | + | teacher1 | C1 | editingteacher | 0 | + | student1 | C1 | student | 0 | + | student2 | C1 | student | 0 | + | student3 | C1 | student | 0 | + | student4 | C1 | student | 0 | + | student5 | C1 | student | 0 | + | student6 | C1 | student | 0 | + | student7 | C1 | student | 0 | + | student8 | C1 | student | 0 | + | student9 | C1 | student | 0 | + | student10 | C1 | student | 0 | + | suspendedstudent11 | C1 | student | 1 | And I log in as "teacher1" And I follow "Course 1" And I expand "Users" node @@ -96,10 +98,10 @@ Feature: Automatic creation of groups And I set the following fields to these values: | Group name | Group 2 | And I press "Save changes" - When I add "Student 0" user to "Group 1" group members - And I add "Student 1" user to "Group 1" group members - And I add "Student 2" user to "Group 2" group members + When I add "Student 1" user to "Group 1" group members + And I add "Student 2" user to "Group 1" group members And I add "Student 3" user to "Group 2" group members + And I add "Student 4" user to "Group 2" group members And I press "Auto-create groups" And I expand all fieldsets And I set the field "Auto create based on" to "Number of groups" @@ -138,4 +140,18 @@ Feature: Automatic creation of groups | Grouping of auto-created groups | No grouping | And I press "Submit" And the "groups" select box should contain "Test 1 (3)" - And the "groups" select box should contain "Test 2 (2)" \ No newline at end of file + And the "groups" select box should contain "Test 2 (2)" + + Scenario: Exclude suspended users when auto-creating groups + Given I set the field "Include only active enrolments" to "1" + And I set the field "Auto create based on" to "Members per group" + When I set the field "Group/member count" to "11" + And I press "Preview" + Then I should not see "Suspended Student 11" + + Scenario: Include suspended users when auto-creating groups + Given I set the field "Include only active enrolments" to "0" + And I set the field "Auto create based on" to "Members per group" + When I set the field "Group/member count" to "11" + And I press "Preview" + Then I should see "Suspended student 11" \ No newline at end of file From 3ee812fab439691bfb1b4d3979af9ef52b1cc207 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Thu, 11 Feb 2016 11:18:47 +0800 Subject: [PATCH 3/3] MDL-53072 group: minor changes to existing code 1) Tidied up conditional check and put additional variable on new line. 2) Prevented change of the default behaviour for 'groups_get_potential_members()'. 3) Added another behat test. --- group/autogroup.php | 9 ++++----- group/lib.php | 2 +- group/tests/behat/auto_creation.feature | 19 ++++++++++++++++++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/group/autogroup.php b/group/autogroup.php index e9921352f76..0c1bb5d65cd 100644 --- a/group/autogroup.php +++ b/group/autogroup.php @@ -96,12 +96,11 @@ if ($editform->is_cancelled()) { $source['groupid'] = $data->groupid; } - $onlyactive = true; - if (!$data->includeonlyactiveenrol && has_capability('moodle/course:viewsuspendedusers', $context)) { - $onlyactive = false; - } + // Display only active users if the option was selected or they do not have the capability to view suspended users. + $onlyactive = !empty($data->includeonlyactiveenrol) || !has_capability('moodle/course:viewsuspendedusers', $context); - $users = groups_get_potential_members($data->courseid, $data->roleid, $source, $orderby, !empty($data->notingroup), $onlyactive); + $users = groups_get_potential_members($data->courseid, $data->roleid, $source, $orderby, !empty($data->notingroup), + $onlyactive); $usercnt = count($users); if ($data->allocateby == 'random') { diff --git a/group/lib.php b/group/lib.php index 3af8deb5b3a..e0c0e4b4574 100644 --- a/group/lib.php +++ b/group/lib.php @@ -722,7 +722,7 @@ function groups_get_possible_roles($context) { */ function groups_get_potential_members($courseid, $roleid = null, $source = null, $orderby = 'lastname ASC, firstname ASC', - $notingroup = null, $onlyactiveenrolments = true) { + $notingroup = null, $onlyactiveenrolments = false) { global $DB; $context = context_course::instance($courseid); diff --git a/group/tests/behat/auto_creation.feature b/group/tests/behat/auto_creation.feature index d6754565558..bc928eb433d 100644 --- a/group/tests/behat/auto_creation.feature +++ b/group/tests/behat/auto_creation.feature @@ -154,4 +154,21 @@ Feature: Automatic creation of groups And I set the field "Auto create based on" to "Members per group" When I set the field "Group/member count" to "11" And I press "Preview" - Then I should see "Suspended student 11" \ No newline at end of file + Then I should see "Suspended student 11" + + Scenario: Do not display 'Include only active enrolments' if user does not have the 'moodle/course:viewsuspendedusers' capability + Given I log out + And I log in as "admin" + And I set the following system permissions of "Teacher" role: + | capability | permission | + | moodle/course:viewsuspendedusers | Prevent | + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I expand "Users" node + And I follow "Groups" + When I press "Auto-create groups" + Then I should not see "Include only active enrolments" + And I set the field "Group/member count" to "11" + And I press "Preview" + And I should not see "Suspended Student 11"