I've got a patch ready adding the pagination tests (and confirming the bug with :include). It depends on ticket 3606 (http://dev.rubyonrails.org/ticket/3606) so I'll submit it as soon as 3606 is checked in. Kev
On 1/25/06, Kevin Clark <[EMAIL PROTECTED]> wrote: > Jeremy, > Awesomeness. I'll write up tests for pagination ASAP, but it's good to > see that my bug can be fixed with the plugin. > > On the other hand it really shouldn't require a plugin. Can someone > check out his code and check in? > > Kev > > On 1/25/06, Jeremy Hopple <[EMAIL PROTECTED]> wrote: > > Kevin, > > > > I have written a patch[1] that addresses this issue, but it hasn't received > > much attention. I have also created a plugin[2] that makes eager loading > > work via pagination. > > > > I wrote new tests to verify the changes I made in ActiveRecord, but I never > > wrote any tests for pagination... so it's great that you wrote some. Maybe > > we could get our patches reviewed as a pair. > > > > Could someone please review this code? I've had a few people use the > > plugin that wraps up this patch and been happy with it... > > > > Thanks, > > Jeremy > > > > [1] http://dev.rubyonrails.org/ticket/2597 > > [2] > > http://wiki.rubyonrails.org/rails/pages/Count+Limit+Associations+Plugin > > > > > > > > On 1/25/06, Kevin Clark <[EMAIL PROTECTED]> wrote: > > > > > > Here's the line of code that is now the bane of my existance: > > > @topic_pages, @topics = paginate(:topics, :include => :posts, > > > :order => 'posts.created_at asc') > > > > > > So, grab topics (eager loading the posts) and then order the topics > > > based on the post created date. > > > > > > This doesn't work because when paginate tries to count the records, it > > > -does- try to order them by posts.created_at but does not include > > > posts in the query. > > > > > > I'd like to write some test cases and fix the bug, but interestingly > > > enough, there are no pagination tests in ActionPack. They just aren't > > > there. In fact, if you 'grep -r "paginate" *' in your rails directory, > > > you won't see any tests for pagination anywhere in the core. > > > > > > I'm going to write the pagination tests I guess, since I need them now. > > > > > > Unfortunately, I need an ActiveRecord connection in ActionPack in > > > order to test this. It seems that > > > test/active_record_assertions_test.rb needs AR as well, > > but the code > > > to connect is messy and localized to that file. > > > > > > I've written a patch > > (http://dev.rubyonrails.org/ticket/3606) > > > extracting the ActiveRecord connection and fixture model loading (not > > > the data, which currently isn't loaded in ActionPack either) to a > > > helper class. > > > > > > All tests pass (without modifications) except two. These two > > > assertions are that the company model (defined in AR/fixtures) should > > > error out when rating = 2. This is residual from changeset #4 > > > > > (http://dev.rubyonrails.org/browser/trunk/actionpack/test/controller/active_record_assertions_test.rb?rev=4 > > ) > > > so I've taken the liberty of writing a hacky setup/reset company > > > helper which is run in setup/teardown for the tests where it matters. > > > All tests now pass. > > > > > > If anyone has widespread objections to this, please raise them ASAP. > > > I'd like to start writing pagination tests so I can fix my original > > > bug and want to base the connection code on my helper. > > > _______________________________________________ > > > Rails-core mailing list > > > [email protected] > > > http://lists.rubyonrails.org/mailman/listinfo/rails-core > > > > > > > > _______________________________________________ Rails-core mailing list [email protected] http://lists.rubyonrails.org/mailman/listinfo/rails-core
