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

Mark soft deleted records as persisted. #2

Closed
simi opened this issue Feb 26, 2013 · 5 comments
Closed

Mark soft deleted records as persisted. #2

simi opened this issue Feb 26, 2013 · 5 comments
Labels

Comments

@simi
Copy link
Owner

simi commented Feb 26, 2013

I had a lot of issues today with this flag. CarrierWave photos were deleted and embeded models returned persisted? #=> false and to_param => nil, which caused a lot of issued in rails url helpers.

I started work in persistence branch.

Anyone against this change?

@simi
Copy link
Owner Author

simi commented Jan 22, 2014

Merged in ea986ac.

@simi simi closed this as completed Jan 22, 2014
@afterthought
Copy link

I see a side effect here.

  1. I think this is the root cause to this cancan issue, Mongoid-paranoia with cancan #17. "Cancan" was a red herring.
  2. The real issue is using the "respond_with @deleted_model" syntax in one's controller. The expected behavior is a redirect to the "index" action. This change breaks that behavior and now respond_with redirects to the "show" action resulting in the "document not found" error.

See: mongoid/mongoid@34e895ab
Notice that Mongoid explicitly returns persisted=false when destroyed. The reason for the change is documented here: https://groups.google.com/d/msg/mongoid/RBb2Se511sc/xKqi7tf90SMJ.

The explanation provided by visnup:

AR marks a destroyed record as !persisted? and polymorphic routes pick up on that:
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb#L109

so, the fix would be to make sure that persisted? in mongoid returns false for destroyed objects

@simi
Copy link
Owner Author

simi commented Feb 4, 2014

This was merged because of mongoid internal changes. I'll take a look what changed, but without this, there were some issues.

@ream88
Copy link

ream88 commented Feb 17, 2014

This should really be reverted imho, because it can and will break existing apps using mongoid-paranoia when mongoid 4.0.0 is released.

@simi You should fix your problems in your specific app, I can help you :)

@simi
Copy link
Owner Author

simi commented Feb 17, 2014

Revert = red CI. Feel free to revert and make it green. I have not changed tests except one. If i remember well, this test will not pass 11ddc35. I'm around to help you. IRC Freenode, name retro|cz.

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

No branches or pull requests

3 participants