Merge pull request phpbb#6794 from rxu/ticket/17489

marc1706 · web-flow · commit 6ecb41f01a98 · 2025-04-03T21:02:40.000+02:00
[ticket/17489] Fix various messenger issues
diff --git a/phpBB/includes/acp/acp_email.php b/phpBB/includes/acp/acp_email.php
@@ -227,6 +227,10 @@ function main($id, $mode)
 					$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 @@ function main($id, $mode)
 							}
 
 							$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))
diff --git a/phpBB/includes/acp/acp_inactive.php b/phpBB/includes/acp/acp_inactive.php
@@ -205,6 +205,10 @@ function main($id, $mode)
 
 						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 @@ function main($id, $mode)
 									]);
 
 									$messenger_method->send();
+									$messenger_method->save_queue();
 								}
 							}
 
@@ -227,11 +232,6 @@ function main($id, $mode)
 						}
 						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,
diff --git a/phpBB/includes/ucp/ucp_activate.php b/phpBB/includes/ucp/ucp_activate.php
@@ -133,6 +133,11 @@ function main($id, $mode)
 
 			$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)
diff --git a/phpBB/includes/ucp/ucp_resend.php b/phpBB/includes/ucp/ucp_resend.php
@@ -138,6 +138,10 @@ function main($id, $mode)
 				$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 @@ function main($id, $mode)
 					}
 				}
 				$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();
diff --git a/phpBB/phpbb/message/message.php b/phpBB/phpbb/message/message.php
@@ -249,6 +249,11 @@ public function send(\phpbb\di\service_collection $messenger, $contact)
 		{
 			/** @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 @@ public function send(\phpbb\di\service_collection $messenger, $contact)
 					{
 						$messenger_method->header('X-AntiAbuse', 'User_id - ' . $this->sender_id);
 					}
+
 					if ($this->sender_username)
 					{
 						$messenger_method->header('X-AntiAbuse', 'Username - ' . $this->sender_username);
diff --git a/phpBB/phpbb/messenger/method/base.php b/phpBB/phpbb/messenger/method/base.php
@@ -481,4 +481,12 @@ protected function set_template_paths(string|array $path_name, string|array $pat
 		$this->setup_template();
 		$this->template->set_custom_style($path_name, $paths);
 	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function header(string $header_name, mixed $header_value): void
+	{
+		return;
+	}
 }
diff --git a/phpBB/phpbb/messenger/method/email.php b/phpBB/phpbb/messenger/method/email.php
@@ -105,7 +105,7 @@ public function init(): void
 		$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 @@ public function subject(string $subject = ''): void
 	 */
 	public function anti_abuse_headers(\phpbb\config\config $config, \phpbb\user $user): void
 	{
-		$this->headers->addHeader('X-AntiAbuse', 'Board servername - ' . $config['server_name']);
-		$this->headers->addHeader('X-AntiAbuse', 'User_id - ' . $user->data['user_id']);
-		$this->headers->addHeader('X-AntiAbuse', 'Username - ' . $user->data['username']);
-		$this->headers->addHeader('X-AntiAbuse', 'User IP - ' . $user->ip);
+		$this->header('X-AntiAbuse', 'Board servername - ' . $config['server_name']);
+		$this->header('X-AntiAbuse', 'User_id - ' . $user->data['user_id']);
+		$this->header('X-AntiAbuse', 'Username - ' . $user->data['username']);
+		$this->header('X-AntiAbuse', 'User IP - ' . $user->ip);
 	}
 
 	/**
@@ -313,7 +313,7 @@ protected function build_headers(): void
 
 		foreach ($headers as $header => $value)
 		{
-			$this->headers->addHeader($header, $value);
+			$this->header($header, $value);
 		}
 
 	}
@@ -576,4 +576,12 @@ public function send(): bool
 
 		return true;
 	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function header(string $header_name, mixed $header_value): void
+	{
+		$this->headers->addHeader($header_name, $header_value);
+	}
 }
diff --git a/phpBB/phpbb/messenger/method/messenger_interface.php b/phpBB/phpbb/messenger/method/messenger_interface.php
@@ -64,5 +64,16 @@ public function send(): bool;
 	 *
 	 * @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;
 }
diff --git a/phpBB/phpbb/messenger/queue.php b/phpBB/phpbb/messenger/queue.php
@@ -116,6 +116,11 @@ public function process(): void
 
 		/** @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()]))
diff --git a/phpBB/phpbb/notification/method/messenger_base.php b/phpBB/phpbb/notification/method/messenger_base.php
@@ -114,6 +114,10 @@ protected function notify_using_messenger($notify_method, $template_dir_prefix =
 				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 @@ protected function notify_using_messenger($notify_method, $template_dir_prefix =
 					], $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();
 	}
diff --git a/tests/functional/memberlist_test.php b/tests/functional/memberlist_test.php
@@ -134,4 +134,29 @@ public function test_group_rank()
 
 		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());
+	}
 }

Original file line number	Diff line number	Diff line change
`@@ -227,6 +227,10 @@ function main($id, $mode)`
`227`	`227`	`$used_lang = $email_list[$i][0]['lang'];`
`228`	`228`	`$used_method = $email_list[$i][0]['method'];`
`229`	`229`
	`230`	`+ /**`
	`231`	`+ * @var \phpbb\messenger\method\messenger_interface $messenger_method`
	`232`	`+ * @psalm-suppress UndefinedMethod`
	`233`	`+ */`
`230`	`234`	`foreach ($messenger_collection_iterator as $messenger_method)`
`231`	`235`	`{`
`232`	`236`	`$notify_method = $messenger_method->get_id();`
`@@ -265,19 +269,12 @@ function main($id, $mode)`
`265`	`269`	`}`
`266`	`270`
`267`	`271`	`$errored = !$messenger_method->send() \|\| $errored;`
	`272`	`+ $messenger_method->save_queue();`
`268`	`273`	`}`
`269`	`274`	`}`
`270`	`275`	`}`
`271`	`276`	`unset($email_list);`
`272`	`277`
`273`		`- if ($use_queue)`
`274`		`- {`
`275`		`- foreach ($messenger_collection_iterator as $messenger_method)`
`276`		`- {`
`277`		`- $messenger_method->save_queue();`
`278`		`- }`
`279`		`- }`
`280`		`-`
`281`	`278`	`if ($generate_log_entry)`
`282`	`279`	`{`
`283`	`280`	`if (!empty($usernames))`
Original file line number	Diff line number	Diff line change
`@@ -205,6 +205,10 @@ function main($id, $mode)`
`205`	`205`
`206`	`206`	`do`
`207`	`207`	`{`
	`208`	`+ /**`
	`209`	`+ * @var \phpbb\messenger\method\messenger_interface $messenger_method`
	`210`	`+ * @psalm-suppress UndefinedMethod`
	`211`	`+ */`
`208`	`212`	`foreach ($messenger_collection_iterator as $messenger_method)`
`209`	`213`	`{`
`210`	`214`	`if ($messenger_method->get_id() == $user_row['user_notify_type'] \|\| $user_row['user_notify_type'] == $messenger_method::NOTIFY_BOTH)`
`@@ -219,6 +223,7 @@ function main($id, $mode)`
`219`	`223`	`]);`
`220`	`224`
`221`	`225`	`$messenger_method->send();`
	`226`	`+ $messenger_method->save_queue();`
`222`	`227`	`}`
`223`	`228`	`}`
`224`	`229`
`@@ -227,11 +232,6 @@ function main($id, $mode)`
`227`	`232`	`}`
`228`	`233`	`while ($row = $db->sql_fetchrow($result));`
`229`	`234`
`230`		`- foreach ($messenger_collection_iterator as $messenger_method)`
`231`		`- {`
`232`		`- $messenger_method->save_queue();`
`233`		`- }`
`234`		`-`
`235`	`235`	`// Add the remind state to the database and increase activation expiration by one day`
`236`	`236`	`$sql = 'UPDATE ' . USERS_TABLE . '`
`237`	`237`	`SET user_reminded = user_reminded + 1,`
Original file line number	Diff line number	Diff line change
`@@ -138,6 +138,10 @@ function main($id, $mode)`
`138`	`138`	`$messenger_collection_iterator = $messenger->getIterator();`
`139`	`139`	`while ($row = $db->sql_fetchrow($result))`
`140`	`140`	`{`
	`141`	`+ /**`
	`142`	`+ * @var \phpbb\messenger\method\messenger_interface $messenger_method`
	`143`	`+ * @psalm-suppress UndefinedMethod`
	`144`	`+ */`
`141`	`145`	`foreach ($messenger_collection_iterator as $messenger_method)`
`142`	`146`	`{`
`143`	`147`	`$messenger_method->set_use_queue(false);`
`@@ -157,12 +161,6 @@ function main($id, $mode)`
`157`	`161`	`}`
`158`	`162`	`}`
`159`	`163`	`$db->sql_freeresult($result);`
`160`		`-`
`161`		`- // Save the queue in the messenger method class (has to be called or these messages could be lost)`
`162`		`- foreach ($messenger_collection_iterator as $messenger_method)`
`163`		`- {`
`164`		`- $messenger_method->save_queue();`
`165`		`- }`
`166`	`164`	`}`
`167`	`165`
`168`	`166`	`$this->update_activation_expiration();`
Original file line number	Diff line number	Diff line change
`@@ -249,6 +249,11 @@ public function send(\phpbb\di\service_collection $messenger, $contact)`
`249`	`249`	`{`
`250`	`250`	`/** @psalm-suppress InvalidTemplateParam */`
`251`	`251`	`$messenger_collection_iterator = $messenger->getIterator();`
	`252`	`+`
	`253`	`+ /**`
	`254`	`+ * @var messenger_interface $messenger_method`
	`255`	`+ * @psalm-suppress UndefinedMethod`
	`256`	`+ */`
`252`	`257`	`foreach ($messenger_collection_iterator as $messenger_method)`
`253`	`258`	`{`
`254`	`259`	`$messenger_method->set_use_queue(false);`
`@@ -264,6 +269,7 @@ public function send(\phpbb\di\service_collection $messenger, $contact)`
`264`	`269`	`{`
`265`	`270`	`$messenger_method->header('X-AntiAbuse', 'User_id - ' . $this->sender_id);`
`266`	`271`	`}`
	`272`	`+`
`267`	`273`	`if ($this->sender_username)`
`268`	`274`	`{`
`269`	`275`	`$messenger_method->header('X-AntiAbuse', 'Username - ' . $this->sender_username);`
Original file line number	Diff line number	Diff line change
`@@ -481,4 +481,12 @@ protected function set_template_paths(string\|array $path_name, string\|array $pat`
`481`	`481`	`$this->setup_template();`
`482`	`482`	`$this->template->set_custom_style($path_name, $paths);`
`483`	`483`	`}`
	`484`	`+`
	`485`	`+ /**`
	`486`	`+ * {@inheritdoc}`
	`487`	`+ */`
	`488`	`+ public function header(string $header_name, mixed $header_value): void`
	`489`	`+ {`
	`490`	`+ return;`
	`491`	`+ }`
`484`	`492`	`}`
Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ public function init(): void`
`105`	`105`	`$this->mail_priority = symfony_email::PRIORITY_NORMAL;`
`106`	`106`
`107`	`107`	`$this->additional_headers = [];`
`108`		`- $this->use_queue = true;`
	`108`	`+ $this->set_use_queue();`
`109`	`109`	`unset($this->template, $this->reply_to, $this->from);`
`110`	`110`	`}`
`111`	`111`
`@@ -242,10 +242,10 @@ public function subject(string $subject = ''): void`
`242`	`242`	`*/`
`243`	`243`	`public function anti_abuse_headers(\phpbb\config\config $config, \phpbb\user $user): void`
`244`	`244`	`{`
`245`		`- $this->headers->addHeader('X-AntiAbuse', 'Board servername - ' . $config['server_name']);`
`246`		`- $this->headers->addHeader('X-AntiAbuse', 'User_id - ' . $user->data['user_id']);`
`247`		`- $this->headers->addHeader('X-AntiAbuse', 'Username - ' . $user->data['username']);`
`248`		`- $this->headers->addHeader('X-AntiAbuse', 'User IP - ' . $user->ip);`
	`245`	`+ $this->header('X-AntiAbuse', 'Board servername - ' . $config['server_name']);`
	`246`	`+ $this->header('X-AntiAbuse', 'User_id - ' . $user->data['user_id']);`
	`247`	`+ $this->header('X-AntiAbuse', 'Username - ' . $user->data['username']);`
	`248`	`+ $this->header('X-AntiAbuse', 'User IP - ' . $user->ip);`
`249`	`249`	`}`
`250`	`250`
`251`	`251`	`/**`
`@@ -313,7 +313,7 @@ protected function build_headers(): void`
`313`	`313`
`314`	`314`	`foreach ($headers as $header => $value)`
`315`	`315`	`{`
`316`		`- $this->headers->addHeader($header, $value);`
	`316`	`+ $this->header($header, $value);`
`317`	`317`	`}`
`318`	`318`
`319`	`319`	`}`
`@@ -576,4 +576,12 @@ public function send(): bool`
`576`	`576`
`577`	`577`	`return true;`
`578`	`578`	`}`
	`579`	`+`
	`580`	`+ /**`
	`581`	`+ * {@inheritdoc}`
	`582`	`+ */`
	`583`	`+ public function header(string $header_name, mixed $header_value): void`
	`584`	`+ {`
	`585`	`+ $this->headers->addHeader($header_name, $header_value);`
	`586`	`+ }`
`579`	`587`	`}`
Original file line number	Diff line number	Diff line change
`@@ -114,6 +114,10 @@ protected function notify_using_messenger($notify_method, $template_dir_prefix =`
`114`	`114`	`continue;`
`115`	`115`	`}`
`116`	`116`
	`117`	`+ /**`
	`118`	`+ * @var \phpbb\messenger\method\messenger_interface $messenger_method`
	`119`	`+ * @psalm-suppress UndefinedMethod`
	`120`	`+ */`
`117`	`121`	`foreach ($messenger_collection_iterator as $messenger_method)`
`118`	`122`	`{`
`119`	`123`	`if ($messenger_method->get_id() == $notify_method \|\| $notify_method == $messenger_method::NOTIFY_BOTH)`
`@@ -126,16 +130,13 @@ protected function notify_using_messenger($notify_method, $template_dir_prefix =`
`126`	`130`	`], $notification->get_email_template_variables()));`
`127`	`131`
`128`	`132`	`$messenger_method->send();`
	`133`	`+`
	`134`	`+ // Save the queue in the messenger method class (has to be called or these messages could be lost)`
	`135`	`+ $messenger_method->save_queue();`
`129`	`136`	`}`
`130`	`137`	`}`
`131`	`138`	`}`
`132`	`139`
`133`		`- // Save the queue in the messenger method class (has to be called or these messages could be lost)`
`134`		`- foreach ($messenger_collection_iterator as $messenger_method)`
`135`		`- {`
`136`		`- $messenger_method->save_queue();`
`137`		`- }`
`138`		`-`
`139`	`140`	`// We're done, empty the queue`
`140`	`141`	`$this->empty_queue();`
`141`	`142`	`}`