From c2f87e0a368111924b0667bb11e919f2ee2ac3f3 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Tue, 31 Aug 2021 12:05:32 +0800
Subject: [PATCH] PERF: Make `TopicViewSerializer#requested_group_name` more
efficient. (#14196)
* Avoid executing a query when the custom field doesn't exist
* Avoid generating an ActiveRecord when all we need is the name.
---
app/serializers/topic_view_serializer.rb | 19 +++++++---------
.../serializers/topic_view_serializer_spec.rb | 22 +++++++++++++++++++
2 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb
index 141fc7fc2cd..6af55aefe78 100644
--- a/app/serializers/topic_view_serializer.rb
+++ b/app/serializers/topic_view_serializer.rb
@@ -260,20 +260,17 @@ class TopicViewSerializer < ApplicationSerializer
end
def requested_group_name
- if scope&.user
- group = Group
- .joins('JOIN group_users ON groups.id = group_users.group_id')
- .find_by(
- id: object.topic.custom_fields['requested_group_id'].to_i,
- group_users: { user_id: scope.user.id, owner: true }
- )
-
- group.name if group
- end
+ Group
+ .joins(:group_users)
+ .where(
+ id: object.topic.custom_fields['requested_group_id'].to_i,
+ group_users: { user_id: scope.user.id, owner: true }
+ )
+ .pluck_first(:name)
end
def include_requested_group_name?
- object.personal_message
+ object.personal_message && object.topic.custom_fields['requested_group_id']
end
def include_published_page?
diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb
index 70ceb935c11..cc4eabbf2fa 100644
--- a/spec/serializers/topic_view_serializer_spec.rb
+++ b/spec/serializers/topic_view_serializer_spec.rb
@@ -484,4 +484,26 @@ describe TopicViewSerializer do
end
end
end
+
+ describe '#requested_group_name' do
+ fab!(:pm) { Fabricate(:private_message_post).topic }
+ fab!(:group) { Fabricate(:group) }
+
+ it 'should return the right group name when PM is a group membership request' do
+ pm.custom_fields[:requested_group_id] = group.id
+ pm.save!
+
+ user = pm.first_post.user
+ group.add_owner(user)
+ json = serialize_topic(pm, user)
+
+ expect(json[:requested_group_name]).to eq(group.name)
+ end
+
+ it 'should not include the attribute for a non group membership request PM' do
+ json = serialize_topic(pm, pm.first_post.user)
+
+ expect(json[:requested_group_name]).to eq(nil)
+ end
+ end
end