From 40a2471b62e0f05573747dfe9153278198d728c6 Mon Sep 17 00:00:00 2001 From: albertlast Date: Wed, 30 Sep 2026 20:20:00 +0200 Subject: [PATCH] Ranks ties in the stats top tens by id The statistics centre ordered each top ten by its count alone, so members, boards or topics with the same count came in whatever order the database returned them, which differs between the engines and, on PostgreSQL, between one request and the next. Each query now breaks ties by id, so what came first ranks first. The likes lists are left as they are; #8187 rewrites those queries. Co-Authored-By: Claude Opus 5.5 Signed-off-by: albertlast --- Sources/Actions/Stats.php | 16 ++-- tests/Integration/Http/StatsTest.php | 118 +++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 8 deletions(-) create mode 100644 tests/Integration/Http/StatsTest.php diff --git a/Sources/Actions/Stats.php b/Sources/Actions/Stats.php index 386efc2e396..f4cae2c2920 100644 --- a/Sources/Actions/Stats.php +++ b/Sources/Actions/Stats.php @@ -265,7 +265,7 @@ public function execute(): void 'SELECT id_member, real_name, posts FROM {db_prefix}members WHERE posts > {int:no_posts} - ORDER BY posts DESC + ORDER BY posts DESC, id_member LIMIT 10', [ 'no_posts' => 0, @@ -301,7 +301,7 @@ public function execute(): void WHERE {query_see_board}' . (!empty(Config::$modSettings['recycle_enable']) && Config::$modSettings['recycle_board'] > 0 ? ' AND b.id_board != {int:recycle_board}' : '') . ' AND b.redirect = {string:blank_redirect} - ORDER BY num_posts DESC + ORDER BY num_posts DESC, id_board LIMIT 10', [ 'recycle_board' => Config::$modSettings['recycle_board'], @@ -338,7 +338,7 @@ public function execute(): void FROM {db_prefix}topics WHERE num_replies != {int:no_replies}' . (Config::$modSettings['postmod_active'] ? ' AND approved = {int:is_approved}' : '') . ' - ORDER BY num_replies DESC + ORDER BY num_replies DESC, id_topic LIMIT 100', [ 'no_replies' => 0, @@ -365,7 +365,7 @@ public function execute(): void WHERE {query_see_board}' . (!empty($topic_ids) ? ' AND t.id_topic IN ({array_int:topic_list})' : (Config::$modSettings['postmod_active'] ? ' AND t.approved = {int:is_approved}' : '')) . ' - ORDER BY t.num_replies DESC + ORDER BY t.num_replies DESC, t.id_topic LIMIT 10', [ 'topic_list' => $topic_ids, @@ -410,7 +410,7 @@ public function execute(): void 'SELECT id_topic FROM {db_prefix}topics WHERE num_views != {int:no_views} - ORDER BY num_views DESC + ORDER BY num_views DESC, id_topic LIMIT 100', [ 'no_views' => 0, @@ -436,7 +436,7 @@ public function execute(): void WHERE {query_see_board}' . (!empty($topic_ids) ? ' AND t.id_topic IN ({array_int:topic_list})' : (Config::$modSettings['postmod_active'] ? ' AND t.approved = {int:is_approved}' : '')) . ' - ORDER BY t.num_views DESC + ORDER BY t.num_views DESC, t.id_topic LIMIT 10', [ 'topic_list' => $topic_ids, @@ -482,7 +482,7 @@ public function execute(): void FROM {db_prefix}topics' . (!empty(Config::$modSettings['recycle_enable']) && Config::$modSettings['recycle_board'] > 0 ? ' WHERE id_board != {int:recycle_board}' : '') . ' GROUP BY id_member_started - ORDER BY hits DESC + ORDER BY hits DESC, id_member_started LIMIT 20', [ 'recycle_board' => Config::$modSettings['recycle_board'], @@ -549,7 +549,7 @@ public function execute(): void FROM {db_prefix}members WHERE is_activated = {int:is_activated}' . (!empty($temp) ? ' AND id_member IN ({array_int:member_list_cached})' : '') . ' - ORDER BY total_time_logged_in DESC + ORDER BY total_time_logged_in DESC, id_member LIMIT 20', [ 'member_list_cached' => $temp, diff --git a/tests/Integration/Http/StatsTest.php b/tests/Integration/Http/StatsTest.php new file mode 100644 index 00000000000..42dc0f73027 --- /dev/null +++ b/tests/Integration/Http/StatsTest.php @@ -0,0 +1,118 @@ +makeMember('Stats Tied First', 1000000); + $second = $this->makeMember('Stats Tied Second', 1000000); + + // Rewriting the earlier row sends it to the end of the table on PostgreSQL. + Db::$db->query( + 'UPDATE {db_prefix}members + SET posts = posts + WHERE id_member = {int:member}', + ['member' => $first], + ); + + $page = $this->fetch('?action=stats'); + $text = $page->text(); + + $this->assertStringContainsString('Stats Tied First', $text, 'the tied members are not in the top ten. The forum said: ' . $page->errorText()); + $this->assertLessThan( + strpos($text, 'Stats Tied Second'), + strpos($text, 'Stats Tied First'), + 'the member who joined later is ranked first', + ); + + $this->assertNoErrorsLogged('the stats page logged something.' . "\n"); + } + + /****************** + * Internal methods + ******************/ + + protected function tearDown(): void + { + if ($this->member_ids !== []) { + Db::$db->query( + 'DELETE FROM {db_prefix}members + WHERE id_member IN ({array_int:members})', + ['members' => $this->member_ids], + ); + + $this->member_ids = []; + } + + parent::tearDown(); + } + + /** + * Makes a member with a given number of posts. + * + * @param string $name Their name. + * @param int $posts How many posts they have made. + * @return int The member's id. + */ + private function makeMember(string $name, int $posts): int + { + $login = 'stats_' . bin2hex(random_bytes(4)); + + $id = (int) Db::$db->insert( + 'insert', + '{db_prefix}members', + [ + 'member_name' => 'string', + 'real_name' => 'string', + 'email_address' => 'string', + 'posts' => 'int', + 'is_activated' => 'int', + 'buddy_list' => 'string', + 'signature' => 'string', + 'ignore_boards' => 'string', + ], + [[$login, $name, $login . '@example.com', $posts, 1, '', '', '']], + ['id_member'], + 1, + ); + + $this->member_ids[] = $id; + + return $id; + } +}