Remove unnecessary test_email extra parameter for mailer->send

It was used only to prevent appending unprocessed subscribers into error message.
Since the message is now composed by on demand by MailerError the parameter is not needed any more.

[MAILPOET-1154]
This commit is contained in:
Rostislav Wolny
2018-09-11 19:52:50 +02:00
parent 665fce9dbb
commit f5c9d0f7db
13 changed files with 28 additions and 53 deletions

View File

@ -21,10 +21,7 @@ class Mailer extends APIEndpoint {
(isset($data['sender'])) ? $data['sender'] : false, (isset($data['sender'])) ? $data['sender'] : false,
(isset($data['reply_to'])) ? $data['reply_to'] : false (isset($data['reply_to'])) ? $data['reply_to'] : false
); );
$extra_params = array( $result = $mailer->send($data['newsletter'], $data['subscriber']);
'test_email' => true
);
$result = $mailer->send($data['newsletter'], $data['subscriber'], $extra_params);
} catch(\Exception $e) { } catch(\Exception $e) {
return $this->errorResponse(array( return $this->errorResponse(array(
$e->getCode() => $e->getMessage() $e->getCode() => $e->getMessage()

View File

@ -79,7 +79,7 @@ class AmazonSES {
} }
if(WPFunctions::wpRemoteRetrieveResponseCode($result) !== 200) { if(WPFunctions::wpRemoteRetrieveResponseCode($result) !== 200) {
$response = simplexml_load_string(WPFunctions::wpRemoteRetrieveBody($result)); $response = simplexml_load_string(WPFunctions::wpRemoteRetrieveBody($result));
$error = $this->error_mapper->getErrorFromResponse($response, $subscriber, $extra_params); $error = $this->error_mapper->getErrorFromResponse($response, $subscriber);
return Mailer::formatMailerErrorResult($error); return Mailer::formatMailerErrorResult($error);
} }
return Mailer::formatMailerSendSuccessResult(); return Mailer::formatMailerSendSuccessResult();

View File

@ -21,7 +21,7 @@ class AmazonSESMapper {
* @see https://docs.aws.amazon.com/ses/latest/DeveloperGuide/api-error-codes.html * @see https://docs.aws.amazon.com/ses/latest/DeveloperGuide/api-error-codes.html
* @return MailerError * @return MailerError
*/ */
function getErrorFromResponse($response, $subscriber, $extra_params) { function getErrorFromResponse($response, $subscriber) {
$message = ($response) ? $message = ($response) ?
$response->Error->Message->__toString() : $response->Error->Message->__toString() :
sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_AMAZONSES); sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_AMAZONSES);
@ -30,11 +30,7 @@ class AmazonSESMapper {
if($response && $response->Error->Code->__toString() === 'MessageRejected') { if($response && $response->Error->Code->__toString() === 'MessageRejected') {
$level = MailerError::LEVEL_SOFT; $level = MailerError::LEVEL_SOFT;
} }
$subscriber_errors = [new SubscriberError($subscriber, null)];
$subscriber_errors = [];
if(empty($extra_params['test_email'])) {
$subscriber_errors[] = new SubscriberError($subscriber, null);
}
return new MailerError(MailerError::OPERATION_SEND, $level, $message, null, $subscriber_errors); return new MailerError(MailerError::OPERATION_SEND, $level, $message, null, $subscriber_errors);
} }
} }

View File

@ -8,24 +8,19 @@ use MailPoet\Mailer\SubscriberError;
class PHPMailMapper { class PHPMailMapper {
use ConnectionErrorMapperTrait; use ConnectionErrorMapperTrait;
function getErrorFromException(\Exception $e, $subscriber, $extra_params) { function getErrorFromException(\Exception $e, $subscriber) {
$level = MailerError::LEVEL_HARD; $level = MailerError::LEVEL_HARD;
if(strpos($e->getMessage(), 'Invalid address') === 0) { if(strpos($e->getMessage(), 'Invalid address') === 0) {
$level = MailerError::LEVEL_SOFT; $level = MailerError::LEVEL_SOFT;
} }
$subscriber_errors = [];
if(empty($extra_params['test_email'])) { $subscriber_errors = [new SubscriberError($subscriber, null)];
$subscriber_errors[] = new SubscriberError($subscriber, null);
}
return new MailerError(MailerError::OPERATION_SEND, $level, $e->getMessage(), null, $subscriber_errors); return new MailerError(MailerError::OPERATION_SEND, $level, $e->getMessage(), null, $subscriber_errors);
} }
function getErrorForSubscriber($subscriber, $extra_params) { function getErrorForSubscriber($subscriber) {
$message = sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_PHPMAIL); $message = sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_PHPMAIL);
$subscriber_errors = []; $subscriber_errors = [new SubscriberError($subscriber, null)];
if(empty($extra_params['test_email'])) {
$subscriber_errors[] = new SubscriberError($subscriber, null);
}
return new MailerError(MailerError::OPERATION_SEND, MailerError::LEVEL_HARD, $message, null, $subscriber_errors); return new MailerError(MailerError::OPERATION_SEND, MailerError::LEVEL_HARD, $message, null, $subscriber_errors);
} }
} }

View File

@ -12,7 +12,7 @@ class SMTPMapper {
* @see https://swiftmailer.symfony.com/docs/sending.html * @see https://swiftmailer.symfony.com/docs/sending.html
* @return MailerError * @return MailerError
*/ */
function getErrorFromException(\Exception $e, $subscriber, $extra_params = []) { function getErrorFromException(\Exception $e, $subscriber) {
// remove redundant information appended by Swift logger to exception messages // remove redundant information appended by Swift logger to exception messages
$message = explode(PHP_EOL, $e->getMessage()); $message = explode(PHP_EOL, $e->getMessage());
@ -20,16 +20,11 @@ class SMTPMapper {
if($e instanceof \Swift_RfcComplianceException) { if($e instanceof \Swift_RfcComplianceException) {
$level = MailerError::LEVEL_SOFT; $level = MailerError::LEVEL_SOFT;
} }
$subscriber_errors = [new SubscriberError($subscriber, null)];
$subscriber_errors = [];
if(empty($extra_params['test_email'])) {
$subscriber_errors[] = new SubscriberError($subscriber, null);
}
return new MailerError(MailerError::OPERATION_SEND, $level, $message[0], null, $subscriber_errors); return new MailerError(MailerError::OPERATION_SEND, $level, $message[0], null, $subscriber_errors);
} }
function getErrorFromLog($log, $subscriber, $extra_params = []) { function getErrorFromLog($log, $subscriber) {
// extract error message from log // extract error message from log
preg_match('/!! (.*?)>>/ism', $log, $message); preg_match('/!! (.*?)>>/ism', $log, $message);
if(!empty($message[1])) { if(!empty($message[1])) {
@ -39,11 +34,7 @@ class SMTPMapper {
} else { } else {
$message = sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_SMTP); $message = sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_SMTP);
} }
$subscriber_errors = [new SubscriberError($subscriber, null)];
$subscriber_errors = [];
if(empty($extra_params['test_email'])) {
$subscriber_errors[] = new SubscriberError($subscriber, null);
}
return new MailerError(MailerError::OPERATION_SEND, MailerError::LEVEL_HARD, $message, null, $subscriber_errors); return new MailerError(MailerError::OPERATION_SEND, MailerError::LEVEL_HARD, $message, null, $subscriber_errors);
} }
} }

View File

@ -8,7 +8,7 @@ use MailPoet\Mailer\SubscriberError;
class SendGridMapper { class SendGridMapper {
use ConnectionErrorMapperTrait; use ConnectionErrorMapperTrait;
function getErrorFromResponse($response, $subscriber, $extra_params) { function getErrorFromResponse($response, $subscriber) {
$response = (!empty($response['errors'][0])) ? $response = (!empty($response['errors'][0])) ?
$response['errors'][0] : $response['errors'][0] :
sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_SENDGRID); sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_SENDGRID);
@ -17,11 +17,7 @@ class SendGridMapper {
if(strpos($response, 'Invalid email address') === 0) { if(strpos($response, 'Invalid email address') === 0) {
$level = MailerError::LEVEL_SOFT; $level = MailerError::LEVEL_SOFT;
} }
$subscriber_errors = [new SubscriberError($subscriber, null)];
$subscriber_errors = [];
if(empty($extra_params['test_email'])) {
$subscriber_errors[] = new SubscriberError($subscriber, null);
}
return new MailerError(MailerError::OPERATION_SEND, $level, $response, null, $subscriber_errors); return new MailerError(MailerError::OPERATION_SEND, $level, $response, null, $subscriber_errors);
} }
} }

View File

@ -33,12 +33,12 @@ class PHPMail {
$mailer = $this->configureMailerWithMessage($newsletter, $subscriber, $extra_params); $mailer = $this->configureMailerWithMessage($newsletter, $subscriber, $extra_params);
$result = $mailer->send(); $result = $mailer->send();
} catch(\Exception $e) { } catch(\Exception $e) {
return Mailer::formatMailerErrorResult($this->error_mapper->getErrorFromException($e, $subscriber, $extra_params)); return Mailer::formatMailerErrorResult($this->error_mapper->getErrorFromException($e, $subscriber));
} }
if($result === true) { if($result === true) {
return Mailer::formatMailerSendSuccessResult(); return Mailer::formatMailerSendSuccessResult();
} else { } else {
$error = $this->error_mapper->getErrorForSubscriber($subscriber, $extra_params); $error = $this->error_mapper->getErrorForSubscriber($subscriber);
return Mailer::formatMailerErrorResult($error); return Mailer::formatMailerErrorResult($error);
} }
} }

View File

@ -49,13 +49,13 @@ class SMTP {
$result = $this->mailer->send($message); $result = $this->mailer->send($message);
} catch(\Exception $e) { } catch(\Exception $e) {
return Mailer::formatMailerErrorResult( return Mailer::formatMailerErrorResult(
$this->error_mapper->getErrorFromException($e, $subscriber, $extra_params) $this->error_mapper->getErrorFromException($e, $subscriber)
); );
} }
if($result === 1) { if($result === 1) {
return Mailer::formatMailerSendSuccessResult(); return Mailer::formatMailerSendSuccessResult();
} else { } else {
$error = $this->error_mapper->getErrorFromLog($this->mailer_logger->dump(), $subscriber, $extra_params); $error = $this->error_mapper->getErrorFromLog($this->mailer_logger->dump(), $subscriber);
return Mailer::formatMailerErrorResult($error); return Mailer::formatMailerErrorResult($error);
} }
} }

View File

@ -35,7 +35,7 @@ class SendGrid {
} }
if(WPFunctions::wpRemoteRetrieveResponseCode($result) !== 200) { if(WPFunctions::wpRemoteRetrieveResponseCode($result) !== 200) {
$response = json_decode($result['body'], true); $response = json_decode($result['body'], true);
$error = $this->error_mapper->getErrorFromResponse($response, $subscriber, $extra_params); $error = $this->error_mapper->getErrorFromResponse($response, $subscriber);
return Mailer::formatMailerErrorResult($error); return Mailer::formatMailerErrorResult($error);
} }
return Mailer::formatMailerSendSuccessResult(); return Mailer::formatMailerSendSuccessResult();

View File

@ -27,7 +27,7 @@ class AmazonSESMapperTest extends \MailPoetTest {
function testGetProperError() { function testGetProperError() {
$response = $this->buildXmlResponseFromArray($this->response_data, new SimpleXMLElement('<response/>')); $response = $this->buildXmlResponseFromArray($this->response_data, new SimpleXMLElement('<response/>'));
$error = $this->mapper->getErrorFromResponse($response, 'john@rambo.com', []); $error = $this->mapper->getErrorFromResponse($response, 'john@rambo.com');
expect($error->getLevel())->equals(MailerError::LEVEL_HARD); expect($error->getLevel())->equals(MailerError::LEVEL_HARD);
expect($error->getMessage())->equals('Some message'); expect($error->getMessage())->equals('Some message');
expect($error->getSubscriberErrors()[0]->getEmail())->equals('john@rambo.com'); expect($error->getSubscriberErrors()[0]->getEmail())->equals('john@rambo.com');
@ -36,7 +36,7 @@ class AmazonSESMapperTest extends \MailPoetTest {
function testGetSoftErrorForRejectedMessage() { function testGetSoftErrorForRejectedMessage() {
$this->response_data['Error']['Code'] = 'MessageRejected'; $this->response_data['Error']['Code'] = 'MessageRejected';
$response = $this->buildXmlResponseFromArray($this->response_data, new SimpleXMLElement('<response/>')); $response = $this->buildXmlResponseFromArray($this->response_data, new SimpleXMLElement('<response/>'));
$error = $this->mapper->getErrorFromResponse($response, 'john@rambo.com', []); $error = $this->mapper->getErrorFromResponse($response, 'john@rambo.com');
expect($error->getLevel())->equals(MailerError::LEVEL_SOFT); expect($error->getLevel())->equals(MailerError::LEVEL_SOFT);
} }

View File

@ -14,21 +14,21 @@ class PHPMailMapperTest extends \MailPoetTest {
} }
function testGetProperErrorForSubscriber() { function testGetProperErrorForSubscriber() {
$error = $this->mapper->getErrorForSubscriber('john@rambo.com', []); $error = $this->mapper->getErrorForSubscriber('john@rambo.com');
expect($error->getLevel())->equals(MailerError::LEVEL_HARD); expect($error->getLevel())->equals(MailerError::LEVEL_HARD);
expect($error->getMessage())->equals('PHPMail has returned an unknown error.'); expect($error->getMessage())->equals('PHPMail has returned an unknown error.');
expect($error->getSubscriberErrors()[0]->getEmail())->equals('john@rambo.com'); expect($error->getSubscriberErrors()[0]->getEmail())->equals('john@rambo.com');
} }
function testGetProperErrorFromException() { function testGetProperErrorFromException() {
$error = $this->mapper->getErrorFromException(new \Exception('Some message'), 'john@rambo.com', []); $error = $this->mapper->getErrorFromException(new \Exception('Some message'), 'john@rambo.com');
expect($error->getLevel())->equals(MailerError::LEVEL_HARD); expect($error->getLevel())->equals(MailerError::LEVEL_HARD);
expect($error->getMessage())->equals('Some message'); expect($error->getMessage())->equals('Some message');
expect($error->getSubscriberErrors()[0]->getEmail())->equals('john@rambo.com'); expect($error->getSubscriberErrors()[0]->getEmail())->equals('john@rambo.com');
} }
function testGetSoftErrorFromExceptionForInvalidEmail() { function testGetSoftErrorFromExceptionForInvalidEmail() {
$error = $this->mapper->getErrorFromException(new \Exception('Invalid address. (Add ...'), 'john@rambo.com', []); $error = $this->mapper->getErrorFromException(new \Exception('Invalid address. (Add ...'), 'john@rambo.com');
expect($error->getLevel())->equals(MailerError::LEVEL_SOFT); expect($error->getLevel())->equals(MailerError::LEVEL_SOFT);
} }
} }

View File

@ -42,7 +42,7 @@ class SMTPMapperTest extends \MailPoetTest {
. '" (code: 550)' . PHP_EOL . '" (code: 550)' . PHP_EOL
. '>> RSET' . PHP_EOL . '>> RSET' . PHP_EOL
. '<< 250 Reset OK' . PHP_EOL; . '<< 250 Reset OK' . PHP_EOL;
$error = $this->mapper->getErrorFromLog($log, 'test@example.com', []); $error = $this->mapper->getErrorFromLog($log, 'test@example.com');
expect($error->getMessage()) expect($error->getMessage())
->equals('Expected response code 250/251/252 but got code "550", with message "550 No such recipient here" (code: 550)'); ->equals('Expected response code 250/251/252 but got code "550", with message "550 No such recipient here" (code: 550)');
expect($error->getSubscriberErrors()[0]->getEmail('moi@mrcasual.com')); expect($error->getSubscriberErrors()[0]->getEmail('moi@mrcasual.com'));

View File

@ -22,7 +22,7 @@ class SendGridMapperTest extends \MailPoetTest {
} }
function testGetProperError() { function testGetProperError() {
$error = $this->mapper->getErrorFromResponse($this->response, 'john@rambo.com', []); $error = $this->mapper->getErrorFromResponse($this->response, 'john@rambo.com');
expect($error->getLevel())->equals(MailerError::LEVEL_HARD); expect($error->getLevel())->equals(MailerError::LEVEL_HARD);
expect($error->getMessage())->equals('Some message'); expect($error->getMessage())->equals('Some message');
expect($error->getSubscriberErrors()[0]->getEmail())->equals('john@rambo.com'); expect($error->getSubscriberErrors()[0]->getEmail())->equals('john@rambo.com');
@ -30,7 +30,7 @@ class SendGridMapperTest extends \MailPoetTest {
function testGetSoftErrorForInvalidEmail() { function testGetSoftErrorForInvalidEmail() {
$this->response['errors'][0] = 'Invalid email address ,,@'; $this->response['errors'][0] = 'Invalid email address ,,@';
$error = $this->mapper->getErrorFromResponse($this->response, ',,@', []); $error = $this->mapper->getErrorFromResponse($this->response, ',,@');
expect($error->getLevel())->equals(MailerError::LEVEL_SOFT); expect($error->getLevel())->equals(MailerError::LEVEL_SOFT);
} }
} }