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

[11.x] Allow queueing Mails by default #54271

Draft
wants to merge 1 commit into
base: 11.x
Choose a base branch
from

Conversation

mpociot
Copy link
Contributor

@mpociot mpociot commented Jan 20, 2025

This PR adds the ability to automatically queue all outgoing mailables by default, no matter if they implement the ShouldQueue interface or not.

I think this method can be beneficial to prevent performance degradation (and application errors) by accidentally sending out mails synchronously that should always be sent via the queue.

The API works in a similar way to the DB::prohibitDestructiveCommands() method.

Usage (for example in your service provider):

<?php
class AppServiceProvider extends ServiceProvider
{
    public function boot(): void
    {
        Mail::queueMailsByDefault();
    }
}

Now, calling send will always queue the mail, regardless of the implemented interface.
Of course, calling sendNow remains unaffected.

@taylorotwell
Copy link
Member

There might be a bit more to think about here. 🤔 Should you be able to customize the connection and queue? What about notifications?

@taylorotwell taylorotwell marked this pull request as draft January 20, 2025 22:25
@timacdonald
Copy link
Member

I wonder if this should only impact HTTP requests?

Does it feel weird that mail send in queued jobs them become a new job?

@donnysim
Copy link
Contributor

donnysim commented Jan 21, 2025

I personally often create a job that sends email instead of using Mailables as I often end up requiring more logic that Mailables cannot do or is obscure and the whole Address support mismatch between Mailable and Message. But we do use a direct email send when there's no need for any logic and with this option it would require using sendNow and all helper functions that run in jobs and being constantly aware that this is on as it will result in 2 jobs instead of one, potentially reducing the queues effectiveness. I suppose the solution will be to wrap this in !runningInConsole?

@mpociot
Copy link
Contributor Author

mpociot commented Jan 21, 2025

Yeah I think it makes sense to only enforce queues when the application is not running via the CLI, like a queue worker.

@taylorotwell I like the idea to customize connection and queue. This would make it easy to enforce that all mails go through a specific queue.

The API could be something like:

Mail:: queueMailsByDefault(queue: 'emails', connection: 'sqs');

And if we do this, I think it would only make sense to have this on the Notification facade / NotificationSender as well.

What do you think?

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.

4 participants