summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames Lopez <james@jameslopez.es>2017-09-20 11:00:06 +0200
committerJames Lopez <james@jameslopez.es>2017-09-28 08:46:39 +0200
commit9621dd0c9d31508bdac2e2e226537302b560ef10 (patch)
treef4e76be89a6cb1a85b04e87cdf33bb408f2bf63f
parent11c8b8bc3c2294ef2e3a33196619f6e61f1e8d82 (diff)
downloadgitlab-ce-9621dd0c9d31508bdac2e2e226537302b560ef10.tar.gz
refactor services to match EE signature
-rw-r--r--app/controllers/admin/users_controller.rb6
-rw-r--r--app/controllers/profiles/avatars_controller.rb2
-rw-r--r--app/controllers/profiles/emails_controller.rb4
-rw-r--r--app/controllers/profiles/notifications_controller.rb2
-rw-r--r--app/controllers/profiles/passwords_controller.rb6
-rw-r--r--app/controllers/profiles/preferences_controller.rb2
-rw-r--r--app/controllers/profiles/two_factor_auths_controller.rb6
-rw-r--r--app/controllers/profiles_controller.rb10
-rw-r--r--app/controllers/sessions_controller.rb2
-rw-r--r--app/models/user.rb10
-rw-r--r--app/services/emails/destroy_service.rb4
-rw-r--r--app/services/users/update_service.rb12
-rw-r--r--lib/api/internal.rb2
-rw-r--r--lib/api/notification_settings.rb2
-rw-r--r--lib/api/users.rb10
-rw-r--r--lib/gitlab/ldap/access.rb2
-rw-r--r--lib/gitlab/o_auth/user.rb2
17 files changed, 43 insertions, 41 deletions
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index cbcef70e957..8b6119548ee 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -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, user_params_with_pass).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, 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).execute(&block)
result[:status] == :success
end
diff --git a/app/controllers/profiles/avatars_controller.rb b/app/controllers/profiles/avatars_controller.rb
index 408650aac54..5a94dc88455 100644
--- a/app/controllers/profiles/avatars_controller.rb
+++ b/app/controllers/profiles/avatars_controller.rb
@@ -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).execute { |user| user.remove_avatar! }
redirect_to profile_path, status: 302
end
diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb
index ddb67d1c4d1..8328d230276 100644
--- a/app/controllers/profiles/emails_controller.rb
+++ b/app/controllers/profiles/emails_controller.rb
@@ -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, current_user, email_params).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, current_user, email: @email.email).execute
respond_to do |format|
format.html { redirect_to profile_emails_url, status: 302 }
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 960b7512602..45d7bca27bb 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -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, current_user, user_params).execute
if result[:status] == :success
flash[:notice] = "Notification settings saved"
diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb
index 7beb52dd8e8..08b438de9e2 100644
--- a/app/controllers/profiles/passwords_controller.rb
+++ b/app/controllers/profiles/passwords_controller.rb
@@ -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, @user, password_attributes).execute
if result[:status] == :success
- Users::UpdateService.new(@user, password_expires_at: nil).execute
+ Users::UpdateService.new(current_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, @user, password_attributes).execute
if result[:status] == :success
flash[:notice] = "Password was successfully updated. Please login with it"
diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb
index cce2a847b53..a13b9a616cb 100644
--- a/app/controllers/profiles/preferences_controller.rb
+++ b/app/controllers/profiles/preferences_controller.rb
@@ -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, user, preferences_params).execute
if result[:status] == :success
flash[:notice] = 'Preferences saved.'
diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb
index 1a4f77639e7..b590846257b 100644
--- a/app/controllers/profiles/two_factor_auths_controller.rb
+++ b/app/controllers/profiles/two_factor_auths_controller.rb
@@ -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, 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, 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, current_user).execute! do |user|
@codes = user.generate_otp_backup_codes!
end
end
diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb
index d83824fef06..9e85997cf6d 100644
--- a/app/controllers/profiles_controller.rb
+++ b/app/controllers/profiles_controller.rb
@@ -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, user_params).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).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).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).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, username: user_params[:username]).execute
options = if result[:status] == :success
{ notice: "Username successfully changed" }
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index be6491d042c..a0bd10d9726 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -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).execute do |user|
@token = user.generate_reset_token
end
diff --git a/app/models/user.rb b/app/models/user.rb
index 09c9b3250eb..e9a3aea44c4 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -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, 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, self, email: email).execute
+ Emails::CreateService.new(self, 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, 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).execute(validate: false)
user
ensure
Gitlab::ExclusiveLease.cancel(lease_key, uuid)
diff --git a/app/services/emails/destroy_service.rb b/app/services/emails/destroy_service.rb
index d586b9dfe0c..02dd803fca6 100644
--- a/app/services/emails/destroy_service.rb
+++ b/app/services/emails/destroy_service.rb
@@ -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).execute do |user|
user.update_secondary_emails!
end
diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb
index 6188b8a4349..fddeeb9d52a 100644
--- a/app/services/users/update_service.rb
+++ b/app/services/users/update_service.rb
@@ -12,12 +12,8 @@ module Users
assign_attributes(&block)
- user_exists = @user.persisted?
-
if @user.save(validate: validate)
- notify_new_user(@user, nil) unless user_exists
-
- success
+ notify_success
else
error(@user.errors.full_messages.uniq.join('. '))
end
@@ -33,6 +29,12 @@ module Users
private
+ def notify_success
+ notify_new_user(@user, nil) unless @user.persisted?
+
+ success
+ end
+
def assign_attributes(&block)
if @user.user_synced_attributes_metadata
params.except!(*@user.user_synced_attributes_metadata.read_only_attributes)
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index c0fef56378f..3d331ffdce2 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -136,7 +136,7 @@ module API
codes = nil
- ::Users::UpdateService.new(user).execute! do |user|
+ ::Users::UpdateService.new(current_user, user).execute! do |user|
codes = user.generate_otp_backup_codes!
end
diff --git a/lib/api/notification_settings.rb b/lib/api/notification_settings.rb
index bcc0833aa5c..1c5592d952d 100644
--- a/lib/api/notification_settings.rb
+++ b/lib/api/notification_settings.rb
@@ -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, current_user, notification_email: new_notification_email).execute
end
notification_setting.update(declared_params(include_missing: false))
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 77ac24ec68d..b157940eaa6 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -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, user_params.except(:extern_uid, :provider)).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, user, declared_params(include_missing: false)).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, 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, current_user, declared_params).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, current_user, email: email.email).execute
end
current_user.update_secondary_emails!
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index fb68627dedf..4e75729ca73 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -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, last_credential_check_at: Time.now).execute
true
else
diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb
index 7704bf715e4..eb32e41c995 100644
--- a/lib/gitlab/o_auth/user.rb
+++ b/lib/gitlab/o_auth/user.rb
@@ -32,7 +32,7 @@ module Gitlab
block_after_save = needs_blocking?
- Users::UpdateService.new(gl_user).execute!
+ Users::UpdateService.new(gl_user, gl_user).execute!
gl_user.block if block_after_save