-
Notifications
You must be signed in to change notification settings - Fork 211
Laravel 4.2 soft delete doesn't work with Ardent #211
Comments
Hi, I got same problem and I fix. |
Please merge! 😄 |
Awesome thank you ! :D |
Ardent->morphTo() was overriding a new eager loading check on Laravel 4.2.
`Ardent->morphTo()` was overriding the check for eager loading on new Laravel 4.2 Eloquent Model. laravel-ardent#211
This is actually not fixing it. Check this one out: #227 |
Is there actually someone still maintaining Ardent? Doesn't look like someone is working on a fix or at least a workaround for this problem. |
As you can see, the latest commit was submitted on Jul 10. Could anyone reach the maintainers? |
On a related note, I was able to get the soft delete to work by using
in addition to
However, calling
I've also tried
and that doesn't seem to work either. |
For me Ardent is dead. It's not really be maintained any more... |
@jloosli Yep, I also tried the "protected $softDeletes = true" trick but it's not solving all problems... |
@jloosli Any updates on the restore() issue? |
I've dropped Ardent from my app, because I assume it's no longer maintained. I've copied all methods that provide support for "cleaner relationships" into my base model so that's still working. For validation I'll use dwightwatson/validating. |
@ananddpatil No. I haven't had a chance to do it yet, but I'll probably try my luck at dwightwatson /validating as well. |
For the love of god someone please fix this. I just spent 4 hours tracking down this bug. |
Nah, it's not going to happen. Ardent is dead. Laravel 5 is incoming and Ardent isn't even working with 4.2... Just forget about it. |
This is being looked at. If there's still someone here, please lend me a hand at #227. |
I don't know if mspivak@19d231e solves the problem or not. I do use my own fork of Ardent. Therefore I cannot test the change. But it looks promising. PS: It's very kind that you maintain this package so those who still rely on it get some support. |
thanks @chriskonnertz. you are correct. @mspivak commit fixed this issue for us for an older Laravel 4.2 project. his other commit also fixed the morphTo issue. cc @igorsantos07 |
Thanks for the followup, @chriskonnertz and @thinksaydo. Would you please help me review the PR #227? Specifically this part here. That seems quite odd to me. |
Honestly I am very disappointed by Ardent. I had to fix the bug myself and I had to create a very dirty fork. The only way that I could help you is to merge the changes of the PR into my fork and I am not going to do that. I do not want to risk that. I do not trust Ardent any more and therefore I do not plan to use it again. I am sure you are doing a great job but I needed that bugfix in August 2014... Plus, I do not use morphTo thus I am not the right person to judge about that PR. PS: My fork does not even override Eloquent's morphTo method. |
Take a look at this very old version: https://github.com/mspivak/ardent/blob/9fbe73399d84fc726dc9e122955de444f4fb4901/src/LaravelBook/Ardent/Ardent.php In line 428 there it is using belongTo. So I guess the red lines in mspivak@1228dfe are just something left over from July 2014. It should not change anything if you merge it. |
I believe you can accept the PR. Or refuse it and create a new PR by yourself. Because essentially all it does is to update the newQuery() method so that it does not use old soft deleting stuff any more. I have created a new PR without the strange part ( #227 ) of the old one: #269 PS: Not tested!!! |
I'm going to accept your new PR, @chriskonnertz. |
I've merged your PR directly as, per other discussions, that seems to be the fix. |
Ok. |
damn. This PR broke my app... :) Looks like on 4.1 this change creates infinite loop of dependancy injections. Will investigate for more details... |
Ok so Laravel 4.1 with the protected $softDelete = true works fine with Ardent. But when you upgrade to Laravel 4.2, it doesn't work anymore since they changed the way soft delete is set. It will still remove the record, but it will remove it permanently instead of updating the deleted_at column.. Do you know when there will be a fix for this?
http://laravel.com/docs/upgrade#upgrade-4.2
Thanks!
The text was updated successfully, but these errors were encountered: