[5.6] Fix: soft-delete does not sync original

[TBlindaruk]

Actually, it is resubmitted for #24399

When we run a soft delete, then actually the original array is not changed, and it causes a problem for restoring after the soft deleting in one model.

So the steps to reproduce the bug:

1. find in the database
2. soft delete the found model
3. restore the foud model
4. find the same model in database

Expectations: model will be restored
Actual result: the model is soft deleted

I have added a test.

    /**
     * @throws \Exception
     */
    public function testUpdateModelAfterSoftDeleting()
    {
        $now = Carbon::now();
        $this->createUsers();

        /** @var SoftDeletesTestUser $userModel */
        $userModel = SoftDeletesTestUser::find(2);
        $userModel->delete();
        $this->assertEquals($now->toDateTimeString(), $userModel->getOriginal('deleted_at'));
        $this->assertNull(SoftDeletesTestUser::find(2));
        $this->assertEquals($userModel, SoftDeletesTestUser::withTrashed()->find(2));
    }
    
    /**
     * @throws \Exception
     */
    public function testRestoreAfterSoftDelete()
    {
        $this->createUsers();
    
        /** @var SoftDeletesTestUser $userModel */
        $userModel = SoftDeletesTestUser::find(2);
        $userModel->delete();
        $userModel->restore();
        
        $this->assertEquals($userModel->id, SoftDeletesTestUser::find(2)->id);
    }

[taylorotwell]

I feel like we used to have the code this way and it got removed because of some other bug it causes? Did you search through the old PRs for soft deletes?

[TBlindaruk]

@taylorotwell ,

• I have looked through the commits on the Database/Eloquent/SoftDeletes.php file. And I can not fond the syncOriginal method.

• have searched in all the repository commits - also not found a syncOriginal()

• In the old PR for soft deleting I can not find the problem related to this too

[TBlindaruk]

I have the little bit changed the calling syncOrigin method to as it in the 'save' method.

[TBlindaruk]

@taylorotwell ?

[mlanin]

This fix destroys original logic completely. original stores the state of the model when it was retrieved. It helps to check what was changed.
With this fix we can't do it, because it seems like model wasn't changed at all. original and attributes store the same values, and both contain deleted_at. This behaviour completely broke my code!

[TBlindaruk]

@mlanin , sorry ;(

[edbentinck]

I really don't understand this change. It causes the 'deleted' model event to behave differently to 'created' and 'updated'. As @mlanin says, with this change you can no longer retrieve changed values.

Since you can manually call syncOriginal anyway, surely it would be more desirable to leave the behavior of deleted in line with the other model events than to break this behavior for everyone without offering an opt-out:

$userModel->delete();
$userModel->syncOriginal();
$userModel->restore();

[TBlindaruk]

@edbentinck you can create PR for revert this.

[mfn]

I feel like we used to have the code this way and it got removed because of some other bug it causes? Did you search through the old PRs for soft deletes?

@TBlindaruk please revert, see also Taylors concern

[TBlindaruk]

@edbentinck Thanks!

[robsonvn]

The deleted_at field should be synced after update for sure, but not all the atributes. We have to find a middle term and not pretent that the bug doesn't exists.

        $updated = $query->update($columns);

        if ($updated) {
            foreach(array_keys($columns) as $column) {
                $this->syncOriginalAttribute($column);
            }
        }