Skip to content

Commit cf0e1a9

Browse files
committed
dont suppress all exceptions, ask if method exists first
fix spec: we actually did not test if Fixnum did not respond since we never undefined the methods add additional case to handle being provided a nil value
1 parent e3f490f commit cf0e1a9

File tree

2 files changed

+28
-9
lines changed

2 files changed

+28
-9
lines changed

lib/api-pagination.rb

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,24 +68,23 @@ def paginate_with_will_paginate(collection, options)
6868

6969
def get_default_per_page_for_kaminari(collection)
7070
default = Kaminari.config.default_per_page
71-
detect_model(collection).default_per_page || default
72-
rescue
73-
default
71+
extract_per_page_from_model(collection, :default_per_page) || default
7472
end
7573

7674
def default_per_page_for_will_paginate(collection)
7775
default = WillPaginate.per_page
78-
detect_model(collection).per_page || default
79-
rescue
80-
default
76+
extract_per_page_from_model(collection, :per_page) || default
8177
end
8278

83-
def detect_model(collection)
84-
if collection.respond_to?(:klass)
79+
def extract_per_page_from_model(collection, accessor)
80+
klass = if collection.respond_to?(:klass)
8581
collection.klass
8682
else
8783
collection.first.class
8884
end
85+
86+
return unless klass.respond_to?(accessor)
87+
klass.send(accessor)
8988
end
9089
end
9190
end

spec/rails_spec.rb

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ class Fixnum
287287
expect(response.header['Per-Page']).to eq('6')
288288
end
289289

290-
it 'should not fail if model does not respond to per page' do
290+
it 'should not fail if the model yields nil for per page' do
291291
class Fixnum
292292
@default_per_page = nil
293293
@per_page = nil
@@ -302,6 +302,26 @@ class Fixnum
302302
end
303303
)
304304
end
305+
306+
# This spec has to be last because if we undefine these methods
307+
# at runtime and then invoke another test that uses them, they will
308+
# raise a NoMethodError
309+
it 'should not fail if model does not respond to per page' do
310+
class Fixnum
311+
class << self
312+
undef_method :default_per_page, :per_page
313+
end
314+
end
315+
316+
get :index_with_no_per_page, params: {count: 100}
317+
318+
expect(response.header['Per-Page']).to eq(
319+
case ApiPagination.config.paginator
320+
when :kaminari then Kaminari.config.default_per_page.to_s
321+
when :will_paginate then WillPaginate.per_page.to_s
322+
end
323+
)
324+
end
305325
end
306326
end
307327
end

0 commit comments

Comments
 (0)