From 4f3a2d2103e0561865d48f1503e587484b5776b9 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Fri, 2 Oct 2015 10:21:33 +0200 Subject: [PATCH] MDL-51637 forum: Make forum_get_discussions_paginated return group post --- mod/forum/externallib.php | 2 +- mod/forum/lib.php | 44 +++++++++++-- mod/forum/tests/lib_test.php | 117 +++++++++++++++++++++++++++++++++++ mod/forum/upgrade.txt | 3 + 4 files changed, 159 insertions(+), 7 deletions(-) diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index d41444d71a6..d8c6e86d13e 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -647,7 +647,7 @@ class mod_forum_external extends external_api { require_capability('mod/forum:viewdiscussion', $modcontext, null, true, 'noviewdiscussionspermission', 'forum'); $sort = 'd.' . $sortby . ' ' . $sortdirection; - $alldiscussions = forum_get_discussions($cm, $sort, true, -1, -1, true, $page, $perpage); + $alldiscussions = forum_get_discussions($cm, $sort, true, -1, -1, true, $page, $perpage, FORUM_POSTS_ALL_USER_GROUPS); if ($alldiscussions) { $canviewfullname = has_capability('moodle/site:viewfullnames', $modcontext); diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 842310af098..981b4be5642 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -64,6 +64,11 @@ if (!defined('FORUM_CRON_USER_CACHE')) { define('FORUM_CRON_USER_CACHE', 5000); } +/** + * FORUM_POSTS_ALL_USER_GROUPS - All the posts in groups where the user is enrolled. + */ +define('FORUM_POSTS_ALL_USER_GROUPS', -2); + /// STANDARD FUNCTIONS /////////////////////////////////////////////////////////// /** @@ -2595,9 +2600,12 @@ function forum_count_discussions($forum, $cm, $course) { * @param bool $userlastmodified * @param int $page * @param int $perpage + * @param int $groupid if groups enabled, get discussions for this group overriding the current group. + * Use FORUM_POSTS_ALL_USER_GROUPS for all the user groups * @return array */ -function forum_get_discussions($cm, $forumsort="d.timemodified DESC", $fullpost=true, $unused=-1, $limit=-1, $userlastmodified=false, $page=-1, $perpage=0) { +function forum_get_discussions($cm, $forumsort="d.timemodified DESC", $fullpost=true, $unused=-1, $limit=-1, + $userlastmodified=false, $page=-1, $perpage=0, $groupid = -1) { global $CFG, $DB, $USER; $timelimit = ''; @@ -2637,13 +2645,28 @@ function forum_get_discussions($cm, $forumsort="d.timemodified DESC", $fullpost= } $groupmode = groups_get_activity_groupmode($cm); - $currentgroup = groups_get_activity_group($cm); if ($groupmode) { + if (empty($modcontext)) { $modcontext = context_module::instance($cm->id); } + // Special case, we received a groupid to override currentgroup. + if ($groupid > 0) { + $course = get_course($cm->course); + if (!groups_group_visible($groupid, $course, $cm)) { + // User doesn't belong to this group, return nothing. + return array(); + } + $currentgroup = $groupid; + } else if ($groupid === -1) { + $currentgroup = groups_get_activity_group($cm); + } else { + // Get discussions for all groups current user can see. + $currentgroup = null; + } + if ($groupmode == VISIBLEGROUPS or has_capability('moodle/site:accessallgroups', $modcontext)) { if ($currentgroup) { $groupselect = "AND (d.groupid = ? OR d.groupid = -1)"; @@ -2653,8 +2676,19 @@ function forum_get_discussions($cm, $forumsort="d.timemodified DESC", $fullpost= } } else { - //seprate groups without access all - if ($currentgroup) { + // Separate groups. + + // Get discussions for all groups current user can see. + if ($currentgroup === null) { + $mygroups = array_keys(groups_get_all_groups($cm->course, $USER->id, $cm->groupingid, 'g.id')); + if (empty($mygroups)) { + $groupselect = "AND d.groupid = -1"; + } else { + list($insqlgroups, $inparamsgroups) = $DB->get_in_or_equal($mygroups); + $groupselect = "AND (d.groupid = -1 OR d.groupid $insqlgroups)"; + $params = array_merge($params, $inparamsgroups); + } + } else if ($currentgroup) { $groupselect = "AND (d.groupid = ? OR d.groupid = -1)"; $params[] = $currentgroup; } else { @@ -2664,8 +2698,6 @@ function forum_get_discussions($cm, $forumsort="d.timemodified DESC", $fullpost= } else { $groupselect = ""; } - - if (empty($forumsort)) { $forumsort = "d.timemodified DESC"; } diff --git a/mod/forum/tests/lib_test.php b/mod/forum/tests/lib_test.php index d427c85c2da..6ccdbc25b5c 100644 --- a/mod/forum/tests/lib_test.php +++ b/mod/forum/tests/lib_test.php @@ -1878,4 +1878,121 @@ class mod_forum_lib_testcase extends advanced_testcase { } + /** + * Test forum_get_discussions + */ + public function test_forum_get_discussions_with_groups() { + global $DB; + + $this->resetAfterTest(true); + + // Create course to add the module. + $course = self::getDataGenerator()->create_course(array('groupmode' => VISIBLEGROUPS, 'groupmodeforce' => 0)); + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + + $role = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + self::getDataGenerator()->enrol_user($user1->id, $course->id, $role->id); + self::getDataGenerator()->enrol_user($user2->id, $course->id, $role->id); + self::getDataGenerator()->enrol_user($user3->id, $course->id, $role->id); + + // Forum forcing separate gropus. + $record = new stdClass(); + $record->course = $course->id; + $forum = self::getDataGenerator()->create_module('forum', $record, array('groupmode' => SEPARATEGROUPS)); + $cm = get_coursemodule_from_instance('forum', $forum->id); + + // Create groups. + $group1 = self::getDataGenerator()->create_group(array('courseid' => $course->id)); + $group2 = self::getDataGenerator()->create_group(array('courseid' => $course->id)); + $group3 = self::getDataGenerator()->create_group(array('courseid' => $course->id)); + + // Add the user1 to g1 and g2 groups. + groups_add_member($group1->id, $user1->id); + groups_add_member($group2->id, $user1->id); + + // Add the user 2 and 3 to only one group. + groups_add_member($group1->id, $user2->id); + groups_add_member($group3->id, $user3->id); + + // Add a few discussions. + $record = array(); + $record['course'] = $course->id; + $record['forum'] = $forum->id; + $record['userid'] = $user1->id; + $record['groupid'] = $group1->id; + $discussiong1u1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + + $record['groupid'] = $group2->id; + $discussiong2u1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + + $record['userid'] = $user2->id; + $record['groupid'] = $group1->id; + $discussiong1u2 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + + $record['userid'] = $user3->id; + $record['groupid'] = $group3->id; + $discussiong3u3 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + + self::setUser($user1); + // Test retrieve discussions not passing the groupid parameter. We will receive only first group discussions. + $discussions = forum_get_discussions($cm); + self::assertCount(2, $discussions); + foreach ($discussions as $discussion) { + self::assertEquals($group1->id, $discussion->groupid); + } + + // Get all my discussions. + $discussions = forum_get_discussions($cm, '', true, -1, -1, false, -1, 0, 0); + self::assertCount(3, $discussions); + + // Get all my g1 discussions. + $discussions = forum_get_discussions($cm, '', true, -1, -1, false, -1, 0, $group1->id); + self::assertCount(2, $discussions); + foreach ($discussions as $discussion) { + self::assertEquals($group1->id, $discussion->groupid); + } + + // Get all my g2 discussions. + $discussions = forum_get_discussions($cm, '', true, -1, -1, false, -1, 0, $group2->id); + self::assertCount(1, $discussions); + $discussion = array_shift($discussions); + self::assertEquals($group2->id, $discussion->groupid); + self::assertEquals($user1->id, $discussion->userid); + self::assertEquals($discussiong2u1->id, $discussion->discussion); + + // Get all my g3 discussions (I'm not enrolled in that group). + $discussions = forum_get_discussions($cm, '', true, -1, -1, false, -1, 0, $group3->id); + self::assertCount(0, $discussions); + + // This group does not exist. + $discussions = forum_get_discussions($cm, '', true, -1, -1, false, -1, 0, $group3->id + 1000); + self::assertCount(0, $discussions); + + self::setUser($user2); + + // Test retrieve discussions not passing the groupid parameter. We will receive only first group discussions. + $discussions = forum_get_discussions($cm); + self::assertCount(2, $discussions); + foreach ($discussions as $discussion) { + self::assertEquals($group1->id, $discussion->groupid); + } + + // Get all my viewable discussions. + $discussions = forum_get_discussions($cm, '', true, -1, -1, false, -1, 0, 0); + self::assertCount(2, $discussions); + foreach ($discussions as $discussion) { + self::assertEquals($group1->id, $discussion->groupid); + } + + // Get all my g2 discussions (I'm not enrolled in that group). + $discussions = forum_get_discussions($cm, '', true, -1, -1, false, -1, 0, $group2->id); + self::assertCount(0, $discussions); + + // Get all my g3 discussions (I'm not enrolled in that group). + $discussions = forum_get_discussions($cm, '', true, -1, -1, false, -1, 0, $group3->id); + self::assertCount(0, $discussions); + + } } diff --git a/mod/forum/upgrade.txt b/mod/forum/upgrade.txt index 03a0e133b0e..2dc8b89b72d 100644 --- a/mod/forum/upgrade.txt +++ b/mod/forum/upgrade.txt @@ -4,6 +4,9 @@ information provided here is intended especially for developers. === 3.0 === * External function get_forums_by_courses now returns and additional field "cancreatediscussions" that indicates if the user can create discussions in the forum. + * A new optional parameter (groupid) has been added to get_forum_discussions. + This parameter can override the automatically calculated current group. + * New constant FORUM_POSTS_ALL_USER_GROUPS, to be used as parameter in functions where we'd need to retrieve all the user posts. === 2.8 === * The following functions have all been marked as deprecated. Many of