7 files changed, 88 insertions, 35 deletions
diff --git a/phpBB/phpbb/notification/type/approve_post.php b/phpBB/phpbb/notification/type/approve_post.php
index e51ff12b3e..5912ad62b4 100644
--- a/phpBB/phpbb/notification/type/approve_post.php
+++ b/phpBB/phpbb/notification/type/approve_post.php
@@ -138,4 +138,12 @@ class approve_post extends \phpbb\notification\type\post
 	{
 		return 'post_approved';
 	}
+
+	/**
+	* {inheritDoc}
+	*/
+	public function get_redirect_url()
+	{
+		return $this->get_url();
+	}
 }
diff --git a/phpBB/phpbb/notification/type/base.php b/phpBB/phpbb/notification/type/base.php
index 0719540bdb..7d08521d40 100644
--- a/phpBB/phpbb/notification/type/base.php
+++ b/phpBB/phpbb/notification/type/base.php
@@ -276,6 +276,14 @@ abstract class base implements \phpbb\notification\type\type_interface
 	}
 
 	/**
+	* {inheritDoc}
+	*/
+	public function get_redirect_url()
+	{
+		return $this->get_url();
+	}
+
+	/**
 	* Prepare to output the notification to the template
 	*
 	* @return array Template variables
diff --git a/phpBB/phpbb/notification/type/bookmark.php b/phpBB/phpbb/notification/type/bookmark.php
index 003998677d..c981695f74 100644
--- a/phpBB/phpbb/notification/type/bookmark.php
+++ b/phpBB/phpbb/notification/type/bookmark.php
@@ -110,10 +110,14 @@ class bookmark extends \phpbb\notification\type\post
 			unset($notify_users[$row['user_id']]);
 
 			$notification = $this->notification_manager->get_item_type_class($this->get_type(), $row);
-			$sql = 'UPDATE ' . $this->notifications_table . '
-				SET ' . $this->db->sql_build_array('UPDATE', $notification->add_responders($post)) . '
-				WHERE notification_id = ' . $row['notification_id'];
-			$this->db->sql_query($sql);
+			$update_responders = $notification->add_responders($post);
+			if (!empty($update_responders))
+			{
+				$sql = 'UPDATE ' . $this->notifications_table . '
+					SET ' . $this->db->sql_build_array('UPDATE', $update_responders) . '
+					WHERE notification_id = ' . $row['notification_id'];
+				$this->db->sql_query($sql);
+			}
 		}
 		$this->db->sql_freeresult($result);
 
diff --git a/phpBB/phpbb/notification/type/post.php b/phpBB/phpbb/notification/type/post.php
index f973becc3b..93fbcbde22 100644
--- a/phpBB/phpbb/notification/type/post.php
+++ b/phpBB/phpbb/notification/type/post.php
@@ -152,10 +152,14 @@ class post extends \phpbb\notification\type\base
 			unset($notify_users[$row['user_id']]);
 
 			$notification = $this->notification_manager->get_item_type_class($this->get_type(), $row);
-			$sql = 'UPDATE ' . $this->notifications_table . '
-				SET ' . $this->db->sql_build_array('UPDATE', $notification->add_responders($post)) . '
-				WHERE notification_id = ' . $row['notification_id'];
-			$this->db->sql_query($sql);
+			$update_responders = $notification->add_responders($post);
+			if (!empty($update_responders))
+			{
+				$sql = 'UPDATE ' . $this->notifications_table . '
+					SET ' . $this->db->sql_build_array('UPDATE', $update_responders) . '
+					WHERE notification_id = ' . $row['notification_id'];
+				$this->db->sql_query($sql);
+			}
 		}
 		$this->db->sql_freeresult($result);
 
@@ -206,7 +210,11 @@ class post extends \phpbb\notification\type\base
 			}
 		}
 
-		if ($trimmed_responders_cnt)
+		if ($trimmed_responders_cnt > 20)
+		{
+			$usernames[] = $this->user->lang('NOTIFICATION_MANY_OTHERS');
+		}
+		else if ($trimmed_responders_cnt)
 		{
 			$usernames[] = $this->user->lang('NOTIFICATION_X_OTHERS', $trimmed_responders_cnt);
 		}
@@ -270,6 +278,14 @@ class post extends \phpbb\notification\type\base
 	}
 
 	/**
+	* {inheritDoc}
+	*/
+	public function get_redirect_url()
+	{
+		return append_sid($this->phpbb_root_path . 'viewtopic.' . $this->php_ext, "t={$this->item_parent_id}&amp;view=unread#unread");
+	}
+
+	/**
 	* Users needed to query before this notification can be displayed
 	*
 	* @return array Array of user_ids
@@ -384,19 +400,27 @@ class post extends \phpbb\notification\type\base
 		// Do not add them as a responder if they were the original poster that created the notification
 		if ($this->get_data('poster_id') == $post['poster_id'])
 		{
-			return array('notification_data' => serialize($this->get_data(false)));
+			return array();
 		}
 
 		$responders = $this->get_data('responders');
 
 		$responders = ($responders === null) ? array() : $responders;
 
+		// Do not add more than 25 responders,
+		// we trim the username list to "a, b, c and x others" anyway
+		// so there is no use to add all of them anyway.
+		if (sizeof($responders) > 25)
+		{
+			return array();
+		}
+
 		foreach ($responders as $responder)
 		{
 			// Do not add them as a responder multiple times
 			if ($responder['poster_id'] == $post['poster_id'])
 			{
-				return array('notification_data' => serialize($this->get_data(false)));
+				return array();
 			}
 		}
 
@@ -407,6 +431,15 @@ class post extends \phpbb\notification\type\base
 
 		$this->set_data('responders', $responders);
 
-		return array('notification_data' => serialize($this->get_data(false)));
+		$serialized_data = serialize($this->get_data(false));
+
+		// If the data is longer then 4000 characters, it would cause a SQL error.
+		// We don't add the username to the list if this is the case.
+		if (utf8_strlen($serialized_data) >= 4000)
+		{
+			return array();
+		}
+
+		return array('notification_data' => $serialized_data);
 	}
 }
diff --git a/phpBB/phpbb/notification/type/post_in_queue.php b/phpBB/phpbb/notification/type/post_in_queue.php
index db16763583..56dfcce588 100644
--- a/phpBB/phpbb/notification/type/post_in_queue.php
+++ b/phpBB/phpbb/notification/type/post_in_queue.php
@@ -119,6 +119,14 @@ class post_in_queue extends \phpbb\notification\type\post
 	}
 
 	/**
+	* {inheritDoc}
+	*/
+	public function get_redirect_url()
+	{
+		return parent::get_url();
+	}
+
+	/**
 	* Function for preparing the data for insertion in an SQL query
 	* (The service handles insertion)
 	*
diff --git a/phpBB/phpbb/notification/type/quote.php b/phpBB/phpbb/notification/type/quote.php
index 745430e114..f4b4d763eb 100644
--- a/phpBB/phpbb/notification/type/quote.php
+++ b/phpBB/phpbb/notification/type/quote.php
@@ -113,29 +113,6 @@ class quote extends \phpbb\notification\type\post
 
 		$notify_users = $this->check_user_notification_options($auth_read[$post['forum_id']]['f_read'], $options);
 
-		// Try to find the users who already have been notified about replies and have not read the topic since and just update their notifications
-		$update_notifications = array();
-		$sql = 'SELECT n.*
-			FROM ' . $this->notifications_table . ' n, ' . $this->notification_types_table . ' nt
-			WHERE n.notification_type_id = ' . (int) $this->notification_type_id . '
-				AND n.item_parent_id = ' . (int) self::get_item_parent_id($post) . '
-				AND n.notification_read = 0
-				AND nt.notification_type_id = n.notification_type_id
-				AND nt.notification_type_enabled = 1';
-		$result = $this->db->sql_query($sql);
-		while ($row = $this->db->sql_fetchrow($result))
-		{
-			// Do not create a new notification
-			unset($notify_users[$row['user_id']]);
-
-			$notification = $this->notification_manager->get_item_type_class($this->get_type(), $row);
-			$sql = 'UPDATE ' . $this->notifications_table . '
-				SET ' . $this->db->sql_build_array('UPDATE', $notification->add_responders($post)) . '
-				WHERE notification_id = ' . $row['notification_id'];
-			$this->db->sql_query($sql);
-		}
-		$this->db->sql_freeresult($result);
-
 		return $notify_users;
 	}
 
@@ -191,6 +168,14 @@ class quote extends \phpbb\notification\type\post
 	}
 
 	/**
+	* {inheritDoc}
+	*/
+	public function get_redirect_url()
+	{
+		return $this->get_url();
+	}
+
+	/**
 	* Get email template
 	*
 	* @return string|bool
diff --git a/phpBB/phpbb/notification/type/type_interface.php b/phpBB/phpbb/notification/type/type_interface.php
index e3e6898172..2f465aae2b 100644
--- a/phpBB/phpbb/notification/type/type_interface.php
+++ b/phpBB/phpbb/notification/type/type_interface.php
@@ -99,6 +99,13 @@ interface type_interface
 	public function get_url();
 
 	/**
+	* Get the url to redirect after the item has been marked as read
+	*
+	* @return string URL
+	*/
+	public function get_redirect_url();
+
+	/**
 	* URL to unsubscribe to this notification
 	*
 	* @param string|bool $method Method name to unsubscribe from (email|jabber|etc), False to unsubscribe from all notifications for this item