Send Form Emails in a separate Job

[okaufmann]

Hi @jasonvarga

We are using many Statamic Forms in a big project. The service we use to send mails is unfortunately very unstable. In the latest version, Statamic sends all E-Mails in a loop by the SendEmails Job.

$this->emailConfigs($this->submission)->each(function ($config) {
    Mail::send(new Email($this->submission, $config, $this->site));
});        

https://github.com/statamic/cms/blob/3.3/src/Forms/SendEmails.php#L29

This is a problem because of the following reasons:

• When a Form has multiple Email configs to send and a single mail fails, the other may not be sent.
• We cannot add any Laravel Job configs like retries or backoff to handle this behaviour.

This PR removes the queuability fromSendEmails and only sends Emails in a new job SendEmail. Further it adds a new config value in the forms namespace to customize the job that is used to send that single mail.

This solves the problems mentioned, as we now can create a new custom SendEmail job class like the following:

<?php

namespace App\Notifications;

use Statamic\Contracts\Forms\Submission;
use Statamic\Forms\SendEmail;
use Statamic\Sites\Site;

class SendStatamicEmail extends SendEmail
{
    public $tries = 3;

    public function __construct(Submission $submission, Site $site, $config)
    {
        parent::__construct($submission, $site, $config);

        $this->queue = 'emails';
    }

    public function backoff()
    {
        return [5, 10, 30];
    }
}

And set it in the config:

<?php

return [
    'send_mail_job' => \App\Notifications\SendStatamicEmail::class,
];

I also thought about config values to set the queue properties from the forms config file but as this will probably not be done much this would blow up the config file unnecessarily.

If you have a better Idea, I'm open to change this PR to match your requirements.

[jasonvarga]

In the latest version, Statamic sends E-Mails all in SendEmails in a loop.

I assume you saw my recent PR. Just to clarify, it's always looped through the email configs and sent the emails in a loop. That was nothing new.

This is a good PR though, thanks!

[okaufmann]

Yes, that was a reason, that I created a PR as the Email sending is now covered by the tests (I will solve the failing tests now).

[okaufmann]

Now the tests are green.

I needed to add another method to get the jobs and be able to call handle on them. Otherwise, dispatch would be called and the tests would be more complex.

What do you think?

[jasonvarga]

I can't push changes to your branch. Can you apply this?

https://gist.github.com/jasonvarga/20c8f6ceff5d1fa0c70f97de449e363c

It changes the SendEmails test to check that the mutiple jobs are dispatched, and adds a new test to check that the single email is sent.

[okaufmann]

I've applied your patch and mentioned you as Co-Author. Thank you for helping me out!

[okaufmann]

If there is anything else I need to do for this PR, please let me know.

[jasonvarga]

Closing in favor of #6481 - Thanks!

[okaufmann]

We cannot find out, why we can't permit you to push to this PR... But thanks for the merge 🎉

[jasonvarga]

It's something to do with the repo being on an organization account. When it's a personal fork I never have trouble pushing. Never worked it out. 🤷

[okaufmann]

Ah! Next time I'll use my personal fork then. Thank you 👌 Have a great weekend 🎉