Skip to content
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

[DX] Add a new mailer capable of rendering Twig templates #15060

Closed
javiereguiluz opened this issue Jun 22, 2015 · 14 comments
Closed

[DX] Add a new mailer capable of rendering Twig templates #15060

javiereguiluz opened this issue Jun 22, 2015 · 14 comments
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony)

Comments

@javiereguiluz
Copy link
Member

The problem

In my opinion, the current solution to send emails with Symfony is too "low level". (Related article: Send mails in Symfony in a way that does not suck!)

In my experience, most Symfony applications want to render a Twig template when sending an email. That's why FOSUserBundle includes the TwigSwiftMailer and that's why looking for Swift_Message::newInstance in GitHub gives you thousands of results with lots of shortcut methods to render a template.

The solution

I propose to create a new renderAndSend() method for the mailer service. It would work as follows:

  1. Simplest case (fixed 'to' and 'from' email addresses):
$this->get('mailer')->renderAndSend('email/notification.html.twig', array(
    'event' => $event,
));
{# app/Resources/views/email/notification.html.twig #}
{% block to 'user@example.com' %}
{% block from 'alerts@example.com' %}
{% block subject %}[{{ 'now'|date(d/m/Y H:i:s) }}] {{ event.title[:128] }}{% endblock %}

{% block body %}
    ...
    <div>{{ event.content }}</div>
    ...
{% endblock %}
  1. {% block body %} is a shortcut to set the HTML content. If you need different contents for HTML and TXT, use these blocks instead:
{% block body_html %} ... {% endblock %}
{% block body_text %} ... {% endblock %}
  1. When the $to or $from email addresses are dynamic, you can pass them in the parameters array:
$this->get('mailer')->renderAndSend('email/notification.html.twig', array(
    'event' => $event,
    'to' => 'user@example.com',
    'from' => 'alerts@example.com',
));

Then you don't need to define the to and from blocks:

{# app/Resources/views/email/notification.html.twig #}
{% block subject %}[{{ 'now'|date(d/m/Y H:i:s) }}] {{ event.title[:128] }}{% endblock %}

{% block body %}
    ...
{% endblock %}
  1. These are the special Twig blocks:
{% block to %} ... {% endblock %}
{% block from %} ... {% endblock %}
{% block subject %} ... {% endblock %}

{% block body %} ... {% endblock %}

{% block body_html %} ... {% endblock %}
{% block body_text %} ... {% endblock %}
  1. If both the Twig block and the parameter are given, the block is used. This allows to use the following:
$this->get('mailer')->renderAndSend('email/notification.html.twig', array(
    'event' => $event,
    'to' => 'user@example.com',
    'user' => $user,
));
{# app/Resources/views/email/notification.html.twig #}
{% block to %}{{ user.name }} <{{ to }}>{% endblock %}
...
  1. The sendAndRender() method would define a third optional parameter to set the file attachments:
$this->get('mailer')->renderAndSend(
    'email/notification.html.twig',
    array('event' => $event),
    array(
        $this->container->getParameter('log_dir').'/traces/'.$event->getId(),
        $this->container->getParameter('log_dir').'/console_output/'.$event->getId(),
    )
);

If the attachments array is associative, the keys are used as the file names:

$this->get('mailer')->renderAndSend(
    'email/notification.html.twig',
    array('event' => $event),
    array(
        'trace.txt'  => $this->container->getParameter('log_dir').'/traces/'.$event->getId(),
        'output.txt' => $this->container->getParameter('log_dir').'/console_output/'.$event->getId(),
    )
);
@fabpot
Copy link
Member

fabpot commented Jun 22, 2015

That should be either part of Swiftmailer directory or the Swiftmailer bundle, but that cannot be in Symfony itself. I think I'd prefer something in Swiftmailer directly as it would be useful outside the context of Symfony as well then.

@javiereguiluz
Copy link
Member Author

@stof do you think someone from the FOS organization could implement this feature based on the TwigSwiftMailer?

@stof
Copy link
Member

stof commented Jun 24, 2015

I'm -1 on rendering Twig blocks directly for each part of the email though. It has multiple drawbacks (and yes, I know I'm the one implementing this in FOSUserBundle, but I learned more about it when using this at work and the FOSUserBundle implementation actually does not deal with these drawbacks properly):

  • rendering blocks directly bypasses the Twig public API and uses an internal API. This means that Twig will not clear the output buffers properly on errors. this is not an issue in the Form renderer, because this one is called from inside a Twig rendering (you would have issues if using the Twig form renderer from another templating engine, but this is totally crazy). But emails are generally not rendered from inside a template, and so an error during the rendering would mess up the output buffers and the partial render of the email would then be displayed when outputting your webpage. See https://gist.github.com/stof/c5aac17a5049dacc4ec3 for the needed code to handle it properly.
  • as it bypasses the public API, global variables need to be injected manually too (fortunately, recent Twig versions have an easy way to do that)
  • using multiple blocks in the same template are a pain for auto-escaping, as each block would need a different configuration: the HTML body should be escaped as HTML, but the subject and the text body should not if you don't want to mess them entirely. This means that you have to remember to wrap the content of these 2 blocks in {% autoescape false %} all the time to avoid issues.

My idea for these points (not yet implemented in my work project) would be to change the API to give a template name prefix only (which would target a folder) and then having multiple templates (body.html.twig, body.txt.twig, subject.txt.twig) in it for the different parts. The autoescaping would then be handled through the normal filename-based guessing and the rendering would use the normal Twig API (btw, it would also avoid having to use an {% embed %} tag in all my body_html blocks to be able to use a common layout, simplifying the templates).

Another thing is that I think this should not be implemented directly in Swiftmailer. This should probably be done in a different service than the mailer, which would build on top of Swiftmailer and Twig_Environment.

In any case, this is something I want to experiment with during the summer. I will post feedback about it while working on it (it will probably have to wait until after July 14th though, after my vacations, as I doubt I will have time to do this before leaving)

@javiereguiluz
Copy link
Member Author

@stof thanks for the detailed explanation. I won't comment about technical details, but about DX details.

  1. "My idea [...] would be to give a template name prefix only (which would target a folder) and then having multiple templates (body.html.twig, body.txt.twig, subject.txt.twig) in it for the different parts."

I don't like this idea at all. Having to create a template just for the subject of the email will make me to never use this mechanism.

  1. "This means that you have to remember to wrap the content of these 2 blocks in {% autoescape false %} all the time to avoid issues."

I don't agree. Developer doesn't have to deal with this. The body and body_html blocks contain HTML contents and the rest of the blocks contain raw text. The developer must agree to this behavior and he/she cannot change it.

  1. The idea of this new mechanism is to provide a convention-based system to send emails rendering Twig templates. It shouldn't be a generic system. It shouldn't be possible to change its well-defined behavior. Its "limitness" is what it makes it useful and improves DX.

@Richtermeister
Copy link
Contributor

Glad to see this discussion happening, since I too struggle with this and usually end up implementing a FOS-inspired Twig Mailer with mixed results.

Just to add to this discussion, regardless of how the actual template/email is rendered, it would be nice to see some overall abstraction of the process. I think Sylius did a great job there:

https://github.com/Sylius/Sylius/tree/master/src/Sylius/Component/Mailer
https://github.com/Sylius/Sylius/tree/master/src/Sylius/Bundle/MailerBundle

@rvanlaak
Copy link
Contributor

Big +1 on this one, did also have to implement an abstraction layer to solve attachments and i18n.

@stof 's point on Twig's error handling in my opinion is one of the biggest concerns I can't overcome right now.

I don't see why you should use Twig blocks for from/to/subject, and think that also could be solved using an OptionsResolver but don't know or you want to add an extra dependency.

https://github.com/lexik/LexikMailerBundle also does a nice job on abstracting the mailings.

@mickaelandrieu
Copy link
Contributor

possible duplicate of symfony/swiftmailer-bundle#108

@sstok
Copy link
Contributor

sstok commented Jul 6, 2015

possible duplicate of symfony/swiftmailer-bundle#108

The issue in the SwiftmailerBundle repository is younger so its not a duplicate :) other issue is a duplicate.

@mickaelandrieu
Copy link
Contributor

@sstok OK, I close the other issue :)

@stof
Copy link
Member

stof commented Aug 13, 2015

I don't agree. Developer doesn't have to deal with this. The body and body_html blocks contain HTML contents and the rest of the blocks contain raw text. The developer must agree to this behavior and he/she cannot change it.

@javiereguiluz If your plain-text email contains HTML, you have a huge issue.

@javiereguiluz javiereguiluz added the DX DX = Developer eXperience (anything that improves the experience of using Symfony) label Jan 26, 2016
@TomasVotruba
Copy link
Contributor

👍 Often used feature

@javiereguiluz
Copy link
Member Author

I'm closing it because this won't be part of the Symfony core (it could be part of Swiftmailer as Fabien said) and it would be really hard to implement (as Stof showed). Thank you all for the discussion.

@TomasVotruba
Copy link
Contributor

@javiereguiluz Thank you for opening the discussion and moving a bit further.

@javiereguiluz
Copy link
Member Author

If any of you are still interested in this idea, we're giving it a shot at swiftmailer/swiftmailer#845 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony)
Projects
None yet
Development

No branches or pull requests

8 participants