Do you want me to modify the line lengths? They're mostly from code I just moved around, and I didn't want to be too invasive. My comments are longer than 80 characters since I didn't realize the length restriction. I can change the line breaks if you want.

Okay, I changed my comments to be within 80 characters, and changed single quotes to double.

note: related to #2638

The downside is we do so by hardcoding in an essentially random total pages count of 100. I chose the number 100, because I wanted to ensure ellipses would appear in Kaminari's "Page 1, 2, 3, ..., Next, Last" links, no matter how many links are configured to be displayed. However, if the resource only had 2 pages worth of items, then clicking the "3" would result in an empty page. That said, without querying the resource, there's no way to know how many pages there should be.

Sorry but I can't merge that, that's a hack and not a solution. In my eyes this is not acceptable.

There are some unexpected behaviors if you trie all variants of index pagination_total: false do and config.paginate = false.

BTW: In my test app I still see count queries:

SELECT COUNT(count_column) FROM (SELECT  1 AS count_column FROM `companies`  WHERE (user_id IS NOT NULL) LIMIT 10000 OFFSET 0) subquery_for_count

Have you read #2638? I'm not very familiar with #2638 but in my understanding it's a kaminari bug.

Apologies, I didn't see that other issue. I tried searching for pagination_total and it didn't come up.

There are some unexpected behaviors if you trie all variants of index pagination_total: false do and config.paginate = false.

I'm sorry, I don't understand this. Can you explain a bit? And if you disable pagination then won't that blow up activeadmin if you have millions of records (the impetus for turning pagination_total: false?

SELECT COUNT(count_column) FROM (SELECT  1 AS count_column FROM `companies`  WHERE (user_id IS NOT NULL) LIMIT 10000 OFFSET 0) subquery_for_count
111

Yeah, I get these, too, but they run instantaneously for me. The LIMIT there narrows it down to just 10,000 records (and in my configuration it's just 30). I think it's to determine how many records are being displayed on the current page, rather than the total number of records?

The query to look out for is SELECT count(*) FROM "companies". Before my patch that one query can take upwards of seconds if you have millions of records.

Sorry but I can't merge that, that's a hack and not a solution. In my eyes this is not acceptable.

Thank you for the blunt feedback. I'll see if I can find another approach.

Okay, I've looked into this some more, including #2638.

Contrary to what #2638 says, I'd argue that this is not a bug in Kaminari, but rather a feature request. It is a bug in activeadmin, because the documentation advertises this as a feature that removes the count (*) and has some code intending that effect, but does not actually do so. I poked around the Kaminari codebase, and unfortunately don't feel I could cleanly implement the feature in the time I'm willing to spend.

While Kaminari does not have an "infinite pages" feature, it does let you specify the total_pages as an option, so it doesn't have to do the query. That's what my PR took advantage of, although I arbitrarily set it to 100.

I looked at #2638 and @azach links to a clever approach called activeadmin_fuzzy_paginate, which I think is a more elegant solution than the way I did. If I changed my PR to use his approach would that suffice?

Let me explain the issue and the approach:

Here's a screenshot of the pagination component:

The activeadmin code that generates this is:

def build_pagination
  options = {}
  options[:param_name] = @param_name if @param_name
  text_node paginate collection, options
end

The pagination component can be configured via the options that are passed in.

There are two parts to the pagination component that are relevant here:

• The links 96, 97, 98, 99, 100. 98 is the current page, and the quantity of links on each side is determined in Kaminari via Kaminari.config.window.
• The links 9999, 1000. These are the links starting from total_pages and counting backwards. The number of links is determined by options[:right].

So the clever approach is still to send in options[:total_pages] so Kaminari doesn't calculate it (like I did in my PR). But instead of hardcoding 100 as the total number of pages, we say the total is the "current page + the window". Since by definition we can't know the total number of pages without a count(*) this basically implements "infinite pages" by always saying there are at least window pages more than whatever we're on. We also set options[:right] = 0 so we don't display the links to the last few pages.

options[:total_pages] = collection.current_page + Kaminari.config.window + 1
options[:right] = 0

To be clear there is still one small downside: Kaminari seems to generate the "Last" link no matter what. The screenshot from my proposed fix looks like this:

If you click "Last", then the pagination on that page will look the same except now 14 will be the "current page", and it will count up 15, 16, 17, 18, ....

So, at this point, I see two possibilities:

• I update my PR with the approach above.
• We wait for someone to implement a new "infinite pages" feature in Kaminari which does basically the above, but also removes the Last link. Then we pass in options[:infinite_pages] = true or something here. Activeadmin lives with a bug until then, and we bump the required kaminari version in the gemspec when it happens. But I don't have the time or understanding of Kaminari codebase to do this, unfortunately.

I hope this new approach is sufficiently "not hacky" so that you'll accept it, but I'll understand if not and close the PR. Please let me know.

Okay, since I had already worked out the code locally for the screenshots, I just pushed up my changes to the PR. Feel free to review and merge if you approve of the approach! :)

First: 👍 for one of the best documented PR's

I still think it's to hacky.

But I have a idea:
What if we change this:

options[:total_pages] = collection.current_page + Kaminari.config.window + 1

to this:

next_page = collection.page(collection.current_page + 1).per(@per_page).limit(1).any?
options[:total_pages] = collection.current_page + (next_page ? 1 : 0)

That's just produce a query like this:

SELECT COUNT(count_column) FROM (SELECT  1 AS count_column FROM `companies` LIMIT 1 OFFSET 30) subquery_for_count

Oh, that's very interesting. I like it! Sorry I haven't had time to respond. I'll try out this approach over the weekend and update the PR.

There is a more easy version:

offset = collection.page(collection.current_page + 1).per(@per_page).limit(1).count
options[:total_pages] = collection.current_page + offset

I looked into these approaches this weekend and unfortunately they didn't work. I put a byebug in the code right where those code portions go, to try them out and see what gets executed in the way of SQL:

(byebug) collection.page(collection.current_page + 1).per(@per_page).limit(1).any?
   (117.3ms)  SELECT COUNT(*) FROM "messages"
true
(byebug)

and

(byebug) offset = collection.page(collection.current_page + 1).per(@per_page).limit(1).count
   (108.0ms)  SELECT COUNT(*) FROM "messages"
989547
(byebug)

I believe it's the usage of Kaminari's #page and #per. Doing it with straight #offset and #limit works:

(byebug) collection.offset( collection.current_page * @per_page + 1).limit(1).count
   (2.1ms)  SELECT COUNT(count_column) FROM (SELECT 1 AS count_column FROM "messages" LIMIT 1 OFFSET 15001) subquery_for_count
1
(byebug)

However, I don't know enough about activeadmin: are we guaranteed that the collection in PaginatedCollection responds to offset and limit? Mine did, since I'm using postgres and activerecord, but I'm not sure what other data stores are supported.

If I can use #offset and #limit, I'll rewrite the PR to use them. Let me know.

If I can't use them, then I'm out of ideas. I'll put in one more plug for the PR's current approach which is to set the total pages as "current page + 1". The only downside is that when you're on the last page, it will still have a "Next" button. In practice, I don't think it's a problem, since we're talking about collections with tens of thousands of pages here, and if a user opts in to pagination_false: true, then they shouldn't expect us to actually know the last page.

collection should be a ActiveRecord::Base or ActiveRecord::Relation object. It's possible but not allowed that collection returns something else. So we can assume that it has offset and limit.

Ah, great! I'll update the PR to use offset and limit when I have a chance.

Need we more test for that?

@losvedir any updates?

@timoschilling - Sorry, for the delay. I made the offset and limit changes as well as hash formatting (just now pushed up) but have been working on tests for the feature. I'm not familiar with cucumber myself so the tests are slow going, and I don't have a ton of time on the side to work on it. Will try to complete the tests this week.

@losvedir 👍

@timoschilling - Okay, ready for review! I've never used cucumber before; please let me know if I should be doing that a different way.

There is a syntax error on Ruby 1.9 https://travis-ci.org/activeadmin/activeadmin/jobs/57748755#L303

Ah, bummer. I don't have Ruby 1.9 on this computer and it's slow to build for me. Will have to take a look tonight.

Ah, ruby 1.9.3 didn't like the unicode literal in my string! Fixed.

I think this collection.offset(collection.current_page * @per_page.to_i).limit(1).count is great in that it's faster than actually counting the items in the collection, but what about leveraging the already queried collection_size from ActiveAdmin::Helpers::Collection?

As it stands there are 3 queries actually run against the table:

SELECT COUNT(count_column) FROM (SELECT  1 AS count_column FROM "users" LIMIT 50 OFFSET 0) subquery_for_count
SELECT COUNT(count_column) FROM (SELECT  1 AS count_column FROM "users" LIMIT 1 OFFSET 50) subquery_for_count
SELECT  "users".* FROM "users"  ORDER BY "users"."id" desc LIMIT 50 OFFSET 0

1. to count the number of items on the current page
2. to peek if there is 1 item on the next page
3. to load the items on the current page

I see two separate potential optimizations here:

• What if we assume there is a next page if the current page as a complete collection? There is a loss in information here no doubt... but if the goal is performance, this would eliminate a potentially expensive query.
• What about actually loading the items on the first collection_size call in ActiveAdmin::Helpers::Collection? Subsequent calls to iterate over the items will hit the cache.