diff options
author | Rémy Coutable <remy@rymai.me> | 2016-03-10 17:15:14 +0100 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-03-15 11:23:57 +0100 |
commit | 76350e2ede187a8bd15e343c30537c90ee557aa7 (patch) | |
tree | 6b5081eda7f9c9c2e7b18e86ae1631dd3b5c7f2c | |
parent | 9403142083bf0ace81fc3059f2d6c5a494e48cbf (diff) | |
download | gitlab-ce-fix/ensure-no-new_ssh_key_email-dead-jobs.tar.gz |
Ensure "new SSH key" email do not ends up as dead Sidekiq jobsfix/ensure-no-new_ssh_key_email-dead-jobs
Related to #2235.
This is done by:
1. Delaying the notification sending after the SSH key is commited in DB
2. Gracefully exit the mailer method if the record cannot be found
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/mailers/emails/profile.rb | 5 | ||||
-rw-r--r-- | app/models/key.rb | 3 | ||||
-rw-r--r-- | spec/mailers/emails/profile_spec.rb | 6 |
4 files changed, 12 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG index d38646ece67..27c595585a9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -25,6 +25,7 @@ v 8.6.0 (unreleased) - Allow to pass name of created artifacts archive in `.gitlab-ci.yml` - Refactor and greatly improve search performance - Add support for cross-project label references + - Ensure "new SSH key" email do not ends up as dead Sidekiq jobs - Update documentation to reflect Guest role not being enforced on internal projects - Allow search for logged out users - Allow to define on which builds the current one depends on diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index 3a83b083109..256cbcd73a1 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -14,7 +14,10 @@ module Emails end def new_ssh_key_email(key_id) - @key = Key.find(key_id) + @key = Key.find_by_id(key_id) + + return unless @key + @current_user = @user = @key.user @target_url = user_url(@user) mail(to: @user.notification_email, subject: subject("SSH key was added to your account")) diff --git a/app/models/key.rb b/app/models/key.rb index 406a1257b5d..0282ad18139 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -16,6 +16,7 @@ require 'digest/md5' class Key < ActiveRecord::Base + include AfterCommitQueue include Sortable belongs_to :user @@ -62,7 +63,7 @@ class Key < ActiveRecord::Base end def notify_user - NotificationService.new.new_key(self) + run_after_commit { NotificationService.new.new_key(self) } end def post_create_hook diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index 5b575da34f3..c6758ccad39 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -11,7 +11,7 @@ describe Notify do let(:example_site_path) { root_path } let(:new_user) { create(:user, email: new_user_address, created_by_id: 1) } let(:token) { 'kETLwRaayvigPq_x3SNM' } - + subject { Notify.new_user_email(new_user.id, token) } it_behaves_like 'an email sent from GitLab' @@ -77,6 +77,10 @@ describe Notify do it 'includes a link to ssh keys page' do is_expected.to have_body_text /#{profile_keys_path}/ end + + context 'with SSH key that does not exist' do + it { expect { Notify.new_ssh_key_email('foo') }.not_to raise_error } + end end describe 'user added email' do |