more refactoring

app/controllers/profiles/avatars_controller.rb

@@ -2,9 +2,7 @@ class Profiles::AvatarsController < Profiles::ApplicationController
   def destroy
     @user = current_user
 
-    Users::UpdateService.new(@user, @user).execute do |user|
-      user.remove_avatar!
-    end
+    Users::UpdateService.new(@user, @user).execute { |user| user.remove_avatar! }
 
     redirect_to profile_path, status: 302
   end

app/controllers/profiles/emails_controller.rb

@@ -5,9 +5,9 @@ class Profiles::EmailsController < Profiles::ApplicationController
   end
 
   def create
-    @email = current_user.emails.new(email_params)
+    @email = Emails::CreateService.new(current_user, current_user, email_params).execute
 
-    if Emails::CreateService.new(current_user, current_user, email_params).execute
+    if @email.errors.blank?
       NotificationService.new.new_email(@email)
     else
       flash[:alert] = @email.errors.full_messages.first
@@ -18,6 +18,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
 
   def destroy
     @email = current_user.emails.find(params[:id])
+
     Emails::DestroyService.new(current_user, current_user, email: @email.email).execute
 
     respond_to do |format|

app/controllers/profiles/two_factor_auths_controller.rb

@@ -41,8 +41,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
 
   def create
     if current_user.validate_and_consume_otp!(params[:pin_code])
-      Users::UpdateService.new(current_user, current_user).execute! do |user|
-        user.otp_required_for_login = true
+      Users::UpdateService.new(current_user, current_user, otp_required_for_login: true).execute! do |user|
         @codes = user.generate_otp_backup_codes!
       end
 

app/services/users/update_service.rb

@@ -17,10 +17,12 @@ module Users
       end
     end
 
-    def execute!(skip_authorization: false, &block)
+    def execute!(*args, &block)
       result = execute(*args, &block)
 
-      raise SomeCustomException(result[:message]) unless result[:status] == :success
+      raise ActiveRecord::RecordInvalid(result[:message]) unless result[:status] == :success
+
+      true
     end
 
     private

spec/services/emails/create_service_spec.rb

@@ -17,15 +17,5 @@ describe Emails::CreateService, services: true do
 
       expect(user.emails).to eq(Email.where(opts))
     end
-
-    it 'does not create an email if the user has no permissions' do
-      expect { described_class.new(create(:user), user, opts).execute }.to raise_error(Gitlab::Access::AccessDeniedError)
-    end
-
-    it 'creates an email if we skip authorization' do
-      expect do
-        described_class.new(create(:user), user, opts).execute(skip_authorization: true)
-      end.to change { Email.count }.by(1)
-    end
   end
 end

spec/services/emails/destroy_service_spec.rb

@@ -10,17 +10,5 @@ describe Emails::DestroyService, services: true do
     it 'removes an email' do
       expect { service.execute }.to change { user.emails.count }.by(-1)
     end
-
-    it 'does not remove an email if the user has no permissions' do
-      expect do
-        described_class.new(create(:user), user, email: email.email).execute
-      end.to raise_error(Gitlab::Access::AccessDeniedError)
-    end
-
-    it 'removes an email if we skip authorization' do
-      expect do
-        described_class.new(create(:user), user, email: email.email).execute(skip_authorization: true)
-      end.to change { Email.count }.by(-1)
-    end
   end
 end