diff --git a/admin/tool/generator/tests/maketestcourse_test.php b/admin/tool/generator/tests/maketestcourse_test.php index 433736a1827..975608fb29f 100644 --- a/admin/tool/generator/tests/maketestcourse_test.php +++ b/admin/tool/generator/tests/maketestcourse_test.php @@ -158,7 +158,7 @@ final class maketestcourse_test extends \advanced_testcase { // Users that started discussions are the same. $forums = $modinfo->get_instances_of('forum'); - $discussions = forum_get_discussions(reset($forums), 'd.timemodified ASC'); + $discussions = forum_get_discussions(reset($forums), 'd.timemodified ASC, d.id ASC'); $lastusernumber = 0; $discussionstarters = array(); foreach ($discussions as $discussion) { diff --git a/mod/forum/classes/local/vaults/post.php b/mod/forum/classes/local/vaults/post.php index 7cf6c53bedf..186457a73b5 100644 --- a/mod/forum/classes/local/vaults/post.php +++ b/mod/forum/classes/local/vaults/post.php @@ -111,7 +111,7 @@ class post extends db_table_vault { stdClass $user, int $discussionid, bool $canseeprivatereplies, - string $orderby = 'created ASC' + string $orderby = 'created ASC, id ASC' ): array { return $this->get_from_discussion_ids($user, [$discussionid], $canseeprivatereplies, $orderby); } @@ -232,7 +232,7 @@ class post extends db_table_vault { stdClass $user, post_entity $post, bool $canseeprivatereplies, - string $orderby = 'created ASC' + string $orderby = 'created ASC, id ASC' ): array { $alias = $this->get_table_alias(); @@ -439,19 +439,15 @@ class post extends db_table_vault { ] = $this->get_private_reply_sql($user, $canseeprivatereplies, "mp"); $sql = " - SELECT posts.* - FROM {" . self::TABLE . "} posts + SELECT p.* + FROM {" . self::TABLE . "} p 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"; + SELECT mp.discussion, mp.created, MAX(mp.id) AS latestpostid + FROM {" . self::TABLE . "} mp + WHERE mp.discussion {$insql} {$privatewhere} + GROUP BY mp.discussion, mp.created + HAVING mp.created = MAX(mp.created) + ) lp on lp.discussion = p.discussion AND lp.latestpostid = p.id"; $records = $this->get_db()->get_records_sql($sql, array_merge($params, $privateparams)); $entities = $this->transform_db_records_to_entities($records); @@ -501,13 +497,14 @@ class post extends db_table_vault { $sql = " SELECT p.* - FROM {" . self::TABLE . "} p - JOIN ( - SELECT mp.discussion, MIN(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"; + FROM {" . self::TABLE . "} p + JOIN ( + SELECT mp.discussion, mp.created, MIN(mp.id) AS firstpostid + FROM {" . self::TABLE . "} mp + WHERE mp.discussion {$insql} + GROUP BY mp.discussion, mp.created + HAVING mp.created = MIN(mp.created) + ) fp ON fp.discussion = p.discussion AND fp.firstpostid = p.id"; $records = $this->get_db()->get_records_sql($sql, $params); return $this->transform_db_records_to_entities($records); @@ -526,7 +523,7 @@ class post extends db_table_vault { int $discussionid, int $userid, bool $canseeprivatereplies, - string $orderby = 'created ASC' + string $orderby = 'created ASC, id ASC' ): array { $user = $this->get_db()->get_record('user', ['id' => (int)$userid], '*', IGNORE_MISSING); diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index 61b19525c4d..26e632e7aac 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -226,6 +226,11 @@ class mod_forum_external extends external_api { 'allowed values are: ' . implode(',', $directionallowedvalues)); } + $orderbysql = "{$sortby} {$sortdirection}"; + if (!empty($sortby) && $sortby != 'id') { + $orderbysql .= ", id {$sortdirection}"; + } + $managerfactory = mod_forum\local\container::get_manager_factory(); $capabilitymanager = $managerfactory->get_capability_manager($forum); @@ -234,7 +239,7 @@ class mod_forum_external extends external_api { $USER, $discussion->get_id(), $capabilitymanager->can_view_any_private_reply($USER), - "{$sortby} {$sortdirection}" + $orderbysql ); $builderfactory = mod_forum\local\container::get_builder_factory(); diff --git a/mod/forum/lib.php b/mod/forum/lib.php index ff9485ef4b4..9474f98498d 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -1702,6 +1702,9 @@ function forum_get_discussions($cm, $forumsort="", $fullpost=true, $unused=-1, $ if (empty($forumsort)) { $forumsort = forum_get_default_sort_order(); } + if (!str_contains($forumsort, 'id')) { + $forumsort .= ', d.id DESC'; + } if (empty($fullpost)) { $postdata = "p.id, p.subject, p.modified, p.discussion, p.userid, p.created"; } else { @@ -1737,7 +1740,7 @@ function forum_get_discussions($cm, $forumsort="", $fullpost=true, $unused=-1, $ $umtable WHERE d.forum = ? AND p.parent = 0 $timelimit $groupselect $updatedsincesql - ORDER BY $forumsort, d.id DESC"; + ORDER BY $forumsort"; return $DB->get_records_sql($sql, $params, $limitfrom, $limitnum); } @@ -4831,7 +4834,7 @@ function forum_discussion_update_last_post($discussionid) { $sql = "SELECT id, userid, modified FROM {forum_posts} WHERE discussion=? - ORDER BY modified DESC"; + ORDER BY modified DESC, id DESC"; // Lets go find the last post if (($lastposts = $DB->get_records_sql($sql, array($discussionid), 0, 1))) { diff --git a/mod/forum/tests/generator/lib.php b/mod/forum/tests/generator/lib.php index ab66bbfd49d..5568404e37a 100644 --- a/mod/forum/tests/generator/lib.php +++ b/mod/forum/tests/generator/lib.php @@ -254,7 +254,7 @@ class mod_forum_generator extends testing_module_generator { $this->forumpostcount++; // Variable to store time. - $time = time() + $this->forumpostcount; + $time = time(); $record = (array) $record; diff --git a/mod/forum/tests/vaults_discussion_list_test.php b/mod/forum/tests/vaults_discussion_list_test.php index 14cdefa531a..05268c84fca 100644 --- a/mod/forum/tests/vaults_discussion_list_test.php +++ b/mod/forum/tests/vaults_discussion_list_test.php @@ -258,27 +258,31 @@ final class vaults_discussion_list_test extends \advanced_testcase { $forum = $datagenerator->create_module('forum', ['course' => $course->id]); $this->getDataGenerator()->enrol_user($user->id, $course->id, null, 'manual'); - $this->assertEquals([], $vault->get_from_forum_id($forum->id, true, true, - null, 0, 0, $user)); + $this->assertEquals([], $vault->get_from_forum_id_and_group_id($forum->id, [1, 2, 3], true, $user->id, + null, 0, 0)); $now = time(); - [$discussion1, $post1] = $this->helper_post_to_forum($forum, $user, ['timestart' => $now - 10, 'timemodified' => 1]); - [$discussion2, $post2] = $this->helper_post_to_forum($forum, $user, ['timestart' => $now - 9, 'timemodified' => 2]); - [$hiddendiscussion, $post3] = $this->helper_post_to_forum($forum, $user, ['timestart' => $now + 10, 'timemodified' => 3]); + [$discussion1, $post1] = $this->helper_post_to_forum($forum, $user, ['timestart' => $now - 10, 'timemodified' => $now + 1]); + [$discussion2, $post2] = $this->helper_post_to_forum($forum, $user, ['timestart' => $now - 9, 'timemodified' => $now + 2]); + [$hiddendiscussion, $post3] = $this->helper_post_to_forum( + $forum, + $user, + ['timestart' => $now + 10, 'timemodified' => $now + 3] + ); [$groupdiscussion1, $post4] = $this->helper_post_to_forum( $forum, $user, - ['timestart' => $now - 8, 'timemodified' => 4, 'groupid' => 1] + ['timestart' => $now - 8, 'timemodified' => $now + 4, 'groupid' => 1] ); [$groupdiscussion2, $post5] = $this->helper_post_to_forum( $forum, $user, - ['timestart' => $now - 7, 'timemodified' => 5, 'groupid' => 2] + ['timestart' => $now - 7, 'timemodified' => $now + 5, 'groupid' => 2] ); [$hiddengroupdiscussion, $post6] = $this->helper_post_to_forum( $forum, $user, - ['timestart' => $now + 11, 'timemodified' => 6, 'groupid' => 3] + ['timestart' => $now + 11, 'timemodified' => $now + 6, 'groupid' => 3] ); $summaries = array_values($vault->get_from_forum_id_and_group_id($forum->id, [1, 2, 3], true, @@ -356,10 +360,10 @@ final class vaults_discussion_list_test extends \advanced_testcase { $summaries = array_values($vault->get_from_forum_id_and_group_id($forum->id, [1, 2, 3], true, $user->id, $vault::SORTORDER_LASTPOST_DESC, 0, 0)); $this->assertCount(6, $summaries); - $this->assertEquals($groupdiscussion2->id, $summaries[0]->get_discussion()->get_id()); - $this->assertEquals($groupdiscussion1->id, $summaries[1]->get_discussion()->get_id()); - $this->assertEquals($hiddendiscussion->id, $summaries[2]->get_discussion()->get_id()); - $this->assertEquals($hiddengroupdiscussion->id, $summaries[3]->get_discussion()->get_id()); + $this->assertEquals($hiddengroupdiscussion->id, $summaries[0]->get_discussion()->get_id()); + $this->assertEquals($hiddendiscussion->id, $summaries[1]->get_discussion()->get_id()); + $this->assertEquals($groupdiscussion2->id, $summaries[2]->get_discussion()->get_id()); + $this->assertEquals($groupdiscussion1->id, $summaries[3]->get_discussion()->get_id()); $this->assertEquals($discussion2->id, $summaries[4]->get_discussion()->get_id()); $this->assertEquals($discussion1->id, $summaries[5]->get_discussion()->get_id()); @@ -369,10 +373,10 @@ final class vaults_discussion_list_test extends \advanced_testcase { $this->assertCount(6, $summaries); $this->assertEquals($discussion1->id, $summaries[0]->get_discussion()->get_id()); $this->assertEquals($discussion2->id, $summaries[1]->get_discussion()->get_id()); - $this->assertEquals($hiddengroupdiscussion->id, $summaries[2]->get_discussion()->get_id()); - $this->assertEquals($hiddendiscussion->id, $summaries[3]->get_discussion()->get_id()); - $this->assertEquals($groupdiscussion1->id, $summaries[4]->get_discussion()->get_id()); - $this->assertEquals($groupdiscussion2->id, $summaries[5]->get_discussion()->get_id()); + $this->assertEquals($groupdiscussion1->id, $summaries[2]->get_discussion()->get_id()); + $this->assertEquals($groupdiscussion2->id, $summaries[3]->get_discussion()->get_id()); + $this->assertEquals($hiddendiscussion->id, $summaries[4]->get_discussion()->get_id()); + $this->assertEquals($hiddengroupdiscussion->id, $summaries[5]->get_discussion()->get_id()); // Sort discussions by replies DESC. $summaries = array_values($vault->get_from_forum_id_and_group_id($forum->id, [1, 2, 3], true, @@ -422,27 +426,27 @@ final class vaults_discussion_list_test extends \advanced_testcase { $this->pin_discussion($discussion1); $this->pin_discussion($hiddendiscussion); - $summaries = array_values($vault->get_from_forum_id($forum->id, true, null, + $summaries = array_values($vault->get_from_forum_id_and_group_id($forum->id, [1, 2, 3], true, null, $vault::SORTORDER_LASTPOST_DESC, 0, 0)); $this->assertCount(6, $summaries); $this->assertEquals($hiddendiscussion->id, $summaries[0]->get_discussion()->get_id()); $this->assertEquals($discussion1->id, $summaries[1]->get_discussion()->get_id()); - $this->assertEquals($groupdiscussion2->id, $summaries[2]->get_discussion()->get_id()); - $this->assertEquals($groupdiscussion1->id, $summaries[3]->get_discussion()->get_id()); - $this->assertEquals($hiddengroupdiscussion->id, $summaries[4]->get_discussion()->get_id()); + $this->assertEquals($hiddengroupdiscussion->id, $summaries[2]->get_discussion()->get_id()); + $this->assertEquals($groupdiscussion2->id, $summaries[3]->get_discussion()->get_id()); + $this->assertEquals($groupdiscussion1->id, $summaries[4]->get_discussion()->get_id()); $this->assertEquals($discussion2->id, $summaries[5]->get_discussion()->get_id()); - $summaries = array_values($vault->get_from_forum_id($forum->id, true, null, + $summaries = array_values($vault->get_from_forum_id_and_group_id($forum->id, [1, 2, 3], true, null, $vault::SORTORDER_LASTPOST_ASC, 0, 0)); $this->assertCount(6, $summaries); $this->assertEquals($discussion1->id, $summaries[0]->get_discussion()->get_id()); $this->assertEquals($hiddendiscussion->id, $summaries[1]->get_discussion()->get_id()); $this->assertEquals($discussion2->id, $summaries[2]->get_discussion()->get_id()); - $this->assertEquals($hiddengroupdiscussion->id, $summaries[3]->get_discussion()->get_id()); - $this->assertEquals($groupdiscussion1->id, $summaries[4]->get_discussion()->get_id()); - $this->assertEquals($groupdiscussion2->id, $summaries[5]->get_discussion()->get_id()); + $this->assertEquals($groupdiscussion1->id, $summaries[3]->get_discussion()->get_id()); + $this->assertEquals($groupdiscussion2->id, $summaries[4]->get_discussion()->get_id()); + $this->assertEquals($hiddengroupdiscussion->id, $summaries[5]->get_discussion()->get_id()); - $summaries = array_values($vault->get_from_forum_id($forum->id, true, null, + $summaries = array_values($vault->get_from_forum_id_and_group_id($forum->id, [1, 2, 3], true, null, $vault::SORTORDER_REPLIES_DESC, 0, 0)); $this->assertCount(6, $summaries); $this->assertEquals($discussion1->id, $summaries[0]->get_discussion()->get_id());