MDL-69541 mod_forum: Post a copy to all groups in quick discussion

When sitting behind the 'Advanced' link in discussion creation, the 'Post a copy to all groups' option is not very much visible
for a discussion creator. There were many proposals within the community to bring this option to the quick discussion creation
form.

The current improvement brings the 'Post a copy to all groups' option back to the quick discussion creation form.

A behat tests was added to cover the feature's behavior.
This commit is contained in:
vtos
2023-12-10 13:36:30 +01:00
parent a891866cbd
commit b3bd8daeb1
2 changed files with 154 additions and 128 deletions
+95 -77
View File
@@ -1,5 +1,4 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
@@ -84,8 +83,6 @@ class mod_forum_post_form extends moodleform {
$mform =& $this->_form;
$course = $this->_customdata['course'];
$cm = $this->_customdata['cm'];
$coursecontext = $this->_customdata['coursecontext'];
$modcontext = $this->_customdata['modcontext'];
$forum = $this->_customdata['forum'];
@@ -120,7 +117,9 @@ class mod_forum_post_form extends moodleform {
$mform->setType('message', PARAM_RAW);
$mform->addRule('message', get_string('required'), 'required', null, 'client');
if (!$inpagereply) {
if ($inpagereply) {
$this->add_group_elements_if_applicable($mform);
} else {
$manageactivities = has_capability('moodle/course:manageactivities', $coursecontext);
if (\mod_forum\subscriptions::is_forcesubscribed($forum)) {
@@ -168,79 +167,7 @@ class mod_forum_post_form extends moodleform {
}
}
if ($groupmode = groups_get_activity_groupmode($cm, $course)) {
$groupdata = groups_get_activity_allowed_groups($cm);
$groupinfo = array();
foreach ($groupdata as $groupid => $group) {
// Check whether this user can post in this group.
// We must make this check because all groups are returned for a visible grouped activity.
if (forum_user_can_post_discussion($forum, $groupid, null, $cm, $modcontext)) {
// Build the data for the groupinfo select.
$groupinfo[$groupid] = format_string($group->name, true, ['context' => $modcontext]);
} else {
unset($groupdata[$groupid]);
}
}
$groupcount = count($groupinfo);
// Check whether a user can post to all of their own groups.
// Posts to all of my groups are copied to each group that the user is a member of. Certain conditions must be met.
// 1) It only makes sense to allow this when a user is in more than one group.
// Note: This check must come before we consider adding accessallgroups, because that is not a real group.
$canposttoowngroups = empty($post->edit) && $groupcount > 1;
// 2) Important: You can *only* post to multiple groups for a top level post. Never any reply.
$canposttoowngroups = $canposttoowngroups && empty($post->parent);
// 3) You also need the canposttoowngroups capability.
$canposttoowngroups = $canposttoowngroups && has_capability('mod/forum:canposttomygroups', $modcontext);
if ($canposttoowngroups) {
// This user is in multiple groups, and can post to all of their own groups.
// Note: This is not the same as accessallgroups. This option will copy a post to all groups that a
// user is a member of.
$mform->addElement('checkbox', 'posttomygroups', get_string('posttomygroups', 'forum'));
$mform->addHelpButton('posttomygroups', 'posttomygroups', 'forum');
$mform->disabledIf('groupinfo', 'posttomygroups', 'checked');
}
// Check whether this user can post to all groups.
// Posts to the 'All participants' group go to all groups, not to each group in a list.
// It makes sense to allow this, even if there currently aren't any groups because there may be in the future.
if (forum_user_can_post_discussion($forum, -1, null, $cm, $modcontext)) {
// Note: We must reverse in this manner because array_unshift renumbers the array.
$groupinfo = array_reverse($groupinfo, true);
$groupinfo[-1] = get_string('allparticipants');
$groupinfo = array_reverse($groupinfo, true);
$groupcount++;
}
// Determine whether the user can select a group from the dropdown. The dropdown is available for several reasons.
// 1) This is a new post (not an edit), and there are at least two groups to choose from.
$canselectgroupfornew = empty($post->edit) && $groupcount > 1;
// 2) This is editing of an existing post and the user is allowed to movediscussions.
// We allow this because the post may have been moved from another forum where groups are not available.
// We show this even if no groups are available as groups *may* have been available but now are not.
$canselectgroupformove =
$groupcount && !empty($post->edit) && has_capability('mod/forum:movediscussions', $modcontext);
// Important: You can *only* change the group for a top level post. Never any reply.
$canselectgroup = empty($post->parent) && ($canselectgroupfornew || $canselectgroupformove);
if ($canselectgroup) {
$mform->addElement('select', 'groupinfo', get_string('group'), $groupinfo);
$mform->setDefault('groupinfo', $post->groupid);
$mform->setType('groupinfo', PARAM_INT);
} else {
if (empty($post->groupid)) {
$groupname = get_string('allparticipants');
} else {
$groupname = format_string($groupdata[$post->groupid]->name);
}
$mform->addElement('static', 'groupinfo', get_string('group'), $groupname);
}
}
$this->add_group_elements_if_applicable($mform);
if (!empty($CFG->forum_enabletimedposts) && !$post->parent &&
has_capability('mod/forum:viewhiddentimedposts', $coursecontext)) {
@@ -343,4 +270,95 @@ class mod_forum_post_form extends moodleform {
}
return $errors;
}
/**
* Adds elements to control what groups will have access to the post or exits when no relevant group mode is set.
*
* @param MoodleQuickForm $mform
*/
private function add_group_elements_if_applicable(MoodleQuickForm $mform): void {
global $CFG;
$course = $this->_customdata['course'];
$cm = $this->_customdata['cm'];
$forum = $this->_customdata['forum'];
$modcontext = $this->_customdata['modcontext'];
$post = $this->_customdata['post'];
if (!groups_get_activity_groupmode($cm, $course)) {
return;
}
$groupdata = groups_get_activity_allowed_groups($cm);
$groupinfo = [];
foreach ($groupdata as $groupid => $group) {
// Check whether this user can post in this group.
// We must make this check because all groups are returned for a visible grouped activity.
if (forum_user_can_post_discussion($forum, $groupid, null, $cm, $modcontext)) {
// Build the data for the groupinfo select.
$groupinfo[$groupid] = format_string($group->name, true, ['context' => $modcontext]);
} else {
unset($groupdata[$groupid]);
}
}
$groupcount = count($groupinfo);
// Check whether a user can post to all of their own groups.
// Posts to all of my groups are copied to each group that the user is a member of. Certain conditions must be met.
// 1) It only makes sense to allow this when a user is in more than one group.
// Note: This check must come before we consider adding accessallgroups, because that is not a real group.
$canposttoowngroups = empty($post->edit) && $groupcount > 1;
// 2) Important: You can *only* post to multiple groups for a top level post. Never any reply.
$canposttoowngroups = $canposttoowngroups && empty($post->parent);
// 3) You also need the canposttoowngroups capability.
$canposttoowngroups = $canposttoowngroups && has_capability('mod/forum:canposttomygroups', $modcontext);
if ($canposttoowngroups) {
// This user is in multiple groups, and can post to all of their own groups.
// Note: This is not the same as accessallgroups. This option will copy a post to all groups that a
// user is a member of.
$mform->addElement('checkbox', 'posttomygroups', get_string('posttomygroups', 'forum'));
$mform->addHelpButton('posttomygroups', 'posttomygroups', 'forum');
$mform->disabledIf('groupinfo', 'posttomygroups', 'checked');
}
// Check whether this user can post to all groups.
// Posts to the 'All participants' group go to all groups, not to each group in a list.
// It makes sense to allow this, even if there currently aren't any groups because there may be in the future.
if (forum_user_can_post_discussion($forum, -1, null, $cm, $modcontext)) {
// Note: We must reverse in this manner because array_unshift renumbers the array.
$groupinfo = array_reverse($groupinfo, true);
$groupinfo[-1] = get_string('allparticipants');
$groupinfo = array_reverse($groupinfo, true);
$groupcount++;
}
// Determine whether the user can select a group from the dropdown. The dropdown is available for several reasons.
// 1) This is a new post (not an edit), and there are at least two groups to choose from.
$canselectgroupfornew = empty($post->edit) && $groupcount > 1;
// 2) This is editing of an existing post and the user is allowed to movediscussions.
// We allow this because the post may have been moved from another forum where groups are not available.
// We show this even if no groups are available as groups *may* have been available but now are not.
$canselectgroupformove =
$groupcount && !empty($post->edit) && has_capability('mod/forum:movediscussions', $modcontext);
// Important: You can *only* change the group for a top level post. Never any reply.
$canselectgroup = empty($post->parent) && ($canselectgroupfornew || $canselectgroupformove);
if ($canselectgroup) {
$mform->addElement('select', 'groupinfo', get_string('group'), $groupinfo);
$mform->setDefault('groupinfo', $post->groupid);
$mform->setType('groupinfo', PARAM_INT);
} else {
if (empty($post->groupid)) {
$groupname = get_string('allparticipants');
} else {
$groupname = format_string($groupdata[$post->groupid]->name);
}
$mform->addElement('static', 'groupinfo', get_string('group'), $groupname);
}
}
}
@@ -1,66 +1,66 @@
@mod @mod_forum
Feature: A user with access to multiple groups should be able to post a copy of a message to all the groups they have access to
In order to post to all groups a user has access to
As a user
I need to have the option to post a copy of a message to all groups
In order to post to all groups a user has access to
As a user
I need to have the option to post a copy of a message to all groups
Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
| student1 | Student | 1 | student1@example.com |
| student2 | Student | 2 | student2@example.com |
| student3 | Student | 3 | student3@example.com |
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
| student1 | Student | 1 | student1@example.com |
| student2 | Student | 2 | student2@example.com |
| student3 | Student | 3 | student3@example.com |
And the following "courses" exist:
| fullname | shortname | category |
| Course 1 | C1 | 0 |
| Course 2 | C2 | 0 |
| Course 1 | C1 | 0 |
| Course 2 | C2 | 0 |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
| teacher1 | C2 | editingteacher |
| student1 | C1 | student |
| student1 | C2 | student |
| student2 | C1 | student |
| student2 | C2 | student |
| student3 | C1 | student |
| student3 | C2 | student |
| user | course | role |
| teacher1 | C1 | editingteacher |
| teacher1 | C2 | editingteacher |
| student1 | C1 | student |
| student1 | C2 | student |
| student2 | C1 | student |
| student2 | C2 | student |
| student3 | C1 | student |
| student3 | C2 | student |
And the following "groups" exist:
| name | course | idnumber |
| Group A | C1 | C1G1 |
| Group B | C1 | C1G2 |
| Group C | C1 | C1G3 |
| Group A | C2 | C2G1 |
| Group B | C2 | C2G2 |
| Group C | C2 | C2G3 |
| name | course | idnumber |
| Group A | C1 | C1G1 |
| Group B | C1 | C1G2 |
| Group C | C1 | C1G3 |
| Group A | C2 | C2G1 |
| Group B | C2 | C2G2 |
| Group C | C2 | C2G3 |
And the following "groupings" exist:
| name | course | idnumber |
| G1 | C2 | G1 |
| G1 | C2 | G1 |
And the following "group members" exist:
| user | group |
| teacher1 | C1G1 |
| teacher1 | C1G2 |
| teacher1 | C1G3 |
| teacher1 | C2G1 |
| teacher1 | C2G1 |
| student1 | C1G1 |
| student1 | C2G1 |
| student1 | C2G2 |
| student2 | C1G1 |
| student2 | C1G2 |
| student3 | C1G1 |
| student3 | C1G2 |
| student3 | C1G3 |
| user | group |
| teacher1 | C1G1 |
| teacher1 | C1G2 |
| teacher1 | C1G3 |
| teacher1 | C2G1 |
| teacher1 | C2G1 |
| student1 | C1G1 |
| student1 | C2G1 |
| student1 | C2G2 |
| student2 | C1G1 |
| student2 | C1G2 |
| student3 | C1G1 |
| student3 | C1G2 |
| student3 | C1G3 |
And the following "grouping groups" exist:
| grouping | group |
| G1 | C2G1 |
| G1 | C2G2 |
| G1 | C2G1 |
| G1 | C2G2 |
And the following "activities" exist:
| activity | name | course | idnumber | groupmode | grouping |
| forum | No group forum | C1 | forum | 0 | |
| forum | Separate group forum | C1 | forum | 1 | |
| forum | Visible group forum | C1 | forum | 2 | |
| forum | Groupings forum | C2 | forum | 1 | G1 |
| activity | name | course | idnumber | groupmode | grouping |
| forum | No group forum | C1 | forum | 0 | |
| forum | Separate group forum | C1 | forum | 1 | |
| forum | Visible group forum | C1 | forum | 2 | |
| forum | Groupings forum | C2 | forum | 1 | G1 |
And the following forum discussions exist in course "Course 1":
| user | forum | name | message | posttomygroups |
| teacher1 | Separate group forum | Discussion 1 | test | 1 |
@@ -91,10 +91,18 @@ Feature: A user with access to multiple groups should be able to post a copy of
Given I log in as "teacher1"
And I am on "Course 2" course homepage
And I add a new discussion to "Groupings forum" forum with:
| Subject | Discussion 1 |
| Message | test |
| Post a copy to all groups | 1 |
| Subject | Discussion 1 |
| Message | test |
| Post a copy to all groups | 1 |
When I am on the "Groupings forum" "forum activity" page logged in as student1
Then I should see "Discussion 1"
And I am on the "Groupings forum" "forum activity" page logged in as student2
And I should not see "Discussion 1"
@javascript @mod_forum_quick_discussion_post_to_all_groups
Scenario: The 'Post a copy to all groups' option can be available in the quick discussion creation form
Given I log in as "teacher1"
And I am on "Course 1" course homepage
And I follow "Separate group forum"
When I click on "Add discussion topic" "link"
Then "Post a copy to all groups" "field" should exist