From b4b78cc6d902600b7d4547f94b268594ab98055f Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sat, 9 Sep 2023 11:38:35 -0400 Subject: [PATCH 01/30] initial commit to handle special softDelete case for relations with pivot table --- src/Database/Traits/SoftDelete.php | 50 ++++++++++++++++++------------ 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index cf19758d9..2d2e67f1f 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -123,13 +123,19 @@ protected function performSoftDeleteOnRelations() continue; } - if ($relation instanceof EloquentModel) { - $relation->delete(); - } - elseif ($relation instanceof CollectionBase) { - $relation->each(function ($model) { - $model->delete(); - }); + if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { + /* + * special case for relations using pivot table + */ + } else { + if ($relation instanceof EloquentModel) { + $relation->delete(); + } + elseif ($relation instanceof CollectionBase) { + $relation->each(function ($model) { + $model->delete(); + }); + } } } } @@ -191,18 +197,24 @@ protected function performRestoreOnRelations() continue; } - $relation = $this->{$name}()->onlyTrashed()->getResults(); - if (!$relation) { - continue; - } - - if ($relation instanceof EloquentModel) { - $relation->restore(); - } - elseif ($relation instanceof CollectionBase) { - $relation->each(function ($model) { - $model->restore(); - }); + if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { + /* + * special case for relations using pivot table + */ + } else { + $relation = $this->{$name}()->onlyTrashed()->getResults(); + if (!$relation) { + continue; + } + + if ($relation instanceof EloquentModel) { + $relation->restore(); + } + elseif ($relation instanceof CollectionBase) { + $relation->each(function ($model) { + $model->restore(); + }); + } } } } From b8d03da42fe59e1176bf79524f71573272068f4a Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sat, 9 Sep 2023 12:57:49 -0400 Subject: [PATCH 02/30] first draft at solving the issue --- src/Database/Traits/SoftDelete.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index 2d2e67f1f..2d413400c 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -124,9 +124,11 @@ protected function performSoftDeleteOnRelations() } if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { - /* - * special case for relations using pivot table - */ + // relations using pivot table + $relation = $this->{$name}(); + $deletedAtColumn = array_get($options, 'deletedAtColumn', 'deleted_at'); + $time = $this->fromDateTime($this->freshTimestamp()); + $query = $relation->newPivotQuery()->update(array($deletedAtColumn => $time)); } else { if ($relation instanceof EloquentModel) { $relation->delete(); @@ -198,9 +200,10 @@ protected function performRestoreOnRelations() } if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { - /* - * special case for relations using pivot table - */ + // relations using pivot table + $relation = $this->{$name}(); + $deletedAtColumn = array_get($options, 'deletedAtColumn', 'deleted_at'); + $query = $relation->newPivotQuery()->update(array($deletedAtColumn => null)); } else { $relation = $this->{$name}()->onlyTrashed()->getResults(); if (!$relation) { From 3ac690838312f4e3881677d0fb6c5653d71b7851 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sat, 9 Sep 2023 13:53:57 -0400 Subject: [PATCH 03/30] code improvements --- src/Database/Traits/SoftDelete.php | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index 2d413400c..343a6b0a6 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -125,10 +125,8 @@ protected function performSoftDeleteOnRelations() if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { // relations using pivot table - $relation = $this->{$name}(); - $deletedAtColumn = array_get($options, 'deletedAtColumn', 'deleted_at'); - $time = $this->fromDateTime($this->freshTimestamp()); - $query = $relation->newPivotQuery()->update(array($deletedAtColumn => $time)); + $value = $this->fromDateTime($this->freshTimestamp()); + $this->updatePivotDeletedAtColumn($name, $options, $value); } else { if ($relation instanceof EloquentModel) { $relation->delete(); @@ -185,6 +183,21 @@ public function restore() return $result; } + /** + * Update relation pivot table deleted_at column + */ + protected function updatePivotDeletedAtColumn(string $relationName, array $options, string|null $value) + { + $relation = $this->{$relationName}(); + + // get deletedAtColumn from the relation options, otherwise use default + $deletedAtColumn = array_get($options, 'deletedAtColumn', 'deleted_at'); + + $query = $relation->newPivotQuery()->update([ + $deletedAtColumn => $value, + ]); + } + /** * Locates relations with softDelete flag and cascades the restore event. * @@ -201,9 +214,7 @@ protected function performRestoreOnRelations() if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { // relations using pivot table - $relation = $this->{$name}(); - $deletedAtColumn = array_get($options, 'deletedAtColumn', 'deleted_at'); - $query = $relation->newPivotQuery()->update(array($deletedAtColumn => null)); + $this->updatePivotDeletedAtColumn($name, $options, null); } else { $relation = $this->{$name}()->onlyTrashed()->getResults(); if (!$relation) { From 22dad8f60b94711b88c4bbd43a52973854fcef06 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 10 Sep 2023 10:24:06 -0400 Subject: [PATCH 04/30] fix the Database Model instead of patching the Auth/User model --- src/Auth/Models/User.php | 11 ----------- src/Database/Model.php | 19 ++++++++++++------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/Auth/Models/User.php b/src/Auth/Models/User.php index 8e7f020ad..114441af7 100644 --- a/src/Auth/Models/User.php +++ b/src/Auth/Models/User.php @@ -154,17 +154,6 @@ public function afterLogin() $this->forceSave(); } - /** - * Delete the user groups - * @return void - */ - public function afterDelete() - { - if ($this->hasRelation('groups')) { - $this->groups()->detach(); - } - } - // // Persistence (used by Cookies and Sessions) // diff --git a/src/Database/Model.php b/src/Database/Model.php index 073566e0c..8c3783dc9 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1016,13 +1016,18 @@ protected function performDeleteOnRelations() continue; } - if ($relation instanceof EloquentModel) { - $relation->forceDelete(); - } - elseif ($relation instanceof CollectionBase) { - $relation->each(function ($model) { - $model->forceDelete(); - }); + if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { + // we want to remove the pivot record, not the actual relation record + $relation()->detach(); + } else { + if ($relation instanceof EloquentModel) { + $relation->forceDelete(); + } + elseif ($relation instanceof CollectionBase) { + $relation->each(function ($model) { + $model->forceDelete(); + }); + } } } From 9fac408d370718c91c40db9e797b736b4d3a42b7 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 10 Sep 2023 12:57:51 -0400 Subject: [PATCH 05/30] the delete relation option should not apply to pivot table records --- src/Database/Model.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index 8c3783dc9..a7e028753 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1008,10 +1008,6 @@ protected function performDeleteOnRelations() * Hard 'delete' definition */ foreach ($relations as $name => $options) { - if (!Arr::get($options, 'delete', false)) { - continue; - } - if (!$relation = $this->{$name}) { continue; } @@ -1020,6 +1016,10 @@ protected function performDeleteOnRelations() // we want to remove the pivot record, not the actual relation record $relation()->detach(); } else { + if (!Arr::get($options, 'delete', false)) { + continue; + } + if ($relation instanceof EloquentModel) { $relation->forceDelete(); } From 3a1464b82888d302431d3f6446ba14fa266b4543 Mon Sep 17 00:00:00 2001 From: Ben Thomson Date: Mon, 11 Sep 2023 08:23:58 +0800 Subject: [PATCH 06/30] Update src/Database/Model.php --- src/Database/Model.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index a7e028753..2c667e375 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1022,8 +1022,7 @@ protected function performDeleteOnRelations() if ($relation instanceof EloquentModel) { $relation->forceDelete(); - } - elseif ($relation instanceof CollectionBase) { + } elseif ($relation instanceof CollectionBase) { $relation->each(function ($model) { $model->forceDelete(); }); From 01df0d9128799af673f9cb301f2ff426a34d8b8a Mon Sep 17 00:00:00 2001 From: Ben Thomson Date: Mon, 11 Sep 2023 08:24:55 +0800 Subject: [PATCH 07/30] Apply suggestions from code review --- src/Database/Traits/SoftDelete.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index 343a6b0a6..6ab3313dd 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -130,8 +130,7 @@ protected function performSoftDeleteOnRelations() } else { if ($relation instanceof EloquentModel) { $relation->delete(); - } - elseif ($relation instanceof CollectionBase) { + } elseif ($relation instanceof CollectionBase) { $relation->each(function ($model) { $model->delete(); }); @@ -223,8 +222,7 @@ protected function performRestoreOnRelations() if ($relation instanceof EloquentModel) { $relation->restore(); - } - elseif ($relation instanceof CollectionBase) { + } elseif ($relation instanceof CollectionBase) { $relation->each(function ($model) { $model->restore(); }); From 8b9d6fed19835d31bdf6e5119b052e14e238ecfd Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 10 Sep 2023 21:49:10 -0400 Subject: [PATCH 08/30] make sure not to leave orphans --- src/Auth/Models/User.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Auth/Models/User.php b/src/Auth/Models/User.php index 114441af7..1150acfb8 100644 --- a/src/Auth/Models/User.php +++ b/src/Auth/Models/User.php @@ -39,7 +39,7 @@ class User extends Model implements \Illuminate\Contracts\Auth\Authenticatable * @var array Relations */ public $belongsToMany = [ - 'groups' => [Group::class, 'table' => 'users_groups'] + 'groups' => [Group::class, 'table' => 'users_groups', 'delete' => true], ]; public $belongsTo = [ From 1c1114b2de70452fd40bdf431506893ad0f08052 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Mon, 11 Sep 2023 07:51:13 -0400 Subject: [PATCH 09/30] no need for delete option, the relation pivot records are detached always as it should --- src/Auth/Models/User.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Auth/Models/User.php b/src/Auth/Models/User.php index 1150acfb8..3865d0a9f 100644 --- a/src/Auth/Models/User.php +++ b/src/Auth/Models/User.php @@ -39,7 +39,7 @@ class User extends Model implements \Illuminate\Contracts\Auth\Authenticatable * @var array Relations */ public $belongsToMany = [ - 'groups' => [Group::class, 'table' => 'users_groups', 'delete' => true], + 'groups' => [Group::class, 'table' => 'users_groups'], ]; public $belongsTo = [ From 6672a0bb83c4be3e5aa10050d201fa3e1b269d18 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Mon, 11 Sep 2023 08:05:05 -0400 Subject: [PATCH 10/30] small tweak --- src/Database/Traits/SoftDelete.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index 6ab3313dd..c655b27ec 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -187,12 +187,12 @@ public function restore() */ protected function updatePivotDeletedAtColumn(string $relationName, array $options, string|null $value) { - $relation = $this->{$relationName}(); + $relation = $this->{$relationName}; // get deletedAtColumn from the relation options, otherwise use default $deletedAtColumn = array_get($options, 'deletedAtColumn', 'deleted_at'); - $query = $relation->newPivotQuery()->update([ + $query = $relation()->newPivotQuery()->update([ $deletedAtColumn => $value, ]); } From 97ff39263017c4aecdaeea57502a4dbb4c1336c1 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Wed, 13 Sep 2023 16:51:11 -0400 Subject: [PATCH 11/30] initial sofdelete trait tests --- tests/Database/Traits/SoftDeleteTest.php | 132 +++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 tests/Database/Traits/SoftDeleteTest.php diff --git a/tests/Database/Traits/SoftDeleteTest.php b/tests/Database/Traits/SoftDeleteTest.php new file mode 100644 index 000000000..fba13d0c9 --- /dev/null +++ b/tests/Database/Traits/SoftDeleteTest.php @@ -0,0 +1,132 @@ +seeded = [ + 'posts' => [], + 'categories' => [] + ]; + + $this->createTables(); + } + + protected function createTables() + { + $this->getBuilder()->create('posts', function ($table) { + $table->increments('id'); + $table->string('title')->default(''); + $table->timestamps(); + $table->timestamp('deleted_at')->nullable(); + }); + + $this->getBuilder()->create('categories', function ($table) { + $table->increments('id'); + $table->string('name'); + $table->timestamps(); + }); + + $this->getBuilder()->create('categories_posts', function ($table) { + $table->primary(['post_id', 'category_id']); + $table->unsignedInteger('post_id'); + $table->unsignedInteger('category_id'); + $table->timestamp('deleted_at')->nullable(); + }); + + $this->seedTables(); + } + + protected function seedTables() + { + $this->seeded['posts'][] = Post::create([ + 'title' => 'First Post', + ]); + $this->seeded['posts'][] = Post::create([ + 'title' => 'Second Post', + ]); + + $this->seeded['categories'][] = Category::create([ + 'name' => 'Category 1' + ]); + $this->seeded['categories'][] = Category::create([ + 'name' => 'Category 2' + ]); + + $this->seeded['posts'][0]->categories()->attach($this->seeded['categories'][0]); + $this->seeded['posts'][0]->categories()->attach($this->seeded['categories'][1]); + + $this->seeded['posts'][1]->categories()->attach($this->seeded['categories'][0]); + $this->seeded['posts'][1]->categories()->attach($this->seeded['categories'][1]); + } + + public function testDeleteAndRestore() + { + $post = Post::where('title', 'First Post')->first(); + $this->assertTrue($post->deleted_at === null); + $this->assertTrue($post->categories()->where('deleted_at', null)->count() === 2); + + $post->delete(); + + $post = Post::withTrashed()->first(); + $this->assertTrue($post->deleted_at != null); + $this->assertTrue($post->categories()->where('deleted_at', '!=', null)->count() === 2); + $post->restore(); + + $post = Post::first(); + $this->assertTrue($post->deleted_at === null); + $this->assertTrue($post->categories()->where('deleted_at', null)->count() === 2); + } +} + +class Post extends \Winter\Storm\Database\Model +{ + use \Winter\Storm\Database\Traits\SoftDelete; + + public $table = 'posts'; + + public $fillable = ['title']; + + protected $dates = [ + 'created_at', + 'updated_at', + 'deleted_at', + ]; + + public $belongsToMany = [ + 'categories' => [ + Category::class, + 'table' => 'categories_posts', + 'key' => 'post_id', + 'otherKey' => 'category_id', + 'softDelete' => true, + ], + ]; +} + +class Category extends \Winter\Storm\Database\Model +{ + public $table = 'categories'; + + public $fillable = ['name']; + + protected $dates = [ + 'created_at', + 'updated_at', + ]; + + public $belongsToMany = [ + 'posts' => [ + Post::class, + 'table' => 'categories_posts', + 'key' => 'category_id', + 'otherKey' => 'post_id', + ], + ]; +} From 3770e942f5ce486ca7210b44e845e6d5dc599e0b Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Thu, 14 Sep 2023 09:05:01 -0400 Subject: [PATCH 12/30] verify the relation "detach" option --- src/Database/Model.php | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index 2c667e375..23b36ec67 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1013,6 +1013,9 @@ protected function performDeleteOnRelations() } if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { + if (!Arr::get($options, 'detach', true)) { + continue; + } // we want to remove the pivot record, not the actual relation record $relation()->detach(); } else { @@ -1029,17 +1032,6 @@ protected function performDeleteOnRelations() } } } - - /* - * Belongs-To-Many should clean up after itself always - */ - if ($type == 'belongsToMany') { - foreach ($relations as $name => $options) { - if (Arr::get($options, 'detach', true)) { - $this->{$name}()->detach(); - } - } - } } } From b82e651f51d9a19582333b2872f1938540ee2aed Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Thu, 14 Sep 2023 10:47:10 -0400 Subject: [PATCH 13/30] add unit test for soft delete/restore --- src/Database/Model.php | 2 +- src/Database/Traits/SoftDelete.php | 5 ++--- tests/Database/Traits/SoftDeleteTest.php | 17 ++++++++--------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index 23b36ec67..f88bab6e2 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1017,7 +1017,7 @@ protected function performDeleteOnRelations() continue; } // we want to remove the pivot record, not the actual relation record - $relation()->detach(); + $this->{$name}()->detach(); } else { if (!Arr::get($options, 'delete', false)) { continue; diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index c655b27ec..aedeec81f 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -127,6 +127,7 @@ protected function performSoftDeleteOnRelations() // relations using pivot table $value = $this->fromDateTime($this->freshTimestamp()); $this->updatePivotDeletedAtColumn($name, $options, $value); + } else { if ($relation instanceof EloquentModel) { $relation->delete(); @@ -187,12 +188,10 @@ public function restore() */ protected function updatePivotDeletedAtColumn(string $relationName, array $options, string|null $value) { - $relation = $this->{$relationName}; - // get deletedAtColumn from the relation options, otherwise use default $deletedAtColumn = array_get($options, 'deletedAtColumn', 'deleted_at'); - $query = $relation()->newPivotQuery()->update([ + $this->{$relationName}()->newPivotQuery()->update([ $deletedAtColumn => $value, ]); } diff --git a/tests/Database/Traits/SoftDeleteTest.php b/tests/Database/Traits/SoftDeleteTest.php index fba13d0c9..258b8208b 100644 --- a/tests/Database/Traits/SoftDeleteTest.php +++ b/tests/Database/Traits/SoftDeleteTest.php @@ -16,6 +16,7 @@ public function setUp(): void ]; $this->createTables(); + $this->seedTables(); } protected function createTables() @@ -39,8 +40,6 @@ protected function createTables() $table->unsignedInteger('category_id'); $table->timestamp('deleted_at')->nullable(); }); - - $this->seedTables(); } protected function seedTables() @@ -68,7 +67,7 @@ protected function seedTables() public function testDeleteAndRestore() { - $post = Post::where('title', 'First Post')->first(); + $post = Post::first(); $this->assertTrue($post->deleted_at === null); $this->assertTrue($post->categories()->where('deleted_at', null)->count() === 2); @@ -102,9 +101,9 @@ class Post extends \Winter\Storm\Database\Model public $belongsToMany = [ 'categories' => [ Category::class, - 'table' => 'categories_posts', - 'key' => 'post_id', - 'otherKey' => 'category_id', + 'table' => 'categories_posts', + 'key' => 'post_id', + 'otherKey' => 'category_id', 'softDelete' => true, ], ]; @@ -124,9 +123,9 @@ class Category extends \Winter\Storm\Database\Model public $belongsToMany = [ 'posts' => [ Post::class, - 'table' => 'categories_posts', - 'key' => 'category_id', - 'otherKey' => 'post_id', + 'table' => 'categories_posts', + 'key' => 'category_id', + 'otherKey' => 'post_id', ], ]; } From e51407c0cf7f343f7492de343794240003ab9461 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Thu, 14 Sep 2023 10:50:55 -0400 Subject: [PATCH 14/30] remove empty line --- src/Database/Traits/SoftDelete.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index aedeec81f..3717fefa1 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -127,7 +127,6 @@ protected function performSoftDeleteOnRelations() // relations using pivot table $value = $this->fromDateTime($this->freshTimestamp()); $this->updatePivotDeletedAtColumn($name, $options, $value); - } else { if ($relation instanceof EloquentModel) { $relation->delete(); From e35a61a3cfab2b649134f900702286141d319f81 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Thu, 14 Sep 2023 16:09:51 -0400 Subject: [PATCH 15/30] remove namespace --- tests/Database/Traits/SoftDeleteTest.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/Database/Traits/SoftDeleteTest.php b/tests/Database/Traits/SoftDeleteTest.php index 258b8208b..2ae8fae5f 100644 --- a/tests/Database/Traits/SoftDeleteTest.php +++ b/tests/Database/Traits/SoftDeleteTest.php @@ -1,8 +1,6 @@ Date: Thu, 14 Sep 2023 16:12:30 -0400 Subject: [PATCH 16/30] relation var only needed in else block --- src/Database/Traits/SoftDelete.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index 3717fefa1..9df5c4077 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -118,16 +118,14 @@ protected function performSoftDeleteOnRelations() if (!array_get($options, 'softDelete', false)) { continue; } - - if (!$relation = $this->{$name}) { - continue; - } - if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { // relations using pivot table $value = $this->fromDateTime($this->freshTimestamp()); $this->updatePivotDeletedAtColumn($name, $options, $value); } else { + if (!$relation = $this->{$name}) { + continue; + } if ($relation instanceof EloquentModel) { $relation->delete(); } elseif ($relation instanceof CollectionBase) { From f8face73cf1becfe39f4dbe9a53078a035c39e13 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Thu, 14 Sep 2023 21:39:14 -0400 Subject: [PATCH 17/30] namespace IS required --- tests/Database/Traits/SoftDeleteTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Database/Traits/SoftDeleteTest.php b/tests/Database/Traits/SoftDeleteTest.php index 2ae8fae5f..258b8208b 100644 --- a/tests/Database/Traits/SoftDeleteTest.php +++ b/tests/Database/Traits/SoftDeleteTest.php @@ -1,6 +1,8 @@ Date: Fri, 15 Sep 2023 07:58:02 -0400 Subject: [PATCH 18/30] skip early if null --- src/Database/Traits/SoftDelete.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index 9df5c4077..5cbf03d6f 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -115,6 +115,9 @@ protected function performSoftDeleteOnRelations() $definitions = $this->getRelationDefinitions(); foreach ($definitions as $type => $relations) { foreach ($relations as $name => $options) { + if (!$relation = $this->{$name}) { + continue; + } if (!array_get($options, 'softDelete', false)) { continue; } @@ -123,9 +126,6 @@ protected function performSoftDeleteOnRelations() $value = $this->fromDateTime($this->freshTimestamp()); $this->updatePivotDeletedAtColumn($name, $options, $value); } else { - if (!$relation = $this->{$name}) { - continue; - } if ($relation instanceof EloquentModel) { $relation->delete(); } elseif ($relation instanceof CollectionBase) { From e9595b2926ea7e8010038ced696e32b234f13771 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sat, 30 Sep 2023 10:18:13 -0400 Subject: [PATCH 19/30] cleanup --- src/Database/Model.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index f88bab6e2..52976dafa 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -998,6 +998,7 @@ protected function performDeleteOnModel() /** * Locates relations with delete flag and cascades the delete event. + * For pivot relations, detach the pivot record unless the detach flag is false. * @return void */ protected function performDeleteOnRelations() @@ -1013,11 +1014,10 @@ protected function performDeleteOnRelations() } if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { - if (!Arr::get($options, 'detach', true)) { - continue; - } // we want to remove the pivot record, not the actual relation record - $this->{$name}()->detach(); + if (Arr::get($options, 'detach', true)) { + $this->{$name}()->detach(); + } } else { if (!Arr::get($options, 'delete', false)) { continue; From 302ce88272f72efaaa7c920260926a6c68b339ef Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sat, 14 Oct 2023 09:21:42 -0400 Subject: [PATCH 20/30] do not remove has{One,Many}Through related records --- src/Database/Model.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Database/Model.php b/src/Database/Model.php index c8e77a213..e7c899597 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1018,6 +1018,9 @@ protected function performDeleteOnRelations() if (Arr::get($options, 'detach', true)) { $this->{$name}()->detach(); } + } else if (in_array($type, ['hasOneThrough', 'hasManyThrough'])) { + // the model does not own the relation, we should not remove it. + continue; } else { if (!Arr::get($options, 'delete', false)) { continue; From 913af0c8916c1c0b9dce790415e2863a473c4bb8 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sat, 14 Oct 2023 10:01:13 -0400 Subject: [PATCH 21/30] belongsTo and morphTo relation should not be deleted --- src/Database/Model.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index e7c899597..518a5c97b 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1018,10 +1018,11 @@ protected function performDeleteOnRelations() if (Arr::get($options, 'detach', true)) { $this->{$name}()->detach(); } - } else if (in_array($type, ['hasOneThrough', 'hasManyThrough'])) { - // the model does not own the relation, we should not remove it. + } else if (in_array($type, ['belongsTo', 'hasOneThrough', 'hasManyThrough', 'morphTo'])) { + // the model does not own the related record, we should not remove it. continue; } else { + // attachOne, attachMany, hasOne, hasMany, morphOne, morphMany if (!Arr::get($options, 'delete', false)) { continue; } From 821b8eeb7e3a92405d0115cd39a8084162a13270 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Sat, 14 Oct 2023 12:40:30 -0600 Subject: [PATCH 22/30] Update src/Database/Model.php --- src/Database/Model.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index 518a5c97b..d0f2f3a06 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1018,7 +1018,7 @@ protected function performDeleteOnRelations() if (Arr::get($options, 'detach', true)) { $this->{$name}()->detach(); } - } else if (in_array($type, ['belongsTo', 'hasOneThrough', 'hasManyThrough', 'morphTo'])) { + } elseif (in_array($type, ['belongsTo', 'hasOneThrough', 'hasManyThrough', 'morphTo'])) { // the model does not own the related record, we should not remove it. continue; } else { From 85dcfc2a3b83243208700a92998754d4ca40b8df Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sat, 14 Oct 2023 18:15:04 -0400 Subject: [PATCH 23/30] only handle known relation types --- src/Database/Model.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index d0f2f3a06..db68736c7 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -1021,8 +1021,7 @@ protected function performDeleteOnRelations() } elseif (in_array($type, ['belongsTo', 'hasOneThrough', 'hasManyThrough', 'morphTo'])) { // the model does not own the related record, we should not remove it. continue; - } else { - // attachOne, attachMany, hasOne, hasMany, morphOne, morphMany + } elseif (in_array($type, ['attachOne', 'attachMany', 'hasOne', 'hasMany', 'morphOne', 'morphMany'])) { if (!Arr::get($options, 'delete', false)) { continue; } From f1355b62dcd967190e6a0fa5b4cef6fbdff42ab1 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sat, 14 Oct 2023 18:21:22 -0400 Subject: [PATCH 24/30] apply same logic to softDeleted relations --- src/Database/Traits/SoftDelete.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Database/Traits/SoftDelete.php b/src/Database/Traits/SoftDelete.php index 5cbf03d6f..fc913b8e8 100644 --- a/src/Database/Traits/SoftDelete.php +++ b/src/Database/Traits/SoftDelete.php @@ -125,7 +125,10 @@ protected function performSoftDeleteOnRelations() // relations using pivot table $value = $this->fromDateTime($this->freshTimestamp()); $this->updatePivotDeletedAtColumn($name, $options, $value); - } else { + } elseif (in_array($type, ['belongsTo', 'hasOneThrough', 'hasManyThrough', 'morphTo'])) { + // the model does not own the related record, we should not remove it. + continue; + } elseif (in_array($type, ['attachOne', 'attachMany', 'hasOne', 'hasMany', 'morphOne', 'morphMany'])) { if ($relation instanceof EloquentModel) { $relation->delete(); } elseif ($relation instanceof CollectionBase) { From 75e186516fc95f608e82d139a5cb1e2dc26fcc71 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 15 Oct 2023 12:33:07 -0400 Subject: [PATCH 25/30] add model delete tests for all relation types --- tests/Database/ModelTest.php | 479 ++++++++++++++++++++++++++++++++++- 1 file changed, 466 insertions(+), 13 deletions(-) diff --git a/tests/Database/ModelTest.php b/tests/Database/ModelTest.php index b2a4b8b2a..2307f0d02 100644 --- a/tests/Database/ModelTest.php +++ b/tests/Database/ModelTest.php @@ -1,14 +1,393 @@ createTable(); + $this->createTables(); + $this->seedTables(); + } + + protected function createTables() + { + $this->getBuilder()->create('comments', function ($table) { + $table->increments('id'); + $table->string('title'); + $table->nullableMorphs('commentable'); + }); + + $this->getBuilder()->create('imageables', function ($table) { + $table->foreignId('image_id')->nullable(); + $table->nullableMorphs('imageable'); + }); + + $this->getBuilder()->create('images', function ($table) { + $table->increments('id'); + $table->string('name'); + }); + + $this->getBuilder()->create('phones', function ($table) { + $table->increments('id'); + $table->string('name'); + $table->foreignId('user_id')->nullable(); + }); + + $this->getBuilder()->create('posts', function ($table) { + $table->increments('id'); + $table->string('title'); + $table->foreignId('user_id')->nullable(); + }); + + $this->getBuilder()->create('role_user', function ($table) { + $table->increments('id'); + $table->foreignId('role_id')->nullable(); + $table->foreignId('user_id')->nullable(); + }); + + $this->getBuilder()->create('roles', function ($table) { + $table->increments('id'); + $table->string('name'); + }); + + $this->getBuilder()->create('tags', function ($table) { + $table->increments('id'); + $table->string('name'); + $table->nullableMorphs('taggable'); + }); + + $this->getBuilder()->create('users', function ($table) { + $table->increments('id'); + $table->string('name'); + $table->foreignId('website_id')->nullable(); + }); + + $this->getBuilder()->create('websites', function ($table) { + $table->increments('id'); + $table->string('url'); + }); + + $this->getBuilder()->create('test_model', function ($table) { + $table->increments('id'); + $table->string('name')->nullable(); + $table->text('data')->nullable(); + $table->text('description')->nullable(); + $table->text('meta')->nullable(); + $table->boolean('on_guard')->nullable(); + $table->timestamps(); + }); + } + + protected function seedTables() + { + $this->seeded['comments'][] = Comment::create(['title' => 'Comment1']); + $this->seeded['comments'][] = Comment::create(['title' => 'Comment2']); + + $this->seeded['images'][] = Image::create(['name' => 'Image1']); + $this->seeded['images'][] = Image::create(['name' => 'Image2']); + + $this->seeded['phones'][] = Phone::create(['name' => 'Phone1']); + $this->seeded['phones'][] = Phone::create(['name' => 'Phone2']); + + $this->seeded['roles'][] = Role::create(['name' => 'Role1']); + $this->seeded['roles'][] = Role::create(['name' => 'Role2']); + + $this->seeded['tags'][] = Tag::create(['name' => 'Tag1']); + $this->seeded['tags'][] = Tag::create(['name' => 'Tag2']); + + $this->seeded['posts'][] = Post::create(['title' => 'Post1']); + $this->seeded['posts'][0]->comments()->add($this->seeded['comments'][0]); + $this->seeded['posts'][0]->images()->attach($this->seeded['images'][0]); + $this->seeded['posts'][0]->tag()->add($this->seeded['tags'][0]); + + $this->seeded['posts'][] = Post::create(['title' => 'Post2']); + $this->seeded['posts'][1]->comments()->add($this->seeded['comments'][1]); + $this->seeded['posts'][1]->images()->attach($this->seeded['images'][1]); + $this->seeded['posts'][1]->tag()->add($this->seeded['tags'][1]); + + $this->seeded['users'][] = User::create(['name' => 'User1']); + $this->seeded['users'][0]->phone()->add($this->seeded['phones'][0]); + $this->seeded['users'][0]->posts()->add($this->seeded['posts'][0]); + $this->seeded['users'][0]->posts()->add($this->seeded['posts'][1]); + $this->seeded['users'][0]->roles()->attach($this->seeded['roles'][0]); + + $this->seeded['users'][] = User::create(['name' => 'User2']); + $this->seeded['users'][1]->phone()->add($this->seeded['phones'][1]); + $this->seeded['users'][1]->roles()->attach($this->seeded['roles'][0]); + $this->seeded['users'][1]->roles()->attach($this->seeded['roles'][1]); + + $this->seeded['websites'][] = Website::create(['url' => 'https://wintercms.com']); + $this->seeded['websites'][0]->users()->add($this->seeded['users'][0]); + + $this->seeded['websites'][] = Website::create(['url' => 'https://wintertricks.com']); + $this->seeded['websites'][1]->users()->add($this->seeded['users'][1]); + } + + // tests hasOneThrough & hasManyThrough + public function testDeleteOnThroughRelations() + { + $website = $this->seeded['websites'][0]; + $user = $this->seeded['users'][0]; + + $this->assertEquals(2, Phone::count()); + $this->assertEquals(2, Post::count()); + + $this->assertEquals(1, $website->phone()->count()); + $this->assertEquals(2, $website->posts()->count()); + + $this->assertEquals(1, $user->phone()->count()); + $this->assertEquals(2, $user->posts()->count()); + + $website->delete(); + + $this->assertEquals(1, $user->phone()->count()); + $this->assertEquals(2, $user->posts()->count()); + + $this->assertEquals(2, Phone::count()); + $this->assertEquals(2, Post::count()); + } + + // tests hasMany + public function testDeleteOnHasManyRelation() + { + $website = $this->seeded['websites'][0]; + $user = $this->seeded['users'][0]; + + $this->assertEquals(2, Phone::count()); + $this->assertEquals(2, Post::count()); + + $this->assertEquals(1, $website->phone()->count()); + $this->assertEquals(2, $website->posts()->count()); + + $this->assertEquals(1, $user->phone()->count()); + $this->assertEquals(2, $user->posts()->count()); + + $this->assertEquals(1, $website->users()->count()); + + $website->delete(); + + $this->assertEquals(1, Website::count()); + $this->assertEquals(2, User::count()); + + // test with relation "delete" flag set to true + Website::extend(function ($model) { + $model->hasMany['users']['delete'] = true; + }); + + $website = Website::find($this->seeded['websites'][1]->id); + + $this->assertEquals(1, $website->users()->count()); + + $website->delete(); + + $this->assertEquals(0, Website::count()); + $this->assertEquals(1, User::count()); + } + + // tests morphMany + public function testDeleteOnMorphManyRelation() + { + $post = $this->seeded['posts'][0]; + + $this->assertEquals(2, Post::count()); + $this->assertEquals(2, Comment::count()); + + $this->assertEquals(1, $post->comments()->count()); + + $post->delete(); + + $this->assertEquals(1, Post::count()); + $this->assertEquals(2, Comment::count()); + + // test with relation "delete" flag set to true + Post::extend(function ($model) { + $model->morphMany['comments']['delete'] = true; + }); + + $post = Post::find($this->seeded['posts'][1]->id); + + $this->assertEquals(1, $post->comments()->count()); + + $post->delete(); + + $this->assertEquals(0, Post::count()); + $this->assertEquals(1, Comment::count()); + } + + // tests belongsToMany + public function testDeleteOnBelongsToManyRelation() + { + $user = $this->seeded['users'][0]; + + $this->assertEquals(2, User::count()); + $this->assertEquals(2, Role::count()); + + $this->assertEquals(1, $user->roles()->count()); + + $this->assertEquals(3, DB::table('role_user')->count()); + + $user->delete(); + + // verify that pivot record has been removed + $this->assertEquals(2, DB::table('role_user')->count()); + + // verify user has been deleted + $this->assertEquals(1, User::count()); + + // verify both roles still exist + $this->assertEquals(2, Role::count()); + + // test with relation "detach" flag set to false (default is true) + User::extend(function ($model) { + $model->belongsToMany['roles']['detach'] = false; + }); + + $user = User::find($this->seeded['users'][1]->id); + + $this->assertEquals(2, $user->roles()->count()); + $this->assertEquals(2, DB::table('role_user')->count()); + + $user->delete(); + + // verify pivot record has NOT been removed + $this->assertEquals(2, DB::table('role_user')->count()); + + // verify both roles still exist + $this->assertEquals(2, Role::count()); + } + + // tests morphToMany + public function testDeleteOnMorphToManyRelation() + { + $post = $this->seeded['posts'][0]; + + $this->assertEquals(2, Post::count()); + $this->assertEquals(2, Image::count()); + + $this->assertEquals(1, $post->images()->count()); + $this->assertEquals(1, $this->seeded['images'][0]->posts()->count()); + + $this->assertEquals(2, DB::table('imageables')->count()); + + $post->delete(); + + // verify that pivot record has been removed + $this->assertEquals(1, DB::table('imageables')->count()); + + // verify post has been deleted + $this->assertEquals(1, Post::count()); + + // verify image still exists + $this->assertEquals(2, Image::count()); + + // test with relation "detach" flag set to false (default is true) + Post::extend(function ($model) { + $model->morphToMany['images']['detach'] = false; + }); + + $post = Post::find($this->seeded['posts'][1]->id); + + $this->assertEquals(1, $post->images()->count()); + + $post->delete(); + + // verify that pivot record has NOT been removed + $this->assertEquals(1, DB::table('imageables')->count()); + + $this->assertEquals(0, Post::count()); + $this->assertEquals(2, Image::count()); + } + + // tests hasOne + public function testDeleteOnHasOneRelation() + { + $user = $this->seeded['users'][0]; + + $this->assertEquals(1, $user->phone()->count()); + $this->assertEquals(2, Phone::count()); + + $user->delete(); + + $this->assertEquals(2, Phone::count()); + + // test with relation "delete" flag set to true + User::extend(function ($model) { + $model->hasOne['phone']['delete'] = true; + }); + + $user = User::find($this->seeded['users'][1]->id); + + $this->assertEquals(1, $user->phone()->count()); + $this->assertEquals(2, Phone::count()); + + $user->delete(); + + $this->assertEquals(1, Phone::count()); + } + + // tests morphOne + public function testDeleteOnMorphOneRelation() + { + $post = $this->seeded['posts'][0]; + + $this->assertEquals(1, $post->tag()->count()); + $this->assertEquals(2, Tag::count()); + + $post->delete(); + + $this->assertEquals(2, Tag::count()); + + // test with relation "delete" flag set to true + Post::extend(function ($model) { + $model->morphOne['tag']['delete'] = true; + }); + + $post = Post::find($this->seeded['posts'][1]->id); + + $this->assertEquals(1, $post->tag()->count()); + $this->assertEquals(2, Tag::count()); + + $post->delete(); + + $this->assertEquals(1, Tag::count()); + } + + // tests belongsTo + public function testDeleteOnBelongsToRelation() + { + $phone = $this->seeded['phones'][0]; + $this->assertEquals(2, User::count()); + $this->assertEquals(2, Phone::count()); + + $phone->delete(); + + // verify phone has been deleted + $this->assertEquals(1, Phone::count()); + + // verify user has NOT been deleted + $this->assertEquals(2, User::count()); + } + + // tests morphTo + public function testDeleteOnMorphToRelation() + { + $comment = $this->seeded['comments'][0]; + $this->assertEquals(2, Comment::count()); + $this->assertEquals(2, Post::count()); + + $comment->delete(); + + // verify comment has been deleted + $this->assertEquals(1, Comment::count()); + + // verify post has NOT been deleted + $this->assertEquals(2, Post::count()); } public function testAddCasts() @@ -189,18 +568,6 @@ public function testUpsert() $this->assertEquals('2', $modelMiddleRow->value); } - protected function createTable() - { - $this->getBuilder()->create('test_model', function ($table) { - $table->increments('id'); - $table->string('name')->nullable(); - $table->text('data')->nullable(); - $table->text('description')->nullable(); - $table->text('meta')->nullable(); - $table->boolean('on_guard')->nullable(); - $table->timestamps(); - }); - } } class TestModelGuarded extends Model @@ -270,3 +637,89 @@ class TestModelMiddle extends Model public $table = 'test_model_middle'; } + +class BaseModel extends Model +{ + protected static $unguarded = true; + public $timestamps = false; +} + +class Comment extends BaseModel +{ + public $morphTo = [ + 'commentable' => [] + ]; +} + +class Image extends BaseModel +{ + public $morphedByMany = [ + 'posts' => [Post::class, 'name' => 'imageable'], + ]; +} + +class Phone extends BaseModel +{ + public $belongsTo = [ + 'user' => [User::class] + ]; +} + +class Post extends BaseModel +{ + public $belongsTo = [ + 'user' => [User::class] + ]; + public $morphOne = [ + 'tag' => [Tag::class, 'name' => 'taggable'] + ]; + public $morphMany = [ + 'comments' => [Comment::class, 'name' => 'commentable'] + ]; + public $morphToMany = [ + 'images' => [Image::class, 'name' => 'imageable'] + ]; +} + +class Role extends BaseModel +{ + public $belongsToMany = [ + 'users' => [User::class] + ]; +} + +class Tag extends BaseModel +{ + public $morphTo = [ + 'taggable' => [] + ]; +} + +class User extends BaseModel +{ + public $hasOne = [ + 'phone' => [Phone::class] + ]; + public $hasMany = [ + 'posts' => [Post::class] + ]; + public $belongsTo = [ + 'website' => [Website::class] + ]; + public $belongsToMany = [ + 'roles' => [Role::class] + ]; +} + +class Website extends BaseModel +{ + public $hasMany = [ + 'users' => [User::class] + ]; + public $hasOneThrough = [ + 'phone' => [Phone::class, 'through' => User::class] + ]; + public $hasManyThrough = [ + 'posts' => [Post::class, 'through' => User::class] + ]; +} From d0ef473258551e6cb89c17b8f9ee702f3625a73c Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 15 Oct 2023 12:34:51 -0400 Subject: [PATCH 26/30] remove empty line --- tests/Database/ModelTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Database/ModelTest.php b/tests/Database/ModelTest.php index 2307f0d02..fb0c0b532 100644 --- a/tests/Database/ModelTest.php +++ b/tests/Database/ModelTest.php @@ -567,7 +567,6 @@ public function testUpsert() $this->assertEquals('2', $modelMiddleRow->value); } - } class TestModelGuarded extends Model From e827d0aa2aaedaa985f59bda72e1d090b4afdc0b Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 15 Oct 2023 12:42:52 -0400 Subject: [PATCH 27/30] set namespace --- tests/Database/ModelTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Database/ModelTest.php b/tests/Database/ModelTest.php index fb0c0b532..4c570ced2 100644 --- a/tests/Database/ModelTest.php +++ b/tests/Database/ModelTest.php @@ -1,9 +1,11 @@ Date: Sun, 15 Oct 2023 12:55:51 -0400 Subject: [PATCH 28/30] add missing test for morphedByMany --- tests/Database/ModelTest.php | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/Database/ModelTest.php b/tests/Database/ModelTest.php index 4c570ced2..5d7659c15 100644 --- a/tests/Database/ModelTest.php +++ b/tests/Database/ModelTest.php @@ -306,6 +306,47 @@ public function testDeleteOnMorphToManyRelation() $this->assertEquals(2, Image::count()); } + // tests morphedByMany + public function testDeleteOnMorphedByManyRelation() + { + $image = $this->seeded['images'][0]; + + $this->assertEquals(2, Image::count()); + $this->assertEquals(2, Post::count()); + + $this->assertEquals(1, $image->posts()->count()); + + $this->assertEquals(2, DB::table('imageables')->count()); + + $image->delete(); + + // verify that pivot record has been removed + $this->assertEquals(1, DB::table('imageables')->count()); + + // verify image has been deleted + $this->assertEquals(1, Image::count()); + + // verify post still exists + $this->assertEquals(2, Post::count()); + + // test with relation "detach" flag set to false (default is true) + Image::extend(function ($model) { + $model->morphedByMany['posts']['detach'] = false; + }); + + $image = Image::find($this->seeded['images'][1]->id); + + $this->assertEquals(1, $image->posts()->count()); + + $image->delete(); + + // verify that pivot record has NOT been removed + $this->assertEquals(1, DB::table('imageables')->count()); + + $this->assertEquals(0, Image::count()); + $this->assertEquals(2, Post::count()); + } + // tests hasOne public function testDeleteOnHasOneRelation() { From fa4a2dccb9117606587a956962a6fb06ca7f370a Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 15 Oct 2023 14:54:28 -0400 Subject: [PATCH 29/30] make tests more resilients without hardcoded counts --- tests/Database/ModelTest.php | 222 +++++++++++++++++++---------------- 1 file changed, 118 insertions(+), 104 deletions(-) diff --git a/tests/Database/ModelTest.php b/tests/Database/ModelTest.php index 5d7659c15..6c8882737 100644 --- a/tests/Database/ModelTest.php +++ b/tests/Database/ModelTest.php @@ -132,50 +132,44 @@ protected function seedTables() } // tests hasOneThrough & hasManyThrough - public function testDeleteOnThroughRelations() + public function testDeleteWithThroughRelations() { $website = $this->seeded['websites'][0]; $user = $this->seeded['users'][0]; - $this->assertEquals(2, Phone::count()); - $this->assertEquals(2, Post::count()); + $phoneCount = Phone::count(); + $postCount = Post::count(); - $this->assertEquals(1, $website->phone()->count()); - $this->assertEquals(2, $website->posts()->count()); + $phoneRelationCount = $website->phone()->count(); + $postsRelationCount = $website->posts()->count(); - $this->assertEquals(1, $user->phone()->count()); - $this->assertEquals(2, $user->posts()->count()); + $this->assertEquals($phoneRelationCount, $user->phone()->count()); + $this->assertEquals($postsRelationCount, $user->posts()->count()); $website->delete(); - $this->assertEquals(1, $user->phone()->count()); - $this->assertEquals(2, $user->posts()->count()); + $this->assertEquals($phoneRelationCount, $user->phone()->count()); + $this->assertEquals($postsRelationCount, $user->posts()->count()); - $this->assertEquals(2, Phone::count()); - $this->assertEquals(2, Post::count()); + $this->assertEquals($phoneCount, Phone::count()); + $this->assertEquals($postCount, Post::count()); } // tests hasMany - public function testDeleteOnHasManyRelation() + public function testDeleteWithHasManyRelation() { $website = $this->seeded['websites'][0]; $user = $this->seeded['users'][0]; - $this->assertEquals(2, Phone::count()); - $this->assertEquals(2, Post::count()); - - $this->assertEquals(1, $website->phone()->count()); - $this->assertEquals(2, $website->posts()->count()); - - $this->assertEquals(1, $user->phone()->count()); - $this->assertEquals(2, $user->posts()->count()); - - $this->assertEquals(1, $website->users()->count()); + $usersRelationCount = $website->users()->count(); + $websiteCount = Website::count(); + $userCount = User::count(); $website->delete(); - $this->assertEquals(1, Website::count()); - $this->assertEquals(2, User::count()); + $this->assertEquals($usersRelationCount, $website->users()->count()); + $this->assertEquals($websiteCount - 1, Website::count()); + $this->assertEquals($userCount, User::count()); // test with relation "delete" flag set to true Website::extend(function ($model) { @@ -184,28 +178,28 @@ public function testDeleteOnHasManyRelation() $website = Website::find($this->seeded['websites'][1]->id); - $this->assertEquals(1, $website->users()->count()); + $websiteCount = Website::count(); + $userCount = User::count(); $website->delete(); - $this->assertEquals(0, Website::count()); - $this->assertEquals(1, User::count()); + $this->assertEquals($websiteCount - 1, Website::count()); + $this->assertEquals($userCount - 1, User::count()); } // tests morphMany - public function testDeleteOnMorphManyRelation() + public function testDeleteWithMorphManyRelation() { $post = $this->seeded['posts'][0]; - $this->assertEquals(2, Post::count()); - $this->assertEquals(2, Comment::count()); - - $this->assertEquals(1, $post->comments()->count()); + $commentsRelationCount = $post->comments()->count(); + $postCount = Post::count(); + $commentCount = Comment::count(); $post->delete(); - $this->assertEquals(1, Post::count()); - $this->assertEquals(2, Comment::count()); + $this->assertEquals($postCount - 1, Post::count()); + $this->assertEquals($commentCount, Comment::count()); // test with relation "delete" flag set to true Post::extend(function ($model) { @@ -214,36 +208,34 @@ public function testDeleteOnMorphManyRelation() $post = Post::find($this->seeded['posts'][1]->id); - $this->assertEquals(1, $post->comments()->count()); + $postCount = Post::count(); + $commentCount = Comment::count(); $post->delete(); - $this->assertEquals(0, Post::count()); - $this->assertEquals(1, Comment::count()); + $this->assertEquals($postCount - 1, Post::count()); + $this->assertEquals($commentCount - 1, Comment::count()); } // tests belongsToMany - public function testDeleteOnBelongsToManyRelation() + public function testDeleteWithBelongsToManyRelation() { $user = $this->seeded['users'][0]; - $this->assertEquals(2, User::count()); - $this->assertEquals(2, Role::count()); - - $this->assertEquals(1, $user->roles()->count()); - - $this->assertEquals(3, DB::table('role_user')->count()); + $userRolePivotCount = DB::table('role_user')->count(); + $userCount = User::count(); + $roleCount = Role::count(); $user->delete(); // verify that pivot record has been removed - $this->assertEquals(2, DB::table('role_user')->count()); + $this->assertEquals($userRolePivotCount - 1, DB::table('role_user')->count()); // verify user has been deleted - $this->assertEquals(1, User::count()); + $this->assertEquals($userCount - 1, User::count()); // verify both roles still exist - $this->assertEquals(2, Role::count()); + $this->assertEquals($roleCount, Role::count()); // test with relation "detach" flag set to false (default is true) User::extend(function ($model) { @@ -252,41 +244,39 @@ public function testDeleteOnBelongsToManyRelation() $user = User::find($this->seeded['users'][1]->id); - $this->assertEquals(2, $user->roles()->count()); - $this->assertEquals(2, DB::table('role_user')->count()); + $userRolePivotCount = DB::table('role_user')->count(); + $userCount = User::count(); + $roleCount = Role::count(); $user->delete(); // verify pivot record has NOT been removed - $this->assertEquals(2, DB::table('role_user')->count()); + $this->assertEquals($userRolePivotCount, DB::table('role_user')->count()); // verify both roles still exist - $this->assertEquals(2, Role::count()); + $this->assertEquals($roleCount, Role::count()); } // tests morphToMany - public function testDeleteOnMorphToManyRelation() + public function testDeleteWithMorphToManyRelation() { $post = $this->seeded['posts'][0]; + $image = $this->seeded['images'][0]; - $this->assertEquals(2, Post::count()); - $this->assertEquals(2, Image::count()); - - $this->assertEquals(1, $post->images()->count()); - $this->assertEquals(1, $this->seeded['images'][0]->posts()->count()); - - $this->assertEquals(2, DB::table('imageables')->count()); + $imageablesPivotCount = DB::table('imageables')->count(); + $postCount = Post::count(); + $imageCount = Image::count(); $post->delete(); // verify that pivot record has been removed - $this->assertEquals(1, DB::table('imageables')->count()); + $this->assertEquals($imageablesPivotCount - 1, DB::table('imageables')->count()); // verify post has been deleted - $this->assertEquals(1, Post::count()); + $this->assertEquals($postCount - 1, Post::count()); // verify image still exists - $this->assertEquals(2, Image::count()); + $this->assertEquals($imageCount, Image::count()); // test with relation "detach" flag set to false (default is true) Post::extend(function ($model) { @@ -295,39 +285,42 @@ public function testDeleteOnMorphToManyRelation() $post = Post::find($this->seeded['posts'][1]->id); - $this->assertEquals(1, $post->images()->count()); + $imageablesPivotCount = DB::table('imageables')->count(); + $postCount = Post::count(); + $imageCount = Image::count(); $post->delete(); // verify that pivot record has NOT been removed - $this->assertEquals(1, DB::table('imageables')->count()); + $this->assertEquals($imageablesPivotCount, DB::table('imageables')->count()); + + // verify post has been deleted + $this->assertEquals($postCount - 1, Post::count()); - $this->assertEquals(0, Post::count()); - $this->assertEquals(2, Image::count()); + // verify image still exists + $this->assertEquals($imageCount, Image::count()); } // tests morphedByMany - public function testDeleteOnMorphedByManyRelation() + public function testDeleteWithMorphedByManyRelation() { $image = $this->seeded['images'][0]; + $post = $this->seeded['posts'][0]; - $this->assertEquals(2, Image::count()); - $this->assertEquals(2, Post::count()); - - $this->assertEquals(1, $image->posts()->count()); - - $this->assertEquals(2, DB::table('imageables')->count()); + $imageablesPivotCount = DB::table('imageables')->count(); + $imageCount = Image::count(); + $postCount = Post::count(); $image->delete(); // verify that pivot record has been removed - $this->assertEquals(1, DB::table('imageables')->count()); + $this->assertEquals($imageablesPivotCount - 1, DB::table('imageables')->count()); // verify image has been deleted - $this->assertEquals(1, Image::count()); + $this->assertEquals($imageCount - 1, Image::count()); // verify post still exists - $this->assertEquals(2, Post::count()); + $this->assertEquals($postCount, Post::count()); // test with relation "detach" flag set to false (default is true) Image::extend(function ($model) { @@ -336,28 +329,37 @@ public function testDeleteOnMorphedByManyRelation() $image = Image::find($this->seeded['images'][1]->id); - $this->assertEquals(1, $image->posts()->count()); + $imageablesPivotCount = DB::table('imageables')->count(); + $imageCount = Image::count(); + $postCount = Post::count(); $image->delete(); // verify that pivot record has NOT been removed - $this->assertEquals(1, DB::table('imageables')->count()); + $this->assertEquals($imageablesPivotCount, DB::table('imageables')->count()); + + // verify image has been deleted + $this->assertEquals($imageCount - 1, Image::count()); - $this->assertEquals(0, Image::count()); - $this->assertEquals(2, Post::count()); + // verify post still exists + $this->assertEquals($postCount, Post::count()); } // tests hasOne - public function testDeleteOnHasOneRelation() + public function testDeleteWithHasOneRelation() { $user = $this->seeded['users'][0]; - $this->assertEquals(1, $user->phone()->count()); - $this->assertEquals(2, Phone::count()); + $userCount = User::count(); + $phoneCount = Phone::count(); $user->delete(); - $this->assertEquals(2, Phone::count()); + // verify user has been deleted + $this->assertEquals($userCount - 1, User::count()); + + // verify phone still exists + $this->assertEquals($phoneCount, Phone::count()); // test with relation "delete" flag set to true User::extend(function ($model) { @@ -366,25 +368,33 @@ public function testDeleteOnHasOneRelation() $user = User::find($this->seeded['users'][1]->id); - $this->assertEquals(1, $user->phone()->count()); - $this->assertEquals(2, Phone::count()); + $userCount = User::count(); + $phoneCount = Phone::count(); $user->delete(); - $this->assertEquals(1, Phone::count()); + // verify user has been deleted + $this->assertEquals($userCount - 1, User::count()); + + // verify phone has been deleted + $this->assertEquals($phoneCount - 1, Phone::count()); } // tests morphOne - public function testDeleteOnMorphOneRelation() + public function testDeleteWithMorphOneRelation() { $post = $this->seeded['posts'][0]; - $this->assertEquals(1, $post->tag()->count()); - $this->assertEquals(2, Tag::count()); + $postCount = Post::count(); + $tagCount = Tag::count(); $post->delete(); - $this->assertEquals(2, Tag::count()); + // verify post has been deleted + $this->assertEquals($postCount - 1, Post::count()); + + // verify tag still exists + $this->assertEquals($tagCount, Tag::count()); // test with relation "delete" flag set to true Post::extend(function ($model) { @@ -393,44 +403,48 @@ public function testDeleteOnMorphOneRelation() $post = Post::find($this->seeded['posts'][1]->id); - $this->assertEquals(1, $post->tag()->count()); - $this->assertEquals(2, Tag::count()); + $postCount = Post::count(); + $tagCount = Tag::count(); $post->delete(); - $this->assertEquals(1, Tag::count()); + // verify post has been deleted + $this->assertEquals($postCount - 1, Post::count()); + + // verify tag has been deleted + $this->assertEquals($tagCount - 1, Tag::count()); } // tests belongsTo - public function testDeleteOnBelongsToRelation() + public function testDeleteWithBelongsToRelation() { $phone = $this->seeded['phones'][0]; - $this->assertEquals(2, User::count()); - $this->assertEquals(2, Phone::count()); + $phoneCount = Phone::count(); + $userCount = User::count(); $phone->delete(); // verify phone has been deleted - $this->assertEquals(1, Phone::count()); + $this->assertEquals($phoneCount - 1, Phone::count()); // verify user has NOT been deleted - $this->assertEquals(2, User::count()); + $this->assertEquals($userCount, User::count()); } // tests morphTo - public function testDeleteOnMorphToRelation() + public function testDeleteWithMorphToRelation() { $comment = $this->seeded['comments'][0]; - $this->assertEquals(2, Comment::count()); - $this->assertEquals(2, Post::count()); + $commentCount = Comment::count(); + $postCount = Post::count(); $comment->delete(); // verify comment has been deleted - $this->assertEquals(1, Comment::count()); + $this->assertEquals($commentCount - 1, Comment::count()); // verify post has NOT been deleted - $this->assertEquals(2, Post::count()); + $this->assertEquals($postCount, Post::count()); } public function testAddCasts() From bed66508347dbe894b4fdfd9eb7a41a53390672d Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 15 Oct 2023 15:05:20 -0400 Subject: [PATCH 30/30] add more comments and cleanup --- tests/Database/ModelTest.php | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/Database/ModelTest.php b/tests/Database/ModelTest.php index 6c8882737..276384ed8 100644 --- a/tests/Database/ModelTest.php +++ b/tests/Database/ModelTest.php @@ -148,6 +148,7 @@ public function testDeleteWithThroughRelations() $website->delete(); + // verify nothing has been deleted $this->assertEquals($phoneRelationCount, $user->phone()->count()); $this->assertEquals($postsRelationCount, $user->posts()->count()); @@ -161,14 +162,15 @@ public function testDeleteWithHasManyRelation() $website = $this->seeded['websites'][0]; $user = $this->seeded['users'][0]; - $usersRelationCount = $website->users()->count(); $websiteCount = Website::count(); $userCount = User::count(); $website->delete(); - $this->assertEquals($usersRelationCount, $website->users()->count()); + // verify website has been deleted $this->assertEquals($websiteCount - 1, Website::count()); + + // verify user still exists $this->assertEquals($userCount, User::count()); // test with relation "delete" flag set to true @@ -183,7 +185,10 @@ public function testDeleteWithHasManyRelation() $website->delete(); + // verify website has been deleted $this->assertEquals($websiteCount - 1, Website::count()); + + // verify user has been deleted $this->assertEquals($userCount - 1, User::count()); } @@ -192,13 +197,15 @@ public function testDeleteWithMorphManyRelation() { $post = $this->seeded['posts'][0]; - $commentsRelationCount = $post->comments()->count(); $postCount = Post::count(); $commentCount = Comment::count(); $post->delete(); + // verify post has been deleted $this->assertEquals($postCount - 1, Post::count()); + + // verify comment still exists $this->assertEquals($commentCount, Comment::count()); // test with relation "delete" flag set to true @@ -213,7 +220,10 @@ public function testDeleteWithMorphManyRelation() $post->delete(); + // verify post has been deleted $this->assertEquals($postCount - 1, Post::count()); + + // verify comment has been deleted $this->assertEquals($commentCount - 1, Comment::count()); } @@ -228,12 +238,12 @@ public function testDeleteWithBelongsToManyRelation() $user->delete(); - // verify that pivot record has been removed - $this->assertEquals($userRolePivotCount - 1, DB::table('role_user')->count()); - // verify user has been deleted $this->assertEquals($userCount - 1, User::count()); + // verify that pivot record has been removed + $this->assertEquals($userRolePivotCount - 1, DB::table('role_user')->count()); + // verify both roles still exist $this->assertEquals($roleCount, Role::count());