laravel / framework

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

[5.4] Why not a Illuminate\Mail\Events\MessageSent event? #18602

Closed CyrilMazur closed 7 years ago

CyrilMazur commented 7 years ago

Description:

There is a Illuminate\Mail\Events\MessageSending event that's triggered before the mailer attempts to send an email. Why not also triggering a Illuminate\Mail\Events\MessageSent event after the email is sent?

I think the use case in the example from the official doc is a bit clumsy (https://laravel.com/docs/5.4/mail#events), because the listener for logging the message is executed whether the email is actually sent or not. There could be an error while actually sending the email (ex: network issue, temporary API error from Mandrill / SparkPost etc...) and MessageSending would still be triggered. However, it would be guaranteed that MessageSent is actually triggered only if the email was successfully sent.

On top of that, we could access the message id (SES) / transmission id (SparkPost) that's set into the swift message's headers AFTER the email is sent, allowing us to do nice things like this:

public function handle(MessageSent $event)
{
    $sparkpostId = $event->message->getHeaders()->get('X-SparkPost-Transmission-ID');

    // log message with sparkpost id, useful for processing webhooks later
}
themsaid commented 7 years ago

Can you please use https://github.com/laravel/internals for such suggestions?

We keep this repo for bug reporting only.

notflip commented 7 years ago

Is this implemented yet? Would be amazing!

devcircus commented 7 years ago

Looks like it's here: https://github.com/laravel/framework/tree/5.5/src/Illuminate/Mail/Events

notflip commented 7 years ago

I upgraded from 5.2 and It's not included.. That's odd.

CyrilMazur commented 7 years ago

Yes https://github.com/laravel/framework/commit/9a16e9f625e8b31072aaa7cb9a70f0f6e88456f4