From 980103e4431f4f1718bf9db003f4bce4eff7d26b Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 29 Feb 2020 12:52:07 -0600 Subject: [PATCH] modqueue: optimize sql queries. * Include appeals and flags. * Avoid an existence query for pools. * Avoid a query checking if the user has previously approved the post. This is a rare condition and it will be prevented anyway if the user tries to reapprove the post. --- app/controllers/moderator/post/queues_controller.rb | 5 +---- app/models/post.rb | 8 ++------ app/models/post_approval.rb | 2 +- test/unit/post_approval_test.rb | 4 ---- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/app/controllers/moderator/post/queues_controller.rb b/app/controllers/moderator/post/queues_controller.rb index 6b3f9364a..c98ee93dc 100644 --- a/app/controllers/moderator/post/queues_controller.rb +++ b/app/controllers/moderator/post/queues_controller.rb @@ -10,10 +10,7 @@ module Moderator cookies.permanent["mq_per_page"] = search_params[:per_page] end - ::Post.without_timeout do - @posts = ::Post.includes(:disapprovals, :uploader).order("posts.id asc").pending_or_flagged.available_for_moderation(search_params[:hidden]).tag_match(search_params[:tags]).paginate(params[:page], :limit => per_page) - @posts.each # hack to force rails to eager load - end + @posts = ::Post.includes(:appeals, :disapprovals, :uploader, flags: [:creator]).reorder(id: :asc).pending_or_flagged.available_for_moderation(search_params[:hidden]).tag_match(search_params[:tags]).paginate(params[:page], :limit => per_page) respond_with(@posts) end diff --git a/app/models/post.rb b/app/models/post.rb index 9e18f67d7..dbb1e828e 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -273,7 +273,7 @@ class Post < ApplicationRecord module ApprovalMethods def is_approvable?(user = CurrentUser.user) - !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && uploader != user && !approved_by?(user) + !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && uploader != user end def flag!(reason, is_deletion: false) @@ -288,10 +288,6 @@ class Post < ApplicationRecord approvals.create(user: approver) end - def approved_by?(user) - approver == user || approvals.where(user: user).exists? - end - def disapproved_by?(user) PostDisapproval.where(:user_id => user.id, :post_id => id).exists? end @@ -989,7 +985,7 @@ class Post < ApplicationRecord end def has_active_pools? - !pools.undeleted.empty? + pools.undeleted.present? end def belongs_to_pool?(pool) diff --git a/app/models/post_approval.rb b/app/models/post_approval.rb index e5764115d..2981143d2 100644 --- a/app/models/post_approval.rb +++ b/app/models/post_approval.rb @@ -20,7 +20,7 @@ class PostApproval < ApplicationRecord errors.add(:base, "You cannot approve a post you uploaded") end - if post.approved_by?(user) + if post.approver == user || post.approvals.where(user: user).exists? errors.add(:base, "You have previously approved this post and cannot approve it again") end end diff --git a/test/unit/post_approval_test.rb b/test/unit/post_approval_test.rb index 765ebb31e..570ff2754 100644 --- a/test/unit/post_approval_test.rb +++ b/test/unit/post_approval_test.rb @@ -22,10 +22,6 @@ class PostApprovalTest < ActiveSupport::TestCase CurrentUser.ip_addr = nil end - should "allow approval" do - assert_equal(false, @post.approved_by?(@approver)) - end - context "That is approved" do should "create a postapproval record" do assert_difference("PostApproval.count") do