diff --git a/app/controllers/modqueue_controller.rb b/app/controllers/modqueue_controller.rb index d10728483..1255a1dac 100644 --- a/app/controllers/modqueue_controller.rb +++ b/app/controllers/modqueue_controller.rb @@ -6,7 +6,7 @@ class ModqueueController < ApplicationController def index authorize :modqueue - @posts = Post.includes(:appeals, :disapprovals, :uploader, :media_asset, flags: [:creator]).in_modqueue.available_for_moderation(CurrentUser.user, hidden: search_params[:hidden]) + @posts = Post.includes(:appeals, :disapprovals, :uploader, :media_asset, flags: [:creator]).available_for_moderation(CurrentUser.user, hidden: search_params[:hidden]) @modqueue_posts = @posts.reselect(nil).reorder(nil).offset(nil).limit(nil) @posts = @posts.paginated_search(params, count_pages: true, count: @modqueue_posts.to_a.size, defaults: { order: "modqueue" }) diff --git a/app/logical/concerns/searchable.rb b/app/logical/concerns/searchable.rb index f5c3110aa..6f939c387 100644 --- a/app/logical/concerns/searchable.rb +++ b/app/logical/concerns/searchable.rb @@ -135,6 +135,17 @@ module Searchable where_numeric_matches(qualified_column, value) end + # where_union(A, B, C) is like `WHERE A OR B OR C`, except it may be faster if the conditions are disjoint. + # where_union(A, B) does `SELECT * FROM table WHERE id IN (SELECT id FROM table WHERE A UNION ALL SELECT id FROM table WHERE B)` + def where_union(*relations, primary_key: :id, foreign_key: :id) + arels = relations.map { |relation| relation.select(foreign_key).arel } + union = arels.reduce do |left, right| + Arel::Nodes::UnionAll.new(left, right) + end + + where(arel_table[primary_key].in(union)) + end + # @param attr [String] the name of the JSON field # @param hash [Hash] the hash of values it should contain def where_json_contains(attr, hash) diff --git a/app/models/post.rb b/app/models/post.rb index 7d40a8f6e..25488c84c 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -80,8 +80,8 @@ class Post < ApplicationRecord scope :banned, -> { where(is_banned: true) } # XXX conflict with deletable scope :active, -> { where(is_pending: false, is_deleted: false, is_flagged: false).where.not(id: PostAppeal.pending) } - scope :appealed, -> { deleted.where(id: PostAppeal.pending.select(:post_id)) } - scope :in_modqueue, -> { pending.or(flagged).or(appealed) } + scope :appealed, -> { where(id: PostAppeal.pending.select(:post_id)) } + scope :in_modqueue, -> { where_union(pending, flagged, appealed) } scope :expired, -> { pending.where("posts.created_at < ?", Danbooru.config.moderation_period.ago) } scope :unflagged, -> { where(is_flagged: false) } @@ -1049,15 +1049,12 @@ class Post < ApplicationRecord end def available_for_moderation(user, hidden: false) - return none if user.is_anonymous? - - approved_posts = user.post_approvals.select(:post_id) disapproved_posts = user.post_disapprovals.select(:post_id) if hidden.present? - where("posts.uploader_id = ? OR posts.id IN (#{approved_posts.to_sql}) OR posts.id IN (#{disapproved_posts.to_sql})", user.id) + in_modqueue.where(id: disapproved_posts) else - where.not(uploader: user).where.not(id: approved_posts).where.not(id: disapproved_posts) + in_modqueue.where.not(id: disapproved_posts) end end @@ -1124,7 +1121,7 @@ class Post < ApplicationRecord when "active" active when "unmoderated" - in_modqueue.available_for_moderation(current_user, hidden: false) + available_for_moderation(current_user, hidden: false) when "all", "any" where("TRUE") else diff --git a/test/unit/post_disapproval_test.rb b/test/unit/post_disapproval_test.rb index 9ba87fbae..402945b73 100644 --- a/test/unit/post_disapproval_test.rb +++ b/test/unit/post_disapproval_test.rb @@ -3,7 +3,7 @@ require "test_helper" class PostDisapprovalTest < ActiveSupport::TestCase context "In all cases" do setup do - @alice = FactoryBot.create(:moderator_user, name: "alice") + @alice = FactoryBot.create(:moderator_user) CurrentUser.user = @alice end @@ -28,25 +28,18 @@ class PostDisapprovalTest < ActiveSupport::TestCase end context "when the current user is alice" do - setup do - CurrentUser.user = @alice - end - should "remove the associated post from alice's moderation queue" do - assert_not(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_1.id)) - assert(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_2.id)) + assert_not(Post.available_for_moderation(@alice, hidden: false).map(&:id).include?(@post_1.id)) + assert(Post.available_for_moderation(@alice, hidden: false).map(&:id).include?(@post_2.id)) end end - context "when the current user is brittony" do - setup do - @brittony = FactoryBot.create(:moderator_user) - CurrentUser.user = @brittony - end + context "when the current user is not the disapprover" do + should "not remove the associated post from the disapprover's moderation queue" do + @mod = create(:moderator_user) - should "not remove the associated post from brittony's moderation queue" do - assert(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_1.id)) - assert(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_2.id)) + assert(Post.available_for_moderation(@mod, hidden: false).map(&:id).include?(@post_1.id)) + assert(Post.available_for_moderation(@mod, hidden: false).map(&:id).include?(@post_2.id)) end end end