-
-
Notifications
You must be signed in to change notification settings - Fork 73
add FallbackMailer #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Some open questions:
|
|
To throw, to log is task for a diffent layer. |
|
Poetic but I don't understand what you mean. Does that mean that I should move the FallbackMailer to Nextras because it cannot be implemented in Nette? |
|
I tried to say that FallbackMailer should not log exceptions. It is not its responsibility. It can provide some callback, but not log them. |
fbf7221 to
f8fb0d5
Compare
| */ | ||
| public function send(Message $mail) | ||
| { | ||
| for ($i = 0; $i < $this->retryCount; $i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about to check, that mailers is not empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll add the check in constructor.
|
This is ready to merge from my point of view. |
1f06b15 to
c0bb198
Compare
| /** | ||
| * @param int | ||
| */ | ||
| public function setRetryCount($retryCount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All setters in nette should return $this. But even better is to omit anything that is not really needed. I think that except addMailer() is not need nothing. And with addMailer() should be if (!$mailers) { throw ... tested later.
|
Wow. Good job. 👍 I was not sure it could be part of nette/mail package. I use for every project on my local devstack something that I called FileMailer. Do you think it could be also part of package? Implementation is really small and cool. <?php
use Nette\Mail\IMailer;
use Nette\Mail\Message;
final class FileMailer implements IMailer
{
/** @var string */
private $path;
/**
* @param string $path
*/
public function __construct($path)
{
$this->path = $path;
}
/**
* @param Message $mail
*/
public function send(Message $mail)
{
@mkdir($path, 0777, TRUE);
file_put_contents($this->path . date('Y-m-d H-i-s') . microtime() . '.eml', $mail->generateMessage());
}
}Should I prepare PR with tests? @JanTvrdik @dg |
|
@f3l1x What do you use it for? |
|
Dump message to folder, open in editor, check design especially for newsletter, verify all EML options/requirements (html vs plain) etc. |
Simple mailer which can retry sending mail multiple times with either one ore multiple mailers.