diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php index 2c94486690e..a09c1dc0741 100644 --- a/backup/moodle2/backup_stepslib.php +++ b/backup/moodle2/backup_stepslib.php @@ -1341,7 +1341,7 @@ class backup_groups_structure_step extends backup_structure_step { $group = new backup_nested_element('group', array('id'), array( 'name', 'idnumber', 'description', 'descriptionformat', 'enrolmentkey', - 'picture', 'timecreated', 'timemodified')); + 'picture', 'visibility', 'participation', 'timecreated', 'timemodified')); $members = new backup_nested_element('group_members'); diff --git a/group/autogroup.php b/group/autogroup.php index 5b85e56e54a..1c55d4a4d0e 100644 --- a/group/autogroup.php +++ b/group/autogroup.php @@ -234,6 +234,7 @@ if ($editform->is_cancelled()) { $newgroup->courseid = $data->courseid; $newgroup->name = $group['name']; $newgroup->enablemessaging = $data->enablemessaging ?? 0; + $newgroup->visibility = GROUPS_VISIBILITY_ALL; $groupid = groups_create_group($newgroup); $createdgroups[] = $groupid; foreach($group['members'] as $user) { diff --git a/group/classes/visibility.php b/group/classes/visibility.php new file mode 100644 index 00000000000..bd032bf80a0 --- /dev/null +++ b/group/classes/visibility.php @@ -0,0 +1,172 @@ +. + +/** + * Group visibility methods + * + * @package core_group + * @copyright 2022 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core_group; + +/** + * Group visibility methods. + */ +class visibility { + + /** + * Store the number groups with visibility other than ALL on the course. + * + * @param int $courseid Course ID to update the cache for. + * @param \cache|null $cache Existing cache instance. If null, once will be created. + * @return void + * @throws \dml_exception + */ + public static function update_hiddengroups_cache(int $courseid, ?\cache $cache = null): void { + global $DB; + if (!$cache) { + $cache = \cache::make('core', 'coursehiddengroups'); + } + $hiddengroups = $DB->count_records_select('groups', 'courseid = ? AND visibility != ?', + [$courseid, GROUPS_VISIBILITY_ALL]); + $cache->set($courseid, $hiddengroups); + } + + /** + * Return whether a course currently had hidden groups. + * + * This can be used as a shortcut to decide whether visibility restrictions need to be applied. If this returns false, + * we may be able to use cached data, or do a much simpler query. + * + * @param int $courseid + * @return bool + * @throws \coding_exception + * @throws \dml_exception + */ + public static function course_has_hidden_groups(int $courseid): bool { + $cache = \cache::make('core', 'coursehiddengroups'); + $hiddengroups = $cache->get($courseid); + if ($hiddengroups === false) { + self::update_hiddengroups_cache($courseid, $cache); + $cache->get($courseid); + } + return $hiddengroups > 0; + } + + /** + * Can the current user view all the groups on the course? + * + * Returns true if there are no groups on the course with visibility != ALL, + * or if the user has viewhiddengroups. + * + * This is useful for deciding whether we need to perform additional visibility checkes + * such as the sql_* methods of this class. + * + * @param int $courseid + * @return bool + */ + public static function can_view_all_groups(int $courseid): bool { + $viewhidden = has_capability('moodle/course:viewhiddengroups', \context_course::instance($courseid)); + $hashidden = self::course_has_hidden_groups($courseid); + return $viewhidden || !$hashidden; + } + + /** + * Return SQL conditions for determining whether a user can see a group and its memberships. + * + * @param int $userid + * @param string $groupsalias The SQL alias being used for the groups table. + * @param string $groupsmembersalias The SQL alias being used for the groups_members table. + * @return array [$where, $params] + */ + public static function sql_group_visibility_where(int $userid, + string $groupsalias = 'g', string $groupsmembersalias = 'gm'): array { + global $USER; + // Apply visibility restrictions. + // Everyone can see who is in groups with ALL visibility. + $where = "({$groupsalias}.visibility = :all"; + $params['all'] = GROUPS_VISIBILITY_ALL; + if ($userid == $USER->id) { + // If the user is looking at their own groups, they can see those with MEMBERS or OWN visibility. + $where .= " OR {$groupsalias}.visibility IN (:members, :own)"; + $params['members'] = GROUPS_VISIBILITY_MEMBERS; + $params['own'] = GROUPS_VISIBILITY_OWN; + } else { + list($memberssql, $membersparams) = self::sql_members_visibility_condition($groupsalias, $groupsmembersalias); + // If someone else's groups, they can see those with MEMBERS visibilty, only if they are a member too. + $where .= " OR ($memberssql)"; + $params = array_merge($params, $membersparams); + } + $where .= ")"; + return [$where, $params]; + } + + /** + * Return SQL conditions for determining whether a user can see a group's members. + * + * @param string $groupsalias The SQL alias being used for the groups table. + * @param string $groupsmembersalias The SQL alias being used for the groups_members table. + * @param string $useralias The SQL alias being used for the user table. + * @return array [$where, $params] + */ + public static function sql_member_visibility_where(string $groupsalias = 'g', + string $groupsmembersalias = 'gm', string $useralias = 'u'): array { + global $USER; + + list($memberssql, $membersparams) = self::sql_members_visibility_condition($groupsalias, $groupsmembersalias); + + $where = " AND ( + {$groupsalias}.visibility = :all + OR ($memberssql) + OR ({$groupsalias}.visibility = :own AND {$useralias}.id = :currentuser2) + )"; + $params = [ + 'all' => GROUPS_VISIBILITY_ALL, + 'own' => GROUPS_VISIBILITY_OWN, + 'currentuser2' => $USER->id, + ]; + $params = array_merge($params, $membersparams); + return [$where, $params]; + } + + /** + * Return a condition to check if a user can view a group because it has MEMBERS visibility and they are a member. + * + * @param string $groupsalias The SQL alias being used for the groups table. + * @param string $groupsmembersalias The SQL alias being used for the groups_members table. + * @return array [$sql, $params] + */ + protected static function sql_members_visibility_condition(string $groupsalias = 'g', + string $groupsmembersalias = 'gm'): array { + global $USER; + $sql = "{$groupsalias}.visibility = :members + AND ( + SELECT gm2.id + FROM {groups_members} gm2 + WHERE gm2.groupid = {$groupsmembersalias}.groupid + AND gm2.userid = :currentuser + ) IS NOT NULL"; + $params = [ + 'members' => GROUPS_VISIBILITY_MEMBERS, + 'currentuser' => $USER->id + ]; + + return [$sql, $params]; + } +} diff --git a/group/externallib.php b/group/externallib.php index 1e25db334a5..9d258195392 100644 --- a/group/externallib.php +++ b/group/externallib.php @@ -22,6 +22,7 @@ use core_external\external_single_structure; use core_external\external_value; use core_external\external_warnings; use core_external\util; +use core_group\visibility; /** * Group external functions @@ -34,6 +35,26 @@ use core_external\util; */ class core_group_external extends external_api { + + /** + * Validate visibility. + * + * @param int $visibility Visibility string, must one of the visibility class constants. + * @throws invalid_parameter_exception if visibility is not an allowed value. + */ + protected static function validate_visibility(int $visibility): void { + $allowed = [ + GROUPS_VISIBILITY_ALL, + GROUPS_VISIBILITY_MEMBERS, + GROUPS_VISIBILITY_OWN, + GROUPS_VISIBILITY_NONE, + ]; + if (!array_key_exists($visibility, $allowed)) { + throw new invalid_parameter_exception('Invalid group visibility provided. Must be one of ' + . join(',', $allowed)); + } + } + /** * Returns description of method parameters * @@ -51,7 +72,14 @@ class core_group_external extends external_api { 'description' => new external_value(PARAM_RAW, 'group description text'), 'descriptionformat' => new external_format_value('description', VALUE_DEFAULT), 'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase', VALUE_OPTIONAL), - 'idnumber' => new external_value(PARAM_RAW, 'id number', VALUE_OPTIONAL) + 'idnumber' => new external_value(PARAM_RAW, 'id number', VALUE_OPTIONAL), + 'visibility' => new external_value(PARAM_INT, + 'group visibility mode. 0 = Visible to all. 1 = Visible to members. ' + . '2 = See own membership. 3 = Membership is hidden. default: 0', + VALUE_DEFAULT, 0), + 'participation' => new external_value(PARAM_BOOL, + 'activity participation enabled? Only for "all" and "members" visibility. Default true.', + VALUE_DEFAULT, true), ) ), 'List of group object. A group has a courseid, a name, a description and an enrolment key.' ) @@ -101,6 +129,9 @@ class core_group_external extends external_api { // Validate format. $group->descriptionformat = util::validate_format($group->descriptionformat); + // Validate visibility. + self::validate_visibility($group->visibility); + // finally create the group $group->id = groups_create_group($group, false); if (!isset($group->enrolmentkey)) { @@ -134,7 +165,11 @@ class core_group_external extends external_api { 'description' => new external_value(PARAM_RAW, 'group description text'), 'descriptionformat' => new external_format_value('description'), 'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase'), - 'idnumber' => new external_value(PARAM_RAW, 'id number') + 'idnumber' => new external_value(PARAM_RAW, 'id number'), + 'visibility' => new external_value(PARAM_INT, + 'group visibility mode. 0 = Visible to all. 1 = Visible to members. 2 = See own membership. ' + . '3 = Membership is hidden.'), + 'participation' => new external_value(PARAM_BOOL, 'participation mode'), ) ), 'List of group object. A group has an id, a courseid, a name, a description and an enrolment key.' ); @@ -168,7 +203,8 @@ class core_group_external extends external_api { $groups = array(); foreach ($params['groupids'] as $groupid) { // validate params - $group = groups_get_group($groupid, 'id, courseid, name, idnumber, description, descriptionformat, enrolmentkey', MUST_EXIST); + $group = groups_get_group($groupid, 'id, courseid, name, idnumber, description, descriptionformat, enrolmentkey, ' + . 'visibility, participation', MUST_EXIST); // now security checks $context = context_course::instance($group->courseid, IGNORE_MISSING); @@ -208,7 +244,11 @@ class core_group_external extends external_api { 'description' => new external_value(PARAM_RAW, 'group description text'), 'descriptionformat' => new external_format_value('description'), 'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase'), - 'idnumber' => new external_value(PARAM_RAW, 'id number') + 'idnumber' => new external_value(PARAM_RAW, 'id number'), + 'visibility' => new external_value(PARAM_INT, + 'group visibility mode. 0 = Visible to all. 1 = Visible to members. 2 = See own membership. ' + . '3 = Membership is hidden.'), + 'participation' => new external_value(PARAM_BOOL, 'participation mode'), ) ) ); @@ -251,7 +291,8 @@ class core_group_external extends external_api { require_capability('moodle/course:managegroups', $context); $gs = groups_get_all_groups($params['courseid'], 0, 0, - 'g.id, g.courseid, g.name, g.idnumber, g.description, g.descriptionformat, g.enrolmentkey'); + 'g.id, g.courseid, g.name, g.idnumber, g.description, g.descriptionformat, g.enrolmentkey, ' + . 'g.visibility, g.participation'); $groups = array(); foreach ($gs as $group) { @@ -280,7 +321,11 @@ class core_group_external extends external_api { 'description' => new external_value(PARAM_RAW, 'group description text'), 'descriptionformat' => new external_format_value('description'), 'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase'), - 'idnumber' => new external_value(PARAM_RAW, 'id number') + 'idnumber' => new external_value(PARAM_RAW, 'id number'), + 'visibility' => new external_value(PARAM_INT, + 'group visibility mode. 0 = Visible to all. 1 = Visible to members. 2 = See own membership. ' + . '3 = Membership is hidden.'), + 'participation' => new external_value(PARAM_BOOL, 'participation mode'), ) ) ); @@ -1499,7 +1544,12 @@ class core_group_external extends external_api { 'description' => new external_value(PARAM_RAW, 'group description text', VALUE_OPTIONAL), 'descriptionformat' => new external_format_value('description', VALUE_DEFAULT), 'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase', VALUE_OPTIONAL), - 'idnumber' => new external_value(PARAM_RAW, 'id number', VALUE_OPTIONAL) + 'idnumber' => new external_value(PARAM_RAW, 'id number', VALUE_OPTIONAL), + 'visibility' => new external_value(PARAM_TEXT, + 'group visibility mode. 0 = Visible to all. 1 = Visible to members. ' + . '2 = See own membership. 3 = Membership is hidden.', VALUE_OPTIONAL), + 'participation' => new external_value(PARAM_BOOL, + 'activity participation enabled? Only for "all" and "members" visibility', VALUE_OPTIONAL), ) ), 'List of group objects. A group is found by the id, then all other details provided will be updated.' ) @@ -1523,13 +1573,13 @@ class core_group_external extends external_api { $transaction = $DB->start_delegated_transaction(); foreach ($params['groups'] as $group) { - $group = (object)$group; + $group = (object) $group; if (trim($group->name) == '') { throw new invalid_parameter_exception('Invalid group name'); } - if (! $currentgroup = $DB->get_record('groups', array('id' => $group->id))) { + if (!$currentgroup = $DB->get_record('groups', array('id' => $group->id))) { throw new invalid_parameter_exception("Group $group->id does not exist"); } @@ -1539,6 +1589,24 @@ class core_group_external extends external_api { throw new invalid_parameter_exception('A different group with the same name already exists in the course'); } + if (isset($group->visibility) || isset($group->participation)) { + $hasmembers = $DB->record_exists('groups_members', ['groupid' => $group->id]); + if (isset($group->visibility)) { + // Validate visibility. + self::validate_visibility($group->visibility); + if ($hasmembers && $group->visibility != $currentgroup->visibility) { + throw new invalid_parameter_exception( + 'The visibility of this group cannot be changed as it currently has members.'); + } + } else { + $group->visibility = $currentgroup->visibility; + } + if (isset($group->participation) && $hasmembers && $group->participation != $currentgroup->participation) { + throw new invalid_parameter_exception( + 'The participation mode of this group cannot be changed as it currently has members.'); + } + } + $group->courseid = $currentgroup->courseid; // Now security checks. diff --git a/group/group_form.php b/group/group_form.php index 15b638870de..563cec6f627 100644 --- a/group/group_form.php +++ b/group/group_form.php @@ -25,6 +25,8 @@ defined('MOODLE_INTERNAL') || die; +use core_group\visibility; + require_once($CFG->dirroot.'/lib/formslib.php'); /** @@ -66,10 +68,27 @@ class group_form extends moodleform { $mform->addHelpButton('enrolmentkey', 'enrolmentkey', 'group'); $mform->setType('enrolmentkey', PARAM_RAW); + $visibilityoptions = [ + GROUPS_VISIBILITY_ALL => get_string('visibilityall', 'group'), + GROUPS_VISIBILITY_MEMBERS => get_string('visibilitymembers', 'group'), + GROUPS_VISIBILITY_OWN => get_string('visibilityown', 'group'), + GROUPS_VISIBILITY_NONE => get_string('visibilitynone', 'group') + ]; + $mform->addElement('select', 'visibility', get_string('visibility', 'group'), $visibilityoptions); + $mform->addHelpButton('visibility', 'visibility', 'group'); + $mform->setType('visibility', PARAM_INT); + + $mform->addElement('advcheckbox', 'participation', '', get_string('participation', 'group')); + $mform->addHelpButton('participation', 'participation', 'group'); + $mform->setType('participation', PARAM_BOOL); + $mform->setDefault('participation', 1); + $mform->disabledIf('participation', 'visibility', 'in', [GROUPS_VISIBILITY_OWN, GROUPS_VISIBILITY_NONE]); + // Group conversation messaging. if (\core_message\api::can_create_group_conversation($USER->id, $coursecontext)) { $mform->addElement('selectyesno', 'enablemessaging', get_string('enablemessaging', 'group')); $mform->addHelpButton('enablemessaging', 'enablemessaging', 'group'); + $mform->disabledIf('enablemessaging', 'visibility', 'in', [GROUPS_VISIBILITY_OWN, GROUPS_VISIBILITY_NONE]); } $mform->addElement('static', 'currentpicture', get_string('currentpicture')); @@ -124,6 +143,16 @@ class group_form extends moodleform { } } + if ($DB->record_exists('groups_members', ['groupid' => $groupid])) { + // If the group has members, lock visibility and participation fields. + /** @var MoodleQuickForm_select $visibility */ + $visibility = $mform->getElement('visibility'); + $visibility->freeze(); + /** @var MoodleQuickForm_advcheckbox $participation */ + $participation = $mform->getElement('participation'); + $participation->freeze(); + } + } /** diff --git a/group/lib.php b/group/lib.php index c898e3ea5b8..197a2f5a84f 100644 --- a/group/lib.php +++ b/group/lib.php @@ -260,6 +260,13 @@ function groups_create_group($data, $editform = false, $editoroptions = false) { } } + $data->visibility ??= GROUPS_VISIBILITY_ALL; + + if (!in_array($data->visibility, [GROUPS_VISIBILITY_ALL, GROUPS_VISIBILITY_MEMBERS])) { + $data->participation = false; + $data->enablemessaging = false; + } + if ($editform and $editoroptions) { $data->description = $data->description_editor['text']; $data->descriptionformat = $data->description_editor['format']; @@ -285,6 +292,8 @@ function groups_create_group($data, $editform = false, $editoroptions = false) { // Invalidate the grouping cache for the course cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($course->id)); + // Rebuild the coursehiddengroups cache for the course. + \core_group\visibility::update_hiddengroups_cache($course->id); // Group conversation messaging. if (\core_message\api::can_create_group_conversation($USER->id, $context)) { @@ -423,6 +432,10 @@ function groups_update_group($data, $editform = false, $editoroptions = false) { throw new moodle_exception('idnumbertaken'); } } + if (isset($data->visibility) && !in_array($data->visibility, [GROUPS_VISIBILITY_ALL, GROUPS_VISIBILITY_MEMBERS])) { + $data->participation = false; + $data->enablemessaging = false; + } if ($editform and $editoroptions) { $data = file_postupdate_standard_editor($data, 'description', $editoroptions, $context, 'group', 'description', $data->id); @@ -432,6 +445,8 @@ function groups_update_group($data, $editform = false, $editoroptions = false) { // Invalidate the group data. cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($data->courseid)); + // Rebuild the coursehiddengroups cache for the course. + \core_group\visibility::update_hiddengroups_cache($data->courseid); $group = $DB->get_record('groups', array('id'=>$data->id)); @@ -575,6 +590,8 @@ function groups_delete_group($grouporid) { cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($group->courseid)); // Purge the group and grouping cache for users. cache_helper::purge_by_definition('core', 'user_group_groupings'); + // Rebuild the coursehiddengroups cache for the course. + \core_group\visibility::update_hiddengroups_cache($group->courseid); // Trigger group event. $params = array( @@ -723,6 +740,8 @@ function groups_delete_groups($courseid, $showfeedback=false) { cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($courseid)); // Purge the group and grouping cache for users. cache_helper::purge_by_definition('core', 'user_group_groupings'); + // Rebuild the coursehiddengroups cache for the course. + \core_group\visibility::update_hiddengroups_cache($courseid); if ($showfeedback) { echo $OUTPUT->notification(get_string('deleted').' - '.get_string('groups', 'group'), 'notifysuccess'); diff --git a/group/tests/behat/backup_restore_groups.feature b/group/tests/behat/backup_restore_groups.feature new file mode 100644 index 00000000000..fd5e776ce6e --- /dev/null +++ b/group/tests/behat/backup_restore_groups.feature @@ -0,0 +1,48 @@ +@core @core_group +Feature: Backup and restore a course containing groups + In order to transfer groups to another course + As a teacher + I want to backup and restore a course retaining the groups + + Background: + Given the following "courses" exist: + | fullname | shortname | format | enablecompletion | numsections | + | Course 1 | C1 | topics | 1 | 3 | + And the following "users" exist: + | username | firstname | lastname | + | teacher1 | Teacher | Teacher | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following "groups" exist: + | name | course | idnumber | visibility | participation | + | Visible to all/Participation | C1 | VP | 0 | 1 | + | Visible to members/Participation | C1 | MP | 1 | 1 | + | See own membership | C1 | O | 2 | 0 | + | Not visible | C1 | N | 3 | 0 | + | Visible to all/Non-Participation | C1 | VN | 0 | 0 | + | Visible to members/Non-Participation | C1 | MN | 1 | 0 | + And I log in as "admin" + And I backup "Course 1" course using this options: + | Confirmation | Filename | test_backup.mbz | + And I restore "test_backup.mbz" backup into a new course using this options: + | Schema | Course name | Restored course | + + @javascript + Scenario Outline: Check restored groups + Given I am on the "Restored course copy 1" "groups" page logged in as teacher1 + When I set the field "Groups" to "" + And I press "Edit group settings" + Then the following fields match these values: + | Group ID number | | + | Group visibility | | + | Allow activity participation | | + + Examples: + | group | idnumber | visibility | participation | + | Visible to all/Participation | VP | 0 | 1 | + | Visible to members/Participation | MP | 1 | 1 | + | See own membership | O | 2 | 0 | + | Not visible | N | 3 | 0 | + | Visible to all/Non-Participation | VN | 0 | 0 | + | Visible to members/Non-Participation | MN | 1 | 0 | diff --git a/group/tests/behat/private_groups.feature b/group/tests/behat/private_groups.feature new file mode 100644 index 00000000000..ee2fb28605c --- /dev/null +++ b/group/tests/behat/private_groups.feature @@ -0,0 +1,129 @@ +@core @core_group +Feature: Private groups + As a teacher + In order to organise students into groups while protecting their privacy + I want to define groups that are not visible to all students + + Background: + Given the following "courses" exist: + | fullname | shortname | format | enablecompletion | numsections | + | Course 1 | C1 | topics | 1 | 3 | + And the following "users" exist: + | username | firstname | lastname | + | teacher1 | Teacher | Teacher | + | student1 | Student | 1 | + | student2 | Student | 2 | + | student3 | Student | 3 | + | student4 | Student | 4 | + | student5 | Student | 5 | + | student6 | Student | 6 | + | student7 | Student | 7 | + | student8 | Student | 8 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + | student3 | C1 | student | + | student4 | C1 | student | + | student5 | C1 | student | + | student6 | C1 | student | + | student7 | C1 | student | + | student8 | C1 | student | + And the following "groups" exist: + | name | course | idnumber | visibility | participation | + | Visible to all/Participation | C1 | VP | 0 | 1 | + | Visible to members/Participation | C1 | MP | 1 | 1 | + | See own membership | C1 | O | 2 | 0 | + | Not visible | C1 | N | 3 | 0 | + | Visible to all/Non-Participation | C1 | VN | 0 | 0 | + | Visible to members/Non-Participation | C1 | MN | 1 | 0 | + And the following "group members" exist: + | user | group | + | student1 | VP | + | student1 | VN | + | student2 | MP | + | student2 | MN | + | student3 | O | + | student4 | N | + | student5 | VP | + | student5 | VN | + | student6 | MP | + | student6 | MN | + | student7 | O | + | student8 | N | + + Scenario: Participants in "Visible to all" groups see their membership and other members: + Given I log in as "student1" + And I am on "Course 1" course homepage + When I follow "Participants" + Then the following should exist in the "participants" table: + | First name / Surname | Groups | + | Student 1 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 2 | No groups | + | Student 3 | No groups | + | Student 4 | No groups | + | Student 5 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 6 | No groups | + | Student 7 | No groups | + | Student 8 | No groups | + + Scenario: Participants in "Visible to members" groups see their membership and other members, plus "Visible to all" + Given I log in as "student2" + And I am on "Course 1" course homepage + When I follow "Participants" + Then the following should exist in the "participants" table: + | First name / Surname | Groups | + | Student 1 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 2 | Visible to members/Non-Participation, Visible to members/Participation | + | Student 3 | No groups | + | Student 4 | No groups | + | Student 5 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 6 | Visible to members/Non-Participation, Visible to members/Participation | + | Student 7 | No groups | + | Student 8 | No groups | + + Scenario: Participants in "See own membership" groups see their membership but not other members, plus "Visible to all" + Given I log in as "student3" + And I am on "Course 1" course homepage + When I follow "Participants" + Then the following should exist in the "participants" table: + | First name / Surname | Groups | + | Student 1 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 2 | No groups | + | Student 3 | See own membership | + | Student 4 | No groups | + | Student 5 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 6 | No groups | + | Student 7 | No groups | + | Student 8 | No groups | + + Scenario: Participants in "Not visible" groups do not see that group, do see "Visible to all" + Given I log in as "student4" + And I am on "Course 1" course homepage + When I follow "Participants" + Then the following should exist in the "participants" table: + | First name / Surname | Groups | + | Student 1 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 2 | No groups | + | Student 3 | No groups | + | Student 4 | No groups | + | Student 5 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 6 | No groups | + | Student 7 | No groups | + | Student 8 | No groups | + + Scenario: View participants list as a teacher: + Given I log in as "teacher1" + And I am on "Course 1" course homepage + When I follow "Participants" + Then the following should exist in the "participants" table: + | First name / Surname | Groups | + | Student 1 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 2 | Visible to members/Non-Participation, Visible to members/Participation | + | Student 3 | See own membership | + | Student 4 | Not visible | + | Student 5 | Visible to all/Non-Participation, Visible to all/Participation | + | Student 6 | Visible to members/Non-Participation, Visible to members/Participation | + | Student 7 | See own membership | + | Student 8 | Not visible | diff --git a/group/tests/behat/update_groups.feature b/group/tests/behat/update_groups.feature index b3b6bfb4d1a..dc8e0aaf948 100644 --- a/group/tests/behat/update_groups.feature +++ b/group/tests/behat/update_groups.feature @@ -142,3 +142,20 @@ Feature: Automatic updating of groups and groupings | Enrolment key | Abcdef-1 | And I press "Save changes" And I should not see "This enrolment key is already used for another group." + + @javascript + Scenario: Visibility and Participation settings are locked once a group has members + Given I set the field "groups" to "Group (with ID)" + And I press "Edit group settings" + And "visibility" "select" should exist + And the field "Group visibility" matches value "Visible to all" + And the "participation" "checkbox" should be enabled + And the field "Allow activity participation" matches value "1" + When the following "group members" exist: + | user | group | + | teacher1 | An ID | + And I reload the page + Then "visibility" "select" should not exist + And "Visible to all" "text" should exist + And the "participation" "checkbox" should be disabled + And the field "Allow activity participation" matches value "1" diff --git a/group/tests/externallib_test.php b/group/tests/externallib_test.php index 719db863050..b19ebc0d752 100644 --- a/group/tests/externallib_test.php +++ b/group/tests/externallib_test.php @@ -60,6 +60,8 @@ class externallib_test extends externallib_advanced_testcase { $group2['courseid'] = $course->id; $group2['name'] = 'Group Test 2'; $group2['description'] = 'Group Test 2 description'; + $group2['visibility'] = GROUPS_VISIBILITY_MEMBERS; + $group2['participation'] = false; $group3 = array(); $group3['courseid'] = $course->id; $group3['name'] = 'Group Test 3'; @@ -92,10 +94,15 @@ class externallib_test extends externallib_advanced_testcase { $this->assertEquals($dbgroup->descriptionformat, $group1['descriptionformat']); $this->assertEquals($dbgroup->enrolmentkey, $group1['enrolmentkey']); $this->assertEquals($dbgroup->idnumber, $group1['idnumber']); + // The visibility and participation attributes were not specified, so should match the default values. + $groupvisibility = GROUPS_VISIBILITY_ALL; + $groupparticipation = true; break; case $group2['name']: $groupdescription = $group2['description']; $groupcourseid = $group2['courseid']; + $groupvisibility = $group2['visibility']; + $groupparticipation = $group2['participation']; break; default: throw new \moodle_exception('unknowgroupname'); @@ -103,6 +110,8 @@ class externallib_test extends externallib_advanced_testcase { } $this->assertEquals($dbgroup->description, $groupdescription); $this->assertEquals($dbgroup->courseid, $groupcourseid); + $this->assertEquals($dbgroup->visibility, $groupvisibility); + $this->assertEquals($dbgroup->participation, $groupparticipation); } try { @@ -120,6 +129,33 @@ class externallib_test extends externallib_advanced_testcase { $froups = core_group_external::create_groups(array($group4)); } + /** + * Test that creating a group with an invalid visibility value throws an exception. + * + * @covers \core_group_external::create_groups + * @return void + */ + public function test_create_group_invalid_visibility(): void { + $this->resetAfterTest(true); + + $course = self::getDataGenerator()->create_course(); + + $group1 = array(); + $group1['courseid'] = $course->id; + $group1['name'] = 'Group Test 1'; + $group1['description'] = 'Group Test 1 description'; + $group1['visibility'] = 1000; + + // Set the required capabilities by the external function. + $context = \context_course::instance($course->id); + $roleid = $this->assignUserCapability('moodle/course:managegroups', $context->id); + $this->assignUserCapability('moodle/course:view', $context->id, $roleid); + + // Call the external function. + $this->expectException('invalid_parameter_exception'); + core_group_external::create_groups([$group1]); + } + /** * Test update_groups */ @@ -161,6 +197,7 @@ class externallib_test extends externallib_advanced_testcase { $group1data['idnumber'] = 'CHANGED'; core_group_external::update_groups(array($group1data)); $group2data['description'] = 'Group Test 2 description CHANGED'; + $group2data['visibility'] = GROUPS_VISIBILITY_MEMBERS; core_group_external::update_groups(array($group2data)); foreach ([$group1, $group2] as $group) { @@ -169,16 +206,20 @@ class externallib_test extends externallib_advanced_testcase { case $group1data['name']: $this->assertEquals($dbgroup->idnumber, $group1data['idnumber']); $groupdescription = $group1data['description']; + // Visibility was not specified, so should match the default value. + $groupvisibility = GROUPS_VISIBILITY_ALL; break; case $group2data['name']: $this->assertEquals($dbgroup->idnumber, $group2data['idnumber']); $groupdescription = $group2data['description']; + $groupvisibility = $group2data['visibility']; break; default: throw new \moodle_exception('unknowngroupname'); break; } $this->assertEquals($dbgroup->description, $groupdescription); + $this->assertEquals($dbgroup->visibility, $groupvisibility); } // Taken idnumber exception. @@ -199,6 +240,103 @@ class externallib_test extends externallib_advanced_testcase { $groups = core_group_external::update_groups(array($group1data)); } + /** + * Test an exception is thrown when an invalid visibility value is passed in an update. + * + * @covers \core_group_external::update_groups + * @return void + */ + public function test_update_groups_invalid_visibility(): void { + $this->resetAfterTest(true); + + $course = self::getDataGenerator()->create_course(); + + $group1data = array(); + $group1data['courseid'] = $course->id; + $group1data['name'] = 'Group Test 1'; + + // Set the required capabilities by the external function. + $context = \context_course::instance($course->id); + $roleid = $this->assignUserCapability('moodle/course:managegroups', $context->id); + $this->assignUserCapability('moodle/course:view', $context->id, $roleid); + + // Create the test group. + $group1 = self::getDataGenerator()->create_group($group1data); + + $group1data['id'] = $group1->id; + unset($group1data['courseid']); + $group1data['visibility'] = 1000; + + $this->expectException('invalid_parameter_exception'); + core_group_external::update_groups(array($group1data)); + } + + /** + * Attempting to change the visibility of a group with members should throw an exception. + * + * @covers \core_group_external::update_groups + * @return void + */ + public function test_update_groups_visibility_with_members(): void { + $this->resetAfterTest(true); + + $course = self::getDataGenerator()->create_course(); + + $group1data = array(); + $group1data['courseid'] = $course->id; + $group1data['name'] = 'Group Test 1'; + + // Set the required capabilities by the external function. + $context = \context_course::instance($course->id); + $roleid = $this->assignUserCapability('moodle/course:managegroups', $context->id); + $this->assignUserCapability('moodle/course:view', $context->id, $roleid); + + // Create the test group and add a member. + $group1 = self::getDataGenerator()->create_group($group1data); + $user1 = self::getDataGenerator()->create_and_enrol($course); + self::getDataGenerator()->create_group_member(['userid' => $user1->id, 'groupid' => $group1->id]); + + $group1data['id'] = $group1->id; + unset($group1data['courseid']); + $group1data['visibility'] = GROUPS_VISIBILITY_MEMBERS; + + $this->expectExceptionMessage('The visibility of this group cannot be changed as it currently has members.'); + core_group_external::update_groups(array($group1data)); + } + + /** + * Attempting to change the participation field of a group with members should throw an exception. + * + * @covers \core_group_external::update_groups + * @return void + */ + public function test_update_groups_participation_with_members(): void { + $this->resetAfterTest(true); + + $course = self::getDataGenerator()->create_course(); + + $group1data = array(); + $group1data['courseid'] = $course->id; + $group1data['name'] = 'Group Test 1'; + + // Set the required capabilities by the external function. + $context = \context_course::instance($course->id); + $roleid = $this->assignUserCapability('moodle/course:managegroups', $context->id); + $this->assignUserCapability('moodle/course:view', $context->id, $roleid); + + // Create the test group and add a member. + $group1 = self::getDataGenerator()->create_group($group1data); + $user1 = self::getDataGenerator()->create_and_enrol($course); + self::getDataGenerator()->create_group_member(['userid' => $user1->id, 'groupid' => $group1->id]); + + $group1data['id'] = $group1->id; + unset($group1data['courseid']); + $group1data['participation'] = false; + + $this->expectExceptionMessage('The participation mode of this group cannot be changed as it currently has members.'); + core_group_external::update_groups(array($group1data)); + } + /** * Test get_groups */ @@ -219,6 +357,8 @@ class externallib_test extends externallib_advanced_testcase { $group2data['courseid'] = $course->id; $group2data['name'] = 'Group Test 2'; $group2data['description'] = 'Group Test 2 description'; + $group2data['visibility'] = GROUPS_VISIBILITY_MEMBERS; + $group2data['participation'] = false; $group1 = self::getDataGenerator()->create_group($group1data); $group2 = self::getDataGenerator()->create_group($group2data); @@ -241,6 +381,9 @@ class externallib_test extends externallib_advanced_testcase { case $group1->name: $groupdescription = $group1->description; $groupcourseid = $group1->courseid; + // The visibility and participation attributes were not specified, so should match the default values. + $groupvisibility = GROUPS_VISIBILITY_ALL; + $groupparticipation = true; $this->assertEquals($dbgroup->descriptionformat, $group1->descriptionformat); $this->assertEquals($dbgroup->enrolmentkey, $group1->enrolmentkey); $this->assertEquals($dbgroup->idnumber, $group1->idnumber); @@ -248,6 +391,8 @@ class externallib_test extends externallib_advanced_testcase { case $group2->name: $groupdescription = $group2->description; $groupcourseid = $group2->courseid; + $groupvisibility = $group2->visibility; + $groupparticipation = $group2->participation; break; default: throw new \moodle_exception('unknowgroupname'); @@ -255,6 +400,8 @@ class externallib_test extends externallib_advanced_testcase { } $this->assertEquals($dbgroup->description, $groupdescription); $this->assertEquals($dbgroup->courseid, $groupcourseid); + $this->assertEquals($dbgroup->visibility, $groupvisibility); + $this->assertEquals($dbgroup->participation, $groupparticipation); } // Call without required capability diff --git a/group/upgrade.txt b/group/upgrade.txt index 894ffc4efbc..9d25ad0d45e 100644 --- a/group/upgrade.txt +++ b/group/upgrade.txt @@ -1,6 +1,22 @@ This files describes API changes in /group/*, information provided here is intended especially for developers. +=== 4.2 === +* `\core_group\visibility` class added to support new `visibility` field in group records. This holds the visibility constants + and helper functions for applying visibility restrictions when querying groups or group members in the database. +* Changes to the group form to support visibility features: + - New `visibility` field. + - New `participation` field. + - `participation` and `enablemessaging` fields are disabled (default: false) when `visibility` is set + to `visibility::OWN` or `visibility::NONE`. +* The following externallib functions now accept `visibility` and `participation` as optional parameters: + - create_groups() + - update_groups() +* The following externallib functions now also return `visibility` and `participation` fields in their responses: + - create_groups() + - get_groups() + - get_course_groups() + === 3.11 === * The groups do not support 'hidepicture' any more, and so the column 'hidepicture' diff --git a/lang/en/cache.php b/lang/en/cache.php index 30ecda933a7..aa644ce4beb 100644 --- a/lang/en/cache.php +++ b/lang/en/cache.php @@ -51,6 +51,7 @@ $string['cachedef_coursesectionspreferences'] = 'Course section preferences'; $string['cachedef_coursecattree'] = 'Course categories tree'; $string['cachedef_coursecompletion'] = 'Course completion status'; $string['cachedef_coursecontacts'] = 'List of course contacts'; +$string['cachedef_coursehiddengroups'] = 'Number of groups on a course with restricted visibility'; $string['cachedef_coursemodinfo'] = 'Accumulated information about modules and sections for each course'; $string['cachedef_courseeditorstate'] = 'Session course state cache keys to detect course changes in the frontend'; $string['cachedef_course_image'] = 'Course images'; diff --git a/lang/en/group.php b/lang/en/group.php index f93966a7950..645b8e55f21 100644 --- a/lang/en/group.php +++ b/lang/en/group.php @@ -174,6 +174,14 @@ $string['nummembers'] = 'Members per group'; $string['mygroups'] = 'My groups'; $string['othergroups'] = 'Other groups'; $string['overview'] = 'Overview'; +$string['participation'] = 'Allow activity participation'; +$string['participation_help'] = 'If enabled, members can select this group when participating in an activity using Separate Groups +or Visible Groups mode. + +This setting is only applicable if the Group visibility is set to "Visible to all" or "Visible to members". Participation is +disabled otherwise. + +This setting cannot be edited once a group has members.'; $string['potentialmembers'] = 'Potential members: {$a}'; $string['potentialmembs'] = 'Potential members'; $string['printerfriendly'] = 'Printer-friendly display'; @@ -198,6 +206,25 @@ $string['toomanygroups'] = 'Insufficient users to populate this number of groups $string['usercount'] = 'User count'; $string['usercounttotal'] = 'User count ({$a})'; $string['usergroupmembership'] = 'Selected user\'s membership:'; +$string['visibility'] = 'Group visibility'; +$string['visibility_help'] = 'Controls the visibility of membership to this group. + +If "Visible to all" is set, all users can see when a user is a member of this group (default). + +If "Visible to members" is set, only members of this group can see when another user is a member. + +If "See own membership" is set, users can see that they are in this group, but cannot see that other users are members of the group. + +If "Membership is hidden" is set, users cannot see that they or anyone else are members of the group. + +Users with moodle/course:viewhiddengroups will always be able to see group membership. + +This setting cannot be edited once a group has members. +'; +$string['visibilityall'] = 'Visible to all'; +$string['visibilitymembers'] = 'Visible to members'; +$string['visibilityown'] = 'See own membership'; +$string['visibilitynone'] = 'Membership is hidden'; $string['memberofgroup'] = 'Group member of: {$a}'; // Deprecated since Moodle 3.11. diff --git a/lang/en/role.php b/lang/en/role.php index 4b366744316..884b247e468 100644 --- a/lang/en/role.php +++ b/lang/en/role.php @@ -204,6 +204,7 @@ $string['course:view'] = 'View courses without participation'; $string['course:viewcoursegrades'] = 'View course grades'; $string['course:viewhiddenactivities'] = 'View hidden activities'; $string['course:viewhiddencourses'] = 'View hidden courses'; +$string['course:viewhiddengroups'] = 'View hidden groups'; $string['course:viewhiddensections'] = 'View hidden sections'; $string['course:viewhiddenuserfields'] = 'View hidden user fields'; $string['course:viewparticipants'] = 'View participants'; diff --git a/lib/db/access.php b/lib/db/access.php index ef1e11b74f2..1d54febf4dc 100644 --- a/lib/db/access.php +++ b/lib/db/access.php @@ -1156,6 +1156,17 @@ $capabilities = array( ) ), + 'moodle/course:viewhiddengroups' => array( + 'riskbitmask' => RISK_PERSONAL, + 'captype' => 'READ', + 'contextlevel' => CONTEXT_COURSE, + 'archetypes' => array( + 'teacher' => CAP_ALLOW, + 'editingteacher' => CAP_ALLOW, + 'manager' => CAP_ALLOW + ) + ), + 'moodle/course:reset' => array( 'riskbitmask' => RISK_DATALOSS, diff --git a/lib/db/caches.php b/lib/db/caches.php index 7a473be7670..8d0c2be2d74 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -119,6 +119,14 @@ $definitions = array( 'staticaccelerationsize' => 2, // The original cache used 1, we've increased that to two. ), + // Whether a course currently has hidden groups. + 'coursehiddengroups' => array( + 'mode' => cache_store::MODE_APPLICATION, + 'simplekeys' => true, // The course id the groupings exist for. + 'simpledata' => true, // Booleans. + 'staticacceleration' => true, // Likely there will be a couple of calls to this. + ), + // Used to cache calendar subscriptions. 'calendar_subscriptions' => array( 'mode' => cache_store::MODE_APPLICATION, diff --git a/lib/db/install.xml b/lib/db/install.xml index 617dd9da70d..d67a2c6f555 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -2322,6 +2322,8 @@ + + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 583ef1cda6a..78f4b207c6b 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -3118,5 +3118,25 @@ privatefiles,moodle|/user/files.php'; upgrade_main_savepoint(true, 2023030300.03); } + if ($oldversion < 2023031000.01) + // Define field id to be added to groups. + $table = new xmldb_table('groups'); + $field = new xmldb_field('visibility', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'picture'); + + // Conditionally launch add field visibility. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Define field participation to be added to groups. + $field = new xmldb_field('participation', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '1', 'visibility'); + + // Conditionally launch add field participation. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + upgrade_main_savepoint(true, 2023031000.01); + } + return true; } diff --git a/lib/grouplib.php b/lib/grouplib.php index bee8583d979..94730d3eed7 100644 --- a/lib/grouplib.php +++ b/lib/grouplib.php @@ -58,6 +58,26 @@ define('GROUPS_JOIN_ANY', 1); */ define('GROUPS_JOIN_ALL', 2); +/** + * All users can see this group and its members. + */ +define('GROUPS_VISIBILITY_ALL', 0); + +/** + * Members of this group can see this group and other members. + */ +define('GROUPS_VISIBILITY_MEMBERS', 1); + +/** + * Members of this group can see the group and their own membership, but not each other's membership + */ +define('GROUPS_VISIBILITY_OWN', 2); + +/** + * No-one can see this group or its members. Members of the group will not know they are in the group. + */ +define('GROUPS_VISIBILITY_NONE', 3); + /** * Determines if a group with a given groupid exists. * @@ -213,6 +233,9 @@ function groups_get_grouping($groupingid, $fields='*', $strictness=IGNORE_MISSIN /** * Gets array of all groups in a specified course (subject to the conditions imposed by the other arguments). * + * If a user does not have moodle/course:viewhiddengroups, the list of groups and members will be restricted based on the + * visibility setting of each group. + * * @category group * @param int $courseid The id of the course. * @param int|int[] $userid optional user id or array of ids, returns only groups continaing one or more of those users. @@ -223,11 +246,12 @@ function groups_get_grouping($groupingid, $fields='*', $strictness=IGNORE_MISSIN * @param bool $withmembers if true return an extra field members (int[]) which is the list of userids that * are members of each group. For this to work, g.id (or g.*) must be included in $fields. * In this case, the final results will always be an array indexed by group id. + * @param bool $participationonly Only return groups where the participation field is true. * @return array returns an array of the group objects (unless you have done something very weird * with the $fields option). */ -function groups_get_all_groups($courseid, $userid=0, $groupingid=0, $fields='g.*', $withmembers=false) { - global $DB; +function groups_get_all_groups($courseid, $userid=0, $groupingid=0, $fields='g.*', $withmembers=false, $participationonly = false) { + global $DB, $USER; // We need to check that we each field in the fields list belongs to the group table and that it has not being // aliased. If its something else we need to avoid the cache and run the query as who knows whats going on. @@ -249,7 +273,7 @@ function groups_get_all_groups($courseid, $userid=0, $groupingid=0, $fields='g.* } } - if (empty($userid) && $knownfields && !$withmembers) { + if (empty($userid) && $knownfields && !$withmembers && \core_group\visibility::can_view_all_groups($courseid)) { // We can use the cache. $data = groups_get_course_data($courseid); if (empty($groupingid)) { @@ -266,6 +290,9 @@ function groups_get_all_groups($courseid, $userid=0, $groupingid=0, $fields='g.* } } } + if ($participationonly) { + $groups = array_filter($groups, fn($group) => $group->participation); + } // Yay! We could use the cache. One more query saved. return $groups; } @@ -289,14 +316,44 @@ function groups_get_all_groups($courseid, $userid=0, $groupingid=0, $fields='g.* array_unshift($params, $courseid); + $visibilityfrom = ''; + $visibilitywhere = ''; + $viewhidden = has_capability('moodle/course:viewhiddengroups', context_course::instance($courseid)); + if (!$viewhidden) { + // Apply group visibility restrictions. Only return groups where visibility is ALL, or the current user is a member and the + // visibility is MEMBERS or OWN. + $userids = []; + if (empty($userid)) { + $userids = [$USER->id]; + $visibilityfrom = "LEFT JOIN {groups_members} gm ON gm.groupid = g.id AND gm.userid = ?"; + } + [$insql, $inparams] = $DB->get_in_or_equal([GROUPS_VISIBILITY_MEMBERS, GROUPS_VISIBILITY_OWN]); + $visibilitywhere = "AND (g.visibility = ? OR (g.visibility $insql AND gm.id IS NOT NULL))"; + $params = array_merge( + $userids, + $params, + [GROUPS_VISIBILITY_ALL], + $inparams + ); + } + + $participationwhere = ''; + if ($participationonly) { + $participationwhere = "AND g.participation = ?"; + $params = array_merge($params, [1]); + } + $results = $DB->get_records_sql(" SELECT $fields FROM {groups} g $userfrom $groupingfrom + $visibilityfrom WHERE g.courseid = ? $userwhere $groupingwhere + $visibilitywhere + $participationwhere ORDER BY g.name ASC", $params); if (!$withmembers) { @@ -312,7 +369,44 @@ function groups_get_all_groups($courseid, $userid=0, $groupingid=0, $fields='g.* $groups[$row->id] = $row; $groups[$row->id]->members = []; } - $groupmembers = $DB->get_records_list('groups_members', 'groupid', array_keys($groups)); + + $gmvisibilityfrom = ''; + $gmvisibilitywhere = ''; + $gmvisibilityparams = []; + if (!$viewhidden) { + // Only return membership records where visibility is ALL, visibility is MEMBERS and the current user is a member, + // or visibility is OWN and the record is for the current user. + $gmvisibilityfrom = " + JOIN {groups} g ON gm.groupid = g.id + "; + $gmvisibilitywhere = " + AND (g.visibility = ? + OR (g.visibility = ? + AND g.id IN (SELECT gm2.groupid FROM {groups_members} gm2 WHERE gm2.groupid = g.id AND gm2.userid = ?)) + OR (g.visibility = ? + AND gm.userid = ?))"; + $gmvisibilityparams = [ + GROUPS_VISIBILITY_ALL, + GROUPS_VISIBILITY_MEMBERS, + $USER->id, + GROUPS_VISIBILITY_OWN, + $USER->id + ]; + } + + $groupmembers = []; + if (!empty($groups)) { + [$gmin, $gmparams] = $DB->get_in_or_equal(array_keys($groups)); + $params = array_merge($gmparams, $gmvisibilityparams); + $gmsql = " + SELECT gm.* + FROM {groups_members} gm + $gmvisibilityfrom + WHERE gm.groupid $gmin + $gmvisibilitywhere"; + $groupmembers = $DB->get_records_sql($gmsql, $params); + } + foreach ($groupmembers as $gm) { $groups[$gm->groupid]->members[$gm->userid] = $gm->userid; } @@ -328,12 +422,23 @@ function groups_get_all_groups($courseid, $userid=0, $groupingid=0, $fields='g.* */ function groups_get_my_groups() { global $DB, $USER; + + $params = ['userid' => $USER->id]; + + $viewhidden = has_capability('moodle/course:viewhiddengroups', context_system::instance()); + $visibilitywhere = ''; + if (!$viewhidden) { + $params['novisibility'] = GROUPS_VISIBILITY_NONE; + $visibilitywhere = 'AND g.visibility != :novisibility'; + } + return $DB->get_records_sql("SELECT * FROM {groups_members} gm JOIN {groups} g ON g.id = gm.groupid - WHERE gm.userid = ? - ORDER BY name ASC", array($USER->id)); + WHERE gm.userid = :userid + $visibilitywhere + ORDER BY name ASC", $params); } /** @@ -347,23 +452,44 @@ function groups_get_my_groups() { function groups_get_user_groups($courseid, $userid=0) { global $USER, $DB; + if (empty($courseid)) { + return ['0' => []]; + } + if (empty($userid)) { $userid = $USER->id; } + $usergroups = false; + $viewhidden = has_capability('moodle/course:viewhiddengroups', context_course::instance($courseid)); + $viewall = \core_group\visibility::can_view_all_groups($courseid); + $cache = cache::make('core', 'user_group_groupings'); - // Try to retrieve group ids from the cache. - $usergroups = $cache->get($userid); + if ($viewall) { + // Try to retrieve group ids from the cache. + $usergroups = $cache->get($userid); + } if ($usergroups === false) { + $sql = "SELECT g.id, g.courseid, gg.groupingid FROM {groups} g JOIN {groups_members} gm ON gm.groupid = g.id LEFT JOIN {groupings_groups} gg ON gg.groupid = g.id - WHERE gm.userid = ?"; + WHERE gm.userid = :userid"; - $rs = $DB->get_recordset_sql($sql, array($userid)); + $params = ['userid' => $userid]; + + if (!$viewhidden) { + // Apply visibility restrictions. + // Everyone can see who is in groups with ALL visibility. + list($visibilitywhere, $visibilityparams) = \core_group\visibility::sql_group_visibility_where($userid); + $sql .= " AND " . $visibilitywhere; + $params = array_merge($params, $visibilityparams); + } + + $rs = $DB->get_recordset_sql($sql, $params); $usergroups = array(); $allgroups = array(); @@ -390,8 +516,10 @@ function groups_get_user_groups($courseid, $userid=0) { $usergroups[$cid]['0'] = array_keys($allgroups[$cid]); // All user groups in the course. } - // Cache the data. - $cache->set($userid, $usergroups); + if ($viewall) { + // Cache the data, if we got the full list of groups. + $cache->set($userid, $usergroups); + } } if (array_key_exists($courseid, $usergroups)) { @@ -432,7 +560,28 @@ function groups_is_member($groupid, $userid=null) { $userid = $USER->id; } - return $DB->record_exists('groups_members', array('groupid'=>$groupid, 'userid'=>$userid)); + $courseid = $DB->get_field('groups', 'courseid', ['id' => $groupid]); + if (!$courseid) { + return false; + } + + if (\core_group\visibility::can_view_all_groups($courseid)) { + return $DB->record_exists('groups_members', ['groupid' => $groupid, 'userid' => $userid]); + } + + $sql = "SELECT * + FROM {groups_members} gm + JOIN {groups} g ON gm.groupid = g.id + WHERE g.id = :groupid + AND gm.userid = :userid"; + $params = ['groupid' => $groupid, 'userid' => $userid]; + + list($visibilitywhere, $visibilityparams) = \core_group\visibility::sql_group_visibility_where($userid); + + $sql .= " AND " . $visibilitywhere; + $params = array_merge($params, $visibilityparams); + + return $DB->record_exists_sql($sql, $params); } /** @@ -489,12 +638,34 @@ function groups_has_membership($cm, $userid=null) { * group or false if no users or an error returned. */ function groups_get_members($groupid, $fields='u.*', $sort='lastname ASC') { - global $DB; + global $DB, $USER; - return $DB->get_records_sql("SELECT $fields - FROM {user} u, {groups_members} gm - WHERE u.id = gm.userid AND gm.groupid = ? - ORDER BY $sort", array($groupid)); + if (empty($groupid)) { + return []; + } + + $courseid = $DB->get_field('groups', 'courseid', ['id' => $groupid]); + + $select = "SELECT $fields"; + $from = "FROM {user} u + JOIN {groups_members} gm ON gm.userid = u.id"; + $where = "WHERE gm.groupid = :groupid"; + $order = "ORDER BY $sort"; + + $params = ['groupid' => $groupid]; + + if (!\core_group\visibility::can_view_all_groups($courseid)) { + $from .= " JOIN {groups} g ON g.id = gm.groupid"; + // Can view memberships of visibility is ALL, visibility is MEMBERS and current user is a member, + // or visibility is OWN and this is their membership. + list($visibilitywhere, $visibilityparams) = \core_group\visibility::sql_member_visibility_where(); + $params = array_merge($params, $visibilityparams); + $where .= $visibilitywhere; + } + + $sql = implode(PHP_EOL, [$select, $from, $where, $order]); + + return $DB->get_records_sql($sql, $params); } @@ -778,11 +949,12 @@ function groups_print_activity_menu($cm, $urlroot, $return=false, $hideallpartic $usergroups = array(); if ($groupmode == VISIBLEGROUPS or $aag) { - $allowedgroups = groups_get_all_groups($cm->course, 0, $cm->groupingid); // any group in grouping + $allowedgroups = groups_get_all_groups($cm->course, 0, $cm->groupingid, 'g.*', false, true); // Any group in grouping. // Get user's own groups and put to the top. - $usergroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid); + $usergroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, true); } else { - $allowedgroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid); // only assigned groups + // Only assigned groups. + $allowedgroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, true); } $activegroup = groups_get_activity_group($cm, true, $allowedgroups); @@ -900,9 +1072,9 @@ function groups_get_activity_group($cm, $update=false, $allowedgroups=null) { if (!is_array($allowedgroups)) { if ($groupmode == VISIBLEGROUPS or $groupmode === 'aag') { - $allowedgroups = groups_get_all_groups($cm->course, 0, $cm->groupingid); + $allowedgroups = groups_get_all_groups($cm->course, 0, $cm->groupingid, 'g.*', false, true); } else { - $allowedgroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid); + $allowedgroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, true); } } @@ -951,10 +1123,10 @@ function groups_get_activity_allowed_groups($cm,$userid=0) { // then they can access all groups for the activity... $context = context_module::instance($cm->id); if ($groupmode == VISIBLEGROUPS or has_capability('moodle/site:accessallgroups', $context, $userid)) { - return groups_get_all_groups($cm->course, 0, $cm->groupingid); + return groups_get_all_groups($cm->course, 0, $cm->groupingid, 'g.*', false, true); } else { // ...otherwise they can only access groups they belong to - return groups_get_all_groups($cm->course, $userid, $cm->groupingid); + return groups_get_all_groups($cm->course, $userid, $cm->groupingid, 'g.*', false, true); } } @@ -1210,7 +1382,8 @@ function _group_verify_activegroup($courseid, $groupmode, $groupingid, array $al $SESSION->activegroup[$courseid][$groupmode][$groupingid] = 0; // all groups by default if user has accessallgroups } else if ($allowedgroups) { - if ($groupmode != SEPARATEGROUPS and $mygroups = groups_get_all_groups($courseid, $USER->id, $groupingid)) { + if ($groupmode != SEPARATEGROUPS + && $mygroups = groups_get_all_groups($courseid, $USER->id, $groupingid, 'g.*', false, true)) { $firstgroup = reset($mygroups); } else { $firstgroup = reset($allowedgroups); @@ -1359,11 +1532,29 @@ function groups_get_groups_members($groupsids, $extrafields=null, $sort='lastnam $userfieldsapi = \core_user\fields::for_userpic()->including(...($extrafields ?? [])); $userfields = $userfieldsapi->get_sql('u', false, '', '', false)->selects; - list($insql, $params) = $DB->get_in_or_equal($groupsids); + list($insql, $params) = $DB->get_in_or_equal($groupsids, SQL_PARAMS_NAMED); + + $courseids = $DB->get_fieldset_sql("SELECT DISTINCT courseid FROM {groups} WHERE id $insql", $params); + + if (count($courseids) > 1) { + // Groups from multiple courses. Have to check permission in system context. + $context = context_system::instance(); + } else { + $courseid = reset($courseids); + $context = context_course::instance($courseid); + } + + $visibilitywhere = ''; + if (!has_capability('moodle/course:viewhiddengroups', $context)) { + list($visibilitywhere, $visibilityparams) = \core_group\visibility::sql_member_visibility_where(); + $params = array_merge($params, $visibilityparams); + } return $DB->get_records_sql("SELECT $userfields - FROM {user} u, {groups_members} gm - WHERE u.id = gm.userid AND gm.groupid $insql + FROM {user} u + JOIN {groups_members} gm ON u.id = gm.userid + JOIN {groups} g ON g.id = gm.groupid + WHERE gm.groupid $insql $visibilitywhere GROUP BY $userfields ORDER BY $sort", $params); } @@ -1380,7 +1571,7 @@ function groups_get_activity_shared_group_members($cm, $userid = null) { global $USER; if (empty($userid)) { - $userid = $USER; + $userid = $USER->id; } $groupsids = array_keys(groups_get_activity_allowed_groups($cm, $userid)); diff --git a/lib/testing/generator/data_generator.php b/lib/testing/generator/data_generator.php index 92e5e79e02b..0c426d1fb50 100644 --- a/lib/testing/generator/data_generator.php +++ b/lib/testing/generator/data_generator.php @@ -541,6 +541,14 @@ EOD; $record['descriptionformat'] = FORMAT_MOODLE; } + if (!isset($record['visibility'])) { + $record['visibility'] = GROUPS_VISIBILITY_ALL; + } + + if (!isset($record['participation'])) { + $record['participation'] = true; + } + $id = groups_create_group((object)$record); // Allow tests to set group pictures. diff --git a/lib/tests/grouplib_test.php b/lib/tests/grouplib_test.php index f5a2bedc232..364f8660738 100644 --- a/lib/tests/grouplib_test.php +++ b/lib/tests/grouplib_test.php @@ -16,6 +16,8 @@ namespace core; +use core_group\visibility; + /** * Unit tests for lib/grouplib.php * @@ -1909,4 +1911,420 @@ class grouplib_test extends \advanced_testcase { $this->assertCount(2, $members); // Now I see members of group 3. $this->assertEqualsCanonicalizing([$user1->id, $user3->id], array_keys($members)); } + + /** + * Generate a set of groups with different visibility levels and users to assign to them. + * + * @return array + */ + protected function create_groups_with_visibilty(): array { + $this->resetAfterTest(true); + $generator = $this->getDataGenerator(); + + // Create courses. + $course = $generator->create_course(); + + // Create users. + $users = [ + 1 => $generator->create_user(), + 2 => $generator->create_user(), + 3 => $generator->create_user(), + 4 => $generator->create_user(), + 5 => $generator->create_user(), + ]; + + // Enrol users. + $generator->enrol_user($users[1]->id, $course->id); + $generator->enrol_user($users[2]->id, $course->id); + $generator->enrol_user($users[3]->id, $course->id); + $generator->enrol_user($users[4]->id, $course->id); + $generator->enrol_user($users[5]->id, $course->id, 'editingteacher'); + + // Create groups. + $groups = [ + 'all' => $generator->create_group(['courseid' => $course->id, 'visibility' => GROUPS_VISIBILITY_ALL]), + 'members' => $generator->create_group(['courseid' => $course->id, 'visibility' => GROUPS_VISIBILITY_MEMBERS]), + 'own' => $generator->create_group(['courseid' => $course->id, 'visibility' => GROUPS_VISIBILITY_OWN]), + 'none' => $generator->create_group(['courseid' => $course->id, 'visibility' => GROUPS_VISIBILITY_NONE]), + ]; + + return [ + $users, + $groups, + $course, + ]; + } + + /** + * Tests getting groups and group members based on visibility settings. + * + * This also covers the groupdata cache, since calls without $withmembers = true use the cache. + * + * @covers \groups_get_all_groups() + */ + public function test_get_all_groups_with_visibility() { + list($users, $groups, $course) = $this->create_groups_with_visibilty(); + + // Assign users to groups. + $generator = $this->getDataGenerator(); + $generator->create_group_member(['groupid' => $groups['all']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[2]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[3]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[4]->id]); + + $this->setUser($users[1]); + $groups1 = groups_get_all_groups($course->id); + // User1 is in groups all, members and own, and can see them. + $this->assertArrayHasKey($groups['all']->id, $groups1); + $this->assertArrayHasKey($groups['members']->id, $groups1); + $this->assertArrayHasKey($groups['own']->id, $groups1); + $this->assertArrayNotHasKey($groups['none']->id, $groups1); + // User1 can see members of groups all and members, but only themselves in group own. + $groupsmembers1 = groups_get_all_groups($course->id, 0, 0, 'g.*', true); + $this->assertArrayHasKey($users[1]->id, $groupsmembers1[$groups['all']->id]->members); + $this->assertArrayHasKey($users[1]->id, $groupsmembers1[$groups['members']->id]->members); + $this->assertArrayHasKey($users[2]->id, $groupsmembers1[$groups['members']->id]->members); + $this->assertArrayHasKey($users[1]->id, $groupsmembers1[$groups['own']->id]->members); + $this->assertArrayNotHasKey($users[3]->id, $groupsmembers1[$groups['own']->id]->members); + + $this->setUser($users[2]); + $groups2 = groups_get_all_groups($course->id); + // User2 is in group members, and can see group all as well. + $this->assertArrayHasKey($groups['all']->id, $groups2); + $this->assertArrayHasKey($groups['members']->id, $groups2); + $this->assertArrayNotHasKey($groups['own']->id, $groups2); + $this->assertArrayNotHasKey($groups['none']->id, $groups2); + // User2 can see members of groups all and members. + $groupsmembers2 = groups_get_all_groups($course->id, 0, 0, 'g.*', true); + $this->assertArrayHasKey($users[1]->id, $groupsmembers2[$groups['all']->id]->members); + $this->assertArrayHasKey($users[1]->id, $groupsmembers2[$groups['members']->id]->members); + $this->assertArrayHasKey($users[2]->id, $groupsmembers2[$groups['members']->id]->members); + + $this->setUser($users[3]); + $groups3 = groups_get_all_groups($course->id); + // User3 is in group own, and can see group all as well. + $this->assertArrayHasKey($groups['all']->id, $groups3); + $this->assertArrayNotHasKey($groups['members']->id, $groups3); + $this->assertArrayHasKey($groups['own']->id, $groups3); + $this->assertArrayNotHasKey($groups['none']->id, $groups3); + $groupsmembers3 = groups_get_all_groups($course->id, 0, 0, 'g.*', true); + // User3 can see members of group all, but only themselves in group own. + $this->assertArrayHasKey($users[1]->id, $groupsmembers3[$groups['all']->id]->members); + $this->assertArrayHasKey($users[3]->id, $groupsmembers3[$groups['own']->id]->members); + $this->assertArrayNotHasKey($users[1]->id, $groupsmembers3[$groups['own']->id]->members); + + $this->setUser($users[4]); + $groups4 = groups_get_all_groups($course->id); + // User4 can see group all and its members. They are in group none but cannot see it. + $this->assertArrayHasKey($groups['all']->id, $groups4); + $this->assertArrayNotHasKey($groups['members']->id, $groups4); + $this->assertArrayNotHasKey($groups['own']->id, $groups4); + $this->assertArrayNotHasKey($groups['none']->id, $groups4); + // User4 can see members of group all. + $groupsmembers4 = groups_get_all_groups($course->id, 0, 0, 'g.*', true); + $this->assertArrayHasKey($users[1]->id, $groupsmembers4[$groups['all']->id]->members); + + $this->setUser($users[5]); + $groups5 = groups_get_all_groups($course->id); + // User5 is has viewallgroups, so can see all groups. + $this->assertArrayHasKey($groups['all']->id, $groups5); + $this->assertArrayHasKey($groups['members']->id, $groups5); + $this->assertArrayHasKey($groups['own']->id, $groups5); + $this->assertArrayHasKey($groups['none']->id, $groups5); + // User5 is has viewallgroups, so can see all members. + $groupsmembers5 = groups_get_all_groups($course->id, 0, 0, 'g.*', true); + $this->assertArrayHasKey($users[1]->id, $groupsmembers5[$groups['all']->id]->members); + $this->assertArrayHasKey($users[1]->id, $groupsmembers5[$groups['members']->id]->members); + $this->assertArrayHasKey($users[2]->id, $groupsmembers5[$groups['members']->id]->members); + $this->assertArrayHasKey($users[1]->id, $groupsmembers5[$groups['own']->id]->members); + $this->assertArrayHasKey($users[3]->id, $groupsmembers5[$groups['own']->id]->members); + $this->assertArrayHasKey($users[4]->id, $groupsmembers5[$groups['none']->id]->members); + + } + + /** + * Tests getting groups the current user is a member of, with visibility settings applied. + * + * @covers \groups_get_my_groups() + */ + public function test_get_my_groups_with_visibility() { + list($users, $groups) = $this->create_groups_with_visibilty(); + + // Assign users to groups. + $generator = $this->getDataGenerator(); + $generator->create_group_member(['groupid' => $groups['all']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['all']->id, 'userid' => $users[5]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[5]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[5]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[5]->id]); + + $generator->role_assign('editingteacher', $users[5]->id, \context_system::instance()); + + // User can see all groups they are in, except group with visibility::NONE. + $this->setUser($users[1]); + $groups1 = groups_get_my_groups(); + $this->assertCount(3, $groups1); + $groupids1 = array_map(function($groupmember) { + return $groupmember->groupid; + }, $groups1); + sort($groupids1); + $this->assertEquals([$groups['all']->id, $groups['members']->id, $groups['own']->id], $groupids1); + + $this->setUser($users[5]); + $groups2 = groups_get_my_groups(); + $this->assertCount(4, $groups2); + $groupids2 = array_map(function($groupmember) { + return $groupmember->groupid; + }, $groups2); + sort($groupids2); + $this->assertEquals([$groups['all']->id, $groups['members']->id, $groups['own']->id, $groups['none']->id], $groupids2); + } + + /** + * Tests getting groups a user is a member of, with visibility settings applied. + * + * @covers \groups_get_user_groups() + */ + public function test_get_user_groups_with_visibility() { + list($users, $groups, $course) = $this->create_groups_with_visibilty(); + + // Assign users to groups. + $generator = $this->getDataGenerator(); + $generator->create_group_member(['groupid' => $groups['all']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[2]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[3]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[4]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[1]->id]); + + // Run as unprivileged user. + $this->setUser($users[1]); + // Own groups - should see all groups except group with visibility::NONE. + $usergroups1 = groups_get_user_groups($course->id, $users[1]->id); + $this->assertEquals([$groups['all']->id, $groups['members']->id, $groups['own']->id], $usergroups1[0]); + // Fellow member of a group with visiblity::MEMBERS. Should see that group. + $usergroups2 = groups_get_user_groups($course->id, $users[2]->id); + $this->assertEquals([$groups['members']->id], $usergroups2[0]); + // Fellow member of a group with visiblity::OWN. Should not see that group. + $usergroups3 = groups_get_user_groups($course->id, $users[3]->id); + $this->assertEmpty($usergroups3[0]); + // Fellow member of a group with visiblity::NONE. Should not see that group. + $usergroups4 = groups_get_user_groups($course->id, $users[4]->id); + $this->assertEmpty($usergroups4[0]); + + // Run as a user with viewhiddengroups. Should see all group memberships for each member. + $this->setUser($users[5]); + $usergroups1 = groups_get_user_groups($course->id, $users[1]->id); + $this->assertEquals([$groups['all']->id, $groups['members']->id, $groups['own']->id, $groups['none']->id], $usergroups1[0]); + // Fellow member of a group with visiblity::MEMBERS. Should see that group. + $usergroups2 = groups_get_user_groups($course->id, $users[2]->id); + $this->assertEquals([$groups['members']->id], $usergroups2[0]); + // Fellow member of a group with visiblity::OWN. Should not see that group. + $usergroups3 = groups_get_user_groups($course->id, $users[3]->id); + $this->assertEquals([$groups['own']->id], $usergroups3[0]); + // Fellow member of a group with visiblity::NONE. Should not see that group. + $usergroups4 = groups_get_user_groups($course->id, $users[4]->id); + $this->assertEquals([$groups['none']->id], $usergroups4[0]); + + } + + /** + * Test groups_is_member() using groups with different visibility settings. + * + * @covers \groups_is_member() + */ + public function test_groups_is_member_with_visibility(): void { + list($users, $groups) = $this->create_groups_with_visibilty(); + + // Assign users to groups. + $generator = $this->getDataGenerator(); + $generator->create_group_member(['groupid' => $groups['all']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[2]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[3]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[4]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[1]->id]); + + // Run as unprivileged user. + $this->setUser($users[1]); + $this->assertTrue(groups_is_member($groups['all']->id, $users[1]->id)); // All can see members. + $this->assertTrue(groups_is_member($groups['members']->id, $users[1]->id)); // Can see members. + $this->assertTrue(groups_is_member($groups['own']->id, $users[1]->id)); // Can see own membership. + $this->assertFalse(groups_is_member($groups['none']->id, $users[1]->id)); // Cannot see group. + + $this->assertFalse(groups_is_member($groups['all']->id, $users[2]->id)); // Not a member. + $this->assertTrue(groups_is_member($groups['members']->id, $users[2]->id)); // Can see other members. + $this->assertFalse(groups_is_member($groups['own']->id, $users[3]->id)); // Can only see own membership, not others. + $this->assertFalse(groups_is_member($groups['none']->id, $users[4]->id)); // Cannot see group. + + // Run as a user not in group 1 or 2. + $this->setUser($users[3]); + $this->assertTrue(groups_is_member($groups['all']->id, $users[1]->id)); // All can see members. + $this->assertFalse(groups_is_member($groups['members']->id, $users[2]->id)); // Cannot see members of the group. + + // Run as a user with viewhiddengroups. Should be able to see memberships that exist in any group. + $this->setUser($users[5]); + $this->assertTrue(groups_is_member($groups['all']->id, $users[1]->id)); + $this->assertTrue(groups_is_member($groups['members']->id, $users[1]->id)); + $this->assertTrue(groups_is_member($groups['own']->id, $users[1]->id)); + $this->assertTrue(groups_is_member($groups['none']->id, $users[1]->id)); + + $this->assertFalse(groups_is_member($groups['all']->id, $users[2]->id)); // Not a member. + $this->assertTrue(groups_is_member($groups['members']->id, $users[2]->id)); + $this->assertTrue(groups_is_member($groups['own']->id, $users[3]->id)); + $this->assertTrue(groups_is_member($groups['none']->id, $users[4]->id)); + } + + /** + * Test groups_get_members() using groups with different visibility settings. + * + * @covers \groups_get_members() + */ + public function test_groups_get_members_with_visibility(): void { + list($users, $groups) = $this->create_groups_with_visibilty(); + + // Assign users to groups. + $generator = $this->getDataGenerator(); + $generator->create_group_member(['groupid' => $groups['all']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[2]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[3]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[4]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[1]->id]); + + // Run as unprivileged user. + $this->setUser($users[1]); + $this->assertEquals([$users[1]->id], + array_keys(groups_get_members($groups['all']->id, 'u.id', 'u.id'))); // All can see members. + $this->assertEquals([$users[1]->id, $users[2]->id], + array_keys(groups_get_members($groups['members']->id, 'u.id', 'u.id'))); // Can see members. + $this->assertEquals([$users[1]->id], + array_keys(groups_get_members($groups['own']->id, 'u.id', 'u.id'))); // Can see own membership. + $this->assertEquals([], array_keys(groups_get_members($groups['none']->id, 'u.id', 'u.id'))); // Cannot see group. + + // Run as a user not in group 1 or 2. + $this->setUser($users[3]); + $this->assertEquals([$users[1]->id], + array_keys(groups_get_members($groups['all']->id, 'u.id', 'u.id'))); // All can see members. + $this->assertEquals([], array_keys(groups_get_members($groups['members']->id, 'u.id', 'u.id'))); // Cannot see members. + + // Run as a user with viewhiddengroups. Should be able to see memberships that exist in any group. + $this->setUser($users[5]); + $this->assertEquals([$users[1]->id], array_keys(groups_get_members($groups['all']->id, 'u.id', 'u.id'))); + $this->assertEquals([$users[1]->id, $users[2]->id], + array_keys(groups_get_members($groups['members']->id, 'u.id', 'u.id'))); + $this->assertEquals([$users[1]->id, $users[3]->id], + array_keys(groups_get_members($groups['own']->id, 'u.id', 'u.id'))); + $this->assertEquals([$users[1]->id, $users[4]->id], + array_keys(groups_get_members($groups['none']->id, 'u.id', 'u.id'))); + } + + /** + * Test groups_get_groups_members() using groups with different visibility settings. + * + * @covers \groups_get_groups_members() + */ + public function test_groups_get_groups_members_with_visibility(): void { + list($users, $groups) = $this->create_groups_with_visibilty(); + + // Assign users to groups. + $generator = $this->getDataGenerator(); + $generator->create_group_member(['groupid' => $groups['all']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['members']->id, 'userid' => $users[2]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[1]->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $users[3]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[4]->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $users[1]->id]); + + $groupids = [$groups['all']->id, $groups['members']->id, $groups['own']->id, $groups['none']->id]; + + $this->setUser($users[1]); + // Can see self in group1/3, other users in group2. + $this->assertEquals([$users[1]->id, $users[2]->id], array_keys(groups_get_groups_members($groupids, null, 'id ASC'))); + + $this->setUser($users[2]); + // Can see self in group2, user1 from group1/2. + $this->assertEquals([$users[1]->id, $users[2]->id], array_keys(groups_get_groups_members($groupids, null, 'id ASC'))); + + $this->setUser($users[3]); + // Can see self in group3, user1 from group1. + $this->assertEquals([$users[1]->id, $users[3]->id], array_keys(groups_get_groups_members($groupids, null, 'id ASC'))); + + $this->setUser($users[4]); + // Can see user1 from group1, cannot see self in group4. + $this->assertEquals([$users[1]->id], array_keys(groups_get_groups_members($groupids, null, 'id ASC'))); + + $this->setUser($users[5]); + // Can see all users from all groups. + $this->assertEquals([$users[1]->id, $users[2]->id, $users[3]->id, $users[4]->id], + array_keys(groups_get_groups_members($groupids, null, 'id ASC'))); + } + + /** + * Only groups with participation == true should be returned for an activity. + * + * @covers \groups_get_activity_allowed_groups() + * @return void + * @throws \coding_exception + */ + public function test_groups_get_activity_allowed_groups(): void { + $this->resetAfterTest(true); + $generator = $this->getDataGenerator(); + + // Create courses. + $course = $generator->create_course(); + + // Create user. + $user = $generator->create_user(); + + // Enrol user. + $generator->enrol_user($user->id, $course->id); + + // Create groups. + $groups = [ + 'all-p' => $generator->create_group(['courseid' => $course->id, 'visibility' => GROUPS_VISIBILITY_ALL]), + 'members-p' => $generator->create_group(['courseid' => $course->id, 'visibility' => GROUPS_VISIBILITY_MEMBERS]), + 'all-n' => $generator->create_group([ + 'courseid' => $course->id, + 'visibility' => GROUPS_VISIBILITY_ALL, + 'participation' => false + ]), + 'members-n' => $generator->create_group([ + 'courseid' => $course->id, + 'visibility' => GROUPS_VISIBILITY_MEMBERS, + 'participation' => false + ]), + 'own' => $generator->create_group(['courseid' => $course->id, 'visibility' => GROUPS_VISIBILITY_OWN]), + 'none' => $generator->create_group(['courseid' => $course->id, 'visibility' => GROUPS_VISIBILITY_NONE]), + ]; + // Add user to all groups. + $generator->create_group_member(['groupid' => $groups['all-p']->id, 'userid' => $user->id]); + $generator->create_group_member(['groupid' => $groups['members-p']->id, 'userid' => $user->id]); + $generator->create_group_member(['groupid' => $groups['all-n']->id, 'userid' => $user->id]); + $generator->create_group_member(['groupid' => $groups['members-n']->id, 'userid' => $user->id]); + $generator->create_group_member(['groupid' => $groups['own']->id, 'userid' => $user->id]); + $generator->create_group_member(['groupid' => $groups['none']->id, 'userid' => $user->id]); + + $module = $generator->create_module('forum', ['course' => $course->id]); + $cm = get_fast_modinfo($course)->get_cm($module->cmid); + + $activitygroups = groups_get_activity_allowed_groups($cm, $user->id); + + $this->assertContains((int)$groups['all-p']->id, array_keys($activitygroups)); + $this->assertContains((int)$groups['members-p']->id, array_keys($activitygroups)); + $this->assertNotContains((int)$groups['all-n']->id, array_keys($activitygroups)); + $this->assertNotContains((int)$groups['members-n']->id, array_keys($activitygroups)); + $this->assertNotContains((int)$groups['own']->id, array_keys($activitygroups)); + $this->assertNotContains((int)$groups['none']->id, array_keys($activitygroups)); + + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 5d85b1306e4..6338b524bc6 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -79,6 +79,17 @@ information provided here is intended especially for developers. Course formats using components will be allowed to use one level indentation only. * The method `flexible_table::set_columnsattributes` now can be used with 'class' key to add custom classes to the DOM. * The editor_tinymce plugin has been removed from core. +* Support for serving of AMD modules built in really old versions of Moodle (<= 3.8) has been removed. + Please ensure that your AMD modules have been rebuilt with a supported Moodle version. +* Addition of new 'visibility' and 'participation' fields in the groups table, and 'moodle/group:viewhiddengroups' capability. + The following grouplib functions will now return groups and members based on group visibility and the user's permissions: + - groups_get_all_groups() + - groups_get_user_groups() + - groups_get_my_groups() + - groups_is_member() + - groups_get_members() + - groups_get_groups_members() + groups_print_activity_menu() will now only return groups where particiation == true. === 4.1 === diff --git a/message/tests/behat/group_conversation.feature b/message/tests/behat/group_conversation.feature index 7974763a8f5..035bdf1074c 100644 --- a/message/tests/behat/group_conversation.feature +++ b/message/tests/behat/group_conversation.feature @@ -108,3 +108,18 @@ Feature: Create conversations for course's groups And I open messaging information And "No participants" "core_message > Message member" should not exist And "Student 4" "core_message > Message member" should exist + + Scenario: Disable messaging for private groups + Given the following "groups" exist: + | name | course | idnumber | visibility | enablemessaging | + | Messaging group | C1 | MG | 0 | 1 | + | No messaging group | C1 | NM | 2 | 1 | + And the following "group members" exist: + | user | group | + | student1 | MG | + | student1 | NM | + When I log in as "student1" + And I open messaging + And I open the "Group" conversations list + Then "Messaging group" "core_message > Message" should exist + Then "No messaging group" "core_message > Message" should not exist diff --git a/mod/assign/override_form.php b/mod/assign/override_form.php index 514db23f603..d0990eb59fb 100644 --- a/mod/assign/override_form.php +++ b/mod/assign/override_form.php @@ -128,7 +128,9 @@ class assign_override_form extends moodleform { $groupchoices = array(); foreach ($groups as $group) { - $groupchoices[$group->id] = format_string($group->name, true, $this->context); + if ($group->visibility != \core_group\visibility::NONE) { + $groupchoices[$group->id] = format_string($group->name, true, $this->context); + } } unset($groups); diff --git a/mod/assign/tests/behat/assign_group_override.feature b/mod/assign/tests/behat/assign_group_override.feature index 8bce7188733..5ffffad3c34 100644 --- a/mod/assign/tests/behat/assign_group_override.feature +++ b/mod/assign/tests/behat/assign_group_override.feature @@ -237,3 +237,23 @@ Feature: Assign group override And I select "Group overrides" from the "jump" singleselect Then I should see "Group 1" in the ".generaltable" "css_element" And I should not see "Group 2" in the ".generaltable" "css_element" + + Scenario: "Not visible" groups should not be available for group overrides + Given the following "groups" exist: + | name | course | idnumber | visibility | participation | + | Visible to all/Participation | C1 | VP | 0 | 1 | + | Visible to members/Participation | C1 | MP | 1 | 1 | + | See own membership | C1 | O | 2 | 0 | + | Not visible | C1 | N | 3 | 0 | + | Visible to all/Non-Participation | C1 | VN | 0 | 0 | + | Visible to members/Non-Participation | C1 | MN | 1 | 0 | + When I am on the "Test assignment name" Activity page logged in as teacher1 + And I navigate to "Overrides" in current page administration + And I select "Group overrides" from the "jump" singleselect + And I press "Add group override" + Then I should see "Visible to all/Participation" in the "Override group" "select" + And I should see "Visible to all/Non-Participation" in the "Override group" "select" + And I should see "Visible to members" in the "Override group" "select" + And I should see "Visible to members/Non-Participation" in the "Override group" "select" + And I should see "See own membership" in the "Override group" "select" + And I should not see "Not visible" in the "Override group" "select" diff --git a/mod/lesson/override_form.php b/mod/lesson/override_form.php index 4fd54a012f0..0674a18cd8d 100644 --- a/mod/lesson/override_form.php +++ b/mod/lesson/override_form.php @@ -113,7 +113,9 @@ class lesson_override_form extends moodleform { $groupchoices = array(); foreach ($groups as $group) { - $groupchoices[$group->id] = format_string($group->name, true, $this->context); + if ($group->visibility != \core_group\visibility::NONE) { + $groupchoices[$group->id] = format_string($group->name, true, $this->context); + } } unset($groups); diff --git a/mod/lesson/tests/behat/lesson_group_override.feature b/mod/lesson/tests/behat/lesson_group_override.feature index 8b574db9ae4..338601619b1 100644 --- a/mod/lesson/tests/behat/lesson_group_override.feature +++ b/mod/lesson/tests/behat/lesson_group_override.feature @@ -369,3 +369,23 @@ Feature: Lesson group override And I select "Group overrides" from the "jump" singleselect Then I should see "Group 1" in the ".generaltable" "css_element" And I should not see "Group 2" in the ".generaltable" "css_element" + + Scenario: "Not visible" groups should not be available for group overrides + Given the following "groups" exist: + | name | course | idnumber | visibility | participation | + | Visible to all/Participation | C1 | VP | 0 | 1 | + | Visible to members/Participation | C1 | MP | 1 | 1 | + | See own membership | C1 | O | 2 | 0 | + | Not visible | C1 | N | 3 | 0 | + | Visible to all/Non-Participation | C1 | VN | 0 | 0 | + | Visible to members/Non-Participation | C1 | MN | 1 | 0 | + When I am on the "lesson1" Activity page logged in as teacher1 + And I navigate to "Overrides" in current page administration + And I select "Group overrides" from the "jump" singleselect + And I follow "Add group override" + Then I should see "Visible to all/Participation" in the "Override group" "select" + And I should see "Visible to all/Non-Participation" in the "Override group" "select" + And I should see "Visible to members" in the "Override group" "select" + And I should see "Visible to members/Non-Participation" in the "Override group" "select" + And I should see "See own membership" in the "Override group" "select" + And I should not see "Not visible" in the "Override group" "select" \ No newline at end of file diff --git a/mod/quiz/classes/form/edit_override_form.php b/mod/quiz/classes/form/edit_override_form.php index 2a21c5be65b..712b64ff2ad 100644 --- a/mod/quiz/classes/form/edit_override_form.php +++ b/mod/quiz/classes/form/edit_override_form.php @@ -113,7 +113,9 @@ class edit_override_form extends moodleform { $groupchoices = []; foreach ($groups as $group) { - $groupchoices[$group->id] = format_string($group->name, true, $this->context); + if ($group->visibility != \core_group\visibility::NONE) { + $groupchoices[$group->id] = format_string($group->name, true, $this->context); + } } unset($groups); diff --git a/mod/quiz/tests/behat/quiz_group_override.feature b/mod/quiz/tests/behat/quiz_group_override.feature index 3b65be0fa02..f657b1951af 100644 --- a/mod/quiz/tests/behat/quiz_group_override.feature +++ b/mod/quiz/tests/behat/quiz_group_override.feature @@ -117,3 +117,23 @@ Feature: Quiz group override And "Edit" "link" should not exist in the "Group 1" "table_row" And "Copy" "link" should not exist in the "Group 1" "table_row" And "Delete" "link" should not exist in the "Group 1" "table_row" + + Scenario: "Not visible" groups should not be available for group overrides + Given the following "groups" exist: + | name | course | idnumber | visibility | participation | + | Visible to all/Participation | C1 | VP | 0 | 1 | + | Visible to members/Participation | C1 | MP | 1 | 1 | + | See own membership | C1 | O | 2 | 0 | + | Not visible | C1 | N | 3 | 0 | + | Visible to all/Non-Participation | C1 | VN | 0 | 0 | + | Visible to members/Non-Participation | C1 | MN | 1 | 0 | + When I am on the "quiz1" Activity page logged in as teacher1 + And I navigate to "Overrides" in current page administration + And I select "Group overrides" from the "jump" singleselect + And I press "Add group override" + Then I should see "Visible to all/Participation" in the "Override group" "select" + And I should see "Visible to all/Non-Participation" in the "Override group" "select" + And I should see "Visible to members" in the "Override group" "select" + And I should see "Visible to members/Non-Participation" in the "Override group" "select" + And I should see "See own membership" in the "Override group" "select" + And I should not see "Not visible" in the "Override group" "select" \ No newline at end of file diff --git a/version.php b/version.php index f61c6a403a0..467637cedb4 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2023031000.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2023031000.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '4.2dev (Build: 20230310)'; // Human-friendly version name