mirror of
https://github.com/discourse/discourse.git
synced 2025-02-25 18:55:32 -06:00
PERF: optimize lookup of reviewable info in post stream
This previously was a hot path in topic view. Avoids an expensive active record operation and instead perform SQL directly which is far more targeted and efficient
This commit is contained in:
@@ -421,17 +421,32 @@ class TopicView
|
|||||||
def reviewable_counts
|
def reviewable_counts
|
||||||
if @reviewable_counts.blank?
|
if @reviewable_counts.blank?
|
||||||
|
|
||||||
# Create a hash with counts by post so we can quickly look up whether there is reviewable content.
|
post_ids = @posts.map(&:id)
|
||||||
@reviewable_counts = {}
|
|
||||||
Reviewable.
|
|
||||||
where(target_type: 'Post', target_id: @posts.map(&:id)).
|
|
||||||
includes(:reviewable_scores).each do |r|
|
|
||||||
|
|
||||||
for_post = (@reviewable_counts[r.target_id] ||= { total: 0, pending: 0, reviewable_id: r.id })
|
sql = <<~SQL
|
||||||
r.reviewable_scores.each do |s|
|
SELECT target_id,
|
||||||
for_post[:total] += 1
|
MAX(r.id) reviewable_id,
|
||||||
for_post[:pending] += 1 if s.pending?
|
COUNT(*) total,
|
||||||
end
|
SUM(CASE WHEN s.status = :pending THEN 1 ELSE 0 END) pending
|
||||||
|
FROM reviewables r
|
||||||
|
JOIN reviewable_scores s ON reviewable_id = r.id
|
||||||
|
WHERE r.target_id IN (:post_ids) AND
|
||||||
|
r.target_type = 'Post'
|
||||||
|
GROUP BY target_id
|
||||||
|
SQL
|
||||||
|
|
||||||
|
@reviewable_counts = {}
|
||||||
|
|
||||||
|
DB.query(
|
||||||
|
sql,
|
||||||
|
pending: ReviewableScore.statuses[:pending],
|
||||||
|
post_ids: post_ids
|
||||||
|
).each do |row|
|
||||||
|
@reviewable_counts[row.target_id] = {
|
||||||
|
total: row.total,
|
||||||
|
pending: row.pending,
|
||||||
|
reviewable_id: row.reviewable_id
|
||||||
|
}
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -2,28 +2,51 @@
|
|||||||
|
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
|
|
||||||
RSpec.describe TopicViewPostsSerializer do
|
describe TopicViewPostsSerializer do
|
||||||
fab!(:user) { Fabricate(:user) }
|
|
||||||
fab!(:post) { Fabricate(:post) }
|
|
||||||
let(:topic) { post.topic }
|
|
||||||
let(:topic_view) { TopicView.new(topic, user, post_ids: [post.id]) }
|
|
||||||
|
|
||||||
subject do
|
it 'should return the right attributes' do
|
||||||
described_class.new(topic_view,
|
|
||||||
|
user = Fabricate(:user)
|
||||||
|
post = Fabricate(:post)
|
||||||
|
topic = post.topic
|
||||||
|
|
||||||
|
reviewable = Fabricate(:reviewable_flagged_post, created_by: user, target: post, topic: topic)
|
||||||
|
|
||||||
|
ReviewableScore.create!(
|
||||||
|
reviewable_id: reviewable.id,
|
||||||
|
user_id: user.id,
|
||||||
|
reviewable_score_type: 0,
|
||||||
|
status: ReviewableScore.statuses[:pending]
|
||||||
|
)
|
||||||
|
|
||||||
|
ReviewableScore.create!(
|
||||||
|
reviewable_id: reviewable.id,
|
||||||
|
user_id: user.id,
|
||||||
|
reviewable_score_type: 0,
|
||||||
|
status: ReviewableScore.statuses[:ignored]
|
||||||
|
)
|
||||||
|
|
||||||
|
topic_view = TopicView.new(topic, user, post_ids: [post.id])
|
||||||
|
|
||||||
|
serializer = TopicViewPostsSerializer.new(
|
||||||
|
topic_view,
|
||||||
scope: Guardian.new(Fabricate(:admin)),
|
scope: Guardian.new(Fabricate(:admin)),
|
||||||
root: false
|
root: false
|
||||||
)
|
)
|
||||||
end
|
|
||||||
|
|
||||||
it 'should return the right attributes' do
|
body = JSON.parse(serializer.to_json)
|
||||||
body = JSON.parse(subject.to_json)
|
|
||||||
|
|
||||||
posts = body["post_stream"]["posts"]
|
posts = body["post_stream"]["posts"]
|
||||||
|
|
||||||
expect(posts.count).to eq(1)
|
expect(posts.count).to eq(1)
|
||||||
expect(posts.first["id"]).to eq(post.id)
|
expect(posts.first["id"]).to eq(post.id)
|
||||||
|
|
||||||
|
expect(posts.first["reviewable_score_count"]).to eq(2)
|
||||||
|
expect(posts.first["reviewable_score_pending_count"]).to eq(1)
|
||||||
|
|
||||||
expect(body["post_stream"]["stream"]).to eq(nil)
|
expect(body["post_stream"]["stream"]).to eq(nil)
|
||||||
expect(body["post_stream"]["timeline_lookup"]).to eq(nil)
|
expect(body["post_stream"]["timeline_lookup"]).to eq(nil)
|
||||||
expect(body["post_stream"]["gaps"]).to eq(nil)
|
expect(body["post_stream"]["gaps"]).to eq(nil)
|
||||||
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user