5 files changed, 295 insertions, 11 deletions
diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php
index 6a0aedf8c6..79fcb892ef 100644
--- a/phpBB/includes/functions_posting.php
+++ b/phpBB/includes/functions_posting.php
@@ -2272,8 +2272,10 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll, &$data, $u
 			case 'edit_first_post':
 			case 'edit':
 			case 'edit_last_post':
-				// @todo: Check whether these notification deletions are correct
-				$phpbb_notifications->delete_notifications('topic', $data['topic_id']);
+				if ($data['topic_visibility'] != ITEM_APPROVED)
+				{
+					$phpbb_notifications->delete_notifications('topic', $data['topic_id']);
+				}
 
 				$phpbb_notifications->delete_notifications(array(
 					'quote',
@@ -2297,8 +2299,10 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll, &$data, $u
 			case 'edit_first_post':
 			case 'edit':
 			case 'edit_last_post':
-				// @todo: Check whether these notification deletions are correct
-				$phpbb_notifications->delete_notifications('topic', $data['topic_id']);
+				if ($data['topic_visibility'] != ITEM_APPROVED)
+				{
+					$phpbb_notifications->delete_notifications('topic', $data['topic_id']);
+				}
 
 				$phpbb_notifications->delete_notifications(array(
 					'quote',
diff --git a/tests/functional/notification_test.php b/tests/functional/notification_test.php
index 9ae37842fe..18e8a4ecc0 100644
--- a/tests/functional/notification_test.php
+++ b/tests/functional/notification_test.php
@@ -59,20 +59,17 @@ class phpbb_functional_notification_test extends phpbb_functional_test_case
 		$this->create_user('notificationtestuser');
 		$this->add_user_group('NEWLY_REGISTERED', array('notificationtestuser'));
 		$this->login('notificationtestuser');
-		$crawler = self::request('GET', 'index.php');
-		$this->assertContains('notificationtestuser', $crawler->filter('#username_logged_in')->text());
 
 		// Post a new post that needs approval
 		$this->create_post(2, 1, 'Re: Welcome to phpBB3', 'This is a test [b]post[/b] posted by notificationtestuser.', array(), 'POST_STORED_MOD');
 		$crawler = self::request('GET', "viewtopic.php?t=1&sid={$this->sid}");
 		$this->assertNotContains('This is a test post posted by notificationtestuser.', $crawler->filter('html')->text());
 
-		// logout
-		$crawler = self::request('GET', 'ucp.php?sid=' . $this->sid . '&mode=logout');
-
-		// admin login
+		// Login as admin
+		$this->logout();
 		$this->login();
 		$this->add_lang('ucp');
+
 		$crawler = self::request('GET', 'ucp.php?i=ucp_notifications');
 
 		// At least one notification should exist
diff --git a/tests/notification/fixtures/submit_post_topic.xml b/tests/notification/fixtures/submit_post_topic.xml
new file mode 100644
index 0000000000..5e179d9b99
--- /dev/null
+++ b/tests/notification/fixtures/submit_post_topic.xml
@@ -0,0 +1,134 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dataset>
+	<table name="phpbb_forums_watch">
+		<column>forum_id</column>
+		<column>user_id</column>
+		<column>notify_status</column>
+		<row>
+			<value>1</value>
+			<value>6</value>
+			<value>0</value>
+		</row>
+		<row>
+			<value>1</value>
+			<value>7</value>
+			<value>0</value>
+		</row>
+		<row>
+			<value>1</value>
+			<value>8</value>
+			<value>0</value>
+		</row>
+	</table>
+	<table name="phpbb_notifications">
+		<column>notification_type_id</column>
+		<column>user_id</column>
+		<column>item_id</column>
+		<column>item_parent_id</column>
+		<column>notification_read</column>
+		<column>notification_data</column>
+		<row>
+			<value>1</value>
+			<value>8</value>
+			<value>1</value>
+			<value>1</value>
+			<value>0</value>
+			<value></value>
+		</row>
+	</table>
+	<table name="phpbb_notification_types">
+		<column>notification_type_id</column>
+		<column>notification_type_name</column>
+		<column>notification_type_enabled</column>
+		<row>
+			<value>1</value>
+			<value>topic</value>
+			<value>1</value>
+		</row>
+	</table>
+	<table name="phpbb_posts">
+		<column>post_id</column>
+		<column>topic_id</column>
+		<column>forum_id</column>
+		<column>post_text</column>
+		<row>
+			<value>1</value>
+			<value>1</value>
+			<value>1</value>
+			<value></value>
+		</row>
+	</table>
+	<table name="phpbb_topics">
+		<column>topic_id</column>
+		<column>forum_id</column>
+		<row>
+			<value>1</value>
+			<value>1</value>
+		</row>
+	</table>
+	<table name="phpbb_users">
+		<column>user_id</column>
+		<column>username_clean</column>
+		<column>user_permissions</column>
+		<column>user_sig</column>
+		<row>
+			<value>2</value>
+			<value>poster</value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>6</value>
+			<value>noauth</value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>7</value>
+			<value>default</value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>8</value>
+			<value>notified</value>
+			<value></value>
+			<value></value>
+		</row>
+	</table>
+	<table name="phpbb_user_notifications">
+		<column>item_type</column>
+		<column>item_id</column>
+		<column>user_id</column>
+		<column>method</column>
+		<column>notify</column>
+		<row>
+			<value>topic</value>
+			<value>0</value>
+			<value>2</value>
+			<value></value>
+			<value>1</value>
+		</row>
+		<row>
+			<value>topic</value>
+			<value>0</value>
+			<value>6</value>
+			<value></value>
+			<value>1</value>
+		</row>
+		<row>
+			<value>topic</value>
+			<value>0</value>
+			<value>7</value>
+			<value></value>
+			<value>1</value>
+		</row>
+		<row>
+			<value>topic</value>
+			<value>0</value>
+			<value>8</value>
+			<value></value>
+			<value>1</value>
+		</row>
+	</table>
+</dataset>
diff --git a/tests/notification/submit_post_base.php b/tests/notification/submit_post_base.php
index 10d676c687..fbfd034381 100644
--- a/tests/notification/submit_post_base.php
+++ b/tests/notification/submit_post_base.php
@@ -101,7 +101,7 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c
 		$user_loader = new \phpbb\user_loader($db, $phpbb_root_path, $phpEx, USERS_TABLE);
 
 		// Notification Types
-		$notification_types = array('quote', 'bookmark', 'post', 'post_in_queue', 'topic', 'approve_topic', 'approve_post');
+		$notification_types = array('quote', 'bookmark', 'post', 'post_in_queue', 'topic', 'topic_in_queue', 'approve_topic', 'approve_post');
 		$notification_types_array = array();
 		foreach ($notification_types as $type)
 		{
diff --git a/tests/notification/submit_post_type_topic_test.php b/tests/notification/submit_post_type_topic_test.php
new file mode 100644
index 0000000000..29f67c9c78
--- /dev/null
+++ b/tests/notification/submit_post_type_topic_test.php
@@ -0,0 +1,149 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2014 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+require_once dirname(__FILE__) . '/submit_post_base.php';
+
+class phpbb_notification_submit_post_type_topic_test extends phpbb_notification_submit_post_base
+{
+	protected $item_type = 'topic';
+
+	public function setUp()
+	{
+		parent::setUp();
+
+		global $auth, $phpbb_log;
+
+		// Add additional permissions
+		$auth->expects($this->any())
+			->method('acl_get_list')
+			->with($this->anything(),
+				$this->stringContains('_'),
+				$this->greaterThan(0))
+			->will($this->returnValueMap(array(
+				array(
+					array(6, 7, 8),
+					'f_read',
+					1,
+					array(
+						1 => array(
+							'f_read' => array(7, 8),
+						),
+					),
+				),
+			)));
+
+		$phpbb_log = $this->getMock('\phpbb\log\null');
+	}
+
+	/**
+	* submit_post() Notifications test
+	*
+	* submit_post() $mode = 'post'
+	* Notification item_type = 'topic'
+	*/
+	public function submit_post_data()
+	{
+		return array(
+			/**
+			* Normal post
+			*
+			* User => State description
+			*	2	=> Poster, should NOT receive a notification
+			*	6	=> Forum subscribed, but no-auth reading, should NOT receive a notification
+			*	7	=> Forum subscribed, should receive a notification
+			*	8	=> Forum subscribed, but already notified, should NOT receive a new notification
+			*/
+			array(
+				array(),
+				array(
+					array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1),
+				),
+				array(
+					array('user_id' => 7, 'item_id' => 2, 'item_parent_id' => 1),
+					array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1),
+					array('user_id' => 8, 'item_id' => 2, 'item_parent_id' => 1),
+				),
+			),
+
+			/**
+			* Unapproved post
+			*
+			* No new notifications
+			*/
+			array(
+				array('force_approved_state' => false),
+				array(
+					array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1),
+				),
+				array(
+					array('user_id' => 8, 'item_id' => 1, 'item_parent_id' => 1),
+				),
+			),
+		);
+	}
+
+	/**
+	 * @dataProvider submit_post_data
+	 */
+	public function test_submit_post($additional_post_data, $expected_before, $expected_after)
+	{
+		$sql = 'SELECT user_id, item_id, item_parent_id
+			FROM ' . NOTIFICATIONS_TABLE . ' n, ' . NOTIFICATION_TYPES_TABLE . " nt
+			WHERE nt.notification_type_name = '" . $this->item_type . "'
+				AND n.notification_type_id = nt.notification_type_id
+			ORDER BY user_id ASC, item_id ASC";
+		$result = $this->db->sql_query($sql);
+		$this->assertEquals($expected_before, $this->db->sql_fetchrowset($result));
+		$this->db->sql_freeresult($result);
+
+		$poll_data = array();
+		$post_data = array_merge($this->post_data, $additional_post_data);
+		submit_post('post', '', 'poster-name', POST_NORMAL, $poll_data, $post_data, false, false);
+
+		// Check whether the notifications got added successfully
+		$result = $this->db->sql_query($sql);
+		$this->assertEquals($expected_after, $this->db->sql_fetchrowset($result),
+			'Check whether the notifications got added successfully');
+		$this->db->sql_freeresult($result);
+
+		if (isset($additional_post_data['force_approved_state']) && $additional_post_data['force_approved_state'] === false)
+		{
+			return;
+		}
+
+		$reply_data = array_merge($this->post_data, array(
+			'topic_id'		=> 2,
+		));
+		$url = submit_post('reply', '', 'poster-name', POST_NORMAL, $poll_data, $reply_data, false, false);
+		$reply_id = 3;
+		$this->assertStringEndsWith('p' . $reply_id, $url, 'Post ID of reply is not ' . $reply_id);
+
+		// Check whether the notifications are still correct after a reply has been added
+		$result = $this->db->sql_query($sql);
+		$this->assertEquals($expected_after, $this->db->sql_fetchrowset($result),
+			'Check whether the notifications are still correct after a reply has been added');
+		$this->db->sql_freeresult($result);
+
+		$result = $this->db->sql_query(
+			'SELECT *
+			FROM ' . POSTS_TABLE . '
+			WHERE post_id = ' . $reply_id);
+		$reply_edit_data = array_merge($this->post_data, $this->db->sql_fetchrow($result), array(
+			'force_approved_state'	=> false,
+			'post_edit_reason'		=> 'PHPBB3-12370',
+		));
+		submit_post('edit', '', 'poster-name', POST_NORMAL, $poll_data, $reply_edit_data, false, false);
+
+		// Check whether the notifications are still correct after the reply has been edit
+		$result = $this->db->sql_query($sql);
+		$this->assertEquals($expected_after, $this->db->sql_fetchrowset($result),
+			'Check whether the notifications are still correct after the reply has been edit');
+		$this->db->sql_freeresult($result);
+	}
+}