diff options
author | James Lopez <james@jameslopez.es> | 2017-06-23 18:11:31 +0300 |
---|---|---|
committer | James Lopez <james@jameslopez.es> | 2017-06-23 18:11:31 +0300 |
commit | b33c638483d6b87ba71a329275ff12e5eb865d72 (patch) | |
tree | af7f0c3caac54bad865e651654772d575d9d0e4b | |
parent | 8f2adb8084c15026115aed39a06e9af04c5e7957 (diff) |
update code based on feedback
-rw-r--r-- | app/controllers/admin/users_controller.rb | 6 | ||||
-rw-r--r-- | app/controllers/profiles/two_factor_auths_controller.rb | 4 | ||||
-rw-r--r-- | app/services/users/build_service.rb | 1 | ||||
-rw-r--r-- | app/services/users/create_service.rb | 1 | ||||
-rw-r--r-- | app/services/users/update_service.rb | 5 | ||||
-rw-r--r-- | lib/api/internal.rb | 6 | ||||
-rw-r--r-- | spec/services/users/update_service_spec.rb | 6 |
7 files changed, 14 insertions, 15 deletions
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 3b90cd77be0..7b65836eef7 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -210,10 +210,8 @@ class Admin::UsersController < Admin::ApplicationController ] end - def update_user - result = Users::UpdateService.new(user).execute do |user| - yield(user) - end + def update_user(&block) + result = Users::UpdateService.new(user).execute(&block) result[:status] == :success end diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 1a4f77639e7..a864f86f3dd 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -41,8 +41,10 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController def create if current_user.validate_and_consume_otp!(params[:pin_code]) + codes = nil + Users::UpdateService.new(current_user, otp_required_for_login: true).execute! do |user| - @codes = user.generate_otp_backup_codes! + codes = user.generate_otp_backup_codes! end render 'create' diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 363135ef09b..ff234a3440f 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -1,5 +1,4 @@ module Users - # Service for building a new user. class BuildService < BaseService def initialize(current_user, params = {}) @current_user = current_user diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index e22f7225ae2..74abc017cea 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -1,5 +1,4 @@ module Users - # Service for creating a new user. class CreateService < BaseService def initialize(current_user, params = {}) @current_user = current_user diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index 2037664f56a..dfbd6016c3f 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -1,5 +1,4 @@ module Users - # Service for updating a user. class UpdateService < BaseService def initialize(user, params = {}) @user = user @@ -7,6 +6,8 @@ module Users end def execute(validate: true, &block) + yield(@user) if block_given? + assign_attributes(&block) if @user.save(validate: validate) @@ -27,8 +28,6 @@ module Users private def assign_attributes(&block) - yield(@user) if block_given? - @user.assign_attributes(params) if params.any? end end diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 9b035808693..f1c79970ba4 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -132,11 +132,13 @@ module API return { success: false, message: 'Two-factor authentication is not enabled for this user' } end + codes = nil + ::Users::UpdateService.new(user).execute! do |user| - @codes = user.generate_otp_backup_codes! + codes = user.generate_otp_backup_codes! end - { success: true, recovery_codes: @codes } + { success: true, recovery_codes: codes } end post "/notify_post_receive" do diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 6c33a232cb0..0b2f840c462 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -7,7 +7,7 @@ describe Users::UpdateService, services: true do it 'updates the name' do result = update_user(user, name: 'New Name') - expect(result).to eq({ status: :success }) + expect(result).to eq(status: :success) expect(user.name).to eq('New Name') end @@ -30,9 +30,9 @@ describe Users::UpdateService, services: true do expect(user.name).to eq('New Name') end - it 'returns an error result when record cannot be updated' do + it 'raises an error when record cannot be updated' do expect do - update_user(user, { email: 'invalid' }) + update_user(user, email: 'invalid') end.to raise_error(ActiveRecord::RecordInvalid) end |