Merge branch 'MDL-50714-master' of git://github.com/andrewnicols/moodle

mod/forum/classes/post_form.php

@@ -159,31 +159,67 @@ function definition() {
             $mform->setConstants(array('timestart'=> 0, 'timeend'=>0));
         }
 
-        if ($groupmode = groups_get_activity_groupmode($cm, $course)) { // hack alert
+        if ($groupmode = groups_get_activity_groupmode($cm, $course)) {
             $groupdata = groups_get_activity_allowed_groups($cm);
-            $groupcount = count($groupdata);
+
             $groupinfo = array();
-            $modulecontext = context_module::instance($cm->id);
+            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] = $group->name;
+                } 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 the user has access to all groups in this forum from the accessallgroups cap.
-            if ($groupmode == VISIBLEGROUPS || has_capability('moodle/site:accessallgroups', $modulecontext)) {
-                // Only allow posting to all groups if the user has access to all groups.
-                $groupinfo = array('0' => get_string('allparticipants'));
+            // 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++;
             }
 
-            $contextcheck = has_capability('mod/forum:movediscussions', $modulecontext) && empty($post->parent) && $groupcount > 1;
-            if ($contextcheck) {
-                if (has_capability('mod/forum:canposttomygroups', $modulecontext)
-                            && !isset($post->edit)) {
-                    $mform->addElement('checkbox', 'posttomygroups', get_string('posttomygroups', 'forum'));
-                    $mform->addHelpButton('posttomygroups', 'posttomygroups', 'forum');
-                    $mform->disabledIf('groupinfo', 'posttomygroups', 'checked');
-                }
+            // 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;
 
-                foreach ($groupdata as $grouptemp) {
-                    $groupinfo[$grouptemp->id] = $grouptemp->name;
-                }
+            // 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);

mod/forum/post.php

@@ -697,8 +697,6 @@
     // WARNING: the $fromform->message array has been overwritten, do not use it anymore!
     $fromform->messagetrust  = trusttext_trusted($modcontext);
 
-    $contextcheck = isset($fromform->groupinfo) && has_capability('mod/forum:movediscussions', $modcontext);
-
     if ($fromform->edit) {           // Updating a post
         unset($fromform->groupid);
         $fromform->id = $fromform->edit;
@@ -722,10 +720,15 @@
         }
 
         // If the user has access to all groups and they are changing the group, then update the post.
-        if ($contextcheck) {
+        if (isset($fromform->groupinfo) && has_capability('mod/forum:movediscussions', $modcontext)) {
             if (empty($fromform->groupinfo)) {
                 $fromform->groupinfo = -1;
             }
+
+            if (!forum_user_can_post_discussion($forum, $fromform->groupinfo, null, $cm, $modcontext)) {
+                print_error('cannotupdatepost', 'forum');
+            }
+
             $DB->set_field('forum_discussions' ,'groupid' , $fromform->groupinfo, array('firstpost' => $fromform->id));
         }
 
@@ -853,6 +856,9 @@
         exit;
 
     } else { // Adding a new discussion.
+        // The location to redirect to after successfully posting.
+        $redirectto = new moodle_url('view.php', array('f' => $fromform->forum));
+
         $fromform->mailnow = empty($fromform->mailnow) ? 0 : 1;
 
         $discussion = $fromform;
@@ -870,17 +876,28 @@
 
         // If we are posting a copy to all groups the user has access to.
         if (isset($fromform->posttomygroups)) {
+            // Post to each of my groups.
             require_capability('mod/forum:canposttomygroups', $modcontext);
+
+            // Fetch all of this user's groups.
+            // Note: all groups are returned when in visible groups mode so we must manually filter.
             $allowedgroups = groups_get_activity_allowed_groups($cm);
-            $groupstopostto = array_keys($allowedgroups);
-        } else {
-            if ($contextcheck) {
-                $fromform->groupid = $fromform->groupinfo;
-            }
-            if (empty($fromform->groupid)) {
-                $fromform->groupid = -1;
+            foreach ($allowedgroups as $groupid => $group) {
+                if (forum_user_can_post_discussion($forum, $groupid, -1, $cm, $modcontext)) {
+                    $groupstopostto[] = $groupid;
+                }
             }
-            $groupstopostto = array($fromform->groupid);
+        } else if (isset($fromform->groupinfo)) {
+            // Use the value provided in the dropdown group selection.
+            $groupstopostto[] = $fromform->groupinfo;
+            $redirectto->param('group', $fromform->groupinfo);
+        } else if (isset($fromform->groupid) && !empty($fromform->groupid)) {
+            // Use the value provided in the hidden form element instead.
+            $groupstopostto[] = $fromform->groupid;
+            $redirectto->param('group', $fromform->groupid);
+        } else {
+            // Use the value for all participants instead.
+            $groupstopostto[] = -1;
         }
 
         // Before we post this we must check that the user will not exceed the blocking threshold.
@@ -934,9 +951,8 @@
             $completion->update_state($cm, COMPLETION_COMPLETE);
         }
 
-        redirect(forum_go_back_to("view.php?f=$fromform->forum"), $message.$subscribemessage, $timemessage);
-
-        exit;
+        // Redirect back to the discussion.
+        redirect(forum_go_back_to($redirectto->out()), $message . $subscribemessage, $timemessage);
     }
 }
 

mod/forum/tests/behat/groups_in_course_no_groups_in_forum.feature

@@ -0,0 +1,62 @@
+@mod @mod_forum
+Feature: Forums in 'No groups' mode allow posting to All participants for all users
+  In order to post to a forum in 'No groups' mode, which is in course which has groups
+  As any user
+  I need to post
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@example.com |
+      | student1 | Student | 1 | student1@example.com |
+    And the following "courses" exist:
+      | fullname | shortname | category |
+      | Course 1 | C1 | 0 |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | student1 | C1 | student |
+    And the following "groups" exist:
+      | name | course | idnumber |
+      | Group A | C1 | G1 |
+      | Group B | C1 | G2 |
+    And the following "group members" exist:
+      | user | group |
+      | teacher1 | G1 |
+      | teacher1 | G2 |
+      | student1 | G1 |
+    And the following "activities" exist:
+      | activity   | name                   | intro                         | course | idnumber     | groupmode |
+      | forum      | Standard forum name    | Standard forum description    | C1     | nogroups     | 0         |
+
+  Scenario: Teacher can post
+    Given I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Standard forum name"
+    And I should not see "Group A"
+    And I should not see "Group B"
+    When I click on "Add a new discussion topic" "button"
+    Then I should not see "Post a copy to all groups"
+    And I should not see "Group" in the "form" "css_element"
+    And I set the following fields to these values:
+      | Subject | Teacher 1 -> Forum  |
+      | Message | Teacher 1 -> Forum  |
+    And I press "Post to forum"
+    And I wait to be redirected
+    And I should see "Teacher 1 -> Forum"
+
+  Scenario: Student can post
+    Given I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Standard forum name"
+    And I should not see "Group A"
+    And I should not see "Group B"
+    When I click on "Add a new discussion topic" "button"
+    Then I should not see "Post a copy to all groups"
+    And I should not see "Group" in the "form" "css_element"
+    And I set the following fields to these values:
+      | Subject | Student 1 -> Forum  |
+      | Message | Student 1 -> Forum  |
+    And I press "Post to forum"
+    And I wait to be redirected
+    And I should see "Student 1 -> Forum"

mod/forum/tests/behat/no_groups_in_course.feature

@@ -0,0 +1,86 @@
+@mod @mod_forum
+Feature: Posting to forums in a course with no groups behaves correctly
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@example.com |
+      | student1 | Student | 1 | student1@example.com |
+    And the following "courses" exist:
+      | fullname | shortname | category |
+      | Course 1 | C1 | 0 |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | student1 | C1 | student |
+    And the following "activities" exist:
+      | activity   | name                   | intro                         | course | idnumber     | groupmode |
+      | forum      | Standard forum         | Standard forum description    | C1     | nogroups     | 0         |
+      | forum      | Visible forum          | Visible forum description     | C1     | visgroups    | 2         |
+      | forum      | Separate forum         | Separate forum description    | C1     | sepgroups    | 1         |
+
+  Scenario: Teachers can post in standard forum
+    Given I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Standard forum"
+    When I click on "Add a new discussion topic" "button"
+    Then I should not see "Post a copy to all groups"
+    And I set the following fields to these values:
+      | Subject | Teacher -> All participants |
+      | Message | Teacher -> All participants |
+    And I press "Post to forum"
+    And I wait to be redirected
+    And I should see "Teacher -> All participants"
+
+  Scenario: Teachers can post in forum with separate groups
+    Given I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Separate forum"
+    When I click on "Add a new discussion topic" "button"
+    Then I should not see "Post a copy to all groups"
+    And I set the following fields to these values:
+      | Subject | Teacher -> All participants |
+      | Message | Teacher -> All participants |
+    And I press "Post to forum"
+    And I wait to be redirected
+    And I should see "Teacher -> All participants"
+
+  Scenario: Teachers can post in forum with visible groups
+    Given I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Visible forum"
+    When I click on "Add a new discussion topic" "button"
+    Then I should not see "Post a copy to all groups"
+    And I set the following fields to these values:
+      | Subject | Teacher -> All participants |
+      | Message | Teacher -> All participants |
+    And I press "Post to forum"
+    And I wait to be redirected
+    And I should see "Teacher -> All participants"
+
+  Scenario: Students can post in standard forum
+    Given I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Standard forum"
+    When I click on "Add a new discussion topic" "button"
+    Then I should not see "Post a copy to all groups"
+    And I set the following fields to these values:
+      | Subject | Student -> All participants |
+      | Message | Student -> All participants |
+    And I press "Post to forum"
+    And I wait to be redirected
+    And I should see "Student -> All participants"
+
+  Scenario: Students cannot post in forum with separate groups
+    Given I log in as "student1"
+    And I follow "Course 1"
+    When I follow "Separate forum"
+    Then I should see "You do not have permission to add a new discussion topic for all participants."
+    And I should not see "Add a new discussion topic"
+
+  Scenario: Teachers can post in forum with visible groups
+    Given I log in as "student1"
+    And I follow "Course 1"
+    When I follow "Visible forum"
+    Then I should see "You do not have permission to add a new discussion topic for all participants."
+    And I should not see "Add a new discussion topic"