diff --git a/app/controllers/artists_controller.rb b/app/controllers/artists_controller.rb index 16f1571a1..5b60fc6e8 100644 --- a/app/controllers/artists_controller.rb +++ b/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 diff --git a/app/controllers/favorites_controller.rb b/app/controllers/favorites_controller.rb index 510350a46..739ccdbee 100644 --- a/app/controllers/favorites_controller.rb +++ b/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' diff --git a/app/controllers/mascots_controller.rb b/app/controllers/mascots_controller.rb index b178f0648..b1bb6b764 100644 --- a/app/controllers/mascots_controller.rb +++ b/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 diff --git a/app/controllers/pools_controller.rb b/app/controllers/pools_controller.rb index 87d544743..d3dc891e7 100644 --- a/app/controllers/pools_controller.rb +++ b/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 diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 5d04dd0f6..c09c81b76 100644 --- a/app/controllers/posts_controller.rb +++ b/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 diff --git a/app/logical/danbooru/paginator/base_extension.rb b/app/logical/danbooru/paginator/base_extension.rb index 796cca671..d1d35fad2 100644 --- a/app/logical/danbooru/paginator/base_extension.rb +++ b/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 diff --git a/app/logical/post_sets/favorites.rb b/app/logical/post_sets/favorites.rb index 4e5d7b66d..61f2880e0 100644 --- a/app/logical/post_sets/favorites.rb +++ b/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 diff --git a/app/logical/post_sets/popular.rb b/app/logical/post_sets/popular.rb index acac363fa..1ad3c03a2 100644 --- a/app/logical/post_sets/popular.rb +++ b/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 diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index 0b7211d2b..3162fb68b 100644 --- a/app/logical/post_sets/post.rb +++ b/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 diff --git a/app/models/application_record.rb b/app/models/application_record.rb index fee98b356..091dd5f5c 100644 --- a/app/models/application_record.rb +++ b/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 diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index cf0d27a47..1372e9a29 100644 --- a/app/models/forum_post.rb +++ b/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) diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index ccbd90985..504530c42 100644 --- a/app/models/forum_topic.rb +++ b/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! diff --git a/app/models/post_set.rb b/app/models/post_set.rb index 67f10f4ec..abcd7e13e 100644 --- a/app/models/post_set.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index bbce50210..5a317a028 100644 --- a/app/models/user.rb +++ b/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 diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index f56017797..cd48e02cf 100644 --- a/app/models/wiki_page.rb +++ b/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 diff --git a/app/views/forum_topics/_listing.html.erb b/app/views/forum_topics/_listing.html.erb index cabf07b69..635ce3fb4 100644 --- a/app/views/forum_topics/_listing.html.erb +++ b/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 %> diff --git a/app/views/pools/index.html.erb b/app/views/pools/index.html.erb index b0990bb82..dfbb28f9a 100644 --- a/app/views/pools/index.html.erb +++ b/app/views/pools/index.html.erb @@ -19,7 +19,7 @@ <%= 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 %> diff --git a/config/danbooru_default_config.rb b/config/danbooru_default_config.rb index 62654c09e..92a26e998 100644 --- a/config/danbooru_default_config.rb +++ b/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) diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 232f836cb..bd9a259d6 100644 --- a/test/functional/users_controller_test.rb +++ b/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 diff --git a/test/unit/forum_post_test.rb b/test/unit/forum_post_test.rb index 2099920d2..aab10c508 100644 --- a/test/unit/forum_post_test.rb +++ b/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) diff --git a/test/unit/paginator_test.rb b/test/unit/paginator_test.rb index 437505958..c2df6ba5c 100644 --- a/test/unit/paginator_test.rb +++ b/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 diff --git a/test/unit/post_sets/favorites_test.rb b/test/unit/post_sets/favorites_test.rb index d9dec38dd..b87bfb5c2 100644 --- a/test/unit/post_sets/favorites_test.rb +++ b/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 diff --git a/test/unit/post_sets/post_test.rb b/test/unit/post_sets/post_test.rb index af14f63c2..d446b5974 100644 --- a/test/unit/post_sets/post_test.rb +++ b/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: 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: 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