From a130cb83056d9ad0403dbc2054531fc69c057713 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 7 Apr 2016 14:38:43 +1000 Subject: [PATCH] FEATURE: move more urgent emails notifications to critical queue Move signup, admin login and password change email notifications to critical queue --- app/controllers/admin/users_controller.rb | 2 +- app/controllers/session_controller.rb | 2 +- app/controllers/users_controller.rb | 4 ++-- .../{forgot_password.rb => critical_user_email.rb} | 3 +-- app/models/user.rb | 2 +- app/services/user_activator.rb | 2 +- lib/email_updater.rb | 4 ++-- spec/components/email_updater_spec.rb | 10 +++++----- spec/controllers/admin/users_controller_spec.rb | 2 +- spec/controllers/session_controller_spec.rb | 4 ++-- spec/controllers/users_controller_spec.rb | 8 ++++---- spec/models/user_spec.rb | 2 +- spec/services/user_activator_spec.rb | 6 +++--- 13 files changed, 25 insertions(+), 26 deletions(-) rename app/jobs/regular/{forgot_password.rb => critical_user_email.rb} (77%) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index b51f6e71a35..8b10de76627 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -336,7 +336,7 @@ class Admin::UsersController < Admin::AdminController email_token = user.email_tokens.create(email: user.email) unless params[:send_email] == '0' || params[:send_email] == 'false' - Jobs.enqueue( :user_email, + Jobs.enqueue( :critical_user_email, type: :account_created, user_id: user.id, email_token: email_token.token) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 0e7da392277..21e8259db3a 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -204,7 +204,7 @@ class SessionController < ApplicationController user_presence = user.present? && user.id != Discourse::SYSTEM_USER_ID && !user.staged if user_presence email_token = user.email_tokens.create(email: user.email) - Jobs.enqueue(:forgot_password, user_id: user.id, email_token: email_token.token) + Jobs.enqueue(:critical_user_email, type: :forgot_password, user_id: user.id, email_token: email_token.token) end json = { result: "ok" } diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d20d090c4fb..0e3967a6b30 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -431,7 +431,7 @@ class UsersController < ApplicationController user = User.where(email: params[:email], admin: true).where.not(id: Discourse::SYSTEM_USER_ID).first if user email_token = user.email_tokens.create(email: user.email) - Jobs.enqueue(:user_email, type: :admin_login, user_id: user.id, email_token: email_token.token) + Jobs.enqueue(:critical_user_email, type: :admin_login, user_id: user.id, email_token: email_token.token) @message = I18n.t("admin_login.success") else @message = I18n.t("admin_login.error") @@ -516,7 +516,7 @@ class UsersController < ApplicationController def enqueue_activation_email @email_token ||= @user.email_tokens.create(email: @user.email) - Jobs.enqueue(:user_email, type: :signup, user_id: @user.id, email_token: @email_token.token) + Jobs.enqueue(:critical_user_email, type: :signup, user_id: @user.id, email_token: @email_token.token) end def search_users diff --git a/app/jobs/regular/forgot_password.rb b/app/jobs/regular/critical_user_email.rb similarity index 77% rename from app/jobs/regular/forgot_password.rb rename to app/jobs/regular/critical_user_email.rb index 6f335d0f30f..4add2d7db30 100644 --- a/app/jobs/regular/forgot_password.rb +++ b/app/jobs/regular/critical_user_email.rb @@ -2,12 +2,11 @@ require_dependency "#{Rails.root}/app/jobs/regular/user_email.rb" module Jobs - class ForgotPassword < UserEmail + class CriticalUserEmail < UserEmail sidekiq_options queue: 'critical' def execute(args) - args[:type] = :forgot_password super(args) end end diff --git a/app/models/user.rb b/app/models/user.rb index 8c9dde86180..d5c1985f368 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -929,7 +929,7 @@ class User < ActiveRecord::Base def send_approval_email if SiteSetting.must_approve_users - Jobs.enqueue(:user_email, + Jobs.enqueue(:critical_user_email, type: :signup_after_approval, user_id: id, email_token: email_tokens.first.token diff --git a/app/services/user_activator.rb b/app/services/user_activator.rb index f5ba7a3703d..c2d5c9dfe47 100644 --- a/app/services/user_activator.rb +++ b/app/services/user_activator.rb @@ -46,7 +46,7 @@ class EmailActivator < UserActivator email_token = user.email_tokens.unconfirmed.active.first email_token = user.email_tokens.create(email: user.email) if email_token.nil? - Jobs.enqueue(:user_email, + Jobs.enqueue(:critical_user_email, type: :signup, user_id: user.id, email_token: email_token.token diff --git a/lib/email_updater.rb b/lib/email_updater.rb index 1bda27bb27e..ff6b13c9903 100644 --- a/lib/email_updater.rb +++ b/lib/email_updater.rb @@ -100,14 +100,14 @@ class EmailUpdater protected def notify_old(old_email, new_email) - Jobs.enqueue :user_email, + Jobs.enqueue :critical_user_email, to_address: old_email, type: :notify_old_email, user_id: @user.id end def send_email(type, email_token) - Jobs.enqueue :user_email, + Jobs.enqueue :critical_user_email, to_address: email_token.email, type: type, user_id: @user.id, diff --git a/spec/components/email_updater_spec.rb b/spec/components/email_updater_spec.rb index 74ec89e2f2f..8db0699a13b 100644 --- a/spec/components/email_updater_spec.rb +++ b/spec/components/email_updater_spec.rb @@ -21,7 +21,7 @@ describe EmailUpdater do let(:updater) { EmailUpdater.new(user.guardian, user) } before do - Jobs.expects(:enqueue).once.with(:user_email, has_entries(type: :confirm_new_email, to_address: new_email)) + Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email)) updater.change_to(new_email) @change_req = user.email_change_requests.first end @@ -48,7 +48,7 @@ describe EmailUpdater do context 'confirming a valid token' do it "updates the user's email" do - Jobs.expects(:enqueue).once.with(:user_email, has_entries(type: :notify_old_email, to_address: old_email)) + Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email, to_address: old_email)) updater.confirm(@change_req.new_email_token.token) expect(updater.errors).to be_blank expect(user.reload.email).to eq(new_email) @@ -65,7 +65,7 @@ describe EmailUpdater do let(:updater) { EmailUpdater.new(user.guardian, user) } before do - Jobs.expects(:enqueue).once.with(:user_email, has_entries(type: :confirm_old_email, to_address: old_email)) + Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_old_email, to_address: old_email)) updater.change_to(new_email) @change_req = user.email_change_requests.first end @@ -92,7 +92,7 @@ describe EmailUpdater do context 'confirming a valid token' do before do - Jobs.expects(:enqueue).once.with(:user_email, has_entries(type: :confirm_new_email, to_address: new_email)) + Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email)) updater.confirm(@change_req.old_email_token.token) @change_req.reload end @@ -117,7 +117,7 @@ describe EmailUpdater do context "completing the new update process" do before do - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :notify_old_email, to_address: old_email)).never + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :notify_old_email, to_address: old_email)).never updater.confirm(@change_req.new_email_token.token) end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index d49745940b4..ac851a9c9fd 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -486,7 +486,7 @@ describe Admin::UsersController do context ".invite_admin" do it 'should invite admin' do - Jobs.expects(:enqueue).with(:user_email, anything).returns(true) + Jobs.expects(:enqueue).with(:critical_user_email, anything).returns(true) xhr :post, :invite_admin, name: 'Bill', username: 'bill22', email: 'bill@bill.com' expect(response).to be_success diff --git a/spec/controllers/session_controller_spec.rb b/spec/controllers/session_controller_spec.rb index 0c128bfc275..62219cb66a5 100644 --- a/spec/controllers/session_controller_spec.rb +++ b/spec/controllers/session_controller_spec.rb @@ -210,7 +210,7 @@ describe SessionController do end it 'sends an activation email' do - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup)) + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) sso = get_sso('/a/') sso.external_id = '666' # the number of the beast sso.email = 'bob@bob.com' @@ -632,7 +632,7 @@ describe SessionController do end it "enqueues an email" do - Jobs.expects(:enqueue).with(:forgot_password, has_entries(user_id: user.id)) + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :forgot_password, user_id: user.id)) xhr :post, :forgot_password, login: user.username end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index ff0e0d58c4e..43887f1ea57 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -321,7 +321,7 @@ describe UsersController do context 'enqueues mail' do it 'enqueues mail with admin email and sso enabled' do - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :admin_login, user_id: admin.id)) + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :admin_login, user_id: admin.id)) put :admin_login, email: admin.email end end @@ -417,7 +417,7 @@ describe UsersController do end it 'creates a user correctly' do - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup)) + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) User.any_instance.expects(:enqueue_welcome_message).with('welcome_user').never post_user @@ -1297,7 +1297,7 @@ describe UsersController do context 'with a valid email_token' do it 'should send the activation email' do - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup)) + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username end end @@ -1315,7 +1315,7 @@ describe UsersController do end it 'should send an email' do - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup)) + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 84a992ae942..42575c62f57 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -47,7 +47,7 @@ describe User do it "enqueues a 'signup after approval' email if must_approve_users is true" do SiteSetting.stubs(:must_approve_users).returns(true) Jobs.expects(:enqueue).with( - :user_email, has_entries(type: :signup_after_approval) + :critical_user_email, has_entries(type: :signup_after_approval) ) user.approve(admin) end diff --git a/spec/services/user_activator_spec.rb b/spec/services/user_activator_spec.rb index 928c760f1ba..43609ee4798 100644 --- a/spec/services/user_activator_spec.rb +++ b/spec/services/user_activator_spec.rb @@ -9,7 +9,7 @@ describe UserActivator do user = Fabricate(:user) activator = EmailActivator.new(user, nil, nil, nil) - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup, email_token: user.email_tokens.first.token)) + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup, email_token: user.email_tokens.first.token)) activator.activate end @@ -20,8 +20,8 @@ describe UserActivator do email_token.update_column(:created_at, 48.hours.ago) activator = EmailActivator.new(user, nil, nil, nil) - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup)) - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup, email_token: email_token.token)).never + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) + Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup, email_token: email_token.token)).never activator.activate user.reload