From 9c6719c8197f526fc0e0c1befdf7125d42ab3810 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 08:20:36 -0700 Subject: [PATCH 1/9] Re-address character encodings for description --- adm/style/settings_add_edit.html | 2 +- controller/acp_controller.php | 6 +++--- manager/manager.php | 8 +------- tests/controller/acp_controller_test.php | 10 ++++++---- tests/functional/announcement_test.php | 2 +- tests/manager/manager_save_announcement_test.php | 16 ---------------- 6 files changed, 12 insertions(+), 32 deletions(-) diff --git a/adm/style/settings_add_edit.html b/adm/style/settings_add_edit.html index 67fc819..a7f52da 100644 --- a/adm/style/settings_add_edit.html +++ b/adm/style/settings_add_edit.html @@ -78,7 +78,7 @@

{{ lang('WARNING') }}


{{ lang('BOARD_ANNOUNCEMENTS_DESC_EXPLAIN') }}
- +
diff --git a/controller/acp_controller.php b/controller/acp_controller.php index d040318..89e035f 100644 --- a/controller/acp_controller.php +++ b/controller/acp_controller.php @@ -215,9 +215,9 @@ protected function action_add() $data['announcement_dismissable'] = $this->request->variable('board_announcements_dismiss', true); $data['announcement_expiry'] = $this->request->variable('board_announcements_expiry', ''); - // Store all Unicode as ASCII character references for portability across DBMS. - $data['announcement_description'] = utf8_encode_ncr($data['announcement_description']); - if (truncate_string($data['announcement_description'], 200, 255) !== $data['announcement_description']) + // Store four-byte Unicode as character references for portability across DBMS. + $data['announcement_description'] = utf8_encode_ucr($data['announcement_description']); + if (utf8_strlen($data['announcement_description']) > 255) { $errors[] = $this->language->lang('BOARD_ANNOUNCEMENTS_DESC_TOO_LONG'); } diff --git a/manager/manager.php b/manager/manager.php index 7630313..464eed7 100644 --- a/manager/manager.php +++ b/manager/manager.php @@ -228,13 +228,7 @@ protected function filter_guests(array $row) */ protected function intersect_data($data) { - $data = array_intersect_key($data, $this->announcement_columns()); - if (isset($data['announcement_description'])) - { - $data['announcement_description'] = utf8_encode_ncr($data['announcement_description']); - } - - return $data; + return array_intersect_key($data, $this->announcement_columns()); } /** diff --git a/tests/controller/acp_controller_test.php b/tests/controller/acp_controller_test.php index b844bf9..3dbe3e1 100644 --- a/tests/controller/acp_controller_test.php +++ b/tests/controller/acp_controller_test.php @@ -397,9 +397,11 @@ public function test_action_add_submit($id, $form, $preview, $submit, $valid_for $failed_update = $submit && !$errors && $id && !$update_success; $successful_submit = $submit && !$errors && !$failed_update; $expected_locations = json_encode(array_values(array_filter($form[7]))); - $has_expected_locations = static function ($data) use ($expected_locations) + $expected_description = utf8_encode_ucr($form[3]); + $has_expected_data = static function ($data) use ($expected_locations, $expected_description) { - return $data['announcement_locations'] === $expected_locations; + return $data['announcement_locations'] === $expected_locations + && $data['announcement_description'] === $expected_description; }; self::$valid_form = $valid_form; @@ -460,14 +462,14 @@ public function test_action_add_submit($id, $form, $preview, $submit, $valid_for ->willReturn($update_success); if ($submit && $id && !$errors) { - $update->with($id, self::callback($has_expected_locations)); + $update->with($id, self::callback($has_expected_data)); } $save = $this->manager->expects($submit && !$id && !$errors ? self::once() : self::never()) ->method('save_announcement'); if ($submit && !$id && !$errors) { - $save->with(self::callback($has_expected_locations)); + $save->with(self::callback($has_expected_data)); } $this->log->expects($successful_submit ? self::once() : self::never()) diff --git a/tests/functional/announcement_test.php b/tests/functional/announcement_test.php index 7deb798..cb5ab19 100644 --- a/tests/functional/announcement_test.php +++ b/tests/functional/announcement_test.php @@ -287,7 +287,7 @@ public function test_unicode_description_rendering() $stored_description = $this->db->sql_fetchfield('announcement_description'); $this->db->sql_freeresult($result); - self::assertSame('Unicode 😀 中文 Кириллица announcement', $stored_description); + self::assertSame('Unicode 😀 中文 Кириллица announcement', $stored_description); $crawler = self::request('GET', $this->get_acp_page()); self::assertStringContainsString($description, $crawler->filter('table > tbody')->text()); diff --git a/tests/manager/manager_save_announcement_test.php b/tests/manager/manager_save_announcement_test.php index 321f864..d09e5b7 100644 --- a/tests/manager/manager_save_announcement_test.php +++ b/tests/manager/manager_save_announcement_test.php @@ -55,20 +55,4 @@ public function test_save_announcement($id, $data) self::assertEquals($expected, $this->manager->get_announcement_data($id, $key)); } } - - /** - * Test direct manager writes encode Unicode descriptions for every DBMS - */ - public function test_save_announcement_encodes_unicode_description() - { - $data = $this->data_save_announcement()[0][1]; - $data['announcement_description'] = 'Unicode 😀 中文 Кириллица announcement'; - - $this->manager->save_announcement($data); - - self::assertSame( - 'Unicode 😀 中文 Кириллица announcement', - $this->manager->get_announcement_data(6, 'announcement_description') - ); - } } From 385db6e694b1cd9f7d25a686245e00b413c87527 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 08:45:30 -0700 Subject: [PATCH 2/9] =?UTF-8?q?Keep=20=E2=80=98f=E2=80=99=20parameter=20as?= =?UTF-8?q?=20forum=20fallback=20when=20getting=20locations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- event/listener.php | 24 +++++++++++++---- tests/event/listener_test.php | 36 ++++++++++++++++++++++++-- tests/functional/announcement_test.php | 6 ++--- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/event/listener.php b/event/listener.php index 78223d8..b79ba62 100644 --- a/event/listener.php +++ b/event/listener.php @@ -166,14 +166,28 @@ public function display_board_announcements($event) */ protected function get_current_location($event = null) { - if ($event !== null) + if ($event === null) { - $this->location = $this->user->page['page_name'] === "index.$this->php_ext" - ? ext::INDEX_ONLY - : ($event['item'] === 'forum' ? (int) $event['item_id'] : 0); + return $this->location ?? 0; } - return $this->location ?? 0; + if ($this->user->page['page_name'] === "index.$this->php_ext") + { + return $this->location = ext::INDEX_ONLY; + } + + if ($event['item'] !== 'forum') + { + return $this->location = 0; + } + + $this->location = (int) $event['item_id']; + if (!$this->location) + { + $this->location = $this->request->variable('f', 0); + } + + return $this->location; } /** diff --git a/tests/event/listener_test.php b/tests/event/listener_test.php index fafaa90..d8c0351 100644 --- a/tests/event/listener_test.php +++ b/tests/event/listener_test.php @@ -269,7 +269,39 @@ public function test_display_board_announcements($user_id, $page, $enabled, $exp ]); } - public function test_query_forum_id_does_not_scope_unrelated_page() + public function test_query_forum_id_fallback() + { + $this->db->sql_query("UPDATE phpbb_board_announcements + SET announcement_locations = '[2]' + WHERE announcement_id = 1"); + + $this->user->data['user_id'] = 2; + $this->user->page['page_name'] = "viewforum.$this->php_ext"; + $this->config['board_announcements_enable'] = true; + + $this->set_listener(); + + $this->template->expects(self::once()) + ->method('assign_block_vars') + ->with('board_announcements', self::callback(static function ($data) { + return (int) $data['BOARD_ANNOUNCEMENT_ID'] === 1; + })); + $this->request->expects(self::exactly(2)) + ->method('variable') + ->willReturnMap([ + ['f', 0, false, \phpbb\request\request_interface::REQUEST, 2], + ['_ba_1', '', true, \phpbb\request\request_interface::COOKIE, ''], + ]); + + $dispatcher = new \phpbb\event\dispatcher(); + $dispatcher->addListener('core.page_header_after', [$this->listener, 'display_board_announcements']); + $dispatcher->trigger_event('core.page_header_after', [ + 'item' => 'forum', + 'item_id' => 0, + ]); + } + + public function test_query_forum_id_does_not_scope_non_forum_event() { $this->db->sql_query("UPDATE phpbb_board_announcements SET announcement_locations = '[2]' @@ -289,7 +321,7 @@ public function test_query_forum_id_does_not_scope_unrelated_page() $dispatcher = new \phpbb\event\dispatcher(); $dispatcher->addListener('core.page_header_after', [$this->listener, 'display_board_announcements']); $dispatcher->trigger_event('core.page_header_after', [ - 'item' => 'forum', + 'item' => 'user', 'item_id' => 0, ]); } diff --git a/tests/functional/announcement_test.php b/tests/functional/announcement_test.php index cb5ab19..be8a307 100644 --- a/tests/functional/announcement_test.php +++ b/tests/functional/announcement_test.php @@ -258,12 +258,12 @@ public function test_locations() self::assertCount(0, $crawler->filter('#phpbb_announcement_' . $index_forum_id)); self::assertStringContainsString('Everywhere announcement', $crawler->filter('#phpbb_announcement_' . $everywhere_id)->text()); - // Test unrelated page with spoofed forum id - see everywhere + // Test forum ID query fallback $crawler = self::request('GET', 'memberlist.php?f=2&sid=' . $this->sid); self::assertCount(1, $crawler->filter('#phpbb_announcement_' . $everywhere_id)); self::assertCount(0, $crawler->filter('#phpbb_announcement_' . $index_id)); - self::assertCount(0, $crawler->filter('#phpbb_announcement_' . $forum_id)); - self::assertCount(0, $crawler->filter('#phpbb_announcement_' . $index_forum_id)); + self::assertCount(1, $crawler->filter('#phpbb_announcement_' . $forum_id)); + self::assertCount(1, $crawler->filter('#phpbb_announcement_' . $index_forum_id)); } /** From 8ae0269e51886346c47ac7047bb21dbe734bfe9f Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 09:05:49 -0700 Subject: [PATCH 3/9] bots now follow guest filtering --- event/listener.php | 5 +++- manager/manager.php | 5 ++-- tests/event/listener_test.php | 34 ++++++++++++++++++---- tests/functional/announcement_test.php | 27 +++++++++++++++++ tests/manager/manager_get_visible_test.php | 14 +++++---- 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/event/listener.php b/event/listener.php index b79ba62..289e02b 100644 --- a/event/listener.php +++ b/event/listener.php @@ -123,7 +123,10 @@ public function display_board_announcements($event) $this->get_current_location($event); - $board_announcements_data = $this->manager->get_visible_announcements($this->user->data['user_id']); + $board_announcements_data = $this->manager->get_visible_announcements( + $this->user->data['user_id'], + $this->user->data['is_registered'] + ); foreach ($board_announcements_data as $data) { diff --git a/manager/manager.php b/manager/manager.php index 464eed7..4fc2ae6 100644 --- a/manager/manager.php +++ b/manager/manager.php @@ -52,13 +52,14 @@ public function get_announcements() * Get all board announcements that can be seen by the user * * @param int $user_id A user identifier + * @param bool $is_registered Whether the user is registered * @return array Array of announcements data, or empty array */ - public function get_visible_announcements($user_id) + public function get_visible_announcements($user_id, $is_registered) { $data = $this->nestedset->where_visible($user_id)->get_all_tree_data(); - if ((int) $user_id === ANONYMOUS) + if (!$is_registered) { return array_filter($data, [$this, 'filter_members']); } diff --git a/tests/event/listener_test.php b/tests/event/listener_test.php index d8c0351..89ea36a 100644 --- a/tests/event/listener_test.php +++ b/tests/event/listener_test.php @@ -171,7 +171,7 @@ public function display_board_announcements_data() $user->data['user_form_salt'] = ''; return [ - [ANONYMOUS, 'index', true, + [ANONYMOUS, false, 'index', true, [ ['board_announcements', [ 'BOARD_ANNOUNCEMENT_ID' => 1, @@ -189,7 +189,7 @@ public function display_board_announcements_data() ]], ] ], - [2, 'index', true, + [2, true, 'index', true, [ ['board_announcements', [ 'BOARD_ANNOUNCEMENT_ID' => 1, @@ -200,7 +200,7 @@ public function display_board_announcements_data() ]], ] ], - [3, 'index', true, + [3, true, 'index', true, [ ['board_announcements', [ 'BOARD_ANNOUNCEMENT_ID' => 1, @@ -218,7 +218,25 @@ public function display_board_announcements_data() ]], ] ], - [4, 'viewforum', true, + [3, false, 'index', true, + [ + ['board_announcements', [ + 'BOARD_ANNOUNCEMENT_ID' => 1, + 'S_BOARD_ANNOUNCEMENT_DISMISS' => true, + 'BOARD_ANNOUNCEMENT' => 'Sample Announcement Test Text 1', + 'BOARD_ANNOUNCEMENT_BGCOLOR' => '', + 'U_BOARD_ANNOUNCEMENT_CLOSE' => 'phpbb_boardannouncements_controller#' . serialize(['id' => 1, 'hash' => generate_link_hash('close_boardannouncement1')]), + ]], + ['board_announcements', [ + 'BOARD_ANNOUNCEMENT_ID' => '3', + 'S_BOARD_ANNOUNCEMENT_DISMISS' => true, + 'BOARD_ANNOUNCEMENT' => 'Sample Announcement Test Text 3', + 'BOARD_ANNOUNCEMENT_BGCOLOR' => '000000', + 'U_BOARD_ANNOUNCEMENT_CLOSE' => 'phpbb_boardannouncements_controller#' . serialize(['id' => 3, 'hash' => generate_link_hash('close_boardannouncement3')]), + ]], + ] + ], + [4, true, 'viewforum', true, [ ['board_announcements', [ 'BOARD_ANNOUNCEMENT_ID' => 2, @@ -229,7 +247,7 @@ public function display_board_announcements_data() ]], ] ], - [5, 'viewforum', false, []], + [5, true, 'viewforum', false, []], ]; } @@ -238,13 +256,15 @@ public function display_board_announcements_data() * * @dataProvider display_board_announcements_data * @param $user_id + * @param $is_registered * @param $page * @param $enabled * @param $expected */ - public function test_display_board_announcements($user_id, $page, $enabled, $expected) + public function test_display_board_announcements($user_id, $is_registered, $page, $enabled, $expected) { $this->user->data['user_id'] = $user_id; + $this->user->data['is_registered'] = $is_registered; $this->user->page['page_name'] = "$page.$this->php_ext"; $this->config['board_announcements_enable'] = $enabled; @@ -276,6 +296,7 @@ public function test_query_forum_id_fallback() WHERE announcement_id = 1"); $this->user->data['user_id'] = 2; + $this->user->data['is_registered'] = true; $this->user->page['page_name'] = "viewforum.$this->php_ext"; $this->config['board_announcements_enable'] = true; @@ -308,6 +329,7 @@ public function test_query_forum_id_does_not_scope_non_forum_event() WHERE announcement_id = 1"); $this->user->data['user_id'] = 2; + $this->user->data['is_registered'] = true; $this->user->page['page_name'] = "memberlist.$this->php_ext"; $this->config['board_announcements_enable'] = true; diff --git a/tests/functional/announcement_test.php b/tests/functional/announcement_test.php index be8a307..4d78321 100644 --- a/tests/functional/announcement_test.php +++ b/tests/functional/announcement_test.php @@ -266,6 +266,33 @@ public function test_locations() self::assertCount(1, $crawler->filter('#phpbb_announcement_' . $index_forum_id)); } + /** + * Test bots receive guest announcements, not registered-user announcements + */ + public function test_bot_audience() + { + $this->login(); + $this->admin_login(); + + $members_id = $this->create_announcement([ + 'board_announcements_users' => ext::MEMBERS, + 'board_announcements_description' => 'Members announcement', + ]); + $guests_id = $this->create_announcement([ + 'board_announcements_users' => ext::GUESTS, + 'board_announcements_description' => 'Guests announcement', + ]); + + self::$client->restart(); + self::$client->setHeader('User-Agent', 'Googlebot/2.1 (+http://www.google.com/bot.html)'); + $crawler = self::request('GET', 'index.php'); + + self::assertCount(0, $crawler->filter('#phpbb_announcement_' . $members_id)); + self::assertCount(1, $crawler->filter('#phpbb_announcement_' . $guests_id)); + + self::$client->restart(); + } + /** * Test encoded Unicode descriptions are decoded when rendered in the ACP */ diff --git a/tests/manager/manager_get_visible_test.php b/tests/manager/manager_get_visible_test.php index 4c2c1b3..411a544 100644 --- a/tests/manager/manager_get_visible_test.php +++ b/tests/manager/manager_get_visible_test.php @@ -20,9 +20,10 @@ class manager_get_visible_test extends manager_base public function data_get_visible_announcements() { return [ - [1, ['ANNOUNCEMENT 1', 'ANNOUNCEMENT 3']], // guest - [2, ['ANNOUNCEMENT 1']], // user who dismissed announcement 2 - [3, ['ANNOUNCEMENT 1', 'ANNOUNCEMENT 2']], // any user not having dismissed any yet + [1, false, ['ANNOUNCEMENT 1', 'ANNOUNCEMENT 3']], // guest + [2, true, ['ANNOUNCEMENT 1']], // user who dismissed announcement 2 + [3, true, ['ANNOUNCEMENT 1', 'ANNOUNCEMENT 2']], // registered user without dismissals + [3, false, ['ANNOUNCEMENT 1', 'ANNOUNCEMENT 3']], // bot ]; } @@ -31,11 +32,12 @@ public function data_get_visible_announcements() * * @dataProvider data_get_visible_announcements * @param int $user_id + * @param bool $is_registered * @param string $expected */ - public function test_get_visible_announcements($user_id, $expected) + public function test_get_visible_announcements($user_id, $is_registered, $expected) { - $result = $this->manager->get_visible_announcements($user_id); + $result = $this->manager->get_visible_announcements($user_id, $is_registered); self::assertEquals($expected, array_column($result, 'announcement_description')); } @@ -45,7 +47,7 @@ public function test_get_visible_announcements($user_id, $expected) */ public function test_visibility_restrictions_with_aliased_query() { - $this->manager->get_visible_announcements(3); + $this->manager->get_visible_announcements(3, true); self::assertSame('ANNOUNCEMENT 1', $this->manager->get_announcement(1)['announcement_description']); } From 648beb3d44c834f9ed1d815d62ba3ed1a8e35390 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 09:07:33 -0700 Subject: [PATCH 4/9] Move: non-AJAX requests rebuild announcement list after reorder --- controller/acp_controller.php | 5 ++++- tests/controller/acp_controller_test.php | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/controller/acp_controller.php b/controller/acp_controller.php index 89e035f..1bce0be 100644 --- a/controller/acp_controller.php +++ b/controller/acp_controller.php @@ -14,6 +14,7 @@ use phpbb\boardannouncements\manager\manager; use phpbb\config\config; use phpbb\controller\helper; +use phpbb\json_response; use phpbb\language\language; use phpbb\log\log; use phpbb\request\request; @@ -411,9 +412,11 @@ protected function action_move() if ($this->request->is_ajax()) { - $json_response = new \phpbb\json_response; + $json_response = new json_response; $json_response->send(['success' => true]); } + + $this->list_announcements(); } /** diff --git a/tests/controller/acp_controller_test.php b/tests/controller/acp_controller_test.php index 3dbe3e1..5ee9fe3 100644 --- a/tests/controller/acp_controller_test.php +++ b/tests/controller/acp_controller_test.php @@ -625,6 +625,10 @@ public function test_action_move($id, $dir, $valid, $error, $is_ajax) ->method('move_announcement') ->with($id, $dir); + $this->manager->expects($valid && !$error && !$is_ajax ? self::once() : self::never()) + ->method('get_announcements') + ->willReturn([]); + $controller->mode_manage(); } From 70f06ce93f2c5eb67eb4f9c239b78a76429628a9 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 09:08:19 -0700 Subject: [PATCH 5/9] Delete: successful AJAX deletion returns JSON instead of rendered ACP page --- controller/acp_controller.php | 9 +++++++-- tests/controller/acp_controller_test.php | 25 ++++++++++++++++++------ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/controller/acp_controller.php b/controller/acp_controller.php index 1bce0be..abf9251 100644 --- a/controller/acp_controller.php +++ b/controller/acp_controller.php @@ -357,7 +357,7 @@ protected function action_delete() $success = false; } - // Only notify user on error or if not ajax + // Report the deletion result to the caller if (!$success) { $this->error('BOARD_ANNOUNCEMENTS_DELETE_ERROR'); @@ -366,7 +366,12 @@ protected function action_delete() { $this->log_change('BOARD_ANNOUNCEMENTS_DELETED_LOG', $description); - if (!$this->request->is_ajax()) + if ($this->request->is_ajax()) + { + $json_response = new json_response; + $json_response->send(['success' => true]); + } + else { $this->success('BOARD_ANNOUNCEMENTS_DELETE_SUCCESS'); } diff --git a/tests/controller/acp_controller_test.php b/tests/controller/acp_controller_test.php index 5ee9fe3..a7ea20b 100644 --- a/tests/controller/acp_controller_test.php +++ b/tests/controller/acp_controller_test.php @@ -489,10 +489,11 @@ public function test_action_add_submit($id, $form, $preview, $submit, $valid_for public function action_delete_data() { return [ - [1, true, true, false], // successfully delete an announcement - [2, true, false, false], // unsuccessfully delete an announcement - [3, false, null, false], // do not confirm deletion - [4, true, false, true], // announcement deleted before confirmation + [1, true, true, false, false], // successfully delete an announcement + [2, true, false, false, false], // unsuccessfully delete an announcement + [3, false, null, false, false], // do not confirm deletion + [4, true, false, true, false], // announcement deleted before confirmation + [5, true, true, false, true], // successfully delete via ajax ]; } @@ -504,8 +505,9 @@ public function action_delete_data() * @param bool $confirm_action * @param bool|null $success * @param bool $throws + * @param bool $is_ajax */ - public function test_action_delete($id, $confirm_action, $success, $throws) + public function test_action_delete($id, $confirm_action, $success, $throws, $is_ajax) { self::$confirm = $confirm_action; @@ -530,7 +532,14 @@ public function test_action_delete($id, $confirm_action, $success, $throws) { if ($success) { - $this->setExpectedTriggerError(E_USER_NOTICE, 'BOARD_ANNOUNCEMENTS_DELETE_SUCCESS'); + if ($is_ajax) + { + $this->setExpectedTriggerError(E_WARNING); + } + else + { + $this->setExpectedTriggerError(E_USER_NOTICE, 'BOARD_ANNOUNCEMENTS_DELETE_SUCCESS'); + } $this->log->expects(self::once()) ->method('add'); } @@ -557,6 +566,10 @@ public function test_action_delete($id, $confirm_action, $success, $throws) } } + $this->request->expects($success ? self::once() : self::never()) + ->method('is_ajax') + ->willReturn($is_ajax); + $controller->mode_manage(); } From 71cc5ca261a56b485d2509fd299310449559bf59 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 09:08:54 -0700 Subject: [PATCH 6/9] Fix tests after expanding description to 255 chars --- tests/controller/acp_controller_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/controller/acp_controller_test.php b/tests/controller/acp_controller_test.php index a7ea20b..456682d 100644 --- a/tests/controller/acp_controller_test.php +++ b/tests/controller/acp_controller_test.php @@ -365,7 +365,7 @@ public function action_add_submit_data() [1, ['add', 1, 'Announcement Text 1', 'Announcement Description 1', 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, false, false], // submit, announcement deleted before update [0, ['add', 0, 'Announcement Text 0', 'Announcement Description 0', 'ffffff', true, 0, [''], true, '', false, false, false], false, true, false, true], // submit, bad form [0, ['add', 0, '', 'Announcement Description 0', 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, true], // submit, bad text - [0, ['add', 0, 'Announcement Text 0', str_repeat('a', 201), 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, true], // submit, description too long + [0, ['add', 0, 'Announcement Text 0', str_repeat('a', 256), 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, true], // submit, description too long [0, ['add', 0, 'Announcement Text 0', str_repeat('a', 186) . str_repeat('&', 14), 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, true], // submit, escaped description exceeds stored length [0, ['add', 0, 'Announcement Text 0', 'Announcement Description 0', 'fffff', true, 0, [''], true, '', false, false, false], false, true, true, true], // submit, background color too short [0, ['add', 0, 'Announcement Text 0', 'Announcement Description 0', 'gggggg', true, 0, [''], true, '', false, false, false], false, true, true, true], // submit, background color is not hexadecimal From 286e1e50acf392dd0cfee30cea742ca30f08265b Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 09:15:52 -0700 Subject: [PATCH 7/9] Format creation/expiry dates using administrator timezone --- adm/style/settings_list.html | 4 ++-- controller/acp_controller.php | 4 ++-- tests/controller/acp_controller_test.php | 24 +++++++++++++++++++++--- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/adm/style/settings_list.html b/adm/style/settings_list.html index 1e1ede1..d505095 100644 --- a/adm/style/settings_list.html +++ b/adm/style/settings_list.html @@ -64,9 +64,9 @@

{{ lang('BOARD_ANNOUNCEMENTS_SETTINGS') }}

{{ lang('G_GUESTS') }} {% endif %} - {{ ba.CREATED_DATE ? ba.CREATED_DATE|date(constant('\\phpbb\\boardannouncements\\ext::DATE_FORMAT')) : '' }} + {{ ba.CREATED_DATE }} {{ ba.S_ENABLED ? _self.baIcon('check', 'settings', 'YES') : _self.baIcon('times', 'delete', 'NO') }} - {{ ba.EXPIRY_DATE ? ba.EXPIRY_DATE|date(constant('\\phpbb\\boardannouncements\\ext::DATE_FORMAT')) : '' }} + {{ ba.EXPIRY_DATE }} {{ ba.S_EXPIRED ? lang('YES') : lang('NO') }} diff --git a/controller/acp_controller.php b/controller/acp_controller.php index abf9251..f9c7e5d 100644 --- a/controller/acp_controller.php +++ b/controller/acp_controller.php @@ -139,8 +139,8 @@ protected function list_announcements() $this->template->assign_block_vars('announcements' , [ 'DESCRIPTION' => $row['announcement_description'], 'USERS' => $row['announcement_users'], - 'CREATED_DATE' => $row['announcement_timestamp'], - 'EXPIRY_DATE' => $row['announcement_expiry'], + 'CREATED_DATE' => $row['announcement_timestamp'] ? $this->user->format_date($row['announcement_timestamp'], ext::DATE_FORMAT) : '', + 'EXPIRY_DATE' => $row['announcement_expiry'] ? $this->user->format_date($row['announcement_expiry'], ext::DATE_FORMAT) : '', 'S_EXPIRED' => $expired, 'S_ENABLED' => $enabled, 'LOCATIONS' => $this->manager->decode_json($row['announcement_locations']), diff --git a/tests/controller/acp_controller_test.php b/tests/controller/acp_controller_test.php index 456682d..90812b4 100644 --- a/tests/controller/acp_controller_test.php +++ b/tests/controller/acp_controller_test.php @@ -189,6 +189,7 @@ public function test_mode_manage($action, $expected) public function test_list_announcements() { $controller = $this->get_controller(); + $this->user->data['user_timezone'] = 'Asia/Tokyo'; $rows = [ [ @@ -229,7 +230,21 @@ public function test_list_announcements() ->with(3); $this->template->expects(self::exactly(3)) - ->method('assign_block_vars'); + ->method('assign_block_vars') + ->withConsecutive( + ['announcements', self::callback(function ($data) use ($rows) { + return $data['CREATED_DATE'] === $this->user->format_date($rows[0]['announcement_timestamp'], \phpbb\boardannouncements\ext::DATE_FORMAT) + && $data['EXPIRY_DATE'] === ''; + })], + ['announcements', self::callback(function ($data) use ($rows) { + return $data['CREATED_DATE'] === $this->user->format_date($rows[1]['announcement_timestamp'], \phpbb\boardannouncements\ext::DATE_FORMAT) + && $data['EXPIRY_DATE'] === ''; + })], + ['announcements', self::callback(function ($data) use ($rows) { + return $data['CREATED_DATE'] === $this->user->format_date($rows[2]['announcement_timestamp'], \phpbb\boardannouncements\ext::DATE_FORMAT) + && $data['EXPIRY_DATE'] === $this->user->format_date($rows[2]['announcement_expiry'], \phpbb\boardannouncements\ext::DATE_FORMAT); + })] + ); $this->template->expects(self::once()) ->method('assign_vars') @@ -398,10 +413,12 @@ public function test_action_add_submit($id, $form, $preview, $submit, $valid_for $successful_submit = $submit && !$errors && !$failed_update; $expected_locations = json_encode(array_values(array_filter($form[7]))); $expected_description = utf8_encode_ucr($form[3]); - $has_expected_data = static function ($data) use ($expected_locations, $expected_description) + $creation_timestamp = 1234567890; + $has_expected_data = static function ($data) use ($id, $expected_locations, $expected_description, $creation_timestamp) { return $data['announcement_locations'] === $expected_locations - && $data['announcement_description'] === $expected_description; + && $data['announcement_description'] === $expected_description + && ($id ? $data['announcement_timestamp'] === $creation_timestamp : $data['announcement_timestamp'] > 0); }; self::$valid_form = $valid_form; @@ -455,6 +472,7 @@ public function test_action_add_submit($id, $form, $preview, $submit, $valid_for 'announcement_uid' => '', 'announcement_bitfield' => '', 'announcement_flags' => 7, + 'announcement_timestamp' => $creation_timestamp, ]); $update = $this->manager->expects($submit && $id && !$errors ? self::once() : self::never()) From bcf5a15954c833c7eb7d9bd1ead6ce68b6198a14 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 09:16:22 -0700 Subject: [PATCH 8/9] =?UTF-8?q?Don=E2=80=99t=20update=20creation=20date=20?= =?UTF-8?q?when=20an=20announcement=20is=20edited?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- controller/acp_controller.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/controller/acp_controller.php b/controller/acp_controller.php index f9c7e5d..8ad091b 100644 --- a/controller/acp_controller.php +++ b/controller/acp_controller.php @@ -206,7 +206,11 @@ protected function action_add() } // Get new announcement values from the form - $data['announcement_timestamp'] = time(); + // Preserve the original creation date when editing an announcement. + if (!$id) + { + $data['announcement_timestamp'] = time(); + } $data['announcement_text'] = $this->request->variable('board_announcements_text', '', true); $data['announcement_description'] = $this->request->variable('board_announcements_description', '', true); $data['announcement_bgcolor'] = $this->request->variable('board_announcements_bgcolor', '', true); From 46a4335a062ad176f638c9adc8b04d5eb484cc89 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 14 Sep 2026 10:03:07 -0700 Subject: [PATCH 9/9] Encode for MSSQL with ncr --- config/services.yml | 1 + controller/acp_controller.php | 14 +++++++++++--- tests/controller/acp_controller_test.php | 20 ++++++++++++++++++-- tests/functional/announcement_test.php | 5 ++++- 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/config/services.yml b/config/services.yml index b428347..d9af922 100644 --- a/config/services.yml +++ b/config/services.yml @@ -34,6 +34,7 @@ services: class: phpbb\boardannouncements\controller\acp_controller arguments: - '@phpbb.boardannouncements.manager' + - '@dbal.conn' - '@config' - '@controller.helper' - '@language' diff --git a/controller/acp_controller.php b/controller/acp_controller.php index 8ad091b..a38afc4 100644 --- a/controller/acp_controller.php +++ b/controller/acp_controller.php @@ -14,6 +14,7 @@ use phpbb\boardannouncements\manager\manager; use phpbb\config\config; use phpbb\controller\helper; +use phpbb\db\driver\driver_interface; use phpbb\json_response; use phpbb\language\language; use phpbb\log\log; @@ -26,6 +27,9 @@ class acp_controller /** @var manager */ protected $manager; + /** @var driver_interface */ + protected $db; + /** @var config */ protected $config; @@ -60,6 +64,7 @@ class acp_controller * Constructor * * @param manager $manager + * @param driver_interface $db * @param config $config * @param helper $controller_helper * @param language $language @@ -70,9 +75,10 @@ class acp_controller * @param $phpbb_root_path * @param $php_ext */ - public function __construct(manager $manager, config $config, helper $controller_helper, language $language, log $log, request $request, template $template, user $user, $phpbb_root_path, $php_ext) + public function __construct(manager $manager, driver_interface $db, config $config, helper $controller_helper, language $language, log $log, request $request, template $template, user $user, $phpbb_root_path, $php_ext) { $this->manager = $manager; + $this->db = $db; $this->config = $config; $this->controller_helper = $controller_helper; $this->language = $language; @@ -220,8 +226,10 @@ protected function action_add() $data['announcement_dismissable'] = $this->request->variable('board_announcements_dismiss', true); $data['announcement_expiry'] = $this->request->variable('board_announcements_expiry', ''); - // Store four-byte Unicode as character references for portability across DBMS. - $data['announcement_description'] = utf8_encode_ucr($data['announcement_description']); + // MSSQL requires all Unicode to be encoded; other DBMS only require four-byte Unicode. + $data['announcement_description'] = strpos($this->db->get_sql_layer(), 'mssql') === 0 + ? utf8_encode_ncr($data['announcement_description']) + : utf8_encode_ucr($data['announcement_description']); if (utf8_strlen($data['announcement_description']) > 255) { $errors[] = $this->language->lang('BOARD_ANNOUNCEMENTS_DESC_TOO_LONG'); diff --git a/tests/controller/acp_controller_test.php b/tests/controller/acp_controller_test.php index 90812b4..e925bb1 100644 --- a/tests/controller/acp_controller_test.php +++ b/tests/controller/acp_controller_test.php @@ -23,6 +23,12 @@ class acp_controller_test extends \phpbb_test_case /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\boardannouncements\manager\manager */ protected $manager; + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\db\driver\driver_interface */ + protected $db; + + /** @var string */ + protected $sql_layer = 'mysqli'; + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\config\config */ protected $config; @@ -105,6 +111,11 @@ protected function setUp(): void $this->manager = $this->getMockBuilder('\phpbb\boardannouncements\manager\manager') ->disableOriginalConstructor() ->getMock(); + $this->db = $this->createMock('\phpbb\db\driver\driver_interface'); + $this->db->method('get_sql_layer') + ->willReturnCallback(function () { + return $this->sql_layer; + }); } /** @@ -116,6 +127,7 @@ protected function get_controller() { $controller = new \phpbb\boardannouncements\controller\acp_controller( $this->manager, + $this->db, $this->config, $this->controller_helper, $this->language, @@ -159,6 +171,7 @@ public function test_mode_manage($action, $expected) ->setMethods(['action_add', 'action_delete', 'action_move', 'action_settings', 'list_announcements']) ->setConstructorArgs([ $this->manager, + $this->db, $this->config, $this->controller_helper, $this->language, @@ -376,6 +389,7 @@ public function action_add_submit_data() [0, ['add', 0, 'Announcement Text 0', 'Announcement Description 0', 'ABCDEF', true, 0, [''], true, '', false, false, false], false, true, true, false], // submit [0, ['add', 0, 'Announcement Text 0', 'Selected locations', 'ffffff', true, 0, [0, -1, 2], true, '', false, false, false], false, true, true, false], // submit, discard location sentinel [0, ['add', 0, 'Announcement Text 0', 'Emoji 😀 description', 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, false], // submit, emoji encoded for storage + [0, ['add', 0, 'Announcement Text 0', 'Unicode 😀 中文 Кириллица', 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, false, true, 'mssqlnative'], // submit, all Unicode encoded for MSSQL [1, ['add', 1, 'Announcement Text 1', 'Announcement Description 1', 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, false], // submit [1, ['add', 1, 'Announcement Text 1', 'Announcement Description 1', 'ffffff', true, 0, [''], true, '', false, false, false], false, true, true, false, false], // submit, announcement deleted before update [0, ['add', 0, 'Announcement Text 0', 'Announcement Description 0', 'ffffff', true, 0, [''], true, '', false, false, false], false, true, false, true], // submit, bad form @@ -404,15 +418,17 @@ public function action_add_submit_data() * @param $valid_form * @param $errors * @param bool $update_success + * @param string $sql_layer * @return void */ - public function test_action_add_submit($id, $form, $preview, $submit, $valid_form, $errors, $update_success = true) + public function test_action_add_submit($id, $form, $preview, $submit, $valid_form, $errors, $update_success = true, $sql_layer = 'mysqli') { + $this->sql_layer = $sql_layer; $controller = $this->get_controller(); $failed_update = $submit && !$errors && $id && !$update_success; $successful_submit = $submit && !$errors && !$failed_update; $expected_locations = json_encode(array_values(array_filter($form[7]))); - $expected_description = utf8_encode_ucr($form[3]); + $expected_description = strpos($sql_layer, 'mssql') === 0 ? utf8_encode_ncr($form[3]) : utf8_encode_ucr($form[3]); $creation_timestamp = 1234567890; $has_expected_data = static function ($data) use ($id, $expected_locations, $expected_description, $creation_timestamp) { diff --git a/tests/functional/announcement_test.php b/tests/functional/announcement_test.php index 4d78321..77debf3 100644 --- a/tests/functional/announcement_test.php +++ b/tests/functional/announcement_test.php @@ -314,7 +314,10 @@ public function test_unicode_description_rendering() $stored_description = $this->db->sql_fetchfield('announcement_description'); $this->db->sql_freeresult($result); - self::assertSame('Unicode 😀 中文 Кириллица announcement', $stored_description); + $expected_description = strpos($this->db->get_sql_layer(), 'mssql') === 0 + ? utf8_encode_ncr($description) + : utf8_encode_ucr($description); + self::assertSame($expected_description, $stored_description); $crawler = self::request('GET', $this->get_acp_page()); self::assertStringContainsString($description, $crawler->filter('table > tbody')->text());