diff --git a/app/logical/danbooru/paginator/active_record_extension.rb b/app/logical/danbooru/paginator/active_record_extension.rb index 667c8dc73..1e9488495 100644 --- a/app/logical/danbooru/paginator/active_record_extension.rb +++ b/app/logical/danbooru/paginator/active_record_extension.rb @@ -37,17 +37,17 @@ module Danbooru c = c.where("#{table_name}.id < ?", before_id.to_i) end - c.reorder("#{table_name}.id desc").tap do |obj| - obj.extend(SequentialCollectionExtension) - obj.sequential_paginator_mode = :before - end + c = c.reorder("#{table_name}.id desc") + c = c.extending(SequentialCollectionExtension) + c.sequential_paginator_mode = :before + c end def paginate_sequential_after(after_id) - limit(records_per_page + 1).where("#{table_name}.id > ?", after_id.to_i).reorder("#{table_name}.id asc").tap do |obj| - obj.extend(SequentialCollectionExtension) - obj.sequential_paginator_mode = :after - end + c = limit(records_per_page + 1).where("#{table_name}.id > ?", after_id.to_i).reorder("#{table_name}.id asc") + c = c.extending(SequentialCollectionExtension) + c.sequential_paginator_mode = :after + c end def paginate_numbered(page) @@ -57,8 +57,7 @@ module Danbooru raise ::Danbooru::Paginator::PaginationError.new("You cannot go beyond page #{Danbooru.config.max_numbered_pages}. Please narrow your search terms.") end - limit(records_per_page).offset((page - 1) * records_per_page).tap do |obj| - obj.extend(NumberedCollectionExtension) + extending(NumberedCollectionExtension).limit(records_per_page).offset((page - 1) * records_per_page).tap do |obj| if records_per_page > 0 obj.total_pages = (obj.total_count.to_f / records_per_page).ceil else diff --git a/app/logical/danbooru/paginator/sequential_collection_extension.rb b/app/logical/danbooru/paginator/sequential_collection_extension.rb index 9cee8fc4a..c78da7846 100644 --- a/app/logical/danbooru/paginator/sequential_collection_extension.rb +++ b/app/logical/danbooru/paginator/sequential_collection_extension.rb @@ -19,7 +19,10 @@ module Danbooru end end - def to_a + # XXX Hack: in sequential pagination we fetch one more record than we need + # so that we can tell when we're on the first or last page. Here we override + # a rails internal method to discard that extra record. See #2044, #3642. + def records if sequential_paginator_mode == :before super.first(records_per_page) else diff --git a/test/unit/paginator_test.rb b/test/unit/paginator_test.rb new file mode 100644 index 000000000..2f1eb9767 --- /dev/null +++ b/test/unit/paginator_test.rb @@ -0,0 +1,34 @@ +require 'test_helper' + +class PaginatorTest < ActiveSupport::TestCase + setup do + @posts = FactoryBot.create_list(:post, 5) + end + + context "sequential pagination (before)" do + should "return the correct set of records" do + expected_posts = Post.limit(3).order(id: :desc) + posts = Post.paginate("b9999999", limit: 3) + + assert_equal(expected_posts.map(&:id), posts.map(&:id)) + end + end + + context "sequential pagination (after)" do + should "return the correct set of records" do + expected_posts = Post.limit(3).order(id: :asc).reverse + posts = Post.paginate("a0", limit: 3) + + assert_equal(expected_posts.map(&:id), posts.map(&:id)) + end + end + + context "numbered pagination" do + should "return the correct set of records" do + expected_posts = Post.limit(3).order(id: :desc) + posts = Post.order(id: :desc).paginate("1", limit: 3) + + assert_equal(expected_posts.map(&:id), posts.map(&:id)) + end + end +end