Conditionally turn off :dependent => :destory on FriendlyId::Slugs #268
Conversation
| module Configuration | ||
| def dependent_destroy | ||
| return @dependent_destroy if defined?(@dependent_destroy) | ||
| @dependent_destroy = options.delete(:dependent_destroy) != false ? :destroy : nil |
There was a problem hiding this comment.
how about:
@dependent_destroy = (:destroy if options.delete(:dependent_destroy))seeing as you're saying != false, wouldn't it be fine to accept any truthy value?
There was a problem hiding this comment.
Then the entire method could be:
def dependent_destroy
@dependent_destroy ||= (:destroy if options.delete(:dependent_destroy))
endUnless I'm missing something about why you're using defined?(@dependent_destroy) (for example, a false value would trigger the ||=)
Thanks
There was a problem hiding this comment.
The != false is to cover the case where the :dependent_destroy is nill (most likely because it has not been set, as this is the default case) We wanted to maintain the same functionality in the default case.
|
Yeah, I think this is fine. I worry a little about letting FriendlyId 4 bloat up with a bunch of configuration options and switches, like FriendlyId 3 had. But I don't really see any better way to handle this situation, and it seems like a rather reasonable use case. At least the "bloat" is now isolated to an optional add on. |
| Leaflet | ||
| end | ||
|
|
||
| test "run!" do |
There was a problem hiding this comment.
How about a slightly more descriptive test name here, to better match the rest of the test suite?
|
Another comment on this: isn't possible to just redefine the slugs relationship in the model? Then there would be no need for another config option. |
|
Closing this as stale. If you still think it should be merged, please see my comments and let me know. |
|
@jasoncodes and I simply overrode the dependent destroy method. # prevent slugs being destroyed
def has_many_dependent_for_slugs; end |
Hi,
We're using acts_as_paranoid on some of our sluggable models. This means they’re not actually deleted when calling destroy. If we delete the slugs, we’re losing the history (and worse, setting up a condition where a unique contstraint may end up being violated)
There is a chicken & egg problem here with FriendlyId::Configuration objects and use. We’d love to get your feedback on the proposed solution.
Thanks!