8000 Fix pagination_total index option to prevent SELECT count(*) queries · activeadmin/activeadmin@453a67d · GitHub
[go: up one dir, main page]
Skip to content

Commit 453a67d

Browse files
author
Gabe Durazo
committed
Fix pagination_total index option to prevent SELECT count(*) queries
The pagination_total option is supposed to prevent expensive SELECT count(*) queries from being performed when viewing a resource's index page. However, while this option was hiding the display of the total number of pages, it did not actually prevent the query from being performed. There were two reasons: In `#build_pagination`, the line: `text_node paginate collection, options` would trigger Kaminari to call `#total_pages` on the collection, which would trigger the query: ``` def paginate(scope, options = {}, &block) options[:total_pages] ||= options[:num_pages] || scope.total_pages ``` By passing in the `:total_pages` option, we short circuit this assignment and avoid the query. We set the total_pages = collection.current_page + Kaminari.config.window. This is essentially "infinite pages", in that the total_pages is always at least window pages more than the current page. That fix was not sufficient. In `#page_entries_info`, the call `collection.num_pages` also triggered a SELECT count(*). I re-arranged the conditionals to avoid that call if `@display_total` is false, while trying to keep the behavior the same.
1 parent 7476978 commit 453a67d

File tree

3 files changed

+27
-16
lines changed

3 files changed

+27
-16
lines changed

‎features/index/pagination.feature

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ Feature: Index Pagination
5353
"""
5454
Given 100 posts exist
5555
When I am on the index page for posts
56-
Then I should see pagination with 4 pages
5756
Then I should see "Displaying Posts 1 - 30"
5857
And I should not see "Displaying Posts 1 - 30 of 100 in total"
5958

‎lib/active_admin/views/components/paginated_collection.rb

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,14 @@ def build_pagination
9797
options = {}
9898
options[:param_name] = @param_name if @param_name
9999

100+
if !@display_total
101+
# The #paginate method in kaminari will query the resource with a
102+
# count(*) to determine how many pages there should be unless
103+
# you pass in the :total_pages option.
104+
options[:total_pages] = collection.current_page + Kaminari.config.window + 1
105+
options[:right] = 0
106+
end
107+
100108
text_node paginate collection, options
101109
end
102110

@@ -117,22 +125,25 @@ def page_entries_info(options = {})
117125
entries_name = I18n.translate key, count: collection.size, default: entry_name.pluralize
118126
end
119127

120-
if collection.num_pages < 2
121-
case collection_size
122-
when 0; I18n.t('active_admin.pagination.empty', model: entries_name)
123-
when 1; I18n.t('active_admin.pagination.one', model: entry_name)
124-
else; I18n.t('active_admin.pagination.one_page', model: entries_name, n: collection.total_count)
125-
end
126-
else
127-
offset = (collection.current_page - 1) * collection.limit_value
128-
if @display_total
129-
total = collection.total_count
130-
I18n.t 'active_admin.pagination.multiple', model: entries_name, total: total,
131-
from: offset + 1, to: offset + collection_size
128+
if @display_total
129+
if collection.num_pages < 2
130+
case collection_size
131+
when 0; I18n.t("active_admin.pagination.empty", model: entries_name)
132+
when 1; I18n.t("active_admin.pagination.one", model: entry_name)
133+
else; I18n.t("active_admin.pagination.one_page", model: entries_name, n: collection.total_count)
134+
end
132135
else
133-
I18n.t 'active_admin.pagination.multiple_without_total', model: entries_name,
136+
offset = (collection.current_page - 1) * collection.limit_value
137+
total = collection.total_count
138+
I18n.t "active_admin.pagination.multiple", model: entries_name, total: total,
134139
from: offset + 1, to: offset + collection_size
135140
end
141+
else
142+
# Do not display total count, in order to prevent a `SELECT count(*)`.
143+
# To do so we must not call `collection.num_pages`
144+
offset = (collection.current_page - 1) * collection.limit_value
145+
I18n.t "active_admin.pagination.multiple_without_total", model: entries_name,
146+
from: offset + 1, to: offset + collection_size
136147
end
137148
end
138149

‎spec/unit/views/components/paginated_collection_spec.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,10 @@ def paginated_collection(*args)
198198
end
199199

200200
describe "set to false" do
201-
let(:pagination) { paginated_collection(collection, pagination_total: false) }
202-
203201
it "should not show the total item counts" do
202+
expect(collection).not_to receive(:num_pages)
203+
expect(collection).not_to receive(:total_pages)
204+
pagination = paginated_collection(collection, pagination_total: false)
204205
info = pagination.find_by_class('pagination_information').first.content.gsub('&nbsp;',' ')
205206
expect(info).to eq "Displaying posts <b>1 - 30</b>"
206207
end

0 commit comments

Comments
 (0)
0