MDL-13936 forum reply refactoring and bugfixing - the forum_user_can_post() did not...

diff --git a/mod/forum/discuss.php b/mod/forum/discuss.php
index f7702fcf20d24112f598c4e7f475ca866c7ed5cd..79b98ea0cdaa4c7193ce3e6cdc350d67e4fa2511 100644 (file)
--- a/mod/forum/discuss.php
+++ b/mod/forum/discuss.php
@@ -144,47 +144,14 @@
 
 /// Check to see if groups are being used in this forum
 /// If so, make sure the current person is allowed to see this discussion
-/// Also, if we know they should be able to reply, then explicitly set $canreply
+/// Also, if we know they should be able to reply, then explicitly set $canreply for performance reasons
 
-    if ($forum->type == 'news') {
-        $capname = 'mod/forum:replynews';
-    } else {
-        $capname = 'mod/forum:replypost';
-    }
-
-    $canreply = false;
     if (isguestuser() or !isloggedin() or has_capability('moodle/legacy:guest', $modcontext, NULL, false)) {
         // allow guests and not-logged-in to see the link - they are prompted to log in after clicking the link
         $canreply = ($forum->type != 'news'); // no reply in news forums
 
-    } else if (has_capability($capname, $modcontext)) {
-        $groupmode = groups_get_activity_groupmode($cm);
-        if ($groupmode) {
-            if (has_capability('moodle/site:accessallgroups', $modcontext)) {
-                $canreply = true;
-            } else {
-                if ($groupmode == SEPARATEGROUPS) {
-                    require_login();
-                    if ($discussion->groupid == -1) {
-                        // can not reply to discussions for "All participants" in separate mode without accessallgroups cap
-                    } else if (groups_is_member($discussion->groupid)) {
-                        $canreply = true;
-                    } else {
-                        // this should not happen
-                        print_heading("Sorry, you can't see this discussion because you are not in this group");
-                        print_footer($course);
-                        die;
-                    }
-
-                } else if ($groupmode == VISIBLEGROUPS) {
-                    if ($discussion->groupid == -1 or groups_is_member($discussion->groupid)) {
-                        $canreply = true;
-                    }
-                }
-            }
-        } else {
-            $canreply = true;
-        }
+    } else {
+        $canreply = forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext);
     }
 
 /// Print the controls across the top

diff --git a/mod/forum/lib.php b/mod/forum/lib.php
index 9688618238d7a83060c7334bfe347b30803b28df..6c3036b31c452b01a611a21d8e62d161b62f099f 100644 (file)
--- a/mod/forum/lib.php
+++ b/mod/forum/lib.php
@@ -372,9 +372,9 @@ function forum_cron() {
                     $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
                     $userto->viewfullnames[$forum->id] = has_capability('moodle/site:viewfullnames', $modcontext);
                 }
-                if (!isset($userto->canpost[$forum->id])) {
+                if (!isset($userto->canpost[$discussion->id])) {
                     $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
-                    $userto->canpost[$forum->id] = forum_user_can_post($forum, $userto, $cm, $modcontext);
+                    $userto->canpost[$discussion->id] = forum_user_can_post($forum, $discussion, $userto, $cm, $course, $modcontext);
                 }
                 if (!isset($userfrom->groups[$forum->id])) {
                     if (!isset($userfrom->groups)) {
@@ -640,14 +640,14 @@ function forum_cron() {
                         $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
                         $userto->viewfullnames[$forum->id] = has_capability('moodle/site:viewfullnames', $modcontext);
                     }
-                    if (!isset($userto->canpost[$forum->id])) {
+                    if (!isset($userto->canpost[$discussion->id])) {
                         $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
-                        $userto->canpost[$forum->id] = forum_user_can_post($forum, $userto, $cm, $modcontext);
+                        $userto->canpost[$discussion->id] = forum_user_can_post($forum, $discussion, $userto, $cm, $course, $modcontext);
                     }
 
                     $strforums      = get_string('forums', 'forum');
                     $canunsubscribe = ! forum_is_forcesubscribed($forum);
-                    $canreply       = $userto->canpost[$forum->id];
+                    $canreply       = $userto->canpost[$discussion->id];
 
                     $posttext .= "\n \n";
                     $posttext .= '=====================================================================';
@@ -798,10 +798,11 @@ function forum_make_mail_text($course, $forum, $discussion, $post, $userfrom, $u
         $viewfullnames = $userto->viewfullnames[$forum->id];
     }
 
-    if (!isset($userto->canpost[$forum->id])) {
-        $canreply = forum_user_can_post($forum, $userto);
+    if (!isset($userto->canpost[$discussion->id])) {
+        $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
+        $canreply = forum_user_can_post($forum, $discussion, $userto, $cm, $course, $modcontext);
     } else {
-        $canreply = $userto->canpost[$forum->id];
+        $canreply = $userto->canpost[$discussion->id];
     }
 
     $by = New stdClass;
@@ -870,10 +871,10 @@ function forum_make_mail_html($course, $forum, $discussion, $post, $userfrom, $u
         return '';
     }
 
-    if (!isset($userto->canpost[$forum->id])) {
-        $canreply = forum_user_can_post($forum, $userto);
+    if (!isset($userto->canpost[$discussion->id])) {
+        $canreply = forum_user_can_post($forum, $discussion, $userto);
     } else {
-        $canreply = $userto->canpost[$forum->id];
+        $canreply = $userto->canpost[$discussion->id];
     }
 
     $strforums = get_string('forums', 'forum');
@@ -4262,7 +4263,7 @@ function forum_user_can_post_discussion($forum, $currentgroup=null, $unused=-1,
  * @param $forum - forum object
  * @param $user - user object
  */
-function forum_user_can_post($forum, $user=NULL, $cm=NULL, $context=NULL) {
+function forum_user_can_post($forum, $discussion, $user=NULL, $cm=NULL, $course=NULL, $context=NULL) {
     global $USER;
     if (empty($user)) {
         $user = $USER;
@@ -4273,13 +4274,26 @@ function forum_user_can_post($forum, $user=NULL, $cm=NULL, $context=NULL) {
         return false;
     }
 
-    if (!$context) {
-        if (!$cm) {
-            debugging('missing cm', DEBUG_DEVELOPER);
-            if (!$cm = get_coursemodule_from_instance('forum', $forum->id, $forum->course)) {
-                error('Course Module ID was incorrect');
-            }
+    if (!isset($discussion->groupid)) {
+        debugging('incorrect discussion parameter', DEBUG_DEVELOPER);
+        return false; 
+    }
+
+    if (!$cm) {
+        debugging('missing cm', DEBUG_DEVELOPER);
+        if (!$cm = get_coursemodule_from_instance('forum', $forum->id, $forum->course)) {
+            error('Course Module ID was incorrect');
+        }
+    }
+
+    if (!$course) {
+        debugging('missing course', DEBUG_DEVELOPER);
+        if (!$course = get_record('course', 'id', $forum->course)) {
+            error('Incorrect course id');
         }
+    }
+
+    if (!$context) {
         $context = get_context_instance(CONTEXT_MODULE, $cm->id);
     }
 
@@ -4294,7 +4308,32 @@ function forum_user_can_post($forum, $user=NULL, $cm=NULL, $context=NULL) {
         $capname = 'mod/forum:replypost';
     }
 
-    return has_capability($capname, $context, $user->id, false);
+    if (!has_capability($capname, $context, $user->id, false)) {
+        return false;
+    }
+
+    if (!$groupmode = groups_get_activity_groupmode($cm, $course)) {
+        return true;
+    }
+
+    if (has_capability('moodle/site:accessallgroups', $context)) {
+        return true;
+    }
+
+    if ($groupmode == VISIBLEGROUPS) {
+        if ($discussion->groupid == -1) {
+            // allow students to reply to all participants discussions - this was not possible in Moodle <1.8
+            return true;
+        }
+        return groups_is_member($discussion->groupid);
+
+    } else {
+        //separate groups
+        if ($discussion->groupid == -1) {
+            return false;
+        }
+        return groups_is_member($discussion->groupid);
+    } 
 }
 
 
@@ -4571,7 +4610,6 @@ function forum_print_latest_discussions($course, $forum, $maxdiscussions=-1, $di
         }
     }
 
-    $canreply = forum_user_can_post($forum, null, $cm, $context);
     $canviewparticipants = has_capability('moodle/course:viewparticipants',$context);
 
     $strdatestring = get_string('strftimerecentfull');
@@ -4663,10 +4701,13 @@ function forum_print_latest_discussions($course, $forum, $maxdiscussions=-1, $di
                     $canviewparticipants, $context);
             break;
             default:
-                if ($canreply or $discussion->replies) {
+                $link = false;
+
+                if ($discussion->replies) {
                     $link = true;
                 } else {
-                    $link = false;
+                    $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
+                    $link = forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext);
                 }
 
                 $discussion->forum = $forum->id;
@@ -4711,7 +4752,8 @@ function forum_print_discussion($course, $cm, $forum, $discussion, $post, $mode,
         $ownpost = false;
     }
     if ($canreply === NULL) {
-        $reply = forum_user_can_post($forum, null, $cm);
+        $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
+        $reply = forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext);
     } else {
         $reply = $canreply;
     }

diff --git a/mod/forum/post.php b/mod/forum/post.php
index c08ccc609bbb1769660bc06cea079e252287356b..034aff7365ef799ccb9dd4e56ff42507301b5a06 100644 (file)
--- a/mod/forum/post.php
+++ b/mod/forum/post.php
@@ -142,7 +142,7 @@
         $coursecontext = get_context_instance(CONTEXT_COURSE, $course->id);
         $modcontext    = get_context_instance(CONTEXT_MODULE, $cm->id);
 
-        if (! forum_user_can_post($forum, null, $cm, $modcontext)) {
+        if (! forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext)) {
             if (has_capability('moodle/legacy:guest', $coursecontext, NULL, false)) {  // User is a guest here!
                 $SESSION->wantsurl = $FULLME;
                 $SESSION->enrolcancel = $_SERVER['HTTP_REFERER'];

diff --git a/mod/forum/view.php b/mod/forum/view.php
index 517b5d78b65b45a29529519e96a930857e9656ff..4c9d3309cea26ff61a16d621ae39841091b54134 100644 (file)
--- a/mod/forum/view.php
+++ b/mod/forum/view.php
@@ -220,16 +220,10 @@
                 set_user_preference("forum_displaymode", $mode);
             }
 
-            $groupmode = groups_get_activity_groupmode($cm, $course);
-            $canreply = NULL;
-            if ($groupmode == SEPARATEGROUPS) {
-                if (!has_capability('moodle/site:accessallgroups', $context)) {
-                    $canreply = false;
-                }
-            }
-            
+            $canreply    = forum_user_can_post($forum, $discussion, $USER, $cm, $course, $context);
+            $canrate     = has_capability('mod/forum:rate', $context);
             $displaymode = get_user_preferences("forum_displaymode", $CFG->forum_displaymode);
-            $canrate = has_capability('mod/forum:rate', $context);
+
             echo ' '; // this should fix the floating in FF
             forum_print_discussion($course, $cm, $forum, $discussion, $post, $displaymode, $canreply, $canrate);
             break;