Skip to content

Commit

Permalink
Fix N+1 queries in moderation and fix preload_first_topic_post
Browse files Browse the repository at this point in the history
  • Loading branch information
glebm committed Apr 27, 2019
1 parent a48a472 commit f1874fb
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 5 deletions.
2 changes: 2 additions & 0 deletions app/controllers/thredded/moderation_controller.rb
Expand Up @@ -19,6 +19,8 @@ def history
@post_moderation_records = accessible_post_moderation_records
.order(created_at: :desc)
.send(Kaminari.config.page_method_name, current_page)
.preload(:messageboard, :post_user, :moderator, post: :postable)
.preload_first_topic_post
end

def activity
Expand Down
10 changes: 8 additions & 2 deletions app/models/concerns/thredded/post_common.rb
Expand Up @@ -21,13 +21,19 @@ module PostCommon
scope :preload_first_topic_post, -> {
posts_table_name = quoted_table_name
result = all
ActiveRecord::Associations::Preloader.new.preload(
result.map(&:postable), :first_post,
owners_by_id = result.each_with_object({}) { |r, h| h[r.postable_id] = r.postable }
preloader = ActiveRecord::Associations::Preloader.new.preload(
owners_by_id.values, :first_post,
unscoped.where(<<~SQL.delete("\n"))
#{posts_table_name}.created_at = (
SELECT MAX(p2.created_at) from #{posts_table_name} p2 WHERE p2.postable_id = #{posts_table_name}.postable_id)
SQL
)
preloader[0].preloaded_records.each do |post|

This comment has been minimized.

Copy link
@igorkasyanchuk

igorkasyanchuk Jun 5, 2019

I've got a crash here http://prntscr.com/ny1x91
for this URL /forum/admin/moderation

This comment has been minimized.

Copy link
@glebm

glebm Jun 5, 2019

Author Collaborator

Can you reproduce it with a test here?

This comment has been minimized.

Copy link
@igorkasyanchuk

igorkasyanchuk Jun 5, 2019

here is it: #817 (it has another fix inside, basically I've created another PR which is fixing another crash)

The test is failed and here some details: http://prntscr.com/ny30k8

This comment has been minimized.

Copy link
@glebm

glebm Jun 6, 2019

Author Collaborator

Thanks! Fixed in c691be6

This comment has been minimized.

Copy link
@igorkasyanchuk

igorkasyanchuk Jun 6, 2019

@glebm check this PR too #813 it has another fix

topic = owners_by_id.delete(post.postable_id)
next unless topic
topic.association(:first_post).target = post
end
result
}

Expand Down
19 changes: 19 additions & 0 deletions app/models/thredded/post_moderation_record.rb
Expand Up @@ -27,6 +27,25 @@ class PostModerationRecord < ActiveRecord::Base
record.errors.add attr, "Post moderation_state is already #{value}" if record.previous_moderation_state == value
end

scope :preload_first_topic_post, -> {
posts_table_name = Thredded::Post.quoted_table_name
result = all
owners_by_id = result.each_with_object({}) { |r, h| h[r.post.postable_id] = r.post.postable }
preloader = ActiveRecord::Associations::Preloader.new.preload(
owners_by_id.values, :first_post,
Thredded::Post.unscoped.where(<<~SQL.delete("\n"))
#{posts_table_name}.created_at = (
SELECT MAX(p2.created_at) from #{posts_table_name} p2 WHERE p2.postable_id = #{posts_table_name}.postable_id)
SQL
)
preloader[0].preloaded_records.each do |post|
topic = owners_by_id.delete(post.postable_id)
next unless topic
topic.association(:first_post).target = post
end
result
}

paginates_per Thredded.posts_per_page

# @return [ActiveRecord::Relation<Thredded.user_class>] users that can read the moderated post.
Expand Down
Expand Up @@ -38,7 +38,8 @@
<% end %>
<% end %>
<% if post %>
<%= render 'thredded/posts_common/header_with_user_and_topic', post: post_view, post_user_link: post_user_link %>
<%= render 'thredded/posts_common/header_with_user_and_topic',
post: post_view, user: record.post_user, post_user_link: post_user_link %>
<% else %>
<header><h2 class="thredded--post--user"><%= post_user_link %></h2></header>
<% end %>
Expand Down
@@ -1,9 +1,11 @@
<%# @param post [Thredded::PostView] %>
<%# @param user [Thredded.user_class] optional %>
<%# @param post_user_link [String] optional %>
<% topic = post.to_model.postable %>
<% post_user_link ||= user_link(post.user) %>
<% user ||= post.user %>
<% post_user_link ||= user_link(user) %>
<header>
<%= image_tag post.avatar_url, class: 'thredded--post--avatar' if post.user %>
<%= image_tag Thredded.avatar_url.call(user), class: 'thredded--post--avatar' if user %>
<h2 class="thredded--post--user-and-topic">
<%=
topic_link = link_to(topic.title, post.permalink_path)
Expand Down

0 comments on commit f1874fb

Please sign in to comment.