[ticket/11103] The service now handles all user loading itself

Delete pm notifications when pms are deleted

PHPBB3-11103

phpBB/includes/functions_privmsgs.php

@@ -982,6 +982,7 @@ function handle_mark_actions($user_id, $mark_action)
 function delete_pm($user_id, $msg_ids, $folder_id)
 {
 	global $db, $user, $phpbb_root_path, $phpEx;
+	global $phpbb_container;
 
 	$user_id	= (int) $user_id;
 	$folder_id	= (int) $folder_id;
@@ -1093,6 +1094,10 @@ function delete_pm($user_id, $msg_ids, $folder_id)
 		$user->data['user_unread_privmsg'] -= $num_unread;
 	}
 
+	// Delete Notifications
+	$phpbb_notifications = $phpbb_container->get('notifications');
+	$phpbb_notifications->delete_notifications('pm', array_keys($delete_rows));
+
 	// Now we have to check which messages we can delete completely
 	$sql = 'SELECT msg_id
 		FROM ' . PRIVMSGS_TO_TABLE . '
@@ -1843,106 +1848,25 @@ function submit_pm($mode, $subject, &$data, $put_in_outbox = true)
 	$db->sql_transaction('commit');
 
 	// Send Notifications
-	if ($mode != 'edit')
+	$phpbb_notifications = $phpbb_container->get('notifications');
+
+	$pm_data = array_merge($data, array(
+		'message_subject'		=> $subject,
+		'recipients'			=> $recipients,
+	));
+
+	if ($mode == 'edit')
 	{
-		$phpbb_notifications = $phpbb_container->get('notifications');
+		$phpbb_notifications->update_notifications('pm', $pm_data);
-
+	}
-		$phpbb_notifications->add_notifications('pm', array(
+	else
-			'author_id'				=> $data['from_user_id'],
+	{
-			'recipients'			=> $recipients,
+		$phpbb_notifications->add_notifications('pm', $pm_data);
-			'message_subject'		=> $subject,
-			'msg_id'				=> $data['msg_id'],
-		));
-
-		//pm_notification($mode, $data['from_username'], $recipients, $subject, $data['message'], $data['msg_id']);
 	}
 
 	return $data['msg_id'];
 }
 
-/**
-* PM Notification
-*/
-function pm_notification($mode, $author, $recipients, $subject, $message, $msg_id)
-{
-	global $db, $user, $config, $phpbb_root_path, $phpEx, $auth;
-
-	$subject = censor_text($subject);
-
-	// Exclude guests, current user and banned users from notifications
-	unset($recipients[ANONYMOUS], $recipients[$user->data['user_id']]);
-
-	if (!sizeof($recipients))
-	{
-		return;
-	}
-
-	if (!function_exists('phpbb_get_banned_user_ids'))
-	{
-		include($phpbb_root_path . 'includes/functions_user.' . $phpEx);
-	}
-	$banned_users = phpbb_get_banned_user_ids(array_keys($recipients));
-	$recipients = array_diff(array_keys($recipients), $banned_users);
-
-	if (!sizeof($recipients))
-	{
-		return;
-	}
-
-	$sql = 'SELECT user_id, username, user_email, user_lang, user_notify_pm, user_notify_type, user_jabber
-		FROM ' . USERS_TABLE . '
-		WHERE ' . $db->sql_in_set('user_id', $recipients);
-	$result = $db->sql_query($sql);
-
-	$msg_list_ary = array();
-	while ($row = $db->sql_fetchrow($result))
-	{
-		if ($row['user_notify_pm'] == 1 && trim($row['user_email']))
-		{
-			$msg_list_ary[] = array(
-				'method'	=> $row['user_notify_type'],
-				'email'		=> $row['user_email'],
-				'jabber'	=> $row['user_jabber'],
-				'name'		=> $row['username'],
-				'lang'		=> $row['user_lang']
-			);
-		}
-	}
-	$db->sql_freeresult($result);
-
-	if (!sizeof($msg_list_ary))
-	{
-		return;
-	}
-
-	include_once($phpbb_root_path . 'includes/functions_messenger.' . $phpEx);
-	$messenger = new messenger();
-
-	foreach ($msg_list_ary as $pos => $addr)
-	{
-		$messenger->template('privmsg_notify', $addr['lang']);
-
-		$messenger->to($addr['email'], $addr['name']);
-		$messenger->im($addr['jabber'], $addr['name']);
-
-		$messenger->assign_vars(array(
-			'SUBJECT'		=> htmlspecialchars_decode($subject),
-			'AUTHOR_NAME'	=> htmlspecialchars_decode($author),
-			'USERNAME'		=> htmlspecialchars_decode($addr['name']),
-
-			'U_INBOX'			=> generate_board_url() . "/ucp.$phpEx?i=pm&folder=inbox",
-			'U_VIEW_MESSAGE'	=> generate_board_url() . "/ucp.$phpEx?i=pm&mode=view&p=$msg_id",
-		));
-
-		$messenger->send($addr['method']);
-	}
-	unset($msg_list_ary);
-
-	$messenger->save_queue();
-
-	unset($messenger);
-}
-
 /**
 * Display Message History
 */

phpBB/includes/notifications/method/base.php

@@ -24,11 +24,31 @@ if (!defined('IN_PHPBB'))
 abstract class phpbb_notifications_method_base implements phpbb_notifications_method_interface
 {
 	protected $phpbb_container;
+	protected $service;
 	protected $db;
 	protected $user;
 	protected $phpbb_root_path;
 	protected $php_ext;
 
+	/**
+	* Desired notifications
+	* unique by (type, type_id, user_id, method)
+	* if multiple methods are desired, multiple rows will exist.
+	*
+	* method of "none" will over-ride any other options
+	*
+	* item_type
+	* item_id
+	* user_id
+	* method
+	* 	none (will never receive notifications)
+	* 	standard (listed in notifications window
+	* 	popup?
+	* 	email
+	* 	jabber
+	*	sms?
+	*/
+
 	/**
 	* Queue of messages to be sent
 	*
@@ -36,11 +56,14 @@ abstract class phpbb_notifications_method_base implements phpbb_notifications_me
 	*/
 	protected $queue = array();
 
-	public function __construct(ContainerBuilder $phpbb_container, $data = array())
+	public function __construct(ContainerBuilder $phpbb_container)
 	{
 		// phpBB Container
 		$this->phpbb_container = $phpbb_container;
 
+		// Service
+		$this->service = $phpbb_container->get('notifications');
+
 		// Some common things we're going to use
 		$this->db = $phpbb_container->get('dbal.conn');
 		$this->user = $phpbb_container->get('user');

phpBB/includes/notifications/method/email.php

@@ -59,14 +59,8 @@ class phpbb_notifications_method_email extends phpbb_notifications_method_base
 		}
 		$banned_users = phpbb_get_banned_user_ids($user_ids);
 
-		$sql = 'SELECT * FROM ' . USERS_TABLE . '
+		// Load all the users we need
-			WHERE ' . $this->db->sql_in_set('user_id', $user_ids);
+		$this->service->load_users($user_ids);
-		$result = $this->db->sql_query($sql);
-		while ($row = $this->db->sql_fetchrow($result))
-		{
-			$users[$row['user_id']] = $row;
-		}
-		$this->db->sql_freeresult($result);
 
 		// Load the messenger
 		if (!class_exists('messenger'))
@@ -84,9 +78,7 @@ class phpbb_notifications_method_email extends phpbb_notifications_method_base
 				continue;
 			}
 
-			$notification->users($users);
+			$user = $this->service->get_user($notification->user_id);
-
-			$user = $notification->get_user($notification->user_id);
 
 			$messenger->template('privmsg_notify', $user['user_lang']);
 

phpBB/includes/notifications/service.php

@@ -31,7 +31,7 @@ class phpbb_notifications_service
 	*
 	* @var array Array of user data that we've loaded from the DB
 	*/
-	protected $users;
+	protected $users = array();
 
 	public function __construct(ContainerBuilder $phpbb_container)
 	{
@@ -76,7 +76,6 @@ class phpbb_notifications_service
 			$item_type_class_name = $this->get_item_type_class_name($row['item_type'], true);
 
 			$notification = new $item_type_class_name($this->phpbb_container, $row);
-			$notification->users($this->users);
 
 			$user_ids = array_merge($user_ids, $notification->users_to_query());
 
@@ -84,24 +83,7 @@ class phpbb_notifications_service
 		}
 		$this->db->sql_freeresult($result);
 
-		// Load the users
+		$this->load_users($user_ids);
-		$user_ids = array_unique($user_ids);
-
-		// @todo do not load users we already have in $this->users
-
-		if (sizeof($user_ids))
-		{
-			// @todo do not select everything
-			$sql = 'SELECT * FROM ' . USERS_TABLE . '
-				WHERE ' . $this->db->sql_in_set('user_id', $user_ids);
-			$result = $this->db->sql_query($sql);
-
-			while ($row = $this->db->sql_fetchrow($result))
-			{
-				$this->users[$row['user_id']] = $row;
-			}
-			$this->db->sql_freeresult($result);
-		}
 
 		return $notifications;
 	}
@@ -122,32 +104,16 @@ class phpbb_notifications_service
 		// Update any existing notifications for this item
 		$this->update_notifications($item_type, $item_id, $data);
 
-		$notify_users = array();
+		$notify_users = $user_ids = array();
 		$notification_objects = $notification_methods = array();
 		$new_rows = array();
 
-		/**
-		* Desired notifications
-		* unique by (type, type_id, user_id, method)
-		* if multiple methods are desired, multiple rows will exist.
-		*
-		* method of "none" will over-ride any other options
-		*
-		* item_type
-		* item_id
-		* user_id
-		* method
-		* 	none (will never receive notifications)
-		* 	standard (listed in notifications window
-		* 	popup?
-		* 	email
-		* 	jabber
-		*	sms?
-		*/
-
 		// find out which users want to receive this type of notification
 		$notify_users = $item_type_class_name::find_users_for_notification($this->phpbb_container, $data);
 
+		// Never send notifications to the anonymous user or the current user!
+		$notify_users = array_diff($notify_users, array(ANONYMOUS, $this->phpbb_container->get('user')->data['user_id']));
+
 		// Make sure not to send new notifications to users who've already been notified about this item
 		// This may happen when an item was added, but now new users are able to see the item
 		$sql = 'SELECT user_id FROM ' . NOTIFICATIONS_TABLE . "
@@ -172,8 +138,12 @@ class phpbb_notifications_service
 
 			$notification->user_id = (int) $user;
 
+			// Store the creation array in our new rows that will be inserted later
 			$new_rows[] = $notification->create_insert_array($data);
 
+			// Users are needed to send notifications
+			$user_ids = array_merge($user_ids, $notification->users_to_query());
+
 			foreach ($methods as $method)
 			{
 				// setup the notification methods and add the notification to the queue
@@ -184,6 +154,7 @@ class phpbb_notifications_service
 						$method_class_name = 'phpbb_notifications_method_' . $method;
 						$notification_methods[$method] = new $method_class_name($this->phpbb_container);
 					}
+
 					$notification_methods[$method]->add_to_queue($notification);
 				}
 			}
@@ -192,6 +163,9 @@ class phpbb_notifications_service
 		// insert into the db
 		$this->db->sql_multi_insert(NOTIFICATIONS_TABLE, $new_rows);
 
+		// We need to load all of the users to send notifications
+		$this->load_users($user_ids);
+
 		// run the queue for each method to send notifications
 		foreach ($notification_methods as $method)
 		{
@@ -203,13 +177,14 @@ class phpbb_notifications_service
 	* Update a notification
 	*
 	* @param string $item_type Type identifier
-	* @param int $item_id Identifier within the type
 	* @param array $data Data specific for this type that will be updated
 	*/
-	public function update_notifications($item_type, $item_id, $data)
+	public function update_notifications($item_type, $data)
 	{
 		$item_type_class_name = $this->get_item_type_class_name($item_type);
 
+		$item_id = $item_type_class_name::get_item_id($data);
+
 		$notification = new $item_type_class_name($this->phpbb_container);
 		$update_array = $notification->create_update_array($data);
 
@@ -224,17 +199,55 @@ class phpbb_notifications_service
 	* Delete a notification
 	*
 	* @param string $item_type Type identifier
-	* @param int $item_id Identifier within the type
+	* @param int|array $item_id Identifier within the type (or array of ids)
 	* @param array $data Data specific for this type that will be updated
 	*/
 	public function delete_notifications($item_type, $item_id)
 	{
 		$sql = 'DELETE FROM ' . NOTIFICATIONS_TABLE . "
 			WHERE item_type = '" . $this->db->sql_escape($item_type) . "'
-				AND item_id = " . (int) $item_id;
+				AND " . (is_array($item_id) ? $this->db->sql_in_set('item_id', $item_id) : 'item_id = ' . (int) $item_id);
 		$this->db->sql_query($sql);
 	}
 
+	/**
+	* Load user helper
+	*
+	* @param array $user_ids
+	*/
+	public function load_users($user_ids)
+	{
+		// Load the users
+		$user_ids = array_unique($user_ids);
+
+		// Do not load users we already have in $this->users
+		$user_ids = array_diff($user_ids, array_keys($this->users));
+
+		if (sizeof($user_ids))
+		{
+			$sql = 'SELECT * FROM ' . USERS_TABLE . '
+				WHERE ' . $this->db->sql_in_set('user_id', $user_ids);
+			$result = $this->db->sql_query($sql);
+
+			while ($row = $this->db->sql_fetchrow($result))
+			{
+				$this->users[$row['user_id']] = $row;
+			}
+			$this->db->sql_freeresult($result);
+		}
+	}
+
+	/**
+	* Get a user row from our users cache
+	*
+	* @param int $user_id
+	* @return array
+	*/
+	public function get_user($user_id)
+	{
+		return $this->users[$user_id];
+	}
+
 	/**
 	* Helper to get the notifications item type class name and clean it if unsafe
 	*/

phpBB/includes/notifications/type/base.php

@@ -24,6 +24,7 @@ if (!defined('IN_PHPBB'))
 abstract class phpbb_notifications_type_base implements phpbb_notifications_type_interface
 {
 	protected $phpbb_container;
+	protected $service;
 	protected $db;
 	protected $phpbb_root_path;
 	protected $php_ext;
@@ -55,6 +56,9 @@ abstract class phpbb_notifications_type_base implements phpbb_notifications_type
 		// phpBB Container
 		$this->phpbb_container = $phpbb_container;
 
+		// Service
+		$this->service = $phpbb_container->get('notifications');
+
 		// Some common things we're going to use
 		$this->db = $phpbb_container->get('dbal.conn');
 
@@ -99,26 +103,6 @@ abstract class phpbb_notifications_type_base implements phpbb_notifications_type
 		$this->data['data'][$name] = $value;
 	}
 
-	/**
-	* Function to store the users loaded from the database (for output to the template)
-	* (The service handles this)
-	*/
-	public function users(&$users)
-	{
-		$this->users = &$users;
-	}
-
-	/**
-	* Get a user row from our users cache
-	*
-	* @param int $user_id
-	* @return array
-	*/
-	public function get_user($user_id)
-	{
-		return $this->users[$user_id];
-	}
-
 	/**
 	* Output the notification to the template
 	*

phpBB/includes/notifications/type/interface.php

@@ -25,6 +25,8 @@ interface phpbb_notifications_type_interface
 
 	public static function get_item_id($type_data);
 
+	public static function find_users_for_notification(ContainerBuilder $phpbb_container, $type_data);
+
 	public function get_title();
 
 	public function get_url();
@@ -32,6 +34,4 @@ interface phpbb_notifications_type_interface
 	public function get_full_url();
 
 	public function create_insert_array($type_data);
-
-	public static function find_users_for_notification(ContainerBuilder $phpbb_container, $type_data);
 }

phpBB/includes/notifications/type/pm.php

@@ -44,6 +44,50 @@ class phpbb_notifications_type_pm extends phpbb_notifications_type_base
 		return $pm['msg_id'];
 	}
 
+	/**
+	* Find the users who want to receive notifications
+	*
+	* @param array $pm Data from
+	* @return array
+	*/
+	public static function find_users_for_notification(ContainerBuilder $phpbb_container, $pm)
+	{
+		$service = $phpbb_container->get('notifications');
+		$db = $phpbb_container->get('dbal.conn');
+		$user = $phpbb_container->get('user');
+
+		if (!sizeof($pm['recipients']))
+		{
+			return array();
+		}
+
+		$service->load_users(array_keys($pm['recipients']));
+
+		$notify_users = array();
+
+		foreach (array_keys($pm['recipients']) as $user_id)
+		{
+			$recipient = $service->get_user($user_id);
+
+			if ($recipient['user_notify_pm'])
+			{
+				$notify_users[$recipient['user_id']] = array();
+
+				if ($recipient['user_notify_type'] == NOTIFY_EMAIL || $recipient['user_notify_type'] == NOTIFY_BOTH)
+				{
+					$notify_users[$recipient['user_id']][] = 'email';
+				}
+
+				if ($recipient['user_notify_type'] == NOTIFY_IM || $recipient['user_notify_type'] == NOTIFY_BOTH)
+				{
+					$notify_users[$recipient['user_id']][] = 'jabber';
+				}
+			}
+		}
+
+		return $notify_users;
+	}
+
 	/**
 	* Get the title of this notification
 	*
@@ -51,7 +95,7 @@ class phpbb_notifications_type_pm extends phpbb_notifications_type_base
 	*/
 	public function get_title()
 	{
-		$user_data = $this->get_user($this->get_data('author_id'));
+		$user_data = $this->service->get_user($this->get_data('from_user_id'));
 
 		$username = get_username_string('no_profile', $user_data['user_id'], $user_data['username'], $user_data['user_colour']);
 
@@ -85,7 +129,7 @@ class phpbb_notifications_type_pm extends phpbb_notifications_type_base
 	*/
 	public function users_to_query()
 	{
-		return array($this->data['author_id']);
+		return array($this->data['from_user_id']);
 	}
 
 	/**
@@ -100,58 +144,10 @@ class phpbb_notifications_type_pm extends phpbb_notifications_type_base
 	{
 		$this->item_id = $pm['msg_id'];
 
-		$this->set_data('author_id', $pm['author_id']);
+		$this->set_data('from_user_id', $pm['from_user_id']);
 
 		$this->set_data('message_subject', $pm['message_subject']);
 
 		return parent::create_insert_array($pm);
 	}
-
-	/**
-	* Find the users who want to receive notifications
-	*
-	* @param array $pm Data from
-	* @return array
-	*/
-	public static function find_users_for_notification(ContainerBuilder $phpbb_container, $pm)
-	{
-		$db = $phpbb_container->get('dbal.conn');
-		$user = $phpbb_container->get('user');
-
-		// Exclude guests and current user from notifications
-		unset($pm['recipients'][ANONYMOUS], $pm['recipients'][$user->data['user_id']]);
-
-		if (!sizeof($pm['recipients']))
-		{
-			return;
-		}
-
-		$sql = 'SELECT user_id, user_notify_pm, user_notify_type
-			FROM ' . USERS_TABLE . '
-			WHERE ' . $db->sql_in_set('user_id', $pm['recipients']);
-		$result = $db->sql_query($sql);
-
-		$pm['recipients'] = array();
-
-		while ($row = $db->sql_fetchrow($result))
-		{
-			if ($row['user_notify_pm'])
-			{
-				$pm['recipients'][$row['user_id']] = array();
-
-				if ($row['user_notify_type'] == NOTIFY_EMAIL || $row['user_notify_type'] == NOTIFY_BOTH)
-				{
-					$pm['recipients'][$row['user_id']][] = 'email';
-				}
-
-				if ($row['user_notify_type'] == NOTIFY_IM || $row['user_notify_type'] == NOTIFY_BOTH)
-				{
-					$pm['recipients'][$row['user_id']][] = 'jabber';
-				}
-			}
-		}
-		$db->sql_freeresult($result);
-
-		return $pm['recipients'];
-	}
 }