Merge pull request #4205 from Laravel-Backpack/fix-hasmany-fallbacks-and-defaults

Delete HasMany subentry when relation column is not nullable

Author: tabacitu

src/app/Library/CrudPanel/Traits/Create.php

@@ -256,19 +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 developer's will
         if ($forceDelete) {
             return $removedEntries->delete();
         }
 
-        if (! $relationColumnIsNullable && $modelInstance->dbColumnHasDefault($relationForeignKey)) {
-            return $removedEntries->update([$relationForeignKey => $modelInstance->getDbColumnDefault($relationForeignKey)]);
+        // get the default that could be set at database level.
+        $dbColumnDefault = $modelInstance->getDbColumnDefault($relationForeignKey);
+
+        // 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();
         }
 
-        return $removedEntries->update([$relationForeignKey => null]);
+        // 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]);
     }
 
     /**

tests/Unit/CrudPanel/CrudPanelCreateTest.php

@@ -6,6 +6,7 @@
 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 Faker\Factory;
@@ -912,6 +913,13 @@ 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 +1090,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);
+    }
 }

tests/Unit/Models/PlanetNonNullable.php

@@ -0,0 +1,59 @@
+<?php
+
+namespace Backpack\CRUD\Tests\Unit\Models;
+
+use Backpack\CRUD\app\Models\Traits\CrudTrait;
+use Illuminate\Database\Eloquent\Model;
+
+class PlanetNonNullable extends Model
+{
+    use CrudTrait;
+
+    /*
+    |--------------------------------------------------------------------------
+    | GLOBAL VARIABLES
+    |--------------------------------------------------------------------------
+    */
+
+    protected $table = 'planets_non_nullable';
+    protected $primaryKey = 'id';
+    public $timestamps = false;
+    protected $fillable = ['title'];
+    // protected $hidden = [];
+    // protected $dates = [];
+
+    /*
+    |--------------------------------------------------------------------------
+    | FUNCTIONS
+    |--------------------------------------------------------------------------
+    */
+
+    /*
+    |--------------------------------------------------------------------------
+    | RELATIONS
+    |--------------------------------------------------------------------------
+    */
+
+    public function user()
+    {
+        return $this->belongsTo('Backpack\CRUD\Tests\Unit\Models\User');
+    }
+
+    /*
+    |--------------------------------------------------------------------------
+    | SCOPES
+    |--------------------------------------------------------------------------
+    */
+
+    /*
+    |--------------------------------------------------------------------------
+    | ACCESORS
+    |--------------------------------------------------------------------------
+    */
+
+    /*
+    |--------------------------------------------------------------------------
+    | MUTATORS
+    |--------------------------------------------------------------------------
+    */
+}

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');

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);

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']]);