Skip to content

Commit

Permalink
Remove to and cc from mail notfiication.
Browse files Browse the repository at this point in the history 
It doesn’t make any sense to have these on notifications. If you need
this you probably should be using Markdown based mailable classes in
Laravel 5.4.
  • Loading branch information
@taylorotwell
taylorotwell committed Dec 20, 2016
1 parent 2efc953 commit ff68549
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 50 deletions.
19 changes: 9 additions & 10 deletions src/Illuminate/Notifications/Channels/MailChannel.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Closure;
use Illuminate\Support\Arr;
use Illuminate\Support\Str;
use Illuminate\Support\Collection;
use Illuminate\Contracts\Mail\Mailer;
use Illuminate\Contracts\Mail\Mailable;
use Illuminate\Notifications\Notification;
Expand Down Expand Up @@ -116,15 +117,7 @@ protected function addressMessage($mailMessage, $notifiable, $message)
{
$this->addSender($mailMessage, $message);

if (is_array($recipients = $this->getRecipients($notifiable, $message))) {
$mailMessage->bcc($recipients);
} else {
$mailMessage->to($recipients);
}

if ($message->cc) {
$mailMessage->cc($message->cc);
}
$mailMessage->bcc($this->getRecipients($notifiable, $message));

This comment has been minimized.

Sign in to view

Copy link
@antonioribeiro

antonioribeiro Jan 16, 2017

Contributor

Have you tried to send bcc only e-mails via Mailgun? I'm pretty sure it's not possible.

[2017-01-16 18:45:06] local.ERROR: GuzzleHttp\Exception\ClientException: Client error: `POST https://api.mailgun.net/v3/nucleoconstelar.com/messages.mime` resulted in a `400 BAD REQUEST` response:
{
  "message": "'to' parameter is not a valid address. please check documentation"
}
}

/**
Expand Down Expand Up @@ -154,7 +147,13 @@ protected function addSender($mailMessage, $message)
*/
protected function getRecipients($notifiable, $message)
{
return empty($message->to) ? $notifiable->routeNotificationFor('mail') : $message->to;
if (is_string($recipients = $notifiable->routeNotificationFor('mail'))) {
$recipients = [$recipients];
}

return collect($recipients)->map(function ($recipient) {
return is_string($recipient) ? $recipient : $recipient->email;
})->all();
}

/**
Expand Down
40 changes: 0 additions & 40 deletions src/Illuminate/Notifications/Messages/MailMessage.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,6 @@ class MailMessage extends SimpleMessage
*/
public $from = [];

/**
* The recipient information for the message.
*
* @var array
*/
public $to = [];

/**
* The "cc" recipients of the message.
*
* @var array
*/
public $cc = [];

/**
* The "reply to" information for the message.
*
Expand Down Expand Up @@ -122,32 +108,6 @@ public function from($address, $name = null)
return $this;
}

/**
* Set the recipient address for the mail message.
*
* @param string|array $address
* @return $this
*/

This comment has been minimized.

Sign in to view

Copy link
@antonkomarev

antonkomarev Mar 24, 2017 •
edited
Loading

Contributor

@taylorotwell Is there any valuable reasons to drop to method? I was used it to send verification email. In my application user don't have primary email until it isn't approved.

return (new MailMessage)
            ->to($this->email)
            ->subject('Email address verification')
            ->action('Verify Email', $actionUrl);

Note: If anybody really need to keep this functionality - you can always just extend MailMessage class and add to method to it or create Mailable class.

This comment has been minimized.

Sign in to view

Copy link
@okdewit

okdewit Mar 28, 2017 •
edited
Loading

I think this should at least be documented in the upgrade guide, we were using it in a similar scenario (verifying changes in email addresses).

@a-komarev: I don't think extending MailMessage works, because getRecipients is called from MailChannel.

This comment has been minimized.

Sign in to view

Copy link
@antonkomarev

antonkomarev Mar 29, 2017

Contributor

@okdewit It's working in my case.

This comment has been minimized.

Sign in to view

Copy link
@okdewit

okdewit Apr 1, 2017 •
edited
Loading

@a-komarev I've solved it in my case by just setting $notifiable->email inside the handler in the Notification class:

public function __construct($newmail)
{
    $this->newmail = $newmail
}

public function toMail(User $notifiable): MailMessage
{
    $notifiable->email = $this->newemail;

    return (new MailMessage)
        ->subject('Email address verification')
        ->action('Verify Email', $actionUrl);
}

The underlying mailer picks up the new address, but I don't persist it to the $user->email until verification with token comes back from the $actionUrl.

This comment has been minimized.

Sign in to view

Copy link
@antonkomarev

antonkomarev Apr 1, 2017

Contributor

@okdewit nice hack 👍 thanks for sharing!
public function to($address)
{
$this->to = $address;

return $this;
}

/**
* Set the recipients of the message.
*
* @param string|array $address
* @return $this
*/
public function cc($address)
{
$this->cc = $address;

return $this;
}

/**
* Set the "reply to" address of the message.
*
Expand Down

0 comments on commit ff68549

Please sign in to comment.