[ticket/13713] Introduce mention notifications for groups

PHPBB3-13713

phpBB/config/default/container/services_text_formatter.yml

@@ -56,6 +56,8 @@ services:
         class: phpbb\textformatter\s9e\mention_helper
         arguments:
             - '@dbal.conn'
+            - '@auth'
+            - '@user'
             - '%core.root_path%'
             - '%core.php_ext%'
 

phpBB/phpbb/textformatter/s9e/mention_helper.php

@@ -1,15 +1,15 @@
 <?php
 /**
-*
-* This file is part of the phpBB Forum Software package.
-*
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
-*
-* For full copyright and license information, please see
-* the docs/CREDITS.txt file.
-*
-*/
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @license       GNU General Public License, version 2 (GPL-2.0)
+ *
+ * For full copyright and license information, please see
+ * the docs/CREDITS.txt file.
+ *
+ */
 
 namespace phpbb\textformatter\s9e;
 
@@ -18,35 +18,54 @@ use s9e\TextFormatter\Utils as TextFormatterUtils;
 class mention_helper
 {
 	/**
-	* @var \phpbb\db\driver\driver_interface
-	*/
+	 * @var \phpbb\db\driver\driver_interface
+	 */
 	protected $db;
 
 	/**
-	* @var string Base URL for a user profile link, uses {ID} as placeholder
-	*/
+	 * @var \phpbb\auth\auth
+	 */
+	protected $auth;
+
+	/**
+	 * @var \phpbb\user
+	 */
+	protected $user;
+
+	/**
+	 * @var string Base URL for a user profile link, uses {ID} as placeholder
+	 */
 	protected $user_profile_url;
 
 	/**
-	* @var string Base URL for a group profile link, uses {ID} as placeholder
-	*/
+	 * @var string Base URL for a group profile link, uses {ID} as placeholder
+	 */
 	protected $group_profile_url;
 
 	/**
-	* @var array Array of users' and groups' colours for each cached ID
-	*/
+	 * @var array Array of users' and groups' colours for each cached ID
+	 */
 	protected $cached_colours = [];
 
 	/**
-	* Constructor
-	*
-	* @param \phpbb\db\driver\driver_interface $db
-	* @param string $root_path
-	* @param string $php_ext
-	*/
-	public function __construct($db, $root_path, $php_ext)
+	 * @var array Array of group IDs allowed to be mentioned by current user
+	 */
+	protected $mentionable_groups = null;
+
+	/**
+	 * Constructor
+	 *
+	 * @param \phpbb\db\driver\driver_interface $db
+	 * @param \phpbb\auth\auth                  $auth
+	 * @param \phpbb\user                       $user
+	 * @param string                            $root_path
+	 * @param string                            $php_ext
+	 */
+	public function __construct($db, $auth, $user, $root_path, $php_ext)
 	{
 		$this->db = $db;
+		$this->auth = $auth;
+		$this->user = $user;
 		$this->user_profile_url = append_sid($root_path . 'memberlist.' . $php_ext, 'mode=viewprofile&u={ID}', false);
 		$this->group_profile_url = append_sid($root_path . 'memberlist.' . $php_ext, 'mode=group&g={ID}', false);
 	}
@@ -109,11 +128,11 @@ class mention_helper
 	}
 
 	/**
-	* Inject dynamic metadata into MENTION tags in given XML
-	*
-	* @param  string $xml Original XML
-	* @return string      Modified XML
-	*/
+	 * Inject dynamic metadata into MENTION tags in given XML
+	 *
+	 * @param  string $xml Original XML
+	 * @return string      Modified XML
+	 */
 	public function inject_metadata($xml)
 	{
 		$profile_urls = [
@@ -129,8 +148,7 @@ class mention_helper
 		return TextFormatterUtils::replaceAttributes(
 			$xml,
 			'MENTION',
-			function ($attributes) use ($profile_urls)
-			{
+			function ($attributes) use ($profile_urls) {
 				if (isset($attributes['type']) && isset($attributes['id']))
 				{
 					$type = $attributes['type'];
@@ -149,15 +167,91 @@ class mention_helper
 		);
 	}
 
+	/**
+	 * Get group IDs allowed to be mentioned by current user
+	 *
+	 * @return array
+	 */
+	protected function get_mentionable_groups()
+	{
+		if (is_array($this->mentionable_groups))
+		{
+			return $this->mentionable_groups;
+		}
+
+		$hidden_restriction = (!$this->auth->acl_gets('a_group', 'a_groupadd', 'a_groupdel')) ? ' AND (g.group_type <> ' . GROUP_HIDDEN . ' OR (ug.user_pending = 0 AND ug.user_id = ' . (int) $this->user->data['user_id'] . '))' : '';
+
+		$query = $this->db->sql_build_query('SELECT', [
+			'SELECT'    => 'g.group_id',
+			'FROM'      => [
+				GROUPS_TABLE => 'g',
+			],
+			'LEFT_JOIN' => [[
+				'FROM' => [
+					USER_GROUP_TABLE => 'ug',
+				],
+				'ON'   => 'g.group_id = ug.group_id',
+			]],
+			'WHERE'     => '(g.group_type <> ' . GROUP_SPECIAL . ' OR ' . $this->db->sql_in_set('g.group_name', ['ADMINISTRATORS', 'GLOBAL_MODERATORS']) . ')' . $hidden_restriction,
+		]);
+		$result = $this->db->sql_query($query);
+
+		$this->mentionable_groups = [];
+
+		while ($row = $this->db->sql_fetchrow($result))
+		{
+			$this->mentionable_groups[] = $row['group_id'];
+		}
+
+		$this->db->sql_freeresult($result);
+
+		return $this->mentionable_groups;
+	}
+
+	/**
+	 * Selects IDs of user members of a certain group
+	 *
+	 * @param array $user_ids Array of already selected user IDs
+	 * @param int   $group_id ID of the group to search members in
+	 */
+	protected function get_user_ids_for_group(&$user_ids, $group_id)
+	{
+		if (!in_array($group_id, $this->get_mentionable_groups()))
+		{
+			return;
+		}
+
+		$query = $this->db->sql_build_query('SELECT', [
+			'SELECT' => 'ug.user_id, ug.group_id',
+			'FROM'   => [
+				USER_GROUP_TABLE => 'ug',
+				GROUPS_TABLE     => 'g',
+			],
+			'WHERE'  => 'g.group_id = ug.group_id',
+		]);
+		// Cache results for 5 minutes
+		$result = $this->db->sql_query($query, 300);
+
+		while ($row = $this->db->sql_fetchrow($result))
+		{
+			if ($row['group_id'] == $group_id)
+			{
+				$user_ids[] = (int) $row['user_id'];
+			}
+		}
+
+		$this->db->sql_freeresult($result);
+	}
+
 	/**
 	 * Get a list of mentioned names
-	 * TODO: decide what to do with groups
 	 *
 	 * @param string $xml  Parsed text
-	 * @param string $type Name type ('u' for users, 'g' for groups)
+	 * @param string $type Name type ('u' for users, 'g' for groups,
+	 *                     'ug' for usernames mentioned separately or as group members)
 	 * @return int[]       List of IDs
 	 */
-	public function get_mentioned_ids($xml, $type = 'u')
+	public function get_mentioned_ids($xml, $type = 'ug')
 	{
 		$ids = array();
 		if (strpos($xml, '<MENTION ') === false)
@@ -168,12 +262,31 @@ class mention_helper
 		$dom = new \DOMDocument;
 		$dom->loadXML($xml);
 		$xpath = new \DOMXPath($dom);
-		/** @var \DOMElement $mention */
-		foreach ($xpath->query('//MENTION') as $mention)
+
+		if ($type === 'ug')
 		{
-			if ($mention->getAttribute('type') === $type)
+			/** @var \DOMElement $mention */
+			foreach ($xpath->query('//MENTION') as $mention)
 			{
-				$ids[] = (int) $mention->getAttribute('id');
+				if ($mention->getAttribute('type') === 'u')
+				{
+					$ids[] = (int) $mention->getAttribute('id');
+				}
+				else if ($mention->getAttribute('type') === 'g')
+				{
+					$this->get_user_ids_for_group($ids, (int) $mention->getAttribute('id'));
+				}
+			}
+		}
+		else
+		{
+			/** @var \DOMElement $mention */
+			foreach ($xpath->query('//MENTION') as $mention)
+			{
+				if ($mention->getAttribute('type') === $type)
+				{
+					$ids[] = (int) $mention->getAttribute('id');
+				}
 			}
 		}
 

tests/notification/base.php

@@ -111,6 +111,8 @@ abstract class phpbb_tests_notification_base extends phpbb_database_test_case
 			'text_formatter.s9e.mention_helper',
 			new \phpbb\textformatter\s9e\mention_helper(
 				$this->db,
+				$auth,
+				$this->user,
 				$phpbb_root_path,
 				$phpEx
 			)

tests/notification/fixtures/submit_post_notification.type.mention.xml

@@ -1,5 +1,23 @@
 <?xml version="1.0" encoding="UTF-8" ?>
 <dataset>
+    <table name="phpbb_groups">
+        <column>group_id</column>
+        <column>group_name</column>
+        <column>group_type</column>
+        <column>group_desc</column>
+        <row>
+            <value>1</value>
+            <value>Normal group</value>
+            <value>0</value>
+            <value></value>
+        </row>
+        <row>
+            <value>2</value>
+            <value>Hidden group</value>
+            <value>2</value>
+            <value></value>
+        </row>
+    </table>
 	<table name="phpbb_notifications">
 		<column>notification_id</column>
 		<column>notification_type_id</column>
@@ -89,6 +107,33 @@
 			<value></value>
 			<value></value>
 		</row>
+		<row>
+			<value>8</value>
+			<value>member of normal group</value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>9</value>
+			<value>member of hidden group</value>
+			<value></value>
+			<value></value>
+		</row>
+	</table>
+	<table name="phpbb_user_group">
+		<column>user_id</column>
+		<column>group_id</column>
+		<column>user_pending</column>
+		<row>
+			<value>8</value>
+			<value>1</value>
+			<value>0</value>
+		</row>
+		<row>
+			<value>9</value>
+			<value>2</value>
+			<value>0</value>
+		</row>
 	</table>
 	<table name="phpbb_user_notifications">
 		<column>item_type</column>
@@ -131,5 +176,12 @@
 			<value>notification.method.board</value>
 			<value>0</value>
 		</row>
+		<row>
+			<value>notification.type.mention</value>
+			<value>0</value>
+			<value>8</value>
+			<value>notification.method.board</value>
+			<value>1</value>
+		</row>
 	</table>
 </dataset>

tests/notification/submit_post_base.php

@@ -139,6 +139,8 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c
 			'text_formatter.s9e.mention_helper',
 			new \phpbb\textformatter\s9e\mention_helper(
 				$this->db,
+				$auth,
+				$this->user,
 				$phpbb_root_path,
 				$phpEx
 			)

tests/notification/submit_post_type_mention_test.php

@@ -31,16 +31,20 @@ class phpbb_notification_submit_post_type_mention_test extends phpbb_notificatio
 				$this->greaterThan(0))
 			->will($this->returnValueMap(array(
 				array(
-					array(3, 4, 5, 6, 7, 8),
+					array(3, 4, 5, 6, 7, 8, 10),
 					'f_read',
 					1,
 					array(
 						1 => array(
-							'f_read' => array(3, 5, 6, 7),
+							'f_read' => array(3, 5, 6, 7, 8),
 						),
 					),
 				),
 			)));
+		$auth->expects($this->any())
+			->method('acl_gets')
+			->with('a_group', 'a_groupadd', 'a_groupdel')
+			->will($this->returnValue(false));
 	}
 
 	/**
@@ -65,6 +69,7 @@ class phpbb_notification_submit_post_type_mention_test extends phpbb_notificatio
 			*	5	=> mentioned, but already notified, should STILL receive a new notification
 			*	6	=> mentioned, but option disabled, should NOT receive a notification
 			*	7	=> mentioned, option set to default, should receive a notification
+			*	8	=> mentioned as a member of group 1, should receive a notification
 			*/
 			array(
 				array(
@@ -75,7 +80,9 @@ class phpbb_notification_submit_post_type_mention_test extends phpbb_notificatio
 						'[mention=u:5]notified[/mention] already notified, should not receive a new notification',
 						'[mention=u:6]disabled[/mention] option disabled, should not receive a notification',
 						'[mention=u:7]default[/mention] option set to default, should receive a notification',
-						'[mention=u:8]doesn\'t exist[/mention] user does not exist, should not receive a notification',
+						'[mention=g:1]normal group[/mention] group members of a normal group shoud receive a notification',
+						'[mention=g:2]hidden group[/mention] group members of a hidden group shoud not receive a notification from a non-member',
+						'[mention=u:10]doesn\'t exist[/mention] user does not exist, should not receive a notification',
 					))),
 					'bbcode_uid'		=> 'uid',
 				),
@@ -87,6 +94,7 @@ class phpbb_notification_submit_post_type_mention_test extends phpbb_notificatio
 					array('user_id' => 5, 'item_id' => 1, 'item_parent_id' => 1),
 					array('user_id' => 5, 'item_id' => 2, 'item_parent_id' => 1),
 					array('user_id' => 7, 'item_id' => 2, 'item_parent_id' => 1),
+					array('user_id' => 8, 'item_id' => 2, 'item_parent_id' => 1),
 				),
 			),