Skip to content

Commit

Permalink
FIX: allow sending PMs to staff via flag even when PMs are disabled (#…
Browse files Browse the repository at this point in the history
…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 <arpit@techapj.com>
  • Loading branch information
arpitjalan committed Jan 24, 2019
1 parent cba6bda commit fabeba7
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 8 deletions.
6 changes: 3 additions & 3 deletions lib/guardian.rb
Expand Up @@ -343,19 +343,19 @@ 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)

(is_group || is_user) &&
# 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
Expand Down
2 changes: 1 addition & 1 deletion lib/guardian/post_guardian.rb
Expand Up @@ -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))

Expand Down
5 changes: 4 additions & 1 deletion lib/topic_creator.rb
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions spec/components/guardian_spec.rb
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions spec/components/topic_creator_spec.rb
Expand Up @@ -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
Expand Down

0 comments on commit fabeba7

Please sign in to comment.