From 05d3859f6f794e7509b44fd367515d45baadec6a Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Sat, 24 Jun 2017 23:58:17 +0200 Subject: [PATCH 01/13] Use Notification model category relation to allow for category eager loading --- src/Notifynder/Parsers/NotificationParser.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Notifynder/Parsers/NotificationParser.php b/src/Notifynder/Parsers/NotificationParser.php index 5d1b183..1bcc0f3 100644 --- a/src/Notifynder/Parsers/NotificationParser.php +++ b/src/Notifynder/Parsers/NotificationParser.php @@ -6,6 +6,7 @@ use Fenos\Notifynder\Exceptions\ExtraParamsException; use Fenos\Notifynder\Models\Notification as ModelNotification; use Fenos\Notifynder\Builder\Notification as BuilderNotification; +use Illuminate\Database\Eloquent\ModelNotFoundException; /** * Class NotificationParser. @@ -20,14 +21,19 @@ class NotificationParser /** * Parse a notification and return the body text. * - * @param array|ModelNotification|BuilderNotification $notification + * @param ModelNotification $notification * @param int $categoryId * @return string * @throws ExtraParamsException */ public function parse($notification, $categoryId) { - $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); From d3ec2cc257712e44b2fe34f9ec4ea62bac52acf5 Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Sun, 25 Jun 2017 00:14:33 +0200 Subject: [PATCH 02/13] Removed unused BuilderNotification import --- src/Notifynder/Parsers/NotificationParser.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Notifynder/Parsers/NotificationParser.php b/src/Notifynder/Parsers/NotificationParser.php index 1bcc0f3..55ab1d2 100644 --- a/src/Notifynder/Parsers/NotificationParser.php +++ b/src/Notifynder/Parsers/NotificationParser.php @@ -2,10 +2,9 @@ namespace Fenos\Notifynder\Parsers; -use Fenos\Notifynder\Models\NotificationCategory; use Fenos\Notifynder\Exceptions\ExtraParamsException; use Fenos\Notifynder\Models\Notification as ModelNotification; -use Fenos\Notifynder\Builder\Notification as BuilderNotification; +use Fenos\Notifynder\Models\NotificationCategory; use Illuminate\Database\Eloquent\ModelNotFoundException; /** From 69dc0aacb02062cd614a70f19078e0ab064af48d Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Sun, 25 Jun 2017 00:16:25 +0200 Subject: [PATCH 03/13] Reorganized imports for styleCI --- src/Notifynder/Parsers/NotificationParser.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Notifynder/Parsers/NotificationParser.php b/src/Notifynder/Parsers/NotificationParser.php index 55ab1d2..b6f3887 100644 --- a/src/Notifynder/Parsers/NotificationParser.php +++ b/src/Notifynder/Parsers/NotificationParser.php @@ -2,10 +2,10 @@ namespace Fenos\Notifynder\Parsers; -use Fenos\Notifynder\Exceptions\ExtraParamsException; -use Fenos\Notifynder\Models\Notification as ModelNotification; use Fenos\Notifynder\Models\NotificationCategory; +use Fenos\Notifynder\Exceptions\ExtraParamsException; use Illuminate\Database\Eloquent\ModelNotFoundException; +use Fenos\Notifynder\Models\Notification as ModelNotification; /** * Class NotificationParser. From 6a34ee082595ecd9d5d23e9cbe06da6707d691cc Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Sun, 25 Jun 2017 09:27:26 +0200 Subject: [PATCH 04/13] Added unit tests --- tests/NotifynderTestCase.php | 11 ++-- .../NotifableCategoryEagerLoadingTest.php | 61 +++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 tests/integration/Traits/NotifableCategoryEagerLoadingTest.php diff --git a/tests/NotifynderTestCase.php b/tests/NotifynderTestCase.php index 7d3d253..6d83032 100644 --- a/tests/NotifynderTestCase.php +++ b/tests/NotifynderTestCase.php @@ -147,20 +147,21 @@ protected function createCar(array $attributes = []) } } - protected function sendNotificationTo(Model $model) + protected function sendNotificationTo(Model $model, Model $category = null) { - $category = $this->createCategory(); - + if (is_null($category)) { + $category = $this->createCategory(); + } return $model ->sendNotificationTo($category->getKey()) ->from(2) ->send(); } - protected function sendNotificationsTo(Model $model, $amount = 10) + protected function sendNotificationsTo(Model $model, $amount = 10, Model $category = null) { while ($amount > 0) { - $this->sendNotificationTo($model); + $this->sendNotificationTo($model, $category); $amount--; } } diff --git a/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php b/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php new file mode 100644 index 0000000..9c70ac6 --- /dev/null +++ b/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php @@ -0,0 +1,61 @@ +connection()->logging()) { + app('db')->connection()->enableQueryLog(); + $this->initialLogStatus = false; + } else { + $this->initialLogStatus = true; + } + } + + public function tearDown() + { + if ($this->initialLogStatus == false) { + app('db')->connection()->disableQueryLog(); + } + parent::tearDown(); + } + + public function testGetNotificationsWithEagerLoadedCategories() + { + $user = $this->createUser(); + $category = $this->createCategory(['text' => 'A Test Notification']); + $this->sendNotificationsTo($user, 25, $category); + + $startCount = count(app('db')->getQueryLog()); + $notifications = $user->getNotificationRelation() + ->with('category') + ->orderBy('created_at', 'desc') + ->get(); + foreach ($notifications as $notification) { + $notification->text; + } + $endCount = count(app('db')->getQueryLog()); + + $this->assertSame(2, $endCount - $startCount); + } + + public function testThrowsModelNotFoundExceptionIfCategoryIsNull() + { + $category = $this->createCategory(); + $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); + $notification->getText(); + } +} From fcc5c5afd203ecbb83ff0b792b3582fd4fdd681d Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Sun, 25 Jun 2017 09:28:51 +0200 Subject: [PATCH 05/13] Fixed StyleCI errors --- .../integration/Traits/NotifableCategoryEagerLoadingTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php b/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php index 9c70ac6..5596c9c 100644 --- a/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php +++ b/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php @@ -6,11 +6,11 @@ class NotifableCategoryEagerLoadingTest extends NotifynderTestCase { private $initialLogStatus; - + public function setUp() { parent::setUp(); - if (!app('db')->connection()->logging()) { + if (! app('db')->connection()->logging()) { app('db')->connection()->enableQueryLog(); $this->initialLogStatus = false; } else { From 274d4eef2c1dfd0b2b0da51970a5a519765b9501 Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Sun, 25 Jun 2017 09:30:09 +0200 Subject: [PATCH 06/13] Fixed remaining StyleCI errors --- tests/NotifynderTestCase.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/NotifynderTestCase.php b/tests/NotifynderTestCase.php index 6d83032..20fc1a2 100644 --- a/tests/NotifynderTestCase.php +++ b/tests/NotifynderTestCase.php @@ -152,6 +152,7 @@ protected function sendNotificationTo(Model $model, Model $category = null) if (is_null($category)) { $category = $this->createCategory(); } + return $model ->sendNotificationTo($category->getKey()) ->from(2) From 213e408e79465ad3d698fd47136013bff46c47a9 Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Mon, 26 Jun 2017 19:44:45 +0200 Subject: [PATCH 07/13] Added eager_load configuration parameter --- src/Notifynder/Collections/Config.php | 10 +++ src/Notifynder/Traits/Notifable.php | 4 +- src/Notifynder/Traits/NotifableBasic.php | 24 +++++- src/Notifynder/Traits/NotifableLaravel53.php | 4 +- src/config/notifynder.php | 12 +++ tests/NotifynderTestCase.php | 10 +-- .../Parsers/NotificationParserTest.php | 23 +++++ .../NotifableCategoryEagerLoadingTest.php | 61 ------------- .../Traits/NotifableEagerLoadingTest.php | 86 +++++++++++++++++++ 9 files changed, 162 insertions(+), 72 deletions(-) create mode 100644 tests/integration/Parsers/NotificationParserTest.php delete mode 100644 tests/integration/Traits/NotifableCategoryEagerLoadingTest.php create mode 100644 tests/integration/Traits/NotifableEagerLoadingTest.php diff --git a/src/Notifynder/Collections/Config.php b/src/Notifynder/Collections/Config.php index db5752f..dad507f 100644 --- a/src/Notifynder/Collections/Config.php +++ b/src/Notifynder/Collections/Config.php @@ -114,6 +114,16 @@ public function set($key, $value = null) app('config')->set('notifynder.'.$key, $value); } + /** + * @param string $key + * @param null $value + */ + 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/Traits/Notifable.php b/src/Notifynder/Traits/Notifable.php index 96e1c70..f939550 100644 --- 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() + private function getLazyLoadedNotificationRelation() { return $this->notifications(); } diff --git a/src/Notifynder/Traits/NotifableBasic.php b/src/Notifynder/Traits/NotifableBasic.php index 8f07432..6bc4ee5 100644 --- 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 protected function getLazyLoadedNotificationRelation(); + /** * 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->getLazyLoadedNotificationRelation()->with($with); + } /** * Get a new NotifynderManager instance with the given category. diff --git a/src/Notifynder/Traits/NotifableLaravel53.php b/src/Notifynder/Traits/NotifableLaravel53.php index 392e4d8..68c66bd 100644 --- 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() + private function getLazyLoadedNotificationRelation() { return $this->notifynderNotifications(); } diff --git a/src/config/notifynder.php b/src/config/notifynder.php index 731c5e3..6d88fbd 100644 --- 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 models 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/NotifynderTestCase.php b/tests/NotifynderTestCase.php index 20fc1a2..7d3d253 100644 --- a/tests/NotifynderTestCase.php +++ b/tests/NotifynderTestCase.php @@ -147,11 +147,9 @@ protected function createCar(array $attributes = []) } } - protected function sendNotificationTo(Model $model, Model $category = null) + protected function sendNotificationTo(Model $model) { - if (is_null($category)) { - $category = $this->createCategory(); - } + $category = $this->createCategory(); return $model ->sendNotificationTo($category->getKey()) @@ -159,10 +157,10 @@ protected function sendNotificationTo(Model $model, Model $category = null) ->send(); } - protected function sendNotificationsTo(Model $model, $amount = 10, Model $category = null) + protected function sendNotificationsTo(Model $model, $amount = 10) { while ($amount > 0) { - $this->sendNotificationTo($model, $category); + $this->sendNotificationTo($model); $amount--; } } diff --git a/tests/integration/Parsers/NotificationParserTest.php b/tests/integration/Parsers/NotificationParserTest.php new file mode 100644 index 0000000..ce948d1 --- /dev/null +++ b/tests/integration/Parsers/NotificationParserTest.php @@ -0,0 +1,23 @@ +createCategory(); + $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, $category->id); + } +} diff --git a/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php b/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php deleted file mode 100644 index 5596c9c..0000000 --- a/tests/integration/Traits/NotifableCategoryEagerLoadingTest.php +++ /dev/null @@ -1,61 +0,0 @@ -connection()->logging()) { - app('db')->connection()->enableQueryLog(); - $this->initialLogStatus = false; - } else { - $this->initialLogStatus = true; - } - } - - public function tearDown() - { - if ($this->initialLogStatus == false) { - app('db')->connection()->disableQueryLog(); - } - parent::tearDown(); - } - - public function testGetNotificationsWithEagerLoadedCategories() - { - $user = $this->createUser(); - $category = $this->createCategory(['text' => 'A Test Notification']); - $this->sendNotificationsTo($user, 25, $category); - - $startCount = count(app('db')->getQueryLog()); - $notifications = $user->getNotificationRelation() - ->with('category') - ->orderBy('created_at', 'desc') - ->get(); - foreach ($notifications as $notification) { - $notification->text; - } - $endCount = count(app('db')->getQueryLog()); - - $this->assertSame(2, $endCount - $startCount); - } - - public function testThrowsModelNotFoundExceptionIfCategoryIsNull() - { - $category = $this->createCategory(); - $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); - $notification->getText(); - } -} diff --git a/tests/integration/Traits/NotifableEagerLoadingTest.php b/tests/integration/Traits/NotifableEagerLoadingTest.php new file mode 100644 index 0000000..7c89646 --- /dev/null +++ b/tests/integration/Traits/NotifableEagerLoadingTest.php @@ -0,0 +1,86 @@ +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']); + } + + private function assertModelHasLoadedRelations($model, $relationNames = []) + { + $modelLoadedRelations = $model->getRelations(); + foreach ($relationNames as $relationName) { + $this->assertArrayHasKey($relationName, $modelLoadedRelations, $relationName. ' relation was not eager loaded'); + } + } + + private function assertModelHasNoLoadedRelations($model, $relationNames = []) + { + $modelLoadedRelations = $model->getRelations(); + foreach ($relationNames as $relationName) { + $this->assertArrayNotHasKey($relationName, $modelLoadedRelations, $relationName. ' relation was eager loaded'); + } + } +} From 8abfa74664043eaa0f88af6b934f58b47fe637b4 Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Mon, 26 Jun 2017 19:55:04 +0200 Subject: [PATCH 08/13] Fixing typo is configuration file comment --- src/config/notifynder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/notifynder.php b/src/config/notifynder.php index 6d88fbd..46b2c04 100644 --- a/src/config/notifynder.php +++ b/src/config/notifynder.php @@ -18,7 +18,7 @@ /* * 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 models to eager load + * or you can specify which relations to eager load * * Possible Values: * false: No relations are eager loaded (default) From 0c09dec67e96387b83eb4a700241966410896f8e Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Mon, 26 Jun 2017 20:20:18 +0200 Subject: [PATCH 09/13] Fixed StyleCI errors --- src/config/notifynder.php | 2 +- .../integration/Traits/NotifableEagerLoadingTest.php | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/config/notifynder.php b/src/config/notifynder.php index 46b2c04..d0c2fc8 100644 --- a/src/config/notifynder.php +++ b/src/config/notifynder.php @@ -20,7 +20,7 @@ * just swap the value to true to eager load relations * or you can specify which relations to eager load * - * Possible Values: + * 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'] diff --git a/tests/integration/Traits/NotifableEagerLoadingTest.php b/tests/integration/Traits/NotifableEagerLoadingTest.php index 7c89646..1aa4c6d 100644 --- a/tests/integration/Traits/NotifableEagerLoadingTest.php +++ b/tests/integration/Traits/NotifableEagerLoadingTest.php @@ -1,8 +1,5 @@ sendNotificationTo($user); $notification = $user->getNotificationRelation(true)->first(); - + $this->assertModelHasLoadedRelations($notification, ['category', 'from', 'to']); } @@ -30,7 +27,7 @@ public function testGetNotificationRelationCanEagerLoadASubsetOfRelationsIfAnArr $this->sendNotificationTo($user); $notification = $user->getNotificationRelation(['category'])->first(); - + $this->assertModelHasLoadedRelations($notification, ['category']); $this->assertModelHasNoLoadedRelations($notification, ['from', 'to']); } @@ -72,7 +69,7 @@ private function assertModelHasLoadedRelations($model, $relationNames = []) { $modelLoadedRelations = $model->getRelations(); foreach ($relationNames as $relationName) { - $this->assertArrayHasKey($relationName, $modelLoadedRelations, $relationName. ' relation was not eager loaded'); + $this->assertArrayHasKey($relationName, $modelLoadedRelations, $relationName.' relation was not eager loaded'); } } @@ -80,7 +77,7 @@ private function assertModelHasNoLoadedRelations($model, $relationNames = []) { $modelLoadedRelations = $model->getRelations(); foreach ($relationNames as $relationName) { - $this->assertArrayNotHasKey($relationName, $modelLoadedRelations, $relationName. ' relation was eager loaded'); + $this->assertArrayNotHasKey($relationName, $modelLoadedRelations, $relationName.' relation was eager loaded'); } } } From c517affaf13e4682e7622b14540e2056e43cf24e Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Tue, 27 Jun 2017 10:29:28 +0200 Subject: [PATCH 10/13] Modifying function visibilities from private to protected --- src/Notifynder/Traits/Notifable.php | 2 +- src/Notifynder/Traits/NotifableLaravel53.php | 2 +- tests/integration/Traits/NotifableEagerLoadingTest.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Notifynder/Traits/Notifable.php b/src/Notifynder/Traits/Notifable.php index f939550..e901ace 100644 --- a/src/Notifynder/Traits/Notifable.php +++ b/src/Notifynder/Traits/Notifable.php @@ -53,7 +53,7 @@ public function notifications() * * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany */ - private function getLazyLoadedNotificationRelation() + protected function getLazyLoadedNotificationRelation() { return $this->notifications(); } diff --git a/src/Notifynder/Traits/NotifableLaravel53.php b/src/Notifynder/Traits/NotifableLaravel53.php index 68c66bd..0974c55 100644 --- a/src/Notifynder/Traits/NotifableLaravel53.php +++ b/src/Notifynder/Traits/NotifableLaravel53.php @@ -53,7 +53,7 @@ public function notifynderNotifications() * * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany */ - private function getLazyLoadedNotificationRelation() + protected function getLazyLoadedNotificationRelation() { return $this->notifynderNotifications(); } diff --git a/tests/integration/Traits/NotifableEagerLoadingTest.php b/tests/integration/Traits/NotifableEagerLoadingTest.php index 1aa4c6d..3dcff67 100644 --- a/tests/integration/Traits/NotifableEagerLoadingTest.php +++ b/tests/integration/Traits/NotifableEagerLoadingTest.php @@ -65,7 +65,7 @@ public function testGetNotificationsReadsEagerLoadConfigurationParameter() $this->assertModelHasNoLoadedRelations($notifications[0], ['to']); } - private function assertModelHasLoadedRelations($model, $relationNames = []) + protected function assertModelHasLoadedRelations($model, $relationNames = []) { $modelLoadedRelations = $model->getRelations(); foreach ($relationNames as $relationName) { @@ -73,7 +73,7 @@ private function assertModelHasLoadedRelations($model, $relationNames = []) } } - private function assertModelHasNoLoadedRelations($model, $relationNames = []) + protected function assertModelHasNoLoadedRelations($model, $relationNames = []) { $modelLoadedRelations = $model->getRelations(); foreach ($relationNames as $relationName) { From 349ed530e734cecc093322386bb510411e99c451 Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Tue, 27 Jun 2017 10:37:33 +0200 Subject: [PATCH 11/13] Removed unused $categoryId parameter from the NotificationParser parse function --- src/Notifynder/Builder/Notification.php | 2 +- src/Notifynder/Models/Notification.php | 2 +- src/Notifynder/Parsers/NotificationParser.php | 3 +-- tests/integration/Parsers/NotificationParserTest.php | 5 ++--- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Notifynder/Builder/Notification.php b/src/Notifynder/Builder/Notification.php index d4df02b..268bd57 100644 --- 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/Models/Notification.php b/src/Notifynder/Models/Notification.php index 0d5ce4d..eaf8975 100644 --- 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 index b6f3887..0c68caa 100644 --- a/src/Notifynder/Parsers/NotificationParser.php +++ b/src/Notifynder/Parsers/NotificationParser.php @@ -21,11 +21,10 @@ class NotificationParser * Parse a notification and return the body text. * * @param ModelNotification $notification - * @param int $categoryId * @return string * @throws ExtraParamsException */ - public function parse($notification, $categoryId) + public function parse($notification) { $category = $notification->category; if (is_null($category)) { diff --git a/tests/integration/Parsers/NotificationParserTest.php b/tests/integration/Parsers/NotificationParserTest.php index ce948d1..8b394d4 100644 --- a/tests/integration/Parsers/NotificationParserTest.php +++ b/tests/integration/Parsers/NotificationParserTest.php @@ -6,9 +6,8 @@ class NotificationParserTest extends NotifynderTestCase { - public function testThrowsModelNotFoundExceptionIfCategoryIsNull() + public function testParseThrowsModelNotFoundExceptionIfCategoryIsNull() { - $category = $this->createCategory(); $from = $this->createUser(); $to = $this->createUser(); $notification = new Notification(); @@ -18,6 +17,6 @@ public function testThrowsModelNotFoundExceptionIfCategoryIsNull() $this->expectException(ModelNotFoundException::class); $parser = new NotificationParser(); - $parser->parse($notification, $category->id); + $parser->parse($notification); } } From cd0d244b6dc1dc9aa9e9f28602451fd6c59c4cfe Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Tue, 27 Jun 2017 11:48:35 +0200 Subject: [PATCH 12/13] Prevent EagerLoad when updating or counting notifications --- src/Notifynder/Traits/Notifable.php | 2 +- src/Notifynder/Traits/NotifableBasic.php | 14 +++++++------- src/Notifynder/Traits/NotifableLaravel53.php | 2 +- .../Traits/NotifableEagerLoadingTest.php | 9 +++++++++ 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/Notifynder/Traits/Notifable.php b/src/Notifynder/Traits/Notifable.php index e901ace..469dd9e 100644 --- a/src/Notifynder/Traits/Notifable.php +++ b/src/Notifynder/Traits/Notifable.php @@ -53,7 +53,7 @@ public function notifications() * * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany */ - protected function getLazyLoadedNotificationRelation() + public function getLazyNotificationRelation() { return $this->notifications(); } diff --git a/src/Notifynder/Traits/NotifableBasic.php b/src/Notifynder/Traits/NotifableBasic.php index 6bc4ee5..4e32931 100644 --- a/src/Notifynder/Traits/NotifableBasic.php +++ b/src/Notifynder/Traits/NotifableBasic.php @@ -14,7 +14,7 @@ trait NotifableBasic * * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany */ - abstract protected function getLazyLoadedNotificationRelation(); + abstract public function getLazyNotificationRelation(); /** * Get the notifications Relationship. @@ -35,7 +35,7 @@ public function getNotificationRelation($eagerLoad = null) $with = $eagerLoad; } - return $this->getLazyLoadedNotificationRelation()->with($with); + return $this->getLazyNotificationRelation()->with($with); } /** @@ -101,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(); } @@ -118,7 +118,7 @@ protected function updateSingleReadStatus($notification, $value) */ public function readAllNotifications() { - return $this->getNotificationRelation()->update(['read' => 1]); + return $this->getLazyNotificationRelation()->update(['read' => 1]); } /** @@ -128,7 +128,7 @@ public function readAllNotifications() */ public function unreadAllNotifications() { - return $this->getNotificationRelation()->update(['read' => 0]); + return $this->getLazyNotificationRelation()->update(['read' => 0]); } /** @@ -138,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 index 0974c55..8efec2f 100644 --- a/src/Notifynder/Traits/NotifableLaravel53.php +++ b/src/Notifynder/Traits/NotifableLaravel53.php @@ -53,7 +53,7 @@ public function notifynderNotifications() * * @return \Illuminate\Database\Eloquent\Relations\HasMany|\Illuminate\Database\Eloquent\Relations\MorphMany */ - protected function getLazyLoadedNotificationRelation() + public function getLazyNotificationRelation() { return $this->notifynderNotifications(); } diff --git a/tests/integration/Traits/NotifableEagerLoadingTest.php b/tests/integration/Traits/NotifableEagerLoadingTest.php index 3dcff67..9140098 100644 --- a/tests/integration/Traits/NotifableEagerLoadingTest.php +++ b/tests/integration/Traits/NotifableEagerLoadingTest.php @@ -2,6 +2,15 @@ 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(); From 51a2ea4505f094399e282e4c766bfab64412e93a Mon Sep 17 00:00:00 2001 From: Mohamed Hassan Date: Tue, 27 Jun 2017 12:10:38 +0200 Subject: [PATCH 13/13] Fixing comment typo --- src/Notifynder/Collections/Config.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Notifynder/Collections/Config.php b/src/Notifynder/Collections/Config.php index dad507f..d302ecf 100644 --- a/src/Notifynder/Collections/Config.php +++ b/src/Notifynder/Collections/Config.php @@ -116,7 +116,6 @@ public function set($key, $value = null) /** * @param string $key - * @param null $value */ public function forget($key) {