From a3e0117ff0fdd7d832902b9f933569b490e16e62 Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Tue, 4 Sep 2018 14:23:55 +0200 Subject: [PATCH 01/10] [ticket/14754] Only one email notification per topic PHPBB3-14754 --- .../container/services_notification.yml | 6 ++ phpBB/phpbb/notification/method/email.php | 79 ++++++++++++++++++- .../notification/method/method_interface.php | 2 +- phpBB/phpbb/notification/type/post.php | 6 ++ tests/notification/base.php | 5 ++ tests/notification/submit_post_base.php | 5 ++ 6 files changed, 101 insertions(+), 2 deletions(-) diff --git a/phpBB/config/default/container/services_notification.yml b/phpBB/config/default/container/services_notification.yml index 6c3cea3dbc..a9db2bdd38 100644 --- a/phpBB/config/default/container/services_notification.yml +++ b/phpBB/config/default/container/services_notification.yml @@ -206,8 +206,14 @@ services: - '@user_loader' - '@user' - '@config' + - '@dbal.conn' - '%core.root_path%' - '%core.php_ext%' + - '%tables.topics_watch%' + - '%tables.topics_track%' + - '%tables.posts%' + - '%tables.forums_watch%' + - '%tables.forums_track%' tags: - { name: notification.method } diff --git a/phpBB/phpbb/notification/method/email.php b/phpBB/phpbb/notification/method/email.php index 6376d13dc7..66182f2d78 100644 --- a/phpBB/phpbb/notification/method/email.php +++ b/phpBB/phpbb/notification/method/email.php @@ -28,21 +28,51 @@ class email extends \phpbb\notification\method\messenger_base /** @var \phpbb\config\config */ protected $config; + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + + /** @var string */ + protected $topics_watch_table; + + /** @var string */ + protected $topics_track_table; + + /** @var string */ + protected $posts_table; + + /** @var string */ + protected $forums_watch_table; + + /** @var string */ + protected $forums_track_table; + /** * Notification Method email Constructor * * @param \phpbb\user_loader $user_loader * @param \phpbb\user $user * @param \phpbb\config\config $config + * @param \phpbb\db\driver\driver_interface $db * @param string $phpbb_root_path * @param string $php_ext + * @param string $topics_watch_table + * @param string $topics_track_table + * @param string $posts_table + * @param string $forums_watch_table + * @param string $forums_track_table */ - public function __construct(\phpbb\user_loader $user_loader, \phpbb\user $user, \phpbb\config\config $config, $phpbb_root_path, $php_ext) + public function __construct(\phpbb\user_loader $user_loader, \phpbb\user $user, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, $phpbb_root_path, $php_ext, $topics_watch_table, $topics_track_table, $posts_table, $forums_watch_table, $forums_track_table) { parent::__construct($user_loader, $phpbb_root_path, $php_ext); $this->user = $user; $this->config = $config; + $this->db = $db; + $this->topics_watch_table = $topics_watch_table; + $this->topics_track_table = $topics_track_table; + $this->posts_table = $posts_table; + $this->forums_watch_table = $forums_watch_table; + $this->forums_track_table = $forums_track_table; } /** @@ -68,6 +98,53 @@ class email extends \phpbb\notification\method\messenger_base return parent::is_available($notification_type) && $this->config['email_enable'] && !empty($this->user->data['user_email']); } + /** + * {@inheritdoc} + */ + public function get_notified_users($notification_type_id, array $options) + { + $notified_users = array(); + + if ($notification_type_id == 'notification.type.post' && !empty($options['item_parent_id'])) + { + // Topics watch + $sql = 'SELECT tw.user_id + FROM ' . $this->topics_watch_table . ' tw + LEFT JOIN ' . $this->topics_track_table . ' tt + ON (tt.user_id = tw.user_id AND tt.topic_id = tw.topic_id) + LEFT JOIN ' . $this->posts_table . ' p + ON (p.topic_id = tw.topic_id) + WHERE tw.topic_id = ' . (int) $options['item_parent_id'] . ' + AND p.post_time > tt.mark_time + HAVING COUNT(p.post_id) > 1'; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $notified_users[$row['user_id']] = $row; + } + $this->db->sql_freeresult($result); + + // Forums watch + $sql = 'SELECT fw.user_id + FROM ' . $this->forums_watch_table . ' fw + LEFT JOIN ' . $this->forums_track_table . ' ft + ON (ft.user_id = fw.user_id AND ft.forum_id = fw.forum_id) + LEFT JOIN ' . $this->posts_table . ' p + ON (p.forum_id = fw.forum_id) + WHERE p.topic_id = ' . (int) $options['item_parent_id'] . ' + AND p.post_time > ft.mark_time + HAVING COUNT(p.post_id) > 1'; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $notified_users[$row['user_id']] = $row; + } + $this->db->sql_freeresult($result); + } + + return $notified_users; + } + /** * Parse the queue and notify the users */ diff --git a/phpBB/phpbb/notification/method/method_interface.php b/phpBB/phpbb/notification/method/method_interface.php index c2e4940485..aa13bfde69 100644 --- a/phpBB/phpbb/notification/method/method_interface.php +++ b/phpBB/phpbb/notification/method/method_interface.php @@ -41,7 +41,7 @@ interface method_interface /** * Return the list of the users already notified * - * @param int $notification_type_id Type of the notification + * @param int $notification_type_id ID of the notification type * @param array $options * @return array User */ diff --git a/phpBB/phpbb/notification/type/post.php b/phpBB/phpbb/notification/type/post.php index f0e938d3ce..64ed91a5f5 100644 --- a/phpBB/phpbb/notification/type/post.php +++ b/phpBB/phpbb/notification/type/post.php @@ -457,6 +457,12 @@ class post extends \phpbb\notification\type\base } $data_array = array_merge(array( + 'poster_id' => $post['poster_id'], + 'topic_title' => $post['topic_title'], + 'post_subject' => $post['post_subject'], + 'post_username' => $post['post_username'], + 'forum_id' => $post['forum_id'], + 'forum_name' => $post['forum_name'], 'post_time' => $post['post_time'], 'post_id' => $post['post_id'], 'topic_id' => $post['topic_id'] diff --git a/tests/notification/base.php b/tests/notification/base.php index f7faf50d68..0e254a66fd 100644 --- a/tests/notification/base.php +++ b/tests/notification/base.php @@ -103,6 +103,11 @@ abstract class phpbb_tests_notification_base extends phpbb_database_test_case $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); $phpbb_container->setParameter('tables.user_notifications', 'phpbb_user_notifications'); $phpbb_container->setParameter('tables.notification_types', 'phpbb_notification_types'); + $phpbb_container->setParameter('tables.topics_watch', 'phpbb_topics_watch'); + $phpbb_container->setParameter('tables.topics_track', 'phpbb_topics_track'); + $phpbb_container->setParameter('tables.posts', 'phpbb_posts'); + $phpbb_container->setParameter('tables.forums_watch', 'phpbb_forums_watch'); + $phpbb_container->setParameter('tables.forums_track', 'phpbb_forums_track'); $this->notifications = new phpbb_notification_manager_helper( array(), diff --git a/tests/notification/submit_post_base.php b/tests/notification/submit_post_base.php index 466d3ec07f..6f8e10f357 100644 --- a/tests/notification/submit_post_base.php +++ b/tests/notification/submit_post_base.php @@ -130,6 +130,11 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); $phpbb_container->setParameter('tables.user_notifications', 'phpbb_user_notifications'); $phpbb_container->setParameter('tables.notification_types', 'phpbb_notification_types'); + $phpbb_container->setParameter('tables.topics_watch', 'phpbb_topics_watch'); + $phpbb_container->setParameter('tables.topics_track', 'phpbb_topics_track'); + $phpbb_container->setParameter('tables.posts', 'phpbb_posts'); + $phpbb_container->setParameter('tables.forums_watch', 'phpbb_forums_watch'); + $phpbb_container->setParameter('tables.forums_track', 'phpbb_forums_track'); $phpbb_container->set('content.visibility', new \phpbb\content_visibility($auth, $config, $phpbb_dispatcher, $db, $user, $phpbb_root_path, $phpEx, FORUMS_TABLE, POSTS_TABLE, TOPICS_TABLE, USERS_TABLE)); $phpbb_container->compile(); From f3664b07d289d34c9181620ded665845cf61d282 Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Thu, 18 Oct 2018 09:35:16 +0200 Subject: [PATCH 02/10] [ticket/14754] Use dedicated table to stop receiving notifications PHPBB3-14754 --- .../container/services_notification.yml | 6 +- phpBB/config/default/container/tables.yml | 1 + .../v32x/add_email_notifications_table.php | 50 +++++++ phpBB/phpbb/notification/method/email.php | 125 ++++++++++-------- tests/notification/base.php | 6 +- tests/notification/submit_post_base.php | 6 +- 6 files changed, 121 insertions(+), 73 deletions(-) create mode 100644 phpBB/phpbb/db/migration/data/v32x/add_email_notifications_table.php diff --git a/phpBB/config/default/container/services_notification.yml b/phpBB/config/default/container/services_notification.yml index a9db2bdd38..a10330143f 100644 --- a/phpBB/config/default/container/services_notification.yml +++ b/phpBB/config/default/container/services_notification.yml @@ -209,11 +209,7 @@ services: - '@dbal.conn' - '%core.root_path%' - '%core.php_ext%' - - '%tables.topics_watch%' - - '%tables.topics_track%' - - '%tables.posts%' - - '%tables.forums_watch%' - - '%tables.forums_track%' + - '%tables.email_notifications%' tags: - { name: notification.method } diff --git a/phpBB/config/default/container/tables.yml b/phpBB/config/default/container/tables.yml index 4aed35710b..de082043fd 100644 --- a/phpBB/config/default/container/tables.yml +++ b/phpBB/config/default/container/tables.yml @@ -20,6 +20,7 @@ parameters: tables.confirm: '%core.table_prefix%confirm' tables.disallow: '%core.table_prefix%disallow' tables.drafts: '%core.table_prefix%drafts' + tables.email_notifications: '%core.table_prefix%email_notifications' tables.ext: '%core.table_prefix%ext' tables.extensions: '%core.table_prefix%extensions' tables.extension_groups: '%core.table_prefix%extension_groups' diff --git a/phpBB/phpbb/db/migration/data/v32x/add_email_notifications_table.php b/phpBB/phpbb/db/migration/data/v32x/add_email_notifications_table.php new file mode 100644 index 0000000000..46a9e45f67 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v32x/add_email_notifications_table.php @@ -0,0 +1,50 @@ + + * @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\db\migration\data\v32x; + +class add_email_notifications_table extends \phpbb\db\migration\migration +{ + static public function depends_on() + { + return array( + '\phpbb\db\migration\data\v32x\v323', + ); + } + + public function update_schema() + { + return array( + 'add_tables' => array( + $this->table_prefix . 'email_notifications' => array( + 'COLUMNS' => array( + 'notification_type_id' => array('USINT', 0), + 'item_id' => array('UINT', 0), + 'item_parent_id' => array('UINT', 0), + 'user_id' => array('UINT', 0), + ), + 'PRIMARY_KEY' => array('notification_type_id', 'item_id', 'item_parent_id', 'user_id'), + ), + ), + ); + } + + public function revert_schema() + { + return array( + 'drop_tables' => array( + $this->table_prefix . 'email_notifications', + ), + ); + } +} diff --git a/phpBB/phpbb/notification/method/email.php b/phpBB/phpbb/notification/method/email.php index 66182f2d78..10318ea215 100644 --- a/phpBB/phpbb/notification/method/email.php +++ b/phpBB/phpbb/notification/method/email.php @@ -32,19 +32,7 @@ class email extends \phpbb\notification\method\messenger_base protected $db; /** @var string */ - protected $topics_watch_table; - - /** @var string */ - protected $topics_track_table; - - /** @var string */ - protected $posts_table; - - /** @var string */ - protected $forums_watch_table; - - /** @var string */ - protected $forums_track_table; + protected $email_notifications_table; /** * Notification Method email Constructor @@ -55,24 +43,16 @@ class email extends \phpbb\notification\method\messenger_base * @param \phpbb\db\driver\driver_interface $db * @param string $phpbb_root_path * @param string $php_ext - * @param string $topics_watch_table - * @param string $topics_track_table - * @param string $posts_table - * @param string $forums_watch_table - * @param string $forums_track_table + * @param string $email_notifications_table */ - public function __construct(\phpbb\user_loader $user_loader, \phpbb\user $user, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, $phpbb_root_path, $php_ext, $topics_watch_table, $topics_track_table, $posts_table, $forums_watch_table, $forums_track_table) + public function __construct(\phpbb\user_loader $user_loader, \phpbb\user $user, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, $phpbb_root_path, $php_ext, $email_notifications_table) { parent::__construct($user_loader, $phpbb_root_path, $php_ext); $this->user = $user; $this->config = $config; $this->db = $db; - $this->topics_watch_table = $topics_watch_table; - $this->topics_track_table = $topics_track_table; - $this->posts_table = $posts_table; - $this->forums_watch_table = $forums_watch_table; - $this->forums_track_table = $forums_track_table; + $this->email_notifications_table = $email_notifications_table; } /** @@ -105,42 +85,18 @@ class email extends \phpbb\notification\method\messenger_base { $notified_users = array(); - if ($notification_type_id == 'notification.type.post' && !empty($options['item_parent_id'])) + $sql = 'SELECT user_id + FROM ' . $this->email_notifications_table . ' + WHERE notification_type_id = ' . (int) $notification_type_id . + (isset($options['item_id']) ? ' AND item_id = ' . (int) $options['item_id'] : '') . + (isset($options['item_parent_id']) ? ' AND item_parent_id = ' . (int) $options['item_parent_id'] : '') . + (isset($options['user_id']) ? ' AND user_id = ' . (int) $options['user_id'] : ''); + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) { - // Topics watch - $sql = 'SELECT tw.user_id - FROM ' . $this->topics_watch_table . ' tw - LEFT JOIN ' . $this->topics_track_table . ' tt - ON (tt.user_id = tw.user_id AND tt.topic_id = tw.topic_id) - LEFT JOIN ' . $this->posts_table . ' p - ON (p.topic_id = tw.topic_id) - WHERE tw.topic_id = ' . (int) $options['item_parent_id'] . ' - AND p.post_time > tt.mark_time - HAVING COUNT(p.post_id) > 1'; - $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $notified_users[$row['user_id']] = $row; - } - $this->db->sql_freeresult($result); - - // Forums watch - $sql = 'SELECT fw.user_id - FROM ' . $this->forums_watch_table . ' fw - LEFT JOIN ' . $this->forums_track_table . ' ft - ON (ft.user_id = fw.user_id AND ft.forum_id = fw.forum_id) - LEFT JOIN ' . $this->posts_table . ' p - ON (p.forum_id = fw.forum_id) - WHERE p.topic_id = ' . (int) $options['item_parent_id'] . ' - AND p.post_time > ft.mark_time - HAVING COUNT(p.post_id) > 1'; - $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $notified_users[$row['user_id']] = $row; - } - $this->db->sql_freeresult($result); + $notified_users[$row['user_id']] = $row; } + $this->db->sql_freeresult($result); return $notified_users; } @@ -150,6 +106,59 @@ class email extends \phpbb\notification\method\messenger_base */ public function notify() { + $insert_buffer = new \phpbb\db\sql_insert_buffer($this->db, $this->email_notifications_table); + + /** @var \phpbb\notification\type\type_interface $notification */ + foreach ($this->queue as $notification) + { + $data = $this->clean_data($notification->get_insert_array()); + $insert_buffer->insert($data); + } + + $insert_buffer->flush(); + return $this->notify_using_messenger(NOTIFY_EMAIL); } + + /** + * {@inheritdoc} + */ + public function mark_notifications($notification_type_id, $item_id, $user_id, $time = false, $mark_read = true) + { + $sql = 'DELETE FROM ' . $this->email_notifications_table . ' + WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '') . + (($user_id !== false) ? ' AND ' . (is_array($user_id) ? $this->db->sql_in_set('user_id', $user_id) : 'user_id = ' . (int) $user_id) : '') . + (($item_id !== false) ? ' AND ' . (is_array($item_id) ? $this->db->sql_in_set('item_id', $item_id) : 'item_id = ' . (int) $item_id) : ''); + $this->db->sql_query($sql); + } + + /** + * {@inheritdoc} + */ + public function mark_notifications_by_parent($notification_type_id, $item_parent_id, $user_id, $time = false, $mark_read = true) + { + $sql = 'DELETE FROM ' . $this->email_notifications_table . ' + WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '') . + (($user_id !== false) ? ' AND ' . (is_array($user_id) ? $this->db->sql_in_set('user_id', $user_id) : 'user_id = ' . (int) $user_id) : '') . + (($item_parent_id !== false) ? ' AND ' . (is_array($item_parent_id) ? $this->db->sql_in_set('item_parent_id', $item_parent_id, false, true) : 'item_parent_id = ' . (int) $item_parent_id) : ''); + $this->db->sql_query($sql); + } + + /** + * Clean data to contain only what we need for email notifications table + * + * @param array $data Notification data + * @return array Cleaned notification data + */ + protected function clean_data(array $data) + { + $model = array( + 'notification_type_id' => null, + 'item_id' => null, + 'item_parent_id' => null, + 'user_id' => null, + ); + + return array_intersect_key($data, $model); + } } diff --git a/tests/notification/base.php b/tests/notification/base.php index 0e254a66fd..5990da011f 100644 --- a/tests/notification/base.php +++ b/tests/notification/base.php @@ -103,11 +103,7 @@ abstract class phpbb_tests_notification_base extends phpbb_database_test_case $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); $phpbb_container->setParameter('tables.user_notifications', 'phpbb_user_notifications'); $phpbb_container->setParameter('tables.notification_types', 'phpbb_notification_types'); - $phpbb_container->setParameter('tables.topics_watch', 'phpbb_topics_watch'); - $phpbb_container->setParameter('tables.topics_track', 'phpbb_topics_track'); - $phpbb_container->setParameter('tables.posts', 'phpbb_posts'); - $phpbb_container->setParameter('tables.forums_watch', 'phpbb_forums_watch'); - $phpbb_container->setParameter('tables.forums_track', 'phpbb_forums_track'); + $phpbb_container->setParameter('tables.email_notifications', 'phpbb_email_notifications'); $this->notifications = new phpbb_notification_manager_helper( array(), diff --git a/tests/notification/submit_post_base.php b/tests/notification/submit_post_base.php index 6f8e10f357..edcbef76e2 100644 --- a/tests/notification/submit_post_base.php +++ b/tests/notification/submit_post_base.php @@ -130,11 +130,7 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); $phpbb_container->setParameter('tables.user_notifications', 'phpbb_user_notifications'); $phpbb_container->setParameter('tables.notification_types', 'phpbb_notification_types'); - $phpbb_container->setParameter('tables.topics_watch', 'phpbb_topics_watch'); - $phpbb_container->setParameter('tables.topics_track', 'phpbb_topics_track'); - $phpbb_container->setParameter('tables.posts', 'phpbb_posts'); - $phpbb_container->setParameter('tables.forums_watch', 'phpbb_forums_watch'); - $phpbb_container->setParameter('tables.forums_track', 'phpbb_forums_track'); + $phpbb_container->setParameter('tables.email_notifications', 'phpbb_email_notifications'); $phpbb_container->set('content.visibility', new \phpbb\content_visibility($auth, $config, $phpbb_dispatcher, $db, $user, $phpbb_root_path, $phpEx, FORUMS_TABLE, POSTS_TABLE, TOPICS_TABLE, USERS_TABLE)); $phpbb_container->compile(); From 82240cb6616027588a744df70a6c7111c3b517ef Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Mon, 22 Oct 2018 08:27:10 +0200 Subject: [PATCH 03/10] [ticket/14754] Fix SQL errors in email::mark_notifications PHPBB3-14754 --- phpBB/phpbb/notification/method/email.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/notification/method/email.php b/phpBB/phpbb/notification/method/email.php index 10318ea215..26855603cb 100644 --- a/phpBB/phpbb/notification/method/email.php +++ b/phpBB/phpbb/notification/method/email.php @@ -126,7 +126,7 @@ class email extends \phpbb\notification\method\messenger_base public function mark_notifications($notification_type_id, $item_id, $user_id, $time = false, $mark_read = true) { $sql = 'DELETE FROM ' . $this->email_notifications_table . ' - WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '') . + WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '1=1') . (($user_id !== false) ? ' AND ' . (is_array($user_id) ? $this->db->sql_in_set('user_id', $user_id) : 'user_id = ' . (int) $user_id) : '') . (($item_id !== false) ? ' AND ' . (is_array($item_id) ? $this->db->sql_in_set('item_id', $item_id) : 'item_id = ' . (int) $item_id) : ''); $this->db->sql_query($sql); @@ -138,7 +138,7 @@ class email extends \phpbb\notification\method\messenger_base public function mark_notifications_by_parent($notification_type_id, $item_parent_id, $user_id, $time = false, $mark_read = true) { $sql = 'DELETE FROM ' . $this->email_notifications_table . ' - WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '') . + WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '1=1') . (($user_id !== false) ? ' AND ' . (is_array($user_id) ? $this->db->sql_in_set('user_id', $user_id) : 'user_id = ' . (int) $user_id) : '') . (($item_parent_id !== false) ? ' AND ' . (is_array($item_parent_id) ? $this->db->sql_in_set('item_parent_id', $item_parent_id, false, true) : 'item_parent_id = ' . (int) $item_parent_id) : ''); $this->db->sql_query($sql); From 5aa4aae0f56c71c1ac52f8e0afa0bd88d1cf8023 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 15 Mar 2020 11:51:53 +0100 Subject: [PATCH 04/10] [ticket/14754] Add test for email notifications PHPBB3-14754 --- tests/notification/base.php | 4 +- .../fixtures/email_notification.type.post.xml | 217 +++++++++++++++++ .../notification_method_email_test.php | 229 ++++++++++++++++++ 3 files changed, 449 insertions(+), 1 deletion(-) create mode 100644 tests/notification/fixtures/email_notification.type.post.xml create mode 100644 tests/notification/notification_method_email_test.php diff --git a/tests/notification/base.php b/tests/notification/base.php index 5990da011f..5c923133f8 100644 --- a/tests/notification/base.php +++ b/tests/notification/base.php @@ -19,7 +19,9 @@ require_once dirname(__FILE__) . '/manager_helper.php'; abstract class phpbb_tests_notification_base extends phpbb_database_test_case { - protected $notifications, $db, $container, $user, $config, $auth, $cache; + /** @var phpbb_notification_manager_helper */ + protected $notifications; + protected $db, $container, $user, $config, $auth, $cache; protected function get_notification_types() { diff --git a/tests/notification/fixtures/email_notification.type.post.xml b/tests/notification/fixtures/email_notification.type.post.xml new file mode 100644 index 0000000000..8787f5bbe5 --- /dev/null +++ b/tests/notification/fixtures/email_notification.type.post.xml @@ -0,0 +1,217 @@ + + + + forum_id + user_id + notify_status + + 1 + 6 + 0 + + + 1 + 7 + 0 + + + 1 + 8 + 0 + +
+ + notification_id + notification_type_id + user_id + item_id + item_parent_id + notification_read + notification_data + + 1 + 1 + 5 + 1 + 1 + 0 + + + + 2 + 1 + 8 + 1 + 1 + 0 + + +
+ + notification_type_id + notification_type_name + notification_type_enabled + + 1 + notification.type.post + 1 + +
+ + post_id + topic_id + forum_id + post_text + + 1 + 1 + 1 + + +
+ + topic_id + forum_id + + 1 + 1 + + + 2 + 1 + +
+ + topic_id + user_id + notify_status + + 1 + 2 + 0 + + + 2 + 2 + 0 + + + 1 + 3 + 0 + + + 1 + 4 + 0 + + + 1 + 5 + 0 + + + 1 + 6 + 0 + +
+ + user_id + username_clean + user_permissions + user_sig + + 2 + poster + + + + + 3 + test + + + + + 4 + unauthorized + + + + + 5 + notified + + + + + 6 + disabled + + + + + 7 + default + + + +
+ + item_type + item_id + user_id + method + notify + + notification.type.post + 0 + 2 + notification.method.email + 1 + + + notification.type.post + 0 + 3 + notification.method.email + 1 + + + notification.type.post + 0 + 4 + notification.method.email + 1 + + + notification.type.post + 0 + 5 + notification.method.email + 1 + + + notification.type.post + 0 + 6 + notification.method.email + 1 + + + notification.type.post + 0 + 7 + notification.method.email + 1 + + + notification.type.post + 0 + 8 + notification.method.email + 1 + +
+ diff --git a/tests/notification/notification_method_email_test.php b/tests/notification/notification_method_email_test.php new file mode 100644 index 0000000000..94b816434c --- /dev/null +++ b/tests/notification/notification_method_email_test.php @@ -0,0 +1,229 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; + +require_once dirname(__FILE__) . '/base.php'; + +class notification_method_email_test extends phpbb_tests_notification_base +{ + /** @var \phpbb\notification\method\email */ + protected $notification_method_email; + + public function getDataSet() + { + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/email_notification.type.post.xml'); + } + + protected function get_notification_methods() + { + return array( + 'notification.method.email', + ); + } + + protected function setUp() + { + phpbb_database_test_case::setUp(); + + global $phpbb_root_path, $phpEx; + + include_once(__DIR__ . '/ext/test/notification/type/test.' . $phpEx); + + global $db, $config, $user, $auth, $cache, $phpbb_container; + + $db = $this->db = $this->new_dbal(); + $config = $this->config = new \phpbb\config\config(array( + 'allow_privmsg' => true, + 'allow_bookmarks' => true, + 'allow_topic_notify' => true, + 'allow_forum_notify' => true, + 'allow_board_notifications' => true, + )); + $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); + $lang = new \phpbb\language\language($lang_loader); + $user = new \phpbb\user($lang, '\phpbb\datetime'); + $this->user = $user; + $this->user_loader = new \phpbb\user_loader($this->db, $phpbb_root_path, $phpEx, 'phpbb_users'); + $auth = $this->auth = new phpbb_mock_notifications_auth(); + $cache_driver = new \phpbb\cache\driver\dummy(); + $cache = $this->cache = new \phpbb\cache\service( + $cache_driver, + $this->config, + $this->db, + $phpbb_root_path, + $phpEx + ); + + $this->phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + + $phpbb_container = $this->container = new ContainerBuilder(); + $loader = new YamlFileLoader($phpbb_container, new FileLocator(__DIR__ . '/fixtures')); + $loader->load('services_notification.yml'); + $phpbb_container->set('user_loader', $this->user_loader); + $phpbb_container->set('user', $user); + $phpbb_container->set('language', $lang); + $phpbb_container->set('config', $this->config); + $phpbb_container->set('dbal.conn', $this->db); + $phpbb_container->set('auth', $auth); + $phpbb_container->set('cache.driver', $cache_driver); + $phpbb_container->set('cache', $cache); + $phpbb_container->set('text_formatter.utils', new \phpbb\textformatter\s9e\utils()); + $phpbb_container->set('dispatcher', $this->phpbb_dispatcher); + $phpbb_container->setParameter('core.root_path', $phpbb_root_path); + $phpbb_container->setParameter('core.php_ext', $phpEx); + $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); + $phpbb_container->setParameter('tables.user_notifications', 'phpbb_user_notifications'); + $phpbb_container->setParameter('tables.notification_types', 'phpbb_notification_types'); + $phpbb_container->setParameter('tables.email_notifications', 'phpbb_email_notifications'); + + $this->notification_method_email = $this->getMockBuilder('\phpbb\notification\method\email') + ->setConstructorArgs([ + $phpbb_container->get('user_loader'), + $phpbb_container->get('user'), + $phpbb_container->get('config'), + $phpbb_container->get('dbal.conn'), + $phpbb_root_path, + $phpEx, + $phpbb_container->getParameter('tables.email_notifications') + ]) + ->setMethods(['notify_using_messenger']) + ->getMock(); + $notification_method_email = $this->notification_method_email; + + $class = new ReflectionClass($notification_method_email); + $empty_queue_method = $class->getMethod('empty_queue'); + $empty_queue_method->setAccessible(true); + + $this->notification_method_email->method('notify_using_messenger') + ->will($this->returnCallback(function () use ($notification_method_email, $empty_queue_method) { + $empty_queue_method->invoke($notification_method_email); + })); + + $phpbb_container->set('notification.method.email', $this->notification_method_email); + + $this->notifications = new phpbb_notification_manager_helper( + array(), + array(), + $this->container, + $this->user_loader, + $this->phpbb_dispatcher, + $this->db, + $this->cache, + $lang, + $this->user, + 'phpbb_notification_types', + 'phpbb_user_notifications' + ); + + $phpbb_container->set('notification_manager', $this->notifications); + $phpbb_container->compile(); + + $this->notifications->setDependencies($this->auth, $this->config); + + $types = array(); + foreach ($this->get_notification_types() as $type) + { + $class = $this->build_type($type); + + $types[$type] = $class; + } + + $this->notifications->set_var('notification_types', $types); + + $methods = array(); + foreach ($this->get_notification_methods() as $method) + { + $class = $this->container->get($method); + + $methods[$method] = $class; + } + + $this->notifications->set_var('notification_methods', $methods); + } + + public function data_notification_email() + { + return [ + [ + [ + 'forum_id' => '1', + 'post_id' => '2', + 'topic_id' => '1', + ], + [ + 2 => ['user_id' => '2'], + 3 => ['user_id' => '3'], + 4 => ['user_id' => '4'], + 5 => ['user_id' => '5'], + 6 => ['user_id' => '6'], + 7 => ['user_id' => '7'], + 8 => ['user_id' => '8'] + ], + ], + [ + [ + 'forum_id' => '1', + 'post_id' => '4', + 'topic_id' => '2', + ], + [ + 2 => ['user_id' => '2'], + 6 => ['user_id' => '6'], + 7 => ['user_id' => '7'], + 8 => ['user_id' => '8'], + ] + ], + [ + [ + 'forum_id' => '2', + 'post_id' => '6', + 'topic_id' => '3', + ], + [ + ] + ], + ]; + } + + /** + * @dataProvider data_notification_email + */ + public function test_notification_email($post_data, $expected_users) + { + $post_data = array_merge(['post_time' => 1349413322], $post_data); + $notification_options = [ + 'item_id' => $post_data['post_id'], + '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); + $this->assertEquals(0, count($notified_users), 'Assert no user has been notified yet'); + + $this->notifications->add_notifications('notification.type.post', $post_data); + + $notified_users = $this->notification_method_email->get_notified_users($this->notifications->get_notification_type_id('notification.type.post'), $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); + + $notified_users2 = $this->notification_method_email->get_notified_users($this->notifications->get_notification_type_id('notification.type.post'), $notification_options); + $this->assertEquals($expected_users, $notified_users2, 'Assert that expected users stay the same after replying to same topic'); + } +} From 858d9271e3822aaa54cfd9d6b24638282da0bf46 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 15 Mar 2020 12:40:35 +0100 Subject: [PATCH 05/10] [ticket/14754] Adjust setUp in test for 3.3 PHPBB3-14754 --- tests/notification/notification_method_email_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/notification/notification_method_email_test.php b/tests/notification/notification_method_email_test.php index 94b816434c..20ee0ef181 100644 --- a/tests/notification/notification_method_email_test.php +++ b/tests/notification/notification_method_email_test.php @@ -34,7 +34,7 @@ class notification_method_email_test extends phpbb_tests_notification_base ); } - protected function setUp() + protected function setUp() : void { phpbb_database_test_case::setUp(); From cc3afd87aab1c698b1b7020a8fa9dd2ffc4fa143 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 15 Mar 2020 12:47:14 +0100 Subject: [PATCH 06/10] [ticket/14754] Move migration for email notification table to 33x PHPBB3-14754 --- .../data/{v32x => v33x}/add_email_notifications_table.php | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename phpBB/phpbb/db/migration/data/{v32x => v33x}/add_email_notifications_table.php (100%) diff --git a/phpBB/phpbb/db/migration/data/v32x/add_email_notifications_table.php b/phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php similarity index 100% rename from phpBB/phpbb/db/migration/data/v32x/add_email_notifications_table.php rename to phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php From c61946329cb7b17ef4a6971d304dcb8f8957c1d2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 15 Mar 2020 12:50:39 +0100 Subject: [PATCH 07/10] [ticket/14754] Refactor migration for 3.3 and use ULINT PHPBB3-14754 --- .../v33x/add_email_notifications_table.php | 42 +++++++++---------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php b/phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php index 46a9e45f67..0ad172f5c0 100644 --- a/phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php +++ b/phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php @@ -11,40 +11,38 @@ * */ -namespace phpbb\db\migration\data\v32x; +namespace phpbb\db\migration\data\v33x; class add_email_notifications_table extends \phpbb\db\migration\migration { static public function depends_on() { - return array( - '\phpbb\db\migration\data\v32x\v323', - ); + return [ + '\phpbb\db\migration\data\v330\v330', + ]; } public function update_schema() { - return array( - 'add_tables' => array( - $this->table_prefix . 'email_notifications' => array( - 'COLUMNS' => array( - 'notification_type_id' => array('USINT', 0), - 'item_id' => array('UINT', 0), - 'item_parent_id' => array('UINT', 0), - 'user_id' => array('UINT', 0), - ), - 'PRIMARY_KEY' => array('notification_type_id', 'item_id', 'item_parent_id', 'user_id'), - ), - ), - ); + return [ + 'add_tables' => [ + $this->table_prefix . 'email_notifications' => [ + 'COLUMNS' => [ + 'notification_type_id' => ['USINT', 0], + 'item_id' => ['ULINT', 0], + 'item_parent_id' => ['ULINT', 0], + 'user_id' => ['ULINT', 0], + ], + 'PRIMARY_KEY' => ['notification_type_id', 'item_id', 'item_parent_id', 'user_id'], + ], + ], + ]; } public function revert_schema() { - return array( - 'drop_tables' => array( - $this->table_prefix . 'email_notifications', - ), - ); + return [ + 'drop_tables' => [$this->table_prefix . 'email_notifications'], + ]; } } From 9f4a240f9175d6cb18e476404a7c25891a467a29 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 15 Mar 2020 15:08:25 +0100 Subject: [PATCH 08/10] [ticket/14754] Rename email notifications table to fit better PHPBB3-14754 --- .../container/services_notification.yml | 2 +- phpBB/config/default/container/tables.yml | 2 +- ...e.php => add_notification_emails_table.php} | 6 +++--- phpBB/phpbb/notification/method/email.php | 18 +++++++++--------- tests/notification/base.php | 2 +- .../notification_method_email_test.php | 4 ++-- tests/notification/submit_post_base.php | 2 +- 7 files changed, 18 insertions(+), 18 deletions(-) rename phpBB/phpbb/db/migration/data/v33x/{add_email_notifications_table.php => add_notification_emails_table.php} (82%) diff --git a/phpBB/config/default/container/services_notification.yml b/phpBB/config/default/container/services_notification.yml index a10330143f..41f80cdd30 100644 --- a/phpBB/config/default/container/services_notification.yml +++ b/phpBB/config/default/container/services_notification.yml @@ -209,7 +209,7 @@ services: - '@dbal.conn' - '%core.root_path%' - '%core.php_ext%' - - '%tables.email_notifications%' + - '%tables.notification_emails%' tags: - { name: notification.method } diff --git a/phpBB/config/default/container/tables.yml b/phpBB/config/default/container/tables.yml index de082043fd..a2cf0b1a34 100644 --- a/phpBB/config/default/container/tables.yml +++ b/phpBB/config/default/container/tables.yml @@ -20,7 +20,6 @@ parameters: tables.confirm: '%core.table_prefix%confirm' tables.disallow: '%core.table_prefix%disallow' tables.drafts: '%core.table_prefix%drafts' - tables.email_notifications: '%core.table_prefix%email_notifications' tables.ext: '%core.table_prefix%ext' tables.extensions: '%core.table_prefix%extensions' tables.extension_groups: '%core.table_prefix%extension_groups' @@ -36,6 +35,7 @@ parameters: tables.migrations: '%core.table_prefix%migrations' tables.moderator_cache: '%core.table_prefix%moderator_cache' tables.modules: '%core.table_prefix%modules' + tables.notification_emails: '%core.table_prefix%notification_emails' tables.notification_types: '%core.table_prefix%notification_types' tables.notifications: '%core.table_prefix%notifications' tables.poll_options: '%core.table_prefix%poll_options' diff --git a/phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php b/phpBB/phpbb/db/migration/data/v33x/add_notification_emails_table.php similarity index 82% rename from phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php rename to phpBB/phpbb/db/migration/data/v33x/add_notification_emails_table.php index 0ad172f5c0..5a5f4dd354 100644 --- a/phpBB/phpbb/db/migration/data/v33x/add_email_notifications_table.php +++ b/phpBB/phpbb/db/migration/data/v33x/add_notification_emails_table.php @@ -13,7 +13,7 @@ namespace phpbb\db\migration\data\v33x; -class add_email_notifications_table extends \phpbb\db\migration\migration +class add_notification_emails_table extends \phpbb\db\migration\migration { static public function depends_on() { @@ -26,7 +26,7 @@ class add_email_notifications_table extends \phpbb\db\migration\migration { return [ 'add_tables' => [ - $this->table_prefix . 'email_notifications' => [ + $this->table_prefix . 'notification_emails' => [ 'COLUMNS' => [ 'notification_type_id' => ['USINT', 0], 'item_id' => ['ULINT', 0], @@ -42,7 +42,7 @@ class add_email_notifications_table extends \phpbb\db\migration\migration public function revert_schema() { return [ - 'drop_tables' => [$this->table_prefix . 'email_notifications'], + 'drop_tables' => [$this->table_prefix . 'notification_emails'], ]; } } diff --git a/phpBB/phpbb/notification/method/email.php b/phpBB/phpbb/notification/method/email.php index 26855603cb..61777e3c49 100644 --- a/phpBB/phpbb/notification/method/email.php +++ b/phpBB/phpbb/notification/method/email.php @@ -31,8 +31,8 @@ class email extends \phpbb\notification\method\messenger_base /** @var \phpbb\db\driver\driver_interface */ protected $db; - /** @var string */ - protected $email_notifications_table; + /** @var string Notification emails table */ + protected $notification_emails_table; /** * Notification Method email Constructor @@ -43,16 +43,16 @@ class email extends \phpbb\notification\method\messenger_base * @param \phpbb\db\driver\driver_interface $db * @param string $phpbb_root_path * @param string $php_ext - * @param string $email_notifications_table + * @param string $notification_emails_table */ - public function __construct(\phpbb\user_loader $user_loader, \phpbb\user $user, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, $phpbb_root_path, $php_ext, $email_notifications_table) + public function __construct(\phpbb\user_loader $user_loader, \phpbb\user $user, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, $phpbb_root_path, $php_ext, $notification_emails_table) { parent::__construct($user_loader, $phpbb_root_path, $php_ext); $this->user = $user; $this->config = $config; $this->db = $db; - $this->email_notifications_table = $email_notifications_table; + $this->notification_emails_table = $notification_emails_table; } /** @@ -86,7 +86,7 @@ class email extends \phpbb\notification\method\messenger_base $notified_users = array(); $sql = 'SELECT user_id - FROM ' . $this->email_notifications_table . ' + FROM ' . $this->notification_emails_table . ' WHERE notification_type_id = ' . (int) $notification_type_id . (isset($options['item_id']) ? ' AND item_id = ' . (int) $options['item_id'] : '') . (isset($options['item_parent_id']) ? ' AND item_parent_id = ' . (int) $options['item_parent_id'] : '') . @@ -106,7 +106,7 @@ class email extends \phpbb\notification\method\messenger_base */ public function notify() { - $insert_buffer = new \phpbb\db\sql_insert_buffer($this->db, $this->email_notifications_table); + $insert_buffer = new \phpbb\db\sql_insert_buffer($this->db, $this->notification_emails_table); /** @var \phpbb\notification\type\type_interface $notification */ foreach ($this->queue as $notification) @@ -125,7 +125,7 @@ class email extends \phpbb\notification\method\messenger_base */ public function mark_notifications($notification_type_id, $item_id, $user_id, $time = false, $mark_read = true) { - $sql = 'DELETE FROM ' . $this->email_notifications_table . ' + $sql = 'DELETE FROM ' . $this->notification_emails_table . ' WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '1=1') . (($user_id !== false) ? ' AND ' . (is_array($user_id) ? $this->db->sql_in_set('user_id', $user_id) : 'user_id = ' . (int) $user_id) : '') . (($item_id !== false) ? ' AND ' . (is_array($item_id) ? $this->db->sql_in_set('item_id', $item_id) : 'item_id = ' . (int) $item_id) : ''); @@ -137,7 +137,7 @@ class email extends \phpbb\notification\method\messenger_base */ public function mark_notifications_by_parent($notification_type_id, $item_parent_id, $user_id, $time = false, $mark_read = true) { - $sql = 'DELETE FROM ' . $this->email_notifications_table . ' + $sql = 'DELETE FROM ' . $this->notification_emails_table . ' WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '1=1') . (($user_id !== false) ? ' AND ' . (is_array($user_id) ? $this->db->sql_in_set('user_id', $user_id) : 'user_id = ' . (int) $user_id) : '') . (($item_parent_id !== false) ? ' AND ' . (is_array($item_parent_id) ? $this->db->sql_in_set('item_parent_id', $item_parent_id, false, true) : 'item_parent_id = ' . (int) $item_parent_id) : ''); diff --git a/tests/notification/base.php b/tests/notification/base.php index 5c923133f8..1cc5893ebd 100644 --- a/tests/notification/base.php +++ b/tests/notification/base.php @@ -105,7 +105,7 @@ abstract class phpbb_tests_notification_base extends phpbb_database_test_case $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); $phpbb_container->setParameter('tables.user_notifications', 'phpbb_user_notifications'); $phpbb_container->setParameter('tables.notification_types', 'phpbb_notification_types'); - $phpbb_container->setParameter('tables.email_notifications', 'phpbb_email_notifications'); + $phpbb_container->setParameter('tables.notification_emails', 'phpbb_notification_emails'); $this->notifications = new phpbb_notification_manager_helper( array(), diff --git a/tests/notification/notification_method_email_test.php b/tests/notification/notification_method_email_test.php index 20ee0ef181..c92a5a0f01 100644 --- a/tests/notification/notification_method_email_test.php +++ b/tests/notification/notification_method_email_test.php @@ -87,7 +87,7 @@ class notification_method_email_test extends phpbb_tests_notification_base $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); $phpbb_container->setParameter('tables.user_notifications', 'phpbb_user_notifications'); $phpbb_container->setParameter('tables.notification_types', 'phpbb_notification_types'); - $phpbb_container->setParameter('tables.email_notifications', 'phpbb_email_notifications'); + $phpbb_container->setParameter('tables.notification_emails', 'phpbb_notification_emails'); $this->notification_method_email = $this->getMockBuilder('\phpbb\notification\method\email') ->setConstructorArgs([ @@ -97,7 +97,7 @@ class notification_method_email_test extends phpbb_tests_notification_base $phpbb_container->get('dbal.conn'), $phpbb_root_path, $phpEx, - $phpbb_container->getParameter('tables.email_notifications') + $phpbb_container->getParameter('tables.notification_emails') ]) ->setMethods(['notify_using_messenger']) ->getMock(); diff --git a/tests/notification/submit_post_base.php b/tests/notification/submit_post_base.php index edcbef76e2..82db7753b5 100644 --- a/tests/notification/submit_post_base.php +++ b/tests/notification/submit_post_base.php @@ -130,7 +130,7 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); $phpbb_container->setParameter('tables.user_notifications', 'phpbb_user_notifications'); $phpbb_container->setParameter('tables.notification_types', 'phpbb_notification_types'); - $phpbb_container->setParameter('tables.email_notifications', 'phpbb_email_notifications'); + $phpbb_container->setParameter('tables.notification_emails', 'phpbb_notification_emails'); $phpbb_container->set('content.visibility', new \phpbb\content_visibility($auth, $config, $phpbb_dispatcher, $db, $user, $phpbb_root_path, $phpEx, FORUMS_TABLE, POSTS_TABLE, TOPICS_TABLE, USERS_TABLE)); $phpbb_container->compile(); From ec6335bdfdecf1570ebbfe173c10709f045f9020 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 16 Apr 2020 22:01:04 +0200 Subject: [PATCH 09/10] [ticket/14754] Clean up code as per review PHPBB3-14754 --- phpBB/phpbb/notification/method/email.php | 24 +++++++++---------- .../notification_method_email_test.php | 12 +++++----- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/phpBB/phpbb/notification/method/email.php b/phpBB/phpbb/notification/method/email.php index 61777e3c49..a9717e70df 100644 --- a/phpBB/phpbb/notification/method/email.php +++ b/phpBB/phpbb/notification/method/email.php @@ -83,7 +83,7 @@ class email extends \phpbb\notification\method\messenger_base */ public function get_notified_users($notification_type_id, array $options) { - $notified_users = array(); + $notified_users = []; $sql = 'SELECT user_id FROM ' . $this->notification_emails_table . ' @@ -111,7 +111,7 @@ class email extends \phpbb\notification\method\messenger_base /** @var \phpbb\notification\type\type_interface $notification */ foreach ($this->queue as $notification) { - $data = $this->clean_data($notification->get_insert_array()); + $data = self::clean_data($notification->get_insert_array()); $insert_buffer->insert($data); } @@ -126,9 +126,9 @@ class email extends \phpbb\notification\method\messenger_base public function mark_notifications($notification_type_id, $item_id, $user_id, $time = false, $mark_read = true) { $sql = 'DELETE FROM ' . $this->notification_emails_table . ' - WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '1=1') . - (($user_id !== false) ? ' AND ' . (is_array($user_id) ? $this->db->sql_in_set('user_id', $user_id) : 'user_id = ' . (int) $user_id) : '') . - (($item_id !== false) ? ' AND ' . (is_array($item_id) ? $this->db->sql_in_set('item_id', $item_id) : 'item_id = ' . (int) $item_id) : ''); + WHERE ' . ($notification_type_id !== false ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : '1=1') . + ($user_id !== false ? ' AND ' . $this->db->sql_in_set('user_id', $user_id) : '') . + ($item_id !== false ? ' AND ' . $this->db->sql_in_set('item_id', $item_id) : ''); $this->db->sql_query($sql); } @@ -138,9 +138,9 @@ class email extends \phpbb\notification\method\messenger_base public function mark_notifications_by_parent($notification_type_id, $item_parent_id, $user_id, $time = false, $mark_read = true) { $sql = 'DELETE FROM ' . $this->notification_emails_table . ' - WHERE ' . (($notification_type_id !== false) ? (is_array($notification_type_id) ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : 'notification_type_id = ' . $notification_type_id) : '1=1') . - (($user_id !== false) ? ' AND ' . (is_array($user_id) ? $this->db->sql_in_set('user_id', $user_id) : 'user_id = ' . (int) $user_id) : '') . - (($item_parent_id !== false) ? ' AND ' . (is_array($item_parent_id) ? $this->db->sql_in_set('item_parent_id', $item_parent_id, false, true) : 'item_parent_id = ' . (int) $item_parent_id) : ''); + WHERE ' . ($notification_type_id !== false ? $this->db->sql_in_set('notification_type_id', $notification_type_id) : '1=1') . + ($user_id !== false ? ' AND ' . $this->db->sql_in_set('user_id', $user_id) : '') . + ($item_parent_id !== false ? ' AND ' . $this->db->sql_in_set('item_parent_id', $item_parent_id, false, true) : ''); $this->db->sql_query($sql); } @@ -150,15 +150,15 @@ class email extends \phpbb\notification\method\messenger_base * @param array $data Notification data * @return array Cleaned notification data */ - protected function clean_data(array $data) + static public function clean_data(array $data) { - $model = array( + $row = [ 'notification_type_id' => null, 'item_id' => null, 'item_parent_id' => null, 'user_id' => null, - ); + ]; - return array_intersect_key($data, $model); + return array_intersect_key($data, $row); } } diff --git a/tests/notification/notification_method_email_test.php b/tests/notification/notification_method_email_test.php index c92a5a0f01..b4734c872d 100644 --- a/tests/notification/notification_method_email_test.php +++ b/tests/notification/notification_method_email_test.php @@ -29,9 +29,9 @@ class notification_method_email_test extends phpbb_tests_notification_base protected function get_notification_methods() { - return array( + return [ 'notification.method.email', - ); + ]; } protected function setUp() : void @@ -45,13 +45,13 @@ class notification_method_email_test extends phpbb_tests_notification_base global $db, $config, $user, $auth, $cache, $phpbb_container; $db = $this->db = $this->new_dbal(); - $config = $this->config = new \phpbb\config\config(array( + $config = $this->config = new \phpbb\config\config([ 'allow_privmsg' => true, 'allow_bookmarks' => true, 'allow_topic_notify' => true, 'allow_forum_notify' => true, 'allow_board_notifications' => true, - )); + ]); $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); $lang = new \phpbb\language\language($lang_loader); $user = new \phpbb\user($lang, '\phpbb\datetime'); @@ -184,7 +184,7 @@ class notification_method_email_test extends phpbb_tests_notification_base 6 => ['user_id' => '6'], 7 => ['user_id' => '7'], 8 => ['user_id' => '8'], - ] + ], ], [ [ @@ -193,7 +193,7 @@ class notification_method_email_test extends phpbb_tests_notification_base 'topic_id' => '3', ], [ - ] + ], ], ]; } From a0c8df66795a5deee133fd35e4d523e0e77b68bd Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 29 Apr 2020 22:14:04 +0200 Subject: [PATCH 10/10] [ticket/14754] Add effectively_installed() to migration PHPBB3-14754 --- .../db/migration/data/v33x/add_notification_emails_table.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/phpBB/phpbb/db/migration/data/v33x/add_notification_emails_table.php b/phpBB/phpbb/db/migration/data/v33x/add_notification_emails_table.php index 5a5f4dd354..3bbf7d8db3 100644 --- a/phpBB/phpbb/db/migration/data/v33x/add_notification_emails_table.php +++ b/phpBB/phpbb/db/migration/data/v33x/add_notification_emails_table.php @@ -22,6 +22,11 @@ class add_notification_emails_table extends \phpbb\db\migration\migration ]; } + public function effectively_installed() + { + return $this->db_tools->sql_table_exists($this->table_prefix . 'notification_emails'); + } + public function update_schema() { return [