From ac8ebfabc7a4fae591ac780f621e4e21d09d070c Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Jun 2012 10:26:48 +0200 Subject: [ticket/10950] Use proper ' in order to fix comment. PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 261ed45727..e6e8e0ab6c 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1173,7 +1173,7 @@ function phpbb_delete_user_pms($user_id) $db->sql_query($sql); } - // Reset the userīs pm count to 0 + // Reset the user's pm count to 0 if (isset($undelivered_user[$user_id])) { $sql = 'UPDATE ' . USERS_TABLE . ' -- cgit v1.2.1 From 35b18676cda35b81277a9532ac9a579abbfda09d Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Jun 2012 10:28:13 +0200 Subject: [ticket/10950] Fix SQL coding style (indentation) in second SQL query. PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index e6e8e0ab6c..d76efd2ff6 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1135,7 +1135,7 @@ function phpbb_delete_user_pms($user_id) $sql = 'SELECT msg_id, author_id, folder_id, pm_unread, pm_new FROM ' . PRIVMSGS_TO_TABLE . ' WHERE author_id = ' . $user_id . ' - AND folder_id = ' . PRIVMSGS_NO_BOX; + AND folder_id = ' . PRIVMSGS_NO_BOX; $result = $db->sql_query($sql); while ($row = $db->sql_fetchrow($result)) -- cgit v1.2.1 From 2a76b7e8690eaf19c25ed4c13540c1e1aab31cdd Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Jun 2012 10:30:09 +0200 Subject: [ticket/10950] Remove redundant if statement. We already know author_id and folder_id. PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index d76efd2ff6..d75b4d92a8 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1140,19 +1140,16 @@ function phpbb_delete_user_pms($user_id) while ($row = $db->sql_fetchrow($result)) { - if ($row['author_id'] == $user_id && $row['folder_id'] == PRIVMSGS_NO_BOX) - { - // Undelivered messages - $undelivered_msg[] = $row['msg_id']; + // Undelivered messages + $undelivered_msg[] = $row['msg_id']; - if (isset($undelivered_user[$row['user_id']])) - { - ++$undelivered_user[$row['user_id']]; - } - else - { - $undelivered_user[$row['user_id']] = 1; - } + if (isset($undelivered_user[$row['user_id']])) + { + ++$undelivered_user[$row['user_id']]; + } + else + { + $undelivered_user[$row['user_id']] = 1; } $delete_ids[(int) $row['msg_id']] = (int) $row['msg_id']; -- cgit v1.2.1 From a50d1a3576b9e92dfbcfc5bb951e2985ae1872a1 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Jun 2012 10:32:16 +0200 Subject: [ticket/10950] Move array initialisation to the front. PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index d75b4d92a8..cdfde4c7ea 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1103,13 +1103,14 @@ function phpbb_delete_user_pms($user_id) // Get PM Information for later deleting // The two queries where split, so we can use our indexes + $undelivered_msg = $undelivered_user = $delete_ids = array(); + // Part 1: get PMs the user received $sql = 'SELECT msg_id, author_id, folder_id, pm_unread, pm_new FROM ' . PRIVMSGS_TO_TABLE . ' WHERE user_id = ' . $user_id; $result = $db->sql_query($sql); - $undelivered_msg = $undelivered_user = $delete_ids = array(); while ($row = $db->sql_fetchrow($result)) { if ($row['author_id'] == $user_id && $row['folder_id'] == PRIVMSGS_NO_BOX) -- cgit v1.2.1 From 9c2930178f37ed881055da3e65286704a3a8220e Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Jun 2012 10:43:43 +0200 Subject: [ticket/10950] Use a variable for the private message id. PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index cdfde4c7ea..d77d46d46d 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1113,10 +1113,13 @@ function phpbb_delete_user_pms($user_id) while ($row = $db->sql_fetchrow($result)) { + $msg_id = (int) $row['msg_id']; + $delete_ids[$msg_id] = $msg_id; + if ($row['author_id'] == $user_id && $row['folder_id'] == PRIVMSGS_NO_BOX) { // Undelivered messages - $undelivered_msg[] = $row['msg_id']; + $undelivered_msg[] = $msg_id; if (isset($undelivered_user[$row['user_id']])) { @@ -1127,8 +1130,6 @@ function phpbb_delete_user_pms($user_id) $undelivered_user[$row['user_id']] = 1; } } - - $delete_ids[(int) $row['msg_id']] = (int) $row['msg_id']; } $db->sql_freeresult($result); @@ -1141,8 +1142,11 @@ function phpbb_delete_user_pms($user_id) while ($row = $db->sql_fetchrow($result)) { + $msg_id = (int) $row['msg_id']; + $delete_ids[$msg_id] = $msg_id; + // Undelivered messages - $undelivered_msg[] = $row['msg_id']; + $undelivered_msg[] = $msg_id; if (isset($undelivered_user[$row['user_id']])) { @@ -1152,8 +1156,6 @@ function phpbb_delete_user_pms($user_id) { $undelivered_user[$row['user_id']] = 1; } - - $delete_ids[(int) $row['msg_id']] = (int) $row['msg_id']; } $db->sql_freeresult($result); -- cgit v1.2.1 From fce385e5bc1212d9c3233b7a1ae2212eb1626e9b Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Jun 2012 10:45:36 +0200 Subject: [ticket/10950] Select the correct columns in SQL queries. PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index d77d46d46d..1800242b71 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1106,7 +1106,7 @@ function phpbb_delete_user_pms($user_id) $undelivered_msg = $undelivered_user = $delete_ids = array(); // Part 1: get PMs the user received - $sql = 'SELECT msg_id, author_id, folder_id, pm_unread, pm_new + $sql = 'SELECT msg_id, user_id, author_id, folder_id FROM ' . PRIVMSGS_TO_TABLE . ' WHERE user_id = ' . $user_id; $result = $db->sql_query($sql); @@ -1134,7 +1134,7 @@ function phpbb_delete_user_pms($user_id) $db->sql_freeresult($result); // Part 2: get PMs the user sent - $sql = 'SELECT msg_id, author_id, folder_id, pm_unread, pm_new + $sql = 'SELECT msg_id, user_id FROM ' . PRIVMSGS_TO_TABLE . ' WHERE author_id = ' . $user_id . ' AND folder_id = ' . PRIVMSGS_NO_BOX; -- cgit v1.2.1 From 30475856c46b3604693b6d5df22be3360ae16822 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Jun 2012 10:47:26 +0200 Subject: [ticket/10950] Add empty line to make unset() call more visible. PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 1800242b71..72dd3c7d20 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1181,6 +1181,7 @@ function phpbb_delete_user_pms($user_id) user_unread_privmsg = 0 WHERE user_id = ' . $user_id; $db->sql_query($sql); + unset($undelivered_user[$user_id]); } -- cgit v1.2.1 From 49afc1f2dc3a3ee17c6abff1e94c25a8ba8b3604 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Jun 2012 11:16:38 +0200 Subject: [ticket/10950] Correct comment for the second query. Only undelivered messages are handled. PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 72dd3c7d20..afd254d6ea 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1133,7 +1133,7 @@ function phpbb_delete_user_pms($user_id) } $db->sql_freeresult($result); - // Part 2: get PMs the user sent + // Part 2: get PMs the user sent, but has yet to be received $sql = 'SELECT msg_id, user_id FROM ' . PRIVMSGS_TO_TABLE . ' WHERE author_id = ' . $user_id . ' -- cgit v1.2.1 From d30dc11f3e1ade19fd8643bdded6f11625da1bb3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 23 Jun 2012 16:02:16 +0200 Subject: [ticket/10950] Add some first and simple unit tests for phpbb_delete_user_pms() Todo: Add cases to in which the msg is also deleted. PHPBB3-10950 --- tests/privmsgs/delete_user_pms_test.php | 95 +++++++++++++++++++++ tests/privmsgs/fixtures/delete_user_pms.xml | 127 ++++++++++++++++++++++++++++ 2 files changed, 222 insertions(+) create mode 100644 tests/privmsgs/delete_user_pms_test.php create mode 100644 tests/privmsgs/fixtures/delete_user_pms.xml diff --git a/tests/privmsgs/delete_user_pms_test.php b/tests/privmsgs/delete_user_pms_test.php new file mode 100644 index 0000000000..b399d94c6d --- /dev/null +++ b/tests/privmsgs/delete_user_pms_test.php @@ -0,0 +1,95 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/delete_user_pms.xml'); + } + + public static function delete_user_pms_data() + { + return array( + // array( + // (user we delete), + // array(remaining privmsgs ids), + // array(remaining privmsgs_to), + // ), + array( + 2, + array( + array('msg_id' => 1), + array('msg_id' => 2), + ), + array( + //array('msg_id' => 1, 'user_id' => 2), + array('msg_id' => 1, 'user_id' => 3), + array('msg_id' => 1, 'user_id' => 4), + //array('msg_id' => 2, 'user_id' => 2), + array('msg_id' => 2, 'user_id' => 4), + ), + ), + array( + 3, + array( + array('msg_id' => 1), + array('msg_id' => 2), + ), + array( + array('msg_id' => 1, 'user_id' => 2), + //array('msg_id' => 1, 'user_id' => 3), + array('msg_id' => 1, 'user_id' => 4), + array('msg_id' => 2, 'user_id' => 2), + array('msg_id' => 2, 'user_id' => 4), + ), + ), + array( + 5, + array( + array('msg_id' => 1), + array('msg_id' => 2), + ), + array( + array('msg_id' => 1, 'user_id' => 2), + array('msg_id' => 1, 'user_id' => 3), + array('msg_id' => 1, 'user_id' => 4), + array('msg_id' => 2, 'user_id' => 2), + array('msg_id' => 2, 'user_id' => 4), + ), + ), + ); + } + + /** + * @dataProvider delete_user_pms_data + */ + public function test_delete_user_pms($delete_user, $remaining_privmsgs, $remaining_privmsgs_to) + { + global $db; + + $db = $this->new_dbal(); + + phpbb_delete_user_pms($delete_user); + + $sql = 'SELECT msg_id + FROM ' . PRIVMSGS_TABLE; + $result = $db->sql_query($sql); + + $this->assertEquals($remaining_privmsgs, $db->sql_fetchrowset($result)); + + $sql = 'SELECT msg_id, user_id + FROM ' . PRIVMSGS_TO_TABLE; + $result = $db->sql_query($sql); + + $this->assertEquals($remaining_privmsgs_to, $db->sql_fetchrowset($result)); + } +} diff --git a/tests/privmsgs/fixtures/delete_user_pms.xml b/tests/privmsgs/fixtures/delete_user_pms.xml new file mode 100644 index 0000000000..848164080c --- /dev/null +++ b/tests/privmsgs/fixtures/delete_user_pms.xml @@ -0,0 +1,127 @@ + + + + user_id + username + username_clean + user_new_privmsg + user_unread_privmsg + user_permissions + user_sig + user_occ + user_interests + + 2 + sender + sender + 0 + 0 + + + + + + + 3 + pm in inbox + pm in inbox + 0 + 0 + + + + + + + 4 + pm in no box + pm in no box + 2 + 2 + + + + + + + 5 + no pms + no pms + 0 + 0 + + + + + +
+ + msg_id + root_level + author_id + message_subject + message_text + + 1 + 0 + 2 + #1 + #1 + + + 2 + 0 + 2 + #2 + #2 + +
+ + msg_id + user_id + author_id + pm_new + pm_unread + folder_id + + 1 + 2 + 2 + 0 + 0 + -2 + + + 1 + 3 + 2 + 0 + 0 + 0 + + + 1 + 4 + 2 + 0 + 0 + -3 + + + 2 + 2 + 2 + 0 + 0 + -2 + + + 2 + 4 + 2 + 0 + 0 + -3 + +
+
-- cgit v1.2.1 From 7988045bda2b9fbf0dc9482ed985b5b680ce4e95 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 4 Jul 2012 13:00:04 +0200 Subject: [ticket/10950] Fix unit tests to reflect desired behaviour See http://wiki.phpbb.com/Deleting_Private_Messages for further explanation. PHPBB3-10950 --- tests/privmsgs/delete_user_pms_test.php | 29 ++++++++++- tests/privmsgs/fixtures/delete_user_pms.xml | 80 ++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/tests/privmsgs/delete_user_pms_test.php b/tests/privmsgs/delete_user_pms_test.php index b399d94c6d..e5c0e82712 100644 --- a/tests/privmsgs/delete_user_pms_test.php +++ b/tests/privmsgs/delete_user_pms_test.php @@ -28,14 +28,23 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case 2, array( array('msg_id' => 1), - array('msg_id' => 2), + //array('msg_id' => 2), + //array('msg_id' => 3), + //array('msg_id' => 4), + //array('msg_id' => 5), ), array( //array('msg_id' => 1, 'user_id' => 2), array('msg_id' => 1, 'user_id' => 3), array('msg_id' => 1, 'user_id' => 4), //array('msg_id' => 2, 'user_id' => 2), - array('msg_id' => 2, 'user_id' => 4), + //array('msg_id' => 2, 'user_id' => 4), + //array('msg_id' => 4, 'user_id' => 3), + //array('msg_id' => 3, 'user_id' => 2), + //array('msg_id' => 4, 'user_id' => 3), + //array('msg_id' => 5, 'user_id' => 2), + //array('msg_id' => 5, 'user_id' => 3), + //array('msg_id' => 5, 'user_id' => 4), ), ), array( @@ -43,6 +52,9 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case array( array('msg_id' => 1), array('msg_id' => 2), + array('msg_id' => 3), + //array('msg_id' => 4), + array('msg_id' => 5), ), array( array('msg_id' => 1, 'user_id' => 2), @@ -50,6 +62,11 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case array('msg_id' => 1, 'user_id' => 4), array('msg_id' => 2, 'user_id' => 2), array('msg_id' => 2, 'user_id' => 4), + array('msg_id' => 3, 'user_id' => 2), + //array('msg_id' => 4, 'user_id' => 3), + array('msg_id' => 5, 'user_id' => 2), + //array('msg_id' => 5, 'user_id' => 3), + array('msg_id' => 5, 'user_id' => 4), ), ), array( @@ -57,6 +74,9 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case array( array('msg_id' => 1), array('msg_id' => 2), + array('msg_id' => 3), + array('msg_id' => 4), + array('msg_id' => 5), ), array( array('msg_id' => 1, 'user_id' => 2), @@ -64,6 +84,11 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case array('msg_id' => 1, 'user_id' => 4), array('msg_id' => 2, 'user_id' => 2), array('msg_id' => 2, 'user_id' => 4), + array('msg_id' => 3, 'user_id' => 2), + array('msg_id' => 4, 'user_id' => 3), + array('msg_id' => 5, 'user_id' => 2), + array('msg_id' => 5, 'user_id' => 3), + array('msg_id' => 5, 'user_id' => 4), ), ), ); diff --git a/tests/privmsgs/fixtures/delete_user_pms.xml b/tests/privmsgs/fixtures/delete_user_pms.xml index 848164080c..56970689a3 100644 --- a/tests/privmsgs/fixtures/delete_user_pms.xml +++ b/tests/privmsgs/fixtures/delete_user_pms.xml @@ -66,14 +66,50 @@ 0 2 #1 - #1 + + 2 - outbox + 3 - inbox + 4 - nobox + 2 0 2 #2 - #2 + + 2 - outbox + 4 - nobox + + + + 3 + 0 + 2 + #3 + + 2 - outbox + + + + 4 + 0 + 2 + #4 + + 3 - nobox + + + + 5 + 0 + 2 + #5 + + 2 - outbox + 3 - nobox + 4 - nobox + @@ -123,5 +159,45 @@ 0-3 + + 3 + 2 + 2 + 0 + 0 + -2 + + + 4 + 3 + 2 + 0 + 0 + -3 + + + 5 + 2 + 2 + 0 + 0 + -2 + + + 5 + 3 + 2 + 0 + 0 + -3 + + + 5 + 4 + 2 + 0 + 0 + -3 +
-- cgit v1.2.1 From 5c8c7b1352454fc8d1bb6e689d52d31b346d5fdb Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 4 Jul 2012 13:10:15 +0200 Subject: [ticket/10950] Recreated the behaviour of phpbb_delete_user_pms() - Get delete_ids, pms of the user as receipt - Get undelivered_msg, pms of the user as sender - Delete undelivered_msg, if there are only NO_BOX, OUTBOX and SENTBOX links - Correct the _new and _unread user values for the receipts - Delete delete_ids, if there are no links to them anymore - Reset _new and _unread values for the user we delete PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 132 +++++++++++++++++++--------------- 1 file changed, 74 insertions(+), 58 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index afd254d6ea..88388841ed 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1103,10 +1103,10 @@ function phpbb_delete_user_pms($user_id) // Get PM Information for later deleting // The two queries where split, so we can use our indexes - $undelivered_msg = $undelivered_user = $delete_ids = array(); + $undelivered_msg = $delete_ids = array(); // Part 1: get PMs the user received - $sql = 'SELECT msg_id, user_id, author_id, folder_id + $sql = 'SELECT msg_id FROM ' . PRIVMSGS_TO_TABLE . ' WHERE user_id = ' . $user_id; $result = $db->sql_query($sql); @@ -1115,26 +1115,13 @@ function phpbb_delete_user_pms($user_id) { $msg_id = (int) $row['msg_id']; $delete_ids[$msg_id] = $msg_id; - - if ($row['author_id'] == $user_id && $row['folder_id'] == PRIVMSGS_NO_BOX) - { - // Undelivered messages - $undelivered_msg[] = $msg_id; - - if (isset($undelivered_user[$row['user_id']])) - { - ++$undelivered_user[$row['user_id']]; - } - else - { - $undelivered_user[$row['user_id']] = 1; - } - } } $db->sql_freeresult($result); // Part 2: get PMs the user sent, but has yet to be received - $sql = 'SELECT msg_id, user_id + // We can not simply delete them. First we have to check, + // whether another user already received and read the message. + $sql = 'SELECT msg_id FROM ' . PRIVMSGS_TO_TABLE . ' WHERE author_id = ' . $user_id . ' AND folder_id = ' . PRIVMSGS_NO_BOX; @@ -1143,61 +1130,90 @@ function phpbb_delete_user_pms($user_id) while ($row = $db->sql_fetchrow($result)) { $msg_id = (int) $row['msg_id']; - $delete_ids[$msg_id] = $msg_id; - - // Undelivered messages - $undelivered_msg[] = $msg_id; - - if (isset($undelivered_user[$row['user_id']])) - { - ++$undelivered_user[$row['user_id']]; - } - else - { - $undelivered_user[$row['user_id']] = 1; - } + $undelivered_msg[$msg_id] = $msg_id; } $db->sql_freeresult($result); - if (empty($delete_ids)) + if (empty($delete_ids) && empty($undelivered_msg)) { return false; } $db->sql_transaction('begin'); - if (sizeof($undelivered_msg)) + if (!empty($undelivered_msg)) { - $sql = 'DELETE FROM ' . PRIVMSGS_TABLE . ' - WHERE ' . $db->sql_in_set('msg_id', $undelivered_msg); - $db->sql_query($sql); - } + // A pm is not undelivered, if for any receipt the message was moved + // from their NO_BOX to another folder. + $sql = 'SELECT msg_id + FROM ' . PRIVMSGS_TO_TABLE . ' + WHERE author_id = ' . $user_id . ' + AND folder_id <> ' . PRIVMSGS_NO_BOX . ' + AND folder_id <> ' . PRIVMSGS_OUTBOX . ' + AND folder_id <> ' . PRIVMSGS_SENTBOX; + $result = $db->sql_query($sql); - // Reset the user's pm count to 0 - if (isset($undelivered_user[$user_id])) - { - $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_new_privmsg = 0, - user_unread_privmsg = 0 - WHERE user_id = ' . $user_id; - $db->sql_query($sql); + while ($row = $db->sql_fetchrow($result)) + { + $msg_id = (int) $row['msg_id']; + unset($undelivered_msg[$msg_id]); + } + $db->sql_freeresult($result); - unset($undelivered_user[$user_id]); - } + if (!empty($undelivered_msg)) + { + $undelivered_user = array(); - foreach ($undelivered_user as $_user_id => $count) - { - $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_new_privmsg = user_new_privmsg - ' . $count . ', - user_unread_privmsg = user_unread_privmsg - ' . $count . ' - WHERE user_id = ' . $_user_id; - $db->sql_query($sql); + // Count the messages we delete, so we can correct the user pm data + $sql = 'SELECT user_id + FROM ' . PRIVMSGS_TO_TABLE . ' + WHERE author_id = ' . $user_id . ' + AND folder_id = ' . PRIVMSGS_NO_BOX . ' + AND ' . $db->sql_in_set('msg_id', $undelivered_msg); + $result = $db->sql_query($sql); + + while ($row = $db->sql_fetchrow($result)) + { + if (isset($undelivered_user[$row['user_id']])) + { + ++$undelivered_user[$row['user_id']]; + } + else + { + $undelivered_user[$row['user_id']] = 1; + } + } + $db->sql_freeresult($result); + + foreach ($undelivered_user as $undelivered_user_id => $count) + { + $sql = 'UPDATE ' . USERS_TABLE . ' + SET user_new_privmsg = user_new_privmsg - ' . $count . ', + user_unread_privmsg = user_unread_privmsg - ' . $count . ' + WHERE user_id = ' . $undelivered_user_id; + $db->sql_query($sql); + } + + $sql = 'DELETE FROM ' . PRIVMSGS_TO_TABLE . ' + WHERE ' . $db->sql_in_set('msg_id', $undelivered_msg); + $db->sql_query($sql); + + $sql = 'DELETE FROM ' . PRIVMSGS_TABLE . ' + WHERE ' . $db->sql_in_set('msg_id', $undelivered_msg); + $db->sql_query($sql); + } } - // Delete private message data - $sql = 'DELETE FROM ' . PRIVMSGS_TO_TABLE . " - WHERE user_id = $user_id - AND " . $db->sql_in_set('msg_id', $delete_ids); + // Reset the user's pm count to 0 + $sql = 'UPDATE ' . USERS_TABLE . ' + SET user_new_privmsg = 0, + user_unread_privmsg = 0 + WHERE user_id = ' . $user_id; + $db->sql_query($sql); + + // Delete private message data of the user + $sql = 'DELETE FROM ' . PRIVMSGS_TO_TABLE . ' + WHERE user_id = ' . (int) $user_id; $db->sql_query($sql); // Now we have to check which messages we can delete completely -- cgit v1.2.1 From 338d29072fd5581968f404576c6faa2dffc4f883 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 4 Jul 2012 13:14:19 +0200 Subject: [ticket/10950] Check $delete_ids to be not empty PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 41 +++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 88388841ed..6c0adccbed 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1216,31 +1216,34 @@ function phpbb_delete_user_pms($user_id) WHERE user_id = ' . (int) $user_id; $db->sql_query($sql); - // Now we have to check which messages we can delete completely - $sql = 'SELECT msg_id - FROM ' . PRIVMSGS_TO_TABLE . ' - WHERE ' . $db->sql_in_set('msg_id', $delete_ids); - $result = $db->sql_query($sql); - - while ($row = $db->sql_fetchrow($result)) - { - unset($delete_ids[$row['msg_id']]); - } - $db->sql_freeresult($result); - if (!empty($delete_ids)) { - // Check if there are any attachments we need to remove - if (!function_exists('delete_attachments')) + // Now we have to check which messages we can delete completely + $sql = 'SELECT msg_id + FROM ' . PRIVMSGS_TO_TABLE . ' + WHERE ' . $db->sql_in_set('msg_id', $delete_ids); + $result = $db->sql_query($sql); + + while ($row = $db->sql_fetchrow($result)) { - include($phpbb_root_path . 'includes/functions_admin.' . $phpEx); + unset($delete_ids[$row['msg_id']]); } + $db->sql_freeresult($result); - delete_attachments('message', $delete_ids, false); + if (!empty($delete_ids)) + { + // Check if there are any attachments we need to remove + if (!function_exists('delete_attachments')) + { + include($phpbb_root_path . 'includes/functions_admin.' . $phpEx); + } - $sql = 'DELETE FROM ' . PRIVMSGS_TABLE . ' - WHERE ' . $db->sql_in_set('msg_id', $delete_ids); - $db->sql_query($sql); + delete_attachments('message', $delete_ids, false); + + $sql = 'DELETE FROM ' . PRIVMSGS_TABLE . ' + WHERE ' . $db->sql_in_set('msg_id', $delete_ids); + $db->sql_query($sql); + } } // Set the remaining author id to anonymous -- cgit v1.2.1 From e68c1fb9e4d5de214c1e483531706ec300ffdb0d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 11 Jul 2012 12:58:57 +0200 Subject: [ticket/10950] Use database count() and group by instead of doing that in php PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 6c0adccbed..1d45961ac4 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1143,7 +1143,7 @@ function phpbb_delete_user_pms($user_id) if (!empty($undelivered_msg)) { - // A pm is not undelivered, if for any receipt the message was moved + // A pm is delivered, if for any receipt the message was moved // from their NO_BOX to another folder. $sql = 'SELECT msg_id FROM ' . PRIVMSGS_TO_TABLE . ' @@ -1165,23 +1165,17 @@ function phpbb_delete_user_pms($user_id) $undelivered_user = array(); // Count the messages we delete, so we can correct the user pm data - $sql = 'SELECT user_id + $sql = 'SELECT user_id, COUNT(msg_id) as num_undelivered_privmsgs FROM ' . PRIVMSGS_TO_TABLE . ' WHERE author_id = ' . $user_id . ' AND folder_id = ' . PRIVMSGS_NO_BOX . ' - AND ' . $db->sql_in_set('msg_id', $undelivered_msg); + AND ' . $db->sql_in_set('msg_id', $undelivered_msg) . ' + GROUP BY user_id'; $result = $db->sql_query($sql); while ($row = $db->sql_fetchrow($result)) { - if (isset($undelivered_user[$row['user_id']])) - { - ++$undelivered_user[$row['user_id']]; - } - else - { - $undelivered_user[$row['user_id']] = 1; - } + $undelivered_user[$row['user_id']] = (int) $row['num_undelivered_privmsgs']; } $db->sql_freeresult($result); -- cgit v1.2.1 From d883535b102ffba8781f485ba94fae237d8376b0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 11 Jul 2012 13:05:36 +0200 Subject: [ticket/10950] Remove deleted entries in tests instead of commenting them out PHPBB3-10950 --- tests/privmsgs/delete_user_pms_test.php | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/tests/privmsgs/delete_user_pms_test.php b/tests/privmsgs/delete_user_pms_test.php index e5c0e82712..a586d691e9 100644 --- a/tests/privmsgs/delete_user_pms_test.php +++ b/tests/privmsgs/delete_user_pms_test.php @@ -28,23 +28,10 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case 2, array( array('msg_id' => 1), - //array('msg_id' => 2), - //array('msg_id' => 3), - //array('msg_id' => 4), - //array('msg_id' => 5), ), array( - //array('msg_id' => 1, 'user_id' => 2), array('msg_id' => 1, 'user_id' => 3), array('msg_id' => 1, 'user_id' => 4), - //array('msg_id' => 2, 'user_id' => 2), - //array('msg_id' => 2, 'user_id' => 4), - //array('msg_id' => 4, 'user_id' => 3), - //array('msg_id' => 3, 'user_id' => 2), - //array('msg_id' => 4, 'user_id' => 3), - //array('msg_id' => 5, 'user_id' => 2), - //array('msg_id' => 5, 'user_id' => 3), - //array('msg_id' => 5, 'user_id' => 4), ), ), array( @@ -53,19 +40,15 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case array('msg_id' => 1), array('msg_id' => 2), array('msg_id' => 3), - //array('msg_id' => 4), array('msg_id' => 5), ), array( array('msg_id' => 1, 'user_id' => 2), - //array('msg_id' => 1, 'user_id' => 3), array('msg_id' => 1, 'user_id' => 4), array('msg_id' => 2, 'user_id' => 2), array('msg_id' => 2, 'user_id' => 4), array('msg_id' => 3, 'user_id' => 2), - //array('msg_id' => 4, 'user_id' => 3), array('msg_id' => 5, 'user_id' => 2), - //array('msg_id' => 5, 'user_id' => 3), array('msg_id' => 5, 'user_id' => 4), ), ), -- cgit v1.2.1 From d9a32ce6143be27b8414072694c969fa16b5329a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 16 Jul 2012 17:22:10 +0200 Subject: [ticket/10950] Update undelivered pm counts in batches not 1 by 1 for each user PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 1d45961ac4..82344d1787 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1175,16 +1175,29 @@ function phpbb_delete_user_pms($user_id) while ($row = $db->sql_fetchrow($result)) { - $undelivered_user[$row['user_id']] = (int) $row['num_undelivered_privmsgs']; + $num_pms = (int) $row['num_undelivered_privmsgs']; + $undelivered_user[$num_pms][] = (int) $row['user_id']; + + if (sizeof($undelivered_user[$num_pms]) > 50) + { + // If there are too many users affected the query might get + // too long, so we update the value for the first bunch here. + $sql = 'UPDATE ' . USERS_TABLE . ' + SET user_new_privmsg = user_new_privmsg - ' . $num_pms . ', + user_unread_privmsg = user_unread_privmsg - ' . $num_pms . ' + WHERE ' . $db->sql_in_set('user_id', $undelivered_user[$num_pms]); + $db->sql_query($sql); + unset($undelivered_user[$num_pms]); + } } $db->sql_freeresult($result); - foreach ($undelivered_user as $undelivered_user_id => $count) + foreach ($undelivered_user as $num_pms => $undelivered_user_set) { $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_new_privmsg = user_new_privmsg - ' . $count . ', - user_unread_privmsg = user_unread_privmsg - ' . $count . ' - WHERE user_id = ' . $undelivered_user_id; + SET user_new_privmsg = user_new_privmsg - ' . $num_pms . ', + user_unread_privmsg = user_unread_privmsg - ' . $num_pms . ' + WHERE ' . $db->sql_in_set('user_id', $undelivered_user_set); $db->sql_query($sql); } -- cgit v1.2.1 From a9c091fad47a3b6936bc7a08617e27163189a20f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 20 Jul 2012 16:10:40 +0200 Subject: [ticket/10950] Fix unit tests to fit the new pm deleting behaviour Undelivered PMs should not be delivered to recipients that have not yet received them. PHPBB3-10950 --- tests/privmsgs/delete_user_pms_test.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/privmsgs/delete_user_pms_test.php b/tests/privmsgs/delete_user_pms_test.php index a586d691e9..265df1596a 100644 --- a/tests/privmsgs/delete_user_pms_test.php +++ b/tests/privmsgs/delete_user_pms_test.php @@ -31,7 +31,6 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case ), array( array('msg_id' => 1, 'user_id' => 3), - array('msg_id' => 1, 'user_id' => 4), ), ), array( -- cgit v1.2.1 From a3517232f943fd8070c98a78f2cf731339b76a74 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 20 Jul 2012 16:13:08 +0200 Subject: [ticket/10950] Delete PMs for users that have not yet read the pm PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 76 ++++++++++++++++++++--------------- phpBB/language/en/acp/users.php | 2 +- 2 files changed, 45 insertions(+), 33 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 82344d1787..0890f057d2 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1144,7 +1144,9 @@ function phpbb_delete_user_pms($user_id) if (!empty($undelivered_msg)) { // A pm is delivered, if for any receipt the message was moved - // from their NO_BOX to another folder. + // from their NO_BOX to another folder. We do not delete such + // messages, but only delete them for users, who have not yet + // received them. $sql = 'SELECT msg_id FROM ' . PRIVMSGS_TO_TABLE . ' WHERE author_id = ' . $user_id . ' @@ -1153,54 +1155,64 @@ function phpbb_delete_user_pms($user_id) AND folder_id <> ' . PRIVMSGS_SENTBOX; $result = $db->sql_query($sql); + $delivered_msg = array(); while ($row = $db->sql_fetchrow($result)) { $msg_id = (int) $row['msg_id']; + $delivered_msg[$msg_id] = $msg_id; unset($undelivered_msg[$msg_id]); } $db->sql_freeresult($result); - if (!empty($undelivered_msg)) - { - $undelivered_user = array(); - - // Count the messages we delete, so we can correct the user pm data - $sql = 'SELECT user_id, COUNT(msg_id) as num_undelivered_privmsgs - FROM ' . PRIVMSGS_TO_TABLE . ' - WHERE author_id = ' . $user_id . ' - AND folder_id = ' . PRIVMSGS_NO_BOX . ' - AND ' . $db->sql_in_set('msg_id', $undelivered_msg) . ' - GROUP BY user_id'; - $result = $db->sql_query($sql); + $undelivered_user = array(); - while ($row = $db->sql_fetchrow($result)) - { - $num_pms = (int) $row['num_undelivered_privmsgs']; - $undelivered_user[$num_pms][] = (int) $row['user_id']; + // Count the messages we delete, so we can correct the user pm data + $sql = 'SELECT user_id, COUNT(msg_id) as num_undelivered_privmsgs + FROM ' . PRIVMSGS_TO_TABLE . ' + WHERE author_id = ' . $user_id . ' + AND folder_id = ' . PRIVMSGS_NO_BOX . ' + AND ' . $db->sql_in_set('msg_id', array_merge($undelivered_msg, $delivered_msg)) . ' + GROUP BY user_id'; + $result = $db->sql_query($sql); - if (sizeof($undelivered_user[$num_pms]) > 50) - { - // If there are too many users affected the query might get - // too long, so we update the value for the first bunch here. - $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_new_privmsg = user_new_privmsg - ' . $num_pms . ', - user_unread_privmsg = user_unread_privmsg - ' . $num_pms . ' - WHERE ' . $db->sql_in_set('user_id', $undelivered_user[$num_pms]); - $db->sql_query($sql); - unset($undelivered_user[$num_pms]); - } - } - $db->sql_freeresult($result); + while ($row = $db->sql_fetchrow($result)) + { + $num_pms = (int) $row['num_undelivered_privmsgs']; + $undelivered_user[$num_pms][] = (int) $row['user_id']; - foreach ($undelivered_user as $num_pms => $undelivered_user_set) + if (sizeof($undelivered_user[$num_pms]) > 50) { + // If there are too many users affected the query might get + // too long, so we update the value for the first bunch here. $sql = 'UPDATE ' . USERS_TABLE . ' SET user_new_privmsg = user_new_privmsg - ' . $num_pms . ', user_unread_privmsg = user_unread_privmsg - ' . $num_pms . ' - WHERE ' . $db->sql_in_set('user_id', $undelivered_user_set); + WHERE ' . $db->sql_in_set('user_id', $undelivered_user[$num_pms]); $db->sql_query($sql); + unset($undelivered_user[$num_pms]); } + } + $db->sql_freeresult($result); + + foreach ($undelivered_user as $num_pms => $undelivered_user_set) + { + $sql = 'UPDATE ' . USERS_TABLE . ' + SET user_new_privmsg = user_new_privmsg - ' . $num_pms . ', + user_unread_privmsg = user_unread_privmsg - ' . $num_pms . ' + WHERE ' . $db->sql_in_set('user_id', $undelivered_user_set); + $db->sql_query($sql); + } + if (!empty($delivered_msg)) + { + $sql = 'DELETE FROM ' . PRIVMSGS_TO_TABLE . ' + WHERE folder_id = ' . PRIVMSGS_NO_BOX . ' + AND ' . $db->sql_in_set('msg_id', $delivered_msg); + $db->sql_query($sql); + } + + if (!empty($undelivered_msg)) + { $sql = 'DELETE FROM ' . PRIVMSGS_TO_TABLE . ' WHERE ' . $db->sql_in_set('msg_id', $undelivered_msg); $db->sql_query($sql); diff --git a/phpBB/language/en/acp/users.php b/phpBB/language/en/acp/users.php index 785283faea..3b82a7022d 100644 --- a/phpBB/language/en/acp/users.php +++ b/phpBB/language/en/acp/users.php @@ -59,7 +59,7 @@ $lang = array_merge($lang, array( 'DELETE_POSTS' => 'Delete posts', 'DELETE_USER' => 'Delete user', - 'DELETE_USER_EXPLAIN' => 'Please note that deleting a user is final, they cannot be recovered.', + 'DELETE_USER_EXPLAIN' => 'Please note that deleting a user is final, they cannot be recovered. Unread private messages sent by this user will be deleted and will not be available to their recipients.', 'FORCE_REACTIVATION_SUCCESS' => 'Successfully forced reactivation.', 'FOUNDER' => 'Founder', -- cgit v1.2.1 From 3036db481a40a395f9021b991dd3d659649ec831 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 20 Jul 2012 18:01:06 +0200 Subject: [ticket/10950] Fix grammar in comments PHPBB3-10950 --- phpBB/includes/functions_privmsgs.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 0890f057d2..b08d6e7f5c 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1118,8 +1118,8 @@ function phpbb_delete_user_pms($user_id) } $db->sql_freeresult($result); - // Part 2: get PMs the user sent, but has yet to be received - // We can not simply delete them. First we have to check, + // Part 2: get PMs the user sent, but have yet to be received + // We cannot simply delete them. First we have to check, // whether another user already received and read the message. $sql = 'SELECT msg_id FROM ' . PRIVMSGS_TO_TABLE . ' @@ -1143,7 +1143,7 @@ function phpbb_delete_user_pms($user_id) if (!empty($undelivered_msg)) { - // A pm is delivered, if for any receipt the message was moved + // A pm is delivered, if for any recipient the message was moved // from their NO_BOX to another folder. We do not delete such // messages, but only delete them for users, who have not yet // received them. -- cgit v1.2.1