activeadmin · mgrunberg · Aug 28, 2024 · Apr 3, 2018 · Apr 3, 2018 · Aug 27, 2024
diff --git a/app/controllers/active_admin/resource_controller/decorators.rb b/app/controllers/active_admin/resource_controller/decorators.rb
@@ -61,7 +61,7 @@ def self.wrap(decorator)
         def self.wrap!(parent, name)
           ::Class.new parent do
             delegate :reorder, :page, :current_page, :total_pages, :limit_value,
-                     :total_count, :total_pages, :offset, :to_key, :group_values,
+                     :total_count, :offset, :to_key, :group_values,
                      :except, :find_each, :ransack, to: :object
 
             define_singleton_method(:name) { name }

diff --git a/lib/active_admin/views/components/paginated_collection.rb b/lib/active_admin/views/components/paginated_collection.rb
@@ -102,7 +102,10 @@ def build_pagination
           # you pass in the :total_pages option. We issue a query to determine
           # if there is another page or not, but the limit/offset make this
           # query fast.
-          offset = @collection.offset(@collection.current_page * @collection.limit_value).limit(1).count
+          offset_scope = @collection.offset(@collection.current_page * @collection.limit_value)
+          # Support array collections. Kaminari::PaginatableArray does not respond to except
+          offset_scope = offset_scope.except(:select, :order) if offset_scope.respond_to?(:except)
+          offset = offset_scope.limit(1).count
           options[:total_pages] = @collection.current_page + offset
           options[:right] = 0
         end

diff --git a/spec/support/matchers/perform_database_query_matcher.rb b/spec/support/matchers/perform_database_query_matcher.rb
@@ -2,10 +2,12 @@
 
 RSpec::Matchers.define :perform_database_query do |query|
   match do |block|
+    query_regexp = query.is_a?(Regexp) ? query : Regexp.new(Regexp.escape(query))
+
     @match = nil
 
     callback = lambda do |_name, _started, _finished, _unique_id, payload|
-      @match = Regexp.new(Regexp.escape(query)).match?(payload[:sql])
+      @match = query_regexp.match?(payload[:sql])
     end
 
     ActiveSupport::Notifications.subscribed(callback, "sql.active_record", &block)

diff --git a/spec/unit/views/components/paginated_collection_spec.rb b/spec/unit/views/components/paginated_collection_spec.rb
@@ -229,16 +229,43 @@ def paginated_collection(*args)
       end
     end
 
-    it "makes no expensive COUNT queries when pagination_total is false" do
-      undecorated_collection = Post.all.page(1).per(30)
+    describe "when pagination_total is false" do
+      it "makes no expensive COUNT queries" do
+        undecorated_collection = Post.all.page(1).per(30)
+
+        expect { paginated_collection(undecorated_collection, pagination_total: false) }
+          .not_to perform_database_query("SELECT COUNT(*) FROM \"posts\"")
+
+        decorated_collection = controller_with_decorator("index", PostDecorator).apply_collection_decorator(undecorated_collection.reset)
+
+        expect { paginated_collection(decorated_collection, pagination_total: false) }
+          .not_to perform_database_query("SELECT COUNT(*) FROM \"posts\"")
+      end
+
+      it "makes a performant COUNT query to figure out if we are on the last page" do
+        # "SELECT COUNT(*) FROM (SELECT 1". Let's make sure the subquery has LIMIT and OFFSET. It shouldn't have ORDER BY
+        count_query = %r{SELECT COUNT\(\*\) FROM \(SELECT 1 .*FROM "posts" (?=.*OFFSET \?)(?=.*LIMIT \?)(?!.*ORDER BY)}
 
-      expect { paginated_collection(undecorated_collection, pagination_total: false) }
-        .not_to perform_database_query("SELECT COUNT(*) FROM \"posts\"")
+        undecorated_collection = Post.all.page(1).per(30)
 
-      decorated_collection = controller_with_decorator("index", PostDecorator).apply_collection_decorator(undecorated_collection)
+        expect { paginated_collection(undecorated_collection, pagination_total: false) }
+          .to perform_database_query(count_query)
 
-      expect { paginated_collection(decorated_collection, pagination_total: false) }
-        .not_to perform_database_query("SELECT COUNT(*) FROM \"posts\"")
+        undecorated_sorted_collection = undecorated_collection.reset.order(id: :desc)
+
+        expect { paginated_collection(undecorated_sorted_collection, pagination_total: false) }
+          .to perform_database_query(count_query)
+
+        decorated_collection = controller_with_decorator("index", PostDecorator).apply_collection_decorator(undecorated_collection.reset)
+
+        expect { paginated_collection(decorated_collection, pagination_total: false) }
+          .to perform_database_query(count_query)
+
+        decorated_sorted_collection = controller_with_decorator("index", PostDecorator).apply_collection_decorator(undecorated_sorted_collection.reset)
+
+        expect { paginated_collection(decorated_sorted_collection, pagination_total: false) }
+          .to perform_database_query(count_query)
+      end
     end
 
     it "makes no COUNT queries to figure out the last element of each page" do