Merge pull request #3586 from s9e/ticket/13680

[ticket/13680] Updated quote notifications

phpBB/config/default/container/services_notification.yml

@@ -221,6 +221,8 @@ services:
             - %tables.notification_types%
             - %tables.notifications%
             - %tables.user_notifications%
+        calls:
+            - [set_utils, [@text_formatter.utils]]
         tags:
             - { name: notification.type }
 

phpBB/phpbb/notification/type/quote.php

@@ -20,6 +20,11 @@ namespace phpbb\notification\type;
 
 class quote extends \phpbb\notification\type\post
 {
+	/**
+	* @var \phpbb\textformatter\utils_interface
+	*/
+	protected $utils;
+
 	/**
 	* Get notification type name
 	*
@@ -30,13 +35,6 @@ class quote extends \phpbb\notification\type\post
 		return 'notification.type.quote';
 	}
 
-	/**
-	* regular expression to match to find usernames
-	*
-	* @var string
-	*/
-	protected static $regular_expression_match = '#\[quote="(.+?)"#';
-
 	/**
 	* Language key used to output the text
 	*
@@ -77,17 +75,16 @@ class quote extends \phpbb\notification\type\post
 			'ignore_users'		=> array(),
 		), $options);
 
-		$usernames = false;
-		preg_match_all(self::$regular_expression_match, $post['post_text'], $usernames);
+		$usernames = $this->utils->get_outermost_quote_authors($post['post_text']);
 
-		if (empty($usernames[1]))
+		if (empty($usernames))
 		{
 			return array();
 		}
 
-		$usernames[1] = array_unique($usernames[1]);
+		$usernames = array_unique($usernames);
 
-		$usernames = array_map('utf8_clean_string', $usernames[1]);
+		$usernames = array_map('utf8_clean_string', $usernames);
 
 		$users = array();
 
@@ -187,4 +184,14 @@ class quote extends \phpbb\notification\type\post
 			'AUTHOR_NAME'		=> htmlspecialchars_decode($user_data['username']),
 		));
 	}
+
+	/**
+	* Set the utils service used to retrieve quote authors
+	*
+	* @param \phpbb\textformatter\utils_interface $utils
+	*/
+	public function set_utils(\phpbb\textformatter\utils_interface $utils)
+	{
+		$this->utils = $utils;
+	}
 }

phpBB/phpbb/textformatter/s9e/utils.php

@@ -34,6 +34,31 @@ class utils implements \phpbb\textformatter\utils_interface
 		return \s9e\TextFormatter\Utils::removeFormatting($xml);
 	}
 
+	/**
+	* Get a list of quote authors, limited to the outermost quotes
+	*
+	* @param  string   $xml Parsed text
+	* @return string[]      List of authors
+	*/
+	public function get_outermost_quote_authors($xml)
+	{
+		$authors = array();
+		if (strpos($xml, '<QUOTE ') === false)
+		{
+			return $authors;
+		}
+
+		$dom = new \DOMDocument;
+		$dom->loadXML($xml);
+		$xpath = new \DOMXPath($dom);
+		foreach ($xpath->query('//QUOTE[not(ancestor::QUOTE)]/@author') as $author)
+		{
+			$authors[] = $author->textContent;
+		}
+
+		return $authors;
+	}
+
 	/**
 	* Remove given BBCode and its content, at given nesting depth
 	*

phpBB/phpbb/textformatter/utils_interface.php

@@ -28,6 +28,14 @@ interface utils_interface
 	*/
 	public function clean_formatting($text);
 
+	/**
+	* Get a list of quote authors, limited to the outermost quotes
+	*
+	* @param  string   $text Parsed text
+	* @return string[]       List of authors
+	*/
+	public function get_outermost_quote_authors($text);
+
 	/**
 	* Remove given BBCode and its content, at given nesting depth
 	*

tests/notification/base.php

@@ -116,7 +116,14 @@ abstract class phpbb_tests_notification_base extends phpbb_database_test_case
 	{
 		global $phpbb_root_path, $phpEx;
 
-		return new $type($this->user_loader, $this->db, $this->cache->get_driver(), $this->user, $this->auth, $this->config, $phpbb_root_path, $phpEx, 'phpbb_notification_types', 'phpbb_notifications', 'phpbb_user_notifications');
+		$instance = new $type($this->user_loader, $this->db, $this->cache->get_driver(), $this->user, $this->auth, $this->config, $phpbb_root_path, $phpEx, 'phpbb_notification_types', 'phpbb_notifications', 'phpbb_user_notifications');
+
+		if ($type === 'phpbb\\notification\\type\\quote')
+		{
+			$instance->set_utils(new \phpbb\textformatter\s9e\utils);
+		}
+
+		return $instance;
 	}
 
 	protected function assert_notifications($expected, $options = array())

tests/notification/manager_helper.php

@@ -48,6 +48,11 @@ class phpbb_notification_manager_helper extends \phpbb\notification\manager
 
 		$item = new $item_type($this->user_loader, $this->db, $this->cache->get_driver(), $this->user, $this->auth, $this->config, $this->phpbb_root_path, $this->php_ext, $this->notification_types_table, $this->notifications_table, $this->user_notifications_table);
 
+		if ($item_type === 'phpbb\\notification\\type\\quote')
+		{
+			$item->set_utils(new \phpbb\textformatter\s9e\utils);
+		}
+
 		$item->set_notification_manager($this);
 
 		$item->set_initial_data($data);

tests/notification/submit_post_base.php

@@ -117,6 +117,11 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c
 				$phpbb_root_path, $phpEx,
 				NOTIFICATION_TYPES_TABLE, NOTIFICATIONS_TABLE, USER_NOTIFICATIONS_TABLE);
 
+			if ($type === 'quote')
+			{
+				$class->set_utils(new \phpbb\textformatter\s9e\utils);
+			}
+
 			$phpbb_container->set('notification.type.' . $type, $class);
 
 			$notification_types_array['notification.type.' . $type] = $class;

tests/notification/submit_post_type_quote_test.php

@@ -51,6 +51,8 @@ class phpbb_notification_submit_post_type_quote_test extends phpbb_notification_
 	*/
 	public function submit_post_data()
 	{
+		$parser = $this->get_test_case_helpers()->set_s9e_services()->get('text_formatter.parser');
+
 		return array(
 			/**
 			* Normal post
@@ -65,15 +67,15 @@ class phpbb_notification_submit_post_type_quote_test extends phpbb_notification_
 			*/
 			array(
 				array(
-					'message'			=> implode(' ', array(
-						'[quote="poster":uid]poster should not be notified[/quote:uid]',
-						'[quote="test":uid]test should be notified[/quote:uid]',
-						'[quote="unauthorized":uid]unauthorized to read, should not receive a notification[/quote:uid]',
-						'[quote="notified":uid]already notified, should not receive a new notification[/quote:uid]',
-						'[quote="disabled":uid]option disabled, should not receive a notification[/quote:uid]',
-						'[quote="default":uid]option set to default, should receive a notification[/quote:uid]',
-						'[quote="doesn\'t exist":uid]user does not exist, should not receive a notification[/quote:uid]',
-					)),
+					'message'			=> $parser->parse(implode(' ', array(
+						'[quote="poster"]poster should not be notified[/quote]',
+						'[quote="test"]test should be notified[/quote]',
+						'[quote="unauthorized"]unauthorized to read, should not receive a notification[/quote]',
+						'[quote="notified"]already notified, should not receive a new notification[/quote]',
+						'[quote="disabled"]option disabled, should not receive a notification[/quote]',
+						'[quote="default"]option set to default, should receive a notification[/quote]',
+						'[quote="doesn\'t exist"]user does not exist, should not receive a notification[/quote]',
+					))),
 					'bbcode_uid'		=> 'uid',
 				),
 				array(
@@ -94,15 +96,15 @@ class phpbb_notification_submit_post_type_quote_test extends phpbb_notification_
 			*/
 			array(
 				array(
-					'message'			=> implode(' ', array(
-						'[quote="poster":uid]poster should not be notified[/quote:uid]',
-						'[quote="test":uid]test should be notified[/quote:uid]',
-						'[quote="unauthorized":uid]unauthorized to read, should not receive a notification[/quote:uid]',
-						'[quote="notified":uid]already notified, should not receive a new notification[/quote:uid]',
-						'[quote="disabled":uid]option disabled, should not receive a notification[/quote:uid]',
-						'[quote="default":uid]option set to default, should receive a notification[/quote:uid]',
-						'[quote="doesn\'t exist":uid]user does not exist, should not receive a notification[/quote:uid]',
-					)),
+					'message'			=> $parser->parse(implode(' ', array(
+						'[quote="poster"]poster should not be notified[/quote]',
+						'[quote="test"]test should be notified[/quote]',
+						'[quote="unauthorized"]unauthorized to read, should not receive a notification[/quote]',
+						'[quote="notified"]already notified, should not receive a new notification[/quote]',
+						'[quote="disabled"]option disabled, should not receive a notification[/quote]',
+						'[quote="default"]option set to default, should receive a notification[/quote]',
+						'[quote="doesn\'t exist"]user does not exist, should not receive a notification[/quote]',
+					))),
 					'bbcode_uid'		=> 'uid',
 					'force_approved_state' => false,
 				),

tests/text_formatter/s9e/utils_test.php

@@ -74,6 +74,40 @@ class phpbb_textformatter_s9e_utils_test extends phpbb_test_case
 		);
 	}
 
+	/**
+	* @dataProvider get_outermost_quote_authors_tests
+	*/
+	public function test_get_outermost_quote_authors($original, $expected)
+	{
+		$container = $this->get_test_case_helpers()->set_s9e_services();
+		$utils     = $container->get('text_formatter.utils');
+		$parser    = $container->get('text_formatter.parser');
+
+		$this->assertSame($expected, $utils->get_outermost_quote_authors($parser->parse($original)));
+	}
+
+	public function get_outermost_quote_authors_tests()
+	{
+		return array(
+			array(
+				'No quotes here',
+				array()
+			),
+			array(
+				'[quote="foo"]..[/quote] [quote]..[/quote]',
+				array('foo')
+			),
+			array(
+				'[quote="foo"]..[/quote] [quote="bar"]..[/quote]',
+				array('foo', 'bar')
+			),
+			array(
+				'[quote="foo"].[quote="baz"]..[/quote].[/quote] [quote="bar"]..[/quote]',
+				array('foo', 'bar')
+			),
+		);
+	}
+
 	/**
 	* @dataProvider get_remove_bbcode_tests
 	*/