Fixes has_many conditions in Rails 4.1

[patbenatar]

Rails 4.1 passes the association into the scope lambda, not the class. Rather, the associated object's class is available within the context of the lambda as model.

Rails 4.0 must have done this differently, or the original code never would have worked.. I haven't had a chance to test my change on 4.0 yet, so while it's confirmed to work on 4.1 it may be breaking 4.0. If that's the case, we could add another version condition here—but I wonder if there's a better way...

Perhaps someone else can help test on 4.0?

[batter]

That's still not the proper syntax I don't think, it still needs to look like this I believe:

{ order(model.version_class_name.constantize.timestamp_sort_order) }
# or this might work
{ order(model.paper_trail_version_class.timestamp_sort_order) }

We'll see what Travis says... Actually I'm not even certain we have test coverage ensuring that the association is sorted the proper way but if we don't then we definitely need to add some.

[patbenatar]

@batter model is actually the constantized version class

Looks like Travis just failed on 1.8.7 using MySQL, but thats unrelated to this change (looks like the database_cleaner version we're using has 1.9 style hashes in it) — that said, I believe you're right that this isn't covered by tests. I'll add some when I have a minute.

[batter]

Well that is nice! That being said, I think we'll need to add some test coverage to ensure the associations are getting sorted in the proper direction with all the versions that we're currently supporting 😃

[patbenatar]

@batter haha yes indeed.. I'll see about adding some tests when I have a minute. How many different versions of rails does the suite test against?

[patbenatar]

@batter Another Q: I see that there is both a spec and test directory—is there a criteria for what goes where?

[batter]

For your first question: Just Rails 3.x and 4.x, but if the methodology for sorting is different in 4.1 from 4.0, we may have to add additional coverage to the Travis matrix.

Second question: Not really. When I stared maintaining the project, there was only unit tests in the test dir, but now there is an RSpec helper built into the gem, so I made an RSpec test suite to start testing that type of stuff. That being said, I prefer working with RSpec so most of the recent tests I have written have been in RSpec, but feel free to work with whichever one you're most comfortable with. I can actually add some of these tests for you if you want. I'll try to do that in a bit.

[patbenatar]

@batter That would be great if you have sometime to get started on testing it. I'm hoping to find some time next week—I'll check in then. Thanks!

[batter]

@patbenatar 9c34a7b should contain the sorting spec. Still wondering whether I should be expanding the matrix to have multiple versions of Rails4 covered in Travis. Question though: is the sorting not working by default on that association on Rails4.1 without this modification for you?

[patbenatar]

@batter Thanks for your help! I think it could be worthwhile to run Travis against all major and minor versions of Rails we want to support: 3.2, 4.0, 4.1, etc

To answer your question, I was getting an exception on 4.1 before making the change in this PR