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

Fix deprecation introduced in ActiveRecord 5.1.0.beta1. Closes #257 #259

Closed CvX closed 7 years ago

CvX commented 7 years ago

Added ruby 2.4 and Rails 5.1.0.beta1 to test matrix. All tests are passing. Also, the change was tested in a live application.

Thanks to @brendon for guidance! 😃

brendon commented 7 years ago

Looks good @CvX :) @swanandp, are you happy for me to merge this? (or you can if you want to) :)

swanandp commented 7 years ago

Merged. Thanks @CvX !

CvX commented 7 years ago

Hi, have to dig up this PR because I've noticed that the code I committed and which ended up in the repository is incorrect. Version conditional used here: https://github.com/swanandp/acts_as_list/pull/259/files#diff-708784f0c89e5cdbdd54e18d2b2243d1R406 is wrong for future AR versions: 6.0, 7.0, etc.

We can either correct the conditional (ActiveRecord::VERSION::MAJOR > 5 || (ActiveRecord::VERSION::MAJOR == 5 && ActiveRecord::VERSION::MINOR >= 1)) or we could use Gem::Version as mentioned before (like this https://github.com/CvX/acts_as_list/commit/0e7f24f29df8003f42ddb29971c04e5a8b7081be)

Please let me know which way do you prefer and I'll submit a PR. Sorry for the inconvenience.

brendon commented 7 years ago

Thanks @CvX, I've pushed a fix for that to master :) Thanks for picking it up! :)