Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve automatic detaching / deletion of relations #156

Merged
merged 31 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
b4b78cc
initial commit to handle special softDelete case for relations with p…
mjauvin Sep 9, 2023
b8d03da
first draft at solving the issue
mjauvin Sep 9, 2023
3ac6908
code improvements
mjauvin Sep 9, 2023
22dad8f
fix the Database Model instead of patching the Auth/User model
mjauvin Sep 10, 2023
9fac408
the delete relation option should not apply to pivot table records
mjauvin Sep 10, 2023
3a1464b
Update src/Database/Model.php
bennothommo Sep 11, 2023
01df0d9
Apply suggestions from code review
bennothommo Sep 11, 2023
8b9d6fe
make sure not to leave orphans
mjauvin Sep 11, 2023
1c1114b
no need for delete option, the relation pivot records are detached al…
mjauvin Sep 11, 2023
6672a0b
small tweak
mjauvin Sep 11, 2023
97ff392
initial sofdelete trait tests
mjauvin Sep 13, 2023
3770e94
verify the relation "detach" option
mjauvin Sep 14, 2023
b82e651
add unit test for soft delete/restore
mjauvin Sep 14, 2023
e51407c
remove empty line
mjauvin Sep 14, 2023
e35a61a
remove namespace
mjauvin Sep 14, 2023
4c9c84d
relation var only needed in else block
mjauvin Sep 14, 2023
f8face7
namespace IS required
mjauvin Sep 15, 2023
00ae0dd
skip early if null
mjauvin Sep 15, 2023
e9595b2
cleanup
mjauvin Sep 30, 2023
4ae7171
Merge branch 'develop' into wip/softdelete-pivot
mjauvin Oct 14, 2023
302ce88
do not remove has{One,Many}Through related records
mjauvin Oct 14, 2023
913af0c
belongsTo and morphTo relation should not be deleted
mjauvin Oct 14, 2023
821b8ee
Update src/Database/Model.php
LukeTowers Oct 14, 2023
85dcfc2
only handle known relation types
mjauvin Oct 14, 2023
f1355b6
apply same logic to softDeleted relations
mjauvin Oct 14, 2023
75e1865
add model delete tests for all relation types
mjauvin Oct 15, 2023
d0ef473
remove empty line
mjauvin Oct 15, 2023
e827d0a
set namespace
mjauvin Oct 15, 2023
4e625a3
add missing test for morphedByMany
mjauvin Oct 15, 2023
fa4a2dc
make tests more resilients without hardcoded counts
mjauvin Oct 15, 2023
bed6650
add more comments and cleanup
mjauvin Oct 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions src/Auth/Models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
];

public $belongsTo = [
Expand Down Expand Up @@ -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();
}
}
mjauvin marked this conversation as resolved.
Show resolved Hide resolved

//
// Persistence (used by Cookies and Sessions)
//
Expand Down
38 changes: 19 additions & 19 deletions src/Database/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -1008,32 +1009,31 @@ protected function performDeleteOnRelations()
* Hard 'delete' definition
*/
foreach ($relations as $name => $options) {
if (!Arr::get($options, 'delete', false)) {
continue;
}

if (!$relation = $this->{$name}) {
continue;
}

if ($relation instanceof EloquentModel) {
$relation->forceDelete();
}
elseif ($relation instanceof CollectionBase) {
$relation->each(function ($model) {
$model->forceDelete();
});
}
}

/*
* Belongs-To-Many should clean up after itself always
*/
if ($type == 'belongsToMany') {
foreach ($relations as $name => $options) {
if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) {
// we want to remove the pivot record, not the actual relation record
if (Arr::get($options, 'detach', true)) {
$this->{$name}()->detach();
}
} else if (in_array($type, ['belongsTo', 'hasOneThrough', 'hasManyThrough', 'morphTo'])) {
LukeTowers marked this conversation as resolved.
Show resolved Hide resolved
// the model does not own the related record, we should not remove it.
continue;
} else {
mjauvin marked this conversation as resolved.
Show resolved Hide resolved
// attachOne, attachMany, hasOne, hasMany, morphOne, morphMany
if (!Arr::get($options, 'delete', false)) {
continue;
}

if ($relation instanceof EloquentModel) {
$relation->forceDelete();
} elseif ($relation instanceof CollectionBase) {
$relation->each(function ($model) {
$model->forceDelete();
});
}
}
}
}
Expand Down
66 changes: 43 additions & 23 deletions src/Database/Traits/SoftDelete.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,24 @@ protected function performSoftDeleteOnRelations()
$definitions = $this->getRelationDefinitions();
foreach ($definitions as $type => $relations) {
foreach ($relations as $name => $options) {
if (!array_get($options, 'softDelete', false)) {
continue;
}

if (!$relation = $this->{$name}) {
continue;
}

if ($relation instanceof EloquentModel) {
$relation->delete();
if (!array_get($options, 'softDelete', false)) {
continue;
}
elseif ($relation instanceof CollectionBase) {
$relation->each(function ($model) {
$model->delete();
});
if (in_array($type, ['belongsToMany', 'morphToMany', 'morphedByMany'])) {
// relations using pivot table
$value = $this->fromDateTime($this->freshTimestamp());
$this->updatePivotDeletedAtColumn($name, $options, $value);
} else {
if ($relation instanceof EloquentModel) {
$relation->delete();
} elseif ($relation instanceof CollectionBase) {
$relation->each(function ($model) {
$model->delete();
});
}
}
}
}
Expand Down Expand Up @@ -177,6 +180,19 @@ public function restore()
return $result;
}

/**
* Update relation pivot table deleted_at column
*/
protected function updatePivotDeletedAtColumn(string $relationName, array $options, string|null $value)
{
// get deletedAtColumn from the relation options, otherwise use default
$deletedAtColumn = array_get($options, 'deletedAtColumn', 'deleted_at');

$this->{$relationName}()->newPivotQuery()->update([
$deletedAtColumn => $value,
]);
}

/**
* Locates relations with softDelete flag and cascades the restore event.
*
Expand All @@ -191,18 +207,22 @@ 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'])) {
// relations using pivot table
$this->updatePivotDeletedAtColumn($name, $options, null);
} 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();
});
}
}
}
}
Expand Down
131 changes: 131 additions & 0 deletions tests/Database/Traits/SoftDeleteTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
<?php

namespace Winter\Storm\Tests\Database\Traits;

class SoftDeleteTest extends \DbTestCase
{
protected $seeded = [];

public function setUp(): void
{
parent::setUp();

$this->seeded = [
'posts' => [],
'categories' => []
];

$this->createTables();
$this->seedTables();
}

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

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::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',
],
];
}
Loading