From e4e9282b6e2bb2f2168b29a87eae3c44887c3623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Klabbers?= Date: Mon, 10 Feb 2020 11:45:35 +0100 Subject: [PATCH] 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, eg 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. --- .../Job/SendEmailNotificationJob.php | 40 +++++++++++++++++++ src/Notification/Job/SendNotificationsJob.php | 24 ++++------- src/Notification/NotificationSyncer.php | 10 +++++ 3 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 src/Notification/Job/SendEmailNotificationJob.php diff --git a/src/Notification/Job/SendEmailNotificationJob.php b/src/Notification/Job/SendEmailNotificationJob.php new file mode 100644 index 00000000000..1595ddd99a1 --- /dev/null +++ b/src/Notification/Job/SendEmailNotificationJob.php @@ -0,0 +1,40 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Flarum\Notification\Job; + +use Flarum\Notification\Blueprint\BlueprintInterface; +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); + } +} diff --git a/src/Notification/Job/SendNotificationsJob.php b/src/Notification/Job/SendNotificationsJob.php index 9c8848982e9..461ee2121d6 100644 --- a/src/Notification/Job/SendNotificationsJob.php +++ b/src/Notification/Job/SendNotificationsJob.php @@ -17,7 +17,6 @@ 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; @@ -28,20 +27,20 @@ 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; + $recipients = $this->recipients; event(new Sending($this->blueprint, $recipients)); @@ -59,16 +58,7 @@ public function handle(NotificationMailer $mailer) 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); - } + $this->chain([new SendEmailNotificationJob($this->blueprint, $recipients)]); } } } diff --git a/src/Notification/NotificationSyncer.php b/src/Notification/NotificationSyncer.php index 5d8e58803da..88d3860990f 100644 --- a/src/Notification/NotificationSyncer.php +++ b/src/Notification/NotificationSyncer.php @@ -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; @@ -112,6 +113,15 @@ public function sync(Blueprint\BlueprintInterface $blueprint, array $users) if (count($newRecipients)) { $this->queue->push(new SendNotificationsJob($blueprint, $newRecipients)); } + + if ($blueprint instanceof MailableInterface) { + /** @var User $recipient */ + foreach($newRecipients as $recipient) { + if ($recipient->shouldEmail($blueprint::getType())) { + $this->queue->push(new SendEmailNotificationJob($blueprint, $recipient)); + } + } + } } /**