PERF: avoid publishing user actions to the user who did the action (#26225)

We never use that information and this also fixes an issue with the BCC plugin which ends up triggering a rate-limit because we were publishing a "NEW_PRIVATE_MESSAGE" to the user sending the BCC for every recipients 💥

Internal - t/118283

app/models/private_message_topic_tracking_state.rb

@@ -127,6 +127,8 @@ class PrivateMessageTopicTrackingState
     scope
       .select(%i[user_id last_read_post_number notification_level])
       .each do |tu|
+        next if tu.user_id == post.user_id # skip post creator
+
         if tu.last_read_post_number.nil? &&
              topic.created_at < tu.user.user_option.treat_as_new_topic_start_date
           next
@@ -166,6 +168,7 @@ class PrivateMessageTopicTrackingState
       .allowed_users
       .pluck(:id)
       .each do |user_id|
+        next if user_id == topic.user_id # skip topic creator
         MessageBus.publish(self.user_channel(user_id), message, user_ids: [user_id])
       end
 

app/models/user_action.rb

@@ -264,10 +264,7 @@ class UserAction < ActiveRecord::Base
   end
 
   def self.log_action!(hash)
-    required_parameters = %i[action_type user_id acting_user_id]
+    required_parameters = %i[action_type user_id acting_user_id target_post_id target_topic_id]
-
-    required_parameters << :target_post_id
-    required_parameters << :target_topic_id
 
     require_parameters(hash, *required_parameters)
 
@@ -290,17 +287,18 @@ class UserAction < ActiveRecord::Base
 
         update_like_count(user_id, hash[:action_type], 1) if topic && !topic.private_message?
 
+        user_ids = user_id != action.acting_user_id ? [user_id] : nil
+
         group_ids = nil
-        if topic && topic.category && topic.category.read_restricted
+        if topic&.category&.read_restricted
-          group_ids = [Group::AUTO_GROUPS[:admins]]
+          group_ids = [Group::AUTO_GROUPS[:admins]] | topic.category.groups.pluck("groups.id")
-          group_ids.concat(topic.category.groups.pluck("groups.id"))
         end
 
-        if action.user
+        if action.user && (user_ids.present? || group_ids.present?)
           MessageBus.publish(
             "/u/#{action.user.username_lower}",
             action.id,
-            user_ids: [user_id],
+            user_ids: user_ids,
             group_ids: group_ids,
           )
         end

spec/lib/post_creator_spec.rb

@@ -259,26 +259,19 @@ RSpec.describe PostCreator do
         p = nil
         messages = MessageBus.track_publish { p = creator.create }
 
-        latest = messages.find { |m| m.channel == "/latest" }
+        expect(messages.find { _1.channel == "/latest" }).not_to eq(nil)
-        expect(latest).not_to eq(nil)
+        expect(messages.find { _1.channel == "/new" }).not_to eq(nil)
+        expect(messages.find { _1.channel == "/unread/#{p.user_id}" }).not_to eq(nil)
+        expect(messages.find { _1.channel == "/user-drafts/#{p.user_id}" }).not_to eq(nil)
 
-        latest = messages.find { |m| m.channel == "/new" }
+        user_action = messages.find { _1.channel == "/u/#{p.user.username}" }
-        expect(latest).not_to eq(nil)
+        expect(user_action).to eq(nil)
-
-        read = messages.find { |m| m.channel == "/unread/#{p.user_id}" }
-        expect(read).not_to eq(nil)
-
-        user_action = messages.find { |m| m.channel == "/u/#{p.user.username}" }
-        expect(user_action).not_to eq(nil)
-
-        draft_count = messages.find { |m| m.channel == "/user-drafts/#{p.user_id}" }
-        expect(draft_count).not_to eq(nil)
 
         topics_stats =
           messages.find { |m| m.channel == "/topic/#{p.topic.id}" && m.data[:type] == :stats }
         expect(topics_stats).to eq(nil)
 
-        expect(messages.filter { |m| m.channel != "/distributed_hash" }.length).to eq(7)
+        expect(messages.filter { _1.channel != "/distributed_hash" }.size).to eq(6)
       end
 
       it "extracts links from the post" do

spec/models/private_message_topic_tracking_state_spec.rb

@@ -89,27 +89,24 @@ RSpec.describe PrivateMessageTopicTrackingState do
     it "should publish the right message_bus message" do
       messages = MessageBus.track_publish { described_class.publish_new(private_message) }
 
-      expect(messages.map(&:channel)).to contain_exactly(
+      expect(messages.map(&:channel)).to contain_exactly(described_class.user_channel(user_2.id))
-        described_class.user_channel(user.id),
-        described_class.user_channel(user_2.id),
-      )
 
-      data =
+      data = messages.first.data
-        messages.find { |message| message.channel == described_class.user_channel(user.id) }.data
 
       expect(data["message_type"]).to eq(described_class::NEW_MESSAGE_TYPE)
+      expect(data["topic_id"]).to eq(private_message.id)
+      expect(data["payload"]["last_read_post_number"]).to eq(nil)
+      expect(data["payload"]["highest_post_number"]).to eq(1)
+      expect(data["payload"]["group_ids"]).to eq([])
+      expect(data["payload"]["created_by_user_id"]).to eq(private_message.user_id)
     end
 
     it "should publish the right message_bus message for a group message" do
       messages = MessageBus.track_publish { described_class.publish_new(group_message) }
 
-      expect(messages.map(&:channel)).to contain_exactly(
+      expect(messages.map(&:channel)).to contain_exactly(described_class.group_channel(group.id))
-        described_class.group_channel(group.id),
-        described_class.user_channel(user.id),
-      )
 
-      data =
+      data = messages.first.data
-        messages.find { |message| message.channel == described_class.group_channel(group.id) }.data
 
       expect(data["message_type"]).to eq(described_class::NEW_MESSAGE_TYPE)
       expect(data["topic_id"]).to eq(group_message.id)
@@ -125,17 +122,13 @@ RSpec.describe PrivateMessageTopicTrackingState do
       messages =
         MessageBus.track_publish { described_class.publish_unread(private_message.first_post) }
 
-      expect(messages.map(&:channel)).to contain_exactly(
+      expect(messages.map(&:channel)).to contain_exactly(described_class.user_channel(user_2.id))
-        described_class.user_channel(user.id),
-        described_class.user_channel(user_2.id),
-      )
 
-      data =
+      data = messages.first.data
-        messages.find { |message| message.channel == described_class.user_channel(user.id) }.data
 
       expect(data["message_type"]).to eq(described_class::UNREAD_MESSAGE_TYPE)
       expect(data["topic_id"]).to eq(private_message.id)
-      expect(data["payload"]["last_read_post_number"]).to eq(1)
+      expect(data["payload"]["last_read_post_number"]).to eq(nil)
       expect(data["payload"]["highest_post_number"]).to eq(1)
       expect(data["payload"]["created_by_user_id"]).to eq(private_message.first_post.user_id)
       expect(data["payload"]["notification_level"]).to eq(NotificationLevels.all[:watching])

spec/models/user_action_spec.rb

@@ -42,7 +42,7 @@ RSpec.describe UserAction do
         end
 
       expect(m[0].group_ids).to eq([Group::AUTO_GROUPS[:admins]])
-      expect(m[0].user_ids).to eq([user.id])
+      expect(m[0].user_ids).to eq(nil)
     end
 
     describe "integration" do