From 1530035f12e52e1434ade3d7823d008170e4c5ce Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Thu, 19 Nov 2020 09:14:02 +0800 Subject: [PATCH] MDL-65252 mod_forum: removal of get_forum_discussion_posts ws --- mod/forum/db/services.php | 10 - mod/forum/externallib.php | 237 ------------- mod/forum/tests/externallib_test.php | 475 +++------------------------ mod/forum/upgrade.txt | 1 + 4 files changed, 39 insertions(+), 684 deletions(-) diff --git a/mod/forum/db/services.php b/mod/forum/db/services.php index 856d6a11d4b..9e94d830a4d 100644 --- a/mod/forum/db/services.php +++ b/mod/forum/db/services.php @@ -48,16 +48,6 @@ $functions = array( 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE) ), - 'mod_forum_get_forum_discussion_posts' => array( - 'classname' => 'mod_forum_external', - 'methodname' => 'get_forum_discussion_posts', - 'classpath' => 'mod/forum/externallib.php', - 'description' => 'Returns a list of forum posts for a discussion.', - 'type' => 'read', - 'capabilities' => 'mod/forum:viewdiscussion, mod/forum:viewqandawithoutposting', - 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE) - ), - 'mod_forum_get_forum_discussions_paginated' => array( 'classname' => 'mod_forum_external', 'methodname' => 'get_forum_discussions_paginated', diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index 407dc7096c6..222612d84de 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -267,243 +267,6 @@ class mod_forum_external extends external_api { ]); } - /** - * Describes the parameters for get_forum_discussion_posts. - * - * @return external_function_parameters - * @since Moodle 2.7 - */ - public static function get_forum_discussion_posts_parameters() { - return new external_function_parameters ( - array( - 'discussionid' => new external_value(PARAM_INT, 'discussion ID', VALUE_REQUIRED), - 'sortby' => new external_value(PARAM_ALPHA, - 'sort by this element: id, created or modified', VALUE_DEFAULT, 'created'), - 'sortdirection' => new external_value(PARAM_ALPHA, 'sort direction: ASC or DESC', VALUE_DEFAULT, 'DESC') - ) - ); - } - - /** - * Returns a list of forum posts for a discussion - * - * @param int $discussionid the post ids - * @param string $sortby sort by this element (id, created or modified) - * @param string $sortdirection sort direction: ASC or DESC - * - * @return array the forum post details - * @since Moodle 2.7 - * @todo MDL-65252 This will be removed in Moodle 3.11 - */ - public static function get_forum_discussion_posts($discussionid, $sortby = "created", $sortdirection = "DESC") { - global $CFG, $DB, $USER, $PAGE; - - $posts = array(); - $warnings = array(); - - // Validate the parameter. - $params = self::validate_parameters(self::get_forum_discussion_posts_parameters(), - array( - 'discussionid' => $discussionid, - 'sortby' => $sortby, - 'sortdirection' => $sortdirection)); - - // Compact/extract functions are not recommended. - $discussionid = $params['discussionid']; - $sortby = $params['sortby']; - $sortdirection = $params['sortdirection']; - - $sortallowedvalues = array('id', 'created', 'modified'); - if (!in_array($sortby, $sortallowedvalues)) { - throw new invalid_parameter_exception('Invalid value for sortby parameter (value: ' . $sortby . '),' . - 'allowed values are: ' . implode(',', $sortallowedvalues)); - } - - $sortdirection = strtoupper($sortdirection); - $directionallowedvalues = array('ASC', 'DESC'); - if (!in_array($sortdirection, $directionallowedvalues)) { - throw new invalid_parameter_exception('Invalid value for sortdirection parameter (value: ' . $sortdirection . '),' . - 'allowed values are: ' . implode(',', $directionallowedvalues)); - } - - $discussion = $DB->get_record('forum_discussions', array('id' => $discussionid), '*', MUST_EXIST); - $forum = $DB->get_record('forum', array('id' => $discussion->forum), '*', MUST_EXIST); - $course = $DB->get_record('course', array('id' => $forum->course), '*', MUST_EXIST); - $cm = get_coursemodule_from_instance('forum', $forum->id, $course->id, false, MUST_EXIST); - - // Validate the module context. It checks everything that affects the module visibility (including groupings, etc..). - $modcontext = context_module::instance($cm->id); - self::validate_context($modcontext); - - // This require must be here, see mod/forum/discuss.php. - require_once($CFG->dirroot . "/mod/forum/lib.php"); - - // Check they have the view forum capability. - require_capability('mod/forum:viewdiscussion', $modcontext, null, true, 'noviewdiscussionspermission', 'forum'); - - if (! $post = forum_get_post_full($discussion->firstpost)) { - throw new moodle_exception('notexists', 'forum'); - } - - // This function check groups, qanda, timed discussions, etc. - if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm)) { - throw new moodle_exception('noviewdiscussionspermission', 'forum'); - } - - $canviewfullname = has_capability('moodle/site:viewfullnames', $modcontext); - - // We will add this field in the response. - $canreply = forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext); - - $forumtracked = forum_tp_is_tracked($forum); - - $sort = 'p.' . $sortby . ' ' . $sortdirection; - $allposts = forum_get_all_discussion_posts($discussion->id, $sort, $forumtracked); - - foreach ($allposts as $post) { - if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm, false)) { - $warning = array(); - $warning['item'] = 'post'; - $warning['itemid'] = $post->id; - $warning['warningcode'] = '1'; - $warning['message'] = 'You can\'t see this post'; - $warnings[] = $warning; - continue; - } - - // Function forum_get_all_discussion_posts adds postread field. - // Note that the value returned can be a boolean or an integer. The WS expects a boolean. - if (empty($post->postread)) { - $post->postread = false; - } else { - $post->postread = true; - } - - $post->isprivatereply = !empty($post->privatereplyto); - - $post->canreply = $canreply; - if (!empty($post->children)) { - $post->children = array_keys($post->children); - } else { - $post->children = array(); - } - - if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm)) { - // The post is available, but has been marked as deleted. - // It will still be available but filled with a placeholder. - $post->userid = null; - $post->userfullname = null; - $post->userpictureurl = null; - - $post->subject = get_string('privacy:request:delete:post:subject', 'mod_forum'); - $post->message = get_string('privacy:request:delete:post:message', 'mod_forum'); - - $post->deleted = true; - $posts[] = $post; - - continue; - } - $post->deleted = false; - - if (forum_is_author_hidden($post, $forum)) { - $post->userid = null; - $post->userfullname = null; - $post->userpictureurl = null; - } else { - $user = new stdclass(); - $user->id = $post->userid; - $user = username_load_fields_from_object($user, $post, null, array('picture', 'imagealt', 'email')); - $post->userfullname = fullname($user, $canviewfullname); - - $userpicture = new user_picture($user); - $userpicture->size = 1; // Size f1. - $post->userpictureurl = $userpicture->get_url($PAGE)->out(false); - } - - $post->subject = external_format_string($post->subject, $modcontext->id); - // Rewrite embedded images URLs. - $options = array('trusted' => $post->messagetrust); - list($post->message, $post->messageformat) = - external_format_text($post->message, $post->messageformat, $modcontext->id, 'mod_forum', 'post', $post->id, - $options); - - // List attachments. - if (!empty($post->attachment)) { - $post->attachments = external_util::get_area_files($modcontext->id, 'mod_forum', 'attachment', $post->id); - } - $messageinlinefiles = external_util::get_area_files($modcontext->id, 'mod_forum', 'post', $post->id); - if (!empty($messageinlinefiles)) { - $post->messageinlinefiles = $messageinlinefiles; - } - // Post tags. - $post->tags = \core_tag\external\util::get_item_tags('mod_forum', 'forum_posts', $post->id); - - $posts[] = $post; - } - - $result = array(); - $result['posts'] = $posts; - $result['ratinginfo'] = \core_rating\external\util::get_rating_info($forum, $modcontext, 'mod_forum', 'post', $posts); - $result['warnings'] = $warnings; - return $result; - } - - /** - * Describes the get_forum_discussion_posts return value. - * - * @return external_single_structure - * @since Moodle 2.7 - */ - public static function get_forum_discussion_posts_returns() { - return new external_single_structure( - array( - 'posts' => new external_multiple_structure( - new external_single_structure( - array( - 'id' => new external_value(PARAM_INT, 'Post id'), - 'discussion' => new external_value(PARAM_INT, 'Discussion id'), - 'parent' => new external_value(PARAM_INT, 'Parent id'), - 'userid' => new external_value(PARAM_INT, 'User id'), - 'created' => new external_value(PARAM_INT, 'Creation time'), - 'modified' => new external_value(PARAM_INT, 'Time modified'), - 'mailed' => new external_value(PARAM_INT, 'Mailed?'), - 'subject' => new external_value(PARAM_RAW, 'The post subject'), - 'message' => new external_value(PARAM_RAW, 'The post message'), - 'messageformat' => new external_format_value('message'), - 'messagetrust' => new external_value(PARAM_INT, 'Can we trust?'), - 'messageinlinefiles' => new external_files('post message inline files', VALUE_OPTIONAL), - 'attachment' => new external_value(PARAM_RAW, 'Has attachments?'), - 'attachments' => new external_files('attachments', VALUE_OPTIONAL), - 'totalscore' => new external_value(PARAM_INT, 'The post message total score'), - 'mailnow' => new external_value(PARAM_INT, 'Mail now?'), - 'children' => new external_multiple_structure(new external_value(PARAM_INT, 'children post id')), - 'canreply' => new external_value(PARAM_BOOL, 'The user can reply to posts?'), - 'postread' => new external_value(PARAM_BOOL, 'The post was read'), - 'userfullname' => new external_value(PARAM_TEXT, 'Post author full name'), - 'userpictureurl' => new external_value(PARAM_URL, 'Post author picture.', VALUE_OPTIONAL), - 'deleted' => new external_value(PARAM_BOOL, 'This post has been removed.'), - 'isprivatereply' => new external_value(PARAM_BOOL, 'The post is a private reply'), - 'tags' => new external_multiple_structure( - \core_tag\external\tag_item_exporter::get_read_structure(), 'Tags', VALUE_OPTIONAL - ), - ), 'post' - ) - ), - 'ratinginfo' => \core_rating\external\util::external_ratings_structure(), - 'warnings' => new external_warnings() - ) - ); - } - - /** - * Mark the get_forum_discussion_posts web service as deprecated. - * - * @return bool - */ - public static function get_forum_discussion_posts_is_deprecated() { - return true; - } - /** * Mark the get_forum_discussions_paginated web service as deprecated. * diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php index 81dc64a0691..99f45ac829d 100644 --- a/mod/forum/tests/externallib_test.php +++ b/mod/forum/tests/externallib_test.php @@ -274,253 +274,6 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $this->assertFalse($response['pinned']); } - /** - * Test get forum posts - */ - public function test_mod_forum_get_forum_discussion_posts() { - global $CFG, $PAGE; - - $this->resetAfterTest(true); - - // Set the CFG variable to allow track forums. - $CFG->forum_trackreadposts = true; - - // Create a user who can track forums. - $record = new stdClass(); - $record->trackforums = true; - $user1 = self::getDataGenerator()->create_user($record); - // Create a bunch of other users to post. - $user2 = self::getDataGenerator()->create_user(); - $user3 = self::getDataGenerator()->create_user(); - - // Set the first created user to the test user. - self::setUser($user1); - - // Create course to add the module. - $course1 = self::getDataGenerator()->create_course(); - - // Forum with tracking off. - $record = new stdClass(); - $record->course = $course1->id; - $record->trackingtype = FORUM_TRACKING_OFF; - $forum1 = self::getDataGenerator()->create_module('forum', $record); - $forum1context = context_module::instance($forum1->cmid); - - // Forum with tracking enabled. - $record = new stdClass(); - $record->course = $course1->id; - $forum2 = self::getDataGenerator()->create_module('forum', $record); - $forum2cm = get_coursemodule_from_id('forum', $forum2->cmid); - $forum2context = context_module::instance($forum2->cmid); - - // Add discussions to the forums. - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user1->id; - $record->forum = $forum1->id; - $discussion1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); - - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user2->id; - $record->forum = $forum1->id; - $discussion2 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); - - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user2->id; - $record->forum = $forum2->id; - $discussion3 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); - - // Add 2 replies to the discussion 1 from different users. - $record = new stdClass(); - $record->discussion = $discussion1->id; - $record->parent = $discussion1->firstpost; - $record->userid = $user2->id; - $discussion1reply1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); - $filename = 'shouldbeanimage.jpg'; - // Add a fake inline image to the post. - $filerecordinline = array( - 'contextid' => $forum1context->id, - 'component' => 'mod_forum', - 'filearea' => 'post', - 'itemid' => $discussion1reply1->id, - 'filepath' => '/', - 'filename' => $filename, - ); - $fs = get_file_storage(); - $timepost = time(); - $fs->create_file_from_string($filerecordinline, 'image contents (not really)'); - - $record->parent = $discussion1reply1->id; - $record->userid = $user3->id; - $record->tags = array('Cats', 'Dogs'); - $discussion1reply2 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); - - // Enrol the user in the course. - $enrol = enrol_get_plugin('manual'); - // Following line enrol and assign default role id to the user. - // So the user automatically gets mod/forum:viewdiscussion on all forums of the course. - $this->getDataGenerator()->enrol_user($user1->id, $course1->id); - $this->getDataGenerator()->enrol_user($user2->id, $course1->id); - - // Delete one user, to test that we still receive posts by this user. - delete_user($user3); - - // Create what we expect to be returned when querying the discussion. - $expectedposts = array( - 'posts' => array(), - 'ratinginfo' => array( - 'contextid' => $forum1context->id, - 'component' => 'mod_forum', - 'ratingarea' => 'post', - 'canviewall' => null, - 'canviewany' => null, - 'scales' => array(), - 'ratings' => array(), - ), - 'warnings' => array(), - ); - - // User pictures are initially empty, we should get the links once the external function is called. - $expectedposts['posts'][] = array( - 'id' => $discussion1reply2->id, - 'discussion' => $discussion1reply2->discussion, - 'parent' => $discussion1reply2->parent, - 'userid' => (int) $discussion1reply2->userid, - 'created' => $discussion1reply2->created, - 'modified' => $discussion1reply2->modified, - 'mailed' => $discussion1reply2->mailed, - 'subject' => $discussion1reply2->subject, - 'message' => file_rewrite_pluginfile_urls($discussion1reply2->message, 'pluginfile.php', - $forum1context->id, 'mod_forum', 'post', $discussion1reply2->id), - 'messageformat' => 1, // This value is usually changed by external_format_text() function. - 'messagetrust' => $discussion1reply2->messagetrust, - 'attachment' => $discussion1reply2->attachment, - 'totalscore' => $discussion1reply2->totalscore, - 'mailnow' => $discussion1reply2->mailnow, - 'children' => array(), - 'canreply' => true, - 'postread' => false, - 'userfullname' => fullname($user3), - 'userpictureurl' => '', - 'deleted' => false, - 'isprivatereply' => false, - 'tags' => \core_tag\external\util::get_item_tags('mod_forum', 'forum_posts', $discussion1reply2->id), - ); - // Cast to expected. - $this->assertCount(2, $expectedposts['posts'][0]['tags']); - $expectedposts['posts'][0]['tags'][0]['isstandard'] = (bool) $expectedposts['posts'][0]['tags'][0]['isstandard']; - $expectedposts['posts'][0]['tags'][1]['isstandard'] = (bool) $expectedposts['posts'][0]['tags'][1]['isstandard']; - - $expectedposts['posts'][] = array( - 'id' => $discussion1reply1->id, - 'discussion' => $discussion1reply1->discussion, - 'parent' => $discussion1reply1->parent, - 'userid' => (int) $discussion1reply1->userid, - 'created' => $discussion1reply1->created, - 'modified' => $discussion1reply1->modified, - 'mailed' => $discussion1reply1->mailed, - 'subject' => $discussion1reply1->subject, - 'message' => file_rewrite_pluginfile_urls($discussion1reply1->message, 'pluginfile.php', - $forum1context->id, 'mod_forum', 'post', $discussion1reply1->id), - 'messageformat' => 1, // This value is usually changed by external_format_text() function. - 'messagetrust' => $discussion1reply1->messagetrust, - 'attachment' => $discussion1reply1->attachment, - 'messageinlinefiles' => array( - array( - 'filename' => $filename, - 'filepath' => '/', - 'filesize' => '27', - 'fileurl' => moodle_url::make_webservice_pluginfile_url($forum1context->id, 'mod_forum', 'post', - $discussion1reply1->id, '/', $filename), - 'timemodified' => $timepost, - 'mimetype' => 'image/jpeg', - 'isexternalfile' => false, - ) - ), - 'totalscore' => $discussion1reply1->totalscore, - 'mailnow' => $discussion1reply1->mailnow, - 'children' => array($discussion1reply2->id), - 'canreply' => true, - 'postread' => false, - 'userfullname' => fullname($user2), - 'userpictureurl' => '', - 'deleted' => false, - 'isprivatereply' => false, - 'tags' => array(), - ); - - // Test a discussion with two additional posts (total 3 posts). - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id, 'modified', 'DESC'); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); - $this->assertEquals(3, count($posts['posts'])); - - // Generate here the pictures because we need to wait to the external function to init the theme. - $userpicture = new user_picture($user3); - $userpicture->size = 1; // Size f1. - $expectedposts['posts'][0]['userpictureurl'] = $userpicture->get_url($PAGE)->out(false); - - $userpicture = new user_picture($user2); - $userpicture->size = 1; // Size f1. - $expectedposts['posts'][1]['userpictureurl'] = $userpicture->get_url($PAGE)->out(false); - - // Unset the initial discussion post. - array_pop($posts['posts']); - $this->assertEquals($expectedposts, $posts); - - // Check we receive the unread count correctly on tracked forum. - forum_tp_count_forum_unread_posts($forum2cm, $course1, true); // Reset static cache. - $result = mod_forum_external::get_forums_by_courses(array($course1->id)); - $result = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $result); - foreach ($result as $f) { - if ($f['id'] == $forum2->id) { - $this->assertEquals(1, $f['unreadpostscount']); - } - } - - // Test discussion without additional posts. There should be only one post (the one created by the discussion). - $posts = mod_forum_external::get_forum_discussion_posts($discussion2->id, 'modified', 'DESC'); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); - $this->assertEquals(1, count($posts['posts'])); - - // Test discussion tracking on not tracked forum. - $result = mod_forum_external::view_forum_discussion($discussion1->id); - $result = external_api::clean_returnvalue(mod_forum_external::view_forum_discussion_returns(), $result); - $this->assertTrue($result['status']); - $this->assertEmpty($result['warnings']); - - // Test posts have not been marked as read. - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id, 'modified', 'DESC'); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); - foreach ($posts['posts'] as $post) { - $this->assertFalse($post['postread']); - } - - // Test discussion tracking on tracked forum. - $result = mod_forum_external::view_forum_discussion($discussion3->id); - $result = external_api::clean_returnvalue(mod_forum_external::view_forum_discussion_returns(), $result); - $this->assertTrue($result['status']); - $this->assertEmpty($result['warnings']); - - // Test posts have been marked as read. - $posts = mod_forum_external::get_forum_discussion_posts($discussion3->id, 'modified', 'DESC'); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); - foreach ($posts['posts'] as $post) { - $this->assertTrue($post['postread']); - } - - // Check we receive 0 unread posts. - forum_tp_count_forum_unread_posts($forum2cm, $course1, true); // Reset static cache. - $result = mod_forum_external::get_forums_by_courses(array($course1->id)); - $result = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $result); - foreach ($result as $f) { - if ($f['id'] == $forum2->id) { - $this->assertEquals(0, $f['unreadpostscount']); - } - } - } - /** * Test get forum posts * @@ -851,152 +604,6 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { } } - /** - * Test get forum posts - */ - public function test_mod_forum_get_forum_discussion_posts_deleted() { - global $CFG, $PAGE; - - $this->resetAfterTest(true); - $generator = self::getDataGenerator()->get_plugin_generator('mod_forum'); - - // Create a course and enrol some users in it. - $course1 = self::getDataGenerator()->create_course(); - - // Create users. - $user1 = self::getDataGenerator()->create_user(); - $this->getDataGenerator()->enrol_user($user1->id, $course1->id); - $user2 = self::getDataGenerator()->create_user(); - $this->getDataGenerator()->enrol_user($user2->id, $course1->id); - - // Set the first created user to the test user. - self::setUser($user1); - - // Create test data. - $forum1 = self::getDataGenerator()->create_module('forum', (object) [ - 'course' => $course1->id, - ]); - $forum1context = context_module::instance($forum1->cmid); - - // Add discussions to the forum. - $discussion = $generator->create_discussion((object) [ - 'course' => $course1->id, - 'userid' => $user1->id, - 'forum' => $forum1->id, - ]); - - $discussion2 = $generator->create_discussion((object) [ - 'course' => $course1->id, - 'userid' => $user2->id, - 'forum' => $forum1->id, - ]); - - // Add replies to the discussion. - $discussionreply1 = $generator->create_post((object) [ - 'discussion' => $discussion->id, - 'parent' => $discussion->firstpost, - 'userid' => $user2->id, - ]); - $discussionreply2 = $generator->create_post((object) [ - 'discussion' => $discussion->id, - 'parent' => $discussionreply1->id, - 'userid' => $user2->id, - 'subject' => '', - 'message' => '', - 'messageformat' => FORMAT_PLAIN, - 'deleted' => 1, - ]); - $discussionreply3 = $generator->create_post((object) [ - 'discussion' => $discussion->id, - 'parent' => $discussion->firstpost, - 'userid' => $user2->id, - ]); - - // Test where some posts have been marked as deleted. - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id, 'modified', 'DESC'); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); - $deletedsubject = get_string('privacy:request:delete:post:subject', 'mod_forum'); - $deletedmessage = get_string('privacy:request:delete:post:message', 'mod_forum'); - - foreach ($posts['posts'] as $post) { - if ($post['id'] == $discussionreply2->id) { - $this->assertTrue($post['deleted']); - $this->assertEquals($deletedsubject, $post['subject']); - $this->assertEquals($deletedmessage, $post['message']); - } else { - $this->assertFalse($post['deleted']); - $this->assertNotEquals($deletedsubject, $post['subject']); - $this->assertNotEquals($deletedmessage, $post['message']); - } - } - } - - /** - * Test get forum posts (qanda forum) - */ - public function test_mod_forum_get_forum_discussion_posts_qanda() { - global $CFG, $DB; - - $this->resetAfterTest(true); - - $record = new stdClass(); - $user1 = self::getDataGenerator()->create_user($record); - $user2 = self::getDataGenerator()->create_user(); - - // Set the first created user to the test user. - self::setUser($user1); - - // Create course to add the module. - $course1 = self::getDataGenerator()->create_course(); - $this->getDataGenerator()->enrol_user($user1->id, $course1->id); - $this->getDataGenerator()->enrol_user($user2->id, $course1->id); - - // Forum with tracking off. - $record = new stdClass(); - $record->course = $course1->id; - $record->type = 'qanda'; - $forum1 = self::getDataGenerator()->create_module('forum', $record); - $forum1context = context_module::instance($forum1->cmid); - - // Add discussions to the forums. - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user2->id; - $record->forum = $forum1->id; - $discussion1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); - - // Add 1 reply (not the actual user). - $record = new stdClass(); - $record->discussion = $discussion1->id; - $record->parent = $discussion1->firstpost; - $record->userid = $user2->id; - $discussion1reply1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); - - // We still see only the original post. - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id, 'modified', 'DESC'); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); - $this->assertEquals(1, count($posts['posts'])); - - // Add a new reply, the user is going to be able to see only the original post and their new post. - $record = new stdClass(); - $record->discussion = $discussion1->id; - $record->parent = $discussion1->firstpost; - $record->userid = $user1->id; - $discussion1reply2 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); - - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id, 'modified', 'DESC'); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); - $this->assertEquals(2, count($posts['posts'])); - - // Now, we can fake the time of the user post, so he can se the rest of the discussion posts. - $discussion1reply2->created -= $CFG->maxeditingtime * 2; - $DB->update_record('forum_posts', $discussion1reply2); - - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id, 'modified', 'DESC'); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); - $this->assertEquals(3, count($posts['posts'])); - } - /** * Test get forum discussions paginated */ @@ -1597,8 +1204,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $createdpost = mod_forum_external::add_discussion_post($discussion->firstpost, 'some subject', 'some text here...'); $createdpost = external_api::clean_returnvalue(mod_forum_external::add_discussion_post_returns(), $createdpost); - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // We receive the discussion and the post. $this->assertEquals(2, count($posts['posts'])); @@ -1676,16 +1283,16 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $dummytext, $options); $createdpost = external_api::clean_returnvalue(mod_forum_external::add_discussion_post_returns(), $createdpost); - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // We receive the discussion and the post. // Can't guarantee order of posts during tests. $postfound = false; foreach ($posts['posts'] as $thispost) { if ($createdpost['postid'] == $thispost['id']) { $this->assertEquals($createdpost['postid'], $thispost['id']); - $this->assertEquals($thispost['attachment'], 1, "There should be a non-inline attachment"); - $this->assertCount(1, $thispost['attachments'], "There should be 1 attachment"); + $this->assertCount(1, $thispost['attachments']); + $this->assertEquals('attachment.txt', $thispost['attachments'][0]['filename']); $this->assertEquals($thispost['attachments'][0]['filename'], $attachfilename, "There should be 1 attachment"); $this->assertStringContainsString('pluginfile.php', $thispost['message']); $postfound = true; @@ -1751,8 +1358,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { // Add a discussion post in a forum discussion where the user is not subscribed (auto-subscribe preference enabled). mod_forum_external::add_discussion_post($discussion1->firstpost, 'some subject', 'some text here...'); - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion1->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // We receive the discussion and the post. $this->assertEquals(2, count($posts['posts'])); // The user should be subscribed to the discussion after adding a discussion post. @@ -1764,8 +1371,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { // Add a discussion post in a forum discussion where the user is subscribed (auto-subscribe preference disabled). mod_forum_external::add_discussion_post($discussion1->firstpost, 'some subject 1', 'some text here 1...'); - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion1->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // We receive the discussion and the post. $this->assertEquals(3, count($posts['posts'])); // The user should still be subscribed to the discussion after adding a discussion post. @@ -1775,8 +1382,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { // Add a discussion post in a forum discussion where the user is not subscribed (auto-subscribe preference disabled). mod_forum_external::add_discussion_post($discussion2->firstpost, 'some subject 2', 'some text here 2...'); - $posts = mod_forum_external::get_forum_discussion_posts($discussion2->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion2->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // We receive the discussion and the post. $this->assertEquals(2, count($posts['posts'])); // The user should still not be subscribed to the discussion after adding a discussion post. @@ -1791,8 +1398,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { mod_forum_external::add_discussion_post($discussion2->firstpost, 'some subject 2', 'some text here 2...', $options); - $posts = mod_forum_external::get_forum_discussion_posts($discussion2->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion2->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // We receive the discussion and the post. $this->assertEquals(3, count($posts['posts'])); // The user should now be subscribed to the discussion after adding a discussion post. @@ -2139,9 +1746,9 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { } /** - * Test get forum posts discussions including rating information. + * Test get posts discussions including rating information. */ - public function test_mod_forum_get_forum_discussion_rating_information() { + public function test_mod_forum_get_discussion_rating_information() { global $DB, $CFG; require_once($CFG->dirroot . '/rating/lib.php'); @@ -2209,8 +1816,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { // Retrieve the rating for the post as student. $this->setUser($user1); - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion->id, 'id', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); $this->assertCount(1, $posts['ratinginfo']['ratings']); $this->assertTrue($posts['ratinginfo']['ratings'][0]['canviewaggregate']); $this->assertFalse($posts['ratinginfo']['canviewall']); @@ -2220,8 +1827,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { // Retrieve the rating for the post as teacher. $this->setUser($teacher); - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion->id, 'id', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); $this->assertCount(1, $posts['ratinginfo']['ratings']); $this->assertTrue($posts['ratinginfo']['ratings'][0]['canviewaggregate']); $this->assertTrue($posts['ratinginfo']['canviewall']); @@ -2340,29 +1947,29 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { // The teacher will receive their private reply. self::setUser($teacher1); - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion->id, 'id', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); $this->assertEquals(2, count($posts['posts'])); $this->assertTrue($posts['posts'][0]['isprivatereply']); // Another teacher on the course will also receive the private reply. self::setUser($teacher2); - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion->id, 'id', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); $this->assertEquals(2, count($posts['posts'])); $this->assertTrue($posts['posts'][0]['isprivatereply']); // The student will receive the private reply. self::setUser($student1); - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion->id, 'id', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); $this->assertEquals(2, count($posts['posts'])); $this->assertTrue($posts['posts'][0]['isprivatereply']); // Another student will not receive the private reply. self::setUser($student2); - $posts = mod_forum_external::get_forum_discussion_posts($discussion->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion->id, 'id', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); $this->assertEquals(1, count($posts['posts'])); $this->assertFalse($posts['posts'][0]['isprivatereply']); @@ -2427,16 +2034,14 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $this->assertEquals($cleantext, $discussions['discussions'][1]['message']); // Get posts now. - $posts = mod_forum_external::get_forum_discussion_posts($discussion2->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion2->id, 'modified', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // Admin message is fully trusted. - $this->assertEquals(1, $posts['posts'][0]['messagetrust']); $this->assertEquals($dangeroustext, $posts['posts'][0]['message']); - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion1->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // Student message is not trusted. - $this->assertEquals(0, $posts['posts'][0]['messagetrust']); $this->assertEquals($cleantext, $posts['posts'][0]['message']); } @@ -2490,16 +2095,14 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $this->assertEquals($cleantext, $discussions['discussions'][1]['message']); // Get posts now. - $posts = mod_forum_external::get_forum_discussion_posts($discussion2->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion2->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // Admin message is not trusted because enabletrusttext is disabled. - $this->assertEquals(0, $posts['posts'][0]['messagetrust']); $this->assertEquals($cleantext, $posts['posts'][0]['message']); - $posts = mod_forum_external::get_forum_discussion_posts($discussion1->id); - $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $posts = mod_forum_external::get_discussion_posts($discussion1->id, 'modified', 'ASC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); // Student message is not trusted. - $this->assertEquals(0, $posts['posts'][0]['messagetrust']); $this->assertEquals($cleantext, $posts['posts'][0]['message']); } @@ -3324,16 +2927,14 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $this->assertFalse(\mod_forum\subscriptions::is_subscribed($user->id, $forum, $discussion->id, $cm)); // Get the post from WS. - $result = mod_forum_external::get_forum_discussion_posts($discussion->id); - $result = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $result); + $result = mod_forum_external::get_discussion_posts($discussion->id, 'modified', 'ASC'); + $result = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $result); $found = false; foreach ($result['posts'] as $post) { if ($post['id'] == $newpost->id) { $this->assertEquals($newpost->subject, $post['subject']); $this->assertEquals($message, $post['message']); $this->assertEquals($messageformat, $post['messageformat']); - $this->assertCount(1, $post['messageinlinefiles']); - $this->assertEquals('fakeimage.png', $post['messageinlinefiles'][0]['filename']); $this->assertCount(1, $post['attachments']); $this->assertEquals('faketxt.txt', $post['attachments'][0]['filename']); $found = true; diff --git a/mod/forum/upgrade.txt b/mod/forum/upgrade.txt index 470e717e458..d798366e4e4 100644 --- a/mod/forum/upgrade.txt +++ b/mod/forum/upgrade.txt @@ -4,6 +4,7 @@ information provided here is intended especially for developers. === 4.0 === * The forum_count_replies() function has been removed from core. +* The mod_forum_get_forum_discussion_posts web service has been removed from core. === 3.11 ===