From 54f5401149fa55993c4a14add5c198b231e6bdcf Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Fri, 5 Jun 2015 15:23:24 +0200 Subject: [PATCH] MDL-50489 forum: Handle qanda forums in get_forum_discussions_paginated I removed the last qanda checks in forum_user_can_see_discussion because they are not necessary and they make the external function fail. A user in a qanda forum can always see a discussion (he needs to see the discussion to be able to reply). What he cannot see are the other user posts unless he has replied to the discussion once and the edition period (usually 30 minutes) has ended. Note also that forum_user_can_see_discussion was originally only used when displaying forums in blog format in order to display the button show more or not, this is the reason this wasn't detected before. --- mod/forum/externallib.php | 15 ++++++--- mod/forum/lib.php | 5 --- mod/forum/tests/externallib_test.php | 48 ++++++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index 6da22b13b06..b5fc29324f2 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -625,6 +625,7 @@ class mod_forum_external extends external_api { require_once($CFG->dirroot . "/mod/forum/lib.php"); $warnings = array(); + $discussions = array(); $params = self::validate_parameters(self::get_forum_discussions_paginated_parameters(), array( @@ -668,9 +669,9 @@ class mod_forum_external extends external_api { require_capability('mod/forum:viewdiscussion', $modcontext, null, true, 'noviewdiscussionspermission', 'forum'); $sort = 'd.' . $sortby . ' ' . $sortdirection; - $discussions = forum_get_discussions($cm, $sort, true, -1, -1, true, $page, $perpage); + $alldiscussions = forum_get_discussions($cm, $sort, true, -1, -1, true, $page, $perpage); - if ($discussions) { + if ($alldiscussions) { $canviewfullname = has_capability('moodle/site:viewfullnames', $modcontext); // Get the unreads array, this takes a forum id and returns data for all discussions. @@ -683,9 +684,13 @@ class mod_forum_external extends external_api { // The forum function returns the replies for all the discussions in a given forum. $replies = forum_count_discussion_replies($forumid, $sort, -1, $page, $perpage); - foreach ($discussions as $did => $discussion) { + foreach ($alldiscussions as $discussion) { + // This function checks for qanda forums. - if (!forum_user_can_see_discussion($forum, $discussion, $modcontext)) { + // Note that the forum_get_discussions returns as id the post id, not the discussion id so we need to do this. + $discussionrec = clone $discussion; + $discussionrec->id = $discussion->discussion; + if (!forum_user_can_see_discussion($forum, $discussionrec, $modcontext)) { $warning = array(); // Function forum_get_discussions returns forum_posts ids not forum_discussions ones. $warning['item'] = 'post'; @@ -762,7 +767,7 @@ class mod_forum_external extends external_api { } } - $discussions[$did] = (array) $discussion; + $discussions[] = $discussion; } } diff --git a/mod/forum/lib.php b/mod/forum/lib.php index b61a50bb98c..0953d997e46 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -5134,11 +5134,6 @@ function forum_user_can_see_discussion($forum, $discussion, $context, $user=NULL return false; } - if ($forum->type == 'qanda' && - !forum_user_has_posted($forum->id, $discussion->id, $user->id) && - !has_capability('mod/forum:viewqandawithoutposting', $context)) { - return false; - } return true; } diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php index 2fd2200744e..e2a57cd21b5 100644 --- a/mod/forum/tests/externallib_test.php +++ b/mod/forum/tests/externallib_test.php @@ -342,11 +342,11 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { // See MDL-41746 for more information. $this->assertDebuggingCalled(); - // Remove the users post from the qanda forum and ensure they can not return the discussion. + // Remove the users post from the qanda forum and ensure they can still see the discussion. $DB->delete_records('forum_posts', array('id' => $discussion2reply1->id)); $discussions = mod_forum_external::get_forum_discussions(array($forum2->id)); $discussions = external_api::clean_returnvalue(mod_forum_external::get_forum_discussions_returns(), $discussions); - $this->assertEquals(0, count($discussions)); + $this->assertEquals(1, count($discussions)); // Call without required view discussion capability. $this->unassignUserCapability('mod/forum:viewdiscussion', null, null, $course1->id); @@ -652,4 +652,48 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $this->assertEquals('requireloginerror', $e->errorcode); } } + + /** + * Test get forum discussions paginated (qanda forums) + */ + public function test_mod_forum_get_forum_discussions_paginated_qanda() { + + $this->resetAfterTest(true); + + // Create courses to add the modules. + $course = self::getDataGenerator()->create_course(); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + // First forum with tracking off. + $record = new stdClass(); + $record->course = $course->id; + $record->type = 'qanda'; + $forum = self::getDataGenerator()->create_module('forum', $record); + + // Add discussions to the forums. + $discussionrecord = new stdClass(); + $discussionrecord->course = $course->id; + $discussionrecord->userid = $user2->id; + $discussionrecord->forum = $forum->id; + $discussion = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($discussionrecord); + + self::setAdminUser(); + $discussions = mod_forum_external::get_forum_discussions_paginated($forum->id); + $discussions = external_api::clean_returnvalue(mod_forum_external::get_forum_discussions_paginated_returns(), $discussions); + + $this->assertCount(1, $discussions['discussions']); + $this->assertCount(0, $discussions['warnings']); + + self::setUser($user1); + $this->getDataGenerator()->enrol_user($user1->id, $course->id); + + $discussions = mod_forum_external::get_forum_discussions_paginated($forum->id); + $discussions = external_api::clean_returnvalue(mod_forum_external::get_forum_discussions_paginated_returns(), $discussions); + + $this->assertCount(1, $discussions['discussions']); + $this->assertCount(0, $discussions['warnings']); + + } }