Framework: Mailables & Notifications (Mailchannel) lack display names in the recipient header.

Created on 31 Jan 2017 · 3Comments · Source: laravel/framework

Laravel Version: 5.3, 5.4
PHP Version: 7.1
Database Driver & Version: MySQL

Description:

There is a discrepancy in how various mailing methods handle the recipients display name.

Many spam filters treat mail without the whole "Firstname Lastname \

We have noticed that important account Notifications & Mailables sometimes end up in spam folders, because the recipient field only includes a mail address without the display name.

Steps To Reproduce:

These types of calls offer no method to set the recipients display name, their
->to($email)/routeNotificationForMail() methods only take an email address as a parameter.

Mail::to($user)->queue(new SomeMailable($parameters));
$user->notify(new SomeNotification($parameters));

For Mailables, I can work around that fact by using ->to($email, $name) on the Mailable itself instead of on the Mail facade:

$mailable = (new SomeMailable(parameters))->to($user->email, $user->getFullName());
Mail::queue($mailable);

For consistency, I think the routeNotificationFor method and to method on the MailableMailer/Mailer should offer a way to return an extra name string, just like how it works for to method on the Mailable class.

Source

okdewit

👍12

Most helpful comment

It's actually quite easy to fix, all is needed to change getRecipients from MailChannel and make it return array like [[[email protected]' => 'Recipient Name One']] Swift setTo allows to pass array of this type of data:

/**
 * Get the recipients of the given message.
 *
 * @param  mixed  $notifiable
 * @param  \Illuminate\Notifications\Messages\MailMessage  $message
 * @return mixed
 */
protected function getRecipients($notifiable, $message)
{
    if (is_string($recipients = $notifiable->routeNotificationFor('mail'))) {
        $recipients = [$recipients];
    }

    return collect($recipients)->map(function ($recipient) {
        if (is_array($recipient)) {
            return $recipient;
        }

        return is_string($recipient) ? $recipient : $recipient->email;
    })->all();
}

And then any model can have:

/**
 * Get user's email addess with name for sending emails
 *
 * @return array
 */
public function routeNotificationForMail()
{
    return [$this->email => $this->full_name];
}

The only problem is that it's quite hard to override MailChannel class, it will require to create own MailChannel, ChannelManager, NotificationServiceProvider, basically just to extend one method getRecipients.

Update: Actually it doesn't even require to override getRecipients, just add routeNotificationForMail and it just works, because map collections method iterates over values only and keeps keys the same, so it returns the same array returned from routeNotificationForMail.

vedmant on 19 Jun 2017

👍3

All 3 comments

/**
 * Get the recipients of the given message.
 *
 * @param  mixed  $notifiable
 * @param  \Illuminate\Notifications\Messages\MailMessage  $message
 * @return mixed
 */
protected function getRecipients($notifiable, $message)
{
    if (is_string($recipients = $notifiable->routeNotificationFor('mail'))) {
        $recipients = [$recipients];
    }

    return collect($recipients)->map(function ($recipient) {
        if (is_array($recipient)) {
            return $recipient;
        }

        return is_string($recipient) ? $recipient : $recipient->email;
    })->all();
}

And then any model can have:

/**
 * Get user's email addess with name for sending emails
 *
 * @return array
 */
public function routeNotificationForMail()
{
    return [$this->email => $this->full_name];
}

vedmant on 19 Jun 2017

👍3

Any updates on this?

Dylan-DPC on 11 Oct 2017

@vedmant judging from the current code this is already possible with the current syntax. So if you're experiencing this issue just define that routeNotificationForMail method on your model.

https://github.com/laravel/framework/blob/a64a900607f110bd365a0265dddd8278badfbed8/src/Illuminate/Notifications/Channels/MailChannel.php#L198-L209

We probably won't do this by default because we can't "assume" the model has a name attribute. The notifiable trait could be used in much scenarios where the model in question doesn't has a name or uses a default attribute. The only assumption we make is an email attribute.