Delete already deleted

[mgralikowski]

Post -> comments (3). One comment is already deleted. Post::first()->delete(); After call this all 3 comments has deleted_at set in one moment. Is this how it should works? Restoring post cause restore all 3 comments, i think should only 2 comment which was deleted with post.

[maguilar92]

@mgralikowski At now we does not have any way to detect what "comments" already was deleted or what comments was deleted on post delete.

You're are free to contribute by submitting a pull request.

[mgralikowski]

Why don't? It's Laravel limitation or sth?

We know what comments already deleted, we can detect it by deleted_at timestamp. Also we know what comments has been deleted with a primary object - again by timestamps.

Before:

Post Comment - Deleted at 2018-01-01 00:00:00 Comment Comment

After cascading delete (we don't touch already deleted): Post - Deleted at 2018-08-21 00:00:00 Comment - Deleted at 2018-01-01 00:00:00 Comment - Deleted at 2018-08-21 00:00:00 Comment - Deleted at 2018-08-21 00:00:00

Restore Post (we restoring comments only with same timestamp): Post Comment - Deleted at 2018-01-01 00:00:00 Comment Comment

[maguilar92]

From one eliminated to another may happen a second, it is not something accurate.

[maguilar92]

@mgralikowski You're feee to contribute by submitting a pull request to apply this feature. I will study how to implement it in an effective and exact way.

[mgralikowski]

I will try if you agree with my reasoning otherwise it doesn't make sense to me ;)

My first question is is about current state of you library. If i delete Post with 10000 comments and deleting will take more than one second then what will we be timestamp for this comments? Do You set current timestamp for every loop or same, "static" timestamp for all child objects?

[maguilar92]

It is laravel who sets the deleted_at column when you delete the record.

The package makes a tour of the relationships you specify by doing a delete so the date may not match.

You could see \Illuminate\Database\Eloquent\SoftDeletingScope extend method, laravel do:

        $builder->onDelete(function (Builder $builder) {
            $column = $this->getDeletedAtColumn($builder);

            return $builder->update([
                $column => $builder->getModel()->freshTimestampString(),
            ]);
        });

[mgralikowski]

So unfortunately the first step is overwrite this schema and use own, custom update with single, common deleted_at variable.

[maguilar92]

Yes, or maybe create a polymorphic relation with deletable_id and deleteable_type. Then on delete set this columns and on restore check this columns instead of deleted_at.

[dmcbrn]

Hi, could this work by adding a deleted_by_cascade boolean flag to 'children' model? And then the library would only trigger restore() method on children where deleted_by_cascade is false. Also, current state should be to run the delete() only on non-deletded children instances, right?

[gcphost]

I was thinking along that line but using a revision number, or timestamp, instead of a boolean flag.

Maybe a timestamp like cascaded_at or something _at.

[dmcbrn]

I'm thinking _at might not be needed as the timestamp is already saved in deleted_at column.

Revision number might be a better improvement. Let me think at loud a bit:

Category -> Post -> Comment

You delete comment and it gets deleted_at. Sibling comments (the ones belonging to the same post) are still there.

You delete Post and all sibling comments get deleted_at and also flag deleted_by_cascade.

Post's sibling posts (belonging to the same Category) are not deleted.

You delete Category and sibling posts get deleted_at and deleted_by_cascade.

Now the issue is what happens to sibling post's comments.

You need to flag them with the name of the model deleted or a hierachy level. In that case the column should be an integer, or better a varchar type for the name of the model. In that case it should work with polymorphic relations also.

---

I'll leave this above so that it's clear where I'm coming from.

My suggestion now is to add the deleted_by_cascade_by_model (maybe a nicer name) varchar column in which we save the name of the model which soft deleted child model instance by cascade. Index the column. This should keep all the relevant information about the cascade soft delete. Somebody just needs to do the code :)

[mgralikowski]

Guys, I'm still for my solution. It doesn't need any additional columns, is easy and cover 99.99% use cases. I am not enough pro to make pull request but i wrote my custom, static code and this code works as i expected.

Take a look for example:

We have a Post with 2000 Comments. One comment is already deleted by author so deleted_at has been set date in past. Now we delete Post as Admin:

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

        static::deleted(function($post) {
            foreach ($post->comments as $comment) {
                /** @var $comment Comment*/
                $comment->delete();
            }
        });
    }

Everything works fine, comments are deleted with a parent article. Already deleted is not touched by this loop. The only problem is timestamp, because we have so much posts and deleting is in loop there can be some difference (one second - is enough to brake my concept). So we need hack this.

<?php
namespace App\Traits;
use Illuminate\Support\Carbon;

trait HasTimestampsCustom
{
    public function freshTimestamp()
    {
        if (session()->has('deletedAtTimeStamp')) {
            return session()->get('deletedAtTimeStamp');
        } else {
            $deletedAtTimeStamp = new Carbon;
            session()->flash('deletedAtTimeStamp', $deletedAtTimeStamp);
            return $deletedAtTimeStamp;
        }
    }
} 

It is a custom Trait, we need use it in all models to overwrite timestamp.

    use SoftDeletes, HasTimestampsCustom;

Now we are sure that Post and Comments has same date and time. So we can distinguish Comments deleted by author and by admin. It's fails if admin delete Post in one moment with Author who want delete his comments but it is so unlikely.. Even if it happens i think it's still better than current state of this library - restoring all.

Finally code with restoring:

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

        static::deleted(function($post) {
            foreach ($post->comments as $comment) {
                /** @var $comment Comment*/
                $comment->delete();
            }
        });

        static::restoring(function($post) {
            /** @var $post Post */
            foreach ($post->comments()->withTrashed()->get() as $comment) {
                /** @var $comment Comment*/
                if ($comment->trashed() && $comment->{$comment->getDeletedAtColumn()} == $post->{$comment->getDeletedAtColumn()}) {
                    $comment->restore();
                }
            }
        });
    }

What do You think?

[Siva-93]

Any standard solution for this? without writing a custom code?

[maguilar92]

I will create new feature that create intermediate table to trace who deleted it, only with timestamp could break when two models deletes in soft cascade on same table.

[maguilar92]

You are free to make a PR with the improvement. I'm not using package anymore and only I will review PR's.