FIX: Don’t email suspended users from group PM

Currently, when a suspended user belongs to a group PM (private message
with more than two people in it) and a staff member sends a message to
this group PM, then the suspended user will receive an email.
This happens because a suspended user can only receive emails from staff
members. But in this case, this can be seen as a bug as the expected
behavior would be instead to not send any email to the suspended user. A
staff member can participate in active discussions like any other
member and so their messages in this context shouldn’t be treated
differently than the ones from regular users.

This patch addresses this issue by checking if a suspended user receives
a message from a group PM or not. If that’s the case then an email won’t
be sent no matter if the post originated from a staff member or not.

app/jobs/regular/user_email.rb

@@ -108,8 +108,12 @@ module Jobs
         return skip_message(SkippedEmailLog.reason_types[:user_email_anonymous_user])
       end
 
-      if user.suspended? && !%w[user_private_message account_suspended].include?(type.to_s)
-        return skip_message(SkippedEmailLog.reason_types[:user_email_user_suspended_not_pm])
+      if user.suspended?
+        if !type.to_s.in?(%w[user_private_message account_suspended])
+          return skip_message(SkippedEmailLog.reason_types[:user_email_user_suspended_not_pm])
+        elsif post.topic.group_pm?
+          return skip_message(SkippedEmailLog.reason_types[:user_email_user_suspended])
+        end
       end
 
       if type.to_s == "digest"

app/models/topic.rb

@@ -2032,6 +2032,10 @@ class Topic < ActiveRecord::Base
     end
   end
 
+  def group_pm?
+    private_message? && all_allowed_users.count > 2
+  end
+
   private
 
   def invite_to_private_message(invited_by, target_user, guardian)

spec/jobs/user_email_spec.rb

@@ -423,37 +423,76 @@ RSpec.describe Jobs::UserEmail do
       end
 
       context "when user is suspended" do
-        it "doesn't send email for a pm from a regular user" do
-          Jobs::UserEmail.new.execute(
-            type: :user_private_message,
-            user_id: suspended.id,
-            post_id: post.id,
-          )
+        context "when topic is a private message" do
+          subject(:send_email) do
+            described_class.new.execute(
+              type: :user_private_message,
+              user_id: suspended.id,
+              post_id: post.id,
+              notification_id: pm_notification.id,
+            )
+          end
 
-          expect(ActionMailer::Base.deliveries).to eq([])
-        end
-
-        it "does send an email for a pm from a staff user" do
-          pm_from_staff = Fabricate(:post, user: Fabricate(:moderator))
-          pm_from_staff.topic.topic_allowed_users.create!(user_id: suspended.id)
-
-          pm_notification =
+          let(:pm_notification) do
             Fabricate(
               :notification,
               user: suspended,
-              topic: pm_from_staff.topic,
-              post_number: pm_from_staff.post_number,
-              data: { original_post_id: pm_from_staff.id }.to_json,
+              topic: post.topic,
+              post_number: post.post_number,
+              data: { original_post_id: post.id }.to_json,
             )
+          end
+          fab!(:moderator) { Fabricate(:moderator) }
+          fab!(:regular_user) { Fabricate(:user) }
 
-          Jobs::UserEmail.new.execute(
-            type: :user_private_message,
-            user_id: suspended.id,
-            post_id: pm_from_staff.id,
-            notification_id: pm_notification.id,
-          )
+          context "when this is not a group PM" do
+            let(:post) { Fabricate(:private_message_post, user: user, recipient: suspended) }
 
-          expect(ActionMailer::Base.deliveries.first.to).to contain_exactly(suspended.email)
+            context "when post is from a staff user" do
+              let(:user) { moderator }
+
+              it "does send an email" do
+                send_email
+                expect(ActionMailer::Base.deliveries.first.to).to contain_exactly(suspended.email)
+              end
+            end
+
+            context "when post is from a regular user" do
+              let(:user) { regular_user }
+
+              it "doesn't send email" do
+                send_email
+                expect(ActionMailer::Base.deliveries).to eq([])
+              end
+            end
+          end
+
+          context "when this is a group PM" do
+            fab!(:group) { Fabricate(:group) }
+            fab!(:users) { Fabricate.times(2, :user) }
+
+            let(:post) { Fabricate(:group_private_message_post, user: user, recipients: group) }
+
+            before { group.users << [suspended, *users] }
+
+            context "when post is from a staff user" do
+              let(:user) { moderator }
+
+              it "does not send an email" do
+                send_email
+                expect(ActionMailer::Base.deliveries).to be_empty
+              end
+            end
+
+            context "when post is from a regular user" do
+              let(:user) { regular_user }
+
+              it "does not send an email" do
+                send_email
+                expect(ActionMailer::Base.deliveries).to be_empty
+              end
+            end
+          end
         end
 
         it "doesn't send PM from system user" do

spec/models/topic_spec.rb

@@ -3458,4 +3458,28 @@ RSpec.describe Topic do
       end
     end
   end
+
+  describe "#group_pm?" do
+    context "when topic is not a private message" do
+      subject(:public_topic) { Fabricate(:topic) }
+
+      it { is_expected.not_to be_a_group_pm }
+    end
+
+    context "when topic is a private message" do
+      subject(:pm_topic) { Fabricate(:private_message_topic) }
+
+      context "when more than two people have access" do
+        let(:other_user) { Fabricate(:user) }
+
+        before { pm_topic.allowed_users << other_user }
+
+        it { is_expected.to be_a_group_pm }
+      end
+
+      context "when no more than two people have access" do
+        it { is_expected.not_to be_a_group_pm }
+      end
+    end
+  end
 end