From 0728f7488ae4f245e88d7412bb40b1cd1fa6a060 Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Tue, 8 Sep 2026 19:26:30 -0400 Subject: [PATCH 1/2] Default `Paginate#base_path` to "/" instead of nil `base_path` is an optional keyword argument, but when it is omitted (or passed as `nil`) and the collection spans more than one page, `next` and `previous` build the path with `@base_path.sub(...)` and raise `NoMethodError: undefined method 'sub' for nil`. Default it to `"/"` (and coerce an explicit `nil`), so an omitted `base_path` yields root-relative paths like `/page/2/` instead of crashing. Callers that pass a `base_path` are unaffected. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB --- lib/perron/paginate.rb | 4 ++-- test/perron/paginate_test.rb | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/perron/paginate.rb b/lib/perron/paginate.rb index 0fc7ef9..2ec63ac 100644 --- a/lib/perron/paginate.rb +++ b/lib/perron/paginate.rb @@ -2,10 +2,10 @@ module Perron class Paginate - def initialize(collection, page:, per_page:, base_path: nil, page_path_template: nil, use_query_params: false) + def initialize(collection, page:, per_page:, base_path: "/", page_path_template: nil, use_query_params: false) @collection = collection @per_page = per_page - @base_path = base_path + @base_path = base_path || "/" @page_path_template = page_path_template || "/page/:page/" @use_query_params = use_query_params diff --git a/test/perron/paginate_test.rb b/test/perron/paginate_test.rb index c4c7333..6dec375 100644 --- a/test/perron/paginate_test.rb +++ b/test/perron/paginate_test.rb @@ -193,4 +193,22 @@ class Perron::PaginateTest < ActiveSupport::TestCase assert_equal "/articles/?page=2", paginate.previous end + + test "next falls back to a root-relative path when base_path is omitted" do + paginate = Perron::Paginate.new((1..10).to_a, page: 1, per_page: 5) + + assert_equal "/page/2/", paginate.next + end + + test "previous falls back to root when base_path is omitted" do + paginate = Perron::Paginate.new((1..10).to_a, page: 2, per_page: 5) + + assert_equal "/", paginate.previous + end + + test "an explicit nil base_path is treated as root" do + paginate = Perron::Paginate.new((1..10).to_a, page: 1, per_page: 5, base_path: nil) + + assert_equal "/page/2/", paginate.next + end end From 8f5fcb5c24c56b94be0e33cc92473b8cb76340d4 Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Tue, 8 Sep 2026 19:34:02 -0400 Subject: [PATCH 2/2] Raise ArgumentError for a non-positive `per_page` `config.pagination.per_page` has no default, so an unconfigured collection reaches `Paginate.new(per_page: nil)` and dies with `TypeError: nil can't be coerced into Float`. `per_page: 0` gives `FloatDomainError: Infinity`, and a negative value gives `ArgumentError: min argument must be smaller than max argument` from the `clamp`. Validate up front with a message that says what is wrong. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB --- lib/perron/paginate.rb | 4 ++++ test/perron/paginate_test.rb | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/perron/paginate.rb b/lib/perron/paginate.rb index 2ec63ac..a10b4a7 100644 --- a/lib/perron/paginate.rb +++ b/lib/perron/paginate.rb @@ -3,6 +3,10 @@ module Perron class Paginate def initialize(collection, page:, per_page:, base_path: "/", page_path_template: nil, use_query_params: false) + unless per_page.is_a?(Integer) && per_page.positive? + raise ArgumentError, "per_page must be a positive integer, got #{per_page.inspect}" + end + @collection = collection @per_page = per_page @base_path = base_path || "/" diff --git a/test/perron/paginate_test.rb b/test/perron/paginate_test.rb index 6dec375..4e2c10b 100644 --- a/test/perron/paginate_test.rb +++ b/test/perron/paginate_test.rb @@ -211,4 +211,14 @@ class Perron::PaginateTest < ActiveSupport::TestCase assert_equal "/page/2/", paginate.next end + + test "raises ArgumentError for a non-positive or non-integer per_page" do + [0, -5, nil, 2.5].each do |bad| + error = assert_raises(ArgumentError) do + Perron::Paginate.new((1..10).to_a, page: 1, per_page: bad) + end + + assert_match(/per_page must be a positive integer/, error.message) + end + end end