Merge branch 'prep-release-3.0.11' into develop-olympus

* prep-release-3.0.11: [ticket/10950] Fix grammar in comments [ticket/10950] Delete PMs for users that have not yet read the pm [ticket/10950] Fix unit tests to fit the new pm deleting behaviour [ticket/10950] Update undelivered pm counts in batches not 1 by 1 for each user [ticket/10950] Remove deleted entries in tests instead of commenting them out [ticket/10950] Use database count() and group by instead of doing that in php [ticket/10950] Check $delete_ids to be not empty [ticket/10950] Recreated the behaviour of phpbb_delete_user_pms() [ticket/10950] Fix unit tests to reflect desired behaviour [ticket/10950] Add some first and simple unit tests for phpbb_delete_user_pms() [ticket/10950] Correct comment for the second query. [ticket/10950] Add empty line to make unset() call more visible. [ticket/10950] Select the correct columns in SQL queries. [ticket/10950] Use a variable for the private message id. [ticket/10950] Move array initialisation to the front. [ticket/10950] Remove redundant if statement. [ticket/10950] Fix SQL coding style (indentation) in second SQL query. [ticket/10950] Use proper ' in order to fix comment.
author: Nils Adermann <naderman@naderman.de> 2012-07-21 16:15:32 +0200
committer: Nils Adermann <naderman@naderman.de> 2012-07-21 16:15:32 +0200
commit: 29bee5b01a7dfa613b1583b1b2c178c641786f4b (patch)
tree: eae85bf5868cfd92db503097354ada7a558e12a2
parent: 1e87e0dab86cd1455cf8d7a282ff81430a6f2f99 (diff)
parent: 3dfcb212dc00aa0f33178bfc816b70ee48440ee2 (diff)
download: forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.tar
forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.tar.gz
forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.tar.bz2
forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.tar.xz
forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.zip
4 files changed, 428 insertions, 84 deletions
diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php
index 261ed45727..b08d6e7f5c 100644
--- a/phpBB/includes/functions_privmsgs.php
+++ b/phpBB/includes/functions_privmsgs.php
@@ -1103,127 +1103,166 @@ 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 = $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
 		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)
-		{
-			// 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;
-			}
-		}
-
-		$delete_ids[(int) $row['msg_id']] = (int) $row['msg_id'];
+		$msg_id = (int) $row['msg_id'];
+		$delete_ids[$msg_id] = $msg_id;
 	}
 	$db->sql_freeresult($result);
 
-	// Part 2: get PMs the user sent
-	$sql = 'SELECT msg_id, author_id, folder_id, pm_unread, pm_new
+	// 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 . '
 		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))
 	{
-		if ($row['author_id'] == $user_id && $row['folder_id'] == PRIVMSGS_NO_BOX)
-		{
-			// 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;
-			}
-		}
-
-		$delete_ids[(int) $row['msg_id']] = (int) $row['msg_id'];
+		$msg_id = (int) $row['msg_id'];
+		$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))
-	{
-		$sql = 'DELETE FROM ' . PRIVMSGS_TABLE . '
-			WHERE ' . $db->sql_in_set('msg_id', $undelivered_msg);
-		$db->sql_query($sql);
-	}
+	if (!empty($undelivered_msg))
+	{
+		// 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.
+		$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);
-		unset($undelivered_user[$user_id]);
-	}
+		$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);
 
-	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);
-	}
+		$undelivered_user = array();
 
-	// Delete private message data
-	$sql = 'DELETE FROM ' . PRIVMSGS_TO_TABLE . "
-		WHERE user_id = $user_id
-			AND " . $db->sql_in_set('msg_id', $delete_ids);
-	$db->sql_query($sql);
+		// 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);
 
-	// 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))
+		{
+			$num_pms = (int) $row['num_undelivered_privmsgs'];
+			$undelivered_user[$num_pms][] = (int) $row['user_id'];
 
-	while ($row = $db->sql_fetchrow($result))
-	{
-		unset($delete_ids[$row['msg_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 $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);
+
+			$sql = 'DELETE FROM ' . PRIVMSGS_TABLE . '
+				WHERE ' . $db->sql_in_set('msg_id', $undelivered_msg);
+			$db->sql_query($sql);
+		}
 	}
-	$db->sql_freeresult($result);
+
+	// 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);
 
 	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
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',
diff --git a/tests/privmsgs/delete_user_pms_test.php b/tests/privmsgs/delete_user_pms_test.php
new file mode 100644
index 0000000000..265df1596a
--- /dev/null
+++ b/tests/privmsgs/delete_user_pms_test.php
@@ -0,0 +1,102 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2011 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/functions_privmsgs.php';
+
+class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case
+{
+	public function getDataSet()
+	{
+		return $this->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(
+					array('msg_id' => 1, 'user_id' => 3),
+				),
+			),
+			array(
+				3,
+				array(
+					array('msg_id' => 1),
+					array('msg_id' => 2),
+					array('msg_id' => 3),
+					array('msg_id' => 5),
+				),
+				array(
+					array('msg_id' => 1, 'user_id' => 2),
+					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' => 5, 'user_id' => 2),
+					array('msg_id' => 5, 'user_id' => 4),
+				),
+			),
+			array(
+				5,
+				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' => 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),
+				),
+			),
+		);
+	}
+
+	/**
+	* @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..56970689a3
--- /dev/null
+++ b/tests/privmsgs/fixtures/delete_user_pms.xml
@@ -0,0 +1,203 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dataset>
+	<table name="phpbb_users">
+		<column>user_id</column>
+		<column>username</column>
+		<column>username_clean</column>
+		<column>user_new_privmsg</column>
+		<column>user_unread_privmsg</column>
+		<column>user_permissions</column>
+		<column>user_sig</column>
+		<column>user_occ</column>
+		<column>user_interests</column>
+		<row>
+			<value>2</value>
+			<value>sender</value>
+			<value>sender</value>
+			<value>0</value>
+			<value>0</value>
+			<value></value>
+			<value></value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>3</value>
+			<value>pm in inbox</value>
+			<value>pm in inbox</value>
+			<value>0</value>
+			<value>0</value>
+			<value></value>
+			<value></value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>4</value>
+			<value>pm in no box</value>
+			<value>pm in no box</value>
+			<value>2</value>
+			<value>2</value>
+			<value></value>
+			<value></value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>5</value>
+			<value>no pms</value>
+			<value>no pms</value>
+			<value>0</value>
+			<value>0</value>
+			<value></value>
+			<value></value>
+			<value></value>
+			<value></value>
+		</row>
+	</table>
+	<table name="phpbb_privmsgs">
+		<column>msg_id</column>
+		<column>root_level</column>
+		<column>author_id</column>
+		<column>message_subject</column>
+		<column>message_text</column>
+		<row>
+			<value>1</value>
+			<value>0</value>
+			<value>2</value>
+			<value>#1</value>
+			<value>
+				2 - outbox
+				3 - inbox
+				4 - nobox
+			</value>
+		</row>
+		<row>
+			<value>2</value>
+			<value>0</value>
+			<value>2</value>
+			<value>#2</value>
+			<value>
+				2 - outbox
+				4 - nobox
+			</value>
+		</row>
+		<row>
+			<value>3</value>
+			<value>0</value>
+			<value>2</value>
+			<value>#3</value>
+			<value>
+				2 - outbox
+			</value>
+		</row>
+		<row>
+			<value>4</value>
+			<value>0</value>
+			<value>2</value>
+			<value>#4</value>
+			<value>
+				3 - nobox
+			</value>
+		</row>
+		<row>
+			<value>5</value>
+			<value>0</value>
+			<value>2</value>
+			<value>#5</value>
+			<value>
+				2 - outbox
+				3 - nobox
+				4 - nobox
+			</value>
+		</row>
+	</table>
+	<table name="phpbb_privmsgs_to">
+		<column>msg_id</column>
+		<column>user_id</column>
+		<column>author_id</column>
+		<column>pm_new</column>
+		<column>pm_unread</column>
+		<column>folder_id</column>
+		<row>
+			<value>1</value>
+			<value>2</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-2</value>
+		</row>
+		<row>
+			<value>1</value>
+			<value>3</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>0</value>
+		</row>
+		<row>
+			<value>1</value>
+			<value>4</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-3</value>
+		</row>
+		<row>
+			<value>2</value>
+			<value>2</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-2</value>
+		</row>
+		<row>
+			<value>2</value>
+			<value>4</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-3</value>
+		</row>
+		<row>
+			<value>3</value>
+			<value>2</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-2</value>
+		</row>
+		<row>
+			<value>4</value>
+			<value>3</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-3</value>
+		</row>
+		<row>
+			<value>5</value>
+			<value>2</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-2</value>
+		</row>
+		<row>
+			<value>5</value>
+			<value>3</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-3</value>
+		</row>
+		<row>
+			<value>5</value>
+			<value>4</value>
+			<value>2</value>
+			<value>0</value>
+			<value>0</value>
+			<value>-3</value>
+		</row>
+	</table>
+</dataset>
author	Nils Adermann <naderman@naderman.de>	2012-07-21 16:15:32 +0200
committer	Nils Adermann <naderman@naderman.de>	2012-07-21 16:15:32 +0200
commit	29bee5b01a7dfa613b1583b1b2c178c641786f4b (patch)
tree	eae85bf5868cfd92db503097354ada7a558e12a2
parent	1e87e0dab86cd1455cf8d7a282ff81430a6f2f99 (diff)
parent	3dfcb212dc00aa0f33178bfc816b70ee48440ee2 (diff)
download	forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.tar forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.tar.gz forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.tar.bz2 forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.tar.xz forums-29bee5b01a7dfa613b1583b1b2c178c641786f4b.zip