[Pools] Fix discrepancy between index page count and actual page count

One was using `user.per_page`, the other config.posts_per_page. Consolidate this
logic to `paginate_posts` to make it more obvious what's going on.

app/controllers/artists_controller.rb

@@ -42,7 +42,7 @@ class ArtistsController < ApplicationController
         return
       end
     end
-    @post_set = PostSets::Post.new(@artist.name, 1, 10)
+    @post_set = PostSets::Post.new(@artist.name, 1, limit: 10)
     respond_with(@artist, methods: [:domains], include: [:urls])
   end
 
@@ -84,7 +84,7 @@ class ArtistsController < ApplicationController
       redirect_to artist_path(@artist)
     else
       @artist = Artist.new(name: params[:name] || "")
-      @post_set = PostSets::Post.new(@artist.name, 1, 10)
+      @post_set = PostSets::Post.new(@artist.name, 1, limit: 10)
       respond_with(@artist)
     end
   end

app/controllers/favorites_controller.rb

@@ -16,7 +16,7 @@ class FavoritesController < ApplicationController
         raise Favorite::HiddenError
       end
 
-      @favorite_set = PostSets::Favorites.new(@user, params[:page], params[:limit])
+      @favorite_set = PostSets::Favorites.new(@user, params[:page], limit: params[:limit])
       respond_with(@favorite_set.posts) do |fmt|
         fmt.json do
           render json: @favorite_set.api_posts, root: 'posts'

app/controllers/mascots_controller.rb

@@ -5,7 +5,7 @@ class MascotsController < ApplicationController
   before_action :admin_only, except: [:index]
 
   def index
-    @mascots = Mascot.search(search_params).paginate(params[:page], limit: 75)
+    @mascots = Mascot.search(search_params).paginate(params[:page], limit: params[:limit])
     respond_with(@mascots)
   end
 

app/controllers/pools_controller.rb

@@ -26,16 +26,14 @@ class PoolsController < ApplicationController
   end
 
   def gallery
-    params[:limit] ||= CurrentUser.user.per_page
-
-    @pools = Pool.search(search_params).paginate(params[:page], :limit => params[:limit], :search_count => params[:search])
+    @pools = Pool.search(search_params).paginate_posts(params[:page], limit: params[:limit], search_count: params[:search])
   end
 
   def show
     @pool = Pool.find(params[:id])
     respond_with(@pool) do |format|
       format.html do
-        @posts = @pool.posts.paginate(params[:page], limit: params[:limit], total_count: @pool.post_ids.count)
+        @posts = @pool.posts.paginate_posts(params[:page], limit: params[:limit], total_count: @pool.post_ids.count)
       end
     end
   end

app/controllers/posts_controller.rb

@@ -12,7 +12,7 @@ class PostsController < ApplicationController
         format.html { redirect_to(@post) }
       end
     else
-      @post_set = PostSets::Post.new(tag_query, params[:page], params[:limit], random: params[:random])
+      @post_set = PostSets::Post.new(tag_query, params[:page], limit: params[:limit], random: params[:random])
       @posts = PostsDecorator.decorate_collection(@post_set.posts)
       respond_with(@posts) do |format|
         format.json do

app/logical/danbooru/paginator/base_extension.rb

@@ -19,6 +19,12 @@ module Danbooru
         end
       end
 
+      # Only paginating posts should respect the per_page user setting
+      def paginate_posts(page, options = {})
+        options[:limit] ||= CurrentUser.user.per_page
+        paginate(page, options)
+      end
+
       def total_pages
         if @pagination_mode == :numbered
           if records_per_page > 0
@@ -55,8 +61,8 @@ module Danbooru
       end
 
       def records_per_page
-        limit = @paginator_options.try(:[], :limit) || Danbooru.config.posts_per_page
-        [limit.to_i, 320].min
+        limit = @paginator_options.try(:[], :limit) || Danbooru.config.records_per_page
+        limit.to_i.clamp(0, 320)
       end
 
       # When paginating large tables, we want to avoid doing an expensive count query

app/logical/post_sets/favorites.rb

@@ -4,10 +4,11 @@ module PostSets
   class Favorites < PostSets::Base
     attr_reader :page, :limit
 
-    def initialize(user, page = 1, limit = CurrentUser.per_page)
+    def initialize(user, page, limit:)
+      super()
       @user = user
       @page = page
-      @limit = limit || CurrentUser.per_page
+      @limit = limit
     end
 
     def public_tag_string
@@ -21,7 +22,7 @@ module PostSets
     def posts
       @post_count ||= ::Post.tag_match("fav:#{@user.name} status:any").count_only
       @posts ||= begin
-        favs = ::Favorite.for_user(@user.id).includes(:post).order(created_at: :desc).paginate(page, total_count: @post_count, limit: @limit)
+        favs = ::Favorite.for_user(@user.id).includes(:post).order(created_at: :desc).paginate_posts(page, total_count: @post_count, limit: @limit)
         new_opts = { pagination_mode: :numbered, records_per_page: favs.records_per_page, total_count: @post_count, current_page: current_page }
         ::Danbooru::Paginator::PaginatedArray.new(favs.map(&:post), new_opts)
       end

app/logical/post_sets/popular.rb

@@ -2,17 +2,17 @@
 
 module PostSets
   class Popular < PostSets::Base
-    attr_reader :date, :scale, :limit
+    attr_reader :date, :scale
 
-    def initialize(date, scale, limit: nil)
+    def initialize(date, scale)
+      super()
       @date = date.blank? ? Time.zone.now : Time.zone.parse(date)
       @scale = scale
-      @limit = limit || CurrentUser.per_page
     end
 
     def posts
       @posts ||= begin
-        query = ::Post.where("created_at between ? and ?", min_date.beginning_of_day, max_date.end_of_day).order("score desc").limit(limit)
+        query = ::Post.where("created_at between ? and ?", min_date.beginning_of_day, max_date.end_of_day).order("score desc").paginate_posts(1)
         query.each # hack to force rails to eager load
         query
       end

app/logical/post_sets/post.rb

@@ -2,17 +2,17 @@
 
 module PostSets
   class Post < PostSets::Base
-    MAX_PER_PAGE = 320
-    attr_reader :tag_array, :public_tag_array, :page, :random, :post_count
+    attr_reader :tag_array, :public_tag_array, :page, :limit, :random, :post_count
 
-    def initialize(tags, page = 1, per_page = nil, random: nil)
+    def initialize(tags, page = 1, limit: nil, random: nil)
+      super()
       tags ||= ""
       @public_tag_array = TagQuery.scan(tags)
       tags += " rating:s" if CurrentUser.safe_mode?
       tags += " -status:deleted" unless TagQuery.has_metatag?(tags, "status", "-status")
       @tag_array = TagQuery.scan(tags)
       @page = page
-      @per_page = per_page
+      @limit = limit || TagQuery.fetch_metatag(tag_array, "limit")
       @random = random.present?
     end
 
@@ -48,17 +48,13 @@ module PostSets
       @safe_posts ||= posts.select { |p| p.safeblocked? && !p.deleteblocked? }
     end
 
-    def per_page
-      (@per_page || TagQuery.fetch_metatag(tag_array, "limit") || CurrentUser.user.per_page).to_i.clamp(0, MAX_PER_PAGE)
-    end
-
     def is_random?
       random || (TagQuery.fetch_metatag(tag_array, "order") == "random" && !TagQuery.has_metatag?(tag_array, "randseed"))
     end
 
     def posts
       @posts ||= begin
-        temp = ::Post.tag_match(tag_string).paginate(page, limit: per_page, includes: [:uploader])
+        temp = ::Post.tag_match(tag_string).paginate_posts(page, limit: limit, includes: [:uploader])
 
         @post_count = temp.total_count
         temp

app/models/application_record.rb

@@ -9,6 +9,10 @@ class ApplicationRecord < ActiveRecord::Base
         extending(Danbooru::Paginator::ActiveRecordExtension).paginate(page, options)
       end
 
+      def paginate_posts(page, options = {})
+        extending(Danbooru::Paginator::ActiveRecordExtension).paginate_posts(page, options)
+      end
+
       def qualified_column_for(attr)
         "#{table_name}.#{column_for_attribute(attr).name}"
       end

app/models/forum_post.rb

@@ -205,7 +205,7 @@ class ForumPost < ApplicationRecord
   end
 
   def forum_topic_page
-    ((ForumPost.where("topic_id = ? and created_at <= ?", topic_id, created_at).count) / Danbooru.config.posts_per_page.to_f).ceil
+    (ForumPost.where("topic_id = ? and created_at <= ?", topic_id, created_at).count / Danbooru.config.records_per_page.to_f).ceil
   end
 
   def is_original_post?(original_post_id = nil)

app/models/forum_topic.rb

@@ -179,7 +179,7 @@ class ForumTopic < ApplicationRecord
   end
 
   def last_page
-    (response_count / Danbooru.config.posts_per_page.to_f).ceil
+    (response_count / Danbooru.config.records_per_page.to_f).ceil
   end
 
   def hide!

app/models/post_set.rb

@@ -224,17 +224,6 @@ class PostSet < ApplicationRecord
       end
     end
 
-    def posts(options = {})
-      offset = options[:offset] || 0
-      limit = options[:limit] || Danbooru.config.posts_per_page
-      slice = post_ids.slice(offset, limit)
-      if slice && slice.any?
-        Post.where(id: slice).sort_by {|record| slice.index {|id| id == record.id}}
-      else
-        []
-      end
-    end
-
     def post_count
       post_ids.size
     end

app/models/user.rb

@@ -75,7 +75,6 @@ class User < ApplicationRecord
   validate :validate_ip_addr_is_not_banned, :on => :create
   validate :validate_sock_puppets, :on => :create, :if => -> { Danbooru.config.enable_sock_puppet_validation? }
   before_validation :normalize_blacklisted_tags, if: ->(rec) { rec.blacklisted_tags_changed? }
-  before_validation :set_per_page
   before_validation :staff_cant_disable_dmail
   before_validation :blank_out_nonexistent_avatars
   validates :blacklisted_tags, length: { maximum: 150_000 }
@@ -323,14 +322,6 @@ class User < ApplicationRecord
       can_approve_posts?
     end
 
-    def set_per_page
-      if per_page.nil?
-        self.per_page = Danbooru.config.posts_per_page
-      end
-
-      return true
-    end
-
     def blank_out_nonexistent_avatars
       if avatar_id.present? && avatar.nil?
         self.avatar_id = nil

app/models/wiki_page.rb

@@ -223,7 +223,7 @@ class WikiPage < ApplicationRecord
   end
 
   def post_set
-    @post_set ||= PostSets::Post.new(title, 1, 4)
+    @post_set ||= PostSets::Post.new(title, 1, limit: 4)
   end
 
   def tags

app/views/forum_topics/_listing.html.erb

@@ -21,7 +21,7 @@
 
           <%= link_to topic.title, forum_topic_path(topic), class: "forum-post-link" %>
 
-          <% if topic.response_count > Danbooru.config.posts_per_page %>
+          <% if topic.last_page > 1 %>
             <%= link_to "page #{topic.last_page}", forum_topic_path(topic, :page => topic.last_page), :class => "last-page" %>
           <% end %>
 

app/views/pools/index.html.erb

@@ -19,7 +19,7 @@
             <td>
               <%= link_to pool.pretty_name, pool_path(pool) %>
 
-              <% if pool.post_count > CurrentUser.user.per_page %>
+              <% if pool.last_page > 1 %>
                 <%= link_to "page #{pool.last_page}", pool_path(pool, :page => pool.last_page), :class => "last-page" %>
               <% end %>
             </td>

config/danbooru_default_config.rb

@@ -87,7 +87,7 @@ module Danbooru
       user.comment_threshold = -10
       user.enable_auto_complete = true
       user.enable_keyboard_navigation = true
-      user.per_page = 75
+      user.per_page = records_per_page
       user.show_post_statistics = true
       user.style_usernames = true
     end
@@ -533,9 +533,9 @@ module Danbooru
       "help:replacement_notice"
     end
 
-    # The number of posts displayed per page.
-    def posts_per_page
-      20
+    # The number of records displayed per page. Posts use `user.per_page` which is configurable by the user
+    def records_per_page
+      75
     end
 
     def is_post_restricted?(post)

test/functional/users_controller_test.rb

@@ -80,6 +80,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
         end
         created_user = User.find(session[:user_id])
         assert_equal("xxx", created_user.name)
+        assert_equal(Danbooru.config.records_per_page, created_user.per_page)
         assert_not_nil(created_user.last_ip_addr)
       end
 

test/unit/forum_post_test.rb

@@ -23,7 +23,7 @@ class ForumPostTest < ActiveSupport::TestCase
 
     context "that belongs to a topic with several pages of posts" do
       setup do
-        Danbooru.config.stubs(:posts_per_page).returns(3)
+        Danbooru.config.stubs(:records_per_page).returns(3)
         @posts = []
         9.times do
           @posts << create(:forum_post, topic_id: @topic.id)

test/unit/paginator_test.rb

@@ -10,6 +10,12 @@ class PaginatorTest < ActiveSupport::TestCase
     assert_equal(is_last_page, records.is_last_page?, "is_last_page")
   end
 
+  def assert_invalid_page_number(model, page)
+    assert_raises(Danbooru::Paginator::PaginationError) do
+      model.paginate(page)
+    end
+  end
+
   { active_record: Blip, opensearch: Post }.each do |type, model| # rubocop:disable Metrics/BlockLength
     context type do
       setup do
@@ -58,6 +64,33 @@ class PaginatorTest < ActiveSupport::TestCase
           assert_paginated(expected_records: [@records[0], @records[1]], is_first_page: true, is_last_page: true) { model.paginate("1", limit: 2) }
         end
       end
+
+      should "fail for invalid page numbers" do
+        assert_invalid_page_number(model, -1)
+        assert_invalid_page_number(model, "-1")
+        assert_invalid_page_number(model, "a")
+        assert_invalid_page_number(model, "751")
+        assert_invalid_page_number(model, "c1")
+      end
+
+      should "apply the correct limit" do
+        assert_equal(Danbooru.config.records_per_page, model.paginate(1).records_per_page)
+        assert_equal(10, model.paginate(1, limit: 10).records_per_page)
+        assert_equal(10, model.paginate(1, limit: "10").records_per_page)
+        assert_equal(320, model.paginate(1, limit: "321").records_per_page)
+        assert_equal(0, model.paginate(1, limit: "0").records_per_page)
+        assert_equal(0, model.paginate(1, limit: "-1").records_per_page)
+        assert_equal(0, model.paginate(1, limit: "a").records_per_page)
+      end
+
+      should "apply the correct limit when paginating posts" do
+        assert_equal(@user.per_page, model.paginate_posts(1).records_per_page)
+        assert_equal(10, model.paginate_posts(1, limit: 10).records_per_page)
+
+        @user.update_columns(per_page: 25)
+        assert_equal(25, model.paginate_posts(1).records_per_page)
+        assert_equal(10, model.paginate_posts(1, limit: 10).records_per_page)
+      end
     end
   end
 end

test/unit/post_sets/favorites_test.rb

@@ -20,7 +20,7 @@ module PostSets
       context "a favorite set for before the most recent post" do
         setup do
           id = ::Favorite.where(user_id: @user.id, post_id: @post_3.id).first.id
-          @set = PostSets::Favorites.new(@user, "b#{id}", 1)
+          @set = PostSets::Favorites.new(@user, "b#{id}", limit: 1)
         end
 
         context "a sequential paginator" do
@@ -39,7 +39,7 @@ module PostSets
       context "a favorite set for after the third most recent post" do
         setup do
           id = ::Favorite.where(user_id: @user.id, post_id: @post_2.id).first.id
-          @set = PostSets::Favorites.new(@user, "a#{id}", 1)
+          @set = PostSets::Favorites.new(@user, "a#{id}", limit: 1)
         end
 
         context "a sequential paginator" do
@@ -52,7 +52,7 @@ module PostSets
       context "a favorite set for before the second most recent post" do
         setup do
           id = ::Favorite.where(user_id: @user.id, post_id: @post_1.id).first.id
-          @set = PostSets::Favorites.new(@user, "b#{id}", 1)
+          @set = PostSets::Favorites.new(@user, "b#{id}", limit: 1)
         end
 
         context "a sequential paginator" do
@@ -65,7 +65,7 @@ module PostSets
       context "a favorite set for after the second most recent post" do
         setup do
           id = ::Favorite.where(user_id: @user.id, post_id: @post_1.id).first.id
-          @set = PostSets::Favorites.new(@user, "a#{id}", 1)
+          @set = PostSets::Favorites.new(@user, "a#{id}", limit: 1)
         end
 
         context "a sequential paginator" do
@@ -77,7 +77,7 @@ module PostSets
 
       context "a favorite set for page 2" do
         setup do
-          @set = PostSets::Favorites.new(@user, 2, 1)
+          @set = PostSets::Favorites.new(@user, 2, limit: 1)
         end
 
         context "a numbered paginator" do
@@ -89,7 +89,7 @@ module PostSets
 
       context "a favorite set with no page specified" do
         setup do
-          @set = PostSets::Favorites.new(@user, nil, 1)
+          @set = PostSets::Favorites.new(@user, nil, limit: 1)
         end
 
         should "return the most recent element" do

test/unit/post_sets/post_test.rb

@@ -16,7 +16,7 @@ module PostSets
 
       context "a set for page 2" do
         setup do
-          @set = PostSets::Post.new("", 2, 1)
+          @set = PostSets::Post.new("", 2, limit: 1)
         end
 
         should "return the second element" do
@@ -32,7 +32,7 @@ module PostSets
 
         context "with no page" do
           setup do
-            @set = PostSets::Post.new("a")
+            @set = PostSets::Post.new("a", nil)
           end
 
           should "return the first element" do
@@ -42,7 +42,7 @@ module PostSets
 
         context "for before the first element" do
           setup do
-            @set = PostSets::Post.new("a", "b#{@post_5.id}", 1)
+            @set = PostSets::Post.new("a", "b#{@post_5.id}", limit: 1)
           end
 
           should "return the second element" do
@@ -52,7 +52,7 @@ module PostSets
 
         context "for after the second element" do
           setup do
-            @set = PostSets::Post.new("a", "a#{@post_4.id}", 1)
+            @set = PostSets::Post.new("a", "a#{@post_4.id}", limit: 1)
           end
 
           should "return the first element" do
@@ -61,28 +61,16 @@ module PostSets
         end
       end
 
-      context "a set going to the 1,001st page" do
-        setup do
-          @set = PostSets::Post.new("a", 1_001)
-        end
+      context "#limit method" do
+        should "take the limit from the params first, then the limit:<n> metatag" do
+          set = PostSets::Post.new("a limit:23 b", 1, limit: "42")
+          assert_equal("42", set.limit)
 
-        should "fail" do
-          assert_raises(Danbooru::Paginator::PaginationError) do
-            @set.posts
-          end
-        end
-      end
+          set = PostSets::Post.new("a limit:23 b", 1)
+          assert_equal("23", set.limit)
 
-      context "#per_page method" do
-        should "take the limit from the params first, then the limit:<n> metatag, then the account settings" do
-          set = PostSets::Post.new("a limit:23 b", 1, 42)
-          assert_equal(42, set.per_page)
-
-          set = PostSets::Post.new("a limit:23 b", 1, nil)
-          assert_equal(23, set.per_page)
-
-          set = PostSets::Post.new("a", 1, nil)
-          assert_equal(CurrentUser.user.per_page, set.per_page)
+          set = PostSets::Post.new("a", 1)
+          assert_nil(set.limit)
         end
       end
     end