FIX: don't allow inviting more than `max_allowed_message_recipients`

* FIX: don't allow inviting more than `max_allowed_message_recipients` setting allows

* add specs for guardian

* user preferences for auto track shouldn't be applicable to PMs (it auto watches on visit)

Execlude PMs from "Automatically track topics I enter..." and "When I post in a topic, set that topic to..." user preferences

* groups take only 1 slot in PM

* just return if topic is a PM

app/controllers/topics_controller.rb

@@ -523,6 +523,12 @@ class TopicsController < ApplicationController
 
     topic = Topic.find_by(id: params[:topic_id])
 
+    unless pm_has_slots?(topic)
+      return render_json_error(I18n.t("pm_reached_recipients_limit",
+        recipients_limit: SiteSetting.max_allowed_message_recipients
+      ))
+    end
+
     if topic.private_message?
       guardian.ensure_can_invite_group_to_private_message!(group, topic)
       topic.invite_group(current_user, group)
@@ -543,6 +549,12 @@ class TopicsController < ApplicationController
       group_names: params[:group_names]
     )
 
+    unless pm_has_slots?(topic)
+      return render_json_error(I18n.t("pm_reached_recipients_limit",
+        recipients_limit: SiteSetting.max_allowed_message_recipients
+      ))
+    end
+
     guardian.ensure_can_invite_to!(topic, groups)
     group_ids = groups.map(&:id)
 
@@ -880,4 +892,7 @@ class TopicsController < ApplicationController
     params[:email]
   end
 
+  def pm_has_slots?(pm)
+    guardian.is_staff? || !pm.reached_recipients_limit?
+  end
 end

app/models/topic.rb

@@ -801,6 +801,11 @@ class Topic < ActiveRecord::Base
     false
   end
 
+  def reached_recipients_limit?
+    return false unless private_message?
+    topic_allowed_users.count + topic_allowed_groups.count >= SiteSetting.max_allowed_message_recipients
+  end
+
   def invite_group(user, group)
     TopicAllowedGroup.create!(topic_id: id, group_id: group.id)
 

app/models/topic_user.rb

@@ -40,7 +40,7 @@ class TopicUser < ActiveRecord::Base
     def auto_notification(user_id, topic_id, reason, notification_level)
       should_change = TopicUser
         .where(user_id: user_id, topic_id: topic_id)
-        .where("notifications_reason_id IS NULL OR (notification_level < :min AND notification_level > :max)", min: notification_level, max: notification_levels[:regular])
+        .where("notifications_reason_id IS NULL OR (notification_level < :max AND notification_level > :min)", max: notification_level, min: notification_levels[:regular])
         .exists?
 
       change(user_id, topic_id, notification_level: notification_level, notifications_reason_id: reason) if should_change
@@ -243,7 +243,8 @@ class TopicUser < ActiveRecord::Base
                                       notification_level =
                                          case when tu.notifications_reason_id is null and (tu.total_msecs_viewed + :msecs) >
                                             coalesce(uo.auto_track_topics_after_msecs,:threshold) and
-                                            coalesce(uo.auto_track_topics_after_msecs, :threshold) >= 0 then
+                                            coalesce(uo.auto_track_topics_after_msecs, :threshold) >= 0
+                                            and t.archetype = 'regular' then
                                               :tracking
                                          else
                                             tu.notification_level

config/locales/server.en.yml

@@ -263,6 +263,7 @@ en:
   topic_not_found: "Something has gone wrong. Perhaps this topic was closed or deleted while you were looking at it?"
   not_accepting_pms: "Sorry, %{username} is not accepting messages at the moment."
   max_pm_recepients: "Sorry, you can send a message to maximum %{recipients_limit} recipients."
+  pm_reached_recipients_limit: "Sorry, you can't have more than %{recipients_limit} recipients in a message."
 
   just_posted_that: "is too similar to what you recently posted"
   invalid_characters: "contains invalid characters"

lib/guardian.rb

@@ -272,6 +272,7 @@ class Guardian
 
     if object.is_a?(Topic) && object.private_message?
       return false unless SiteSetting.enable_personal_messages?
+      return false if object.reached_recipients_limit? && !is_staff?
     end
 
     if object.is_a?(Topic) && object.category

lib/post_creator.rb

@@ -531,6 +531,7 @@ class PostCreator
     if @user.staged
       TopicUser.auto_notification_for_staging(@user.id, @topic.id, TopicUser.notification_reasons[:auto_watch])
     else
+      return if @topic.private_message?
       notification_level = @user.user_option.notification_level_when_replying || NotificationLevels.topic_levels[:tracking]
       TopicUser.auto_notification(@user.id, @topic.id, TopicUser.notification_reasons[:created_post], notification_level)
     end

spec/components/guardian_spec.rb

@@ -568,6 +568,7 @@ describe Guardian do
       let(:user) { Fabricate(:user, trust_level: TrustLevel[2]) }
       let!(:pm) { Fabricate(:private_message_topic, user: user) }
       let(:admin) { Fabricate(:admin) }
+      let(:moderator) { Fabricate(:moderator) }
 
       context "when private messages are disabled" do
         it "allows an admin to invite to the pm" do
@@ -586,6 +587,22 @@ describe Guardian do
           expect(Guardian.new(user).can_invite_to?(pm)).to be_falsey
         end
       end
+
+      context "when PM has receached the maximum number of recipients" do
+        before do
+          SiteSetting.max_allowed_message_recipients = 2
+        end
+
+        it "doesn't allow a regular user to invite" do
+          expect(Guardian.new(user).can_invite_to?(pm)).to be_falsey
+        end
+
+        it "allows staff to invite" do
+          expect(Guardian.new(admin).can_invite_to?(pm)).to be_truthy
+          pm.grant_permission_to_user(moderator.email)
+          expect(Guardian.new(moderator).can_invite_to?(pm)).to be_truthy
+        end
+      end
     end
   end
 

spec/components/post_creator_spec.rb

@@ -1054,6 +1054,22 @@ describe PostCreator do
       topic_user = TopicUser.find_by(user_id: user.id, topic_id: post.topic_id)
       expect(topic_user.notification_level).to eq(TopicUser.notification_levels[:regular])
     end
+
+    it "user preferences for notification level when replying doesn't affect PMs" do
+      user.user_option.update!(notification_level_when_replying: 1)
+
+      admin = Fabricate(:admin)
+      pm = Fabricate(:private_message_topic, user: admin)
+
+      pm.invite(admin, user.username)
+      PostCreator.create(
+        user,
+        topic_id: pm.id,
+        raw: "this is a test reply 123 123 ;)"
+      )
+      topic_user = TopicUser.find_by(user_id: user.id, topic_id: pm.id)
+      expect(topic_user.notification_level).to eq(3)
+    end
   end
 
   describe '#create!' do

spec/models/topic_user_spec.rb

@@ -372,6 +372,18 @@ describe TopicUser do
         TopicUser.update_last_read(new_user, topic, 2, 2, SiteSetting.default_other_auto_track_topics_after_msecs + 1)
         expect(topic_new_user.notification_level).to eq(TopicUser.notification_levels[:regular])
       end
+
+      it 'should not automatically track PMs' do
+        new_user.user_option.update!(auto_track_topics_after_msecs: 0)
+
+        another_user = Fabricate(:user)
+        pm = Fabricate(:private_message_topic, user: another_user)
+        pm.invite(another_user, new_user.username)
+
+        TopicUser.track_visit!(pm.id, new_user.id)
+        TopicUser.update_last_read(new_user, pm.id, 2, 2, 1000)
+        expect(TopicUser.get(pm, new_user).notification_level).to eq(TopicUser.notification_levels[:watching])
+      end
     end
   end
 

spec/requests/topics_controller_spec.rb

@@ -2026,6 +2026,37 @@ RSpec.describe TopicsController do
         expect(response.status).to eq(400)
       end
 
+      describe "when PM has reached maximum allowed numbers of recipients" do
+        let(:user2) { Fabricate(:user) }
+        let(:pm) { Fabricate(:private_message_topic, user: user) }
+
+        let(:moderator) { Fabricate(:moderator) }
+        let(:moderator_pm) { Fabricate(:private_message_topic, user: moderator) }
+
+        before do
+          SiteSetting.max_allowed_message_recipients = 2
+        end
+
+        it "doesn't allow normal users to invite" do
+          post "/t/#{pm.id}/invite.json", params: {
+            user: user2.username
+          }
+          expect(response.status).to eq(422)
+          expect(JSON.parse(response.body)["errors"]).to contain_exactly(
+            I18n.t("pm_reached_recipients_limit", recipients_limit: SiteSetting.max_allowed_message_recipients)
+          )
+        end
+
+        it "allows staff to bypass limits" do
+          sign_in(moderator)
+          post "/t/#{moderator_pm.id}/invite.json", params: {
+            user: user2.username
+          }
+          expect(response.status).to eq(200)
+          expect(moderator_pm.reload.topic_allowed_users.count).to eq(3)
+        end
+      end
+
       describe 'when user does not have permission to invite to the topic' do
         let(:topic) { Fabricate(:private_message_topic) }
 
@@ -2115,6 +2146,37 @@ RSpec.describe TopicsController do
         expect(pm.allowed_groups.first.id).to eq(admins.id)
       end
     end
+
+    context "when PM has reached maximum allowed numbers of recipients" do
+      let(:group) { Fabricate(:group, messageable_level: 99) }
+      let(:pm) { Fabricate(:private_message_topic, user: user) }
+
+      let(:moderator) { Fabricate(:moderator) }
+      let(:moderator_pm) { Fabricate(:private_message_topic, user: moderator) }
+
+      before do
+        SiteSetting.max_allowed_message_recipients = 2
+      end
+
+      it "doesn't allow normal users to invite" do
+        post "/t/#{pm.id}/invite-group.json", params: {
+          group: group.name
+        }
+        expect(response.status).to eq(422)
+        expect(JSON.parse(response.body)["errors"]).to contain_exactly(
+          I18n.t("pm_reached_recipients_limit", recipients_limit: SiteSetting.max_allowed_message_recipients)
+        )
+      end
+
+      it "allows staff to bypass limits" do
+        sign_in(moderator)
+        post "/t/#{moderator_pm.id}/invite-group.json", params: {
+          group: group.name
+        }
+        expect(response.status).to eq(200)
+        expect(moderator_pm.reload.topic_allowed_users.count + moderator_pm.topic_allowed_groups.count).to eq(3)
+      end
+    end
   end
 
   describe 'shared drafts' do