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.3] Support custom recipient(s) with MailMessage #15100

Merged
merged 1 commit into from
Aug 29, 2016

Conversation

tillkruss
Copy link
Collaborator

@tillkruss tillkruss commented Aug 27, 2016

This comes in handy when sending "email address confirmation" requests.

@tillkruss tillkruss changed the title Support custom recipient(s) with MailMessage [5.3] Support custom recipient(s) with MailMessage Aug 27, 2016
@spyric
Copy link
Contributor

spyric commented Aug 28, 2016

Why don't you want to add routeNotificationForMail function to your Notifiable model?

@tillkruss
Copy link
Collaborator Author

It's quite an effort to override the returned email address of routeNotificationForMail() and just calling to() when needed is much easier.

@taylorotwell
Copy link
Member

Aren't you sending to a user object? I still don't understand why using the email address on the user object wouldn't work fine?

@tillkruss
Copy link
Collaborator Author

I often encounter this workflow:

  • The user has an email address: john@doe.com
  • The user changes his email address to: johndoe@example.com
  • The email address in the users table remains as john@doe.com until the user opens the link that has been sent to johndoe@example.com

Using routeNotificationForMail() on the Notifiable object doesn't allow me to send a notification to the "unverified" email address, only to the current one.

@taylorotwell taylorotwell merged commit e6063d7 into laravel:5.3 Aug 29, 2016
@tillkruss tillkruss deleted the mail-channel-to branch August 29, 2016 14:24
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

4 participants