FIX: Make can_send_private_messages not reliant on system user (#18812)

Since the system user is a regular user, it can have its
`allow_private_messages` user option turned off, which
with our current `can_send_private_message?(Discourse.system_user)`
check inside the CurrentUserSerializer, will prevent any
user from sending messages in the UI if the system user is not
accepting PMs.

This commit adds a new `can_send_private_messages?` method to
the Guardian, which can be used in serializers and not depend
on the system user. When the user actually sends a message
we still rely on the old `can_send_private_message?(target)`
call to see if they are allowed to send the message to the target.
The new method is just to say they can "generally" send
private messages.

app/serializers/current_user_serializer.rb

@@ -170,7 +170,7 @@ class CurrentUserSerializer < BasicUserSerializer
   end
 
   def can_send_private_messages
-    scope.can_send_private_message?(Discourse.system_user)
+    scope.can_send_private_messages?
   end
 
   def can_edit

app/serializers/user_card_serializer.rb

@@ -141,7 +141,7 @@ class UserCardSerializer < BasicUserSerializer
   # Needed because 'send_private_message_to_user' will always return false
   # when the current user is being serialized
   def can_send_private_messages
-    scope.can_send_private_message?(Discourse.system_user)
+    scope.can_send_private_messages?
   end
 
   def can_send_private_message_to_user

lib/guardian.rb

@@ -445,23 +445,42 @@ class Guardian
     can_send_private_message?(group)
   end
 
-  def can_send_private_message?(target, notify_moderators: false)
+  ##
-    is_user = target.is_a?(User)
+  # This should be used as a general, but not definitive, check for whether
-    is_group = target.is_a?(Group)
+  # the user can send private messages _generally_, which is mostly useful
+  # for changing the UI.
+  #
+  # Please otherwise use can_send_private_message?(target, notify_moderators)
+  # to check if a single target can be messaged.
+  def can_send_private_messages?(notify_moderators: false)
     from_system = @user.is_system_user?
     from_bot = @user.bot?
 
-    (is_group || is_user) &&
     # User is authenticated
     authenticated? &&
+      # User can send PMs, this can be covered by trust levels as well via AUTO_GROUPS
+      (is_staff? || from_bot || from_system || \
+       (@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map)) || notify_moderators)
+  end
+
+  ##
+  # This should be used as a final check for when a user is sending a message
+  # to a target user or group.
+  def can_send_private_message?(target, notify_moderators: false)
+    target_is_user = target.is_a?(User)
+    target_is_group = target.is_a?(Group)
+    from_system = @user.is_system_user?
+
+    # Must be a valid target
+    (target_is_group || target_is_user) &&
+    # User is authenticated and can send PMs, this can be covered by trust levels as well via AUTO_GROUPS
+    can_send_private_messages?(notify_moderators: notify_moderators) &&
     # User disabled private message
-    (is_staff? || is_group || target.user_option.allow_private_messages) &&
+    (is_staff? || target_is_group || target.user_option.allow_private_messages) &&
-    # User can send PMs, this can be covered by trust levels as well via AUTO_GROUPS
-    (is_staff? || from_bot || from_system || (@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map)) || notify_moderators) &&
     # Can't send PMs to suspended users
-    (is_staff? || is_group || !target.suspended?) &&
+    (is_staff? || target_is_group || !target.suspended?) &&
     # Check group messageable level
-    (from_system || is_user || Group.messageable(@user).where(id: target.id).exists? || notify_moderators) &&
+    (from_system || target_is_user || Group.messageable(@user).where(id: target.id).exists? || notify_moderators) &&
     # Silenced users can only send PM to staff
     (!is_silenced? || target.staff?)
   end

spec/lib/guardian_spec.rb

@@ -420,6 +420,78 @@ RSpec.describe Guardian do
     end
   end
 
+  describe 'can_send_private_messages' do
+    fab!(:suspended_user) { Fabricate(:user, suspended_till: 1.week.from_now, suspended_at: 1.day.ago) }
+
+    it "returns false when the user is nil" do
+      expect(Guardian.new(nil).can_send_private_messages?).to be_falsey
+    end
+
+    it "disallows pms to other users if the user is not in the automated trust level used for personal_message_enabled_groups" do
+      SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_2]
+      user.update!(trust_level: TrustLevel[1])
+      Group.user_trust_level_change!(user.id, TrustLevel[1])
+      user.reload
+      expect(Guardian.new(user).can_send_private_messages?).to be_falsey
+      user.update!(trust_level: TrustLevel[2])
+      Group.user_trust_level_change!(user.id, TrustLevel[2])
+      user.reload
+      expect(Guardian.new(user).can_send_private_messages?).to be_truthy
+    end
+
+    context "when personal_message_enabled_groups does contain the user" do
+      let(:group) { Fabricate(:group) }
+      before do
+        SiteSetting.personal_message_enabled_groups = group.id
+      end
+
+      it "returns true" do
+        expect(Guardian.new(user).can_send_private_messages?).to be_falsey
+        GroupUser.create(user: user, group: group)
+        user.reload
+        expect(Guardian.new(user).can_send_private_messages?).to be_truthy
+      end
+    end
+
+    context "when personal_message_enabled_groups does not contain the user" do
+      let(:group) { Fabricate(:group) }
+      before do
+        SiteSetting.personal_message_enabled_groups = group.id
+      end
+
+      it "returns false if user is not staff member" do
+        expect(Guardian.new(trust_level_4).can_send_private_messages?).to be_falsey
+        GroupUser.create(user: trust_level_4, group: group)
+        trust_level_4.reload
+        expect(Guardian.new(trust_level_4).can_send_private_messages?).to be_truthy
+      end
+
+      it "returns true for staff member" do
+        expect(Guardian.new(moderator).can_send_private_messages?).to be_truthy
+        expect(Guardian.new(admin).can_send_private_messages?).to be_truthy
+      end
+
+      it "returns true for bot user" do
+        expect(Guardian.new(Fabricate(:bot)).can_send_private_messages?).to be_truthy
+      end
+
+      it "returns true for system user" do
+        expect(Guardian.new(Discourse.system_user).can_send_private_messages?).to be_truthy
+      end
+    end
+
+    context "when author is silenced" do
+      before do
+        user.silenced_till = 1.year.from_now
+        user.save
+      end
+
+      it "returns true, since there is no target user, we do that check separately" do
+        expect(Guardian.new(user).can_send_private_messages?).to be_truthy
+      end
+    end
+  end
+
   describe 'can_reply_as_new_topic' do
     fab!(:topic) { Fabricate(:topic) }
     fab!(:private_message) { Fabricate(:private_message_topic) }