From b3bd8daeb18d760c4c0f6aacfc403fb861f54946 Mon Sep 17 00:00:00 2001 From: vtos Date: Fri, 9 Jun 2023 17:16:16 +0200 Subject: [PATCH] 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. --- mod/forum/classes/post_form.php | 172 ++++++++++-------- .../behat/post_to_multiple_groups.feature | 110 +++++------ 2 files changed, 154 insertions(+), 128 deletions(-) diff --git a/mod/forum/classes/post_form.php b/mod/forum/classes/post_form.php index ea1e07b406e..4db30310903 100644 --- a/mod/forum/classes/post_form.php +++ b/mod/forum/classes/post_form.php @@ -1,5 +1,4 @@ _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); + } + } } diff --git a/mod/forum/tests/behat/post_to_multiple_groups.feature b/mod/forum/tests/behat/post_to_multiple_groups.feature index b191a6f4b66..1a53afcff9d 100644 --- a/mod/forum/tests/behat/post_to_multiple_groups.feature +++ b/mod/forum/tests/behat/post_to_multiple_groups.feature @@ -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