From 93aceedf9b3588fdb8124417996546b6e5796163 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 21 May 2024 10:48:45 +0100 Subject: [PATCH] MDL-81514 groups: Add participationonly option to activity group menu groups_print_activity_menu() and groups_get_activity_group() now include an additional $participationonly parameter, which is true by default. This can be set false when we want the user to be able to select a non-participation group within an activity, for example if a teacher wants to filter assignment submissions by non-participation groups. It should never be used when the menu is displayed to students, as this may allow them to participate using non-participation groups. groups_sort_menu_options() now has a $splitparticipation parameter, which will split non-participation groups out into their own optgroup at the end of the menu. --- group/upgrade.txt | 7 +++ lang/en/group.php | 1 + lib/grouplib.php | 86 +++++++++++++++++++++++++++++-------- lib/tests/grouplib_test.php | 82 ++++++++++++++++++++++++++++++++++- 4 files changed, 156 insertions(+), 20 deletions(-) diff --git a/group/upgrade.txt b/group/upgrade.txt index f176eb310c811..6b0269a97353e 100644 --- a/group/upgrade.txt +++ b/group/upgrade.txt @@ -1,6 +1,13 @@ This files describes API changes in /group/*, information provided here is intended especially for developers. +=== 4.5 === +* `groups_print_activity_menu()` and `groups_get_activity_group()` now include an additional `$participationonly` parameter, + which is true by default. This can be set false when we want the user to be able to select a non-participation group within + an activity, for example if a teacher wants to filter assignment submissions by non-participation groups. It should never be + used when the menu is displayed to students, as this may allow them to participate using non-participation groups. + Non-participation groups are listed in their own optgroup. + === 4.3 === * The following external methods now return group names correctly formatted: - `core_group_get_groups` diff --git a/lang/en/group.php b/lang/en/group.php index 21bf1c262c077..9c9a4a08a2ab7 100644 --- a/lang/en/group.php +++ b/lang/en/group.php @@ -179,6 +179,7 @@ $string['messagingdisabled'] = 'Successfully disabled messaging in {$a} group(s)'; $string['messagingenabled'] = 'Successfully enabled messaging in {$a} group(s)'; $string['mygroups'] = 'My groups'; +$string['nonparticipation'] = 'Non-participation'; $string['othergroups'] = 'Other groups'; $string['overview'] = 'Overview'; $string['participation'] = 'Show group in dropdown menu for activities in group mode'; diff --git a/lib/grouplib.php b/lib/grouplib.php index 40c4bcc102f57..1e2ba84047591 100644 --- a/lib/grouplib.php +++ b/lib/grouplib.php @@ -833,11 +833,17 @@ function groups_list_to_menu($groups) { * Own groups are removed from allowed groups * @param array $allowedgroups All groups user is allowed to see * @param array $usergroups Groups user belongs to + * @param bool $splitparticipation If true, split each optgroup into "Participation" and "Non-participation" optgroups. * @return array */ -function groups_sort_menu_options($allowedgroups, $usergroups) { - $useroptions = array(); +function groups_sort_menu_options($allowedgroups, $usergroups, bool $splitparticipation = false) { + $nonparticipationgroups = []; + $useroptions = []; if ($usergroups) { + if ($splitparticipation) { + [$usergroups, $usernonparticipation] = groups_split_participation_groups($usergroups); + $nonparticipationgroups = array_merge($nonparticipationgroups, $usernonparticipation); + } $useroptions = groups_list_to_menu($usergroups); // Remove user groups from other groups list. @@ -846,21 +852,54 @@ function groups_sort_menu_options($allowedgroups, $usergroups) { } } - $allowedoptions = array(); + $allowedoptions = []; if ($allowedgroups) { + if ($splitparticipation) { + [$allowedgroups, $allowednonparticipation] = groups_split_participation_groups($allowedgroups); + $nonparticipationgroups = array_merge($nonparticipationgroups, $allowednonparticipation); + } $allowedoptions = groups_list_to_menu($allowedgroups); } if ($useroptions && $allowedoptions) { - return array( - 1 => array(get_string('mygroups', 'group') => $useroptions), - 2 => array(get_string('othergroups', 'group') => $allowedoptions) - ); + $options = [ + 1 => [ + get_string('mygroups', 'group') => $useroptions, + ], + 2 => [ + get_string('othergroups', 'group') => $allowedoptions, + ], + ]; } else if ($useroptions) { - return $useroptions; + $options = $useroptions; } else { - return $allowedoptions; + $options = $allowedoptions; + } + if ($nonparticipationgroups) { + $options[3] = [ + get_string('nonparticipation', 'group') => groups_list_to_menu($nonparticipationgroups), + ]; + } + return $options; +} + +/** + * Split the list of groups into participation and non-participation groups. + * + * @param array $groups List of group records + * @return array[] Menu options for the records, split into "Participation" and "Non-participation" optgroups. + */ +function groups_split_participation_groups(array $groups): array { + $participation = []; + $nonparticipation = []; + foreach ($groups as $group) { + if ($group->participation) { + $participation[] = $group; + } else { + $nonparticipation[] = $group; + } } + return [$participation, $nonparticipation]; } /** @@ -934,9 +973,14 @@ function groups_allgroups_course_menu($course, $urlroot, $update = false, $activ * selecting this option does not prevent groups_get_activity_group from * returning 0; it will still do that if the user has chosen 'all participants' * in another activity, or not chosen anything.) + * @param bool $participationonly By default, this menu will only contain groups with the "participation" + * flag set true. Setting this argument to false will return all groups that the user is allowed to see. + * This should only be used for cases such as a teacher wanting to filter submissions by group, not for + * students choosing a group to submit their work under, otherwise it negates the point of the participation + * flag. * @return mixed void or string depending on $return param */ -function groups_print_activity_menu($cm, $urlroot, $return=false, $hideallparticipants=false) { +function groups_print_activity_menu($cm, $urlroot, $return=false, $hideallparticipants=false, bool $participationonly = true) { global $USER, $OUTPUT; if ($urlroot instanceof moodle_url) { @@ -966,22 +1010,23 @@ 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, 'g.*', false, true); // Any group in grouping. + // Any group in grouping. + $allowedgroups = groups_get_all_groups($cm->course, 0, $cm->groupingid, 'g.*', false, $participationonly); // Get user's own groups and put to the top. - $usergroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, true); + $usergroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, $participationonly); } else { // Only assigned groups. - $allowedgroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, true); + $allowedgroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, $participationonly); } - $activegroup = groups_get_activity_group($cm, true, $allowedgroups); + $activegroup = groups_get_activity_group($cm, true, $allowedgroups, $participationonly); $groupsmenu = array(); if ((!$allowedgroups or $groupmode == VISIBLEGROUPS or $aag) and !$hideallparticipants) { $groupsmenu[0] = get_string('allparticipants'); } - $groupsmenu += groups_sort_menu_options($allowedgroups, $usergroups); + $groupsmenu += groups_sort_menu_options($allowedgroups, $usergroups, !$participationonly); if ($groupmode == VISIBLEGROUPS) { $grouplabel = get_string('groupsvisible'); @@ -1068,13 +1113,16 @@ function groups_get_course_group($course, $update=false, $allowedgroups=null) { /** * Returns group active in activity, changes the group by default if 'group' page param present * - * @category group * @param stdClass|cm_info $cm course module object * @param bool $update change active group if group param submitted * @param array $allowedgroups list of groups user may access (INTERNAL, to be used only from groups_print_activity_menu()) + * @param bool $participationonly By default, only allow groups with the "participation" + * flag set true. Setting this argument to false will allow setting a non-participation group as the active group. + * This should be used with care, and only for users who should be able to see non-participation groups within an activity. * @return mixed false if groups not used, int if groups used, 0 means all groups (access must be verified in SEPARATE mode) + * @category group */ -function groups_get_activity_group($cm, $update=false, $allowedgroups=null) { +function groups_get_activity_group($cm, $update=false, $allowedgroups=null, bool $participationonly = true) { global $USER, $SESSION; if (!$groupmode = groups_get_activity_groupmode($cm)) { @@ -1089,9 +1137,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, 'g.*', false, true); + $allowedgroups = groups_get_all_groups($cm->course, 0, $cm->groupingid, 'g.*', false, $participationonly); } else { - $allowedgroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, true); + $allowedgroups = groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.*', false, $participationonly); } } diff --git a/lib/tests/grouplib_test.php b/lib/tests/grouplib_test.php index b801ab986e6e0..5b70117a98253 100644 --- a/lib/tests/grouplib_test.php +++ b/lib/tests/grouplib_test.php @@ -1236,14 +1236,18 @@ public function test_groups_get_user_groups() { /** * Create dummy groups array for use in menu tests * @param int $number + * @param bool $alternateparticipation If true, set participation = 1 for even groups, and 0 for odd groups. * @return array */ - protected function make_group_list($number) { + protected function make_group_list($number, $alternateparticipation = false) { $testgroups = array(); for ($a = 0; $a < $number; $a++) { $grp = new \stdClass(); $grp->id = 100 + $a; $grp->name = 'test group ' . $grp->id; + if ($alternateparticipation) { + $grp->participation = ($a % 2 == 0); + } $testgroups[$grp->id] = $grp; } return $testgroups; @@ -1302,6 +1306,82 @@ public function test_groups_sort_menu_options_user_both_many_groups() { ), groups_sort_menu_options($this->make_group_list(13), $this->make_group_list(2))); } + /** + * Splitting allowed groups by participation returns an optgroup for non-participation groups. + * + * @covers ::groups_sort_menu_options() + * @return void + */ + public function test_groups_split_participation_allowed_groups_only(): void { + $this->assertEquals( + [ + 100 => 'test group 100', + 3 => [ + get_string('nonparticipation', 'group') => [ + 101 => 'test group 101', + ], + ], + ], + groups_sort_menu_options($this->make_group_list(2, true), [], true), + ); + } + + /** + * Splitting user groups by participation returns an optgroup for non-participation groups. + * + * @covers ::groups_sort_menu_options() + * @return void + */ + public function test_groups_split_participation_options_user_groups_only(): void { + $this->assertEquals( + [ + 100 => 'test group 100', + 3 => [ + get_string('nonparticipation', 'group') => [ + 101 => 'test group 101', + ], + ], + ], + groups_sort_menu_options([], $this->make_group_list(2, true), true), + ); + } + + /** + * Splitting allowed and user groups by participation returns optgroups. + * + * One optgroup for user participation groups, one for other participation groups, and one for non-participation groups. + * + * @covers ::groups_sort_menu_options() + * @return void + */ + public function test_groups_split_participation_options_user_both(): void { + $this->assertEquals( + [ + 1 => [ + get_string('mygroups', 'group') => [ + 100 => 'test group 100', + ], + ], + 2 => [ + get_string('othergroups', 'group') => [ + 102 => 'test group 102', + ], + ], + 3 => [ + get_string('nonparticipation', 'group') => [ + 101 => 'test group 101', + 103 => 'test group 103', + ], + ], + ], + groups_sort_menu_options( + $this->make_group_list(4, true), + $this->make_group_list(2, true), + true + ), + ); + } + /** * Tests for groups_user_groups_visible. */