DEV: Show message when cannot invite user to PM (#10336)

* DEV: Show message when cannot invite user to PM

When inviting a user to a PM return a message that says, "Sorry, this
user can't be invited." if they have been muted or are not in a users
allowed pm users list.

* Minor refactor & improved some text

app/controllers/topics_controller.rb

@@ -689,7 +689,7 @@ class TopicsController < ApplicationController
 
         render json: json, status: 422
       end
-    rescue Topic::UserExists => e
+    rescue Topic::UserExists, Topic::NotAllowed => e
       render json: { errors: [e.message] }, status: 422
     end
   end

app/models/topic.rb

@@ -2,6 +2,7 @@
 
 class Topic < ActiveRecord::Base
   class UserExists < StandardError; end
+  class NotAllowed < StandardError; end
   include ActionView::Helpers::SanitizeHelper
   include RateLimiter::OnCreateRecord
   include HasCustomFields
@@ -1020,7 +1021,11 @@ class Topic < ActiveRecord::Base
       end
 
       if invite_existing_muted?(target_user, invited_by)
-        return true
+        raise NotAllowed.new(I18n.t("topic_invite.not_allowed"))
+      end
+
+      if !allowed_pm_user?(target_user, invited_by)
+        raise NotAllowed.new(I18n.t("topic_invite.not_allowed"))
       end
 
       if private_message?
@@ -1055,6 +1060,43 @@ class Topic < ActiveRecord::Base
     false
   end
 
+  def allowed_pm_user?(target_user, invited_by)
+    return true if target_user.staff?
+    users = TopicAllowedUser.where(topic: self).pluck(:user_id)
+    users << target_user.id << invited_by.id
+    users.uniq
+
+    users_with_allowed_pms = allowed_pms_enabled(users).pluck(:id).uniq
+
+    if users_with_allowed_pms.any?
+      users_sender_can_pm = allowed_pms_enabled(users)
+        .where("allowed_pm_users.allowed_pm_user_id" => invited_by.id)
+        .pluck(:id).uniq
+
+      return false unless users_sender_can_pm.include? target_user.id
+
+      can_users_receive_from_sender = allowed_pms_enabled([invited_by.id])
+        .where("allowed_pm_users.allowed_pm_user_id IN (:user_ids)", user_ids: users.delete(invited_by.id))
+        .pluck(:id).uniq
+
+      users_not_allowed = users_with_allowed_pms - users_sender_can_pm - can_users_receive_from_sender
+      return false if users_not_allowed.any?
+    end
+
+    true
+  end
+
+  def allowed_pms_enabled(user_ids)
+    User
+      .joins("LEFT JOIN user_options ON user_options.user_id = users.id")
+      .joins("LEFT JOIN allowed_pm_users ON allowed_pm_users.user_id = users.id")
+      .where("
+        user_options.user_id IS NOT NULL AND
+        user_options.user_id IN (:user_ids) AND
+        user_options.enable_allowed_pm_users
+      ", user_ids: user_ids)
+  end
+
   def email_already_exists_for?(invite)
     invite.email_already_exists && private_message?
   end

app/services/user_updater.rb

@@ -231,11 +231,10 @@ class UserUpdater
   end
 
   def update_allowed_pm_users(usernames)
-    #return unless guardian.can_ignore_users?
-
     usernames ||= ""
     desired_usernames = usernames.split(",").reject { |username| user.username == username }
-    desired_ids = User.where(username: desired_usernames).where(admin: false, moderator: false).pluck(:id)
+    desired_ids = User.where(username: desired_usernames).pluck(:id)
+
     if desired_ids.empty?
       AllowedPmUser.where(user_id: user.id).destroy_all
     else

config/locales/client.en.yml

@@ -2466,6 +2466,7 @@ en:
         success: "We've invited that user to participate in this message."
         success_group: "We've invited that group to participate in this message."
         error: "Sorry, there was an error inviting that user."
+        not_allowed: "Sorry, that user can't be invited."
         group_name: "group name"
 
       controls: "Topic Controls"

config/locales/server.en.yml

@@ -244,6 +244,7 @@ en:
   topic_invite:
     failed_to_invite: "The user cannot be invited into this topic without a group membership in either one of the following groups: %{group_names}."
     user_exists: "Sorry, that user has already been invited. You may only invite a user to a topic once."
+    not_allowed: "Sorry, that user can't be invited."
 
   backup:
     operation_already_running: "An operation is currently running. Can't start a new job right now."

spec/models/topic_spec.rb

@@ -683,8 +683,9 @@ describe Topic do
         context "from a muted user" do
           before { MutedUser.create!(user: another_user, muted_user: user) }
 
-          it 'silently fails' do
+          it 'fails with an error message' do
-            expect(topic.invite(user, another_user.username)).to eq(true)
+            expect { topic.invite(user, another_user.username) }
+              .to raise_error(Topic::NotAllowed)
             expect(topic.allowed_users).to_not include(another_user)
             expect(Post.last).to be_blank
             expect(Notification.last).to be_blank
@@ -701,6 +702,49 @@ describe Topic do
               .to raise_error(Topic::UserExists)
           end
         end
+
+        context "when invited_user has enabled allow_list" do
+          fab!(:user2) { Fabricate(:user) }
+          fab!(:admin) { Fabricate(:admin) }
+          fab!(:pm) { Fabricate(:private_message_topic, user: user, topic_allowed_users: [
+            Fabricate.build(:topic_allowed_user, user: user),
+            Fabricate.build(:topic_allowed_user, user: user2)
+          ]) }
+
+          it 'succeeds when inviter is in allowed list' do
+            AllowedPmUser.create!(user: another_user, allowed_pm_user: user)
+            expect(topic.invite(user, another_user.username)).to eq(true)
+          end
+
+          it 'should raise error when inviter not in allowed list' do
+            another_user.user_option.update!(enable_allowed_pm_users: true)
+            AllowedPmUser.create!(user: another_user, allowed_pm_user: user2)
+            expect { topic.invite(user, another_user.username) }
+              .to raise_error(Topic::NotAllowed)
+          end
+
+          it 'should succeed for staff even when not allowed' do
+            another_user.user_option.update!(enable_allowed_pm_users: true)
+            AllowedPmUser.create!(user: another_user, allowed_pm_user: user2)
+            expect(topic.invite(another_user, admin.username)).to eq(true)
+          end
+
+          it 'should raise error when target_user is not in inviters allowed list' do
+            user.user_option.update!(enable_allowed_pm_users: true)
+            another_user.user_option.update!(enable_allowed_pm_users: true)
+            AllowedPmUser.create!(user: another_user, allowed_pm_user: user)
+            expect { topic.invite(user, another_user.username) }
+              .to raise_error(Topic::NotAllowed)
+          end
+
+          it 'should raise error if target_user has not allowed any of the other participants' do
+            user2.user_option.update!(enable_allowed_pm_users: true)
+            AllowedPmUser.create!(user: user2, allowed_pm_user: user)
+
+            expect { pm.invite(user, another_user.username) }
+              .to raise_error(Topic::NotAllowed)
+          end
+        end
       end
 
       describe 'by email' do
@@ -820,8 +864,9 @@ describe Topic do
         context "for a muted topic" do
           before { TopicUser.change(another_user.id, topic.id, notification_level: TopicUser.notification_levels[:muted]) }
 
-          it 'silently fails' do
+          it 'fails with an error message' do
-            expect(topic.invite(user, another_user.username)).to eq(true)
+            expect { topic.invite(user, another_user.username) }
+              .to raise_error(Topic::NotAllowed)
             expect(topic.allowed_users).to_not include(another_user)
             expect(Post.last).to be_blank
             expect(Notification.last).to be_blank