Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.4] Soft deleted models should still existing #17381

Closed
wants to merge 1 commit into from
Closed

[5.4] Soft deleted models should still existing #17381

wants to merge 1 commit into from

Conversation

JuanDMeGon
Copy link
Contributor

Hi guys :)

When a model is soft deleted, its "exists" property should keep equals true. It was not completely removed and technically still existing.

This minor change wil allow deleting and immediately forceDelete.

Example:

function someExample(Model $instance)
{
   //Independently of it uses SoftDeletes we can call delete()
   $instance->delete();

    if (someConditionToForceDeleteNow) {
        $instance->forceDelete();
    }
    return $instance;
}

If the instance was force deleted or not, equally the deleted_at attributes has been set and can be returned in the response.

Without this change, the instance will never be force deleted, and if we do not call the delete() before we will never have the deteleted_at date updated/set.

@GrahamCampbell GrahamCampbell changed the title Soft deleted models should still existing [5.3] Soft deleted models should still existing Jan 18, 2017
@GrahamCampbell GrahamCampbell changed the base branch from 5.3 to 5.4 January 18, 2017 01:04
@GrahamCampbell GrahamCampbell changed the title [5.3] Soft deleted models should still existing [5.4] Soft deleted models should still existing Jan 18, 2017
@JuanDMeGon
Copy link
Contributor Author

I think that I made a mess here. How can I resolve the conflicts from my side? I tried merging but did not work.

Sorry.

@morloderex
Copy link
Contributor

Try rebasing.

@taylorotwell
Copy link
Member

This would be a breaking change now since we didn't have time to get to this before 5.4 release. Sorry :/ ... it could be considered possibly for 5.5.

@JuanDMeGon
Copy link
Contributor Author

Ok, I understand. Thanks for the excellent job. 5.4 has been a fantastic release as always.

I'll send it to the dev branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants