Skip to content

Conversation

@JanTvrdik
Copy link
Contributor

Simple mailer which can retry sending mail multiple times with either one ore multiple mailers.

@JanTvrdik
Copy link
Contributor Author

Some open questions:

  • Should the total number of retries by $retryCount or $retryCount * count($this->mailers)?
  • Should we log the exceptions? I don't know how to do it without depending on Tracy\ILogger. Or should we just emit warnings?

@dg
Copy link
Member

dg commented May 20, 2016

To throw, to log is task for a diffent layer.

@JanTvrdik
Copy link
Contributor Author

JanTvrdik commented May 20, 2016

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?

@dg
Copy link
Member

dg commented May 20, 2016

I tried to say that FallbackMailer should not log exceptions. It is not its responsibility. It can provide some callback, but not log them.

@JanTvrdik JanTvrdik force-pushed the pr/fallback_mailer branch 3 times, most recently from fbf7221 to f8fb0d5 Compare May 20, 2016 14:17
*/
public function send(Message $mail)
{
for ($i = 0; $i < $this->retryCount; $i++) {
Copy link
Member

@milo milo May 21, 2016

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?

Copy link
Contributor Author

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.

@JanTvrdik
Copy link
Contributor Author

This is ready to merge from my point of view.

@JanTvrdik JanTvrdik force-pushed the pr/fallback_mailer branch from 1f06b15 to c0bb198 Compare June 5, 2016 06:03
/**
* @param int
*/
public function setRetryCount($retryCount)
Copy link
Member

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.

@dg dg closed this in 22d3ee5 Jun 25, 2016
@JanTvrdik JanTvrdik deleted the pr/fallback_mailer branch June 27, 2016 13:43
@f3l1x
Copy link
Member

f3l1x commented Jun 27, 2016

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

@JanTvrdik
Copy link
Contributor Author

@f3l1x What do you use it for?

@f3l1x
Copy link
Member

f3l1x commented Jun 27, 2016

Dump message to folder, open in editor, check design especially for newsletter, verify all EML options/requirements (html vs plain) etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants