FIX: N+1 query for chat message serializer on mentions ()

Followup to d4a5b79592a79201076fb02845d2da7db1df1646,
this introduced an N1 because every message in the list
we had to query users for the mentions and then the user's
status too. Instead we can just include both in Chat::MessagesQuery.
This commit is contained in:
Martin Brennan 2023-05-26 12:44:03 +02:00 committed by GitHub
parent b7229953f7
commit 594636183d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 11 additions and 6 deletions
plugins/chat
app
queries/chat
serializers/chat
spec/models/chat

@ -76,10 +76,14 @@ module Chat
.includes(:uploads) .includes(:uploads)
.includes(chat_channel: :chatable) .includes(chat_channel: :chatable)
.includes(:thread) .includes(:thread)
.includes(:chat_mentions)
.where(chat_channel_id: channel.id) .where(chat_channel_id: channel.id)
query = query.includes(user: :user_status) if SiteSetting.enable_user_status if SiteSetting.enable_user_status
query = query.includes(user: :user_status)
query = query.includes(chat_mentions: { user: :user_status })
else
query = query.includes(chat_mentions: :user)
end
query query
end end

@ -27,9 +27,10 @@ module Chat
has_many :uploads, serializer: ::UploadSerializer, embed: :objects has_many :uploads, serializer: ::UploadSerializer, embed: :objects
def mentioned_users def mentioned_users
User object
.where(id: object.chat_mentions.pluck(:user_id)) .chat_mentions
.order("users.id ASC") .map(&:user)
.sort_by(&:id)
.map { |user| BasicUserWithStatusSerializer.new(user, root: false) } .map { |user| BasicUserWithStatusSerializer.new(user, root: false) }
.as_json .as_json
end end

@ -451,7 +451,7 @@ describe Chat::Message do
notification = Fabricate(:notification) notification = Fabricate(:notification)
mention_1 = Fabricate(:chat_mention, chat_message: message_1, notification: notification) mention_1 = Fabricate(:chat_mention, chat_message: message_1, notification: notification)
message_1.destroy! message_1.reload.destroy!
expect { mention_1.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { mention_1.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound)