From 287ce2fddd8cefc0b030b9d498eb638359c9cb6c Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Thu, 3 Dec 2015 15:10:05 +1030 Subject: [PATCH] Fix crash when loading notifications in some instances Specifically, the crash would occur when the first notification had a subject without a discussion relationship (e.g. the Subscriptions extension's newPost notification, where the subject itself was a discussion). Instead of simply eager loading the nested subject.discussion relationship, we load discussions manually instead. --- CHANGELOG.md | 1 + .../ListNotificationsController.php | 33 ++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e286550fb..58fd986e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Fix error when sorting discussions by "oldest" (#627) - Fix composer preview button on mobile (#196) - Enable "Start a Discussion" button if global permissions are restricted but tag-specific permissions are granted (#640) +- Fix crash when loading notifications in some instances - Improve composer appearance/usability on mobile - Show "reply" action in discussion menu on mobile - Fix some issues with dropdown positioning diff --git a/src/Api/Controller/ListNotificationsController.php b/src/Api/Controller/ListNotificationsController.php index 86fb2d69e..861c0b655 100644 --- a/src/Api/Controller/ListNotificationsController.php +++ b/src/Api/Controller/ListNotificationsController.php @@ -10,6 +10,7 @@ namespace Flarum\Api\Controller; +use Flarum\Core\Discussion; use Flarum\Core\Repository\NotificationRepository; use Flarum\Core\Exception\PermissionDeniedException; use Psr\Http\Message\ServerRequestInterface; @@ -66,6 +67,36 @@ class ListNotificationsController extends AbstractCollectionController $offset = $this->extractOffset($request); $include = $this->extractInclude($request); - return $this->notifications->findByUser($actor, $limit, $offset)->load($include); + $notifications = $this->notifications->findByUser($actor, $limit, $offset) + ->load(array_diff($include, ['subject.discussion'])) + ->all(); + + if (in_array('subject.discussion', $include)) { + $this->loadSubjectDiscussions($notifications); + } + + return $notifications; + } + + /** + * @param \Flarum\Core\Notification[] $notifications + */ + private function loadSubjectDiscussions(array $notifications) + { + $ids = []; + + foreach ($notifications as $notification) { + if ($notification->subject && $notification->subject->discussion_id) { + $ids[] = $notification->subject->discussion_id; + } + } + + $discussions = Discussion::find(array_unique($ids)); + + foreach ($notifications as $notification) { + if ($notification->subject && $notification->subject->discussion_id) { + $notification->subject->setRelation('discussion', $discussions->find($notification->subject->discussion_id)); + } + } } }