laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.51k stars 11.02k forks source link

Mailable in MailChannel notifications #18265

Closed Mevrael closed 7 years ago

Mevrael commented 7 years ago

Since this PR: https://github.com/laravel/framework/pull/15318/commits/41eccb8d947d8072b0521c5c9cabe2adbc002cc9

Mailable can be used in MailChannel, however, per design it's not fully fits the way how notification works. I am also not sure how it should work.

@butschster have you though about how $user->email should be used with Mailable?

Currently I have to put ->to() manually, while in all other Notification drivers and MailChannel itself ->to() should be taken from the $notifiable User automatically.

    public function toMail(User $notifiable)
    {
        return (new InvoiceSentMail($this->invoice))
            ->to($notifiable->routeNotificationForMail()) // should work without that line?
            ->subject($this->subject);
    }

In that PR ->send() is just called directly on Mailable not taking $notifiable into account.

So the main question of this issue is - should everyone be able to return Mailable in toMail() without specifying recipient and use $notifiable automatically like it is with other Notification API?

themsaid commented 7 years ago

yes you need to tell the mailer where to send the email to, you can do it via to, bcc, cc, or a combination of all of them, I don't think having a default behaviour is desired.

Closing since it's not a bug report, you can suggest a default behaviour via a PR or use the internals repo to post your suggestion and see if we can come up with a good flow.