total_pages returns 1 when there are more than 1

[smoyte]

This was a tricky bug that was resulting in no pagination links even with multiple pages of results. I fixed it by putting a call to total_pages right after the main model query in the controller. At this point in execution it returns the correct value. But if I move it to the bottom of the template (where the pagination links are) it returns 1 and the pagination doesn't work. Anyone ever seen this before?

[charly]

Ah yes I have the same problem see #414. I haven't found the source of the problem but some pry debugging showed me it happens once the the sql call is made on the ActiveRelation.

When the WP/AR total_entries gives me the number of records it is the same as the number of records per page so the resulting total_page is 1 !

The mismatch happens in /activerecord-4.1.6/lib/active_record/relation.rb :

def size
  loaded? ? @records.length : count(:all)
end

count(:all) gives me the right amount of entries (3056) but @records.length outputs 118 I'll keep investigating when I have time

[charly]

This bug shows when I combine an Foo.eager_load(:has_one_bar) with an order(:bla). In my case :

Book.eager_load(:amazon).order(:title).page(...)

The logs then show no count (which is consistent with total_entries not being counted) and the normal sql call with limit set on the per_page value.

If I remove the eager_load or the order clause it works fine, so (at least for me) it's the combination of those 2 that causes the bug. Why ? duno....

[charly]

So I finally nailed it : when I eager_load my has_one association ActiveRecord doesn't load the expected number of records (e.g 29 instead of 30) therefore the total_entries are set in replace instead of doing a proper count later on:

    def replace(array)
      result = super

      # The collection is shorter then page limit? Rejoice, because
      # then we know that we are on the last page!
      if total_entries.nil? and length < per_page and (current_page == 1 or length > 0)
        self.total_entries = offset + length
      end

      result
    end

length < per_page shouldn't be true here, but because ActiveRecord doesn't load the @records properly WillPaginate thinks it can skip the count and rely on the offset.

Ultimately the culprit is ActiveRecord. Any thoughts ?

[smoyte]

Nice work. I don't quite follow your narrative about replace (what is replace for?) I will dig into the code and also try to find the original spot in my code I was having trouble when I get some time.

Thanks for looking into it!

[charly]

replace is a small wraper for Array#replace to avoid an extra sql call. It's used to set WilPaginate::Collection here :

/will_paginate/active_record.rb

      def to_a
        if current_page.nil? then super # workaround for Active Record 3.0
        else
          ::WillPaginate::Collection.create(current_page, limit_value) do |col|
            col.replace super
            col.total_entries ||= total_entries
          end
        end
      end

[charly]

...and the reason this eager load messes up the number of records loaded is because I have both a has_many :amazons and a has_one :amazon. So as soon as I have a duplicate amazon#book_id in my DB, activerecord removes an extra record from the collection... Book.preload(:amazon) on the other hand does a proper count...

So not really a bug although I don't remember seeing any recommendation for not doing that that.

For the record, transform your has_one into a belongs_to if you need has_one & has_many on the same resource.

[mislav]

Thanks all for the debugging session. I'm not clear on what (if anything) should be changed in this library, so I'm closing this for now, but if there is still a nasty bug, I appreciate a PR.