From aa76f6b9f2b85a1f85da40d71eb533589c106a09 Mon Sep 17 00:00:00 2001 From: rxu <rxu@mail.ru> Date: Wed, 1 Jul 2020 13:26:35 +0700 Subject: [PATCH 1/3] [ticket/16544] Use forum_notify.txt for forum subscription email notifications PHPBB3-16544 --- .../container/services_notification.yml | 10 ++ phpBB/includes/functions_posting.php | 2 + phpBB/install/schemas/schema_data.sql | 2 + phpBB/language/en/ucp.php | 1 + phpBB/phpbb/notification/type/forum.php | 145 ++++++++++++++++++ phpBB/phpbb/notification/type/post.php | 12 -- tests/notification/submit_post_base.php | 2 +- 7 files changed, 161 insertions(+), 13 deletions(-) create mode 100644 phpBB/phpbb/notification/type/forum.php diff --git a/phpBB/config/default/container/services_notification.yml b/phpBB/config/default/container/services_notification.yml index 41f80cdd30..c18e358114 100644 --- a/phpBB/config/default/container/services_notification.yml +++ b/phpBB/config/default/container/services_notification.yml @@ -176,6 +176,16 @@ services: tags: - { name: notification.type } + notification.type.forum: + class: phpbb\notification\type\forum + shared: false + parent: notification.type.post + calls: + - [set_user_loader, ['@user_loader']] + - [set_config, ['@config']] + tags: + - { name: notification.type } + # ----- Notification's methods ----- # Service MUST NOT be shared for all the plugins to work. notification.method_collection: diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index c056d93fb4..9314616e2b 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -2399,6 +2399,7 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll_ary, &$data 'notification.type.quote', 'notification.type.bookmark', 'notification.type.post', + 'notification.type.forum', ), $notification_data); break; @@ -2417,6 +2418,7 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll_ary, &$data 'notification.type.bookmark', 'notification.type.topic', 'notification.type.post', + 'notification.type.forum', ), $notification_data); break; } diff --git a/phpBB/install/schemas/schema_data.sql b/phpBB/install/schemas/schema_data.sql index da7cad3915..92a3e9008a 100644 --- a/phpBB/install/schemas/schema_data.sql +++ b/phpBB/install/schemas/schema_data.sql @@ -845,5 +845,7 @@ INSERT INTO phpbb_user_notifications (item_type, item_id, user_id, method) VALUE INSERT INTO phpbb_user_notifications (item_type, item_id, user_id, method) VALUES('notification.type.post', 0, 2, 'notification.method.email'); INSERT INTO phpbb_user_notifications (item_type, item_id, user_id, method) VALUES('notification.type.topic', 0, 2, 'notification.method.board'); INSERT INTO phpbb_user_notifications (item_type, item_id, user_id, method) VALUES('notification.type.topic', 0, 2, 'notification.method.email'); +INSERT INTO phpbb_user_notifications (item_type, item_id, user_id, method) VALUES('notification.type.forum', 0, 2, 'notification.method.board'); +INSERT INTO phpbb_user_notifications (item_type, item_id, user_id, method) VALUES('notification.type.forum', 0, 2, 'notification.method.email'); # POSTGRES COMMIT # diff --git a/phpBB/language/en/ucp.php b/phpBB/language/en/ucp.php index 7e78c84976..7cfbc2d50c 100644 --- a/phpBB/language/en/ucp.php +++ b/phpBB/language/en/ucp.php @@ -336,6 +336,7 @@ $lang = array_merge($lang, array( 'NOTIFICATION_TYPE' => 'Notification type', 'NOTIFICATION_TYPE_BOOKMARK' => 'Someone replies to a topic you have bookmarked', 'NOTIFICATION_TYPE_GROUP_REQUEST' => 'Someone requests to join a group you lead', + 'NOTIFICATION_TYPE_FORUM' => 'Someone replies to a topic in a forum to which you are subscribed', 'NOTIFICATION_TYPE_IN_MODERATION_QUEUE' => 'A post or topic needs approval', 'NOTIFICATION_TYPE_MODERATION_QUEUE' => 'Your topics/posts are approved or disapproved by a moderator', 'NOTIFICATION_TYPE_PM' => 'Someone sends you a private message', diff --git a/phpBB/phpbb/notification/type/forum.php b/phpBB/phpbb/notification/type/forum.php new file mode 100644 index 0000000000..3c1c4579f1 --- /dev/null +++ b/phpBB/phpbb/notification/type/forum.php @@ -0,0 +1,145 @@ +<?php +/** +* +* This file is part of the phpBB Forum Software package. +* +* @copyright (c) phpBB Limited <https://www.phpbb.com> +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace phpbb\notification\type; + +/** +* Forum notifications class +* This class handles notifications for replies to a topic in a forum user subscribed to +*/ + +class forum extends \phpbb\notification\type\post +{ + /** + * Get notification type name + * + * @return string + */ + public function get_type() + { + return 'notification.type.forum'; + } + + /** + * Notification option data (for outputting to the user) + * + * @var bool|array False if the service should use it's default data + * Array of data (including keys 'id', 'lang', and 'group') + */ + static public $notification_option = [ + 'lang' => 'NOTIFICATION_TYPE_FORUM', + 'group' => 'NOTIFICATION_GROUP_POSTING', + ]; + + /** + * Find the users who want to receive notifications + * + * @param array $post Data from submit_post + * @param array $options Options for finding users for notification + * + * @return array + */ + public function find_users_for_notification($post, $options = []) + { + $options = array_merge([ + 'ignore_users' => [], + ], $options); + + $users = []; + + $sql = 'SELECT user_id + FROM ' . FORUMS_WATCH_TABLE . ' + WHERE forum_id = ' . (int) $post['forum_id'] . ' + AND notify_status = ' . NOTIFY_YES . ' + AND user_id <> ' . (int) $post['poster_id']; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $users[] = (int) $row['user_id']; + } + $this->db->sql_freeresult($result); + + $notify_users = $this->get_authorised_recipients($users, $post['forum_id'], $options, true); + + if (empty($notify_users)) + { + return []; + } + + // Try to find the users who already have been notified about replies and have not read the topic since and just update their notifications + $notified_users = $this->notification_manager->get_notified_users($this->get_type(), [ + 'item_parent_id' => static::get_item_parent_id($post), + 'read' => 0, + ]); + + foreach ($notified_users as $user => $notification_data) + { + unset($notify_users[$user]); + + /** @var post $notification */ + $notification = $this->notification_manager->get_item_type_class($this->get_type(), $notification_data); + $update_responders = $notification->add_responders($post); + if (!empty($update_responders)) + { + $this->notification_manager->update_notification($notification, $update_responders, [ + 'item_parent_id' => self::get_item_parent_id($post), + 'read' => 0, + 'user_id' => $user, + ]); + } + } + + return $notify_users; + } + + /** + * Get email template + * + * @return string|bool + */ + public function get_email_template() + { + return 'forum_notify'; + } + + /** + * Get email template variables + * + * @return array + */ + public function get_email_template_variables() + { + if ($this->get_data('post_username')) + { + $username = $this->get_data('post_username'); + } + else + { + $username = $this->user_loader->get_username($this->get_data('poster_id'), 'username'); + } + + return [ + 'AUTHOR_NAME' => htmlspecialchars_decode($username), + 'FORUM_NAME' => htmlspecialchars_decode(censor_text($this->get_data('forum_name'))), + 'POST_SUBJECT' => htmlspecialchars_decode(censor_text($this->get_data('post_subject'))), + 'TOPIC_TITLE' => htmlspecialchars_decode(censor_text($this->get_data('topic_title'))), + + 'U_VIEW_POST' => generate_board_url() . "/viewtopic.{$this->php_ext}?p={$this->item_id}#p{$this->item_id}", + 'U_NEWEST_POST' => generate_board_url() . "/viewtopic.{$this->php_ext}?f={$this->get_data('forum_id')}&t={$this->item_parent_id}&e=1&view=unread#unread", + 'U_TOPIC' => generate_board_url() . "/viewtopic.{$this->php_ext}?f={$this->get_data('forum_id')}&t={$this->item_parent_id}", + 'U_VIEW_TOPIC' => generate_board_url() . "/viewtopic.{$this->php_ext}?f={$this->get_data('forum_id')}&t={$this->item_parent_id}", + 'U_FORUM' => generate_board_url() . "/viewforum.{$this->php_ext}?f={$this->get_data('forum_id')}", + 'U_STOP_WATCHING_FORUM' => generate_board_url() . "/viewtopic.{$this->php_ext}?uid={$this->user_id}&f={$this->get_data('forum_id')}&t={$this->item_parent_id}&unwatch=forum", + ]; + } +} diff --git a/phpBB/phpbb/notification/type/post.php b/phpBB/phpbb/notification/type/post.php index 64ed91a5f5..a25328ff43 100644 --- a/phpBB/phpbb/notification/type/post.php +++ b/phpBB/phpbb/notification/type/post.php @@ -129,18 +129,6 @@ class post extends \phpbb\notification\type\base } $this->db->sql_freeresult($result); - $sql = 'SELECT user_id - FROM ' . FORUMS_WATCH_TABLE . ' - WHERE forum_id = ' . (int) $post['forum_id'] . ' - AND notify_status = ' . NOTIFY_YES . ' - AND user_id <> ' . (int) $post['poster_id']; - $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $users[] = (int) $row['user_id']; - } - $this->db->sql_freeresult($result); - $notify_users = $this->get_authorised_recipients($users, $post['forum_id'], $options, true); if (empty($notify_users)) diff --git a/tests/notification/submit_post_base.php b/tests/notification/submit_post_base.php index 82db7753b5..3906c1d74c 100644 --- a/tests/notification/submit_post_base.php +++ b/tests/notification/submit_post_base.php @@ -135,7 +135,7 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c $phpbb_container->compile(); // Notification Types - $notification_types = array('quote', 'bookmark', 'post', 'post_in_queue', 'topic', 'topic_in_queue', 'approve_topic', 'approve_post'); + $notification_types = array('quote', 'bookmark', 'post', 'post_in_queue', 'topic', 'topic_in_queue', 'approve_topic', 'approve_post', 'forum'); $notification_types_array = array(); foreach ($notification_types as $type) { From 02f54ef2196dda61b2b507dd2a09ac097672a551 Mon Sep 17 00:00:00 2001 From: rxu <rxu@mail.ru> Date: Wed, 1 Jul 2020 21:12:35 +0700 Subject: [PATCH 2/3] [ticket/16544] Fix notifications tests PHPBB3-16544 --- tests/notification/base.php | 1 + .../fixtures/email_notification.type.post.xml | 54 ++++ .../submit_post_notification.type.forum.xml | 262 ++++++++++++++++++ .../submit_post_notification.type.post.xml | 54 ++++ .../notification_method_email_test.php | 58 +++- tests/notification/notification_test.php | 1 + .../submit_post_type_forum_test.php | 109 ++++++++ .../submit_post_type_post_test.php | 15 +- 8 files changed, 545 insertions(+), 9 deletions(-) create mode 100644 tests/notification/fixtures/submit_post_notification.type.forum.xml create mode 100644 tests/notification/submit_post_type_forum_test.php diff --git a/tests/notification/base.php b/tests/notification/base.php index 1cc5893ebd..1622d12363 100644 --- a/tests/notification/base.php +++ b/tests/notification/base.php @@ -32,6 +32,7 @@ abstract class phpbb_tests_notification_base extends phpbb_database_test_case 'notification.type.bookmark', 'notification.type.disapprove_post', 'notification.type.disapprove_topic', + 'notification.type.forum', 'notification.type.pm', 'notification.type.post', 'notification.type.post_in_queue', diff --git a/tests/notification/fixtures/email_notification.type.post.xml b/tests/notification/fixtures/email_notification.type.post.xml index 8787f5bbe5..6d064141b7 100644 --- a/tests/notification/fixtures/email_notification.type.post.xml +++ b/tests/notification/fixtures/email_notification.type.post.xml @@ -56,6 +56,11 @@ <value>notification.type.post</value> <value>1</value> </row> + <row> + <value>2</value> + <value>notification.type.forum</value> + <value>1</value> + </row> </table> <table name="phpbb_posts"> <column>post_id</column> @@ -213,5 +218,54 @@ <value>notification.method.email</value> <value>1</value> </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>2</value> + <value>notification.method.email</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>3</value> + <value>notification.method.email</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>4</value> + <value>notification.method.email</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>5</value> + <value>notification.method.email</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>6</value> + <value>notification.method.email</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>7</value> + <value>notification.method.email</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>8</value> + <value>notification.method.email</value> + <value>1</value> + </row> </table> </dataset> diff --git a/tests/notification/fixtures/submit_post_notification.type.forum.xml b/tests/notification/fixtures/submit_post_notification.type.forum.xml new file mode 100644 index 0000000000..b8df34bedc --- /dev/null +++ b/tests/notification/fixtures/submit_post_notification.type.forum.xml @@ -0,0 +1,262 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<dataset> + <table name="phpbb_forums_watch"> + <column>forum_id</column> + <column>user_id</column> + <column>notify_status</column> + <row> + <value>1</value> + <value>2</value> + <value>0</value> + </row> + <row> + <value>1</value> + <value>3</value> + <value>0</value> + </row> + <row> + <value>1</value> + <value>4</value> + <value>0</value> + </row> + <row> + <value>1</value> + <value>5</value> + <value>0</value> + </row> + <row> + <value>1</value> + <value>6</value> + <value>0</value> + </row> + </table> + <table name="phpbb_notifications"> + <column>notification_id</column> + <column>notification_type_id</column> + <column>user_id</column> + <column>item_id</column> + <column>item_parent_id</column> + <column>notification_read</column> + <column>notification_data</column> + <row> + <value>1</value> + <value>1</value> + <value>5</value> + <value>1</value> + <value>1</value> + <value>0</value> + <value></value> + </row> + <row> + <value>2</value> + <value>1</value> + <value>8</value> + <value>1</value> + <value>1</value> + <value>0</value> + <value></value> + </row> + </table> + <table name="phpbb_notification_types"> + <column>notification_type_id</column> + <column>notification_type_name</column> + <column>notification_type_enabled</column> + <row> + <value>1</value> + <value>notification.type.forum</value> + <value>1</value> + </row> + <row> + <value>2</value> + <value>notification.type.post</value> + <value>1</value> + </row> + </table> + <table name="phpbb_posts"> + <column>post_id</column> + <column>topic_id</column> + <column>forum_id</column> + <column>post_text</column> + <row> + <value>1</value> + <value>1</value> + <value>1</value> + <value></value> + </row> + </table> + <table name="phpbb_topics"> + <column>topic_id</column> + <column>forum_id</column> + <row> + <value>1</value> + <value>1</value> + </row> + </table> + <table name="phpbb_topics_watch"> + <column>topic_id</column> + <column>user_id</column> + <column>notify_status</column> + <row> + <value>1</value> + <value>6</value> + <value>0</value> + </row> + <row> + <value>1</value> + <value>7</value> + <value>0</value> + </row> + <row> + <value>1</value> + <value>8</value> + <value>0</value> + </row> + </table> + <table name="phpbb_users"> + <column>user_id</column> + <column>username_clean</column> + <column>user_permissions</column> + <column>user_sig</column> + <row> + <value>2</value> + <value>poster</value> + <value></value> + <value></value> + </row> + <row> + <value>3</value> + <value>test</value> + <value></value> + <value></value> + </row> + <row> + <value>4</value> + <value>unauthorized</value> + <value></value> + <value></value> + </row> + <row> + <value>5</value> + <value>notified</value> + <value></value> + <value></value> + </row> + <row> + <value>6</value> + <value>disabled</value> + <value></value> + <value></value> + </row> + <row> + <value>7</value> + <value>default</value> + <value></value> + <value></value> + </row> + </table> + <table name="phpbb_user_notifications"> + <column>item_type</column> + <column>item_id</column> + <column>user_id</column> + <column>method</column> + <column>notify</column> + <row> + <value>notification.type.post</value> + <value>0</value> + <value>2</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.post</value> + <value>0</value> + <value>3</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.post</value> + <value>0</value> + <value>4</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.post</value> + <value>0</value> + <value>5</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.post</value> + <value>0</value> + <value>6</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.post</value> + <value>0</value> + <value>7</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.post</value> + <value>0</value> + <value>8</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>2</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>3</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>4</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>5</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>6</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>7</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>8</value> + <value>notification.method.board</value> + <value>1</value> + </row> + </table> +</dataset> diff --git a/tests/notification/fixtures/submit_post_notification.type.post.xml b/tests/notification/fixtures/submit_post_notification.type.post.xml index 920b271525..75bfb03e89 100644 --- a/tests/notification/fixtures/submit_post_notification.type.post.xml +++ b/tests/notification/fixtures/submit_post_notification.type.post.xml @@ -56,6 +56,11 @@ <value>notification.type.post</value> <value>1</value> </row> + <row> + <value>2</value> + <value>notification.type.forum</value> + <value>1</value> + </row> </table> <table name="phpbb_posts"> <column>post_id</column> @@ -204,5 +209,54 @@ <value>notification.method.board</value> <value>1</value> </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>2</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>3</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>4</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>5</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>6</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>7</value> + <value>notification.method.board</value> + <value>1</value> + </row> + <row> + <value>notification.type.forum</value> + <value>0</value> + <value>8</value> + <value>notification.method.board</value> + <value>1</value> + </row> </table> </dataset> diff --git a/tests/notification/notification_method_email_test.php b/tests/notification/notification_method_email_test.php index b4734c872d..cf84ddc795 100644 --- a/tests/notification/notification_method_email_test.php +++ b/tests/notification/notification_method_email_test.php @@ -157,7 +157,20 @@ class notification_method_email_test extends phpbb_tests_notification_base public function data_notification_email() { return [ + /** + * Normal post + * + * User => State description + * 2 => Topic id=1 and id=2 subscribed, should receive a new topics post notification + * 3 => Topic id=1 subscribed, should receive a new topic post notification + * 4 => Topic id=1 subscribed, should receive a new topic post notification + * 5 => Topic id=1 subscribed, post id=1 already notified, should receive a new topic post notification + * 6 => Topic id=1 and forum id=1 subscribed, should receive a new topic/forum post notification + * 7 => Forum id=1 subscribed, should NOT receive a new topic post but a forum post notification + * 8 => Forum id=1 subscribed, post id=1 already notified, should NOT receive a new topic post but a forum post notification + */ [ + 'notification.type.post', [ 'forum_id' => '1', 'post_id' => '2', @@ -169,11 +182,23 @@ class notification_method_email_test extends phpbb_tests_notification_base 4 => ['user_id' => '4'], 5 => ['user_id' => '5'], 6 => ['user_id' => '6'], + ], + ], + [ + 'notification.type.forum', + [ + 'forum_id' => '1', + 'post_id' => '3', + 'topic_id' => '1', + ], + [ + 6 => ['user_id' => '6'], 7 => ['user_id' => '7'], 8 => ['user_id' => '8'] ], ], [ + 'notification.type.post', [ 'forum_id' => '1', 'post_id' => '4', @@ -181,12 +206,33 @@ class notification_method_email_test extends phpbb_tests_notification_base ], [ 2 => ['user_id' => '2'], + ], + ], + [ + 'notification.type.forum', + [ + 'forum_id' => '1', + 'post_id' => '5', + 'topic_id' => '2', + ], + [ 6 => ['user_id' => '6'], 7 => ['user_id' => '7'], 8 => ['user_id' => '8'], ], ], [ + 'notification.type.post', + [ + 'forum_id' => '2', + 'post_id' => '6', + 'topic_id' => '3', + ], + [ + ], + ], + [ + 'notification.type.forum', [ 'forum_id' => '2', 'post_id' => '6', @@ -201,7 +247,7 @@ class notification_method_email_test extends phpbb_tests_notification_base /** * @dataProvider data_notification_email */ - public function test_notification_email($post_data, $expected_users) + public function test_notification_email($notification_type, $post_data, $expected_users) { $post_data = array_merge(['post_time' => 1349413322], $post_data); $notification_options = [ @@ -209,21 +255,21 @@ class notification_method_email_test extends phpbb_tests_notification_base 'item_parent_id' => $post_data['topic_id'], ]; - $notified_users = $this->notification_method_email->get_notified_users($this->notifications->get_notification_type_id('notification.type.post'), $notification_options); + $notified_users = $this->notification_method_email->get_notified_users($this->notifications->get_notification_type_id($notification_type), $notification_options); $this->assertEquals(0, count($notified_users), 'Assert no user has been notified yet'); - $this->notifications->add_notifications('notification.type.post', $post_data); + $this->notifications->add_notifications($notification_type, $post_data); - $notified_users = $this->notification_method_email->get_notified_users($this->notifications->get_notification_type_id('notification.type.post'), $notification_options); + $notified_users = $this->notification_method_email->get_notified_users($this->notifications->get_notification_type_id($notification_type), $notification_options); $this->assertEquals($expected_users, $notified_users, 'Assert that expected users have been notified'); $post_data['post_id']++; $notification_options['item_id'] = $post_data['post_id']; $post_data['post_time'] = 1349413323; - $this->notifications->add_notifications('notification.type.post', $post_data); + $this->notifications->add_notifications($notification_type, $post_data); - $notified_users2 = $this->notification_method_email->get_notified_users($this->notifications->get_notification_type_id('notification.type.post'), $notification_options); + $notified_users2 = $this->notification_method_email->get_notified_users($this->notifications->get_notification_type_id($notification_type), $notification_options); $this->assertEquals($expected_users, $notified_users2, 'Assert that expected users stay the same after replying to same topic'); } } diff --git a/tests/notification/notification_test.php b/tests/notification/notification_test.php index 6bbabfc602..24a9d2e686 100644 --- a/tests/notification/notification_test.php +++ b/tests/notification/notification_test.php @@ -72,6 +72,7 @@ class phpbb_notification_test extends phpbb_tests_notification_base public function test_subscriptions() { $expected_subscriptions = array( + 'notification.type.forum' => array('notification.method.board'), 'notification.type.post' => array('notification.method.board'), 'notification.type.topic' => array('notification.method.board'), 'notification.type.quote' => array('notification.method.board'), diff --git a/tests/notification/submit_post_type_forum_test.php b/tests/notification/submit_post_type_forum_test.php new file mode 100644 index 0000000000..1635f65863 --- /dev/null +++ b/tests/notification/submit_post_type_forum_test.php @@ -0,0 +1,109 @@ +<?php +/** +* +* This file is part of the phpBB Forum Software package. +* +* @copyright (c) phpBB Limited <https://www.phpbb.com> +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +require_once dirname(__FILE__) . '/submit_post_base.php'; + +class phpbb_notification_submit_post_type_forum_test extends phpbb_notification_submit_post_base +{ + protected $item_type = 'notification.type.forum'; + + public function setUp(): void + { + parent::setUp(); + + global $auth; + + // Add additional permissions + $auth->expects($this->any()) + ->method('acl_get_list') + ->with($this->anything(), + $this->stringContains('_'), + $this->greaterThan(0)) + ->will($this->returnValueMap(array( + array( + array(3, 4, 5, 6), + 'f_read', + 1, + array( + 1 => array( + 'f_read' => array(3, 5, 6), + ), + ), + ), + array( + array(3, 4, 5, 6, 7, 8), + 'f_read', + 1, + array( + 1 => array( + 'f_read' => array(3, 5, 6, 7, 8), + ), + ), + ), + ))); + } + + /** + * submit_post() Notifications test + * + * submit_post() $mode = 'reply' + * Notification item_type = 'notification.type.forum' + */ + public function submit_post_data() + { + return array( + /** + * Normal post + * + * User => State description + * 2 => Poster, should NOT receive a notification + * 3 => Forum subscribed, should receive a notification + * 4 => Forum subscribed, but unauthed to read, should NOT receive a notification + * 5 => Forum subscribed, but already notified, should NOT receive a new notification + * 6 => Topic and forum subscribed, should receive ONE notification + * 7 => Topic subscribed, should NOT receive a notification + * 8 => Topic subscribed, and already notified, should NOT receive a new notification + */ + array( + array(), + array( + array('user_id' => 5, 'item_id' => 1, 'item_parent_id' => 1), + array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1), + ), + array( + array('user_id' => 3, 'item_id' => 2, 'item_parent_id' => 1), + array('user_id' => 5, 'item_id' => 1, 'item_parent_id' => 1), + array('user_id' => 6, 'item_id' => 2, 'item_parent_id' => 1), + array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1), + ), + ), + + /** + * Unapproved post + * + * No new notifications + */ + array( + array('force_approved_state' => false), + array( + array('user_id' => 5, 'item_id' => 1, 'item_parent_id' => 1), + array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1), + ), + array( + array('user_id' => 5, 'item_id' => 1, 'item_parent_id' => 1), + array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1), + ), + ), + ); + } +} diff --git a/tests/notification/submit_post_type_post_test.php b/tests/notification/submit_post_type_post_test.php index 5580d0924e..9d94358f99 100644 --- a/tests/notification/submit_post_type_post_test.php +++ b/tests/notification/submit_post_type_post_test.php @@ -30,6 +30,16 @@ class phpbb_notification_submit_post_type_post_test extends phpbb_notification_s $this->stringContains('_'), $this->greaterThan(0)) ->will($this->returnValueMap(array( + array( + array(3, 4, 5, 6), + 'f_read', + 1, + array( + 1 => array( + 'f_read' => array(3, 5, 6), + ), + ), + ), array( array(3, 4, 5, 6, 7, 8), 'f_read', @@ -61,8 +71,8 @@ class phpbb_notification_submit_post_type_post_test extends phpbb_notification_s * 4 => Topic subscribed, but unauthed to read, should NOT receive a notification * 5 => Topic subscribed, but already notified, should NOT receive a new notification * 6 => Topic and forum subscribed, should receive ONE notification - * 7 => Forum subscribed, should receive a notification - * 8 => Forum subscribed, but already notified, should NOT receive a new notification + * 7 => Forum subscribed, should NOT receive a notification + * 8 => Forum subscribed, and already notified, should NOT receive a new notification */ array( array(), @@ -74,7 +84,6 @@ class phpbb_notification_submit_post_type_post_test extends phpbb_notification_s array('user_id' => 3, 'item_id' => 2, 'item_parent_id' => 1), array('user_id' => 5, 'item_id' => 1, 'item_parent_id' => 1), array('user_id' => 6, 'item_id' => 2, 'item_parent_id' => 1), - array('user_id' => 7, 'item_id' => 2, 'item_parent_id' => 1), array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1), ), ), From 8e2dd65c6e46d206b61fc9c0f43b925d3ef09512 Mon Sep 17 00:00:00 2001 From: rxu <rxu@mail.ru> Date: Wed, 2 Sep 2020 22:10:37 +0700 Subject: [PATCH 3/3] [ticket/16544] Correctly mark notification read PHPBB3-16544 --- phpBB/includes/functions.php | 3 + phpBB/phpbb/notification/type/forum.php | 77 +++++++++++++------------ 2 files changed, 42 insertions(+), 38 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index eb68c547d1..7341a24e16 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -580,6 +580,7 @@ function markread($mode, $forum_id = false, $topic_id = false, $post_time = 0, $ 'notification.type.post', 'notification.type.approve_topic', 'notification.type.approve_post', + 'notification.type.forum', ), false, $user->data['user_id'], $post_time); if ($config['load_db_lastread'] && $user->data['is_registered']) @@ -663,6 +664,7 @@ function markread($mode, $forum_id = false, $topic_id = false, $post_time = 0, $ 'notification.type.bookmark', 'notification.type.post', 'notification.type.approve_post', + 'notification.type.forum', ), $topic_ids, $user->data['user_id'], $post_time); // Add 0 to forums array to mark global announcements correctly @@ -773,6 +775,7 @@ function markread($mode, $forum_id = false, $topic_id = false, $post_time = 0, $ 'notification.type.bookmark', 'notification.type.post', 'notification.type.approve_post', + 'notification.type.forum', ), $topic_id, $user->data['user_id'], $post_time); if ($config['load_db_lastread'] && $user->data['is_registered']) diff --git a/phpBB/phpbb/notification/type/forum.php b/phpBB/phpbb/notification/type/forum.php index 3c1c4579f1..161ec1e780 100644 --- a/phpBB/phpbb/notification/type/forum.php +++ b/phpBB/phpbb/notification/type/forum.php @@ -1,54 +1,54 @@ <?php /** -* -* This file is part of the phpBB Forum Software package. -* -* @copyright (c) phpBB Limited <https://www.phpbb.com> -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited <https://www.phpbb.com> + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\notification\type; /** -* Forum notifications class -* This class handles notifications for replies to a topic in a forum user subscribed to -*/ + * Forum notifications class + * This class handles notifications for replies to a topic in a forum user subscribed to + */ class forum extends \phpbb\notification\type\post { /** - * Get notification type name - * - * @return string - */ + * Get notification type name + * + * @return string + */ public function get_type() { return 'notification.type.forum'; } /** - * Notification option data (for outputting to the user) - * - * @var bool|array False if the service should use it's default data - * Array of data (including keys 'id', 'lang', and 'group') - */ + * Notification option data (for outputting to the user) + * + * @var bool|array False if the service should use its default data + * Array of data (including keys 'id', 'lang', and 'group') + */ static public $notification_option = [ 'lang' => 'NOTIFICATION_TYPE_FORUM', 'group' => 'NOTIFICATION_GROUP_POSTING', ]; /** - * Find the users who want to receive notifications - * - * @param array $post Data from submit_post - * @param array $options Options for finding users for notification - * - * @return array - */ + * Find the users who want to receive notifications + * + * @param array $post Data from submit_post + * @param array $options Options for finding users for notification + * + * @return array + */ public function find_users_for_notification($post, $options = []) { $options = array_merge([ @@ -76,7 +76,8 @@ class forum extends \phpbb\notification\type\post return []; } - // Try to find the users who already have been notified about replies and have not read the topic since and just update their notifications + // Try to find the users who already have been notified about replies and have not read them + // Just update their notifications $notified_users = $this->notification_manager->get_notified_users($this->get_type(), [ 'item_parent_id' => static::get_item_parent_id($post), 'read' => 0, @@ -103,20 +104,20 @@ class forum extends \phpbb\notification\type\post } /** - * Get email template - * - * @return string|bool - */ + * Get email template + * + * @return string|bool + */ public function get_email_template() { return 'forum_notify'; } /** - * Get email template variables - * - * @return array - */ + * Get email template variables + * + * @return array + */ public function get_email_template_variables() { if ($this->get_data('post_username'))