From f9ab3848ed8c2c07c6e699d43b7b9ae22134cd5b Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Thu, 7 Jun 2018 09:44:35 +0530 Subject: [PATCH] FEATURE: support disabling emails for non-staff users --- .../discourse/components/global-notice.js.es6 | 2 +- app/controllers/admin/backups_controller.rb | 2 +- config/locales/server.en.yml | 2 +- config/site_settings.yml | 7 ++- .../20180607095414_migrate_disable_emails.rb | 11 +++++ lib/email/sender.rb | 7 ++- script/bulk_import/vanilla.rb | 2 +- script/import_scripts/base.rb | 2 +- script/import_scripts/ipboard.rb | 2 +- script/import_scripts/mbox.rb | 2 +- script/import_scripts/modx.rb | 2 +- script/import_scripts/mybb.rb | 2 +- script/import_scripts/mylittleforum.rb | 2 +- spec/components/email/sender_spec.rb | 45 ++++++++++++++----- .../admin/backups_controller_spec.rb | 4 +- 15 files changed, 69 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20180607095414_migrate_disable_emails.rb diff --git a/app/assets/javascripts/discourse/components/global-notice.js.es6 b/app/assets/javascripts/discourse/components/global-notice.js.es6 index 9ea7f4104bd..c8479c462df 100644 --- a/app/assets/javascripts/discourse/components/global-notice.js.es6 +++ b/app/assets/javascripts/discourse/components/global-notice.js.es6 @@ -22,7 +22,7 @@ export default Ember.Component.extend(bufferedRender({ notices.push([I18n.t("read_only_mode.enabled"), 'alert-read-only']); } - if (this.siteSettings.disable_emails) { + if (this.siteSettings.disable_emails === "yes" || this.siteSettings.disable_emails === "non-staff") { notices.push([I18n.t("emails_are_disabled"), 'alert-emails-disabled']); } diff --git a/app/controllers/admin/backups_controller.rb b/app/controllers/admin/backups_controller.rb index 9fd0f9c9bb2..ef8950fd5b5 100644 --- a/app/controllers/admin/backups_controller.rb +++ b/app/controllers/admin/backups_controller.rb @@ -99,7 +99,7 @@ class Admin::BackupsController < Admin::AdminController client_id: params.fetch(:client_id), publish_to_message_bus: true, } - SiteSetting.set_and_log(:disable_emails, true, current_user) + SiteSetting.set_and_log(:disable_emails, 'yes', current_user) BackupRestore.restore!(current_user.id, opts) rescue BackupRestore::OperationRunningError render json: failed_json.merge(message: I18n.t("backup.operation_already_running")) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index fd9451fa165..62138552cdb 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1486,7 +1486,7 @@ en: alternative_reply_by_email_addresses: "List of alternative templates for reply by email incoming email addresses. Example: %{reply_key}@reply.example.com|replies+%{reply_key}@example.com" incoming_email_prefer_html: "Use HTML instead of text for incoming email." - disable_emails: "Prevent Discourse from sending any kind of emails" + disable_emails: "Prevent Discourse from sending any kind of emails. Select 'yes' to disable emails for all users. Select 'non-staff' to disable emails for non-staff users only." strip_images_from_short_emails: "Strip images from emails having size less than 2800 Bytes" short_email_length: "Short email length in Bytes" diff --git a/config/site_settings.yml b/config/site_settings.yml index 637fee437c2..b972b8f80d4 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -783,8 +783,13 @@ email: email_prefix: '' email_site_title: '' disable_emails: - default: false client: true + type: enum + default: 'no' + choices: + - 'no' + - 'yes' + - 'non-staff' strip_images_from_short_emails: true short_email_length: 2800 display_name_on_email_from: diff --git a/db/migrate/20180607095414_migrate_disable_emails.rb b/db/migrate/20180607095414_migrate_disable_emails.rb new file mode 100644 index 00000000000..d679ba7eb72 --- /dev/null +++ b/db/migrate/20180607095414_migrate_disable_emails.rb @@ -0,0 +1,11 @@ +class MigrateDisableEmails < ActiveRecord::Migration[5.1] + def up + execute "UPDATE site_settings SET data_type = 7 WHERE name = 'disable_emails';" + execute "UPDATE site_settings SET value = 'yes' WHERE value = 't' AND name = 'disable_emails';" + execute "UPDATE site_settings SET value = 'no' WHERE value = 'f' AND name = 'disable_emails';" + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 6ded4b88e9c..f36b9496924 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -22,7 +22,7 @@ module Email end def send - return if SiteSetting.disable_emails && @email_type.to_s != "admin_login" + return if SiteSetting.disable_emails == "yes" && @email_type.to_s != "admin_login" return if ActionMailer::Base::NullMail === @message return if ActionMailer::Base::NullMail === (@message.message rescue nil) @@ -30,6 +30,11 @@ module Email return skip(I18n.t('email_log.message_blank')) if @message.blank? return skip(I18n.t('email_log.message_to_blank')) if @message.to.blank? + if SiteSetting.disable_emails == "non-staff" + user = User.find_by_email(to_address) + return unless user && user.staff? + end + if @message.text_part return skip(I18n.t('email_log.text_part_body_blank')) if @message.text_part.body.to_s.blank? else diff --git a/script/bulk_import/vanilla.rb b/script/bulk_import/vanilla.rb index 771a3ef67bb..584b9956194 100644 --- a/script/bulk_import/vanilla.rb +++ b/script/bulk_import/vanilla.rb @@ -55,7 +55,7 @@ class BulkImport::Vanilla < BulkImport::Base # SiteSetting.port = 3000 # SiteSetting.automatic_backups_enabled = false - # SiteSetting.disable_emails = true + # SiteSetting.disable_emails = "non-staff" # etc. import_users diff --git a/script/import_scripts/base.rb b/script/import_scripts/base.rb index fc50cc13cd6..92f33b9c9ce 100644 --- a/script/import_scripts/base.rb +++ b/script/import_scripts/base.rb @@ -75,7 +75,7 @@ class ImportScripts::Base min_personal_message_post_length: 1, min_personal_message_title_length: 1, allow_duplicate_topic_titles: true, - disable_emails: true, + disable_emails: "non-staff", max_attachment_size_kb: 102400, max_image_size_kb: 102400, authorized_extensions: '*' diff --git a/script/import_scripts/ipboard.rb b/script/import_scripts/ipboard.rb index 18708898bf7..f1b10b6a5b6 100644 --- a/script/import_scripts/ipboard.rb +++ b/script/import_scripts/ipboard.rb @@ -70,7 +70,7 @@ class ImportScripts::IpboardSQL < ImportScripts::Base # Site settings # ################# # don't send any emails - SiteSetting.disable_emails = true + SiteSetting.disable_emails = "non-staff" # don't send digests (so you can enable email without users noticing) SiteSetting.disable_digest_emails = true # keep site and users private diff --git a/script/import_scripts/mbox.rb b/script/import_scripts/mbox.rb index d0b379aa12a..2f125cbf33d 100755 --- a/script/import_scripts/mbox.rb +++ b/script/import_scripts/mbox.rb @@ -27,7 +27,7 @@ class ImportScripts::Mbox < ImportScripts::Base BATCH_SIZE = 1000 # Site settings - SiteSetting.disable_emails = true + SiteSetting.disable_emails = "non-staff" # Comment out if each file contains a single message # Use formail to split yourself: http://linuxcommand.org/man_pages/formail1.html diff --git a/script/import_scripts/modx.rb b/script/import_scripts/modx.rb index 191135f6bfe..682954bce0f 100644 --- a/script/import_scripts/modx.rb +++ b/script/import_scripts/modx.rb @@ -24,7 +24,7 @@ class ImportScripts::Modx < ImportScripts::Base def initialize super - SiteSetting.disable_emails = true + SiteSetting.disable_emails = "non-staff" @old_username_to_new_usernames = {} diff --git a/script/import_scripts/mybb.rb b/script/import_scripts/mybb.rb index f9201ecacfa..cc3ed9e6626 100644 --- a/script/import_scripts/mybb.rb +++ b/script/import_scripts/mybb.rb @@ -37,7 +37,7 @@ class ImportScripts::MyBB < ImportScripts::Base end def execute - SiteSetting.disable_emails = true + SiteSetting.disable_emails = "non-staff" import_users import_categories import_posts diff --git a/script/import_scripts/mylittleforum.rb b/script/import_scripts/mylittleforum.rb index f0ec1524720..4b10494eb4f 100644 --- a/script/import_scripts/mylittleforum.rb +++ b/script/import_scripts/mylittleforum.rb @@ -33,7 +33,7 @@ class ImportScripts::MylittleforumSQL < ImportScripts::Base QUIET = true # Site settings - SiteSetting.disable_emails = true + SiteSetting.disable_emails = "non-staff" if FORCE_HOSTNAME SiteSetting.force_hostname = FORCE_HOSTNAME end diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb index 7c73e7e2fb0..65838ab1d06 100644 --- a/spec/components/email/sender_spec.rb +++ b/spec/components/email/sender_spec.rb @@ -3,18 +3,41 @@ require 'email/sender' describe Email::Sender do - it "doesn't deliver mail when mails are disabled" do - SiteSetting.disable_emails = true - Mail::Message.any_instance.expects(:deliver_now).never - message = Mail::Message.new(to: "hello@world.com" , body: "hello") - expect(Email::Sender.new(message, :hello).send).to eq(nil) - end + context "disable_emails is enabled" do + let(:user) { Fabricate(:user) } + let(:moderator) { Fabricate(:moderator) } - it "delivers mail when mails are disabled but the email_type is admin_login" do - SiteSetting.disable_emails = true - Mail::Message.any_instance.expects(:deliver_now).once - message = Mail::Message.new(to: "hello@world.com" , body: "hello") - Email::Sender.new(message, :admin_login).send + context "disable_emails is enabled for everyone" do + before { SiteSetting.disable_emails = "yes" } + + it "doesn't deliver mail when mails are disabled" do + Mail::Message.any_instance.expects(:deliver_now).never + message = Mail::Message.new(to: moderator.email , body: "hello") + expect(Email::Sender.new(message, :hello).send).to eq(nil) + end + + it "delivers mail when mails are disabled but the email_type is admin_login" do + Mail::Message.any_instance.expects(:deliver_now).once + message = Mail::Message.new(to: moderator.email , body: "hello") + Email::Sender.new(message, :admin_login).send + end + end + + context "disable_emails is enabled for non-staff users" do + before { SiteSetting.disable_emails = "non-staff" } + + it "doesn't deliver mail to normal user" do + Mail::Message.any_instance.expects(:deliver_now).never + message = Mail::Message.new(to: user.email, body: "hello") + expect(Email::Sender.new(message, :hello).send).to eq(nil) + end + + it "delivers mail to staff user" do + Mail::Message.any_instance.expects(:deliver_now).once + message = Mail::Message.new(to: moderator.email, body: "hello") + Email::Sender.new(message, :hello).send + end + end end it "doesn't deliver mail when the message is of type NullMail" do diff --git a/spec/controllers/admin/backups_controller_spec.rb b/spec/controllers/admin/backups_controller_spec.rb index f3ebecd1476..6531a01712f 100644 --- a/spec/controllers/admin/backups_controller_spec.rb +++ b/spec/controllers/admin/backups_controller_spec.rb @@ -169,12 +169,12 @@ describe Admin::BackupsController do describe ".restore" do it "starts a restore" do - expect(SiteSetting.disable_emails).to eq(false) + expect(SiteSetting.disable_emails).to eq("no") BackupRestore.expects(:restore!).with(@admin.id, filename: backup_filename, publish_to_message_bus: true, client_id: "foo") post :restore, params: { id: backup_filename, client_id: "foo" }, format: :json - expect(SiteSetting.disable_emails).to eq(true) + expect(SiteSetting.disable_emails).to eq("yes") expect(response.status).to eq(200) end