Skip to content

Commit

Permalink
MDL-81514 groups: Add participationonly option to activity group menu
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
marxjohnson committed May 21, 2024
1 parent f49d120 commit 93aceed
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 20 deletions.
7 changes: 7 additions & 0 deletions group/upgrade.txt
Original file line number Diff line number Diff line change
@@ -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`
Expand Down
1 change: 1 addition & 0 deletions lang/en/group.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
86 changes: 67 additions & 19 deletions lib/grouplib.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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];
}

/**
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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)) {
Expand All @@ -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);
}
}

Expand Down
82 changes: 81 additions & 1 deletion lib/tests/grouplib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down

0 comments on commit 93aceed

Please sign in to comment.