From 72ffabf619d145be91f7f9b85a1ae29a9c11d071 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 29 Aug 2018 23:14:16 +0200 Subject: [PATCH] UX: Improve email testing admin tool. (#6308) --- .../controllers/admin-email-index.js.es6 | 33 ++++++++--------- .../admin/templates/email-index.hbs | 2 +- app/controllers/admin/email_controller.rb | 8 ++++- config/locales/server.en.yml | 6 ++++ spec/requests/admin/email_controller_spec.rb | 35 +++++++++++++++++++ 5 files changed, 63 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/admin/controllers/admin-email-index.js.es6 b/app/assets/javascripts/admin/controllers/admin-email-index.js.es6 index 27b7bb498cf..1bab3fe2979 100644 --- a/app/assets/javascripts/admin/controllers/admin-email-index.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-email-index.js.es6 @@ -28,30 +28,25 @@ export default Ember.Controller.extend({ sentTestEmail: false }); - var self = this; ajax("/admin/email/test", { type: "POST", data: { email_address: this.get("testEmailAddress") } }) - .then( - function() { - self.set("sentTestEmail", true); - }, - function(e) { - if (e.responseJSON && e.responseJSON.errors) { - bootbox.alert( - I18n.t("admin.email.error", { - server_error: e.responseJSON.errors[0] - }) - ); - } else { - bootbox.alert(I18n.t("admin.email.test_error")); - } - } + .then(response => + this.set("sentTestEmailMessage", response.send_test_email_message) ) - .finally(function() { - self.set("sendingEmail", false); - }); + .catch(e => { + if (e.responseJSON && e.responseJSON.errors) { + bootbox.alert( + I18n.t("admin.email.error", { + server_error: e.responseJSON.errors[0] + }) + ); + } else { + bootbox.alert(I18n.t("admin.email.test_error")); + } + }) + .finally(() => this.set("sendingEmail", false)); } } }); diff --git a/app/assets/javascripts/admin/templates/email-index.hbs b/app/assets/javascripts/admin/templates/email-index.hbs index 80b3d5d062d..c21e3a5ca63 100644 --- a/app/assets/javascripts/admin/templates/email-index.hbs +++ b/app/assets/javascripts/admin/templates/email-index.hbs @@ -21,7 +21,7 @@
- {{#if sentTestEmail}}{{i18n 'admin.email.sent_test'}}{{/if}} + {{#if sentTestEmailMessage}}{{sentTestEmailMessage}}{{/if}}
{{/if}} diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index c626ffb2254..59d91fbc0ad 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -11,7 +11,13 @@ class Admin::EmailController < Admin::AdminController params.require(:email_address) begin Jobs::TestEmail.new.execute(to_address: params[:email_address]) - render body: nil + if SiteSetting.disable_emails == "yes" + render json: { sent_test_email_message: I18n.t("admin.email.sent_test_disabled") } + elsif SiteSetting.disable_emails == "non-staff" && !User.find_by_email(params[:email_address])&.staff? + render json: { sent_test_email_message: I18n.t("admin.email.sent_test_disabled_for_non_staff") } + else + render json: { sent_test_email_message: I18n.t("admin.email.sent_test") } + end rescue => e render json: { errors: [e.message] }, status: 422 end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 8c4eca02b15..784ed97e34d 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1963,6 +1963,12 @@ en: totp: "Use an authenticator app instead" backup_code: "Use a backup code instead" + admin: + email: + sent_test: "sent!" + sent_test_disabled: "cannot send because emails are disabled" + sent_test_disabled_for_non_staff: "cannot send because emails are disabled for non-staff" + user: deactivated: "Was deactivated due to too many bounced emails to '%{email}'." deactivated_by_staff: "Deactivated by staff" diff --git a/spec/requests/admin/email_controller_spec.rb b/spec/requests/admin/email_controller_spec.rb index e59f3cc125a..a6d30ba958f 100644 --- a/spec/requests/admin/email_controller_spec.rb +++ b/spec/requests/admin/email_controller_spec.rb @@ -106,6 +106,41 @@ describe Admin::EmailController do expect(ActionMailer::Base.deliveries.map(&:to).flatten).to include('eviltrout@test.domain') end end + + context 'with SiteSetting.disable_emails' do + let(:eviltrout) { Fabricate(:evil_trout) } + let(:admin) { Fabricate(:admin) } + + it 'does not sends mail to anyone when setting is "yes"' do + SiteSetting.disable_emails = 'yes' + + post "/admin/email/test.json", params: { email_address: admin.email } + + incoming = JSON.parse(response.body) + expect(incoming['sent_test_email_message']).to eq(I18n.t("admin.email.sent_test_disabled")) + end + + it 'sends mail to staff only when setting is "non-staff"' do + SiteSetting.disable_emails = 'non-staff' + + post "/admin/email/test.json", params: { email_address: admin.email } + incoming = JSON.parse(response.body) + expect(incoming['sent_test_email_message']).to eq(I18n.t("admin.email.sent_test")) + + post "/admin/email/test.json", params: { email_address: eviltrout.email } + incoming = JSON.parse(response.body) + expect(incoming['sent_test_email_message']).to eq(I18n.t("admin.email.sent_test_disabled_for_non_staff")) + end + + it 'sends mail to everyone when setting is "no"' do + SiteSetting.disable_emails = 'no' + + post "/admin/email/test.json", params: { email_address: eviltrout.email } + + incoming = JSON.parse(response.body) + expect(incoming['sent_test_email_message']).to eq(I18n.t("admin.email.sent_test")) + end + end end describe '#preview_digest' do