Fixes #73 : allow Message to not has "To" header #160
Conversation
|
LGTM |
|
+1 |
test/Transport/SendmailTest.php
Outdated
| $this->transport->send($message); | ||
| } | ||
|
|
||
| public function testDonotAllowMessageDoesnotHasToAndCcAndBccHeader() |
There was a problem hiding this comment.
Please fix test method name, I think it should be:
testDoNotAllowMessageWithoutToAndCcAndBccHeaders
test/Transport/SendmailTest.php
Outdated
|
|
||
| public function testDonotAllowMessageDoesnotHasToAndCcAndBccHeader() | ||
| { | ||
| $this->expectException('Zend\Mail\Exception\RuntimeException'); |
There was a problem hiding this comment.
Please move this before line which should thrown an exception. Like that it's confusing, and the exception could be thrown anywhere below. Can we also use ::class notation, please?
src/Transport/Sendmail.php
Outdated
| @@ -148,8 +148,14 @@ protected function prepareRecipients(Mail\Message $message) | |||
| { | |||
| $headers = $message->getHeaders(); | |||
|
|
|||
| if (! $headers->has('to')) { | |||
| throw new Exception\RuntimeException('Invalid email; contains no "To" header'); | |||
| if (! ($hasTo = $headers->has('to')) && ! $headers->has('cc') && ! $headers->has('bcc')) { | |||
There was a problem hiding this comment.
I got a bit confused with this assignment in if statement, in general I like them, but only when we have single condition, here it's more tricky so I would suggest to move assignment before the if statement and then use the variable in the statement with other checks:
$hasTo = $headers->has('to');
if (! $hasTo && ! $headers->has('cc') && ! $headers->has('bcc')) {…d move expectException() before transport->send() call
|
Manually merged via 8ca640a, thanks! |
Fixes #73
Check at least one of "To", "Cc", and "Bcc"