Commit 67d06dee authored by James Lopez's avatar James Lopez

refactor users update service

parent cbb90d8f
...@@ -128,7 +128,7 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -128,7 +128,7 @@ class Admin::UsersController < Admin::ApplicationController
end end
respond_to do |format| respond_to do |format|
result = Users::UpdateService.new(current_user, 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! user.skip_reconfirmation!
end end
...@@ -219,7 +219,7 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -219,7 +219,7 @@ class Admin::UsersController < Admin::ApplicationController
end end
def update_user(&block) def update_user(&block)
result = Users::UpdateService.new(current_user, user).execute(&block) result = Users::UpdateService.new(current_user, user: user).execute(&block)
result[:status] == :success result[:status] == :success
end end
......
...@@ -2,7 +2,7 @@ class Profiles::AvatarsController < Profiles::ApplicationController ...@@ -2,7 +2,7 @@ class Profiles::AvatarsController < Profiles::ApplicationController
def destroy def destroy
@user = current_user @user = current_user
Users::UpdateService.new(current_user, @user).execute { |user| user.remove_avatar! } Users::UpdateService.new(current_user, user: @user).execute { |user| user.remove_avatar! }
redirect_to profile_path, status: 302 redirect_to profile_path, status: 302
end end
......
...@@ -7,7 +7,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController ...@@ -7,7 +7,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController
end end
def update def update
result = Users::UpdateService.new(current_user, current_user, user_params).execute result = Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute
if result[:status] == :success if result[:status] == :success
flash[:notice] = "Notification settings saved" flash[:notice] = "Notification settings saved"
......
...@@ -21,10 +21,10 @@ class Profiles::PasswordsController < Profiles::ApplicationController ...@@ -21,10 +21,10 @@ class Profiles::PasswordsController < Profiles::ApplicationController
password_automatically_set: false password_automatically_set: false
} }
result = Users::UpdateService.new(current_user, @user, password_attributes).execute result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute
if result[:status] == :success if result[:status] == :success
Users::UpdateService.new(current_user, @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' redirect_to root_path, notice: 'Password successfully changed'
else else
...@@ -46,7 +46,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController ...@@ -46,7 +46,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
return return
end end
result = Users::UpdateService.new(current_user, @user, password_attributes).execute result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute
if result[:status] == :success if result[:status] == :success
flash[:notice] = "Password was successfully updated. Please login with it" flash[:notice] = "Password was successfully updated. Please login with it"
......
...@@ -6,7 +6,7 @@ class Profiles::PreferencesController < Profiles::ApplicationController ...@@ -6,7 +6,7 @@ class Profiles::PreferencesController < Profiles::ApplicationController
def update def update
begin begin
result = Users::UpdateService.new(current_user, user, preferences_params).execute result = Users::UpdateService.new(current_user, preferences_params.merge(user: user)).execute
if result[:status] == :success if result[:status] == :success
flash[:notice] = 'Preferences saved.' flash[:notice] = 'Preferences saved.'
......
...@@ -10,7 +10,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController ...@@ -10,7 +10,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
current_user.otp_grace_period_started_at = Time.current current_user.otp_grace_period_started_at = Time.current
end end
Users::UpdateService.new(current_user, current_user).execute! Users::UpdateService.new(current_user, user: current_user).execute!
if two_factor_authentication_required? && !current_user.two_factor_enabled? if two_factor_authentication_required? && !current_user.two_factor_enabled?
two_factor_authentication_reason( two_factor_authentication_reason(
...@@ -41,7 +41,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController ...@@ -41,7 +41,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
def create def create
if current_user.validate_and_consume_otp!(params[:pin_code]) if current_user.validate_and_consume_otp!(params[:pin_code])
Users::UpdateService.new(current_user, 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! @codes = user.generate_otp_backup_codes!
end end
...@@ -70,7 +70,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController ...@@ -70,7 +70,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
end end
def codes def codes
Users::UpdateService.new(current_user, current_user).execute! do |user| Users::UpdateService.new(current_user, user: current_user).execute! do |user|
@codes = user.generate_otp_backup_codes! @codes = user.generate_otp_backup_codes!
end end
end end
......
...@@ -10,7 +10,7 @@ class ProfilesController < Profiles::ApplicationController ...@@ -10,7 +10,7 @@ class ProfilesController < Profiles::ApplicationController
def update def update
respond_to do |format| respond_to do |format|
result = Users::UpdateService.new(current_user, @user, user_params).execute result = Users::UpdateService.new(current_user, user_params.merge(user: @user)).execute
if result[:status] == :success if result[:status] == :success
message = "Profile was successfully updated" message = "Profile was successfully updated"
...@@ -25,7 +25,7 @@ class ProfilesController < Profiles::ApplicationController ...@@ -25,7 +25,7 @@ class ProfilesController < Profiles::ApplicationController
end end
def reset_private_token def reset_private_token
Users::UpdateService.new(current_user, @user).execute! do |user| Users::UpdateService.new(current_user, user: @user).execute! do |user|
user.reset_authentication_token! user.reset_authentication_token!
end end
...@@ -35,7 +35,7 @@ class ProfilesController < Profiles::ApplicationController ...@@ -35,7 +35,7 @@ class ProfilesController < Profiles::ApplicationController
end end
def reset_incoming_email_token def reset_incoming_email_token
Users::UpdateService.new(current_user, @user).execute! do |user| Users::UpdateService.new(current_user, user: @user).execute! do |user|
user.reset_incoming_email_token! user.reset_incoming_email_token!
end end
...@@ -45,7 +45,7 @@ class ProfilesController < Profiles::ApplicationController ...@@ -45,7 +45,7 @@ class ProfilesController < Profiles::ApplicationController
end end
def reset_rss_token def reset_rss_token
Users::UpdateService.new(current_user, @user).execute! do |user| Users::UpdateService.new(current_user, user: @user).execute! do |user|
user.reset_rss_token! user.reset_rss_token!
end end
...@@ -61,7 +61,7 @@ class ProfilesController < Profiles::ApplicationController ...@@ -61,7 +61,7 @@ class ProfilesController < Profiles::ApplicationController
end end
def update_username def update_username
result = Users::UpdateService.new(current_user, @user, username: user_params[:username]).execute result = Users::UpdateService.new(current_user, user: @user, username: user_params[:username]).execute
options = if result[:status] == :success options = if result[:status] == :success
{ notice: "Username successfully changed" } { notice: "Username successfully changed" }
......
...@@ -55,7 +55,7 @@ class SessionsController < Devise::SessionsController ...@@ -55,7 +55,7 @@ class SessionsController < Devise::SessionsController
return unless user && user.require_password_creation? return unless user && user.require_password_creation?
Users::UpdateService.new(current_user, user).execute do |user| Users::UpdateService.new(current_user, user: user).execute do |user|
@token = user.generate_reset_token @token = user.generate_reset_token
end end
......
...@@ -60,7 +60,7 @@ class User < ActiveRecord::Base ...@@ -60,7 +60,7 @@ class User < ActiveRecord::Base
lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i) lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i)
return unless lease.try_obtain return unless lease.try_obtain
Users::UpdateService.new(self, self).execute(validate: false) Users::UpdateService.new(self, user: self).execute(validate: false)
end end
attr_accessor :force_random_password attr_accessor :force_random_password
...@@ -1000,7 +1000,7 @@ class User < ActiveRecord::Base ...@@ -1000,7 +1000,7 @@ class User < ActiveRecord::Base
if attempts_exceeded? if attempts_exceeded?
lock_access! unless access_locked? lock_access! unless access_locked?
else else
Users::UpdateService.new(self, self).execute(validate: false) Users::UpdateService.new(self, user: self).execute(validate: false)
end end
end end
...@@ -1186,7 +1186,7 @@ class User < ActiveRecord::Base ...@@ -1186,7 +1186,7 @@ class User < ActiveRecord::Base
&creation_block &creation_block
) )
Users::UpdateService.new(user, user).execute(validate: false) Users::UpdateService.new(user, user: user).execute(validate: false)
user user
ensure ensure
Gitlab::ExclusiveLease.cancel(lease_key, uuid) Gitlab::ExclusiveLease.cancel(lease_key, uuid)
......
...@@ -7,7 +7,7 @@ module Emails ...@@ -7,7 +7,7 @@ module Emails
private private
def update_secondary_emails! def update_secondary_emails!
result = ::Users::UpdateService.new(@current_user, @user).execute do |user| result = ::Users::UpdateService.new(@current_user, user: @user).execute do |user|
user.update_secondary_emails! user.update_secondary_emails!
end end
......
...@@ -2,9 +2,9 @@ module Users ...@@ -2,9 +2,9 @@ module Users
class UpdateService < BaseService class UpdateService < BaseService
include NewUserNotifier include NewUserNotifier
def initialize(current_user, user, params = {}) def initialize(current_user, params = {})
@current_user = current_user @current_user = current_user
@user = user @user = params[:user]
@params = params.dup @params = params.dup
end end
......
...@@ -136,7 +136,7 @@ module API ...@@ -136,7 +136,7 @@ module API
codes = nil codes = nil
::Users::UpdateService.new(current_user, user).execute! do |user| ::Users::UpdateService.new(current_user, user: user).execute! do |user|
codes = user.generate_otp_backup_codes! codes = user.generate_otp_backup_codes!
end end
......
...@@ -35,7 +35,7 @@ module API ...@@ -35,7 +35,7 @@ module API
new_notification_email = params.delete(:notification_email) new_notification_email = params.delete(:notification_email)
if new_notification_email if new_notification_email
::Users::UpdateService.new(current_user, current_user, notification_email: new_notification_email).execute ::Users::UpdateService.new(current_user, user: current_user, notification_email: new_notification_email).execute
end end
notification_setting.update(declared_params(include_missing: false)) notification_setting.update(declared_params(include_missing: false))
......
...@@ -166,7 +166,7 @@ module API ...@@ -166,7 +166,7 @@ module API
user_params[:password_expires_at] = Time.now if user_params[:password].present? user_params[:password_expires_at] = Time.now if user_params[:password].present?
result = ::Users::UpdateService.new(current_user, 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 if result[:status] == :success
present user, with: Entities::UserPublic present user, with: Entities::UserPublic
......
...@@ -16,7 +16,7 @@ module Gitlab ...@@ -16,7 +16,7 @@ module Gitlab
def self.allowed?(user) def self.allowed?(user)
self.open(user) do |access| self.open(user) do |access|
if access.allowed? if access.allowed?
Users::UpdateService.new(user, user, last_credential_check_at: Time.now).execute Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute
true true
else else
......
...@@ -32,7 +32,7 @@ module Gitlab ...@@ -32,7 +32,7 @@ module Gitlab
block_after_save = needs_blocking? block_after_save = needs_blocking?
Users::UpdateService.new(gl_user, gl_user).execute! Users::UpdateService.new(gl_user, user: gl_user).execute!
gl_user.block if block_after_save gl_user.block if block_after_save
......
...@@ -31,13 +31,13 @@ describe Users::UpdateService do ...@@ -31,13 +31,13 @@ describe Users::UpdateService do
end end
def update_user(user, opts) def update_user(user, opts)
described_class.new(user, user, opts).execute described_class.new(user, opts.merge(user: user)).execute
end end
end end
describe '#execute!' do describe '#execute!' do
it 'updates the name' do it 'updates the name' do
service = described_class.new(user, user, name: 'New Name') service = described_class.new(user, user: user, name: 'New Name')
expect(service).not_to receive(:notify_new_user) expect(service).not_to receive(:notify_new_user)
result = service.execute! result = service.execute!
...@@ -55,7 +55,7 @@ describe Users::UpdateService do ...@@ -55,7 +55,7 @@ describe Users::UpdateService do
it 'fires system hooks when a new user is saved' do it 'fires system hooks when a new user is saved' do
system_hook_service = spy(:system_hook_service) system_hook_service = spy(:system_hook_service)
user = build(:user) user = build(:user)
service = described_class.new(user, 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(:notify_new_user).and_call_original
expect(service).to receive(:system_hook_service).and_return(system_hook_service) expect(service).to receive(:system_hook_service).and_return(system_hook_service)
...@@ -65,7 +65,7 @@ describe Users::UpdateService do ...@@ -65,7 +65,7 @@ describe Users::UpdateService do
end end
def update_user(user, opts) def update_user(user, opts)
described_class.new(user, user, opts).execute! described_class.new(user, opts.merge(user: user)).execute!
end end
end end
end end
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