From 3925e5892c7e9fe9671f99c84cdafe56e4a83480 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Thu, 14 May 2015 22:39:57 +0930 Subject: [PATCH] Rework notifications architecture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The recipient(s) are the concern of the notifier/sender, not the notification itself - Allow “retraction” of notifications (e.g. if a discussion is stickied, but then it is unstickied) - Misc. cleanup --- .../Events/DiscussionRenamedNotifier.php | 22 +++++-------- src/Core/Models/Notification.php | 2 +- src/Core/Notifications/Notifier.php | 23 +++++++++++-- .../Senders/NotificationAlerter.php | 15 +++++++-- .../Senders/NotificationEmailer.php | 6 ++-- .../Senders/NotificationSender.php | 3 +- .../Senders/RetractableSender.php | 8 +++++ .../Types/AlertableNotification.php | 8 ----- .../Types/DiscussionRenamedNotification.php | 26 ++++++++------- src/Core/Notifications/Types/Notification.php | 32 +------------------ .../EloquentNotificationRepository.php | 6 ++-- 11 files changed, 73 insertions(+), 78 deletions(-) create mode 100644 src/Core/Notifications/Senders/RetractableSender.php delete mode 100644 src/Core/Notifications/Types/AlertableNotification.php diff --git a/src/Core/Handlers/Events/DiscussionRenamedNotifier.php b/src/Core/Handlers/Events/DiscussionRenamedNotifier.php index ad37a8e98..79342cc1b 100755 --- a/src/Core/Handlers/Events/DiscussionRenamedNotifier.php +++ b/src/Core/Handlers/Events/DiscussionRenamedNotifier.php @@ -27,10 +27,16 @@ class DiscussionRenamedNotifier { $post = $this->createPost($event); - $event->discussion->addPost($post); + $post = $event->discussion->addPost($post); if ($event->discussion->start_user_id !== $event->user->id) { - $this->sendNotification($event, $post); + $notification = new DiscussionRenamedNotification($event->discussion, $post->user, $post); + + if ($post->exists) { + $this->notifier->send($notification, [$post->discussion->startUser]); + } else { + $this->notifier->retract($notification); + } } } @@ -43,16 +49,4 @@ class DiscussionRenamedNotifier $event->discussion->title ); } - - protected function sendNotification(DiscussionWasRenamed $event, DiscussionRenamedPost $post) - { - $notification = new DiscussionRenamedNotification( - $event->discussion->startUser, - $event->user, - $post, - $event->oldTitle - ); - - $this->notifier->send($notification); - } } diff --git a/src/Core/Models/Notification.php b/src/Core/Models/Notification.php index 1aab33e72..ba9de28ed 100644 --- a/src/Core/Models/Notification.php +++ b/src/Core/Models/Notification.php @@ -60,7 +60,7 @@ class Notification extends Model */ public function setDataAttribute($value) { - $this->attributes['data'] = json_encode($value); + $this->attributes['data'] = $value ? json_encode($value) : null; } /** diff --git a/src/Core/Notifications/Notifier.php b/src/Core/Notifications/Notifier.php index 66bc0be85..401040896 100644 --- a/src/Core/Notifications/Notifier.php +++ b/src/Core/Notifications/Notifier.php @@ -1,6 +1,7 @@ container = $container; } - public function send(Notification $notification) + public function send(Notification $notification, array $users) { foreach ($this->methods as $method => $sender) { $sender = $this->container->make($sender); - if ($notification->getRecipient()->shouldNotify($notification::getType(), $method) && $sender::compatibleWith($notification)) { - $sender->send($notification); + + if ($sender::compatibleWith($notification)) { + foreach ($users as $user) { + if ($user->shouldNotify($notification::getType(), $method)) { + $sender->send($notification, $user); + } + } + } + } + } + + public function retract(Notification $notification) + { + foreach ($this->methods as $method => $sender) { + $sender = $this->container->make($sender); + + if ($sender instanceof RetractableSender && $sender::compatibleWith($notification)) { + $sender->retract($notification); } } } diff --git a/src/Core/Notifications/Senders/NotificationAlerter.php b/src/Core/Notifications/Senders/NotificationAlerter.php index 5cecf714e..9d02fab48 100644 --- a/src/Core/Notifications/Senders/NotificationAlerter.php +++ b/src/Core/Notifications/Senders/NotificationAlerter.php @@ -2,22 +2,31 @@ use Flarum\Core\Notifications\Types\Notification; use Flarum\Core\Models\Notification as NotificationModel; +use Flarum\Core\Models\User; use ReflectionClass; -class NotificationAlerter implements NotificationSender +class NotificationAlerter implements NotificationSender, RetractableSender { - public function send(Notification $notification) + public function send(Notification $notification, User $user) { $model = NotificationModel::alert( - $notification->getRecipient()->id, + $user->id, $notification::getType(), $notification->getSender()->id, $notification->getSubject()->id, $notification->getAlertData() ); + $model->save(); } + public function retract(Notification $notification) + { + $models = NotificationModel::where('type', $notification::getType()) + ->where('subject_id', $notification->getSubject()->id) + ->delete(); + } + public static function compatibleWith($className) { return (new ReflectionClass($className))->implementsInterface('Flarum\Core\Notifications\Types\AlertableNotification'); diff --git a/src/Core/Notifications/Senders/NotificationEmailer.php b/src/Core/Notifications/Senders/NotificationEmailer.php index ff24467ab..c903b3e09 100644 --- a/src/Core/Notifications/Senders/NotificationEmailer.php +++ b/src/Core/Notifications/Senders/NotificationEmailer.php @@ -1,6 +1,7 @@ forum = $forum; } - public function send(Notification $notification) + public function send(Notification $notification, User $user) { - $this->mailer->send($notification->getEmailView(), ['notification' => $notification], function ($message) use ($notification) { - $recipient = $notification->getRecipient(); + $this->mailer->send($notification->getEmailView(), ['notification' => $notification], function ($message) use ($notification, $recipient) { $message->to($recipient->email, $recipient->username) ->subject('['.$this->forum->title.'] '.$notification->getEmailSubject()); }); diff --git a/src/Core/Notifications/Senders/NotificationSender.php b/src/Core/Notifications/Senders/NotificationSender.php index 7b36faba0..a94abab6d 100644 --- a/src/Core/Notifications/Senders/NotificationSender.php +++ b/src/Core/Notifications/Senders/NotificationSender.php @@ -1,10 +1,11 @@ discussion = $discussion; + $this->sender = $sender; $this->post = $post; - $this->oldTitle = $oldTitle; - - parent::__construct($recipient, $sender); } public function getSubject() { - return $this->post->discussion; + return $this->discussion; + } + + public function getSender() + { + return $this->sender; } public function getAlertData() { - return [ - 'number' => $this->post->number, - 'oldTitle' => $this->oldTitle - ]; + return ['postNumber' => $this->post->number]; } public static function getType() diff --git a/src/Core/Notifications/Types/Notification.php b/src/Core/Notifications/Types/Notification.php index de3635b4b..8f2d2d2f8 100644 --- a/src/Core/Notifications/Types/Notification.php +++ b/src/Core/Notifications/Types/Notification.php @@ -1,36 +1,6 @@ recipient = $recipient; - $this->sender = $sender; - } - - public function getRecipient() - { - return $this->recipient; - } - - public function getSender() - { - return $this->sender; - } - - public static function getType() - { - return null; - } - - public static function getSubjectModel() - { - return null; - } + abstract public static function getType(); } diff --git a/src/Core/Repositories/EloquentNotificationRepository.php b/src/Core/Repositories/EloquentNotificationRepository.php index dbe3f990d..fcfed55de 100644 --- a/src/Core/Repositories/EloquentNotificationRepository.php +++ b/src/Core/Repositories/EloquentNotificationRepository.php @@ -5,15 +5,15 @@ use DB; class EloquentNotificationRepository implements NotificationRepositoryInterface { - public function findByUser($userId, $count = null, $start = 0) + public function findByUser($userId, $limit = null, $offset = 0) { $primaries = Notification::select(DB::raw('MAX(id) AS id'), DB::raw('SUM(is_read = 0) AS unread_count')) ->where('user_id', $userId) ->whereIn('type', array_keys(Notification::getTypes())) ->groupBy('type', 'subject_id') ->orderBy('time', 'desc') - ->skip($start) - ->take($count); + ->skip($offset) + ->take($limit); return Notification::with('subject') ->select('notifications.*', 'p.unread_count')