-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix: fixes issue #371 #34
fix: fixes issue #371 #34
Conversation
Allows the deleting observer to return false and abort the current delete operation
This pull request is in regards to this thread . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment on this change.
fixes if conditional statement
@foxbecoding thanks for pushing, but it wasn't quite the change that was needed. I've looked at this myself and have just pushed a change to your branch. If a listener (or observer) prevents the delete from happening, then we're in an invalid state - i.e. the JSON:API implementation hasn't been able to do what the client has asked for, i.e. soft delete the model. Therefore, I think in this scenario we need to throw an exception - which is the change I've put in. I've also added a test for this scenario. Developers should actually prevent this scenario from happening. For example, if there's a specific reason why a model can't be deleted (which is what your listenered is enforcing), that should actually be picked up when checking the request from the client. I.e. you should use authorisation or validation to prevent the request - because there's no way the server can complete it. Hopefully that makes sense? |
I'm going to merge this into the major release for Laravel 11, as this is technically a breaking change. |
@lindyhopchris Thank you so much for working on this issue, but do you have a solution that would work outside of Laravel 11? My team is currently using Laravel 9.x. |
@foxbecoding ah yeah, there's no way I can support Laravel 9 as that's really old. You should however be able to sort this out yourself in your app, without much difficult. All you need to do is copy and paste the soft delete driver into your app: And tweak it if desired. Then to use on your schemes, just write a trait like this one but return your own soft delete driver class: |
Allows the deleting observer to return false and abort the current delete operation