Mailer: add inline css and extend send() method without callback

[Mevrael]

1) Right now each time to send an email you have to write many extra lines of code, like:

        Mail::send('emails.reminder', ['user' => $user], function ($m) use ($user) {
            $m->from('hello@app.com', 'Your Application');

            $m->to($user->email, $user->name)->subject('Your Reminder!');
        });

While it could be something like Mail::send($user->email, 'reminder')

2) You have to manually control each mail's subject and pass it to closure while it could be stored in lang/en/mail.php where key = template name

3) Each email template requires inline CSS and since maintaining it is a pain, there are some composer packages could be used to automatically inline css from file.

My current working codebase:

I have extended Mailer with 2 new methods:

First one

    public function sendTo($email_to, $view, array $data = []) // data is optional now
    {
        $subject = Lang::get('mail.'.$view); // automatically get subject by template name

        parent::send(
            'emails.' . $view,
            $data,
            function ($message) use ($email_to, $subject) {
                $message->to($email_to);
                $message->subject($subject);
            }
        );
    }

$message->from() is set in mail config 'from' => ['address' => 'hello@app.com', 'name' => 'Your Application'],

second method extends getView() and parses resources/assets/css/email.css and makes inline CSS via package Pelago\Emogrifier

    protected function getView($view, $data)
    {
        $content = $this->views->make($view, $data)->render();
        $css = file_get_contents(resource_path('assets/css/email.css'));
        $inliner = new Emogrifier();
        $inliner->setCss($css);
        $inliner->setHtml($content);
        $content = $inliner->emogrify();

        return $content;
    }

4) I also added new ENV variable in mail config to allow each developer set his own email for testing

    'to' => [
        'address' => env('MAIL_FORCE_TO_ADDRESS', null),
        'name' => null
    ],

[brayniverse]

I like the idea of not having to rewrite the callback every time, it would make it easier to maintain when you are sending the same emails from multiple locations within your application.

After reading your proposal I thought about what syntax would offer the most flexibility and this is what I came up with.

class ReminderEmail extends Email
{
    protected $user;
    protected $data;

    public function __construct(User $user, array $data = [])
    {
        $this->user = $user;
        $this->data = $data;
    }

    protected function view()
    {
        return view('emails.reminder', $this->data);
    }

    protected function message(Message $message)
    {
        $message->to($this->user->email, $this->user->name);

        $message->subject(trans('emails.reminder.subject'));
    }
}

The view() method allows you to use your library to inject the inline CSS.

Mail::send(new ReminderEmail($user, ['foo' => 'bar']));

In theory implementing this would not require us to break Laravel's current functionality because we could check whether the first argument of Mail::send() is an instance of Illuminate\Mailer\Email. By default we could create an abstract Email class in App\Emails which we could setup some sensible defaults in.

I must admit that I haven't looked into how the Illuminate\Mail\Mailer class works and don't know if this is possible but I am open to discussing it further, and am happy to prepare a PR implementing it (if people see any value in it).

[Mevrael]

You can't restrict sending mails only to $user model. What if you just have a subscribers/email list? To send an email you need only 2 parts:

1. e-mail, where to send
2. mail itself, subject and contents linked together

I like the idea of making callback easily configurable and ability to reuse same email from any part of applciation, however, not sure you realy need to make a new class for each type of email. I'm sure everything can be much simpler.

You don't need to define simple callbacks each time for each type of email, that why I suggested to add it in Mailer's core itself. You can call reminder email easily by it's ID (= view name in emails folder and = key in lang/en/emails.php) with Mail::send($email, 'reminder', $data);

For next email you just create new view in emails folder and subject for it in lang folder.

In cases where you will need very specific logic, probably, you could use callback which will be optional in that case Mail::send($email, 'reminder', $data = [], $callback = null);

Or we realy could define custom methods in Mailer like

Mail::sendReminder($email, $data=[])->withFile($file);
Mail::sendOrderCompleted($email, $data=[])->cc($admin_email);

[crynobone]

Each email template requires inline CSS and since maintaining it is a pain, there are some composer packages could be used to automatically inline css from file.

Wouldn't the current event available for Mailer enough for this.

    /**
     * The event handler mappings for the application.
     *
     * @var array
     */
    protected $listen = [
        \Illuminate\Mail\Events\MessageSending::class => [CssInliner::class],
    ];

and use something such as https://github.com/orchestral/notifier/blob/master/src/Events/CssInliner.php

[brayniverse]

@Mevrael with my proposal the constructor is optional and would be used to inject any dependencies like the user object, an email address as a string, or even any attachments you want to include.

The Mail::sendReminder(...) syntax feels very familiar if you have used Laravel's Fluent DB syntax; User::whereEmail('foo@bar.com'), which is nice. However you are relying on users following a convention when naming their views and lang files. A more configurable solution would be better, maybe you could tie it all up in a MailProvider?

What do you think about what @crynobone is recommending? I don't have anything setup to test this out myself, could you decorate the message using the MessageSending event?

[tomschlick]

100% for what @brayniverse suggested in his first comment.

This level of complexity should be done with classes so you can use polymorphism to build yourself a base class and extend upon that. Checking if the first parameter is an object that implements an interface should be the cleanest way to go about it.

This is basically how I have structured my newsletter / notification system. It builds everything up in classes and then finally passes it all through the closures like normal.

[tomschlick]

@Mevrael / @brayniverse have either of you created a pull request or have working code for this yet? If not I might try to take care of it. Seems like a good addition, especially with the syntax @brayniverse came up with.

[brayniverse]

@tomschlick I wasn't certain if what I suggested would be useful to enough people. If the conversation continued and started looking that way, I was prepared to submit a PR, however if you are then please feel free to submit one yourself. Let me know you need any help.

[Mevrael]

@tomschlick @brayniverse Sorry I don't have much free time right now and the all code example I have is in 1st post. I still would like to hear more opinions and see code examples here instead of making PR so we could discuss implementations. It's also easier to do it here, also avoiding closing PRs.

[brayniverse]

It looks as though we may be getting some changes to the Mailer implementation, in 5.3, in the shape of reusable Mail classes. I haven't closely reviewed the changes yet, so I can't tell if this will solve your problem @Mevrael, but it's interesting nonetheless.

/cc @tomschlick

[tomschlick]

Awesome! Can't wait to use that implementation in 5.3. Thanks @taylorotwell

[taylorotwell]

Indeed. I will be showing a full demo of new mail things off at Laracon next week.

[tomschlick]

This can probably be closed now that we have this in 5.3! 👍