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.

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"

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"
-    default: 4
+    enum: "TrustLevelAndStaffSetting"
-    enum: "TrustLevelSetting"
   min_trust_to_flag_posts:
     default: 1
     enum: "TrustLevelSetting"

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? ||
+    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'
-        # TODO: 2019 evaluate if we need this flexibility
+    SiteSetting.enable_personal_messages && @user.has_trust_level?(SiteSetting.min_trust_to_send_email_messages.to_i)
-        # perhaps we enable this unconditionally to TL4?
-        @user.has_trust_level?(SiteSetting.min_trust_to_send_email_messages) &&
-        SiteSetting.enable_personal_email_messages
-      )
-    )
   end
 
   def can_export_entity?(entity)

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])
+          SiteSetting.min_trust_to_send_email_messages = 'staff'
-          expect { TopicCreator.create(user, Guardian.new(user), pm_valid_attrs.merge(target_emails: 'test@example.com')) }.to raise_error(ActiveRecord::Rollback)
+          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