From 0b41b236d7a1c5339636e73eb42190e8e4fc434e Mon Sep 17 00:00:00 2001
From: Natalie Tay <natalie.tay@discourse.org>
Date: Wed, 13 Mar 2024 11:05:34 +0800
Subject: [PATCH] FIX: Avoid sending user emails if @ mentioning a staged user
 in a topic (#26102)

Avoid sending user emails if @ mentioning a staged user

Some cases, unknowingly mentioning a staged user would invite
them into topics, sending them an email about it.
---
 app/services/notification_emailer.rb       |  8 ++++++-
 spec/services/notification_emailer_spec.rb | 27 +++++++++++++++++++---
 spec/services/post_alerter_spec.rb         |  2 +-
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/app/services/notification_emailer.rb b/app/services/notification_emailer.rb
index 0614a623530..951896d3965 100644
--- a/app/services/notification_emailer.rb
+++ b/app/services/notification_emailer.rb
@@ -100,7 +100,13 @@ class NotificationEmailer
       user = notification.user
       return unless user.active? || user.staged?
       return if SiteSetting.must_approve_users? && !user.approved? && !user.staged?
-      return if user.staged? && (type == :user_linked || type == :user_quoted)
+      if user.staged? &&
+           (
+             type == :user_linked || type == :user_quoted || type == :user_mentioned ||
+               type == :group_mentioned
+           )
+        return
+      end
 
       return unless EMAILABLE_POST_TYPES.include?(post_type)
 
diff --git a/spec/services/notification_emailer_spec.rb b/spec/services/notification_emailer_spec.rb
index 3f93bb01c48..c4667cedc83 100644
--- a/spec/services/notification_emailer_spec.rb
+++ b/spec/services/notification_emailer_spec.rb
@@ -18,6 +18,7 @@ RSpec.describe NotificationEmailer do
       notification_type: Notification.types[type],
       topic: topic,
       post_number: post.post_number,
+      skip_send_email: true,
     )
   end
 
@@ -42,7 +43,8 @@ RSpec.describe NotificationEmailer do
       it "enqueues a job if the user is staged for non-linked and non-quoted types" do
         notification.user.staged = true
 
-        if type == :user_linked || type == :user_quoted
+        if type == :user_linked || type == :user_quoted || type == :user_mentioned ||
+             type == :group_mentioned
           expect_not_enqueued_with(job: :user_email, args: { type: type }) do
             NotificationEmailer.process_notification(notification, no_delay: no_delay)
           end
@@ -59,7 +61,8 @@ RSpec.describe NotificationEmailer do
         notification.user.staged = true
         SiteSetting.must_approve_users = true
 
-        if type == :user_linked || type == :user_quoted
+        if type == :user_linked || type == :user_quoted || type == :user_mentioned ||
+             type == :group_mentioned
           expect_not_enqueued_with(job: :user_email, args: { type: type }) do
             NotificationEmailer.process_notification(notification, no_delay: no_delay)
           end
@@ -281,7 +284,7 @@ RSpec.describe NotificationEmailer do
     let(:no_delay) { true }
     let(:type) { :user_quoted }
 
-    after { DiscoursePluginRegistry.reset! }
+    after { DiscoursePluginRegistry.reset_register! :email_notification_filters }
 
     it "sends email when all filters return true" do
       plugin.register_email_notification_filter { |_| true }
@@ -301,4 +304,22 @@ RSpec.describe NotificationEmailer do
       end
     end
   end
+
+  context "with a staged user" do
+    context "when notification is mentioned or group_mentioned type" do
+      it "doesn't enqueue the job to send user email" do
+        staged_user = Fabricate(:staged)
+        mentioned = create_notification(:mentioned, staged_user)
+        group_mentioned = create_notification(:group_mentioned, staged_user)
+
+        expect_not_enqueued_with(job: :user_email) do
+          NotificationEmailer.process_notification(mentioned, no_delay: Time.zone.now)
+        end
+
+        expect_not_enqueued_with(job: :user_email) do
+          NotificationEmailer.process_notification(group_mentioned, no_delay: Time.zone.now)
+        end
+      end
+    end
+  end
 end
diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb
index 420a4ec8952..2667926580e 100644
--- a/spec/services/post_alerter_spec.rb
+++ b/spec/services/post_alerter_spec.rb
@@ -112,7 +112,7 @@ RSpec.describe PostAlerter do
       expect(Notification.where(user_id: pm.user_id).count).to eq(1)
     end
 
-    it "notifies about private message even if direct mention" do
+    it "prioritises 'private_message' type even if direct mention" do
       pm = Fabricate(:topic, archetype: "private_message", category_id: nil)
       op =
         Fabricate(:post, topic: pm, user: pm.user, raw: "Hello @#{user.username}, nice to meet you")