From e0aa99fabbc0fe31d5762c5fadd468707a652829 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Tue, 29 Sep 2015 16:41:05 +0930 Subject: [PATCH] Properly mark all notifications as read Previously, clicking the "mark all notifications as read" button would individually mark each of the visible notifications as read. Since we now always show a badge with the number of unread notifications, we need to make sure that all notifications (not just the visible ones) can be marked as read. Otherwise it would be possible to get stuck with an unread badge there. This commit adds a new API endpoint which marks *all* of a user's notifications as read. The JSON-API spec doesn't cover this kind of thing (updating all instances of a certain resource type), so I'm a bit unsure regarding what the endpoint should actually be. For now I've gone with POST /notifications/read, but I'm open to suggestions. ref #500 --- js/forum/src/components/NotificationList.js | 9 ++-- .../Actions/Notifications/ReadAllAction.php | 45 ++++++++++++++++++ src/Api/ApiServiceProvider.php | 7 +++ .../Commands/ReadAllNotifications.php | 31 +++++++++++++ .../Commands/ReadAllNotificationsHandler.php | 46 +++++++++++++++++++ .../Notifications/NotificationRepository.php | 12 +++++ 6 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 src/Api/Actions/Notifications/ReadAllAction.php create mode 100644 src/Core/Notifications/Commands/ReadAllNotifications.php create mode 100644 src/Core/Notifications/Commands/ReadAllNotificationsHandler.php diff --git a/js/forum/src/components/NotificationList.js b/js/forum/src/components/NotificationList.js index 3d24a1168..8ba947458 100644 --- a/js/forum/src/components/NotificationList.js +++ b/js/forum/src/components/NotificationList.js @@ -134,10 +134,11 @@ export default class NotificationList extends Component { app.session.user.pushAttributes({unreadNotificationsCount: 0}); - app.cache.notifications.forEach(notification => { - if (!notification.isRead()) { - notification.save({isRead: true}); - } + app.cache.notifications.forEach(notification => notification.pushAttributes({isRead: true})); + + app.request({ + url: app.forum.attribute('apiUrl') + '/notifications/read', + method: 'POST' }); } } diff --git a/src/Api/Actions/Notifications/ReadAllAction.php b/src/Api/Actions/Notifications/ReadAllAction.php new file mode 100644 index 000000000..496c93924 --- /dev/null +++ b/src/Api/Actions/Notifications/ReadAllAction.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Flarum\Api\Actions\Notifications; + +use Flarum\Core\Notifications\Commands\ReadAllNotifications; +use Illuminate\Contracts\Bus\Dispatcher; +use Flarum\Api\Request; +use Flarum\Api\Actions\DeleteAction; + +class ReadAllAction extends DeleteAction +{ + /** + * @var Dispatcher + */ + protected $bus; + + /** + * @param Dispatcher $bus + */ + public function __construct(Dispatcher $bus) + { + $this->bus = $bus; + } + + /** + * Mark all notifications as read. + * + * @param Request $request + * @return void + */ + protected function delete(Request $request) + { + $this->bus->dispatch( + new ReadAllNotifications($request->actor) + ); + } +} diff --git a/src/Api/ApiServiceProvider.php b/src/Api/ApiServiceProvider.php index d40ba9b9f..6c5dc3004 100644 --- a/src/Api/ApiServiceProvider.php +++ b/src/Api/ApiServiceProvider.php @@ -178,6 +178,13 @@ class ApiServiceProvider extends ServiceProvider $this->action('Flarum\Api\Actions\Notifications\IndexAction') ); + // Mark all notifications as read + $routes->post( + '/notifications/read', + 'flarum.api.notifications.readAll', + $this->action('Flarum\Api\Actions\Notifications\ReadAllAction') + ); + // Mark a single notification as read $routes->patch( '/notifications/{id}', diff --git a/src/Core/Notifications/Commands/ReadAllNotifications.php b/src/Core/Notifications/Commands/ReadAllNotifications.php new file mode 100644 index 000000000..85147e633 --- /dev/null +++ b/src/Core/Notifications/Commands/ReadAllNotifications.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Flarum\Core\Notifications\Commands; + +use Flarum\Core\Users\User; + +class ReadAllNotifications +{ + /** + * The user performing the action. + * + * @var User + */ + public $actor; + + /** + * @param User $actor The user performing the action. + */ + public function __construct(User $actor) + { + $this->actor = $actor; + } +} diff --git a/src/Core/Notifications/Commands/ReadAllNotificationsHandler.php b/src/Core/Notifications/Commands/ReadAllNotificationsHandler.php new file mode 100644 index 000000000..61bb52c8c --- /dev/null +++ b/src/Core/Notifications/Commands/ReadAllNotificationsHandler.php @@ -0,0 +1,46 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Flarum\Core\Notifications\Commands; + +use Flarum\Core\Notifications\Notification; +use Flarum\Core\Notifications\NotificationRepository; + +class ReadAllNotificationsHandler +{ + /** + * @var NotificationRepository + */ + protected $notifications; + + /** + * @param NotificationRepository $notifications + */ + public function __construct(NotificationRepository $notifications) + { + $this->notifications = $notifications; + } + + /** + * @param ReadAllNotifications $command + * + * @return void + */ + public function handle(ReadAllNotifications $command) + { + $actor = $command->actor; + + if ($actor->isGuest()) { + throw new PermissionDeniedException; + } + + $this->notifications->markAllAsRead($actor); + } +} diff --git a/src/Core/Notifications/NotificationRepository.php b/src/Core/Notifications/NotificationRepository.php index bad0f2e26..f71f1bf62 100644 --- a/src/Core/Notifications/NotificationRepository.php +++ b/src/Core/Notifications/NotificationRepository.php @@ -42,4 +42,16 @@ class NotificationRepository ->latest('time') ->get(); } + + /** + * Mark all of a user's notifications as read. + * + * @param User $user + * + * @return void + */ + public function markAllAsRead(User $user) + { + Notification::where('user_id', $user->id)->update(['is_read' => true]); + } }