diff --git a/mod/forum/classes/local/builders/exported_discussion_summaries.php b/mod/forum/classes/local/builders/exported_discussion_summaries.php index fd430a3ef5f..ba1e0aa862d 100644 --- a/mod/forum/classes/local/builders/exported_discussion_summaries.php +++ b/mod/forum/classes/local/builders/exported_discussion_summaries.php @@ -81,22 +81,21 @@ class exported_discussion_summaries { * @param legacy_data_mapper_factory $legacydatamapperfactory Legacy data mapper factory * @param exporter_factory $exporterfactory Exporter factory * @param vault_factory $vaultfactory Vault factory - * @param rating_manager $ratingmanager Rating manager + * @param manager_factory $managerfactory Manager factory */ public function __construct( renderer_base $renderer, legacy_data_mapper_factory $legacydatamapperfactory, exporter_factory $exporterfactory, vault_factory $vaultfactory, - manager_factory $managerfactory, - rating_manager $ratingmanager + manager_factory $managerfactory ) { $this->renderer = $renderer; $this->legacydatamapperfactory = $legacydatamapperfactory; $this->exporterfactory = $exporterfactory; $this->vaultfactory = $vaultfactory; $this->managerfactory = $managerfactory; - $this->ratingmanager = $ratingmanager; + $this->ratingmanager = $managerfactory->get_rating_manager(); } /** diff --git a/mod/forum/classes/local/exporters/group.php b/mod/forum/classes/local/exporters/group.php index a639a794f14..cbc019217d3 100644 --- a/mod/forum/classes/local/exporters/group.php +++ b/mod/forum/classes/local/exporters/group.php @@ -26,7 +26,6 @@ namespace mod_forum\local\exporters; defined('MOODLE_INTERNAL') || die(); -use mod_forum\local\entities\author as author_entity; use core\external\exporter; use renderer_base; use stdClass; diff --git a/mod/forum/classes/local/factories/builder.php b/mod/forum/classes/local/factories/builder.php index ef624b89a9a..2a71d5fb2c7 100644 --- a/mod/forum/classes/local/factories/builder.php +++ b/mod/forum/classes/local/factories/builder.php @@ -105,8 +105,7 @@ class builder { $this->legacydatamapperfactory, $this->exporterfactory, $this->vaultfactory, - $this->managerfactory, - $this->managerfactory->get_rating_manager() + $this->managerfactory ); } } diff --git a/mod/forum/classes/local/vaults/forum.php b/mod/forum/classes/local/vaults/forum.php index e8b83471a15..853e87dfcc6 100644 --- a/mod/forum/classes/local/vaults/forum.php +++ b/mod/forum/classes/local/vaults/forum.php @@ -163,17 +163,20 @@ class forum extends db_table_vault { * @return forum_entity|null */ public function get_from_post_id(int $id) : ?forum_entity { - $db = $this->get_db(); $alias = $this->get_table_alias(); - $tablefields = $db->get_preload_columns(self::TABLE, $alias); - $coursemodulefields = $db->get_preload_columns('course_modules', 'cm_'); - $coursefields = $db->get_preload_columns('course', 'c_'); + $thistable = new dml_table(self::TABLE, $alias, $alias); + $coursemoduletable = new dml_table('course_modules', 'cm', 'cm_'); + $coursetable = new dml_table('course', 'c', 'c_'); + + $tablefields = $thistable->get_field_select(); + $coursemodulefields = $coursemoduletable->get_field_select(); + $coursefields = $coursetable->get_field_select(); $fields = implode(', ', [ - $db->get_preload_columns_sql($tablefields, $alias), + $tablefields, context_helper::get_preload_record_columns_sql('ctx'), - $db->get_preload_columns_sql($coursemodulefields, 'cm'), - $db->get_preload_columns_sql($coursefields, 'c'), + $coursemodulefields, + $coursefields, ]); $tables = "{forum_posts} p"; diff --git a/mod/forum/classes/privacy/provider.php b/mod/forum/classes/privacy/provider.php index da480480199..892196d5e40 100644 --- a/mod/forum/classes/privacy/provider.php +++ b/mod/forum/classes/privacy/provider.php @@ -96,6 +96,7 @@ class provider implements 'subject' => 'privacy:metadata:forum_posts:subject', 'message' => 'privacy:metadata:forum_posts:message', 'userid' => 'privacy:metadata:forum_posts:userid', + 'privatereplyto' => 'privacy:metadata:forum_posts:privatereplyto', ], 'privacy:metadata:forum_posts'); // The 'forum_queue' table contains user data, but it is only a temporary cache of other data. diff --git a/mod/forum/deprecatedlib.php b/mod/forum/deprecatedlib.php index 4621e799bbc..f884344b38d 100644 --- a/mod/forum/deprecatedlib.php +++ b/mod/forum/deprecatedlib.php @@ -1628,6 +1628,7 @@ function forum_print_latest_discussions($course, $forum, $maxdiscussions = -1, $ * @param bool $children * @return int * @deprecated since Moodle 3.7 + * @todo MDL-65252 This will be removed in Moodle 4.1 */ function forum_count_replies($post, $children = true) { global $USER; diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index 3435877e3ce..ba39260253d 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -279,6 +279,7 @@ class mod_forum_external extends external_api { * * @return array the forum post details * @since Moodle 2.7 + * @todo MDL-65252 This will be removed in Moodle 4.1 */ public static function get_forum_discussion_posts($discussionid, $sortby = "created", $sortdirection = "DESC") { global $CFG, $DB, $USER, $PAGE; diff --git a/mod/forum/lang/en/forum.php b/mod/forum/lang/en/forum.php index 74ef9956fdb..05c95c52f7c 100644 --- a/mod/forum/lang/en/forum.php +++ b/mod/forum/lang/en/forum.php @@ -464,6 +464,7 @@ $string['privacy:metadata:forum_posts:parent'] = 'The parent post that was repli $string['privacy:metadata:forum_posts:subject'] = 'The subject of the forum post.'; $string['privacy:metadata:forum_posts:totalscore'] = 'The message of the forum post.'; $string['privacy:metadata:forum_posts:userid'] = 'The ID of the user who authored the forum post.'; +$string['privacy:metadata:forum_posts:privatereplyto'] = 'The ID of the user this reply was sent to.'; $string['privacy:metadata:forum_queue'] = 'Temporary log of posts that will be mailed in digest form'; $string['privacy:metadata:forum_queue:discussionid'] = 'Forum discussion ID'; $string['privacy:metadata:forum_queue:postid'] = 'Forum post ID'; diff --git a/mod/forum/post.php b/mod/forum/post.php index 9e606ce4232..4daa039ea3d 100644 --- a/mod/forum/post.php +++ b/mod/forum/post.php @@ -85,7 +85,6 @@ if (!isloggedin() or isguestuser()) { } } - $capabilitymanager = $managerfactory->get_capability_manager($forumentity); $forum = $forumdatamapper->to_legacy_object($forumentity); $modcontext = $forumentity->get_context(); $course = $forumentity->get_course_record(); diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php index 65685bbf21e..8fa9dceb49e 100644 --- a/mod/forum/tests/externallib_test.php +++ b/mod/forum/tests/externallib_test.php @@ -420,6 +420,308 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { } } + /** + * Test get forum posts + * + * Tests is similar to the get_forum_discussion_posts only utilizing the new return structure and entities + */ + public function test_mod_forum_get_discussion_posts() { + global $CFG, $PAGE; + + $this->resetAfterTest(true); + + // Set the CFG variable to allow track forums. + $CFG->forum_trackreadposts = true; + + $urlfactory = mod_forum\local\container::get_url_factory(); + $legacyfactory = mod_forum\local\container::get_legacy_data_mapper_factory(); + $entityfactory = mod_forum\local\container::get_entity_factory(); + + // 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(); + $user2entity = $entityfactory->get_author_from_stdclass($user2); + $exporteduser2 = [ + 'id' => (int) $user2->id, + 'fullname' => fullname($user2), + 'groups' => [], + 'urls' => [ + 'profile' => $urlfactory->get_author_profile_url($user2entity), + 'profileimage' => $urlfactory->get_author_profile_image_url($user2entity), + ] + ]; + $user2->fullname = $exporteduser2['fullname']; + + $user3 = self::getDataGenerator()->create_user(['fullname' => "Mr Pants 1"]); + $user3entity = $entityfactory->get_author_from_stdclass($user3); + $exporteduser3 = [ + 'id' => (int) $user3->id, + 'fullname' => fullname($user3), + 'groups' => [], + 'urls' => [ + 'profile' => $urlfactory->get_author_profile_url($user3entity), + 'profileimage' => $urlfactory->get_author_profile_image_url($user3entity), + ] + ]; + $user3->fullname = $exporteduser3['fullname']; + $forumgenerator = self::getDataGenerator()->get_plugin_generator('mod_forum'); + + // 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 = $forumgenerator->create_discussion($record); + + $record = new stdClass(); + $record->course = $course1->id; + $record->userid = $user2->id; + $record->forum = $forum1->id; + $discussion2 = $forumgenerator->create_discussion($record); + + $record = new stdClass(); + $record->course = $course1->id; + $record->userid = $user2->id; + $record->forum = $forum2->id; + $discussion3 = $forumgenerator->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 = $forumgenerator->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; + $discussion1reply2 = $forumgenerator->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. + $isolatedurl = $urlfactory->get_discussion_view_url_from_discussion_id($discussion1reply2->discussion); + $isolatedurl->params(['parent' => $discussion1reply2->id]); + $expectedposts['posts'][] = array( + 'id' => $discussion1reply2->id, + 'discussionid' => $discussion1reply2->discussion, + 'parentid' => $discussion1reply2->parent, + 'hasparent' => true, + 'timecreated' => $discussion1reply2->created, + '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. + 'unread' => null, + 'isdeleted' => false, + 'isprivatereply' => false, + 'haswordcount' => false, + 'wordcount' => null, + 'author'=> $exporteduser3, + 'attachments' => [], + 'tags' => [], + 'html' => [ + 'rating' => null, + 'taglist' => null, + 'authorsubheading' => $forumgenerator->get_author_subheading_html((object)$exporteduser3, $discussion1reply2->created) + ], + 'capabilities' => [ + 'view' => 1, + 'edit' => 0, + 'delete' => 0, + 'split' => 0, + 'reply' => 1, + 'export' => 0, + 'controlreadstatus' => 0 + ], + 'urls' => [ + 'view' => $urlfactory->get_view_post_url_from_post_id($discussion1reply2->discussion, $discussion1reply2->id), + 'viewisolated' => $isolatedurl->out(false), + 'viewparent' => $urlfactory->get_view_post_url_from_post_id($discussion1reply2->discussion, $discussion1reply2->parent), + 'edit' => null, + 'delete' =>null, + 'split' => null, + 'reply' => (new moodle_url('/mod/forum/post.php#mformforum', [ + 'reply' => $discussion1reply2->id + ]))->out(false), + 'export' => null, + 'markasread' => null, + 'markasunread' => null, + 'discuss' => $urlfactory->get_discussion_view_url_from_discussion_id($discussion1reply2->discussion), + ], + ); + + + $isolatedurl = $urlfactory->get_discussion_view_url_from_discussion_id($discussion1reply1->discussion); + $isolatedurl->params(['parent' => $discussion1reply1->id]); + $expectedposts['posts'][] = array( + 'id' => $discussion1reply1->id, + 'discussionid' => $discussion1reply1->discussion, + 'parentid' => $discussion1reply1->parent, + 'hasparent' => true, + 'timecreated' => $discussion1reply1->created, + '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. + 'unread' => null, + 'isdeleted' => false, + 'isprivatereply' => false, + 'haswordcount' => false, + 'wordcount' => null, + 'author'=> $exporteduser2, + 'attachments' => [], + 'tags' => [], + 'html' => [ + 'rating' => null, + 'taglist' => null, + 'authorsubheading' => $forumgenerator->get_author_subheading_html((object)$exporteduser2, $discussion1reply1->created) + ], + 'capabilities' => [ + 'view' => 1, + 'edit' => 0, + 'delete' => 0, + 'split' => 0, + 'reply' => 1, + 'export' => 0, + 'controlreadstatus' => 0 + ], + 'urls' => [ + 'view' => $urlfactory->get_view_post_url_from_post_id($discussion1reply1->discussion, $discussion1reply1->id), + 'viewisolated' => $isolatedurl->out(false), + 'viewparent' => $urlfactory->get_view_post_url_from_post_id($discussion1reply1->discussion, $discussion1reply1->parent), + 'edit' => null, + 'delete' =>null, + 'split' => null, + 'reply' => (new moodle_url('/mod/forum/post.php#mformforum', [ + 'reply' => $discussion1reply1->id + ]))->out(false), + 'export' => null, + 'markasread' => null, + 'markasunread' => null, + 'discuss' => $urlfactory->get_discussion_view_url_from_discussion_id($discussion1reply1->discussion), + ], + ); + + // Test a discussion with two additional posts (total 3 posts). + $posts = mod_forum_external::get_discussion_posts($discussion1->id, 'modified', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); + $this->assertEquals(3, count($posts['posts'])); + + // 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_discussion_posts($discussion2->id, 'modified', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_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_discussion_posts($discussion1->id, 'modified', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); + foreach ($posts['posts'] as $post) { + $this->assertNull($post['unread']); + } + + // 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_discussion_posts($discussion3->id, 'modified', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_discussion_posts_returns(), $posts); + foreach ($posts['posts'] as $post) { + $this->assertFalse($post['unread']); + } + + // 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 */ diff --git a/mod/forum/tests/generator/lib.php b/mod/forum/tests/generator/lib.php index 597f64ce0d4..a2134d7332f 100644 --- a/mod/forum/tests/generator/lib.php +++ b/mod/forum/tests/generator/lib.php @@ -350,4 +350,22 @@ class mod_forum_generator extends testing_module_generator { } return $post; } + + /** + * Extracted from exporter/post.php + * + * Get the HTML to display as a subheading in a post. + * + * @param stdClass $exportedauthor The exported author object + * @param int $timecreated The post time created timestamp if it's to be displayed + * @return string + */ + public function get_author_subheading_html(stdClass $exportedauthor, int $timecreated) : string { + $fullname = $exportedauthor->fullname; + $profileurl = $exportedauthor->urls['profile'] ?? null; + $formatteddate = userdate($timecreated, get_string('strftimedaydatetime', 'core_langconfig')); + $name = $profileurl ? "{$fullname}" : $fullname; + $date = ""; + return get_string('bynameondate', 'mod_forum', ['name' => $name, 'date' => $date]); + } }