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

[5.6] Allow passing of recipient name in Mail notifications #24606

Merged
merged 3 commits into from
Jun 15, 2018
Merged

[5.6] Allow passing of recipient name in Mail notifications #24606

merged 3 commits into from
Jun 15, 2018

Conversation

Propaganistas
Copy link
Contributor

@Propaganistas Propaganistas commented Jun 15, 2018

MailChannel only passes an email address to the Mailer when sending mail notifications. It's a common situation to also know the notifiable's name and hence it seems appropriate to include this as the recipient's name in order to gain trust:

hello@example.org
   vs
John Doe <hello@example.org>

This rather small fix allows mail notifications to include the recipient name by using common syntax in the routeNotificationForMail() method:

/**
 * Route notifications for the Mail channel.
 *
 * @param  \Illuminate\Notifications\Notification  $notification
 * @return string
 */
public function routeNotificationForMail($notification)
{
    return [
        $this->email => $this->name,
    ];
}

Multiple addresses remain supported, with or without a set name:

/**
 * Route notifications for the Mail channel.
 *
 * @param  \Illuminate\Notifications\Notification  $notification
 * @return string
 */
public function routeNotificationForMail($notification)
{
    return [
        $this->email => $this->name,
        $this->email_2,
        $this->email_3 => $this->otherName,
    ];
}

@taylorotwell taylorotwell merged commit d5a7ba8 into laravel:5.6 Jun 15, 2018
PaegleK pushed a commit to PaegleK/framework that referenced this pull request Jul 24, 2018
PR laravel#24606 unfortunately broke on-demand notifications addressed to multiple recipients.
This fixes Issue  laravel#24729 and laravel#24956
taylorotwell pushed a commit that referenced this pull request Jul 26, 2018
* Fix multiple notification recipients

PR #24606 unfortunately broke on-demand notifications addressed to multiple recipients.
This fixes Issue  #24729 and #24956

* Added test for multiple mail recipients.

* Remove unnecessary spaces.
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.

None yet

2 participants