[TASK] Add proper error message for failed messages

This commit adds proper error messages when mail sending via the
mail queue has failed. It also refactors the MailTemplateService that it
does not use duplicate code to send mails.

Classes/Controller/QueueController.php

@@ -172,23 +172,27 @@ class QueueController extends ActionController {
 	 * send or resend a mail in the queue
 	 *
 	 * @param int $uid
-	 * @param string $selectedTemplate
-	 * @param string $selectedExtension
 	 * @throws \InvalidArgumentException
 	 * @throws \TYPO3\CMS\Extbase\Mvc\Exception\StopActionException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\IllegalObjectTypeException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\UnknownObjectException
 	 * @throws \TYPO3\CMS\Extbase\Mvc\Exception\UnsupportedRequestTypeException
 	 */
-	public function sendMailAction($uid, $selectedTemplate, $selectedExtension) {
-
+	public function sendMailAction($uid) {
 		$mailService = new MailTemplateService();
-		$mailService->sendMailFromQueue($uid);
+		if ($mailService->sendMailFromQueue($uid)) {
+			$message = LocalizationUtility::translate('backend.success_mail_queue', 'sg_mail');
+			$this->addFlashMessage($message, '', FlashMessage::OK);
+		} else {
+			$message = LocalizationUtility::translate(
+				'backend.error_mail_queue',
+				'sg_mail',
+				[\implode(', ', $mailService->getMailMessage()->getFailedRecipients())]
+			);
+			$this->addFlashMessage($message, '', FlashMessage::ERROR);
+		}
 
-		$message = LocalizationUtility::translate('backend.success_mail_queue', 'sg_mail');
-		$this->addFlashMessage($message, '', FlashMessage::OK);
-		$arguments = $this->request->getArguments();
-		$this->redirect('index', NULL, NULL, $arguments);
+		$this->redirect('index', NULL, NULL, $this->request->getArguments());
 	}
 
 	/**

Classes/Service/MailTemplateService.php

@@ -156,10 +156,10 @@ class MailTemplateService {
 	 *
 	 * @param string $templateName
 	 * @param string $extensionKey
-	 * @param string $markers
+	 * @param array $markers
 	 * @throws \InvalidArgumentException
 	 */
-	public function __construct($templateName = '', $extensionKey = '', $markers = '') {
+	public function __construct($templateName = '', $extensionKey = '', $markers = []) {
 		$this->templateName = $templateName;
 		$this->extensionKey = $extensionKey;
 		$this->markers = $markers;
@@ -492,8 +492,8 @@ class MailTemplateService {
 	/**
 	 * Send the Email
 	 *
-	 * @param boolean $isPreview
-	 * @return boolean email was sent or added to mail queue successfully?
+	 * @param bool $isPreview
+	 * @return bool email was sent or added to mail queue successfully?
 	 * @throws \TYPO3\CMS\Core\Cache\Exception\NoSuchCacheException
 	 * @throws \InvalidArgumentException
 	 * @throws \BadFunctionCallException
@@ -501,6 +501,7 @@ class MailTemplateService {
 	 * @throws \Exception
 	 */
 	public function sendEmail($isPreview = FALSE): bool {
+		$success = FALSE;
 		if (TYPO3_MODE === 'FE') {
 			/** @var TypoScriptFrontendController $typoscriptFrontendController */
 			$typoscriptFrontendController = $GLOBALS['TSFE'];
@@ -526,7 +527,9 @@ class MailTemplateService {
 
 		$isTemplateBlacklisted = self::isTemplateBlacklisted($this->extensionKey, $this->templateName, $siteRootId);
 		if ($isTemplateBlacklisted) {
-			return TRUE;
+			// @TODO: This needs to be changed, because if the template is blacklisted, the email can not be sent
+			$success = TRUE;
+			return $success;
 		}
 
 		/** @var Template $template */
@@ -572,7 +575,7 @@ class MailTemplateService {
 					if (file_exists($defaultTemplateFile)) {
 						$defaultTemplateContent = file_get_contents($defaultTemplateFile);
 					} else {
-						return FALSE;
+						return $success;
 					}
 				}
 			}
@@ -648,26 +651,17 @@ class MailTemplateService {
 		$emailBody = nl2br($emailBody);
 		$emailBody = preg_replace('/(<br[\s]?[\/]?>[\s]*){3,}/', '<br><br>', $emailBody);
 
-		$currentTimestamp = 0;
-		if ($this->ignoreMailQueue) {
-			// @TODO arghh... why is this not sharing the code with sendMailFromQueue????
-			$this->mailMessage->setBody($emailBody, 'text/html');
-			$plaintextService = GeneralUtility::makeInstance(PlaintextService::class);
-			$plainTextBody = $plaintextService->makePlain($emailBody);
-			$this->mailMessage->addPart($plainTextBody, 'text/plain');
-			$this->mailMessage->send();
-			$dateTime = new DateTime();
-			$currentTimestamp = $dateTime->getTimestamp();
-		}
-
 		if (!$isPreview) {
-			$this->addMailToMailQueue(
+			$mail = $this->addMailToMailQueue(
 				$this->extensionKey, $this->templateName, $subject, $emailBody, $this->priority,
-				$currentTimestamp, $currentTimestamp, $this->language, $siteRootId
+				0, 0, $this->language, $siteRootId
 			);
+			if ($this->ignoreMailQueue) {
+				$success = $this->sendMailFromQueue($mail->getUid());
+			}
 		}
 
-		return TRUE;
+		return $success;
 	}
 
 	/**
@@ -682,6 +676,7 @@ class MailTemplateService {
 	 * @param int $lastSendingTime
 	 * @param string $language
 	 * @param int $pid
+	 * @return Mail
 	 * @throws \InvalidArgumentException
 	 * @throws \BadFunctionCallException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\IllegalObjectTypeException
@@ -719,12 +714,14 @@ class MailTemplateService {
 		$mailRepository = $this->objectManager->get(MailRepository::class);
 		$mailRepository->add($mail);
 		$this->persistenceManager->persistAll();
+		return $mail;
 	}
 
 	/**
 	 * Send a Mail from the queue, identified by its id
 	 *
 	 * @param int $uid
+	 * @return bool
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\IllegalObjectTypeException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\UnknownObjectException
 	 * @throws \Exception
@@ -776,8 +773,13 @@ class MailTemplateService {
 				$mailToSend->setSendingTime($dateTime->getTimestamp());
 			}
 			$mailToSend->setLastSendingTime($dateTime->getTimestamp());
-			$this->mailMessage->send();
-			$mailRepository->update($mailToSend);
+			$success = $this->mailMessage->send();
+			if ($success) {
+				$mailRepository->update($mailToSend);
+			} else {
+				$this->mailMessage->getFailedRecipients();
+			}
+			return $success;
 		}
 	}
 

Resources/Private/Language/de.locallang.xlf

@@ -355,6 +355,10 @@ Bitte registrieren Sie Ihre Templates in den entsprechenden ext_localconf.php Da
 				<source><![CDATA[Email succesfully sent]]></source>
 				<target><![CDATA[Die E-Mail wurde erfolgreich versendet]]></target>
 			</trans-unit>
+			<trans-unit id="backend.error_mail_queue">
+				<source><![CDATA[Email could not be sent to %s]]></source>
+				<target><![CDATA[Die E-Mail konnte nicht an %s versendet werden ]]></target>
+			</trans-unit>
 			<trans-unit id="backend.template_editor" approved="yes">
 				<source><![CDATA[Template Editor]]></source>
 				<target><![CDATA[Template-Editor]]></target>

Resources/Private/Language/locallang.xlf

@@ -277,6 +277,9 @@ Please register your configurations in the according ext_localconf.php.]]></sour
 			<trans-unit id="backend.success_mail_queue">
 				<source><![CDATA[Email succesfully sent]]></source>
 			</trans-unit>
+			<trans-unit id="backend.error_mail_queue">
+				<source><![CDATA[Email could not be sent to %s]]></source>
+			</trans-unit>
 			<trans-unit id="backend.template_editor">
 				<source><![CDATA[Template Editor]]></source>
 			</trans-unit>

Resources/Private/Templates/Queue/Index.html

@@ -131,10 +131,10 @@
 						<f:for each="{pages}" as="page">
 							<tr data-uid="{page.pid}">
 								<td nowrap="nowrap" class="col-title">
-									<a href="#" onclick="sgMailGoToPage({page.uid}, '{page.path}'); return false;">
+									<f:link.action action="index" additionalParams="{id: page.uid, returnUrl: returnUrl}">
 										<sgm:backend.icon table="pages" row="{page}" clickMenu="0" />
 										{page._thePathFull}
-									</a>
+									</f:link.action>
 								</td>
 							</tr>
 						</f:for>