-
Notifications
You must be signed in to change notification settings - Fork 165
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 compatibility between Mongoid::Paranoia and Mongoid::Slug #165
Conversation
Currently, when Mongoid Paranoia deletes a document, it's `_slugs` field value remains in the DB. This can cause the unique index to throw an error when saving a new record with the same slug. In order to fix this, we must unset the `_slugs` field when a paranoid doc is destroyed, and reset the slug when it is restored. Includes the following changes: - Add support for callbacks around the `restore` method, if not natively supported by the Mongoid::Paranoia implementation (simi/mongoid-paranoia supports it, mongoid-paranoia gem and Mongoid 3 do not.) - Revert test mongoid-paranoia repo to 'simi/mongoid-paranoia'. Seems currently there are two competing mongoid-paranoia repos (see here: ream88/mongoid-paranoia#1) - Rename `set_slug` method to `apply_slug`. The are new `set_slug` and `unset_slug` methods which literally call Mongoid `set` and `unset`.
The change looks reasonable, but it currently fails on Mongoid 4. Have you had a chance to take a look at the cause? It may be related to the change of Mongoid Paranoia version in the Gemfile. |
- Add paranoid specs for both permanent and non-permanent slug cases - Add better handling of edge case of re-saving destroyed paranoid documents - Rename set_slug, unset_slug etc to set_slug!, etc. (add ! to method name) - Update CHANGELOG.md and README.md
Issue was not related to Mongoid Paranoia gem; I had forgot to rerun specs for Mongoid 4 on local. Issues were:
Anyway, seeing edge case (1) has led me to a slightly better implementation which I've committed, and specs are passing. |
LGTM. Thanks! I was not aware of @simi's work on Mongoid Paranoia, but after taking a look it seems like simi actively maintains his gem and has significantly more traction. I agree with using simi/mongoid-paranoia for now. |
Fix compatibility between Mongoid::Paranoia and Mongoid::Slug
Yeah there's currently some discussion about use of the |
I'd love to see this released. Just came to the readme to see how to integrate with paranoia and the readme implies that it already is, which was a little misleading. Is there any reason its not already launched? Anything I can help with to get this pushed? |
We intend to do some changes to the index, and we have already performed some improvements on master so instead of making the user migrate twice we want them to do it max once. So we are essentially waiting for this issue to be resolved: #172 |
Currently, when Mongoid Paranoia deletes a document, it's
_slugs
field value remains in the DB. This can cause the unique index to throw an error when saving a new record with the same slug.In order to fix this, we must unset the
_slugs
field when a paranoid doc is destroyed, and reset the slug when it is restored.This PR includes the following changes:
restore
method, if not natively supported by the Mongoid::Paranoia implementation (simi/mongoid-paranoia supports it, mongoid-paranoia gem and Mongoid 3 do not.)set_slug
method toapply_slug
. The are newset_slug!
andunset_slug!
methods which literally call Mongoidset
andunset
.