From af769b6a3a589c83887fee94d1c5a1657d3d664c Mon Sep 17 00:00:00 2001 From: Yuki Nishijima Date: Sun, 24 Aug 2014 17:29:21 -0700 Subject: [PATCH] Raise ZeroPerPageOperation when total_pages/current_page is called after per(0) closes #296, closes #585 --- lib/kaminari.rb | 1 + lib/kaminari/exceptions.rb | 3 +++ lib/kaminari/models/page_scope_methods.rb | 8 +++++++- spec/models/active_record/scopes_spec.rb | 18 ++++++++++++++++-- spec/models/array_spec.rb | 18 ++++++++++++++++-- 5 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 lib/kaminari/exceptions.rb diff --git a/lib/kaminari.rb b/lib/kaminari.rb index 41304ff..a63aae7 100644 --- a/lib/kaminari.rb +++ b/lib/kaminari.rb @@ -26,6 +26,7 @@ EOC # load Kaminari components require 'kaminari/config' +require 'kaminari/exceptions' require 'kaminari/helpers/action_view_extension' require 'kaminari/helpers/paginator' require 'kaminari/models/page_scope_methods' diff --git a/lib/kaminari/exceptions.rb b/lib/kaminari/exceptions.rb new file mode 100644 index 0000000..4987db5 --- /dev/null +++ b/lib/kaminari/exceptions.rb @@ -0,0 +1,3 @@ +module Kaminari + class ZeroPerPageOperation < ZeroDivisionError; end +end diff --git a/lib/kaminari/models/page_scope_methods.rb b/lib/kaminari/models/page_scope_methods.rb index eb34948..8dc5e9c 100644 --- a/lib/kaminari/models/page_scope_methods.rb +++ b/lib/kaminari/models/page_scope_methods.rb @@ -3,8 +3,10 @@ module Kaminari # Specify the per_page value for the preceding page scope # Model.page(3).per(10) def per(num) - if (n = num.to_i) <= 0 + if (n = num.to_i) < 0 || !(/^\d/ =~ num.to_s) self + elsif n.zero? + limit(n) elsif max_per_page && max_per_page < n limit(max_per_page).offset(offset_value / limit_value * max_per_page) else @@ -29,6 +31,8 @@ module Kaminari else total_pages_count end + rescue FloatDomainError => e + raise ZeroPerPageOperation, "The number of total pages was incalculable. Perhaps you called .per(0)?" end #FIXME for compatibility. remove num_pages at some time in the future alias num_pages total_pages @@ -40,6 +44,8 @@ module Kaminari offset_without_padding = 0 if offset_without_padding < 0 (offset_without_padding / limit_value) + 1 + rescue ZeroDivisionError => e + raise ZeroPerPageOperation, "Current page was incalculable. Perhaps you called .per(0)?" end # Next page number in the collection diff --git a/spec/models/active_record/scopes_spec.rb b/spec/models/active_record/scopes_spec.rb index e938c62..7db6c38 100644 --- a/spec/models/active_record/scopes_spec.rb +++ b/spec/models/active_record/scopes_spec.rb @@ -83,6 +83,11 @@ if defined? ActiveRecord subject { model_class.page(1).per(nil) } it { should have(model_class.default_per_page).users } end + + context "page 1 per 0" do + subject { model_class.page(1).per(0) } + it { should have(0).users } + end end describe '#padding' do @@ -115,9 +120,11 @@ if defined? ActiveRecord its(:total_pages) { should == 1 } end - context 'per 0 (using default)' do + context 'per 0' do subject { model_class.page(50).per(0) } - its(:total_pages) { should == 4 } + it "raises Kaminari::ZeroPerPageOperation" do + expect { subject.total_pages }.to raise_error(Kaminari::ZeroPerPageOperation) + end end context 'per -1 (using default)' do @@ -157,6 +164,13 @@ if defined? ActiveRecord end describe '#current_page' do + context 'any page, per 0' do + subject { model_class.page.per(0) } + it "raises Kaminari::ZeroPerPageOperation" do + expect { subject.current_page }.to raise_error(Kaminari::ZeroPerPageOperation) + end + end + context 'page 1' do subject { model_class.page } its(:current_page) { should == 1 } diff --git a/spec/models/array_spec.rb b/spec/models/array_spec.rb index 40938dc..a6b24af 100644 --- a/spec/models/array_spec.rb +++ b/spec/models/array_spec.rb @@ -54,6 +54,11 @@ describe Kaminari::PaginatableArray do it { should have(5).users } its(:first) { should == 1 } end + + context "page 1 per 0" do + subject { array.page(1).per(0) } + it { should have(0).users } + end end describe '#total_pages' do @@ -72,9 +77,11 @@ describe Kaminari::PaginatableArray do its(:total_pages) { should == 1 } end - context 'per 0 (using default)' do + context 'per 0' do subject { array.page(50).per(0) } - its(:total_pages) { should == 4 } + it "raises Kaminari::ZeroPerPageOperation" do + expect { subject.total_pages }.to raise_error(Kaminari::ZeroPerPageOperation) + end end context 'per -1 (using default)' do @@ -94,6 +101,13 @@ describe Kaminari::PaginatableArray do end describe '#current_page' do + context 'any page, per 0' do + subject { array.page.per(0) } + it "raises Kaminari::ZeroPerPageOperation" do + expect { subject.current_page }.to raise_error(Kaminari::ZeroPerPageOperation) + end + end + context 'page 1' do subject { array.page } its(:current_page) { should == 1 }