From 83915a5d468a53889f4bfd41a0f88aa56340b169 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 24 Feb 2017 19:13:02 -0600 Subject: [PATCH] modqueue: avoid N+1 queries for disapprovals, uploaders. Fixes an N+1 queries problem in the /moderator/post/queue view by prefetching disapprovals and uploaders. Also the way disapproval messages were previously rendered triggered a bunch of sql queries for each post: SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 [["post_id", 52]] SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND "post_disapprovals"."reason" = $2 [["post_id", 52], ["reason", "breaks_rules"]] SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND "post_disapprovals"."reason" = $2 [["post_id", 52], ["reason", "poor_quality"]] SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND "post_disapprovals"."reason" IN ('disinterest', 'legacy') [["post_id", 52]] SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND (message is not null and message <> '') [["post_id", 52]] SELECT "post_disapprovals".* FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND (message is not null and message <> '') [["post_id", 52]] This refactors to bring it down to one: SELECT "post_disapprovals".* FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 [["post_id", 52]] --- .../moderator/post/queues_controller.rb | 4 ++-- .../post_disapprovals/_compact_counts.html.erb | 16 ++++++++-------- app/views/post_disapprovals/_counts.html.erb | 18 +++++++++--------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/controllers/moderator/post/queues_controller.rb b/app/controllers/moderator/post/queues_controller.rb index 06b80dd18..e274c19fb 100644 --- a/app/controllers/moderator/post/queues_controller.rb +++ b/app/controllers/moderator/post/queues_controller.rb @@ -15,7 +15,7 @@ module Moderator end ::Post.without_timeout do - @posts = ::Post.order("posts.id asc").pending_or_flagged.available_for_moderation(params[:hidden]).tag_match(params[:query]).paginate(params[:page], :limit => per_page) + @posts = ::Post.includes(:disapprovals, :uploader).order("posts.id asc").pending_or_flagged.available_for_moderation(params[:hidden]).tag_match(params[:query]).paginate(params[:page], :limit => per_page) @posts.each # hack to force rails to eager load end respond_with(@posts) @@ -25,7 +25,7 @@ module Moderator cookies.permanent[:moderated] = Time.now.to_i ::Post.without_timeout do - @posts = ::Post.order("posts.id asc").pending_or_flagged.available_for_moderation(false).reorder("random()").limit(RANDOM_COUNT) + @posts = ::Post.includes(:disapprovals, :uploader).order("posts.id asc").pending_or_flagged.available_for_moderation(false).reorder("random()").limit(RANDOM_COUNT) @posts.each # hack to force rails to eager load if @posts.empty? diff --git a/app/views/post_disapprovals/_compact_counts.html.erb b/app/views/post_disapprovals/_compact_counts.html.erb index 9e08a9745..78ced11bc 100644 --- a/app/views/post_disapprovals/_compact_counts.html.erb +++ b/app/views/post_disapprovals/_compact_counts.html.erb @@ -1,17 +1,17 @@ -<% if disapprovals.count > 0 && (CurrentUser.can_approve_posts? || post.created_at < 3.days.ago) %> - <% if disapprovals.breaks_rules.count > 0 %> - (breaks rules: <%= disapprovals.breaks_rules.count %>) +<% if CurrentUser.can_approve_posts? || post.created_at < 3.days.ago %> + <% if disapprovals.map(&:reason).grep("breaks_rules").count > 0 %> + (breaks rules: <%= disapprovals.map(&:reason).grep("breaks_rules").count %>) <% end %> - <% if disapprovals.poor_quality.count > 0 %> - (poor quality: <%= disapprovals.poor_quality.count %>) + <% if disapprovals.map(&:reason).grep("poor_quality").count > 0 %> + (poor quality: <%= disapprovals.map(&:reason).grep("poor_quality").count %>) <% end %> - <% if disapprovals.disinterest.count > 0 %> - (no interest: <%= disapprovals.disinterest.count %>) + <% if disapprovals.map(&:reason).grep(/disinterest|legacy/).count > 0 %> + (no interest: <%= disapprovals.map(&:reason).grep(/disinterest|legacy/).count %>) <% end %> - <% if disapprovals.with_message.any? %> + <% if disapprovals.map(&:message).any?(&:present?) %> (messages: <%= disapprovals.map(&:message).select(&:present?).map { |msg| format_text(msg, ragel: true, inline: true) }.to_sentence.html_safe %>) <% end %> <% end %> diff --git a/app/views/post_disapprovals/_counts.html.erb b/app/views/post_disapprovals/_counts.html.erb index 612e4f7e7..f858c96af 100644 --- a/app/views/post_disapprovals/_counts.html.erb +++ b/app/views/post_disapprovals/_counts.html.erb @@ -1,20 +1,20 @@ -<% if disapprovals.count > 0 && (CurrentUser.can_approve_posts? || post.created_at < 3.days.ago) %> +<% if CurrentUser.can_approve_posts? || post.created_at < 3.days.ago %>

- It has been reviewed by <%= pluralize disapprovals.count, "moderator" %>. + It has been reviewed by <%= pluralize disapprovals.length, "moderator" %>. - <% if disapprovals.breaks_rules.count > 0 %> - <%= disapprovals.breaks_rules.count %> believe it breaks the rules. + <% if disapprovals.map(&:reason).grep("breaks_rules").count > 0 %> + <%= disapprovals.map(&:reason).grep("breaks_rules").count %> believe it breaks the rules. <% end %> - <% if disapprovals.poor_quality.count > 0 %> - <%= disapprovals.poor_quality.count %> believe it has poor quality. + <% if disapprovals.map(&:reason).grep("poor_quality").count > 0 %> + <%= disapprovals.map(&:reason).grep("poor_quality").count %> believe it has poor quality. <% end %> - <% if disapprovals.disinterest.count > 0 %> - <%= disapprovals.disinterest.count %> did not like the post enough to approve it. + <% if disapprovals.map(&:reason).grep(/disinterest|legacy/).count > 0 %> + <%= disapprovals.map(&:reason).grep(/disinterest|legacy/).count %> did not like the post enough to approve it. <% end %> - <% if disapprovals.with_message.any? %> + <% if disapprovals.map(&:message).any?(&:present?) %> Messages: <%= disapprovals.map(&:message).select(&:present?).map { |msg| format_text(msg, ragel: true, inline: true) }.to_sentence.html_safe %>. <% end %>