Commit a628db7b authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fix/ensure-no-new_ssh_key_email-dead-jobs' into 'master'

Ensure "new SSH key" email do not ends up as dead Sidekiq 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

/cc @dblessing @stanhu @DouweM 

See merge request !3163
parents 7196ee10 76350e2e
...@@ -25,6 +25,7 @@ v 8.6.0 (unreleased) ...@@ -25,6 +25,7 @@ v 8.6.0 (unreleased)
- Allow to pass name of created artifacts archive in `.gitlab-ci.yml` - Allow to pass name of created artifacts archive in `.gitlab-ci.yml`
- Refactor and greatly improve search performance - Refactor and greatly improve search performance
- Add support for cross-project label references - 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 - Update documentation to reflect Guest role not being enforced on internal projects
- Allow search for logged out users - Allow search for logged out users
- Allow to define on which builds the current one depends on - Allow to define on which builds the current one depends on
......
...@@ -14,7 +14,10 @@ module Emails ...@@ -14,7 +14,10 @@ module Emails
end end
def new_ssh_key_email(key_id) 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 @current_user = @user = @key.user
@target_url = user_url(@user) @target_url = user_url(@user)
mail(to: @user.notification_email, subject: subject("SSH key was added to your account")) mail(to: @user.notification_email, subject: subject("SSH key was added to your account"))
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
require 'digest/md5' require 'digest/md5'
class Key < ActiveRecord::Base class Key < ActiveRecord::Base
include AfterCommitQueue
include Sortable include Sortable
belongs_to :user belongs_to :user
...@@ -62,7 +63,7 @@ class Key < ActiveRecord::Base ...@@ -62,7 +63,7 @@ class Key < ActiveRecord::Base
end end
def notify_user def notify_user
NotificationService.new.new_key(self) run_after_commit { NotificationService.new.new_key(self) }
end end
def post_create_hook def post_create_hook
......
...@@ -77,6 +77,10 @@ describe Notify do ...@@ -77,6 +77,10 @@ describe Notify do
it 'includes a link to ssh keys page' do it 'includes a link to ssh keys page' do
is_expected.to have_body_text /#{profile_keys_path}/ is_expected.to have_body_text /#{profile_keys_path}/
end end
context 'with SSH key that does not exist' do
it { expect { Notify.new_ssh_key_email('foo') }.not_to raise_error }
end
end end
describe 'user added email' do describe 'user added email' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment