Model fresh() & refresh() behavior strange with SoftDelete trait

[woodspire]

@taylorotwell

Adding the SoftDelete trait to the user model.

$user->delete();

$this->assertsNull($user->fresh());
$this->assertsNull($user->refresh()); 

Both asserts fail, because fresh() & refresh() use a query WITHOUT scopes, which exclude the SoftDelete trait scope.

I think we should either:

• fix the code in laravel/framework so that it returns null.
• add note in laravel/doc eloquent softdelete section to explain that fresh() & refresh() only check if the entry exist in the db with the primary key.

Note: The returned model does contains the deleted_at attribute with a date value. $user->trashed() is correct.

[hamzaelmaghari]

Hello @woodspire
It seems there are a few potential avenues for improvement:

Firstly, adjusting the fresh() and refresh() methods in the Laravel framework to consider SoftDeletes could be a solid solution. By incorporating the SoftDeletes scope into these methods, we ensure they return null when a model has been soft deleted.

Additionally, updating the Laravel documentation's Eloquent SoftDelete section to clarify that fresh() and refresh() only check for the existence of the entry in the database with the primary key, regardless of soft deletion status, would be beneficial.

We could also consider introducing new methods tailored specifically for handling soft deleted models. Perhaps implementing softFresh() or softRefresh() methods to explicitly fetch soft deleted models when needed could offer developers a more direct way to work with them.

Engaging with the Laravel community on this issue might also be helpful. Opening a discussion via GitHub issues or forums could gather additional insights and perspectives, ultimately leading to a more collaborative and widely accepted solution.

By exploring these avenues, we can enhance the functionality of SoftDeletes within Laravel, providing developers with clearer documentation and more reliable methods for working with soft deleted models. What do you think about these ideas?

[woodspire]

@hamzaelmaghari

Thank you for your response.

I asked here instead of creating an issue or a PR because I don't want to spend time and then the ticket/PR is closed.
That's why I was asking Taylor directly to know if I should create a PR in laravel/docs to exmplain the shortcoming or create a PR in laravel/framework. Obviously, it's a lot more difficult to fix this problem in laravel/framework (and to create a test to make sure the new feature works AND it doesn't break previous code).

I was thinking of adding a new property or method on the Model class to list the scopes that should be kept.
I am pretty sure I must target the main branch because it won't be backward compatible.
The current method to get a new query without any scope will stay but all the calls currently done to it inside the framework used by the fresh() & refresh() method will need to use the new method (tentative name: getQueryWithBasicScopes)