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

[9.x] Remove assertDeleted #39661

Merged
merged 2 commits into from
Nov 18, 2021

Conversation

jasonmccreary
Copy link
Contributor

@jasonmccreary jasonmccreary commented Nov 17, 2021

This removes the asymmetrical, proxy assertion assertDeleted. Unlike other assertions, its opposite assertion does not exist (assertNotDeleted). It is also superseded in #38766 by assertModelExists and assertModelMissing.

Tests using this assertion may either adopt one of these more communicative methods, or fallback to the original assertDatabaseMissing.

Targeting Laravel 9 as this is a breaking change. Alternatively, I am glad to open a PR to add assertNotDeleted to provide symmetry. I believe removal is cleaner though as all paths should be covered by the other 4 assertions.

@jasonmccreary jasonmccreary changed the title Remove assert deleted Remove assertDeleted Nov 17, 2021
@GrahamCampbell GrahamCampbell changed the title Remove assertDeleted [9.x] Remove assertDeleted Nov 17, 2021
@taylorotwell
Copy link
Member

What are the current differences between assertDeleted and assertModelMissing?

@jasonmccreary
Copy link
Contributor Author

jasonmccreary commented Nov 17, 2021

@taylorotwell, when passed a model, none. They both call assertDatabaseMissing underneath (ref, ref)

I originally added assertDeleted (#30648). I'm not sure why I made it a proxy method and didn't add its counterpart. 😅 🤦

As you can see from my original example, it was intended to do what assertModelExists and assertModelMissing ultimately did.

@taylorotwell taylorotwell merged commit 9894c2c into laravel:master Nov 18, 2021
taylorotwell pushed a commit to laravel/docs that referenced this pull request Feb 10, 2022
This assertion was removed in laravel 9 laravel/framework#39661
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.

2 participants