PERF: Make `TopicViewSerializer#requested_group_name` more efficient. (#14196)

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.
diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb
index 141fc7f..6af55ae 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 70ceb93..cc4eabb 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

GitHub sha: c2f87e0a368111924b0667bb11e919f2ee2ac3f3

This commit appears in #14196 which was approved by martin. It was merged by tgxworld.