Merge branch 'refactor-services' into 'master'
Refactor services to match EE signature See merge request gitlab-org/gitlab-ce!14385
This commit is contained in:
commit
355a1d8a6f
|
@ -22,8 +22,7 @@ class Admin::ApplicationsController < Admin::ApplicationController
|
|||
@application = Doorkeeper::Application.new(application_params)
|
||||
|
||||
if @application.save
|
||||
flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
|
||||
redirect_to admin_application_url(@application)
|
||||
redirect_to_admin_page
|
||||
else
|
||||
render :new
|
||||
end
|
||||
|
@ -42,6 +41,13 @@ class Admin::ApplicationsController < Admin::ApplicationController
|
|||
redirect_to admin_applications_url, status: 302, notice: 'Application was successfully destroyed.'
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def redirect_to_admin_page
|
||||
flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
|
||||
redirect_to admin_application_url(@application)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def set_application
|
||||
|
|
|
@ -128,7 +128,7 @@ class Admin::UsersController < Admin::ApplicationController
|
|||
end
|
||||
|
||||
respond_to do |format|
|
||||
result = Users::UpdateService.new(user, user_params_with_pass).execute do |user|
|
||||
result = Users::UpdateService.new(current_user, user_params_with_pass.merge(user: user)).execute do |user|
|
||||
user.skip_reconfirmation!
|
||||
end
|
||||
|
||||
|
@ -155,7 +155,7 @@ class Admin::UsersController < Admin::ApplicationController
|
|||
|
||||
def remove_email
|
||||
email = user.emails.find(params[:email_id])
|
||||
success = Emails::DestroyService.new(user, email: email.email).execute
|
||||
success = Emails::DestroyService.new(current_user, user: user, email: email.email).execute
|
||||
|
||||
respond_to do |format|
|
||||
if success
|
||||
|
@ -219,7 +219,7 @@ class Admin::UsersController < Admin::ApplicationController
|
|||
end
|
||||
|
||||
def update_user(&block)
|
||||
result = Users::UpdateService.new(user).execute(&block)
|
||||
result = Users::UpdateService.new(current_user, user: user).execute(&block)
|
||||
|
||||
result[:status] == :success
|
||||
end
|
||||
|
|
|
@ -12,10 +12,14 @@ class ConfirmationsController < Devise::ConfirmationsController
|
|||
|
||||
def after_confirmation_path_for(resource_name, resource)
|
||||
if signed_in?(resource_name)
|
||||
after_sign_in_path_for(resource)
|
||||
after_sign_in(resource)
|
||||
else
|
||||
flash[:notice] += " Please sign in."
|
||||
new_session_path(resource_name)
|
||||
end
|
||||
end
|
||||
|
||||
def after_sign_in(resource)
|
||||
after_sign_in_path_for(resource)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -21,14 +21,20 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
|
|||
@application.owner = current_user
|
||||
|
||||
if @application.save
|
||||
flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
|
||||
redirect_to oauth_application_url(@application)
|
||||
redirect_to_oauth_application_page
|
||||
else
|
||||
set_index_vars
|
||||
render :index
|
||||
end
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def redirect_to_oauth_application_page
|
||||
flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
|
||||
redirect_to oauth_application_url(@application)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def verify_user_oauth_applications_enabled
|
||||
|
|
|
@ -2,7 +2,7 @@ class Profiles::AvatarsController < Profiles::ApplicationController
|
|||
def destroy
|
||||
@user = current_user
|
||||
|
||||
Users::UpdateService.new(@user).execute { |user| user.remove_avatar! }
|
||||
Users::UpdateService.new(current_user, user: @user).execute { |user| user.remove_avatar! }
|
||||
|
||||
redirect_to profile_path, status: 302
|
||||
end
|
||||
|
|
|
@ -5,7 +5,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def create
|
||||
@email = Emails::CreateService.new(current_user, email_params).execute
|
||||
@email = Emails::CreateService.new(current_user, email_params.merge(user: current_user)).execute
|
||||
|
||||
if @email.errors.blank?
|
||||
NotificationService.new.new_email(@email)
|
||||
|
@ -19,7 +19,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
|
|||
def destroy
|
||||
@email = current_user.emails.find(params[:id])
|
||||
|
||||
Emails::DestroyService.new(current_user, email: @email.email).execute
|
||||
Emails::DestroyService.new(current_user, user: current_user, email: @email.email).execute
|
||||
|
||||
respond_to do |format|
|
||||
format.html { redirect_to profile_emails_url, status: 302 }
|
||||
|
|
|
@ -14,7 +14,7 @@ class Profiles::KeysController < Profiles::ApplicationController
|
|||
@key = Keys::CreateService.new(current_user, key_params).execute
|
||||
|
||||
if @key.persisted?
|
||||
redirect_to profile_key_path(@key)
|
||||
redirect_to_profile_key_path
|
||||
else
|
||||
@keys = current_user.keys.select(&:persisted?)
|
||||
render :index
|
||||
|
@ -50,6 +50,12 @@ class Profiles::KeysController < Profiles::ApplicationController
|
|||
end
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def redirect_to_profile_key_path
|
||||
redirect_to profile_key_path(@key)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def key_params
|
||||
|
|
|
@ -7,7 +7,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def update
|
||||
result = Users::UpdateService.new(current_user, user_params).execute
|
||||
result = Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute
|
||||
|
||||
if result[:status] == :success
|
||||
flash[:notice] = "Notification settings saved"
|
||||
|
|
|
@ -21,10 +21,10 @@ class Profiles::PasswordsController < Profiles::ApplicationController
|
|||
password_automatically_set: false
|
||||
}
|
||||
|
||||
result = Users::UpdateService.new(@user, password_attributes).execute
|
||||
result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute
|
||||
|
||||
if result[:status] == :success
|
||||
Users::UpdateService.new(@user, password_expires_at: nil).execute
|
||||
Users::UpdateService.new(current_user, user: @user, password_expires_at: nil).execute
|
||||
|
||||
redirect_to root_path, notice: 'Password successfully changed'
|
||||
else
|
||||
|
@ -46,7 +46,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
|
|||
return
|
||||
end
|
||||
|
||||
result = Users::UpdateService.new(@user, password_attributes).execute
|
||||
result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute
|
||||
|
||||
if result[:status] == :success
|
||||
flash[:notice] = "Password was successfully updated. Please login with it"
|
||||
|
|
|
@ -6,7 +6,7 @@ class Profiles::PreferencesController < Profiles::ApplicationController
|
|||
|
||||
def update
|
||||
begin
|
||||
result = Users::UpdateService.new(user, preferences_params).execute
|
||||
result = Users::UpdateService.new(current_user, preferences_params.merge(user: user)).execute
|
||||
|
||||
if result[:status] == :success
|
||||
flash[:notice] = 'Preferences saved.'
|
||||
|
|
|
@ -10,7 +10,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
|
|||
current_user.otp_grace_period_started_at = Time.current
|
||||
end
|
||||
|
||||
Users::UpdateService.new(current_user).execute!
|
||||
Users::UpdateService.new(current_user, user: current_user).execute!
|
||||
|
||||
if two_factor_authentication_required? && !current_user.two_factor_enabled?
|
||||
two_factor_authentication_reason(
|
||||
|
@ -41,7 +41,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
|
|||
|
||||
def create
|
||||
if current_user.validate_and_consume_otp!(params[:pin_code])
|
||||
Users::UpdateService.new(current_user, otp_required_for_login: true).execute! do |user|
|
||||
Users::UpdateService.new(current_user, user: current_user, otp_required_for_login: true).execute! do |user|
|
||||
@codes = user.generate_otp_backup_codes!
|
||||
end
|
||||
|
||||
|
@ -70,7 +70,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def codes
|
||||
Users::UpdateService.new(current_user).execute! do |user|
|
||||
Users::UpdateService.new(current_user, user: current_user).execute! do |user|
|
||||
@codes = user.generate_otp_backup_codes!
|
||||
end
|
||||
end
|
||||
|
|
|
@ -10,7 +10,7 @@ class ProfilesController < Profiles::ApplicationController
|
|||
|
||||
def update
|
||||
respond_to do |format|
|
||||
result = Users::UpdateService.new(@user, user_params).execute
|
||||
result = Users::UpdateService.new(current_user, user_params.merge(user: @user)).execute
|
||||
|
||||
if result[:status] == :success
|
||||
message = "Profile was successfully updated"
|
||||
|
@ -25,7 +25,7 @@ class ProfilesController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def reset_private_token
|
||||
Users::UpdateService.new(@user).execute! do |user|
|
||||
Users::UpdateService.new(current_user, user: @user).execute! do |user|
|
||||
user.reset_authentication_token!
|
||||
end
|
||||
|
||||
|
@ -35,7 +35,7 @@ class ProfilesController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def reset_incoming_email_token
|
||||
Users::UpdateService.new(@user).execute! do |user|
|
||||
Users::UpdateService.new(current_user, user: @user).execute! do |user|
|
||||
user.reset_incoming_email_token!
|
||||
end
|
||||
|
||||
|
@ -45,7 +45,7 @@ class ProfilesController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def reset_rss_token
|
||||
Users::UpdateService.new(@user).execute! do |user|
|
||||
Users::UpdateService.new(current_user, user: @user).execute! do |user|
|
||||
user.reset_rss_token!
|
||||
end
|
||||
|
||||
|
@ -61,7 +61,7 @@ class ProfilesController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def update_username
|
||||
result = Users::UpdateService.new(@user, username: user_params[:username]).execute
|
||||
result = Users::UpdateService.new(current_user, user: @user, username: user_params[:username]).execute
|
||||
|
||||
options = if result[:status] == :success
|
||||
{ notice: "Username successfully changed" }
|
||||
|
|
|
@ -55,7 +55,7 @@ class SessionsController < Devise::SessionsController
|
|||
|
||||
return unless user && user.require_password_creation?
|
||||
|
||||
Users::UpdateService.new(user).execute do |user|
|
||||
Users::UpdateService.new(current_user, user: user).execute do |user|
|
||||
@token = user.generate_reset_token
|
||||
end
|
||||
|
||||
|
|
|
@ -60,7 +60,7 @@ class User < ActiveRecord::Base
|
|||
lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i)
|
||||
return unless lease.try_obtain
|
||||
|
||||
Users::UpdateService.new(self).execute(validate: false)
|
||||
Users::UpdateService.new(self, user: self).execute(validate: false)
|
||||
end
|
||||
|
||||
attr_accessor :force_random_password
|
||||
|
@ -526,8 +526,8 @@ class User < ActiveRecord::Base
|
|||
def update_emails_with_primary_email
|
||||
primary_email_record = emails.find_by(email: email)
|
||||
if primary_email_record
|
||||
Emails::DestroyService.new(self, email: email).execute
|
||||
Emails::CreateService.new(self, email: email_was).execute
|
||||
Emails::DestroyService.new(self, user: self, email: email).execute
|
||||
Emails::CreateService.new(self, user: self, email: email_was).execute
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -1000,7 +1000,7 @@ class User < ActiveRecord::Base
|
|||
if attempts_exceeded?
|
||||
lock_access! unless access_locked?
|
||||
else
|
||||
Users::UpdateService.new(self).execute(validate: false)
|
||||
Users::UpdateService.new(self, user: self).execute(validate: false)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -1186,7 +1186,7 @@ class User < ActiveRecord::Base
|
|||
&creation_block
|
||||
)
|
||||
|
||||
Users::UpdateService.new(user).execute(validate: false)
|
||||
Users::UpdateService.new(user, user: user).execute(validate: false)
|
||||
user
|
||||
ensure
|
||||
Gitlab::ExclusiveLease.cancel(lease_key, uuid)
|
||||
|
|
|
@ -1,7 +1,8 @@
|
|||
module Emails
|
||||
class BaseService
|
||||
def initialize(user, opts)
|
||||
@user = user
|
||||
def initialize(current_user, opts)
|
||||
@current_user = current_user
|
||||
@user = opts.delete(:user)
|
||||
@email = opts[:email]
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,13 +1,13 @@
|
|||
module Emails
|
||||
class DestroyService < ::Emails::BaseService
|
||||
def execute
|
||||
Email.find_by_email!(@email).destroy && update_secondary_emails!
|
||||
update_secondary_emails! if Email.find_by_email!(@email).destroy
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def update_secondary_emails!
|
||||
result = ::Users::UpdateService.new(@user).execute do |user|
|
||||
result = ::Users::UpdateService.new(@current_user, user: @user).execute do |user|
|
||||
user.update_secondary_emails!
|
||||
end
|
||||
|
||||
|
|
|
@ -2,22 +2,21 @@ module Users
|
|||
class UpdateService < BaseService
|
||||
include NewUserNotifier
|
||||
|
||||
def initialize(user, params = {})
|
||||
@user = user
|
||||
def initialize(current_user, params = {})
|
||||
@current_user = current_user
|
||||
@user = params.delete(:user)
|
||||
@params = params.dup
|
||||
end
|
||||
|
||||
def execute(validate: true, &block)
|
||||
yield(@user) if block_given?
|
||||
|
||||
assign_attributes(&block)
|
||||
|
||||
user_exists = @user.persisted?
|
||||
|
||||
if @user.save(validate: validate)
|
||||
notify_new_user(@user, nil) unless user_exists
|
||||
assign_attributes(&block)
|
||||
|
||||
success
|
||||
if @user.save(validate: validate)
|
||||
notify_success(user_exists)
|
||||
else
|
||||
error(@user.errors.full_messages.uniq.join('. '))
|
||||
end
|
||||
|
@ -33,6 +32,12 @@ module Users
|
|||
|
||||
private
|
||||
|
||||
def notify_success(user_exists)
|
||||
notify_new_user(@user, nil) unless user_exists
|
||||
|
||||
success
|
||||
end
|
||||
|
||||
def assign_attributes(&block)
|
||||
if @user.user_synced_attributes_metadata
|
||||
params.except!(*@user.user_synced_attributes_metadata.read_only_attributes)
|
||||
|
|
|
@ -136,7 +136,7 @@ module API
|
|||
|
||||
codes = nil
|
||||
|
||||
::Users::UpdateService.new(user).execute! do |user|
|
||||
::Users::UpdateService.new(current_user, user: user).execute! do |user|
|
||||
codes = user.generate_otp_backup_codes!
|
||||
end
|
||||
|
||||
|
|
|
@ -35,7 +35,7 @@ module API
|
|||
new_notification_email = params.delete(:notification_email)
|
||||
|
||||
if new_notification_email
|
||||
::Users::UpdateService.new(current_user, notification_email: new_notification_email).execute
|
||||
::Users::UpdateService.new(current_user, user: current_user, notification_email: new_notification_email).execute
|
||||
end
|
||||
|
||||
notification_setting.update(declared_params(include_missing: false))
|
||||
|
|
|
@ -166,7 +166,7 @@ module API
|
|||
|
||||
user_params[:password_expires_at] = Time.now if user_params[:password].present?
|
||||
|
||||
result = ::Users::UpdateService.new(user, user_params.except(:extern_uid, :provider)).execute
|
||||
result = ::Users::UpdateService.new(current_user, user_params.except(:extern_uid, :provider).merge(user: user)).execute
|
||||
|
||||
if result[:status] == :success
|
||||
present user, with: Entities::UserPublic
|
||||
|
@ -326,7 +326,7 @@ module API
|
|||
user = User.find_by(id: params.delete(:id))
|
||||
not_found!('User') unless user
|
||||
|
||||
email = Emails::CreateService.new(user, declared_params(include_missing: false)).execute
|
||||
email = Emails::CreateService.new(current_user, declared_params(include_missing: false).merge(user: user)).execute
|
||||
|
||||
if email.errors.blank?
|
||||
NotificationService.new.new_email(email)
|
||||
|
@ -367,7 +367,7 @@ module API
|
|||
not_found!('Email') unless email
|
||||
|
||||
destroy_conditionally!(email) do |email|
|
||||
Emails::DestroyService.new(current_user, email: email.email).execute
|
||||
Emails::DestroyService.new(current_user, user: user, email: email.email).execute
|
||||
end
|
||||
|
||||
user.update_secondary_emails!
|
||||
|
@ -672,7 +672,7 @@ module API
|
|||
requires :email, type: String, desc: 'The new email'
|
||||
end
|
||||
post "emails" do
|
||||
email = Emails::CreateService.new(current_user, declared_params).execute
|
||||
email = Emails::CreateService.new(current_user, declared_params.merge(user: current_user)).execute
|
||||
|
||||
if email.errors.blank?
|
||||
NotificationService.new.new_email(email)
|
||||
|
@ -691,7 +691,7 @@ module API
|
|||
not_found!('Email') unless email
|
||||
|
||||
destroy_conditionally!(email) do |email|
|
||||
Emails::DestroyService.new(current_user, email: email.email).execute
|
||||
Emails::DestroyService.new(current_user, user: current_user, email: email.email).execute
|
||||
end
|
||||
|
||||
current_user.update_secondary_emails!
|
||||
|
|
|
@ -16,7 +16,7 @@ module Gitlab
|
|||
def self.allowed?(user)
|
||||
self.open(user) do |access|
|
||||
if access.allowed?
|
||||
Users::UpdateService.new(user, last_credential_check_at: Time.now).execute
|
||||
Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute
|
||||
|
||||
true
|
||||
else
|
||||
|
|
|
@ -32,7 +32,7 @@ module Gitlab
|
|||
|
||||
block_after_save = needs_blocking?
|
||||
|
||||
Users::UpdateService.new(gl_user).execute!
|
||||
Users::UpdateService.new(gl_user, user: gl_user).execute!
|
||||
|
||||
gl_user.block if block_after_save
|
||||
|
||||
|
|
|
@ -2,7 +2,7 @@ require 'spec_helper'
|
|||
|
||||
describe Emails::CreateService do
|
||||
let(:user) { create(:user) }
|
||||
let(:opts) { { email: 'new@email.com' } }
|
||||
let(:opts) { { email: 'new@email.com', user: user } }
|
||||
|
||||
subject(:service) { described_class.new(user, opts) }
|
||||
|
||||
|
|
|
@ -4,7 +4,7 @@ describe Emails::DestroyService do
|
|||
let!(:user) { create(:user) }
|
||||
let!(:email) { create(:email, user: user) }
|
||||
|
||||
subject(:service) { described_class.new(user, email: email.email) }
|
||||
subject(:service) { described_class.new(user, user: user, email: email.email) }
|
||||
|
||||
describe '#execute' do
|
||||
it 'removes an email' do
|
||||
|
|
|
@ -31,13 +31,13 @@ describe Users::UpdateService do
|
|||
end
|
||||
|
||||
def update_user(user, opts)
|
||||
described_class.new(user, opts).execute
|
||||
described_class.new(user, opts.merge(user: user)).execute
|
||||
end
|
||||
end
|
||||
|
||||
describe '#execute!' do
|
||||
it 'updates the name' do
|
||||
service = described_class.new(user, name: 'New Name')
|
||||
service = described_class.new(user, user: user, name: 'New Name')
|
||||
expect(service).not_to receive(:notify_new_user)
|
||||
|
||||
result = service.execute!
|
||||
|
@ -55,7 +55,7 @@ describe Users::UpdateService do
|
|||
it 'fires system hooks when a new user is saved' do
|
||||
system_hook_service = spy(:system_hook_service)
|
||||
user = build(:user)
|
||||
service = described_class.new(user, name: 'John Doe')
|
||||
service = described_class.new(user, user: user, name: 'John Doe')
|
||||
expect(service).to receive(:notify_new_user).and_call_original
|
||||
expect(service).to receive(:system_hook_service).and_return(system_hook_service)
|
||||
|
||||
|
@ -65,7 +65,7 @@ describe Users::UpdateService do
|
|||
end
|
||||
|
||||
def update_user(user, opts)
|
||||
described_class.new(user, opts).execute!
|
||||
described_class.new(user, opts.merge(user: user)).execute!
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue