From 9b8e599868f1a337365d3050f6306231319c0d01 Mon Sep 17 00:00:00 2001 From: Peter Date: Thu, 2 May 2019 13:44:34 +0800 Subject: [PATCH] MDL-65457 mod_forum: Provide latest post's author Provide an updated private replies restriction to latest post Get the latest post's author and time which takes into account private replies. --- .../exported_discussion_summaries.php | 37 ++++++++++-- .../local/exporters/discussion_summaries.php | 8 ++- .../local/exporters/discussion_summary.php | 6 +- .../classes/local/factories/exporter.php | 6 +- mod/forum/classes/local/vaults/post.php | 56 +++++++++++-------- mod/forum/tests/vaults_post_test.php | 55 +++++++++++------- 6 files changed, 112 insertions(+), 56 deletions(-) diff --git a/mod/forum/classes/local/builders/exported_discussion_summaries.php b/mod/forum/classes/local/builders/exported_discussion_summaries.php index 13efbf44333..3687b327e52 100644 --- a/mod/forum/classes/local/builders/exported_discussion_summaries.php +++ b/mod/forum/classes/local/builders/exported_discussion_summaries.php @@ -124,10 +124,16 @@ class exported_discussion_summaries { $groupsbyauthorid = $this->get_author_groups_from_posts($posts, $forum); $replycounts = $postvault->get_reply_count_for_discussion_ids($user, $discussionids, $canseeanyprivatereply); - $latestposts = $postvault->get_latest_post_id_for_discussion_ids($user, $discussionids, $canseeanyprivatereply); - $postauthorids = array_unique(array_reduce($discussions, function($carry, $summary) { + $latestposts = $postvault->get_latest_posts_for_discussion_ids($user, $discussionids, $canseeanyprivatereply); + $latestauthors = $this->get_latest_posts_authors($latestposts); + $latestpostsids = array_map(function($post) { + return $post->get_id(); + }, $latestposts); + + $postauthorids = array_unique(array_reduce($discussions, function($carry, $summary) use ($latestposts){ $firstpostauthorid = $summary->get_first_post_author()->get_id(); - $lastpostauthorid = $summary->get_latest_post_author()->get_id(); + $discussion = $summary->get_discussion(); + $lastpostauthorid = $latestposts[$discussion->get_id()]->get_author_id(); return array_merge($carry, [$firstpostauthorid, $lastpostauthorid]); }, [])); $postauthorcontextids = $this->get_author_context_ids($postauthorids); @@ -149,16 +155,18 @@ class exported_discussion_summaries { $groupsbyauthorid, $replycounts, $unreadcounts, - $latestposts, + $latestpostsids, $postauthorcontextids, - $favourites + $favourites, + $latestauthors ); $exportedposts = (array) $summaryexporter->export($this->renderer); $firstposts = $postvault->get_first_post_for_discussion_ids($discussionids); - array_walk($exportedposts['summaries'], function($summary) use ($firstposts) { + array_walk($exportedposts['summaries'], function($summary) use ($firstposts, $latestposts) { $summary->discussion->times['created'] = (int) $firstposts[$summary->discussion->firstpostid]->created; + $summary->discussion->times['modified'] = (int) $latestposts[$summary->discussion->id]->get_time_created(); }); // Pass the current, preferred sort order for the discussions list. @@ -201,6 +209,23 @@ class exported_discussion_summaries { return $ids; } + /** + * Returns a mapped array of discussionid to the authors of the latest post + * + * @param array $latestposts Mapped array of discussion to latest posts. + * @return array Array of authors mapped to the discussion + */ + private function get_latest_posts_authors($latestposts) { + $authors = $this->vaultfactory->get_author_vault()->get_authors_for_posts($latestposts); + + $mappedauthors = array_reduce($latestposts, function($carry, $item) use ($authors) { + $carry[$item->get_discussion_id()] = $authors[$item->get_author_id()]; + + return $carry; + }, []); + return $mappedauthors; + } + /** * Get the groups details for all groups available to the forum. * @param forum_entity $forum The forum entity diff --git a/mod/forum/classes/local/exporters/discussion_summaries.php b/mod/forum/classes/local/exporters/discussion_summaries.php index e5fbb36820a..47bfb9b1474 100644 --- a/mod/forum/classes/local/exporters/discussion_summaries.php +++ b/mod/forum/classes/local/exporters/discussion_summaries.php @@ -119,12 +119,15 @@ class discussion_summaries extends exporter { protected function get_other_values(renderer_base $output) { $exporteddiscussions = []; $related = $this->related; + $latestauthors = $this->related['latestauthors']; foreach ($this->discussions as $discussion) { $discussionid = $discussion->get_discussion()->get_id(); $replycount = isset($this->discussionreplycount[$discussionid]) ? $this->discussionreplycount[$discussionid] : 0; $unreadcount = isset($this->discussionunreadcount[$discussionid]) ? $this->discussionunreadcount[$discussionid] : 0; $latestpostid = isset($this->latestpostids[$discussionid]) ? $this->latestpostids[$discussionid] : 0; + $latestauthor = $latestauthors[$discussionid] ?? null; + $related['latestauthor'] = $latestauthor; $exporter = new discussion_summary( $discussion, $this->groupsbyid, @@ -133,7 +136,7 @@ class discussion_summaries extends exporter { $unreadcount, $latestpostid, $this->postauthorcontextids[$discussion->get_first_post_author()->get_id()], - $this->postauthorcontextids[$discussion->get_latest_post_author()->get_id()], + $this->postauthorcontextids[$latestauthor->get_id()], $related ); $exporteddiscussions[] = $exporter->export($output); @@ -160,7 +163,8 @@ class discussion_summaries extends exporter { 'capabilitymanager' => 'mod_forum\local\managers\capability', 'urlfactory' => 'mod_forum\local\factories\url', 'user' => 'stdClass', - 'favouriteids' => 'int[]?' + 'favouriteids' => 'int[]?', + 'latestauthors' => 'mod_forum\local\entities\author[]?' ]; } } diff --git a/mod/forum/classes/local/exporters/discussion_summary.php b/mod/forum/classes/local/exporters/discussion_summary.php index 3d009ee222d..f24ffc3e652 100644 --- a/mod/forum/classes/local/exporters/discussion_summary.php +++ b/mod/forum/classes/local/exporters/discussion_summary.php @@ -129,6 +129,7 @@ class discussion_summary extends exporter { $capabilitymanager = $this->related['capabilitymanager']; $forum = $this->related['forum']; $user = $this->related['user']; + $latestpostauthor = $this->related['latestauthor']; $discussion = $this->summary->get_discussion(); $related = (array) (object) $this->related; @@ -154,7 +155,7 @@ class discussion_summary extends exporter { ); $latestpostauthor = new author( - $this->summary->get_latest_post_author(), + $latestpostauthor ?? $this->summary->get_latest_post_author(), $this->latestpostauthorcontextid, [], $capabilitymanager->can_view_post( @@ -189,7 +190,8 @@ class discussion_summary extends exporter { 'capabilitymanager' => 'mod_forum\local\managers\capability', 'urlfactory' => 'mod_forum\local\factories\url', 'user' => 'stdClass', - 'favouriteids' => 'int[]?' + 'favouriteids' => 'int[]?', + 'latestauthor' => 'mod_forum\local\entities\author?' ]; } } diff --git a/mod/forum/classes/local/factories/exporter.php b/mod/forum/classes/local/factories/exporter.php index ae007bcf235..7dd43889862 100644 --- a/mod/forum/classes/local/factories/exporter.php +++ b/mod/forum/classes/local/factories/exporter.php @@ -176,7 +176,8 @@ class exporter { array $discussionunreadcount = [], array $latestpostids = [], array $postauthorcontextids = [], - array $favourites = [] + array $favourites = [], + array $latestauthors = [] ) : discussion_summaries_exporter { return new discussion_summaries_exporter( $discussions, @@ -193,7 +194,8 @@ class exporter { 'capabilitymanager' => $this->managerfactory->get_capability_manager($forum), 'urlfactory' => $this->urlfactory, 'user' => $user, - 'favouriteids' => $favourites + 'favouriteids' => $favourites, + 'latestauthors' => $latestauthors ] ); } diff --git a/mod/forum/classes/local/vaults/post.php b/mod/forum/classes/local/vaults/post.php index 18ef42b0c7e..d1ae66fb0dd 100644 --- a/mod/forum/classes/local/vaults/post.php +++ b/mod/forum/classes/local/vaults/post.php @@ -355,42 +355,51 @@ class post extends db_table_vault { } /** - * Get a mapping of the most recent post in each discussion based on post creation time. + * Get a mapping of the most recent post record in each discussion based on post creation time. * - * @param stdClass $user The user to fetch counts for - * @param int[] $discussionids The list of discussions to fetch counts for - * @param bool $canseeprivatereplies Whether this user can see all private replies or not - * @return int[] The post id of the most recent post for each discussions returned in an associative array + * @param stdClass $user + * @param array $discussionids + * @param bool $canseeprivatereplies + * @return array + * @throws \coding_exception + * @throws \dml_exception */ - public function get_latest_post_id_for_discussion_ids( - stdClass $user, array $discussionids, bool $canseeprivatereplies) : array { - global $CFG; + public function get_latest_posts_for_discussion_ids( + stdClass $user, array $discussionids, bool $canseeprivatereplies) : array { if (empty($discussionids)) { return []; } - $alias = $this->get_table_alias(); list($insql, $params) = $this->get_db()->get_in_or_equal($discussionids, SQL_PARAMS_NAMED); [ 'where' => $privatewhere, 'params' => $privateparams, - ] = $this->get_private_reply_sql($user, $canseeprivatereplies); + ] = $this->get_private_reply_sql($user, $canseeprivatereplies, "mp"); $sql = " - SELECT p.discussion, MAX(p.id) - FROM {" . self::TABLE . "} p - JOIN ( - SELECT mp.discussion, MAX(mp.created) AS created - FROM {" . self::TABLE . "} mp - WHERE mp.discussion {$insql} - GROUP BY mp.discussion - ) lp ON lp.discussion = p.discussion AND lp.created = p.created - WHERE 1 = 1 {$privatewhere} - GROUP BY p.discussion"; + SELECT posts.* + FROM {" . self::TABLE . "} posts + JOIN ( + SELECT p.discussion, MAX(p.id) as latestpostid + FROM {" . self::TABLE . "} p + JOIN ( + SELECT mp.discussion, MAX(mp.created) AS created + FROM {" . self::TABLE . "} mp + WHERE mp.discussion {$insql} {$privatewhere} + GROUP BY mp.discussion + ) lp ON lp.discussion = p.discussion AND lp.created = p.created + GROUP BY p.discussion + ) plp on plp.discussion = posts.discussion AND plp.latestpostid = posts.id"; - return $this->get_db()->get_records_sql_menu($sql, array_merge($params, $privateparams)); + $records = $this->get_db()->get_records_sql($sql, array_merge($params, $privateparams)); + $entities = $this->transform_db_records_to_entities($records); + + return array_reduce($entities, function($carry, $entity) { + $carry[$entity->get_discussion_id()] = $entity; + return $carry; + }, []); } /** @@ -400,11 +409,12 @@ class post extends db_table_vault { * @param bool $canseeprivatereplies Whether this user can see all private replies or not * @return array The SQL WHERE clause, and parameters to use in the SQL. */ - private function get_private_reply_sql(stdClass $user, bool $canseeprivatereplies) { + private function get_private_reply_sql(stdClass $user, bool $canseeprivatereplies, $posttablealias = "p") { $params = []; $privatewhere = ''; if (!$canseeprivatereplies) { - $privatewhere = ' AND (p.privatereplyto = :privatereplyto OR p.userid = :privatereplyfrom OR p.privatereplyto = 0)'; + $privatewhere = " AND ({$posttablealias}.privatereplyto = :privatereplyto OR " . + "{$posttablealias}.userid = :privatereplyfrom OR {$posttablealias}.privatereplyto = 0)"; $params['privatereplyto'] = $user->id; $params['privatereplyfrom'] = $user->id; } diff --git a/mod/forum/tests/vaults_post_test.php b/mod/forum/tests/vaults_post_test.php index 2f24eb3f1dc..80040330ba5 100644 --- a/mod/forum/tests/vaults_post_test.php +++ b/mod/forum/tests/vaults_post_test.php @@ -734,17 +734,18 @@ class mod_forum_vaults_post_testcase extends advanced_testcase { } /** - * Test get_latest_post_id_for_discussion_ids. + * Test get_latest_posts_for_discussion_ids. * - * @covers ::get_latest_post_id_for_discussion_ids + * @covers ::get_latest_posts_for_discussion_ids * @covers :: */ - public function test_get_latest_post_id_for_discussion_ids() { + public function test_get_latest_posts_for_discussion_ids() { $this->resetAfterTest(); $datagenerator = $this->getDataGenerator(); - $user = $datagenerator->create_user(); $course = $datagenerator->create_course(); + [$teacher, $otherteacher] = $this->helper_create_users($course, 2, 'teacher'); + [$user, $user2] = $this->helper_create_users($course, 2, 'student'); $forum = $datagenerator->create_module('forum', ['course' => $course->id]); [$discussion1, $post1] = $this->helper_post_to_forum($forum, $user); $post2 = $this->helper_reply_to_post($post1, $user); @@ -753,43 +754,55 @@ class mod_forum_vaults_post_testcase extends advanced_testcase { [$discussion2, $post5] = $this->helper_post_to_forum($forum, $user); $post6 = $this->helper_reply_to_post($post5, $user); [$discussion3, $post7] = $this->helper_post_to_forum($forum, $user); + $post8 = $this->helper_post_to_discussion($forum, $discussion3, $teacher, [ + 'privatereplyto' => $user->id, + ]); - $ids = $this->vault->get_latest_post_id_for_discussion_ids($user, [$discussion1->id], false); + $ids = $this->vault->get_latest_posts_for_discussion_ids($user, [$discussion1->id], false); $this->assertCount(1, $ids); - $this->assertEquals($post4->id, $ids[$discussion1->id]); + $this->assertEquals($post4->id, $ids[$discussion1->id]->get_id()); - $ids = $this->vault->get_latest_post_id_for_discussion_ids($user, + $ids = $this->vault->get_latest_posts_for_discussion_ids($user, [$discussion1->id, $discussion2->id], false); $this->assertCount(2, $ids); - $this->assertEquals($post4->id, $ids[$discussion1->id]); - $this->assertEquals($post6->id, $ids[$discussion2->id]); + $this->assertEquals($post4->id, $ids[$discussion1->id]->get_id()); + $this->assertEquals($post6->id, $ids[$discussion2->id]->get_id()); - $ids = $this->vault->get_latest_post_id_for_discussion_ids($user, + $ids = $this->vault->get_latest_posts_for_discussion_ids($user, [$discussion1->id, $discussion2->id, $discussion3->id], false); $this->assertCount(3, $ids); - $this->assertEquals($post4->id, $ids[$discussion1->id]); - $this->assertEquals($post6->id, $ids[$discussion2->id]); - $this->assertEquals($post7->id, $ids[$discussion3->id]); + $this->assertEquals($post4->id, $ids[$discussion1->id]->get_id()); + $this->assertEquals($post6->id, $ids[$discussion2->id]->get_id()); + $this->assertEquals($post8->id, $ids[$discussion3->id]->get_id()); - $ids = $this->vault->get_latest_post_id_for_discussion_ids($user, [ + // Checks the user who doesn't have access to the private reply. + $ids = $this->vault->get_latest_posts_for_discussion_ids($user2, + [$discussion1->id, $discussion2->id, $discussion3->id], false); + $this->assertCount(3, $ids); + $this->assertEquals($post4->id, $ids[$discussion1->id]->get_id()); + $this->assertEquals($post6->id, $ids[$discussion2->id]->get_id()); + $this->assertEquals($post7->id, $ids[$discussion3->id]->get_id()); + + // Checks the user with the private reply to. + $ids = $this->vault->get_latest_posts_for_discussion_ids($user, [ $discussion1->id, $discussion2->id, $discussion3->id, $discussion3->id + 1000 ], false); $this->assertCount(3, $ids); - $this->assertEquals($post4->id, $ids[$discussion1->id]); - $this->assertEquals($post6->id, $ids[$discussion2->id]); - $this->assertEquals($post7->id, $ids[$discussion3->id]); + $this->assertEquals($post4->id, $ids[$discussion1->id]->get_id()); + $this->assertEquals($post6->id, $ids[$discussion2->id]->get_id()); + $this->assertEquals($post8->id, $ids[$discussion3->id]->get_id()); } /** - * Test get_latest_post_id_for_discussion_ids when no discussion ids were provided. + * Test get_latest_posts_for_discussion_ids when no discussion ids were provided. * - * @covers ::get_latest_post_id_for_discussion_ids + * @covers ::get_latest_posts_for_discussion_ids * @covers :: */ - public function test_get_latest_post_id_for_discussion_ids_empty() { + public function test_get_latest_posts_for_discussion_ids_empty() { $this->resetAfterTest(); $datagenerator = $this->getDataGenerator(); @@ -797,7 +810,7 @@ class mod_forum_vaults_post_testcase extends advanced_testcase { $course = $datagenerator->create_course(); $forum = $datagenerator->create_module('forum', ['course' => $course->id]); - $this->assertEquals([], $this->vault->get_latest_post_id_for_discussion_ids($user, [], false)); + $this->assertEquals([], $this->vault->get_latest_posts_for_discussion_ids($user, [], false)); } /**