From 77284d9dc0ec8b6dfc14e9cbcfdfe35722ae886b Mon Sep 17 00:00:00 2001 From: Pedro X Date: Wed, 16 Feb 2022 10:54:36 +0000 Subject: [PATCH 1/6] fix hasmany with defaults, fallback and db default. add tests --- src/app/Library/CrudPanel/Traits/Create.php | 14 +++-- tests/Unit/CrudPanel/CrudPanelCreateTest.php | 42 +++++++++++++ tests/Unit/Models/PlanetNonNullable.php | 59 +++++++++++++++++++ tests/Unit/Models/User.php | 5 ++ ...4745_create_pivotable_relations_tables.php | 6 ++ .../database/seeds/MorphableSeeders.php | 8 +++ 6 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 tests/Unit/Models/PlanetNonNullable.php diff --git a/src/app/Library/CrudPanel/Traits/Create.php b/src/app/Library/CrudPanel/Traits/Create.php index 396615bdc5..501ec6b9bf 100644 --- a/src/app/Library/CrudPanel/Traits/Create.php +++ b/src/app/Library/CrudPanel/Traits/Create.php @@ -264,11 +264,15 @@ private function handleManyRelationItemRemoval($modelInstance, $removedEntries, return $removedEntries->delete(); } - if (! $relationColumnIsNullable && $modelInstance->dbColumnHasDefault($relationForeignKey)) { - return $removedEntries->update([$relationForeignKey => $modelInstance->getDbColumnDefault($relationForeignKey)]); - } - - return $removedEntries->update([$relationForeignKey => null]); + $dbColumnDefault = $modelInstance->getDbColumnDefault($relationForeignKey); + + if ($relationColumnIsNullable) { + return $removedEntries->update([$relationForeignKey => $dbColumnDefault]); + }else{ + return !is_null($dbColumnDefault) ? + $removedEntries->update([$relationForeignKey => $dbColumnDefault]) : + $removedEntries->delete(); + } } /** diff --git a/tests/Unit/CrudPanel/CrudPanelCreateTest.php b/tests/Unit/CrudPanel/CrudPanelCreateTest.php index 271c255a4f..0e747af855 100644 --- a/tests/Unit/CrudPanel/CrudPanelCreateTest.php +++ b/tests/Unit/CrudPanel/CrudPanelCreateTest.php @@ -8,6 +8,7 @@ use Backpack\CRUD\Tests\Unit\Models\Planet; use Backpack\CRUD\Tests\Unit\Models\Universe; use Backpack\CRUD\Tests\Unit\Models\User; +use Backpack\CRUD\Tests\Unit\Models\PlanetNonNullable; use Faker\Factory; use Illuminate\Support\Arr; @@ -912,6 +913,14 @@ public function testHasManyCreatableRelationship() $this->assertEquals($inputData['universes'][0]['title'], $entry->fresh()->universes->first()->title); $this->assertEquals(3, $entry->fresh()->universes->first()->id); $this->assertEquals(1, Universe::all()->count()); + + $inputData['universes'] = null; + + $this->crudPanel->update($entry->id, $inputData); + + $this->assertEquals(0, count($entry->fresh()->universes)); + $this->assertEquals(0, Universe::all()->count()); + } public function testHasManySelectableRelationshipWithoutForceDelete() @@ -1082,4 +1091,37 @@ public function testHasManySelectableRelationshipNonNullableForeignKeyAndDefault $this->assertCount(2, $comets); $this->assertEquals(0, $comets->first()->user_id); } + + public function testHasManySelectableRelationshipNonNullable() + { + $this->crudPanel->setModel(User::class); + $this->crudPanel->addFields($this->userInputFieldsNoRelationships, 'both'); + $this->crudPanel->addField([ + 'name' => 'planetsNonNullable', + 'force_delete' => false, + 'fallback_id' => false, + ], 'both'); + + $faker = Factory::create(); + $inputData = [ + 'name' => $faker->name, + 'email' => $faker->safeEmail, + 'password' => bcrypt($faker->password()), + 'remember_token' => null, + 'planetsNonNullable' => [1, 2], + ]; + + $entry = $this->crudPanel->create($inputData); + + $this->assertCount(2, $entry->planetsNonNullable); + + $inputData['planetsNonNullable'] = null; + + $this->crudPanel->update($entry->id, $inputData); + + $this->assertCount(0, $entry->fresh()->planetsNonNullable); + + $planets = PlanetNonNullable::all(); + $this->assertCount(0, $planets); + } } diff --git a/tests/Unit/Models/PlanetNonNullable.php b/tests/Unit/Models/PlanetNonNullable.php new file mode 100644 index 0000000000..695ead4def --- /dev/null +++ b/tests/Unit/Models/PlanetNonNullable.php @@ -0,0 +1,59 @@ +belongsTo('Backpack\CRUD\Tests\Unit\Models\User'); + } + + /* + |-------------------------------------------------------------------------- + | SCOPES + |-------------------------------------------------------------------------- + */ + + /* + |-------------------------------------------------------------------------- + | ACCESORS + |-------------------------------------------------------------------------- + */ + + /* + |-------------------------------------------------------------------------- + | MUTATORS + |-------------------------------------------------------------------------- + */ +} diff --git a/tests/Unit/Models/User.php b/tests/Unit/Models/User.php index af5445b639..3c16cc242f 100644 --- a/tests/Unit/Models/User.php +++ b/tests/Unit/Models/User.php @@ -76,6 +76,11 @@ public function planets() return $this->hasMany('Backpack\CRUD\Tests\Unit\Models\Planet'); } + public function planetsNonNullable() + { + return $this->hasMany('Backpack\CRUD\Tests\Unit\Models\PlanetNonNullable'); + } + public function comets() { return $this->hasMany('Backpack\CRUD\Tests\Unit\Models\Comet'); diff --git a/tests/config/database/migrations/2020_03_31_114745_create_pivotable_relations_tables.php b/tests/config/database/migrations/2020_03_31_114745_create_pivotable_relations_tables.php index 87989b4eae..000cfae0af 100644 --- a/tests/config/database/migrations/2020_03_31_114745_create_pivotable_relations_tables.php +++ b/tests/config/database/migrations/2020_03_31_114745_create_pivotable_relations_tables.php @@ -75,6 +75,12 @@ public function up() $table->string('title')->nullable(); }); + Schema::create('planets_non_nullable', function (Blueprint $table) { + $table->bigIncrements('id'); + $table->bigInteger('user_id'); + $table->string('title')->nullable(); + }); + Schema::create('comets', function (Blueprint $table) { $table->bigIncrements('id'); $table->bigInteger('user_id')->default(0); diff --git a/tests/config/database/seeds/MorphableSeeders.php b/tests/config/database/seeds/MorphableSeeders.php index 32f8c746bb..f6137b8d3a 100644 --- a/tests/config/database/seeds/MorphableSeeders.php +++ b/tests/config/database/seeds/MorphableSeeders.php @@ -43,6 +43,14 @@ public function run() 'title' => $faker->title, ]]); + DB::table('planets_non_nullable')->insert([[ + 'title' => $faker->title, + 'user_id' => '' + ], [ + 'title' => $faker->title, + 'user_id' => '' + ]]); + DB::table('comets')->insert([['user_id' => ''], ['user_id' => '']]); DB::table('bangs')->insert([['name' => 'bang1'], ['name' => 'bang2']]); From f4b5a6a05fd769d856f6133dbb79b13bd7682068 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Wed, 16 Feb 2022 10:54:50 +0000 Subject: [PATCH 2/6] Apply fixes from StyleCI [ci skip] [skip ci] --- src/app/Library/CrudPanel/Traits/Create.php | 8 ++++---- tests/Unit/CrudPanel/CrudPanelCreateTest.php | 3 +-- tests/config/database/seeds/MorphableSeeders.php | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/app/Library/CrudPanel/Traits/Create.php b/src/app/Library/CrudPanel/Traits/Create.php index 501ec6b9bf..fe20c4914d 100644 --- a/src/app/Library/CrudPanel/Traits/Create.php +++ b/src/app/Library/CrudPanel/Traits/Create.php @@ -268,11 +268,11 @@ private function handleManyRelationItemRemoval($modelInstance, $removedEntries, if ($relationColumnIsNullable) { return $removedEntries->update([$relationForeignKey => $dbColumnDefault]); - }else{ - return !is_null($dbColumnDefault) ? + } else { + return ! is_null($dbColumnDefault) ? $removedEntries->update([$relationForeignKey => $dbColumnDefault]) : - $removedEntries->delete(); - } + $removedEntries->delete(); + } } /** diff --git a/tests/Unit/CrudPanel/CrudPanelCreateTest.php b/tests/Unit/CrudPanel/CrudPanelCreateTest.php index 0e747af855..beed90367c 100644 --- a/tests/Unit/CrudPanel/CrudPanelCreateTest.php +++ b/tests/Unit/CrudPanel/CrudPanelCreateTest.php @@ -6,9 +6,9 @@ use Backpack\CRUD\Tests\Unit\Models\Bang; use Backpack\CRUD\Tests\Unit\Models\Comet; use Backpack\CRUD\Tests\Unit\Models\Planet; +use Backpack\CRUD\Tests\Unit\Models\PlanetNonNullable; use Backpack\CRUD\Tests\Unit\Models\Universe; use Backpack\CRUD\Tests\Unit\Models\User; -use Backpack\CRUD\Tests\Unit\Models\PlanetNonNullable; use Faker\Factory; use Illuminate\Support\Arr; @@ -920,7 +920,6 @@ public function testHasManyCreatableRelationship() $this->assertEquals(0, count($entry->fresh()->universes)); $this->assertEquals(0, Universe::all()->count()); - } public function testHasManySelectableRelationshipWithoutForceDelete() diff --git a/tests/config/database/seeds/MorphableSeeders.php b/tests/config/database/seeds/MorphableSeeders.php index f6137b8d3a..e20498b197 100644 --- a/tests/config/database/seeds/MorphableSeeders.php +++ b/tests/config/database/seeds/MorphableSeeders.php @@ -45,10 +45,10 @@ public function run() DB::table('planets_non_nullable')->insert([[ 'title' => $faker->title, - 'user_id' => '' + 'user_id' => '', ], [ 'title' => $faker->title, - 'user_id' => '' + 'user_id' => '', ]]); DB::table('comets')->insert([['user_id' => ''], ['user_id' => '']]); From d0c6d51f8c0cb335910e34fa9f0f6f401bdb7db9 Mon Sep 17 00:00:00 2001 From: Pedro Martins Date: Thu, 17 Feb 2022 10:39:10 +0000 Subject: [PATCH 3/6] Update src/app/Library/CrudPanel/Traits/Create.php Co-authored-by: Cristian Tabacitu --- src/app/Library/CrudPanel/Traits/Create.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/app/Library/CrudPanel/Traits/Create.php b/src/app/Library/CrudPanel/Traits/Create.php index fe20c4914d..ed3a6d311a 100644 --- a/src/app/Library/CrudPanel/Traits/Create.php +++ b/src/app/Library/CrudPanel/Traits/Create.php @@ -266,12 +266,10 @@ private function handleManyRelationItemRemoval($modelInstance, $removedEntries, $dbColumnDefault = $modelInstance->getDbColumnDefault($relationForeignKey); - if ($relationColumnIsNullable) { + if ($relationColumnIsNullable || $dbColumnDefault !== null) { return $removedEntries->update([$relationForeignKey => $dbColumnDefault]); } else { - return ! is_null($dbColumnDefault) ? - $removedEntries->update([$relationForeignKey => $dbColumnDefault]) : - $removedEntries->delete(); + return $removedEntries->delete(); } } From 183f229f6a0a720c94068358885f4c031c79f07d Mon Sep 17 00:00:00 2001 From: Pedro X Date: Mon, 21 Feb 2022 10:06:22 +0000 Subject: [PATCH 4/6] add comments, refactor condition --- src/app/Library/CrudPanel/Traits/Create.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/app/Library/CrudPanel/Traits/Create.php b/src/app/Library/CrudPanel/Traits/Create.php index ed3a6d311a..643238f188 100644 --- a/src/app/Library/CrudPanel/Traits/Create.php +++ b/src/app/Library/CrudPanel/Traits/Create.php @@ -256,21 +256,28 @@ private function handleManyRelationItemRemoval($modelInstance, $removedEntries, $forceDelete = $relationDetails['force_delete'] ?? false; $fallbackId = $relationDetails['fallback_id'] ?? false; + // developer provided a fallback_id he knows what he's doing, just use it. if ($fallbackId) { return $removedEntries->update([$relationForeignKey => $fallbackId]); } + // developer set force_delete => true, so we don't care if it's nullable or not, + // we just follow developers will. if ($forceDelete) { return $removedEntries->delete(); } + // get the default that could be set at database level. $dbColumnDefault = $modelInstance->getDbColumnDefault($relationForeignKey); - if ($relationColumnIsNullable || $dbColumnDefault !== null) { - return $removedEntries->update([$relationForeignKey => $dbColumnDefault]); - } else { + // if column is not nullable in database, and there is no column default (null), + // we will delete the entry from the database, otherwise it will throw and ugly DB error. + if (!$relationColumnIsNullable && $dbColumnDefault === null) { return $removedEntries->delete(); } + + // if column is nullable we just set it to the column default (null when it does exist, or the default value when it does). + return $removedEntries->update([$relationForeignKey => $dbColumnDefault]); } /** From 6ee7cbd0aa712aeff5ef6696d9b87bf9a84488d0 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Mon, 21 Feb 2022 10:06:38 +0000 Subject: [PATCH 5/6] Apply fixes from StyleCI [ci skip] [skip ci] --- src/app/Library/CrudPanel/Traits/Create.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/Library/CrudPanel/Traits/Create.php b/src/app/Library/CrudPanel/Traits/Create.php index 643238f188..a4f7f71db8 100644 --- a/src/app/Library/CrudPanel/Traits/Create.php +++ b/src/app/Library/CrudPanel/Traits/Create.php @@ -262,7 +262,7 @@ private function handleManyRelationItemRemoval($modelInstance, $removedEntries, } // developer set force_delete => true, so we don't care if it's nullable or not, - // we just follow developers will. + // we just follow developers will. if ($forceDelete) { return $removedEntries->delete(); } @@ -272,7 +272,7 @@ private function handleManyRelationItemRemoval($modelInstance, $removedEntries, // if column is not nullable in database, and there is no column default (null), // we will delete the entry from the database, otherwise it will throw and ugly DB error. - if (!$relationColumnIsNullable && $dbColumnDefault === null) { + if (! $relationColumnIsNullable && $dbColumnDefault === null) { return $removedEntries->delete(); } From 3c15e72c1cc52e930a7e04c4dcc7bc72e682bf29 Mon Sep 17 00:00:00 2001 From: Cristian Tabacitu Date: Mon, 21 Feb 2022 17:54:02 +0200 Subject: [PATCH 6/6] Update src/app/Library/CrudPanel/Traits/Create.php --- src/app/Library/CrudPanel/Traits/Create.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/Library/CrudPanel/Traits/Create.php b/src/app/Library/CrudPanel/Traits/Create.php index a4f7f71db8..8dd6a8827a 100644 --- a/src/app/Library/CrudPanel/Traits/Create.php +++ b/src/app/Library/CrudPanel/Traits/Create.php @@ -262,7 +262,7 @@ private function handleManyRelationItemRemoval($modelInstance, $removedEntries, } // developer set force_delete => true, so we don't care if it's nullable or not, - // we just follow developers will. + // we just follow developer's will if ($forceDelete) { return $removedEntries->delete(); }