From 0f91fef57f6e90e3b75926c3e4cd2091f3ad9264 Mon Sep 17 00:00:00 2001 From: albertlast Date: Wed, 30 Sep 2026 20:34:24 +0200 Subject: [PATCH] Lists ten topic starters in the statistics centre The top topic starters are counted from the topics table, twenty of them, and their names looked up afterwards. Each was placed by its rank and those ranked after index 10 were skipped, which kept eleven. A starter with no member to name, such as topics credited to a guest or to a deleted member, kept its place empty, so the two faults could hide each other. Now the members found are taken in rank order and the first ten kept. Co-Authored-By: Claude Opus 5.5 Signed-off-by: albertlast --- Sources/Actions/Stats.php | 11 +- tests/Integration/Http/StatsStartersTest.php | 179 +++++++++++++++++++ 2 files changed, 184 insertions(+), 6 deletions(-) create mode 100644 tests/Integration/Http/StatsStartersTest.php diff --git a/Sources/Actions/Stats.php b/Sources/Actions/Stats.php index 386efc2e396..0bf4ce0ead8 100644 --- a/Sources/Actions/Stats.php +++ b/Sources/Actions/Stats.php @@ -517,11 +517,6 @@ public function execute(): void while ($row_members = Db::$db->fetch_assoc($members_result)) { $i = array_search($row_members['id_member'], array_keys($members)); - // skip all not top 10 - if ($i > 10) { - continue; - } - Utils::$context['stats_blocks']['starters'][$i] = [ 'name' => $row_members['real_name'], 'id' => $row_members['id_member'], @@ -534,9 +529,13 @@ public function execute(): void $max_num = $members[$row_members['id_member']]; } } - ksort(Utils::$context['stats_blocks']['starters']); Db::$db->free_result($members_result); + // Starters with no member to list, such as guests and deleted members, + // were counted but not found, so the top ten are the first ten found. + ksort(Utils::$context['stats_blocks']['starters']); + Utils::$context['stats_blocks']['starters'] = \array_slice(Utils::$context['stats_blocks']['starters'], 0, 10); + foreach (Utils::$context['stats_blocks']['starters'] as $i => $topic) { Utils::$context['stats_blocks']['starters'][$i]['percent'] = round(($topic['num'] * 100) / $max_num); Utils::$context['stats_blocks']['starters'][$i]['num'] = Lang::numberFormat(Utils::$context['stats_blocks']['starters'][$i]['num']); diff --git a/tests/Integration/Http/StatsStartersTest.php b/tests/Integration/Http/StatsStartersTest.php new file mode 100644 index 00000000000..601e1d62571 --- /dev/null +++ b/tests/Integration/Http/StatsStartersTest.php @@ -0,0 +1,179 @@ +makeMember('Stats Starter ' . $i); + + $this->makeTopicStartedBy($member_id, 0); + $this->makeTopicStartedBy($member_id, 1); + } + + $page = $this->fetch('?action=stats'); + + $starters = $page->xpath('//div[contains(@class, "half_content")][.//span[contains(@class, "starters")]]//dl/dt'); + + $this->assertSame(10, $starters->length, 'the top ten starters list the wrong number. The forum said: ' . $page->errorText()); + + $this->assertNoErrorsLogged('the stats page logged something.' . "\n"); + } + + /** + * Topics credited to nobody, or to a member since deleted, are counted + * too, but there is no name to list. Such a starter must not take one of + * the ten places and leave it empty. + */ + public function testStartersWithNoMemberDoNotTakeAPlace(): void + { + for ($i = 1; $i <= 11; $i++) { + $member_id = $this->makeMember('Stats Starter ' . $i); + + $this->makeTopicStartedBy($member_id, 0); + $this->makeTopicStartedBy($member_id, 1); + } + + // Ranked first, with more topics than anybody, but not a member. + for ($n = 0; $n < 3; $n++) { + $this->makeTopicStartedBy(0, $n); + } + + $page = $this->fetch('?action=stats'); + + $starters = $page->xpath('//div[contains(@class, "half_content")][.//span[contains(@class, "starters")]]//dl/dt'); + + $this->assertSame(10, $starters->length, 'the top ten starters list the wrong number. The forum said: ' . $page->errorText()); + + $this->assertNoErrorsLogged('the stats page logged something.' . "\n"); + } + + /****************** + * Internal methods + ******************/ + + protected function tearDown(): void + { + if ($this->topic_ids !== []) { + Db::$db->query( + 'DELETE FROM {db_prefix}topics + WHERE id_topic IN ({array_int:topics})', + ['topics' => $this->topic_ids], + ); + + $this->topic_ids = []; + } + + 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. + * + * @param string $name Their name. + * @return int The member's id. + */ + private function makeMember(string $name): int + { + $login = 'starter_' . bin2hex(random_bytes(4)); + + $id = (int) Db::$db->insert( + 'insert', + '{db_prefix}members', + [ + 'member_name' => 'string', + 'real_name' => 'string', + 'email_address' => 'string', + 'is_activated' => 'int', + 'buddy_list' => 'string', + 'signature' => 'string', + 'ignore_boards' => 'string', + ], + [[$login, $name, $login . '@example.com', 1, '', '', '']], + ['id_member'], + 1, + ); + + $this->member_ids[] = $id; + + return $id; + } + + /** + * Makes a topic row credited to a member. The list only counts topics, so + * it needs no messages, but each topic needs message ids of its own: the + * last message and the board are a unique key. + * + * @param int $member_id The member who started it. + * @param int $n Which of the member's topics this is, from 0. + */ + private function makeTopicStartedBy(int $member_id, int $n): void + { + $message_id = 900000000 + $member_id * 10 + $n; + + $this->topic_ids[] = (int) Db::$db->insert( + 'insert', + '{db_prefix}topics', + [ + 'id_board' => 'int', + 'id_first_msg' => 'int', + 'id_last_msg' => 'int', + 'id_member_started' => 'int', + 'id_member_updated' => 'int', + 'approved' => 'int', + ], + [[1, $message_id, $message_id, $member_id, $member_id, 1]], + ['id_topic'], + 1, + ); + } +}