Merge pull request #6794 from rxu/ticket/17489

[ticket/17489] Fix various messenger issues

phpBB/includes/acp/acp_email.php

@@ -227,6 +227,10 @@ class acp_email
 					$used_lang = $email_list[$i][0]['lang'];
 					$used_method = $email_list[$i][0]['method'];
 
+					/**
+					 * @var \phpbb\messenger\method\messenger_interface $messenger_method
+					 * @psalm-suppress UndefinedMethod
+					 */
 					foreach ($messenger_collection_iterator as $messenger_method)
 					{
 						$notify_method = $messenger_method->get_id();
@@ -265,19 +269,12 @@ class acp_email
 							}
 
 							$errored = !$messenger_method->send() || $errored;
+							$messenger_method->save_queue();
 						}
 					}
 				}
 				unset($email_list);
 
-				if ($use_queue)
-				{
-					foreach ($messenger_collection_iterator as $messenger_method)
-					{
-						$messenger_method->save_queue();
-					}
-				}
-
 				if ($generate_log_entry)
 				{
 					if (!empty($usernames))

phpBB/includes/acp/acp_inactive.php

@@ -205,6 +205,10 @@ class acp_inactive
 
 						do
 						{
+							/**
+							 * @var \phpbb\messenger\method\messenger_interface $messenger_method
+							 * @psalm-suppress UndefinedMethod
+							 */
 							foreach ($messenger_collection_iterator as $messenger_method)
 							{
 								if ($messenger_method->get_id() == $user_row['user_notify_type'] || $user_row['user_notify_type'] == $messenger_method::NOTIFY_BOTH)
@@ -219,6 +223,7 @@ class acp_inactive
 									]);
 
 									$messenger_method->send();
+									$messenger_method->save_queue();
 								}
 							}
 
@@ -227,11 +232,6 @@ class acp_inactive
 						}
 						while ($row = $db->sql_fetchrow($result));
 
-						foreach ($messenger_collection_iterator as $messenger_method)
-						{
-							$messenger_method->save_queue();
-						}
-
 						// Add the remind state to the database and increase activation expiration by one day
 						$sql = 'UPDATE ' . USERS_TABLE . '
 							SET user_reminded = user_reminded + 1,

phpBB/includes/ucp/ucp_activate.php

@@ -133,6 +133,11 @@ class ucp_activate
 
 			$messenger = $phpbb_container->get('messenger.method_collection');
 			$messenger_collection_iterator = $messenger->getIterator();
+
+			/**
+			 * @var \phpbb\messenger\method\messenger_interface $messenger_method
+			 * @psalm-suppress UndefinedMethod
+			 */
 			foreach ($messenger_collection_iterator as $messenger_method)
 			{
 				if ($messenger_method->get_id() == $user_row['user_notify_type'] || $user_row['user_notify_type'] == $messenger_method::NOTIFY_BOTH)

phpBB/includes/ucp/ucp_resend.php

@@ -138,6 +138,10 @@ class ucp_resend
 				$messenger_collection_iterator = $messenger->getIterator();
 				while ($row = $db->sql_fetchrow($result))
 				{
+					/**
+					 * @var \phpbb\messenger\method\messenger_interface $messenger_method
+					 * @psalm-suppress UndefinedMethod
+					 */
 					foreach ($messenger_collection_iterator as $messenger_method)
 					{
 						$messenger_method->set_use_queue(false);
@@ -157,12 +161,6 @@ class ucp_resend
 					}
 				}
 				$db->sql_freeresult($result);
-
-				// Save the queue in the messenger method class (has to be called or these messages could be lost)
-				foreach ($messenger_collection_iterator as $messenger_method)
-				{
-					$messenger_method->save_queue();
-				}
 			}
 
 			$this->update_activation_expiration();

phpBB/phpbb/message/message.php

@@ -249,6 +249,11 @@ class message
 		{
 			/** @psalm-suppress InvalidTemplateParam */
 			$messenger_collection_iterator = $messenger->getIterator();
+
+			/**
+			 * @var messenger_interface $messenger_method
+			 * @psalm-suppress UndefinedMethod
+			 */
 			foreach ($messenger_collection_iterator as $messenger_method)
 			{
 				$messenger_method->set_use_queue(false);
@@ -264,6 +269,7 @@ class message
 					{
 						$messenger_method->header('X-AntiAbuse', 'User_id - ' . $this->sender_id);
 					}
+
 					if ($this->sender_username)
 					{
 						$messenger_method->header('X-AntiAbuse', 'Username - ' . $this->sender_username);

phpBB/phpbb/messenger/method/base.php

@@ -481,4 +481,12 @@ abstract class base implements messenger_interface
 		$this->setup_template();
 		$this->template->set_custom_style($path_name, $paths);
 	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function header(string $header_name, mixed $header_value): void
+	{
+		return;
+	}
 }

phpBB/phpbb/messenger/method/email.php

@@ -105,7 +105,7 @@ class email extends base
 		$this->mail_priority = symfony_email::PRIORITY_NORMAL;
 
 		$this->additional_headers = [];
-		$this->use_queue = true;
+		$this->set_use_queue();
 		unset($this->template, $this->reply_to, $this->from);
 	}
 
@@ -242,10 +242,10 @@ class email extends base
 	 */
 	public function anti_abuse_headers(\phpbb\config\config $config, \phpbb\user $user): void
 	{
-		$this->headers->addHeader('X-AntiAbuse', 'Board servername - ' . $config['server_name']);
+		$this->header('X-AntiAbuse', 'Board servername - ' . $config['server_name']);
-		$this->headers->addHeader('X-AntiAbuse', 'User_id - ' . $user->data['user_id']);
+		$this->header('X-AntiAbuse', 'User_id - ' . $user->data['user_id']);
-		$this->headers->addHeader('X-AntiAbuse', 'Username - ' . $user->data['username']);
+		$this->header('X-AntiAbuse', 'Username - ' . $user->data['username']);
-		$this->headers->addHeader('X-AntiAbuse', 'User IP - ' . $user->ip);
+		$this->header('X-AntiAbuse', 'User IP - ' . $user->ip);
 	}
 
 	/**
@@ -313,7 +313,7 @@ class email extends base
 
 		foreach ($headers as $header => $value)
 		{
-			$this->headers->addHeader($header, $value);
+			$this->header($header, $value);
 		}
 
 	}
@@ -576,4 +576,12 @@ class email extends base
 
 		return true;
 	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function header(string $header_name, mixed $header_value): void
+	{
+		$this->headers->addHeader($header_name, $header_value);
+	}
 }

phpBB/phpbb/messenger/method/messenger_interface.php

@@ -64,5 +64,16 @@ interface messenger_interface
 	 *
 	 * @return void
 	 */
+
 	public function error(string $msg): void;
+
+	/**
+	 * Add message header
+	 *
+	 * @param string	$header_name	Message header name
+	 * @param mixed		$header_value	Message header value
+	 *
+	 * @return void
+	 */
+	public function header(string $header_name, mixed $header_value): void;
 }

phpBB/phpbb/messenger/queue.php

@@ -116,6 +116,11 @@ class queue
 
 		/** @psalm-suppress InvalidTemplateParam */
 		$messenger_collection_iterator = $this->messenger_method_collection->getIterator();
+
+		/**
+		 * @var \phpbb\messenger\method\messenger_interface $messenger_method
+		 * @psalm-suppress UndefinedMethod
+		 */
 		foreach ($messenger_collection_iterator as $messenger_method)
 		{
 			if (isset($this->queue_data[$messenger_method->get_queue_object_name()]))

phpBB/phpbb/notification/method/messenger_base.php

@@ -114,6 +114,10 @@ abstract class messenger_base extends \phpbb\notification\method\base
 				continue;
 			}
 
+			/**
+			 * @var \phpbb\messenger\method\messenger_interface $messenger_method
+			 * @psalm-suppress UndefinedMethod
+			 */
 			foreach ($messenger_collection_iterator as $messenger_method)
 			{
 				if ($messenger_method->get_id() == $notify_method || $notify_method == $messenger_method::NOTIFY_BOTH)
@@ -126,16 +130,13 @@ abstract class messenger_base extends \phpbb\notification\method\base
 					], $notification->get_email_template_variables()));
 
 					$messenger_method->send();
+
+					// Save the queue in the messenger method class (has to be called or these messages could be lost)
+					$messenger_method->save_queue();
 				}
 			}
 		}
 
-		// Save the queue in the messenger method class (has to be called or these messages could be lost)
-		foreach ($messenger_collection_iterator as $messenger_method)
-		{
-			$messenger_method->save_queue();
-		}
-
 		// We're done, empty the queue
 		$this->empty_queue();
 	}

tests/functional/memberlist_test.php

@@ -134,4 +134,29 @@ class phpbb_functional_memberlist_test extends phpbb_functional_test_case
 
 		unlink(__DIR__ . '/../../phpBB/images/ranks/valid.jpg');
 	}
+
+	public function test_email()
+	{
+		$this->login();
+		$this->admin_login();
+		$this->add_lang(['acp/board', 'acp/common', 'memberlist']);
+
+		$crawler = self::request('GET', "adm/index.php?sid={$this->sid}&i=acp_board&mode=email");
+		$form = $crawler->selectButton('Submit')->form([
+			'config[board_email_form]' => 1,
+		]);
+		$crawler = self::submit($form);
+		$this->assertContainsLang('CONFIG_UPDATED', $crawler->filter('.successbox')->text());
+
+		$crawler = self::request('GET', 'memberlist.php?mode=email&u=2');
+		$this->assertStringContainsString($this->lang('SEND_EMAIL_USER', 'admin'), $crawler->filter('.titlespace')->text());
+
+		$form = $crawler->selectButton($this->lang('SEND_EMAIL'))->form([
+			'subject' => 'Test email form message',
+			'message' => 'This is a test email message sent from a member profile email form.',
+		]);
+		$crawler = self::submit($form);
+
+		$this->assertContainsLang('EMAIL_SENT', $crawler->text());
+	}
 }