diff --git a/core/recenttopics.php b/core/recenttopics.php index 68dc026..74fca51 100644 --- a/core/recenttopics.php +++ b/core/recenttopics.php @@ -332,8 +332,23 @@ public function display_recent_topics($tpl_loopname = 'recent_topics', $context $this->excluded_topics = explode(',', $this->config['rt_anti_topics']); $min_topic_level = $this->config['rt_min_topic_level']; + $this->sort_topics = $this->config['rt_sort_start_time'] ? 'topic_time' : 'topic_last_post_time'; + // if user can set recent topic sorting order and it is set then use the preference + if ($this->auth->acl_get('u_rt_sort_start_time') && isset($this->user->data['user_rt_sort_start_time'])) + { + $this->sort_topics = $this->user->data['user_rt_sort_start_time'] ? 'topic_time' : 'topic_last_post_time'; + } + + $this->get_forum_list(); + // No forums to display + if (count($this->forum_ids) == 0) + { + return; + } + //limit number of pages to be shown // compute as product of topics per page and max number of pages. + // The count needs the forum list above, or it only counts m_approve forums (issue #195). $this->total_topics_limit = 0; if ((int) $this->config['rt_page_number'] == 0) { @@ -352,20 +367,6 @@ public function display_recent_topics($tpl_loopname = 'recent_topics', $context } - $this->sort_topics = $this->config['rt_sort_start_time'] ? 'topic_time' : 'topic_last_post_time'; - // if user can set recent topic sorting order and it is set then use the preference - if ($this->auth->acl_get('u_rt_sort_start_time') && isset($this->user->data['user_rt_sort_start_time'])) - { - $this->sort_topics = $this->user->data['user_rt_sort_start_time'] ? 'topic_time' : 'topic_last_post_time'; - } - - $this->get_forum_list(); - // No forums to display - if (count($this->forum_ids) == 0) - { - return; - } - $topics_count = $this->get_topic_list(); if (count($this->topic_list) == 0) diff --git a/event/ucp_listener.php b/event/ucp_listener.php index 7d032bc..285416a 100644 --- a/event/ucp_listener.php +++ b/event/ucp_listener.php @@ -100,7 +100,7 @@ public static function getSubscribedEvents() return array( 'core.ucp_prefs_view_data' => 'ucp_prefs_get_data', 'core.ucp_prefs_view_update_data' => 'ucp_prefs_set_data', - 'core.ucp_register_data_after' => 'ucp_register_set_data' + 'core.ucp_register_register_after' => 'ucp_register_set_data' ); } @@ -277,8 +277,9 @@ public function ucp_prefs_set_data($event) /** * set a newly registered account's Recent Topics preferences from default. * - * Fired after the user row has been inserted, so the user_rt_* columns are written by a second - * UPDATE of our own rather than merged into an existing $sql_ary. + * Listens to core.ucp_register_register_after, which fires after user_add() has inserted the row + * and carries the new user_id, so the user_rt_* columns are written by a second UPDATE of our own. + * core.ucp_register_data_after fires during form validation with no user_id (issue #196). * * @param \phpbb\event\data $event Event object; reads ['user_id'] * @return void diff --git a/tests/core/topic_count_test.php b/tests/core/topic_count_test.php new file mode 100644 index 0000000..aa79340 --- /dev/null +++ b/tests/core/topic_count_test.php @@ -0,0 +1,118 @@ +createMock(\phpbb\content_visibility::class); + $content_visibility->method('get_forums_visibility_sql') + ->willReturnCallback(function ($mode, $forum_ids) use (&$visibility_forum_ids) { + $visibility_forum_ids[] = $forum_ids; + return '1=1'; + }); + + $auth = $this->createMock(\phpbb\auth\auth::class); + $auth->method('acl_get')->willReturnCallback(function ($opt) { + return $opt === 'u_rt_view'; + }); + $auth->method('acl_getf')->willReturnCallback(function ($opt) { + return $opt === 'f_read' ? [1 => ['f_read' => 1], 2 => ['f_read' => 1]] : []; + }); + + $user = $this->createMock(\phpbb\user::class); + $user->data = ['user_id' => 2, 'user_rt_enable' => 1, 'is_registered' => true]; + $user->method('get_passworded_forums')->willReturn([]); + + $queried = []; + $db = $this->createMock(\phpbb\db\driver\driver_interface::class); + $db->method('sql_in_set')->willReturnCallback(function ($field, $ids) use (&$queried) { + if ($field === 'forum_id') + { + $queried = array_values($ids); + } + return '1=1'; + }); + $db->method('sql_build_query')->willReturn('SELECT 1'); + $db->method('sql_query')->willReturn('result'); + $db->method('sql_query_limit')->willReturn('result'); + $db->method('sql_fetchfield')->willReturn(0); + $db->method('sql_fetchrow')->willReturnCallback(function () use (&$queried) { + $forum_id = array_shift($queried); + return $forum_id === null ? false : ['forum_id' => $forum_id]; + }); + + $rt = new \avathar\recenttopics\core\recenttopics( + $auth, + $this->createMock(\phpbb\cache\service::class), + new \phpbb\config\config([ + 'rt_parents' => 0, + 'rt_location' => 'RT_TOP', + 'rt_unread_only' => 0, + 'rt_number' => 5, + 'rt_anti_topics' => '0', + 'rt_min_topic_level' => 0, + 'rt_page_number' => 1, + 'rt_page_numbermax' => 0, + 'rt_sort_start_time' => 0, + ]), + $this->createMock(\phpbb\language\language::class), + $content_visibility, + $db, + new \phpbb\event\dispatcher(), + $this->createMock(\phpbb\pagination::class), + $this->createMock(\phpbb\request\request_interface::class), + $this->createMock(\phpbb\template\template::class), + $user, + '/', + 'php', + $this->createMock(\phpbb\config\db_text::class) + ); + + $rt->display_recent_topics(); + + $this->assertNotEmpty($visibility_forum_ids, 'no topic query was built'); + foreach ($visibility_forum_ids as $forum_ids) + { + $forum_ids = array_values((array) $forum_ids); + sort($forum_ids); + $this->assertSame([1, 2], $forum_ids); + } + } +} +} diff --git a/tests/event/ucp_listener_test.php b/tests/event/ucp_listener_test.php index d11954f..1e8888f 100644 --- a/tests/event/ucp_listener_test.php +++ b/tests/event/ucp_listener_test.php @@ -75,10 +75,23 @@ public function test_getSubscribedEvents() $this->assertEquals(array( 'core.ucp_prefs_view_data', 'core.ucp_prefs_view_update_data', - 'core.ucp_register_data_after', + 'core.ucp_register_register_after', ), array_keys(\avathar\recenttopics\event\ucp_listener::getSubscribedEvents())); } + /** + * The registration defaults must hang off an event that fires after user_add() and + * carries the new user_id. core.ucp_register_data_after fires during form validation, + * has no user_id, and made the UPDATE hit user_id = 0 (#196). + */ + public function test_register_defaults_run_after_the_account_exists() + { + $events = \avathar\recenttopics\event\ucp_listener::getSubscribedEvents(); + + $this->assertSame('ucp_register_set_data', $events['core.ucp_register_register_after'] ?? null); + $this->assertArrayNotHasKey('core.ucp_register_data_after', $events); + } + /** * A user holding every RT permission has all six preferences persisted. */ @@ -273,7 +286,8 @@ public function test_ucp_register_set_data() ->willReturn("user_rt_enable = 1"); $this->db->expects($this->once()) - ->method('sql_query'); + ->method('sql_query') + ->with($this->stringContains('WHERE user_id = 3')); $this->set_listener(); diff --git a/tests/tests.md b/tests/tests.md index bc64b0d..dc2571f 100644 --- a/tests/tests.md +++ b/tests/tests.md @@ -43,6 +43,7 @@ tests/ │ └── page_controller_test.php Dedicated /rt and /rt/simple page controller ├── core/ │ ├── forum_list_test.php Which forums the list may draw topics from +│ ├── topic_count_test.php "Show all pages" count uses the forum list │ └── recenttopics_events_test.php Public event API contract (contrib/events.md) └── functional/ └── recenttopics_test.php End-to-end browser tests @@ -88,7 +89,7 @@ It creates the listener with mocked dependencies — a `config` object with real There are three handlers: - `ucp_prefs_get_data` — runs on page load AND on form submit; builds the data array and (only on page load) renders the UCP template block - `ucp_prefs_set_data` — maps the submitted form fields to the SQL column names used in `phpbb_users` -- `ucp_register_set_data` — runs when a new account is created and writes the global config defaults to that user's row +- `ucp_register_set_data` — runs on `core.ucp_register_register_after`, after the new account is inserted, and writes the global config defaults to that user's row **What this test file does:** Creates the listener with mocks for auth, config, request, template, user, language, and db. Because `ucp_register_set_data` actually runs a database query, it is the only handler tested with mock expectations on `sql_build_array` and `sql_query`. @@ -99,7 +100,8 @@ Creates the listener with mocks for auth, config, request, template, user, langu | `test_ucp_prefs_set_data` | Submits 5 preference fields | Each `data['rt_*']` field must map to the correct `sql_ary['user_rt_*']` column; a mismatch means preferences silently fail to save | | `test_ucp_prefs_get_data_no_submit` | Page load (submit = false) | Must: merge user DB values into `data`, call `add_lang()`, call `template->assign_vars()` | | `test_ucp_prefs_get_data_on_submit` | Form submit (submit = true) | Must: merge data; must NOT call `template->assign_vars()` — template must only be touched on page load, not on form processing | -| `test_ucp_register_set_data` | New user registration (user_id = 3) | Must call `sql_build_array('UPDATE', ...)` with all 5 default values, then `sql_query()` — verified by mock expectations on the db object | +| `test_register_defaults_run_after_the_account_exists` | Event map | `ucp_register_set_data` must be on `core.ucp_register_register_after` (fires after `user_add()`, carries `user_id`) and not on `core.ucp_register_data_after` (fires during validation, no `user_id`, so the UPDATE hit `user_id = 0`; #196) | +| `test_ucp_register_set_data` | New user registration (user_id = 3) | Must call `sql_build_array('UPDATE', ...)` with all 5 default values, then `sql_query()` with `WHERE user_id = 3` — verified by mock expectations on the db object | --- @@ -174,7 +176,21 @@ It builds `core\recenttopics` with a mocked `auth` (returning the readable forum --- -## 6. Functional tests (`functional/recenttopics_test.php`) +## 6. Page count (`core/topic_count_test.php`) + +**What this code does:** +With "Show all recent topic pages" (`rt_page_number`) on, `display_recent_topics()` counts the user's recent topics to set the page limit. The count must use the same forum list as the topic list itself. + +**What this test file does:** +It runs `display_recent_topics()` with mocked `auth`, `user`, `db` and `content_visibility`, a real event dispatcher, and `rt_page_number = 1`. The `content_visibility` mock records the forum ids each topic query is scoped to. + +| Test | Scenario | What it verifies | +|------|----------|-----------------| +| `test_page_count_query_uses_forum_list` | Readable forums 1 and 2; "Show all pages" on | Every topic query, including the page count, is scoped to forums 1 and 2. Before the fix the count ran before `get_forum_list()` and got `null`, so it only counted forums where the user has `m_approve` (#195) | + +--- + +## 7. Functional tests (`functional/recenttopics_test.php`) **What this code does:** These tests start a real phpBB installation (using the test framework's built-in install), enable the extension, and make HTTP requests using a real browser-like crawler (Symfony DomCrawler). They check the actual rendered HTML for specific elements.