Moved sending emails to the syncer

This separates sending each individual mail, thus hardening the app.
There are still many improvements possible in this code, e.g. chaining
these commands, making emails just another notification type and
listening to the Notify event instead. We can postpone this to a later
stable release.
This commit is contained in:
Daniël Klabbers 2020-02-10 11:45:35 +01:00 committed by Franz Liedke
parent cd8a8e9dd7
commit 2ba29a9088
No known key found for this signature in database
GPG Key ID: 9A0231A879B055F4
3 changed files with 68 additions and 24 deletions

View File

@ -0,0 +1,39 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Notification\Job;
use Flarum\Notification\MailableInterface;
use Flarum\Notification\NotificationMailer;
use Flarum\Queue\AbstractJob;
use Flarum\User\User;
class SendEmailNotificationJob extends AbstractJob
{
/**
* @var MailableInterface
*/
private $blueprint;
/**
* @var User
*/
private $recipient;
public function __construct(MailableInterface $blueprint, User $recipient)
{
$this->blueprint = $blueprint;
$this->recipient = $recipient;
}
public function handle(NotificationMailer $mailer)
{
$mailer->send($this->blueprint, $this->recipient);
}
}

View File

@ -13,9 +13,7 @@ use Carbon\Carbon;
use Flarum\Notification\Blueprint\BlueprintInterface;
use Flarum\Notification\Event\Notifying;
use Flarum\Notification\Event\Sending;
use Flarum\Notification\MailableInterface;
use Flarum\Notification\Notification;
use Flarum\Notification\NotificationMailer;
use Flarum\Queue\AbstractJob;
use Flarum\User\User;
@ -27,22 +25,21 @@ class SendNotificationsJob extends AbstractJob
private $blueprint;
/**
* @var array
* @var User[]
*/
private $recipientIds;
private $recipients;
public function __construct(BlueprintInterface $blueprint, array $recipientIds = [])
public function __construct(BlueprintInterface $blueprint, array $recipients = [])
{
$this->blueprint = $blueprint;
$this->recipientIds = $recipientIds;
$this->recipients = $recipients;
}
public function handle(NotificationMailer $mailer)
public function handle()
{
$now = Carbon::now('utc')->toDateTimeString();
$recipients = $this->recipientIds;
event(new Sending($this->blueprint, $recipients));
event(new Sending($this->blueprint, $this->recipients));
$attributes = $this->blueprint->getAttributes();
@ -52,22 +49,9 @@ class SendNotificationsJob extends AbstractJob
'user_id' => $user->id,
'created_at' => $now
];
}, $recipients)
}, $this->recipients)
);
event(new Notifying($this->blueprint, $recipients));
if ($this->blueprint instanceof MailableInterface) {
$this->email($mailer, $this->blueprint, $recipients);
}
}
protected function email(NotificationMailer $mailer, MailableInterface $blueprint, array $recipients)
{
foreach ($recipients as $user) {
if ($user->shouldEmail($blueprint::getType())) {
$mailer->send($blueprint, $user);
}
}
}
}

View File

@ -10,6 +10,7 @@
namespace Flarum\Notification;
use Flarum\Notification\Blueprint\BlueprintInterface;
use Flarum\Notification\Job\SendEmailNotificationJob;
use Flarum\Notification\Job\SendNotificationsJob;
use Flarum\User\User;
use Illuminate\Contracts\Queue\Queue;
@ -109,10 +110,15 @@ class NotificationSyncer
// Create a notification record, and send an email, for all users
// receiving this notification for the first time (we know because they
// didn't have a record in the database).
// didn't have a record in the database). As both operations can be
// intensive on resources (database and mail server), we queue them.
if (count($newRecipients)) {
$this->queue->push(new SendNotificationsJob($blueprint, $newRecipients));
}
if ($blueprint instanceof MailableInterface) {
$this->mailNotifications($blueprint, $newRecipients);
}
}
/**
@ -154,6 +160,21 @@ class NotificationSyncer
static::$onePerUser = false;
}
/**
* Mail a notification to a list of users.
*
* @param MailableInterface $blueprint
* @param User[] $recipients
*/
protected function mailNotifications(MailableInterface $blueprint, array $recipients)
{
foreach ($recipients as $user) {
if ($user->shouldEmail($blueprint::getType())) {
$this->queue->push(new SendEmailNotificationJob($blueprint, $user));
}
}
}
/**
* Set the deleted status of a list of notification records.
*