From af406297a384370901c3731d04cf91b5ff593838 Mon Sep 17 00:00:00 2001 From: Yuki Nishijima Date: Thu, 11 Apr 2019 13:12:03 -0400 Subject: [PATCH] Spike on deprecating #current_page method --- .../lib/kaminari/helpers/helper_methods.rb | 6 +++--- .../lib/kaminari/helpers/paginator.rb | 3 ++- kaminari-core/lib/kaminari/helpers/tags.rb | 1 + .../lib/kaminari/models/page_scope_methods.rb | 4 ++++ .../helpers/action_view_extension_test.rb | 11 ++++++++++ .../test/models/active_record/scopes_test.rb | 1 - kaminari-core/test/models/array_test.rb | 20 ------------------- 7 files changed, 21 insertions(+), 25 deletions(-) diff --git a/kaminari-core/lib/kaminari/helpers/helper_methods.rb b/kaminari-core/lib/kaminari/helpers/helper_methods.rb index 295669a..896e0a1 100644 --- a/kaminari-core/lib/kaminari/helpers/helper_methods.rb +++ b/kaminari-core/lib/kaminari/helpers/helper_methods.rb @@ -69,7 +69,7 @@ module Kaminari # # It will return `nil` if there is no next page. def next_page_path(scope, options = {}) - Kaminari::Helpers::NextPage.new(self, options.reverse_merge(current_page: scope.current_page)).url if scope.next_page + Kaminari::Helpers::NextPage.new(self, options).url if scope.next_page end alias path_to_next_page next_page_path @@ -83,7 +83,7 @@ module Kaminari # # It will return `nil` if there is no previous page. def prev_page_path(scope, options = {}) - Kaminari::Helpers::PrevPage.new(self, options.reverse_merge(current_page: scope.current_page)).url if scope.prev_page + Kaminari::Helpers::PrevPage.new(self, options).url if scope.prev_page end alias previous_page_path prev_page_path alias path_to_previous_page prev_page_path @@ -110,7 +110,7 @@ module Kaminari # * :ANY_OTHER_VALUES - Any other hash key & values would be directly passed into each tag as :locals value. def paginate(scope, paginator_class: Kaminari::Helpers::Paginator, template: nil, **options) options[:total_pages] ||= scope.total_pages - options.reverse_merge! current_page: scope.current_page, per_page: scope.limit_value, remote: false + options.reverse_merge! per_page: scope.limit_value, remote: false paginator = paginator_class.new (template || self), options paginator.to_s diff --git a/kaminari-core/lib/kaminari/helpers/paginator.rb b/kaminari-core/lib/kaminari/helpers/paginator.rb index d89cc42..af27695 100644 --- a/kaminari-core/lib/kaminari/helpers/paginator.rb +++ b/kaminari-core/lib/kaminari/helpers/paginator.rb @@ -9,10 +9,11 @@ module Kaminari class Paginator < Tag def initialize(template, window: nil, outer_window: Kaminari.config.outer_window, left: Kaminari.config.left, right: Kaminari.config.right, inner_window: Kaminari.config.window, **options) #:nodoc: @window_options = {window: window || inner_window, left: left.zero? ? outer_window : left, right: right.zero? ? outer_window : right} + @param_name = options[:param_name] || Kaminari.config.param_name @template, @options, @theme, @views_prefix, @last = template, options, options[:theme], options[:views_prefix], nil @window_options.merge! @options - @window_options[:current_page] = @options[:current_page] = PageProxy.new(@window_options, @options[:current_page], nil) + @window_options[:current_page] = @options[:current_page] = PageProxy.new(@window_options, (@options[:current_page] || @template.params[@param_name] || 1).to_i, nil) #XXX Using parent template's buffer class for rendering each partial here. This might cause problems if the handler mismatches @output_buffer = if defined?(::ActionView::OutputBuffer) diff --git a/kaminari-core/lib/kaminari/helpers/tags.rb b/kaminari-core/lib/kaminari/helpers/tags.rb index 99dfebc..6c7732f 100644 --- a/kaminari-core/lib/kaminari/helpers/tags.rb +++ b/kaminari-core/lib/kaminari/helpers/tags.rb @@ -26,6 +26,7 @@ module Kaminari @params = @params.with_indifferent_access @params.except!(*PARAM_KEY_BLACKLIST) @params.merge! params + @options[:current_page] ||= (@params[@param_name] || 1).to_i end def to_s(locals = {}) #:nodoc: diff --git a/kaminari-core/lib/kaminari/models/page_scope_methods.rb b/kaminari-core/lib/kaminari/models/page_scope_methods.rb index d5ac025..f9130ba 100644 --- a/kaminari-core/lib/kaminari/models/page_scope_methods.rb +++ b/kaminari-core/lib/kaminari/models/page_scope_methods.rb @@ -45,6 +45,10 @@ module Kaminari # Current page number def current_page + ActiveSupport::Deprecation.warn "The #current_page method has been deprecated and will be made private in the " \ + "next major version. This method will eventually be removed entirely once the " \ + "internal code is refactored." + offset_without_padding = offset_value offset_without_padding -= @_padding if defined?(@_padding) && @_padding offset_without_padding = 0 if offset_without_padding < 0 diff --git a/kaminari-core/test/helpers/action_view_extension_test.rb b/kaminari-core/test/helpers/action_view_extension_test.rb index 232726f..0cb6b0a 100644 --- a/kaminari-core/test/helpers/action_view_extension_test.rb +++ b/kaminari-core/test/helpers/action_view_extension_test.rb @@ -77,6 +77,7 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) end test "page: 20 (out of range)" do + view.params[:page] = 20 users = User.page(20) html = view.paginate users, params: {controller: 'users', action: 'index'} @@ -92,6 +93,7 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) sub_test_case 'having previous pages' do test 'the default behaviour' do + view.params[:page] = 3 users = User.page(3) html = view.link_to_previous_page users, 'Previous', params: {controller: 'users', action: 'index'} assert_match(/page=2/, html) @@ -130,6 +132,7 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) end test '#link_to_previous_page accepts ActionController::Parameters' do + view.params[:page] = 3 users = User.page(3) params = ActionController::Parameters.new(controller: 'users', action: 'index', status: 'active') @@ -494,6 +497,7 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) end test 'the second page' do + view.params[:page] = 2 users = User.page(2).per(10) html = view.rel_next_prev_link_tags users, params: {controller: 'users', action: 'index'} @@ -504,6 +508,7 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) end test 'the last page' do + view.params[:page] = 4 users = User.page(4).per(10) html = view.rel_next_prev_link_tags users, params: {controller: 'users', action: 'index'} @@ -524,6 +529,7 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) end test 'the last page' do + view.params[:page] = 2 users = User.page(2).per(1) assert_nil view.path_to_next_page(users, params: {controller: 'users', action: 'index'}) end @@ -540,11 +546,13 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) end test 'the second page' do + view.params[:page] = 2 users = User.page(2).per(1) assert_equal '/users', view.path_to_prev_page(users, params: {controller: 'users', action: 'index'}) end test 'the last page' do + view.params[:page] = 3 users = User.page(3).per(1) assert_equal'/users?page=2', view.path_to_prev_page(users, params: {controller: 'users', action: 'index'}) end @@ -561,6 +569,7 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) end test 'the last page' do + view.params[:page] = 2 users = User.page(2).per(1) assert_nil view.next_page_url(users, params: {controller: 'users', action: 'index'}) end @@ -577,11 +586,13 @@ if defined?(::Rails::Railtie) && defined?(::ActionView) end test 'the second page' do + view.params[:page] = 2 users = User.page(2).per(1) assert_equal 'http://test.host/users', view.prev_page_url(users, params: {controller: 'users', action: 'index'}) end test 'the last page' do + view.params[:page] = 3 users = User.page(3).per(1) assert_equal'http://test.host/users?page=2', view.prev_page_url(users, params: {controller: 'users', action: 'index'}) end diff --git a/kaminari-core/test/models/active_record/scopes_test.rb b/kaminari-core/test/models/active_record/scopes_test.rb index d852844..12ecb8a 100644 --- a/kaminari-core/test/models/active_record/scopes_test.rb +++ b/kaminari-core/test/models/active_record/scopes_test.rb @@ -211,7 +211,6 @@ if defined? ActiveRecord test 'page 19 per 5 padding 5' do relation = model_class.page(19).per(5).padding(5) - assert_equal 19, relation.current_page assert_equal 19, relation.total_pages end diff --git a/kaminari-core/test/models/array_test.rb b/kaminari-core/test/models/array_test.rb index dba6da5..5346889 100644 --- a/kaminari-core/test/models/array_test.rb +++ b/kaminari-core/test/models/array_test.rb @@ -18,7 +18,6 @@ class PaginatableArrayTest < ActiveSupport::TestCase sub_test_case '#page' do def assert_first_page_of_array(arr) assert_equal 25, arr.count - assert_equal 1, arr.current_page assert_equal 1, arr.first end @@ -34,7 +33,6 @@ class PaginatableArrayTest < ActiveSupport::TestCase arr = @array.page 2 assert_equal 25, arr.count - assert_equal 2, arr.current_page assert_equal 26, arr.first end @@ -82,7 +80,6 @@ class PaginatableArrayTest < ActiveSupport::TestCase test 'page 19 per 5 padding 5' do arr = @array.page(19).per(5).padding(5) - assert_equal 19, arr.current_page assert_equal 19, arr.total_pages end @@ -121,20 +118,6 @@ class PaginatableArrayTest < ActiveSupport::TestCase end end - sub_test_case '#current_page' do - test 'any page, per 0' do - assert_raise(Kaminari::ZeroPerPageOperation) { @array.page.per(0).current_page } - end - - test 'page 1' do - assert_equal 1, @array.page(1).current_page - end - - test 'page 2' do - assert_equal 2, @array.page(2).per(3).current_page - end - end - sub_test_case '#next_page' do test 'page 1' do assert_equal 2, @array.page.next_page @@ -175,7 +158,6 @@ class PaginatableArrayTest < ActiveSupport::TestCase assert_equal 10, arr.count assert_equal 1, arr.first - assert_equal 5, arr.current_page assert_equal 9999, arr.total_count end @@ -184,7 +166,6 @@ class PaginatableArrayTest < ActiveSupport::TestCase assert_equal 10, arr.count assert_equal 1, arr.first - assert_equal 1, arr.current_page assert_equal 15, arr.total_count end @@ -193,7 +174,6 @@ class PaginatableArrayTest < ActiveSupport::TestCase assert_equal 5, arr.count assert_equal 11, arr.first - assert_equal 2, arr.current_page assert_equal 15, arr.total_count end end