allow CSV pagination to be customizable

This fixes the performance issues that activeadmin#3375 introduced by switching
from `find_each` to Kaminari, since Kaminari has to do a SQL count. For
those that want that behavior they can enable it by setting
`paginate_with: :kaminari`, but the fastest option should be the
default for Active Admin.

I chose this approach instead of activeadmin#4082 because databases don’t
guarantee any particular order when requesting records, so it’s
possible that the resulting CSV won’t include all records.

In this commit I removed the `batch_size` method. Ideally it should be
a config option, but I didn’t have the time / motivation to write tests
for it. Currently both pagination methods use a 1000 record page size.

This commit refactors the tests that were creating their own
controllers, to reduce duplication.

While writing this I came across a bug with the byte order mark / BOM
character. Our Cucumber tests had set it to an empty string, which
would cause the unit tests to fail. Turns out we weren’t encoding that
character. I updated the code to fix that, but I didn’t think it was
important enough to write a separate test for.

Author: seanlinsley

docs/4-csv-format.md

@@ -56,3 +56,25 @@ environments where CSV streaming is disabled, you can change this setting:
 
 config.disable_streaming_in = ['development', 'staging']
 ```
+
+# Pagination
+
+By default we use `find_each` to paginate through your records because it's the
+best option made available by Rails. Note that it relies on the table's primary
+key, and orders the collection by ID. If you would like to configure either of
+those, you can use the `paginate_with` option.
+
+```ruby
+ActiveAdmin.register Post do
+  controller do
+    def collection
+      Post.order created_at: :desc
+    end
+  end
+
+  csv paginate_with: :kaminari
+end
+```
+
+You can also provide any custom pagination by passing a proc/lambda. The only
+requirement is that it needs to respond to `each`.

lib/active_admin/csv_builder.rb

@@ -26,14 +26,15 @@ def self.default_for_resource(resource)
       end
     end
 
-    attr_reader :columns, :options, :byte_order_mark, :column_names, :csv_options
+    attr_reader :columns, :options, :paginate_with, :byte_order_mark, :column_names, :csv_options
 
     COLUMN_TRANSITIVE_OPTIONS = [:humanize_name].freeze
 
     def initialize(options={}, &block)
       @resource = options.delete(:resource)
       @block    = block
       @options  = ActiveAdmin.application.csv_options.merge options
+      @paginate_with   = @options.delete(:paginate_with) { :find_each }
       @byte_order_mark = @options.delete :byte_order_mark
       @column_names    = @options.delete(:column_names) { true }
       @csv_options     = @options.except :encoding_options
@@ -47,17 +48,15 @@ def build(controller, csv)
       @collection = controller.send :find_collection, except: :pagination
       columns     = exec_columns controller.view_context
 
-      csv << byte_order_mark if byte_order_mark
+      csv << encode(byte_order_mark) if byte_order_mark
 
       if column_names
-        csv << CSV.generate_line(columns.map{ |c| encode c.name, options }, csv_options)
+        csv << CSV.generate_line(columns.map{ |c| encode c.name }, csv_options)
       end
 
-      (1..paginated_collection.total_pages).each do |page|
-        paginated_collection(page).each do |resource|
-          resource = controller.send :apply_decorator, resource
-          csv << CSV.generate_line(build_row(resource, columns, options), csv_options)
-        end
+      each_resource do |resource|
+        resource = controller.send :apply_decorator, resource
+        csv << CSV.generate_line(build_row(resource, columns), csv_options)
       end
 
       csv
@@ -72,13 +71,13 @@ def exec_columns(view_context = nil)
       @columns
     end
 
-    def build_row(resource, columns, options)
+    def build_row(resource, columns)
       columns.map do |column|
-        encode call_method_or_proc_on(resource, column.data), options
+        encode call_method_or_proc_on(resource, column.data)
       end
     end
 
-    def encode(content, options)
+    def encode(content)
       if options[:encoding]
         content.to_s.encode options[:encoding], options[:encoding_options]
       else
@@ -98,12 +97,23 @@ def column_transitive_options
       @column_transitive_options ||= @options.slice(*COLUMN_TRANSITIVE_OPTIONS)
     end
 
-    def paginated_collection(page_no = 1)
-      @collection.public_send(Kaminari.config.page_method_name, page_no).per(batch_size)
+    def each_resource
+      case paginate_with
+      when :find_each
+        @collection.find_each{ |resource| yield resource }
+      when :kaminari
+        (1..kaminari_collection.total_pages).each do |page|
+          kaminari_collection(page).each{ |resource| yield resource }
+        end
+      when Proc
+        paginate_with.call(@collection).each{ |resource| yield resource }
+      else
+        fail "unexpected argument for paginate_with: #{paginate_with}"
+      end
     end
 
-    def batch_size
-      1000
+    def kaminari_collection(page = 1)
+      @collection.public_send(Kaminari.config.page_method_name, page).per 1000
     end
 
     class Column

spec/unit/csv_builder_spec.rb

@@ -6,7 +6,27 @@
   let(:builder) { ActiveAdmin::CSVBuilder.new options, &block }
   let(:options) { {} }
   let(:block)   { ->{} }
-  before{ |ex| builder.send :exec_columns unless ex.metadata[:skip_exec] }
+
+  let(:view_context) {
+    context = double
+    method = MethodOrProcHelper.instance_method(:call_method_or_proc_on).bind(context)
+    allow(context).to receive(:call_method_or_proc_on){ |*args| method.call *args }
+    context
+  }
+  let(:controller) {
+    controller = double view_context: view_context, find_collection: collection
+    allow(controller).to receive(:apply_decorator) { |r| r }
+    controller
+  }
+  let(:collection) { Post.all }
+
+  before{ |ex| builder.send :exec_columns, view_context unless ex.metadata[:skip_exec] }
+
+  before :all do
+    Post.destroy_all
+    @post1 = Post.create!(title: "Hello1", published_date: Date.today - 2.day )
+    @post2 = Post.create!(title: "Hello2", published_date: Date.today - 1.day )
+  end
 
   context 'when empty' do
     it "has no columns" do
@@ -158,46 +178,45 @@
     end
   end
 
-  context "with access to the controller", skip_exec: true do
-    let(:dummy_view_context) { double(controller: dummy_controller) }
-    let(:dummy_controller) { double(names: %w(title summary updated_at created_at))}
+  context "with access to the controller"do
+    let(:view_context) { double controller: double(names: %w(title summary updated_at created_at)) }
     let(:block) {
       -> {
         column "id"
         controller.names.each{ |name| column name }
       }
     }
-    before{ builder.send :exec_columns, dummy_view_context }
 
     it "builds columns provided by the controller" do
       expect(builder.columns.map(&:data)).to match_array([:id, :title, :summary, :updated_at, :created_at])
     end
   end
 
-  context "build csv using the supplied order" do
-    before do
-      @post1 = Post.create!(title: "Hello1", published_date: Date.today - 2.day )
-      @post2 = Post.create!(title: "Hello2", published_date: Date.today - 1.day )
+  it "generates data ignoring pagination" do
+    expect(controller).to receive(:find_collection).with(except: :pagination).once
+    expect(builder).to    receive(:build_row).and_return([]).twice
+    builder.build controller, []
+  end
+
+  describe "paginate_with: :per_page" do
+    it "works" do
+      expect(collection).to receive(:find_each).and_call_original
+      builder.build controller, []
     end
-    let(:dummy_controller) {
-      class DummyController
-        def find_collection(*)
-          collection
-        end
+  end
 
-        def collection
-          Post.order('published_date DESC')
-        end
+  describe "paginate_with: <Proc>" do
+    let(:options) { {paginate_with: ->collection { collection }} }
 
-        def apply_decorator(resource)
-          resource
-        end
+    it "works" do
+      expect(collection).to receive(:each).and_call_original
+      builder.build controller, []
+    end
+  end
 
-        def view_context
-        end
-      end
-      DummyController.new
-    }
+  describe "paginate_with: :kaminari" do
+    let(:options) { {paginate_with: :kaminari} }
+    let(:collection) { Post.order published_date: :desc }
     let(:block) {
       -> {
         column "id"
@@ -209,71 +228,43 @@ def view_context
     it "generates data with the supplied order" do
       expect(builder).to receive(:build_row).and_return([]).once.ordered { |post| expect(post.id).to eq @post2.id }
       expect(builder).to receive(:build_row).and_return([]).once.ordered { |post| expect(post.id).to eq @post1.id }
-      builder.build dummy_controller, []
-    end
-
-    it "generates data ignoring pagination" do
-      expect(dummy_controller).to receive(:find_collection).
-        with(except: :pagination).once.
-        and_call_original
-      expect(builder).to receive(:build_row).and_return([]).twice
-      builder.build dummy_controller, []
+      builder.build controller, []
     end
-
   end
 
-  context "build csv using specified encoding and encoding_options" do
-    let(:dummy_controller) do
-      class DummyController
-        def find_collection(*)
-          collection
-        end
-
-        def collection
-          Post
-        end
-
-        def apply_decorator(resource)
-          resource
-        end
-
-        def view_context
-        end
-      end
-      DummyController.new
-    end
+  describe ":encoding and :encoding_options" do
     let(:encoding) { Encoding::ASCII }
     let(:encoding_options) { {} }
     let(:options) { {encoding: encoding, encoding_options: encoding_options} }
     let(:block) {
       -> {
-        column "おはようございます"
-        column "title"
+        column("おはようございます") { |p| p.title }
       }
     }
 
-    context "Shift-JIS with options" do
+    context "Shift-JIS" do
       let(:encoding) { Encoding::Shift_JIS }
       let(:encoding_options) { {invalid: :replace, undef: :replace, replace: "?"} }
 
       it "encodes the CSV" do
-        receiver = []
-        builder.build dummy_controller, receiver
-        line = receiver.last
-        expect(line.encoding).to eq(encoding)
+        csv = []
+        builder.build controller, csv
+
+        expect(csv.map(&:encoding).uniq).to eq [encoding]
+        expect(csv).to include "おはようございます\n".encode(encoding, encoding_options)
       end
     end
 
-    context "ASCII with options" do
+    context "ASCII" do
       let(:encoding) { Encoding::ASCII }
       let(:encoding_options) { {invalid: :replace, undef: :replace, replace: "__REPLACED__"} }
 
       it "encodes the CSV without errors" do
-        receiver = []
-        builder.build dummy_controller, receiver
-        line = receiver.last
-        expect(line.encoding).to eq(encoding)
-        expect(line).to include("__REPLACED__")
+        csv = []
+        builder.build controller, csv
+
+        expect(csv.map(&:encoding).uniq).to eq [encoding]
+        expect(csv.first).to include "__REPLACED__"
       end
     end
   end