MDL-22077 mod_forum: external unit tests and vault updates
authorPeter <peter@moodle.com>
Wed, 3 Apr 2019 02:46:11 +0000 (10:46 +0800)
committerPeter <peter@moodle.com>
Thu, 4 Apr 2019 03:56:01 +0000 (11:56 +0800)
* Unit tests for external functions
* Use the new dml_table to fetch a table's columns

mod/forum/classes/local/builders/exported_discussion_summaries.php
mod/forum/classes/local/exporters/group.php
mod/forum/classes/local/factories/builder.php
mod/forum/classes/local/vaults/forum.php
mod/forum/classes/privacy/provider.php
mod/forum/deprecatedlib.php
mod/forum/externallib.php
mod/forum/lang/en/forum.php
mod/forum/post.php
mod/forum/tests/externallib_test.php
mod/forum/tests/generator/lib.php

index fd430a3..ba1e0aa 100644 (file)
@@ -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();
     }
 
     /**
index a639a79..cbc0192 100644 (file)
@@ -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;
index ef624b8..2a71d5f 100644 (file)
@@ -105,8 +105,7 @@ class builder {
             $this->legacydatamapperfactory,
             $this->exporterfactory,
             $this->vaultfactory,
-            $this->managerfactory,
-            $this->managerfactory->get_rating_manager()
+            $this->managerfactory
         );
     }
 }
index e8b8347..853e87d 100644 (file)
@@ -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";
index da48048..892196d 100644 (file)
@@ -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.
index 4621e79..f884344 100644 (file)
@@ -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;
index 3435877..ba39260 100644 (file)
@@ -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;
index 74ef995..05c95c5 100644 (file)
@@ -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';
index 9e606ce..4daa039 100644 (file)
@@ -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();
index 65685bb..8fa9dce 100644 (file)
@@ -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
      */
index 597f64c..a2134d7 100644 (file)
@@ -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 ? "<a href=\"{$profileurl}\">{$fullname}</a>" : $fullname;
+        $date = "<time>{$formatteddate}</time>";
+        return get_string('bynameondate', 'mod_forum', ['name' => $name, 'date' => $date]);
+    }
 }