Merge branch 'main' into prep-1.0.1

Author: iMattPro

README.md

@@ -33,24 +33,24 @@ To run this extension from the repo (and not from a pre-built package) on a loca
 
 ## Testing Push Notifications
 
-Testing push notifications necessitates user-to-user interactions to observe the notification behavior accurately. Follow the steps outlined below to effectively test push notifications:
+Testing push notifications necessitates user-to-user interactions to observe the notification behavior accurately. Follow the steps outlined below to effectively test push notifications (the browser recommendations are what we have seen work in local environments):
 
 1. **User Account Setup:**
-	- Create at least two distinct user accounts for testing purposes.
-    - In the notifications preferences for _**User Account 1**_, subscribe to receive Push Notifications (and assign web push notification types if necessary).
+	- Create at least two distinct board user accounts for testing purposes.
+    - Using Google Chrome, visit `UCP -> Board Preferences -> Edit notification options` for _**User Account 1**_ and enable Push Notifications (and enable all web push notification types if necessary). Your browser may ask you to allow notifications, which you should accept. Leave Chrome open and running the background.
 
 2. **Message, Quote, or Reply Interaction:**
-	- Initiate a user-to-user interaction by performing one of the following actions using _**User Account 2**_:
+	- Initiate a user-to-user interaction by performing one of the following actions using _**User Account 2**_ in separate browser such as Firefox, Edge or Safari:
 		- **Private Message:** Send a direct message from _**User Account 2**_ to _**User Account 1**_.
 		- **Quote:** Quote a post or message authored by _**User Account 1**_ using _**User Account 2**_.
 		- **Reply:** Respond to a post or message authored by _**User Account 1**_ using _**User Account 2**_.
 
 3. **Observing Push Notifications:**
-	- Once the interaction is performed from _**User Account 2**_ to engage with _**User Account 1**_, you promptly should see a notification for _**User Account 1**_.
+	- Once the interaction is performed from _**User Account 2**_ to engage with _**User Account 1**_, you promptly should see a notification from Google Chrome for _**User Account 1**_.
 
 4. **Caveats for Local Testing**
-    - Local testing of Push Notifications only works from a `localhost` address or if your local server has a secure SSL certificate.
-    - We have seen success on Windows using manually installed PHP, Apache and MySQL. However, for reasons not yet known we do not see success on Mac using MAMP.
+    - Local testing of Push Notifications only works from a `http://localhost` address or if your local test server has a secure SSL certificate, e.g.: `https://local.phpbb.board`.
+    - Depending on your local server's setup, operating system, and browsers, it is still possible that testing push notifications may not work (for example, in a local environment running on macOS, only Chrome will show notifications).
 
 ## License
 

config/services.yml

@@ -30,7 +30,6 @@ services:
         arguments:
             - '@config'
             - '@dbal.conn'
-            - '@language'
             - '@log'
             - '@user_loader'
             - '@user'
@@ -49,8 +48,11 @@ services:
             - '@controller.helper'
             - '@dbal.conn'
             - '@phpbb.wpn.form_helper'
+            - '@language'
+            - '@notification_manager'
             - '@path_helper'
             - '@request'
+            - '@user_loader'
             - '@user'
             - '@template.twig.environment'
             - '%tables.phpbb.wpn.notification_push%'

notification/method/webpush.php

@@ -14,7 +14,6 @@
 use phpbb\config\config;
 use phpbb\controller\helper;
 use phpbb\db\driver\driver_interface;
-use phpbb\language\language;
 use phpbb\log\log_interface;
 use phpbb\notification\method\messenger_base;
 use phpbb\notification\type\type_interface;
@@ -36,9 +35,6 @@ class webpush extends messenger_base implements extended_method_interface
 	/** @var driver_interface */
 	protected $db;
 
-	/** @var language */
-	protected $language;
-
 	/** @var log_interface */
 	protected $log;
 
@@ -65,7 +61,6 @@ class webpush extends messenger_base implements extended_method_interface
 	 *
 	 * @param config $config
 	 * @param driver_interface $db
-	 * @param language $language
 	 * @param log_interface $log
 	 * @param user_loader $user_loader
 	 * @param user $user
@@ -75,14 +70,13 @@ class webpush extends messenger_base implements extended_method_interface
 	 * @param string $notification_webpush_table
 	 * @param string $push_subscriptions_table
 	 */
-	public function __construct(config $config, driver_interface $db, language $language, log_interface $log, user_loader $user_loader, user $user, path_helper $path_helper,
+	public function __construct(config $config, driver_interface $db, log_interface $log, user_loader $user_loader, user $user, path_helper $path_helper,
 								string $phpbb_root_path, string $php_ext, string $notification_webpush_table, string $push_subscriptions_table)
 	{
 		parent::__construct($user_loader, $phpbb_root_path, $php_ext);
 
 		$this->config = $config;
 		$this->db = $db;
-		$this->language = $language;
 		$this->log = $log;
 		$this->user = $user;
 		$this->path_helper = $path_helper;
@@ -145,29 +139,15 @@ public function notify()
 	{
 		$insert_buffer = new \phpbb\db\sql_insert_buffer($this->db, $this->notification_webpush_table);
 
-		// Load all users data we want to notify
-		$notify_users = $this->load_recipients_data();
-
 		/** @var type_interface $notification */
 		foreach ($this->queue as $notification)
 		{
 			$data = $notification->get_insert_array();
-
-			// Change notification language if needed only
-			$recipient_data = $this->user_loader->get_user($notification->user_id);
-			if ($this->language->get_used_language() !== $recipient_data['user_lang'])
-			{
-				$this->language->set_user_language($recipient_data['user_lang'], true);
-			}
-
 			$data += [
-				'push_data'		=> json_encode([
-					'heading'	=> $this->config['sitename'],
-					'title'		=> strip_tags($notification->get_title()),
-					'text'		=> strip_tags($notification->get_reference()),
-					'url'		=> htmlspecialchars_decode($notification->get_url()),
-					'avatar'	=> $this->prepare_avatar($notification->get_avatar()),
-				]),
+				'push_data'				=> json_encode(array_merge(
+					$data,
+					['notification_type_name' => $notification->get_type()]
+				)),
 				'notification_time'		=> time(),
 				'push_token'			=> hash('sha256', random_bytes(32))
 			];
@@ -178,30 +158,41 @@ public function notify()
 
 		$insert_buffer->flush();
 
-		// Restore current user's language if needed only
-		if ($this->language->get_used_language() !== $this->user->data['user_lang'])
-		{
-			$this->language->set_user_language($this->user->data['user_lang'], true);
-		}
-
-		$this->notify_using_webpush($notify_users);
+		$this->notify_using_webpush();
 
 		return false;
 	}
 
 	/**
 	 * Notify using Web Push
 	 *
-	 * @param array	$notify_users	Array of user ids to notify
 	 * @return void
 	 */
-	protected function notify_using_webpush($notify_users = []): void
+	protected function notify_using_webpush(): void
 	{
 		if (empty($this->queue))
 		{
 			return;
 		}
 
+		// Load all users we want to notify
+		$user_ids = [];
+		foreach ($this->queue as $notification)
+		{
+			$user_ids[] = $notification->user_id;
+		}
+
+		// Do not send push notifications to banned users
+		if (!function_exists('phpbb_get_banned_user_ids'))
+		{
+			include($this->phpbb_root_path . 'includes/functions_user.' . $this->php_ext);
+		}
+		$banned_users = phpbb_get_banned_user_ids($user_ids);
+
+		// Load all the users we need
+		$notify_users = array_diff($user_ids, $banned_users);
+		$this->user_loader->load_users($notify_users, [USER_IGNORE]);
+
 		// Get subscriptions for users
 		$user_subscription_map = $this->get_user_subscription_map($notify_users);
 
@@ -361,6 +352,14 @@ public static function clean_data(array $data): array
 		return array_intersect_key($data, $row);
 	}
 
+	/**
+	 * Get template data for the UCP
+	 *
+	 * @param helper $controller_helper
+	 * @param form_helper $form_helper
+	 *
+	 * @return array
+	 */
 	public function get_ucp_template_data(helper $controller_helper, form_helper $form_helper): array
 	{
 		$subscription_map = $this->get_user_subscription_map([$this->user->id()]);
@@ -461,46 +460,6 @@ protected function clean_expired_subscriptions(array $user_subscription_map, arr
 		$this->remove_subscriptions($remove_subscriptions);
 	}
 
-	/**
-	 * Takes an avatar string (usually in full html format already) and extracts the url.
-	 * If the avatar url is a relative path, it's converted to an absolute path.
-	 *
-	 * Converts:
-	 *    <img class="avatar" src="./path/to/avatar=123456789.gif" width="123" height="123" alt="User avatar" />
-	 * or <img class="avatar" src="./styles/prosilver/theme/images/no_avatar.gif" data-src="./path/to/avatar=123456789.gif" width="123" height="123" alt="User avatar" />
-	 * into https://myboard.url/path/to/avatar=123456789.gif
-	 *
-	 * @param string $avatar
-	 * @return array 'src' => Absolute path to avatar image
-	 */
-	protected function prepare_avatar($avatar): array
-	{
-		$pattern = '/src=["\']?([^"\'>]+)["\']?/';
-
-		preg_match_all($pattern, $avatar, $matches);
-
-		$path = !empty($matches[1]) ? end($matches[1]) : $avatar;
-
-		return ['src' => preg_replace('#^' . preg_quote($this->path_helper->get_web_root_path(), '#') . '#', $this->get_board_url(), $path, 1)];
-	}
-
-	/**
-	 * Returns the board url (and caches it in the function)
-	 *
-	 * @return string the generated board url
-	 */
-	protected function get_board_url()
-	{
-		static $board_url;
-
-		if (empty($board_url))
-		{
-			$board_url = generate_board_url() . '/';
-		}
-
-		return $board_url;
-	}
-
 	/**
 	 * Set web push padding for endpoint
 	 *
@@ -523,31 +482,4 @@ protected function set_endpoint_padding(\Minishlink\WebPush\WebPush $web_push, s
 			}
 		}
 	}
-
-	/**
-	 * Load all users data to send notifications
-	 *
-	 * @return array	Array of user ids to notify
-	 */
-	protected function load_recipients_data(): array
-	{
-		$notify_users = $user_ids = [];
-		foreach ($this->queue as $notification)
-		{
-			$user_ids[] = $notification->user_id;
-		}
-
-		// Do not send push notifications to banned users
-		if (!function_exists('phpbb_get_banned_user_ids'))
-		{
-			include($this->phpbb_root_path . 'includes/functions_user.' . $this->php_ext);
-		}
-		$banned_users = phpbb_get_banned_user_ids($user_ids);
-
-		// Load all the users we need
-		$notify_users = array_diff($user_ids, $banned_users);
-		$this->user_loader->load_users($notify_users, [USER_IGNORE]);
-
-		return $notify_users;
-	}
 }

tests/event/listener_test.php

@@ -108,7 +108,6 @@ protected function setUp(): void
 		$this->notification_method_webpush = new \phpbb\webpushnotifications\notification\method\webpush(
 			$this->config,
 			$db,
-			$this->language,
 			new \phpbb\log\dummy(),
 			$user_loader,
 			$this->user,
@@ -233,6 +232,15 @@ public function test_load_template_data($user_id, $method_data, $subscriptions,
 	{
 		$this->config['wpn_webpush_dropdown_subscribe'] = true;
 		$this->user->data['user_id'] = $user_id;
+
+		$this->set_listener();
+
+		$method_data['method'] = $this->notification_method_webpush;
+
+		$this->notifications->expects(self::once())
+			->method('get_subscription_methods')
+			->willReturn([$method_data['id'] => $method_data]);
+
 		$this->template->expects($expected ? self::once() : self::never())
 			->method('assign_vars')
 			->with([
@@ -243,16 +251,7 @@ public function test_load_template_data($user_id, $method_data, $subscriptions,
 				'U_WEBPUSH_WORKER_URL'			=> $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_worker_controller'),
 				'SUBSCRIPTIONS'					=> $subscriptions,
 				'WEBPUSH_FORM_TOKENS'			=> $this->form_helper->get_form_tokens(\phpbb\webpushnotifications\ucp\controller\webpush::FORM_TOKEN_UCP),
-			]
-		);
-
-		$this->set_listener();
-
-		$method_data['method'] = $this->notification_method_webpush;
-
-		$this->notifications->expects(self::once())
-			->method('get_subscription_methods')
-			->willReturn([$method_data['id'] => $method_data]);
+			]);
 
 		$dispatcher = new \phpbb\event\dispatcher();
 		$dispatcher->addListener('core.page_header_after', [$this->listener, 'load_template_data']);

tests/notification/notification_method_webpush_test.php

@@ -171,7 +171,6 @@ protected function setUp(): void
 		$this->notification_method_webpush = new webpush(
 			$phpbb_container->get('config'),
 			$phpbb_container->get('dbal.conn'),
-			$phpbb_container->get('language'),
 			$phpbb_container->get('log'),
 			$phpbb_container->get('user_loader'),
 			$phpbb_container->get('user'),