Commit 6ecb3b01 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '35917_create_services_for_keys' into 'master'

created services for keys

See merge request gitlab-org/gitlab-ce!13331
parents ff7eb46d cc2daa74
...@@ -10,9 +10,8 @@ class Admin::DeployKeysController < Admin::ApplicationController ...@@ -10,9 +10,8 @@ class Admin::DeployKeysController < Admin::ApplicationController
end end
def create def create
@deploy_key = deploy_keys.new(create_params.merge(user: current_user)) @deploy_key = DeployKeys::CreateService.new(current_user, create_params.merge(public: true)).execute
if @deploy_key.persisted?
if @deploy_key.save
redirect_to admin_deploy_keys_path redirect_to admin_deploy_keys_path
else else
render 'new' render 'new'
......
...@@ -7,9 +7,9 @@ class Profiles::GpgKeysController < Profiles::ApplicationController ...@@ -7,9 +7,9 @@ class Profiles::GpgKeysController < Profiles::ApplicationController
end end
def create def create
@gpg_key = current_user.gpg_keys.new(gpg_key_params) @gpg_key = GpgKeys::CreateService.new(current_user, gpg_key_params).execute
if @gpg_key.save if @gpg_key.persisted?
redirect_to profile_gpg_keys_path redirect_to profile_gpg_keys_path
else else
@gpg_keys = current_user.gpg_keys.select(&:persisted?) @gpg_keys = current_user.gpg_keys.select(&:persisted?)
......
...@@ -11,9 +11,9 @@ class Profiles::KeysController < Profiles::ApplicationController ...@@ -11,9 +11,9 @@ class Profiles::KeysController < Profiles::ApplicationController
end end
def create def create
@key = current_user.keys.new(key_params) @key = Keys::CreateService.new(current_user, key_params).execute
if @key.save if @key.persisted?
redirect_to profile_key_path(@key) redirect_to profile_key_path(@key)
else else
@keys = current_user.keys.select(&:persisted?) @keys = current_user.keys.select(&:persisted?)
......
...@@ -22,7 +22,7 @@ class Projects::DeployKeysController < Projects::ApplicationController ...@@ -22,7 +22,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
end end
def create def create
@key = DeployKey.new(create_params.merge(user: current_user)) @key = DeployKeys::CreateService.new(current_user, create_params).execute
unless @key.valid? && @project.deploy_keys << @key unless @key.valid? && @project.deploy_keys << @key
flash[:alert] = @key.errors.full_messages.join(', ').html_safe flash[:alert] = @key.errors.full_messages.join(', ').html_safe
......
...@@ -28,10 +28,4 @@ class DeployKey < Key ...@@ -28,10 +28,4 @@ class DeployKey < Key
def can_push_to?(project) def can_push_to?(project)
can_push? && has_access_to?(project) can_push? && has_access_to?(project)
end end
private
# we don't want to notify the user for deploy keys
def notify_user
end
end end
...@@ -36,7 +36,6 @@ class GpgKey < ActiveRecord::Base ...@@ -36,7 +36,6 @@ class GpgKey < ActiveRecord::Base
before_validation :extract_fingerprint, :extract_primary_keyid before_validation :extract_fingerprint, :extract_primary_keyid
after_commit :update_invalid_gpg_signatures, on: :create after_commit :update_invalid_gpg_signatures, on: :create
after_commit :notify_user, on: :create
def primary_keyid def primary_keyid
super&.upcase super&.upcase
...@@ -107,8 +106,4 @@ class GpgKey < ActiveRecord::Base ...@@ -107,8 +106,4 @@ class GpgKey < ActiveRecord::Base
# only allows one key # only allows one key
self.primary_keyid = Gitlab::Gpg.primary_keyids_from_key(key).first self.primary_keyid = Gitlab::Gpg.primary_keyids_from_key(key).first
end end
def notify_user
NotificationService.new.new_gpg_key(self)
end
end end
...@@ -28,7 +28,6 @@ class Key < ActiveRecord::Base ...@@ -28,7 +28,6 @@ class Key < ActiveRecord::Base
delegate :name, :email, to: :user, prefix: true delegate :name, :email, to: :user, prefix: true
after_commit :add_to_shell, on: :create after_commit :add_to_shell, on: :create
after_commit :notify_user, on: :create
after_create :post_create_hook after_create :post_create_hook
after_commit :remove_from_shell, on: :destroy after_commit :remove_from_shell, on: :destroy
after_destroy :post_destroy_hook after_destroy :post_destroy_hook
...@@ -118,8 +117,4 @@ class Key < ActiveRecord::Base ...@@ -118,8 +117,4 @@ class Key < ActiveRecord::Base
"type is forbidden. Must be #{allowed_types}" "type is forbidden. Must be #{allowed_types}"
end end
def notify_user
NotificationService.new.new_key(self)
end
end end
module DeployKeys
class CreateService < Keys::BaseService
def execute
DeployKey.create(params.merge(user: user))
end
end
end
module GpgKeys
class CreateService < Keys::BaseService
def execute
key = user.gpg_keys.create(params)
notification_service.new_gpg_key(key) if key.persisted?
key
end
end
end
module Keys
class BaseService
attr_accessor :user, :params
def initialize(user, params)
@user, @params = user, params
end
def notification_service
NotificationService.new
end
end
end
module Keys
class CreateService < ::Keys::BaseService
def execute
key = user.keys.create(params)
notification_service.new_key(key) if key.persisted?
key
end
end
end
---
title: creation of keys moved to services
merge_request: 13331
author: haseebeqx
...@@ -140,18 +140,6 @@ describe GpgKey do ...@@ -140,18 +140,6 @@ describe GpgKey do
end end
end end
describe 'notification', :mailer do
let(:user) { create(:user) }
it 'sends a notification' do
perform_enqueued_jobs do
create(:gpg_key, user: user)
end
should_email(user)
end
end
describe '#revoke' do describe '#revoke' do
it 'invalidates all associated gpg signatures and destroys the key' do it 'invalidates all associated gpg signatures and destroys the key' do
gpg_key = create :gpg_key gpg_key = create :gpg_key
......
...@@ -169,16 +169,4 @@ describe Key, :mailer do ...@@ -169,16 +169,4 @@ describe Key, :mailer do
expect(described_class.new(key: " #{valid_key} ").key).to eq(valid_key) expect(described_class.new(key: " #{valid_key} ").key).to eq(valid_key)
end end
end end
describe 'notification' do
let(:user) { create(:user) }
it 'sends a notification' do
perform_enqueued_jobs do
create(:key, user: user)
end
should_email(user)
end
end
end end
require 'spec_helper'
describe DeployKeys::CreateService do
let(:user) { create(:user) }
let(:params) { attributes_for(:deploy_key) }
subject { described_class.new(user, params) }
it "creates a deploy key" do
expect { subject.execute }.to change { DeployKey.where(params.merge(user: user)).count }.by(1)
end
end
require 'spec_helper'
describe GpgKeys::CreateService do
let(:user) { create(:user) }
let(:params) { attributes_for(:gpg_key) }
subject { described_class.new(user, params) }
context 'notification', :mailer do
it 'sends a notification' do
perform_enqueued_jobs do
subject.execute
end
should_email(user)
end
end
it 'creates a gpg key' do
expect { subject.execute }.to change { user.gpg_keys.where(params).count }.by(1)
end
end
require 'spec_helper'
describe Keys::CreateService do
let(:user) { create(:user) }
let(:params) { attributes_for(:key) }
subject { described_class.new(user, params) }
context 'notification', :mailer do
it 'sends a notification' do
perform_enqueued_jobs do
subject.execute
end
should_email(user)
end
end
it 'creates a key' do
expect { subject.execute }.to change { user.keys.where(params).count }.by(1)
end
end
...@@ -84,7 +84,6 @@ describe NotificationService, :mailer do ...@@ -84,7 +84,6 @@ describe NotificationService, :mailer do
let!(:key) { create(:personal_key, key_options) } let!(:key) { create(:personal_key, key_options) }
it { expect(notification.new_key(key)).to be_truthy } it { expect(notification.new_key(key)).to be_truthy }
it { should_email(key.user) }
describe 'never emails the ghost user' do describe 'never emails the ghost user' do
let(:key_options) { { user: User.ghost } } let(:key_options) { { user: User.ghost } }
......
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