When softDelete, getDirty is empty in deleted event

[hulkur]

• Laravel Version: 7.12.0
• PHP Version: 7.4.6
• Database Driver & Version:

Description:

When soft-deleting a model changes are not visible to deleted event handlers as syncOriginal is called before firing the event.

In real and forced delete DB record is deleted, syncOriginal is not needed and everything is ok.

In soft-delete:

• Model::delete() calls $this->performDeleteOnModel() (goes to SoftDeletes trait method)
  • SoftDeletes::performDeleteOnModel calls runSoftDelete
  • SoftDeletes::runSoftDelete updates DB record and calls $this->syncOriginalAttributes
• Model::delete() fires deleted event

Result is that in deleted event handlers model getDirty() is empty but it should contain at least deleted_at attribute.

Proposal:

Make the order similar to update which soft-delete in reality is. (as in Model::finishSave)

Move $this->fireModelEvent('deleted', false); from Model::delete to the end of Model::performDeleteOnModel and add to softDeletes::runSoftDelete between update and syncOriginal

$query->update($columns);

$this->fireModelEvent('deleted', false);

$this->syncOriginalAttributes(array_keys($columns));

This way all the changes done in softDelete logic is visible to deleted event handlers.

Note: This creates BC breaks if someone has overwritten performDeleteOnModel or runSoftDelete in their app.

PR or not to PR ?

[taylorotwell]

I'm curious what the use case would be for dirty attributes in a soft delete operation? Aren't you just calling $foo->delete()? What is being updated?

[hulkur]

Yes, I call ->delete() and in runSoftDelete() I have more changes to the model (ex: deleted_by).
Then in deleted event handler change set is stored for auditing. This requires usage of getDirty().

[hulkur]

This can be done without BC break too - return boolean from performDeleteOnModel and fire event based on that.

Currently performDeleteOnModel returns null so it should return false on Model and true on SoftDeletes. SoftDeletes still has to fire the event itself.

Not so nice, but an option.

[taylorotwell]

Converted to discussion as this is a feature request IMO.

[cschra]

I am hooking in here, as I have a similar issue.

In my opinion it seems logical to have the "deleted_at" field dirty on delete. (as well as created_at and updated_at are dirty on create/update events). In the current version no state changes are reported on delete, which is fine in my opinion as long as there is no soft-delete. However, with soft delete enabled it is not logical to have getDirty return an empty array when in reality the deleted_at timestamp changed.

I want to make clear that this differs from other requests, that want to combine save() and delete() operations (i.e. #36064 and #18119). I am not requesting an Eloquent core change, changing a model and deleting it should remain two seperate actions. But my intention is to convince you to a uniform behavior across the different model actions.

The current workaround for me is to check if deleted_at is still null, but this does not feel "framework native" to me.
I hope you reconsider. I am happy to discuss, provide a PR and/or tests for this scenario.

kind regards

[ScriptingBeating]

I am having the same issue I thought deleted_at should be returned when you check for getDirty() attributes. IMO this should not be considered a feature request but a bug

[SalemC]

I have the same issue here, I'm using model events to track changes to model attributes with the $model->getDirty() method.

I'd argue 'deleted_at' not being returned by the getDirty method when its value has changed - therefore, making it 'dirty' - is unexpected behavior. Even more so when looking at the method description: "Get the attributes that have been changed since the last sync." - 'deleted_at' is an attribute and it has changed since last sync, so why isn't it included in the return value?

[doraemonxxx]

+1

I have the same issue. Thanks

[ZacharyDuBois]

I would also argue that $model->getDirty() in context of a soft deleted event should return the updated_at and deleted_at attributes as they were touched. This is inconsistent with the restored event that will return created_at and updated_at but not deleted_at. Additionally, the restored event fires the updated event too.

[nunodonato]

Going to add a +1 here as well. Spent a lot of time trying to debug an issue and turns out it was because of this :(

[nikrovir]

Have same problem. +1