2

Summary

I'm trying to let my model controller delete multiple models (selected by an array of IDs) including their selected related models (selected by an array of relationship names).

Situation

  • Model Post with relationships:

    • (One-to-Many) with Comment model under $post->comments()
    • (One-to-Many) with Image model under $post->images()
    • (Many-to-One) with User model under $post->user()
  • In PostController there's the destroy_multiple method that handles the deletion, and where I have:
    • Array $ids with IDs of Post models to delete (e.g. [1,2,4])
    • Array $related_models with relationship names to delete as well (e.g. ['user','comments'] but could be a different selection in each call)

Attempts

1) Iterate and delete:

Post::findMany($ids)->each(function($item) use ($related_models) {
    foreach ($related_models as $relation) {
        $item->{$relation}()->delete();
    }
    $item->delete();
});

Problem: All models have to be retrieved first, and for each model, all selected related models have to be deleted. This is a LOT of overhead.

2) Delete related models and models:

// For every selected relationship, delete related models
foreach ($related_models as $relation) {
    $class = 'App\\' . studly_case(str_singular($relation));
    $class::whereIn('post_id', $ids)->delete();
}

// Delete the models
Post::destroy($ids);

Problem: This only works for the one-to-many relationships and only provided the database columns are named according to the Laravel standard.

Question

What is the most efficient way to do this, while:

  • using the defined relationships to ensure the right database naming, columns, etc (as in Attempt 1)
  • keeping the performance (without having to retrieve the models) (as in Attempt 2)
  • keeping the optionality of choosing the $ids and $related_models

?

Notes

  • I know that deleting a parent model (User in this case) is not very good practice, but it's there for the sake of the question. ;)
  • Using the database CASCADE constraint on the foreign keys of the related models' tables (in the migration) makes it lose the optionality. Related models are now always deleted, and also in other instances besides the delete_multiple method.
Philip
  • 2,888
  • 2
  • 24
  • 36

3 Answers3

0

You can update your migration like this:

Schema::table('comments', function (Blueprint $table) {
    ...
    $table->unsignedInteger('posts_id');
    $table->foreign('posts_id')
          ->references('id')
          ->on('posts')
          ->onDelete('cascade');
});

The above migration will auto remove every comment related to the post you are deleting.

dqhuy78
  • 1
  • 2
  • True, but then I loose the optionality of choosing which related models to delete at that time. As well as having them deleted in all deletion instances, not just through `delete_multiple`. – Philip Feb 18 '19 at 04:20
  • You can apply it to only the related model you want to delete. So when remove a Post, any related models you want to remove with it will have the `onDelete('cascade')` in their migration file. In your case is the User and Comment model, with the given Post's id `[1, 2, 4]` you can do just one line: `Post::whereIn('id', $arrayOfId)->delete()` – dqhuy78 Feb 18 '19 at 04:28
  • I know, but I want the user to choose if the related models should be deleted or not (hence the `$related_models` array). – Philip Feb 18 '19 at 04:30
  • I do appreciate the suggestion, however :) – Philip Feb 18 '19 at 04:31
0

You should delete the Relationship models first

Add this method in your Post model:

public function delete()
{
    $this->images()->delete();
    $this->comments()->delete();

    return parent::delete(); 
}

Call from your controller logic This will delete the relationship model first and then delete itself

Post::findMany($ids)->each(function ($item) {
        $item->delete();
    });
Halfacht
  • 924
  • 1
  • 12
  • 22
  • That's Attempt 1, which has a lot of overhead (db and memory). And in your code it always deletes all the related models (not just the ones the user has selected). – Philip Feb 18 '19 at 06:35
  • @Philip He ask for the same. He want to delete all ids data which is passed in $ids variable – Harish Patel Feb 18 '19 at 06:36
  • All `post` models with IDs in `$ids` and only the related models as provided by `$related_models`. So in some cases none, in other cases one or more of the mentioned relationships based on what the user has selected in the 'confirm delete' modal. – Philip Feb 18 '19 at 06:42
0

How about declaring the delete method in your model.

class Post extends Eloquent
{
    public function comments()
    {
        return $this->hasMany(Comment::class);
    }

    public function images()
    {
        return $this->hasMany(Images::class);
    }

    public static function boot() {
        parent::boot();

        static::deleting(function($post) { 
                $post->comments()->delete();
                $post->images()->delete();
                // or call another method here after you declare above
        });
    }
}

So in your controller, call the simple delete function.

$post = Post::find($id);
$post->delete();

I use this in my delete relationship function from another answer https://stackoverflow.com/a/20108037/15909588

Karl Hill
  • 12,937
  • 5
  • 58
  • 95
  • In this solution there is no parameter telling the deleting event which relations to delete as well and which to leave alone. Do you know of a way to forward a custom parameter/argument to the deleting event handler? (So you could do `$model->delete( ['comments'] )`, leaving the `images` alone.) – Philip Apr 28 '22 at 20:51