diff --git a/js/forum/src/components/discussion-renamed-notification.js b/js/forum/src/components/discussion-renamed-notification.js index 77290a1da..067288a04 100644 --- a/js/forum/src/components/discussion-renamed-notification.js +++ b/js/forum/src/components/discussion-renamed-notification.js @@ -8,7 +8,7 @@ export default class DiscussionRenamedNotification extends Notification { return super.view({ href: app.route.discussion(notification.subject(), notification.content().postNumber), icon: 'pencil', - content: [username(notification.sender()), ' renamed'] + content: [username(notification.sender()), ' changed the title'] }); } } diff --git a/migrations/2015_02_24_000000_create_notifications_table.php b/migrations/2015_02_24_000000_create_notifications_table.php index aea969118..830797196 100644 --- a/migrations/2015_02_24_000000_create_notifications_table.php +++ b/migrations/2015_02_24_000000_create_notifications_table.php @@ -14,7 +14,7 @@ class CreateNotificationsTable extends Migration public function up() { Schema::create('notifications', function (Blueprint $table) { - + $table->increments('id'); $table->integer('user_id')->unsigned(); $table->integer('sender_id')->unsigned()->nullable(); @@ -24,6 +24,7 @@ class CreateNotificationsTable extends Migration $table->binary('data')->nullable(); $table->dateTime('time'); $table->boolean('is_read')->default(0); + $table->boolean('is_deleted')->default(0); }); } diff --git a/src/Api/Actions/Notifications/IndexAction.php b/src/Api/Actions/Notifications/IndexAction.php index 8bc24129a..4a23c05aa 100644 --- a/src/Api/Actions/Notifications/IndexAction.php +++ b/src/Api/Actions/Notifications/IndexAction.php @@ -73,6 +73,6 @@ class IndexAction extends SerializeCollectionAction $user->markNotificationsAsRead()->save(); - return $this->notifications->findByUser($user->id, $request->limit, $request->offset); + return $this->notifications->findByUser($user, $request->limit, $request->offset); } } diff --git a/src/Api/Serializers/BaseSerializer.php b/src/Api/Serializers/BaseSerializer.php index 846992a0d..6fe6a1487 100644 --- a/src/Api/Serializers/BaseSerializer.php +++ b/src/Api/Serializers/BaseSerializer.php @@ -64,9 +64,8 @@ abstract class BaseSerializer extends SerializerAbstract } } - if (is_array($serializer)) { - $class = get_class(is_object($data) ? $data : $model->$relation()->getRelated()); - $serializer = $serializer[$class]; + if ($serializer instanceof Closure) { + $serializer = $serializer($model, $data); } $serializer = new $serializer($this->actor, $links); return $many ? $serializer->collection($data) : $serializer->resource($data); diff --git a/src/Api/Serializers/NotificationSerializer.php b/src/Api/Serializers/NotificationSerializer.php index f3b57fac4..fc21ccaf9 100644 --- a/src/Api/Serializers/NotificationSerializer.php +++ b/src/Api/Serializers/NotificationSerializer.php @@ -4,10 +4,21 @@ class NotificationSerializer extends BaseSerializer { /** * The resource type. + * * @var string */ protected $type = 'notifications'; + /** + * A map of notification types (key) to the serializer that should be used + * to output the notification's subject (value). + * + * @var array + */ + public static $subjects = [ + 'discussionRenamed' => 'Flarum\Api\Serializers\DiscussionBasicSerializer' + ]; + /** * Serialize attributes of an notification model for JSON output. * @@ -40,9 +51,8 @@ class NotificationSerializer extends BaseSerializer public function subject() { - return $this->hasOne([ - 'Flarum\Core\Models\Discussion' => 'Flarum\Api\Serializers\DiscussionSerializer', - 'Flarum\Core\Models\CommentPost' => 'Flarum\Api\Serializers\PostSerializer' - ]); + return $this->hasOne(function ($notification) { + return static::$subjects[$notification->type]; + }); } } diff --git a/src/Core/CoreServiceProvider.php b/src/Core/CoreServiceProvider.php index e91d3ec89..32d792960 100644 --- a/src/Core/CoreServiceProvider.php +++ b/src/Core/CoreServiceProvider.php @@ -16,6 +16,7 @@ use League\Flysystem\Adapter\Local; use Flarum\Core\Events\RegisterDiscussionGambits; use Flarum\Core\Events\RegisterUserGambits; use Flarum\Extend\Permission; +use Flarum\Extend\NotificationType; class CoreServiceProvider extends ServiceProvider { @@ -43,6 +44,12 @@ class CoreServiceProvider extends ServiceProvider 'Flarum\Core\Handlers\Commands' ); }); + + $events->subscribe('Flarum\Core\Handlers\Events\DiscussionRenamedNotifier'); + $this->extend( + (new NotificationType('Flarum\Core\Notifications\DiscussionRenamedNotification', 'Flarum\Api\Serializers\DiscussionBasicSerializer')) + ->enableByDefault('alert'), + ); } /** @@ -52,8 +59,6 @@ class CoreServiceProvider extends ServiceProvider */ public function register() { - $this->app->register('Flarum\Core\Notifications\NotificationServiceProvider'); - // Register a singleton entity that represents this forum. This entity // will be used to check for global forum permissions (like viewing the // forum, registering, and starting discussions.) @@ -89,6 +94,11 @@ class CoreServiceProvider extends ServiceProvider $this->app->when('Flarum\Core\Handlers\Commands\DeleteAvatarCommandHandler') ->needs('League\Flysystem\FilesystemInterface') ->give($avatarFilesystem); + + $this->app->bind( + 'Flarum\Core\Repositories\NotificationRepositoryInterface', + 'Flarum\Core\Repositories\EloquentNotificationRepository' + ); } public function registerGambits() diff --git a/src/Core/Handlers/Commands/PostReplyCommandHandler.php b/src/Core/Handlers/Commands/PostReplyCommandHandler.php index 68b39b80d..f930c2625 100644 --- a/src/Core/Handlers/Commands/PostReplyCommandHandler.php +++ b/src/Core/Handlers/Commands/PostReplyCommandHandler.php @@ -4,7 +4,7 @@ use Flarum\Core\Events\PostWillBeSaved; use Flarum\Core\Repositories\DiscussionRepositoryInterface as DiscussionRepository; use Flarum\Core\Models\CommentPost; use Flarum\Core\Support\DispatchesEvents; -use Flarum\Core\Notifications\Notifier; +use Flarum\Core\Notifications\NotificationSyncer; class PostReplyCommandHandler { @@ -12,12 +12,12 @@ class PostReplyCommandHandler protected $discussions; - protected $notifier; + protected $notifications; - public function __construct(DiscussionRepository $discussions, Notifier $notifier) + public function __construct(DiscussionRepository $discussions, NotificationSyncer $notifications) { $this->discussions = $discussions; - $this->notifier = $notifier; + $this->notifications = $notifications; } public function handle($command) @@ -46,7 +46,7 @@ class PostReplyCommandHandler $post->save(); - $this->notifier->onePerUser(function () use ($post) { + $this->notifications->onePerUser(function () use ($post) { $this->dispatchEventsFor($post); }); diff --git a/src/Core/Handlers/Events/DiscussionRenamedNotifier.php b/src/Core/Handlers/Events/DiscussionRenamedNotifier.php index 79342cc1b..1833f471a 100755 --- a/src/Core/Handlers/Events/DiscussionRenamedNotifier.php +++ b/src/Core/Handlers/Events/DiscussionRenamedNotifier.php @@ -2,15 +2,17 @@ use Flarum\Core\Events\DiscussionWasRenamed; use Flarum\Core\Models\DiscussionRenamedPost; -use Flarum\Core\Notifications\Types\DiscussionRenamedNotification; -use Flarum\Core\Notifications\Notifier; +use Flarum\Core\Notifications\DiscussionRenamedNotification; +use Flarum\Core\Notifications\NotificationSyncer; use Illuminate\Contracts\Events\Dispatcher; class DiscussionRenamedNotifier { - public function __construct(Notifier $notifier) + protected $notifications; + + public function __construct(NotificationSyncer $notifications) { - $this->notifier = $notifier; + $this->notifications = $notifications; } /** @@ -25,28 +27,19 @@ class DiscussionRenamedNotifier public function whenDiscussionWasRenamed(DiscussionWasRenamed $event) { - $post = $this->createPost($event); - - $post = $event->discussion->addPost($post); - - if ($event->discussion->start_user_id !== $event->user->id) { - $notification = new DiscussionRenamedNotification($event->discussion, $post->user, $post); - - if ($post->exists) { - $this->notifier->send($notification, [$post->discussion->startUser]); - } else { - $this->notifier->retract($notification); - } - } - } - - protected function createPost(DiscussionWasRenamed $event) - { - return DiscussionRenamedPost::reply( + $post = DiscussionRenamedPost::reply( $event->discussion->id, $event->user->id, $event->oldTitle, $event->discussion->title ); + + $post = $event->discussion->addPost($post); + + if ($event->discussion->start_user_id !== $event->user->id) { + $notification = new DiscussionRenamedNotification($post); + + $this->notifications->sync($notification, $post->exists ? [$event->discussion->startUser] : []); + } } } diff --git a/src/Core/Models/User.php b/src/Core/Models/User.php index 03dfe9e3c..0bedb6ef9 100755 --- a/src/Core/Models/User.php +++ b/src/Core/Models/User.php @@ -327,7 +327,13 @@ class User extends Model public function getUnreadNotificationsCount() { - return $this->notifications()->where('time', '>', $this->notification_read_time ?: 0)->where('is_read', 0)->count(\DB::raw('DISTINCT type, subject_id')); + $types = array_keys(Notification::getTypes()); + + return $this->notifications() + ->whereIn('type', array_filter($types, [$this, 'shouldAlert'])) + ->where('time', '>', $this->notification_read_time ?: 0) + ->where('is_read', 0) + ->count(\DB::raw('DISTINCT type, subject_id')); } public function getPreferencesAttribute($value) @@ -354,14 +360,19 @@ class User extends Model ]; } - public static function notificationPreferenceKey($type, $sender) + public static function notificationPreferenceKey($type, $method) { - return 'notify_'.$type.'_'.$sender; + return 'notify_'.$type.'_'.$method; } - public function shouldNotify($type, $method) + public function shouldAlert($type) { - return $this->preference(static::notificationPreferenceKey($type, $method)); + return $this->preference(static::notificationPreferenceKey($type, 'alert')); + } + + public function shouldEmail($type) + { + return $this->preference(static::notificationPreferenceKey($type, 'email')); } public function preference($key, $default = null) diff --git a/src/Core/Notifications/DiscussionRenamedNotification.php b/src/Core/Notifications/DiscussionRenamedNotification.php new file mode 100644 index 000000000..e047826b7 --- /dev/null +++ b/src/Core/Notifications/DiscussionRenamedNotification.php @@ -0,0 +1,38 @@ +post = $post; + } + + public function getSubject() + { + return $this->post->discussion; + } + + public function getSender() + { + return $this->post->user; + } + + public function getData() + { + return ['postNumber' => (int) $this->post->number]; + } + + public static function getType() + { + return 'discussionRenamed'; + } + + public static function getSubjectModel() + { + return 'Flarum\Core\Models\Discussion'; + } +} diff --git a/src/Core/Notifications/NotificationAbstract.php b/src/Core/Notifications/NotificationAbstract.php new file mode 100644 index 000000000..72a67f3c3 --- /dev/null +++ b/src/Core/Notifications/NotificationAbstract.php @@ -0,0 +1,54 @@ +forum = $forum; } - public function send(Notification $notification, User $user) + public function send(NotificationInterface $notification, User $user) { $this->mailer->send( $notification->getEmailView(), @@ -25,9 +23,4 @@ class NotificationEmailer implements NotificationSender } ); } - - public static function compatibleWith($class) - { - return (new ReflectionClass($class))->implementsInterface('Flarum\Core\Notifications\Types\EmailableNotification'); - } } diff --git a/src/Core/Notifications/NotificationServiceProvider.php b/src/Core/Notifications/NotificationServiceProvider.php deleted file mode 100644 index 918ab2dbc..000000000 --- a/src/Core/Notifications/NotificationServiceProvider.php +++ /dev/null @@ -1,38 +0,0 @@ -subscribe('Flarum\Core\Handlers\Events\DiscussionRenamedNotifier'); - - $notifier->registerMethod('alert', 'Flarum\Core\Notifications\Senders\NotificationAlerter'); - $notifier->registerMethod('email', 'Flarum\Core\Notifications\Senders\NotificationEmailer'); - - $this->extend( - (new NotificationType('Flarum\Core\Notifications\Types\DiscussionRenamedNotification'))->enableByDefault('alert') - ); - } - - public function register() - { - $this->app->bind( - 'Flarum\Core\Repositories\NotificationRepositoryInterface', - 'Flarum\Core\Repositories\EloquentNotificationRepository' - ); - - $this->app->singleton('Flarum\Core\Notifications\Notifier'); - $this->app->alias('Flarum\Core\Notifications\Notifier', 'flarum.notifier'); - } -} diff --git a/src/Core/Notifications/NotificationSyncer.php b/src/Core/Notifications/NotificationSyncer.php new file mode 100644 index 000000000..7ca70e389 --- /dev/null +++ b/src/Core/Notifications/NotificationSyncer.php @@ -0,0 +1,92 @@ +notifications = $notifications; + $this->mailer = $mailer; + } + + /** + * Sync a notification so that it is visible to the specified users, and not + * visible to anyone else. If it is being made visible for the first time, + * attempt to send the user an email. + * + * @param \Flarum\Core\Notifications\NotificationInterface $notification + * @param \Flarum\Core\Models\User[] $users + * @return void + */ + public function sync(NotificationInterface $notification, array $users) + { + $attributes = [ + 'type' => $notification::getType(), + 'sender_id' => $notification->getSender()->id, + 'subject_id' => $notification->getSubject()->id, + 'data' => ($data = $notification->getData()) ? json_encode($data) : null + ]; + + $toDelete = Notification::where($attributes)->get(); + $toUndelete = []; + $newRecipients = []; + + foreach ($users as $user) { + $existing = $toDelete->where('user_id', $user->id)->first(); + + if (($k = $toDelete->search($existing)) !== false) { + $toUndelete[] = $existing->id; + $toDelete->pull($k); + } elseif (! $this->onePerUser || ! in_array($user->id, $this->sentTo)) { + $newRecipients[] = $user; + $this->sentTo[] = $user->id; + } + } + + if (count($toDelete)) { + Notification::whereIn('id', $toDelete->lists('id'))->update(['is_deleted' => true]); + } + + if (count($toUndelete)) { + Notification::whereIn('id', $toUndelete)->update(['is_deleted' => false]); + } + + if (count($newRecipients)) { + $now = Carbon::now('utc')->toDateTimeString(); + + Notification::insert( + array_map(function ($user) use ($attributes, $notification, $now) { + return $attributes + ['user_id' => $user->id, 'time' => $now]; + }, $newRecipients) + ); + + foreach ($newRecipients as $user) { + if ($user->shouldEmail($notification::getType())) { + $this->mailer->send($notification, $user); + } + } + } + } + + public function onePerUser(Closure $callback) + { + $this->sentTo = []; + $this->onePerUser = true; + + $callback(); + + $this->onePerUser = false; + } +} diff --git a/src/Core/Notifications/Notifier.php b/src/Core/Notifications/Notifier.php deleted file mode 100644 index 7e692f095..000000000 --- a/src/Core/Notifications/Notifier.php +++ /dev/null @@ -1,86 +0,0 @@ -container = $container; - } - - public function send(Notification $notification, array $users) - { - foreach ($users as $user) { - if ($this->onePerUser && in_array($user->id, $this->sentTo)) { - continue; - } - - foreach ($this->methods as $method => $sender) { - $sender = $this->container->make($sender); - - if ($sender::compatibleWith($notification) && - $user->shouldNotify($notification::getType(), $method)) { - $sender->send($notification, $user); - $this->sentTo[] = $user->id; - } - } - } - } - - 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); - } - } - } - - public function registerMethod($name, $class) - { - $this->methods[$name] = $class; - } - - public function registerType($class) - { - $this->types[] = $class; - } - - public function getMethods() - { - return $this->methods; - } - - public function getTypes() - { - return $this->types; - } - - public function onePerUser(Closure $callback) - { - $this->sentTo = []; - $this->onePerUser = true; - - $callback(); - - $this->onePerUser = false; - } -} diff --git a/src/Core/Notifications/Senders/NotificationAlerter.php b/src/Core/Notifications/Senders/NotificationAlerter.php deleted file mode 100644 index 9d02fab48..000000000 --- a/src/Core/Notifications/Senders/NotificationAlerter.php +++ /dev/null @@ -1,34 +0,0 @@ -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/NotificationSender.php b/src/Core/Notifications/Senders/NotificationSender.php deleted file mode 100644 index a94abab6d..000000000 --- a/src/Core/Notifications/Senders/NotificationSender.php +++ /dev/null @@ -1,11 +0,0 @@ -discussion = $discussion; - $this->sender = $sender; - $this->post = $post; - } - - public function getSubject() - { - return $this->discussion; - } - - public function getSender() - { - return $this->sender; - } - - public function getAlertData() - { - return ['postNumber' => $this->post->number]; - } - - public static function getType() - { - return 'discussionRenamed'; - } - - public static function getSubjectModel() - { - return 'Flarum\Core\Models\Discussion'; - } -} diff --git a/src/Core/Notifications/Types/EmailableNotification.php b/src/Core/Notifications/Types/EmailableNotification.php deleted file mode 100644 index 17691c24d..000000000 --- a/src/Core/Notifications/Types/EmailableNotification.php +++ /dev/null @@ -1,8 +0,0 @@ -where('user_id', $userId) - ->whereIn('type', array_keys(Notification::getTypes())) + ->where('user_id', $user->id) + ->whereIn('type', array_filter(array_keys(Notification::getTypes()), [$user, 'shouldAlert'])) + ->where('is_deleted', false) ->groupBy('type', 'subject_id') ->orderBy('time', 'desc') ->skip($offset) diff --git a/src/Core/Repositories/NotificationRepositoryInterface.php b/src/Core/Repositories/NotificationRepositoryInterface.php index cf7678ecd..d23f5d8f1 100644 --- a/src/Core/Repositories/NotificationRepositoryInterface.php +++ b/src/Core/Repositories/NotificationRepositoryInterface.php @@ -1,6 +1,8 @@ class = $class; + $this->serializer = $serializer; } public function enableByDefault($method) @@ -24,17 +28,16 @@ class NotificationType implements ExtenderInterface public function extend(Application $app) { - $notifier = $app['flarum.notifier']; $class = $this->class; - $notifier->registerType($class); - Notification::registerType($class); - foreach ($notifier->getMethods() as $method => $sender) { - if ($sender::compatibleWith($class)) { - User::registerPreference(User::notificationPreferenceKey($class::getType(), $method), 'boolval', in_array($method, $this->enabled)); - } + User::registerPreference(User::notificationPreferenceKey($class::getType(), 'alert'), 'boolval', in_array('alert', $this->enabled)); + + if ($class::isEmailable()) { + User::registerPreference(User::notificationPreferenceKey($class::getType(), 'email'), 'boolval', in_array('email', $this->enabled)); } + + NotificationSerializer::$subjects[$class::getType()] = $this->serializer; } }