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

Commit 6ab8c65

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. 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 6ab8c65

File tree

3 files changed

+34
-16
lines changed

3 files changed

+34
-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: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,16 @@ 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. We issue a query to determine
104+
# if there is another page or not.
105+
offset = collection.offset(collection.current_page * @per_page.to_i + 1).limit(1).count
106+
options[:total_pages] = collection.current_page + offset
107+
options[:right] = 0
108+
end
109+
100110
text_node paginate collection, options
101111
end
102112

@@ -117,22 +127,30 @@ def page_entries_info(options = {})
117127
entries_name = I18n.translate key, count: collection.size, default: entry_name.pluralize
118128
end
119129

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)
130+
if @display_total
131+
if collection.num_pages < 2
132+
case collection_size
133+
when 0; I18n.t("active_admin.pagination.empty", model: entries_name)
134+
when 1; I18n.t("active_admin.pagination.one", model: entry_name)
135+
else; I18n.t("active_admin.pagination.one_page", model: entries_name, n: collection.total_count)
136+
end
137+
else
138+
offset = (collection.current_page - 1) * collection.limit_value
139+
total = collection.total_count
140+
I18n.t "active_admin.pagination.multiple",
141+
model: entries_name,
142+
total: total,
143+
from: offset + 1,
144+
to: offset + collection_size
125145
end
126146
else
147+
# Do not display total count, in order to prevent a `SELECT count(*)`.
148+
# To do so we must not call `collection.num_pages`
127149
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
132-
else
133-
I18n.t 'active_admin.pagination.multiple_without_total', model: entries_name,
134-
from: offset + 1, to: offset + collection_size
135-
end
150+
I18n.t "active_admin.pagination.multiple_without_total",
151+
model: entries_name,
152+
from: offset + 1,
153+
to: offset + collection_size
136154
end
137155
end
138156

‎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