search

brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.05k stars 356 forks source link

Use `ActiveSupport.on_load` to hook into ActiveRecord #271

Closed rono23 closed 7 years ago

rono23 commented 7 years ago

There is a weird behavior of config belongs_to_required_by_default on Rails 5.0.2 when gem is not playing nice with the rails hooks.

Fix #268 and related rails/rails#23589

brendon commented 7 years ago

Thanks for this @rono23. @swanandp, what do you think about using concerns for this? I'd prefer to see if we could do this without using concerns and just use plain Ruby instead.

@rono23, would you be interested in trying that? As per: https://github.com/plataformatec/devise/commit/c2c74b0a39238e7d997486814a1c8f75fdaf276f

brendon commented 7 years ago

@rono23, I couldn't directly work on your PR for some reason but I've made some changes here: https://github.com/swanandp/acts_as_list/tree/pr/271

https://github.com/swanandp/acts_as_list/commit/bd4f4266d0905ef16fefa51b9abba93e530e76ab

Check that out and see if it still works in your project. If so, I can close this and merge that one instead :)

rono23 commented 7 years ago

@brendon Thank you for your review and it also works on my project.

FYI, here is also using ActiveSupport::Concern.

swanandp commented 7 years ago

@rono23 Looks good, but let's not move to ActiveSupport::Concern yet. We can discuss it in another PR — but it certainly isn't required here.

brendon commented 7 years ago

Haha! That slipped through the cracks :) Not sure how I didn't notice the concern on that no-update code. I'll close this and merge my PR instead then look at removing the Concern in the other code.