Merge pull request #95 from mach-kernel/fix-detect-model

Better behavior for #detect_model

Author: davidcelis

api-pagination.gemspec

@@ -21,4 +21,5 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'railties', '>= 5.0.0'
   s.add_development_dependency 'actionpack', '>= 5.0.0'
   s.add_development_dependency 'sequel', '>= 4.9.0'
+  s.add_development_dependency 'activerecord-nulldb-adapter', '>= 0.3.9'
 end

lib/api-pagination.rb

@@ -118,24 +118,23 @@ def paginate_with_will_paginate(collection, options)
 
     def get_default_per_page_for_kaminari(collection)
       default = Kaminari.config.default_per_page
-      detect_model(collection).default_per_page || default
-    rescue
-      default
+      extract_per_page_from_model(collection, :default_per_page) || default
     end
 
     def default_per_page_for_will_paginate(collection)
       default = WillPaginate.per_page
-      detect_model(collection).per_page || default
-    rescue
-      default
+      extract_per_page_from_model(collection, :per_page) || default
     end
 
-    def detect_model(collection)
-      if collection.respond_to?(:table_name)
-        collection.table_name.classify.constantize
+    def extract_per_page_from_model(collection, accessor)
+      klass = if collection.respond_to?(:klass)
+        collection.klass
       else
         collection.first.class
       end
+
+      return unless klass.respond_to?(accessor)
+      klass.send(accessor)
     end
   end
 end

spec/active_record_spec.rb

@@ -0,0 +1,50 @@
+require 'spec_helper'
+require 'support/active_record/foo'
+require 'nulldb_rspec'
+
+ActiveRecord::Base.establish_connection(
+  adapter: :nulldb,
+  schema: 'spec/support/active_record/schema.rb'
+)
+
+NullDB.configure { |ndb| def ndb.project_root; Dir.pwd; end; }
+
+shared_examples 'produces_correct_sql' do 
+  it 'produces correct sql for first page' do
+    allow(collection).to receive(:count).and_return(collection_size)
+    paginated_sql, _ = ApiPagination.paginate(collection, per_page: per_page)
+    expect(paginated_sql.to_sql).to eql(Foo.limit(per_page).offset(0).to_sql)
+  end
+end
+
+describe 'ActiveRecord Support' do 
+  let(:collection) { Foo.all }
+  let(:collection_size) { 50 }
+  let(:per_page) { 5 }
+  
+  if ApiPagination.config.paginator == :will_paginate
+    require 'will_paginate/active_record'
+  end
+
+  context "pagination with #{ApiPagination.config.paginator}" do 
+    include_examples 'produces_correct_sql'
+  end
+
+
+  if ApiPagination.config.paginator != :pagy
+    context 'reflections' do
+      it 'invokes the correct methods to determine type' do 
+        expect(collection).to receive(:klass).at_least(:once)
+                                             .and_call_original
+        ApiPagination.paginate(collection)
+      end
+
+      it 'does not fail if table name is not snake cased class name' do
+        allow(collection).to receive(:table_name).and_return(SecureRandom.uuid)
+        expect { ApiPagination.paginate(collection) }.to_not raise_error
+      end
+    end
+  end
+end
+
+

spec/rails_spec.rb

@@ -262,13 +262,21 @@ class Fixnum
           end
         end
 
+        after :all do 
+          class Fixnum
+            class << self
+              undef_method :default_per_page, :per_page
+            end
+          end
+        end
+
         it 'should use default per page from model' do
           get :index_with_no_per_page, params: {count: 100}
 
           expect(response.header['Per-Page']).to eq('6')
         end
 
-        it 'should not fail if model does not respond to per page' do
+        it 'should not fail if the model yields nil for per page' do
           class Fixnum
             @default_per_page = nil
             @per_page = nil
@@ -286,5 +294,19 @@ class Fixnum
         end
       end
     end
+
+    context 'default per page in objects without paginator defaults' do 
+      it 'should not fail if model does not respond to per page' do
+        get :index_with_no_per_page, params: {count: 100}
+
+        expect(response.header['Per-Page']).to eq(
+          case ApiPagination.config.paginator
+          when :pagy          then Pagy::VARS[:items].to_s
+          when :kaminari      then Kaminari.config.default_per_page.to_s
+          when :will_paginate then WillPaginate.per_page.to_s
+          end
+        )
+      end
+    end
   end
-end
+end

spec/support/active_record/foo.rb

@@ -0,0 +1,3 @@
+require 'active_record'
+
+class Foo < ActiveRecord::Base; end

spec/support/active_record/schema.rb

@@ -0,0 +1,5 @@
+ActiveRecord::Schema.define(version: 0) do
+  create_table "foos", :force => true do |t|
+    t.string   "foo"
+  end
+end