From a4fe72bd533ef6ebe2040b0dbc8c26ebed1528e2 Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Thu, 2 May 2013 15:40:43 -0500 Subject: [PATCH 1/8] [ticket/11420] Fix notification options conversion PHPBB3-11420 --- .../db/migration/data/310/notifications.php | 64 ---------- .../db/migration/data/310/notifications3.php | 106 +++++++++++++++++ tests/notification/convert_test.php | 110 ++++++++++++++++++ tests/notification/fixtures/convert.xml | 52 +++++++++ 4 files changed, 268 insertions(+), 64 deletions(-) create mode 100644 phpBB/includes/db/migration/data/310/notifications3.php create mode 100644 tests/notification/convert_test.php create mode 100644 tests/notification/fixtures/convert.xml diff --git a/phpBB/includes/db/migration/data/310/notifications.php b/phpBB/includes/db/migration/data/310/notifications.php index 82bfd4cb2d..17c939d95a 100644 --- a/phpBB/includes/db/migration/data/310/notifications.php +++ b/phpBB/includes/db/migration/data/310/notifications.php @@ -91,70 +91,6 @@ class phpbb_db_migration_data_310_notifications extends phpbb_db_migration ), )), array('config.add', array('load_notifications', 1)), - array('custom', array(array($this, 'convert_notifications'))), ); } - - public function convert_notifications() - { - $convert_notifications = array( - array( - 'check' => ($this->config['allow_topic_notify']), - 'item_type' => 'post', - ), - array( - 'check' => ($this->config['allow_forum_notify']), - 'item_type' => 'topic', - ), - array( - 'check' => ($this->config['allow_bookmarks']), - 'item_type' => 'bookmark', - ), - array( - 'check' => ($this->config['allow_privmsg']), - 'item_type' => 'pm', - ), - ); - - foreach ($convert_notifications as $convert_data) - { - if ($convert_data['check']) - { - $sql = 'SELECT user_id, user_notify_type - FROM ' . USERS_TABLE . ' - WHERE user_notify = 1'; - $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $this->sql_query('INSERT INTO ' . $this->table_prefix . 'user_notifications ' . $this->db->sql_build_array('INSERT', array( - 'item_type' => $convert_data['item_type'], - 'item_id' => 0, - 'user_id' => $row['user_id'], - 'method' => '', - ))); - - if ($row['user_notify_type'] == NOTIFY_EMAIL || $row['user_notify_type'] == NOTIFY_BOTH) - { - $this->sql_query('INSERT INTO ' . $this->table_prefix . 'user_notifications ' . $this->db->sql_build_array('INSERT', array( - 'item_type' => $convert_data['item_type'], - 'item_id' => 0, - 'user_id' => $row['user_id'], - 'method' => 'email', - ))); - } - - if ($row['user_notify_type'] == NOTIFY_IM || $row['user_notify_type'] == NOTIFY_BOTH) - { - $this->sql_query('INSERT INTO ' . $this->table_prefix . 'user_notifications ' . $this->db->sql_build_array('INSERT', array( - 'item_type' => $convert_data['item_type'], - 'item_id' => 0, - 'user_id' => $row['user_id'], - 'method' => 'jabber', - ))); - } - } - $this->db->sql_freeresult($result); - } - } - } } diff --git a/phpBB/includes/db/migration/data/310/notifications3.php b/phpBB/includes/db/migration/data/310/notifications3.php new file mode 100644 index 0000000000..22839dbd7b --- /dev/null +++ b/phpBB/includes/db/migration/data/310/notifications3.php @@ -0,0 +1,106 @@ +table_prefix . 'user_notifications'; + $insert_buffer = new phpbb_db_sql_insert_buffer($this->db, $insert_table); + + $this->perform_conversion($insert_buffer, $insert_table); + } + + /** + * Perform the conversion (separate for testability) + */ + public function perform_conversion($insert_buffer, $insert_table) + { + $sql = 'DELETE FROM ' . $insert_table; + $this->db->sql_query($sql); + + $sql = 'SELECT user_id, user_notify_type, user_notify_pm + FROM ' . USERS_TABLE; + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $notification_methods = array(); + + // In-board notification + $notification_methods[] = ''; + + if ($row['user_notify_type'] == NOTIFY_EMAIL || $row['user_notify_type'] == NOTIFY_BOTH) + { + $notification_methods[] = 'email'; + } + + if ($row['user_notify_type'] == NOTIFY_IM || $row['user_notify_type'] == NOTIFY_BOTH) + { + $notification_methods[] = 'jabber'; + } + + // Notifications for posts + foreach (array('post', 'topic') as $item_type) + { + $this->add_method_rows( + $insert_buffer, + $item_type, + 0, + $row['user_id'], + $notification_methods + ); + } + + if ($row['user_notify_pm']) + { + // Notifications for private messages + // User either gets all methods or no method + $this->add_method_rows( + $insert_buffer, + 'pm', + 0, + $row['user_id'], + $notification_methods + ); + } + } + $this->db->sql_freeresult($result); + + $insert_buffer->flush(); + } + + protected function add_method_rows(phpbb_db_sql_insert_buffer $insert_buffer, $item_type, $item_id, $user_id, array $methods) + { + $row_base = array( + 'item_type' => $item_type, + 'item_id' => (int) $item_id, + 'user_id' => (int) $user_id, + 'notify' => 1 + ); + + foreach ($methods as $method) + { + $row_base['method'] = $method; + $insert_buffer->insert($row_base); + } + } +} diff --git a/tests/notification/convert_test.php b/tests/notification/convert_test.php new file mode 100644 index 0000000000..fdd0d19e72 --- /dev/null +++ b/tests/notification/convert_test.php @@ -0,0 +1,110 @@ +createXMLDataSet(dirname(__FILE__) . '/fixtures/convert.xml'); + } + + protected function setUp() + { + parent::setUp(); + + global $phpbb_root_path, $phpEx; + + $this->db = $this->new_dbal(); + + $this->migration = new phpbb_db_migration_data_310_notifications3( + new phpbb_config(array()), + $this->db, + new phpbb_db_tools($this->db), + $phpbb_root_path, + $phpEx, + 'phpbb_' + ); + } + + public function test_convert() + { + $buffer = new phpbb_mock_sql_insert_buffer($this->db, 'phpbb_user_notifications'); + $this->migration->perform_conversion($buffer, 'phpbb_user_notifications'); + + $expected = array_merge( + $this->create_expected('post', 1, 'email'), + $this->create_expected('topic', 1, 'email'), + + $this->create_expected('post', 2, 'email'), + $this->create_expected('topic', 2, 'email'), + $this->create_expected('pm', 2, 'email'), + + $this->create_expected('post', 3, 'jabber'), + $this->create_expected('topic', 3, 'jabber'), + + $this->create_expected('post', 4, 'jabber'), + $this->create_expected('topic', 4, 'jabber'), + $this->create_expected('pm', 4, 'jabber'), + + $this->create_expected('post', 5, 'both'), + $this->create_expected('topic', 5, 'both'), + + $this->create_expected('post', 6, 'both'), + $this->create_expected('topic', 6, 'both'), + $this->create_expected('pm', 6, 'both') + ); + + $this->assertEquals($expected, $buffer->get_buffer()); + } + + protected function create_expected($type, $user_id, $method = '') + { + $return = array(); + + if ($method != '') + { + $return[] = array( + 'item_type' => $type, + 'item_id' => 0, + 'user_id' => $user_id, + 'method' => '', + 'notify' => 1, + ); + } + + if ($method == 'email' || $method == 'both') + { + $return[] = array( + 'item_type' => $type, + 'item_id' => 0, + 'user_id' => $user_id, + 'method' => 'email', + 'notify' => 1, + ); + } + + if ($method == 'jabber' || $method == 'both') + { + $return[] = array( + 'item_type' => $type, + 'item_id' => 0, + 'user_id' => $user_id, + 'method' => 'jabber', + 'notify' => 1, + ); + } + + return $return; + } +} diff --git a/tests/notification/fixtures/convert.xml b/tests/notification/fixtures/convert.xml new file mode 100644 index 0000000000..a244070a95 --- /dev/null +++ b/tests/notification/fixtures/convert.xml @@ -0,0 +1,52 @@ + + + + user_id + username + username_clean + user_notify_type + user_notify_pm + + 1 + 1 + 1 + 0 + 0 + + + 2 + 2 + 2 + 0 + 1 + + + 3 + 3 + 3 + 1 + 0 + + + 4 + 4 + 4 + 1 + 1 + + + 5 + 5 + 5 + 2 + 0 + + + 6 + 6 + 6 + 2 + 1 + +
+ From 77436595630d3aa267ebb9b00d12e899818a34b7 Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Fri, 3 May 2013 08:37:09 -0500 Subject: [PATCH 2/8] [ticket/11420] Forgot to include mock sql_insert_buffer PHPBB3-11420 --- tests/mock/sql_insert_buffer.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/mock/sql_insert_buffer.php diff --git a/tests/mock/sql_insert_buffer.php b/tests/mock/sql_insert_buffer.php new file mode 100644 index 0000000000..ba09aa8d7f --- /dev/null +++ b/tests/mock/sql_insert_buffer.php @@ -0,0 +1,21 @@ +buffer)) ? true : false; + } + + public function get_buffer() + { + return $this->buffer; + } +} From cb13747c4bdff5fb8ab034909474f276eff212ee Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Fri, 10 May 2013 13:44:38 -0500 Subject: [PATCH 3/8] [ticket/11420] Rename migrations file to something more helpful PHPBB3-11420 --- ...{notifications3.php => notification_options_reconvert.php} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename phpBB/includes/db/migration/data/310/{notifications3.php => notification_options_reconvert.php} (93%) diff --git a/phpBB/includes/db/migration/data/310/notifications3.php b/phpBB/includes/db/migration/data/310/notification_options_reconvert.php similarity index 93% rename from phpBB/includes/db/migration/data/310/notifications3.php rename to phpBB/includes/db/migration/data/310/notification_options_reconvert.php index 22839dbd7b..9f2d4b2d75 100644 --- a/phpBB/includes/db/migration/data/310/notifications3.php +++ b/phpBB/includes/db/migration/data/310/notification_options_reconvert.php @@ -7,11 +7,11 @@ * */ -class phpbb_db_migration_data_310_notifications3 extends phpbb_db_migration +class phpbb_db_migration_data_310_notification_options_reconvert extends phpbb_db_migration { static public function depends_on() { - return array('phpbb_db_migration_data_310_notifications2'); + return array('phpbb_db_migration_data_310_notifications_schema_fix'); } public function update_data() From bdaa40bb550c18f9c7feb4e2448a31a53e2a5bd2 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Sat, 6 Jul 2013 12:58:52 -0500 Subject: [PATCH 4/8] [ticket/11420] Fix comments, license link PHPBB3-11420 --- .../310/notification_options_reconvert.php | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/db/migration/data/310/notification_options_reconvert.php b/phpBB/includes/db/migration/data/310/notification_options_reconvert.php index 9f2d4b2d75..d994d7ec5f 100644 --- a/phpBB/includes/db/migration/data/310/notification_options_reconvert.php +++ b/phpBB/includes/db/migration/data/310/notification_options_reconvert.php @@ -3,7 +3,7 @@ * * @package migration * @copyright (c) 2013 phpBB Group -* @license http://opensource.org/licenses/gpl-license.php GNU Public License v2 +* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 * */ @@ -31,8 +31,11 @@ class phpbb_db_migration_data_310_notification_options_reconvert extends phpbb_d /** * Perform the conversion (separate for testability) + * + * @param phpbb_db_sql_insert_buffer $insert_buffer + * @param string $insert_table */ - public function perform_conversion($insert_buffer, $insert_table) + public function perform_conversion(phpbb_db_sql_insert_buffer $insert_buffer, $insert_table) { $sql = 'DELETE FROM ' . $insert_table; $this->db->sql_query($sql); @@ -58,7 +61,7 @@ class phpbb_db_migration_data_310_notification_options_reconvert extends phpbb_d $notification_methods[] = 'jabber'; } - // Notifications for posts + // Notifications for posts foreach (array('post', 'topic') as $item_type) { $this->add_method_rows( @@ -88,6 +91,15 @@ class phpbb_db_migration_data_310_notification_options_reconvert extends phpbb_d $insert_buffer->flush(); } + /** + * Insert method rows to DB + * + * @param phpbb_db_sql_insert_buffer $insert_buffer + * @param string $item_type + * @param int $item_id + * @param int $user_id + * @param string $methods + */ protected function add_method_rows(phpbb_db_sql_insert_buffer $insert_buffer, $item_type, $item_id, $user_id, array $methods) { $row_base = array( From 9f85a4d118544e6097005ea3ef37e1e627838278 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Sat, 6 Jul 2013 12:59:26 -0500 Subject: [PATCH 5/8] [ticket/11420] Use !==, === when comparing strings PHPBB3-11420 --- tests/notification/convert_test.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/notification/convert_test.php b/tests/notification/convert_test.php index fdd0d19e72..e07c144e16 100644 --- a/tests/notification/convert_test.php +++ b/tests/notification/convert_test.php @@ -72,7 +72,7 @@ class phpbb_notification_convert_test extends phpbb_database_test_case { $return = array(); - if ($method != '') + if ($method !== '') { $return[] = array( 'item_type' => $type, @@ -83,7 +83,7 @@ class phpbb_notification_convert_test extends phpbb_database_test_case ); } - if ($method == 'email' || $method == 'both') + if ($method === 'email' || $method === 'both') { $return[] = array( 'item_type' => $type, @@ -94,7 +94,7 @@ class phpbb_notification_convert_test extends phpbb_database_test_case ); } - if ($method == 'jabber' || $method == 'both') + if ($method === 'jabber' || $method === 'both') { $return[] = array( 'item_type' => $type, From 91165e0758ccb47878fe33708ffeee5021614083 Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Thu, 11 Jul 2013 15:49:32 -0500 Subject: [PATCH 6/8] [ticket/11420] Fix tests PHPBB3-11420 --- tests/notification/fixtures/convert.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/notification/fixtures/convert.xml b/tests/notification/fixtures/convert.xml index a244070a95..98dd5fe0d5 100644 --- a/tests/notification/fixtures/convert.xml +++ b/tests/notification/fixtures/convert.xml @@ -6,12 +6,14 @@ username_clean user_notify_type user_notify_pm + user_permissions 1 1 1 0 0 + 2 @@ -19,6 +21,7 @@ 2 0 1 + 3 @@ -26,6 +29,7 @@ 3 1 0 + 4 @@ -33,6 +37,7 @@ 4 1 1 + 5 @@ -40,6 +45,7 @@ 5 2 0 + 6 @@ -47,6 +53,7 @@ 6 2 1 + From 9bfb567854b6c7487fcdc8dd5cd58748b0016911 Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Thu, 11 Jul 2013 15:58:02 -0500 Subject: [PATCH 7/8] [ticket/11420] Fix tests PHPBB3-11420 --- tests/notification/convert_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/notification/convert_test.php b/tests/notification/convert_test.php index e07c144e16..4d00fa0a1e 100644 --- a/tests/notification/convert_test.php +++ b/tests/notification/convert_test.php @@ -27,7 +27,7 @@ class phpbb_notification_convert_test extends phpbb_database_test_case $this->db = $this->new_dbal(); - $this->migration = new phpbb_db_migration_data_310_notifications3( + $this->migration = new phpbb_db_migration_data_310_notification_options_reconvert( new phpbb_config(array()), $this->db, new phpbb_db_tools($this->db), From fa8f62a604d5d885f6126619ca1dc3db7b0731a7 Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Fri, 12 Jul 2013 09:14:36 -0500 Subject: [PATCH 8/8] [ticket/11420] Fix tests PHPBB3-11420 --- tests/notification/fixtures/convert.xml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/notification/fixtures/convert.xml b/tests/notification/fixtures/convert.xml index 98dd5fe0d5..3f0a065cc4 100644 --- a/tests/notification/fixtures/convert.xml +++ b/tests/notification/fixtures/convert.xml @@ -7,6 +7,9 @@ user_notify_type user_notify_pm user_permissions + user_sig + user_occ + user_interests 1 1 @@ -14,6 +17,9 @@ 0 0 + + + 2 @@ -22,6 +28,9 @@ 0 1 + + + 3 @@ -30,6 +39,9 @@ 1 0 + + + 4 @@ -38,6 +50,9 @@ 1 1 + + + 5 @@ -46,6 +61,9 @@ 2 0 + + + 6 @@ -54,6 +72,9 @@ 2 1 + + +