FEATURE: Improve assign mailer site setting (#57)

app/mailers/assign_mailer.rb

@@ -5,6 +5,12 @@ require_dependency 'email/message_builder'
 class AssignMailer < ActionMailer::Base
   include Email::BuildEmailHelper
 
+  def self.levels
+    @levels ||= Enum.new(never: 'never',
+                         different_users: 'different_users',
+                         always: 'always')
+  end
+
   def send_assignment(to_address, topic, assigned_by)
     opts = {
       template: 'assign_mailer',

app/models/assign_mailer_site_settings.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require_dependency 'enum_site_setting'
+
+class AssignMailerSiteSettings < EnumSiteSetting
+
+  def self.valid_value?(val)
+    values.any? { |v| v[:value].to_s == val.to_s }
+  end
+
+  def self.values
+    @values ||= [
+      { name: 'discourse_assign.assign_mailer.never', value: AssignMailer.levels[:never] },
+      { name: 'discourse_assign.assign_mailer.different_users', value: AssignMailer.levels[:different_users] },
+      { name: 'discourse_assign.assign_mailer.always', value: AssignMailer.levels[:always] }
+    ]
+  end
+
+  def self.translate_names?
+    true
+  end
+end

config/locales/client.en.yml

@@ -30,6 +30,10 @@ en:
       claim:
         title: "claim"
         help: "Assign topic to yourself"
+      assign_mailer:
+        never: 'Never'
+        different_users: 'Only if assigner and assignee are different users'
+        always: 'Always'
       reminders_frequency:
         description: "Frequency for receiving assigned topics reminders"
         never: "Never"

config/locales/server.en.yml

@@ -9,7 +9,7 @@ en:
     assign_other_regex: "Regex that needs to pass for assigning topics to others via mention. Example 'your list'."
     unassign_on_group_archive: "When a message is archived by a group, unassign message (reassign if moved back to inbox)"
     unassign_on_close: "When a topic is closed unassign topic"
-    assign_mailer_enabled: "When enabled, the assigned user will receive a notification email on each assignment"
+    assign_mailer: "When to send notification email for assignments"
     remind_assigns: "Remind users about pending assigns."
     remind_assigns_frequency: "Frequency for reminding users about assigned topics."
     max_assigned_topics: "Maximum number of topics that can be assigned to a user."

config/settings.yml

@@ -12,7 +12,9 @@ plugins:
   assigns_user_url_path:
     client: true
     default: "/u/{username}/activity/assigned"
-  assign_mailer_enabled: false
+  assign_mailer:
+    default: "never"
+    enum: "AssignMailerSiteSettings"
   remind_assigns_frequency:
     client: true
     enum: "RemindAssignsFrequencySiteSettings"

db/migrate/20191119174425_rename_site_setting_assign_emailer.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class RenameSiteSettingAssignEmailer < ActiveRecord::Migration[6.0]
+  def up
+    execute "UPDATE site_settings
+             SET name = 'assign_mailer', value = '#{AssignMailer.levels[:always]}', data_type = #{SiteSettings::TypeSupervisor.types[:enum]}
+             WHERE name = 'assign_mailer_enabled' AND value = 't' AND data_type = #{SiteSettings::TypeSupervisor.types[:enum]}"
+
+    execute "UPDATE site_settings
+             SET name = 'assign_mailer', value = '#{AssignMailer.levels[:never]}', data_type = #{SiteSettings::TypeSupervisor.types[:enum]}
+             WHERE name = 'assign_mailer_enabled' AND value = 'f' AND data_type = #{SiteSettings::TypeSupervisor.types[:enum]}"
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/topic_assigner.rb

@@ -189,7 +189,7 @@ class ::TopicAssigner
       )
     end
 
-    if SiteSetting.assign_mailer_enabled
+    if SiteSetting.assign_mailer == AssignMailer.levels[:always] || (SiteSetting.assign_mailer == AssignMailer.levels[:different_users] && @assigned_by.id != assign_to.id)
       if !@topic.muted?(assign_to)
         message = AssignMailer.send_assignment(assign_to.email, @topic, @assigned_by)
         Email::Sender.new(message, :assign_message).send

spec/lib/topic_assigner_spec.rb

@@ -116,7 +116,7 @@ RSpec.describe TopicAssigner do
     end
 
     it "doesn't assign the same user more than once" do
-      SiteSetting.assign_mailer_enabled = true
+      SiteSetting.assign_mailer = AssignMailer.levels[:always]
       another_mod = Fabricate(:moderator, groups: [assign_allowed_group])
 
       Email::Sender.any_instance.expects(:send).once
@@ -284,4 +284,39 @@ RSpec.describe TopicAssigner do
       expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to eq([topic])
     end
   end
+
+  context "assign_emailer" do
+    let(:post) { Fabricate(:post) }
+    let(:topic) { post.topic }
+    let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) }
+    let(:moderator2) { Fabricate(:moderator, groups: [assign_allowed_group]) }
+
+    it "send an email if set to 'always'" do
+      SiteSetting.assign_mailer = AssignMailer.levels[:always]
+
+      expect { TopicAssigner.new(topic, moderator).assign(moderator) }
+        .to change { ActionMailer::Base.deliveries.size }.by(1)
+    end
+
+    it "doesn't send an email if the assigner and assignee are not different" do
+      SiteSetting.assign_mailer = AssignMailer.levels[:different_users]
+
+      expect { TopicAssigner.new(topic, moderator).assign(moderator2) }
+        .to change { ActionMailer::Base.deliveries.size }.by(1)
+    end
+
+    it "doesn't send an email if the assigner and assignee are not different" do
+      SiteSetting.assign_mailer = AssignMailer.levels[:different_users]
+
+      expect { TopicAssigner.new(topic, moderator).assign(moderator) }
+        .to change { ActionMailer::Base.deliveries.size }.by(0)
+    end
+
+    it "doesn't send an email" do
+      SiteSetting.assign_mailer = AssignMailer.levels[:never]
+
+      expect { TopicAssigner.new(topic, moderator).assign(moderator2) }
+        .to change { ActionMailer::Base.deliveries.size }.by(0)
+    end
+  end
 end