Merge pull request #281 from gaussian1/fix-280

Add Notification Eager Load Support
fenos · Jul 19, 2017 · 31e9341 · 31e9341
2 parents 875e199 + 51a2ea4
commit 31e9341
Show file tree

Hide file tree

Showing 10 changed files with 178 additions and 17 deletions.
diff --git a/src/Notifynder/Builder/Notification.php b/src/Notifynder/Builder/Notification.php
@@ -165,7 +165,7 @@ public function getText()
             $notification = new ModelNotification($this);
             $notifynderParse = new NotificationParser();
 
-            return $notifynderParse->parse($notification, $this->category_id);
+            return $notifynderParse->parse($notification);
         }
     }
 

diff --git a/src/Notifynder/Collections/Config.php b/src/Notifynder/Collections/Config.php
@@ -114,6 +114,15 @@ public function set($key, $value = null)
         app('config')->set('notifynder.'.$key, $value);
     }
 
+    /**
+     * @param string $key
+     */
+    public function forget($key)
+    {
+        Arr::forget($this->items, $key);
+        app('config')->offsetUnset('notifynder.'.$key);
+    }
+
     public function reload()
     {
         $this->items = app('config')->get('notifynder');

diff --git a/src/Notifynder/Models/Notification.php b/src/Notifynder/Models/Notification.php
@@ -132,7 +132,7 @@ public function getTextAttribute()
     {
         if (! array_key_exists('text', $this->attributes)) {
             $notifynderParse = new NotificationParser();
-            $this->attributes['text'] = $notifynderParse->parse($this, $this->category_id);
+            $this->attributes['text'] = $notifynderParse->parse($this);
         }
 
         return $this->attributes['text'];

diff --git a/src/Notifynder/Parsers/NotificationParser.php b/src/Notifynder/Parsers/NotificationParser.php
@@ -4,8 +4,8 @@
 
 use Fenos\Notifynder\Models\NotificationCategory;
 use Fenos\Notifynder\Exceptions\ExtraParamsException;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Fenos\Notifynder\Models\Notification as ModelNotification;
-use Fenos\Notifynder\Builder\Notification as BuilderNotification;
 
 /**
  * Class NotificationParser.
@@ -20,14 +20,18 @@ class NotificationParser
     /**
      * Parse a notification and return the body text.
      *
-     * @param array|ModelNotification|BuilderNotification $notification
-     * @param int $categoryId
+     * @param ModelNotification $notification
      * @return string
      * @throws ExtraParamsException
      */
-    public function parse($notification, $categoryId)
+    public function parse($notification)
     {
-        $category = NotificationCategory::findOrFail($categoryId);
+        $category = $notification->category;
+        if (is_null($category)) {
+            throw (new ModelNotFoundException)->setModel(
+                NotificationCategory::class, $notification->category_id
+            );
+        }
         $text = $category->template_body;
 
         $specialValues = $this->getValues($text);

diff --git a/src/Notifynder/Traits/Notifable.php b/src/Notifynder/Traits/Notifable.php
@@ -49,11 +49,11 @@ public function notifications()
     }
 
     /**
-     * Get the notifications Relationship.
+     * Get the notifications Relationship without any eager loading.
      *
      * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany
      */
-    public function getNotificationRelation()
+    public function getLazyNotificationRelation()
     {
         return $this->notifications();
     }

diff --git a/src/Notifynder/Traits/NotifableBasic.php b/src/Notifynder/Traits/NotifableBasic.php
@@ -9,12 +9,34 @@
  */
 trait NotifableBasic
 {
+    /**
+     * Get the notifications Relationship without any eager loading.
+     *
+     * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany
+     */
+    abstract public function getLazyNotificationRelation();
+
     /**
      * Get the notifications Relationship.
      *
+     * @param array|bool $eagerLoad
      * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany
      */
-    abstract public function getNotificationRelation();
+    public function getNotificationRelation($eagerLoad = null)
+    {
+        $with = [];
+        if (is_null($eagerLoad)) {
+            $eagerLoad = notifynder_config('eager_load', false);
+        }
+
+        if ($eagerLoad === true) {
+            $with = ['category', 'from', 'to']; // all relations
+        } elseif (is_array($eagerLoad)) {
+            $with = $eagerLoad;
+        }
+
+        return $this->getLazyNotificationRelation()->with($with);
+    }
 
     /**
      * Get a new NotifynderManager instance with the given category.
@@ -79,10 +101,10 @@ public function unreadNotification($notification)
     protected function updateSingleReadStatus($notification, $value)
     {
         if (! TypeChecker::isNotification($notification, false)) {
-            $notification = $this->getNotificationRelation()->findOrFail($notification);
+            $notification = $this->getLazyNotificationRelation()->findOrFail($notification);
         }
 
-        if ($this->getNotificationRelation()->where($notification->getKeyName(), $notification->getKey())->exists()) {
+        if ($this->getLazyNotificationRelation()->where($notification->getKeyName(), $notification->getKey())->exists()) {
             return $value ? $notification->read() : $notification->unread();
         }
 
@@ -96,7 +118,7 @@ protected function updateSingleReadStatus($notification, $value)
      */
     public function readAllNotifications()
     {
-        return $this->getNotificationRelation()->update(['read' => 1]);
+        return $this->getLazyNotificationRelation()->update(['read' => 1]);
     }
 
     /**
@@ -106,7 +128,7 @@ public function readAllNotifications()
      */
     public function unreadAllNotifications()
     {
-        return $this->getNotificationRelation()->update(['read' => 0]);
+        return $this->getLazyNotificationRelation()->update(['read' => 0]);
     }
 
     /**
@@ -116,7 +138,7 @@ public function unreadAllNotifications()
      */
     public function countUnreadNotifications()
     {
-        return $this->getNotificationRelation()->byRead(0)->count();
+        return $this->getLazyNotificationRelation()->byRead(0)->count();
     }
 
     /**

diff --git a/src/Notifynder/Traits/NotifableLaravel53.php b/src/Notifynder/Traits/NotifableLaravel53.php
@@ -49,11 +49,11 @@ public function notifynderNotifications()
     }
 
     /**
-     * Get the notifications Relationship.
+     * Get the notifications Relationship without any eager loading.
      *
      * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany
      */
-    public function getNotificationRelation()
+    public function getLazyNotificationRelation()
     {
         return $this->notifynderNotifications();
     }

diff --git a/src/config/notifynder.php b/src/config/notifynder.php
@@ -15,6 +15,18 @@
      */
     'model' => 'App\User',
 
+    /*
+     * Do you want notifynder to eager load its related models?
+     * just swap the value to true to eager load relations
+     * or you can specify which relations to eager load
+     *
+     * Possible Values:
+     * false: No relations are eager loaded (default)
+     * true: All relations are eager loaded
+     * array: Specify which relations to eager load can be any combination of ['category', 'from', 'to']
+     */
+    'eager_load' => false,
+
     /*
      * Do you want have notifynder that work polymorphically?
      * just swap the value to true and you will able to use it!

diff --git a/tests/integration/Parsers/NotificationParserTest.php b/tests/integration/Parsers/NotificationParserTest.php
@@ -0,0 +1,22 @@
+<?php
+
+use Fenos\Notifynder\Builder\Notification;
+use Fenos\Notifynder\Parsers\NotificationParser;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
+
+class NotificationParserTest extends NotifynderTestCase
+{
+    public function testParseThrowsModelNotFoundExceptionIfCategoryIsNull()
+    {
+        $from = $this->createUser();
+        $to = $this->createUser();
+        $notification = new Notification();
+        $notification->set('category_id', null);
+        $notification->set('from_id', $from->getKey());
+        $notification->set('to_id', $to->getKey());
+
+        $this->expectException(ModelNotFoundException::class);
+        $parser = new NotificationParser();
+        $parser->parse($notification);
+    }
+}
diff --git a/tests/integration/Traits/NotifableEagerLoadingTest.php b/tests/integration/Traits/NotifableEagerLoadingTest.php
@@ -0,0 +1,92 @@
+<?php
+
+class NotifableEagerLoadingTest extends NotifynderTestCase
+{
+    public function testGetLazyNotificationRelationDoesnotEagerLoad()
+    {
+        $user = $this->createUser();
+        $this->sendNotificationTo($user);
+
+        $notification = $user->getLazyNotificationRelation()->first();
+        $this->assertModelHasNoLoadedRelations($notification, ['category', 'from', 'to']);
+    }
+
+    public function testGetNotificationRelationReadsConfigurationParameterIfNothingIsPassed()
+    {
+        $user = $this->createUser();
+        $this->sendNotificationTo($user);
+
+        $notification = $user->getNotificationRelation()->first();
+        $this->assertModelHasNoLoadedRelations($notification, ['category', 'from', 'to']);
+    }
+
+    public function testGetNotificationRelationCanEagerLoadAllRelationsIfTrueIsPassed()
+    {
+        $user = $this->createUser();
+        $this->sendNotificationTo($user);
+
+        $notification = $user->getNotificationRelation(true)->first();
+
+        $this->assertModelHasLoadedRelations($notification, ['category', 'from', 'to']);
+    }
+
+    public function testGetNotificationRelationCanEagerLoadASubsetOfRelationsIfAnArrayIsPassed()
+    {
+        $user = $this->createUser();
+        $this->sendNotificationTo($user);
+
+        $notification = $user->getNotificationRelation(['category'])->first();
+
+        $this->assertModelHasLoadedRelations($notification, ['category']);
+        $this->assertModelHasNoLoadedRelations($notification, ['from', 'to']);
+    }
+
+    public function testGetNotificationRelationDoesnotEagerLoadIfConfigurationParameterIsMissing()
+    {
+        $user = $this->createUser();
+        $this->sendNotificationTo($user);
+
+        $config = app('notifynder.config');
+        $config->forget('eager_load');
+
+        $notification = $user->getNotificationRelation()->first();
+        $this->assertModelHasNoLoadedRelations($notification, ['category', 'from', 'to']);
+    }
+
+    public function testGetNotificationsReadsEagerLoadConfigurationParameter()
+    {
+        $user = $this->createUser();
+        $this->sendNotificationTo($user);
+
+        $config = app('notifynder.config');
+
+        $config->set('eager_load', true);
+        $notifications = $user->getNotifications();
+        $this->assertModelHasLoadedRelations($notifications[0], ['category', 'from', 'to']);
+
+        $config->set('eager_load', false);
+        $notifications = $user->getNotifications();
+        $this->assertModelHasNoLoadedRelations($notifications[0], ['category', 'from', 'to']);
+
+        $config->set('eager_load', ['category', 'from']);
+        $notifications = $user->getNotifications();
+        $this->assertModelHasLoadedRelations($notifications[0], ['category', 'from']);
+        $this->assertModelHasNoLoadedRelations($notifications[0], ['to']);
+    }
+
+    protected function assertModelHasLoadedRelations($model, $relationNames = [])
+    {
+        $modelLoadedRelations = $model->getRelations();
+        foreach ($relationNames as $relationName) {
+            $this->assertArrayHasKey($relationName, $modelLoadedRelations, $relationName.' relation was not eager loaded');
+        }
+    }
+
+    protected function assertModelHasNoLoadedRelations($model, $relationNames = [])
+    {
+        $modelLoadedRelations = $model->getRelations();
+        foreach ($relationNames as $relationName) {
+            $this->assertArrayNotHasKey($relationName, $modelLoadedRelations, $relationName.' relation was eager loaded');
+        }
+    }
+}