From fabeba788d06481bba054fa674b9938125d6a1eb Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Thu, 24 Jan 2019 16:56:59 +0530 Subject: [PATCH] FIX: allow sending PMs to staff via flag even when PMs are disabled (#6938) * FIX: allow sending PMs to staff via flag even when PMs are disabled FIX: allow sending PMs to staff via flag even if the user trust level is insufficient * Update lib/topic_creator.rb Co-Authored-By: techAPJ --- lib/guardian.rb | 6 +++--- lib/guardian/post_guardian.rb | 2 +- lib/topic_creator.rb | 5 ++++- spec/components/guardian_spec.rb | 4 +--- spec/components/topic_creator_spec.rb | 6 ++++++ 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/guardian.rb b/lib/guardian.rb index cb80bb781fe7f..6e9b00fb65d41 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -343,7 +343,7 @@ def can_invite_group_to_private_message?(group, topic) can_send_private_message?(group) end - def can_send_private_message?(target) + def can_send_private_message?(target, notify_moderators: false) is_user = target.is_a?(User) is_group = target.is_a?(Group) @@ -351,11 +351,11 @@ def can_send_private_message?(target) # User is authenticated authenticated? && # Have to be a basic level at least - @user.has_trust_level?(SiteSetting.min_trust_to_send_messages) && + (@user.has_trust_level?(SiteSetting.min_trust_to_send_messages) || notify_moderators) && # User disabled private message (is_staff? || is_group || target.user_option.allow_private_messages) && # PMs are enabled - (is_staff? || SiteSetting.enable_personal_messages) && + (is_staff? || SiteSetting.enable_personal_messages || notify_moderators) && # Can't send PMs to suspended users (is_staff? || is_group || !target.suspended?) && # Check group messageable level diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 0163496b82191..fd48124dcb761 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -48,7 +48,7 @@ def post_can_act?(post, action_key, opts: {}, can_see_post: nil) (!SiteSetting.allow_flagging_staff?) && post&.user&.staff? - if [:notify_user, :notify_moderators].include?(action_key) && + if action_key == :notify_user && (!SiteSetting.enable_personal_messages? || !@user.has_trust_level?(SiteSetting.min_trust_to_send_messages)) diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 840a0bd635a3e..97bb4ef8f1259 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -260,7 +260,10 @@ def add_groups(topic, groups) end def check_can_send_permission!(topic, obj) - rollback_with!(topic, :cant_send_pm) unless @opts[:skip_validations] || @guardian.can_send_private_message?(obj) + unless @opts[:skip_validations] || + @guardian.can_send_private_message?(obj, notify_moderators: topic&.subtype == TopicSubtype.notify_moderators)) + rollback_with!(topic, :cant_send_pm) + end end def find_or_create_user(email, display_name) diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 040c9a0caf5d0..8ab84d25fdd88 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -171,15 +171,13 @@ SiteSetting.enable_personal_messages = false user.trust_level = TrustLevel[2] expect(Guardian.new(user).post_can_act?(post, :notify_user)).to be_falsey - expect(Guardian.new(user).post_can_act?(post, :notify_moderators)).to be_falsey end - it "returns false for notify_user and notify_moderators if private messages are enabled but threshold not met" do + it "returns false for notify_user if private messages are enabled but threshold not met" do SiteSetting.enable_personal_messages = true SiteSetting.min_trust_to_send_messages = 2 user.trust_level = TrustLevel[1] expect(Guardian.new(user).post_can_act?(post, :notify_user)).to be_falsey - expect(Guardian.new(user).post_can_act?(post, :notify_moderators)).to be_falsey end describe "trust levels" do diff --git a/spec/components/topic_creator_spec.rb b/spec/components/topic_creator_spec.rb index 22d30bee759d5..4ea17bf22a5b0 100644 --- a/spec/components/topic_creator_spec.rb +++ b/spec/components/topic_creator_spec.rb @@ -154,6 +154,12 @@ expect(TopicCreator.create(user, Guardian.new(user), pm_to_email_valid_attrs)).to be_valid end + + it "enable_personal_messages setting should not be checked when sending private message to staff via flag" do + SiteSetting.enable_personal_messages = false + SiteSetting.min_trust_to_send_messages = TrustLevel[4] + expect(TopicCreator.create(user, Guardian.new(user), pm_valid_attrs.merge(subtype: TopicSubtype.notify_moderators))).to be_valid + end end context 'failure cases' do