FIX: Simplify send PM to email settings (#12583)

FIX: Simplify send PM to email settings (#12583)

This feature used to be controlled by two site settings enable_personal_email_messages and min_trust_to_send_email_messages. I removed enable_personal_email_messages and unhide min_trust_to_send_email_messages to simplify the process of enabling / disabling this feature.

diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 8b0c98b..d26ca60 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1850,6 +1850,7 @@ en:
     min_trust_to_allow_self_wiki: "The minimum trust level required to make user's own post wiki."
 
     min_trust_to_send_messages: "The minimum trust level required to create new personal messages."
+    min_trust_to_send_email_messages: "The minimum trust level requred to send personal messages via email."
     min_trust_to_flag_posts: "The minimum trust level required to flag posts"
     min_trust_to_post_links: "The minimum trust level required to include links in posts"
     min_trust_to_post_embedded_media: "The minimum trust level required to embed media items in a post"
diff --git a/config/site_settings.yml b/config/site_settings.yml
index f4b3097..04d7de5 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -773,11 +773,6 @@ posting:
     client: true
   enable_system_message_replies:
     default: true
-  enable_personal_email_messages:
-    hidden: true
-    default: false
-    client: true
-    validator: "EnablePrivateEmailMessagesValidator"
   editing_grace_period: 300
   editing_grace_period_max_diff: 100
   editing_grace_period_max_diff_high_trust: 400
@@ -1419,9 +1414,8 @@ trust:
     default: 1
     enum: "TrustLevelSetting"
   min_trust_to_send_email_messages:
-    hidden: true
-    default: 4
-    enum: "TrustLevelSetting"
+    default: "4"
+    enum: "TrustLevelAndStaffSetting"
   min_trust_to_flag_posts:
     default: 1
     enum: "TrustLevelSetting"
diff --git a/lib/guardian.rb b/lib/guardian.rb
index f9488f9..18363e0 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -445,18 +445,13 @@ class Guardian
 
   def can_send_private_messages_to_email?
     # Staged users must be enabled to create a temporary user.
-    SiteSetting.enable_staged_users &&
+    return false if !SiteSetting.enable_staged_users
     # User is authenticated
-    authenticated? &&
+    return false if !authenticated?
     # User is trusted enough
-    (is_staff? ||
-      (
-        # TODO: 2019 evaluate if we need this flexibility
-        # perhaps we enable this unconditionally to TL4?
-        @user.has_trust_level?(SiteSetting.min_trust_to_send_email_messages) &&
-        SiteSetting.enable_personal_email_messages
-      )
-    )
+    return is_admin? if SiteSetting.min_trust_to_send_email_messages.to_s == 'admin'
+    return is_staff? if SiteSetting.min_trust_to_send_email_messages.to_s == 'staff'
+    SiteSetting.enable_personal_messages && @user.has_trust_level?(SiteSetting.min_trust_to_send_email_messages.to_i)
   end
 
   def can_export_entity?(entity)
diff --git a/spec/components/topic_creator_spec.rb b/spec/components/topic_creator_spec.rb
index 32484ae..945bd39 100644
--- a/spec/components/topic_creator_spec.rb
+++ b/spec/components/topic_creator_spec.rb
@@ -197,16 +197,6 @@ describe TopicCreator do
           expect(TopicCreator.create(user, Guardian.new(user), pm_valid_attrs)).to be_valid
         end
 
-        it "should be possible for a trusted user to send private messages via email" do
-          SiteSetting.manual_polling_enabled = true
-          SiteSetting.reply_by_email_address = "sam+%{reply_key}@sam.com"
-          SiteSetting.reply_by_email_enabled = true
-          SiteSetting.enable_personal_email_messages = true
-          SiteSetting.min_trust_to_send_email_messages = TrustLevel[1]
-
-          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]
@@ -219,7 +209,6 @@ describe TopicCreator do
           SiteSetting.manual_polling_enabled = true
           SiteSetting.reply_by_email_address = "sam+%{reply_key}@sam.com"
           SiteSetting.reply_by_email_enabled = true
-          SiteSetting.enable_personal_email_messages = true
           SiteSetting.min_trust_to_send_email_messages = TrustLevel[1]
           attrs = pm_to_email_valid_attrs.dup
           attrs[:target_emails] = "t" * 256
@@ -236,24 +225,29 @@ describe TopicCreator do
             TopicCreator.create(user, Guardian.new(user), pm_valid_attrs)
           end.to raise_error(ActiveRecord::Rollback)
         end
-
-        it "min_trust_to_send_email_messages should be checked when sending private messages via email" do
-          SiteSetting.min_trust_to_send_email_messages = TrustLevel[4]
-
-          expect do
-            TopicCreator.create(user, Guardian.new(user), pm_to_email_valid_attrs)
-          end.to raise_error(ActiveRecord::Rollback)
-        end
       end
 
       context 'to emails' do
         it 'works for staff' do
-          expect(TopicCreator.create(admin, Guardian.new(admin), pm_valid_attrs.merge(target_emails: 'test@example.com'))).to be_valid
+          SiteSetting.min_trust_to_send_email_messages = 'staff'
+          expect(TopicCreator.create(admin, Guardian.new(admin), pm_to_email_valid_attrs)).to be_valid
+        end
+
+        it 'work for trusted users' do
+          SiteSetting.min_trust_to_send_email_messages = 3
+          user.update!(trust_level: 3)
+          expect(TopicCreator.create(user, Guardian.new(user), pm_to_email_valid_attrs)).to be_valid
         end
 
         it 'does not work for non-staff' do
-          user.update!(trust_level: TrustLevel[4])
-          expect { TopicCreator.create(user, Guardian.new(user), pm_valid_attrs.merge(target_emails: 'test@example.com')) }.to raise_error(ActiveRecord::Rollback)
+          SiteSetting.min_trust_to_send_email_messages = 'staff'
+          expect { TopicCreator.create(user, Guardian.new(user), pm_to_email_valid_attrs) }.to raise_error(ActiveRecord::Rollback)
+        end
+
+        it 'does not work for untrusted users' do
+          SiteSetting.min_trust_to_send_email_messages = 3
+          user.update!(trust_level: 2)
+          expect { TopicCreator.create(user, Guardian.new(user), pm_to_email_valid_attrs) }.to raise_error(ActiveRecord::Rollback)
         end
       end
     end

GitHub sha: e026af11

1 Like

This commit appears in #12583 which was approved by eviltrout. It was merged by nbianca.