added users update service

fix spec

added bang option to spec

use update service on ldap call and updated specs and service

update profiles controller to use new service

refactor profiles controller and update service

update users controller

finish off refactoring users controller

fix profiles controller

updated emails, notifications and passwords controller

update preferences controller

update notification settings, fix api specs

fix specs

fix api and controller issues

added service in the rest of controllers and classes

add emails service specs

add more spec examples

add create and destroy services for emails

fixed specs

update to use emails destroy service

fix specs

update missing email actions

add missing user updates

refactor emails service

more refactoring based on feedback

add missing action to block

more refactoring

fix spec failures

fix specs

refactor update user service not to do auth checks

app/controllers/admin/users_controller.rb

@@ -54,7 +54,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def block
-    if user.block
+    if update_user { |user| user.block }
       redirect_back_or_admin_user(notice: "Successfully blocked")
     else
       redirect_back_or_admin_user(alert: "Error occurred. User was not blocked")
@@ -64,7 +64,7 @@ class Admin::UsersController < Admin::ApplicationController
   def unblock
     if user.ldap_blocked?
       redirect_back_or_admin_user(alert: "This user cannot be unlocked manually from GitLab")
-    elsif user.activate
+    elsif update_user { |user| user.activate }
       redirect_back_or_admin_user(notice: "Successfully unblocked")
     else
       redirect_back_or_admin_user(alert: "Error occurred. User was not unblocked")
@@ -72,7 +72,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def unlock
-    if user.unlock_access!
+    if update_user { |user| user.unlock_access! }
       redirect_back_or_admin_user(alert: "Successfully unlocked")
     else
       redirect_back_or_admin_user(alert: "Error occurred. User was not unlocked")
@@ -80,7 +80,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def confirm
-    if user.confirm
+    if update_user { |user| user.confirm }
       redirect_back_or_admin_user(notice: "Successfully confirmed")
     else
       redirect_back_or_admin_user(alert: "Error occurred. User was not confirmed")
@@ -88,7 +88,8 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def disable_two_factor
-    user.disable_two_factor!
+    update_user { |user| user.disable_two_factor! }
+
     redirect_to admin_user_path(user),
       notice: 'Two-factor Authentication has been disabled for this user'
   end
@@ -124,15 +125,18 @@ class Admin::UsersController < Admin::ApplicationController
     end
 
     respond_to do |format|
-      user.skip_reconfirmation!
-      if user.update_attributes(user_params_with_pass)
+      result = Users::UpdateService.new(user, user_params_with_pass).execute do |user|
+        user.skip_reconfirmation!
+      end
+
+      if result[:status] == :success
         format.html { redirect_to [:admin, user], notice: 'User was successfully updated.' }
         format.json { head :ok }
       else
         # restore username to keep form action url.
         user.username = params[:id]
         format.html { render "edit" }
-        format.json { render json: user.errors, status: :unprocessable_entity }
+        format.json { render json: [result[:message]], status: result[:status] }
       end
     end
   end
@@ -148,13 +152,16 @@ class Admin::UsersController < Admin::ApplicationController
 
   def remove_email
     email = user.emails.find(params[:email_id])
-    email.destroy
-
-    user.update_secondary_emails!
+    success = Emails::DestroyService.new(current_user, user, email: email.email).execute
 
     respond_to do |format|
-      format.html { redirect_back_or_admin_user(notice: "Successfully removed email.") }
-      format.js { head :ok }
+      if success
+        format.html { redirect_back_or_admin_user(notice: 'Successfully removed email.') }
+        format.json { head :ok }
+      else
+        format.html { redirect_back_or_admin_user(alert: 'There was an error removing the e-mail.') }
+        format.json { render json: 'There was an error removing the e-mail.', status: 400 }
+      end
     end
   end
 
@@ -209,4 +216,12 @@ class Admin::UsersController < Admin::ApplicationController
       namespace_attributes: [:id, :shared_runners_minutes_limit, :plan]
     ]
   end
+
+  def update_user
+    result = Users::UpdateService.new(user).execute do |user|
+      yield(user)
+    end
+
+    result[:status] == :success
+  end
 end

app/controllers/profiles/avatars_controller.rb

 class Profiles::AvatarsController < Profiles::ApplicationController
   def destroy
     @user = current_user
-    @user.remove_avatar!
 
-    @user.save
+    Users::UpdateService.new(@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 @email.save
+    if @email.errors.blank?
       NotificationService.new.new_email(@email)
     else
       flash[:alert] = @email.errors.full_messages.first
@@ -18,9 +18,8 @@ class Profiles::EmailsController < Profiles::ApplicationController
 
   def destroy
     @email = current_user.emails.find(params[:id])
-    @email.destroy
 
-    current_user.update_secondary_emails!
+    Emails::DestroyService.new(current_user, current_user, email: @email.email).execute
 
     respond_to do |format|
       format.html { redirect_to profile_emails_url, status: 302 }

app/controllers/profiles/notifications_controller.rb

@@ -7,7 +7,9 @@ class Profiles::NotificationsController < Profiles::ApplicationController
   end
 
   def update
-    if current_user.update_attributes(user_params)
+    result = Users::UpdateService.new(current_user, user_params).execute
+
+    if result[:status] == :success
       flash[:notice] = "Notification settings saved"
     else
       flash[:alert] = "Failed to save new settings"

app/controllers/profiles/passwords_controller.rb

@@ -15,17 +15,17 @@ class Profiles::PasswordsController < Profiles::ApplicationController
       return
     end
 
-    new_password = user_params[:password]
-    new_password_confirmation = user_params[:password_confirmation]
-
-    result = @user.update_attributes(
-      password: new_password,
-      password_confirmation: new_password_confirmation,
+    password_attributes = {
+      password: user_params[:password],
+      password_confirmation: user_params[:password_confirmation],
       password_automatically_set: false
-    )
+    }
+
+    result = Users::UpdateService.new(@user, password_attributes).execute
+
+    if result[:status] == :success
+      Users::UpdateService.new(@user, password_expires_at: nil).execute
 
-    if result
-      @user.update_attributes(password_expires_at: nil)
       redirect_to root_path, notice: 'Password successfully changed'
     else
       render :new
@@ -46,7 +46,9 @@ class Profiles::PasswordsController < Profiles::ApplicationController
       return
     end
 
-    if @user.update_attributes(password_attributes)
+    result = Users::UpdateService.new(@user, password_attributes).execute
+
+    if result[:status] == :success
       flash[:notice] = "Password was successfully updated. Please login with it"
       redirect_to new_user_session_path
     else

app/controllers/profiles/preferences_controller.rb

@@ -6,7 +6,9 @@ class Profiles::PreferencesController < Profiles::ApplicationController
 
   def update
     begin
-      if @user.update_attributes(preferences_params)
+      result = Users::UpdateService.new(user, preferences_params).execute
+
+      if result[:status] == :success
         flash[:notice] = 'Preferences saved.'
       else
         flash[:alert] = 'Failed to save preferences.'

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
 
-    current_user.save! if current_user.changed?
+    Users::UpdateService.new(current_user).execute!
 
     if two_factor_authentication_required? && !current_user.two_factor_enabled?
       two_factor_authentication_reason(
@@ -41,9 +41,9 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
 
   def create
     if current_user.validate_and_consume_otp!(params[:pin_code])
-      current_user.otp_required_for_login = true
-      @codes = current_user.generate_otp_backup_codes!
-      current_user.save!
+      Users::UpdateService.new(current_user, otp_required_for_login: true).execute! do |user|
+        @codes = user.generate_otp_backup_codes!
+      end
 
       render 'create'
     else
@@ -70,8 +70,9 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
   end
 
   def codes
-    @codes = current_user.generate_otp_backup_codes!
-    current_user.save!
+    Users::UpdateService.new(current_user).execute! do |user|
+      @codes = user.generate_otp_backup_codes!
+    end
   end
 
   def destroy

app/controllers/profiles_controller.rb

@@ -12,39 +12,47 @@ class ProfilesController < Profiles::ApplicationController
     user_params.except!(:email) if @user.external_email?
 
     respond_to do |format|
-      if @user.update_attributes(user_params)
+      result = Users::UpdateService.new(@user, user_params).execute
+
+      if result[:status] == :success
         message = "Profile was successfully updated"
+
         format.html { redirect_back_or_default(default: { action: 'show' }, options: { notice: message }) }
         format.json { render json: { message: message } }
       else
-        message = @user.errors.full_messages.uniq.join('. ')
-        format.html { redirect_back_or_default(default: { action: 'show' }, options: { alert: "Failed to update profile. #{message}" }) }
-        format.json { render json: { message: message }, status: :unprocessable_entity }
+        format.html { redirect_back_or_default(default: { action: 'show' }, options: { alert: result[:message] }) }
+        format.json { render json: result }
       end
     end
   end
 
   def reset_private_token
-    if current_user.reset_authentication_token!
-      flash[:notice] = "Private token was successfully reset"
+    Users::UpdateService.new(@user).execute!(skip_authorization: true) do |user|
+      user.reset_authentication_token!
     end
 
+    flash[:notice] = "Private token was successfully reset"
+
     redirect_to profile_account_path
   end
 
   def reset_incoming_email_token
-    if current_user.reset_incoming_email_token!
-      flash[:notice] = "Incoming email token was successfully reset"
+    Users::UpdateService.new(@user).execute!(skip_authorization: true) do |user|
+      user.reset_incoming_email_token!
     end
 
+    flash[:notice] = "Incoming email token was successfully reset"
+
     redirect_to profile_account_path
   end
 
   def reset_rss_token
-    if current_user.reset_rss_token!
-      flash[:notice] = "RSS token was successfully reset"
+    Users::UpdateService.new(@user).execute!(skip_authorization: true) do |user|
+      user.reset_rss_token!
     end
 
+    flash[:notice] = "RSS token was successfully reset"
+
     redirect_to profile_account_path
   end
 
@@ -55,12 +63,13 @@ class ProfilesController < Profiles::ApplicationController
   end
 
   def update_username
-    if @user.update_attributes(username: user_params[:username])
-      options = { notice: "Username successfully changed" }
-    else
-      message = @user.errors.full_messages.uniq.join('. ')
-      options = { alert: "Username change failed - #{message}" }
-    end
+    result = Users::UpdateService.new(@user, username: user_params[:username]).execute
+
+    options = if result[:status] == :success
+                { notice: "Username successfully changed" }
+              else
+                { alert: "Username change failed - #{result[:message]}" }
+              end
 
     redirect_back_or_default(default: { action: 'show' }, options: options)
   end

app/controllers/sessions_controller.rb

@@ -61,10 +61,11 @@ class SessionsController < Devise::SessionsController
 
     return unless user && user.require_password?
 
-    token = user.generate_reset_token
-    user.save
+    Users::UpdateService.new(user).execute do |user|
+      @token = user.generate_reset_token
+    end
 
-    redirect_to edit_user_password_path(reset_password_token: token),
+    redirect_to edit_user_password_path(reset_password_token: @token),
       notice: "Please create a password for your new account."
   end
 

app/models/user.rb

@@ -55,7 +55,7 @@ class User < ActiveRecord::Base
     lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i)
     return unless lease.try_obtain
 
-    save(validate: false)
+    Users::UpdateService.new(self).execute(validate: false)
   end
 
   attr_accessor :force_random_password
@@ -521,10 +521,8 @@ class User < ActiveRecord::Base
   def update_emails_with_primary_email
     primary_email_record = emails.find_by(email: email)
     if primary_email_record
-      primary_email_record.destroy
-      emails.create(email: email_was)
-
-      update_secondary_emails!
+      Emails::DestroyService.new(self, self, email: email).execute
+      Emails::CreateService.new(self, self, email: email_was).execute
     end
   end
 
@@ -996,7 +994,7 @@ class User < ActiveRecord::Base
     if attempts_exceeded?
       lock_access! unless access_locked?
     else
-      save(validate: false)
+      Users::UpdateService.new(self).execute(validate: false)
     end
   end
 
@@ -1159,7 +1157,8 @@ class User < ActiveRecord::Base
       email: email,
       &creation_block
     )
-    user.save(validate: false)
+
+    Users::UpdateService.new(user).execute(validate: false)
     user
   ensure
     Gitlab::ExclusiveLease.cancel(lease_key, uuid)

app/services/emails/base_service.rb

+module Emails
+  class BaseService
+    def initialize(current_user, user, opts)
+      @current_user = current_user
+      @user = user
+      @email = opts[:email]
+    end
+  end
+end

app/services/emails/create_service.rb

+module Emails
+  class CreateService < ::Emails::BaseService
+    def execute
+      @user.emails.create(email: @email)
+    end
+  end
+end

app/services/emails/destroy_service.rb

+module Emails
+  class DestroyService < ::Emails::BaseService
+    def execute
+      Email.find_by_email!(@email).destroy && update_secondary_emails!
+    end
+
+    private
+
+    def update_secondary_emails!
+      result = ::Users::UpdateService.new(@current_user).execute do |user|
+        user.update_secondary_emails!
+      end
+
+      result[:status] == 'success'
+    end
+  end
+end

app/services/users/update_service.rb

+module Users
+  # Service for updating a user.
+  class UpdateService < BaseService
+    def initialize(user, params = {})
+      @user = user
+      @params = params.dup
+    end
+
+    def execute(validate: true, &block)
+      assign_attributes(&block)
+
+      if @user.save(validate: validate)
+        success
+      else
+        error(@user.errors.full_messages.uniq.join('. '))
+      end
+    end
+
+    def execute!(*args, &block)
+      result = execute(*args, &block)
+
+      raise ActiveRecord::RecordInvalid.new(@user) unless result[:status] == :success
+
+      true
+    end
+
+    private
+
+    def assign_attributes(&block)
+      yield(@user) if block_given?
+
+      @user.assign_attributes(params) if params.any?
+    end
+  end
+end

lib/api/internal.rb

@@ -144,10 +144,11 @@ module API
           return { success: false, message: 'Two-factor authentication is not enabled for this user' }
         end
 
-        codes = user.generate_otp_backup_codes!
-        user.save!
+        ::Users::UpdateService.new(user).execute! do |user|
+          @codes = user.generate_otp_backup_codes!
+        end
 
-        { success: true, recovery_codes: codes }
+        { success: true, recovery_codes: @codes }
       end
 
       post "/notify_post_receive" do

lib/api/notification_settings.rb

@@ -34,7 +34,10 @@ module API
           notification_setting.transaction do
             new_notification_email = params.delete(:notification_email)
 
-            current_user.update(notification_email: new_notification_email) if new_notification_email
+            if new_notification_email
+              ::Users::UpdateService.new(current_user, notification_email: new_notification_email).execute
+            end
+
             notification_setting.update(declared_params(include_missing: false))
           end
         rescue ArgumentError => e # catch level enum error

lib/api/users.rb

@@ -104,7 +104,7 @@ module API
         authenticated_as_admin!
 
         params = declared_params(include_missing: false)
-        user = ::Users::CreateService.new(current_user, params).execute
+        user = ::Users::CreateService.new(current_user, params).execute(skip_authorization: true)
 
         if user.persisted?
           present user, with: Entities::UserPublic
@@ -162,7 +162,9 @@ module API
 
         user_params[:password_expires_at] = Time.now if user_params[:password].present?
 
-        if user.update_attributes(user_params.except(:extern_uid, :provider))
+        result = ::Users::UpdateService.new(user, user_params.except(:extern_uid, :provider)).execute
+
+        if result[:status] == :success
           present user, with: Entities::UserPublic
         else
           render_validation_error!(user)
@@ -240,9 +242,9 @@ module API
         user = User.find_by(id: params.delete(:id))
         not_found!('User') unless user
 
-        email = user.emails.new(declared_params(include_missing: false))
+        email = Emails::CreateService.new(current_user, user, declared_params(include_missing: false)).execute
 
-        if email.save
+        if email.errors.blank?
           NotificationService.new.new_email(email)
           present email, with: Entities::Email
         else
@@ -280,8 +282,7 @@ module API
         email = user.emails.find_by(id: params[:email_id])
         not_found!('Email') unless email
 
-        email.destroy
-        user.update_secondary_emails!
+        Emails::DestroyService.new(current_user, user, email: email.email).execute
       end
 
       desc 'Delete a user. Available only for admins.' do
@@ -493,9 +494,9 @@ module API
         requires :email, type: String, desc: 'The new email'
       end
       post "emails" do
-        email = current_user.emails.new(declared_params)
+        email = Emails::CreateService.new(current_user, current_user, declared_params).execute
 
-        if email.save
+        if email.errors.blank?
           NotificationService.new.new_email(email)
           present email, with: Entities::Email
         else
@@ -511,8 +512,7 @@ module API
         email = current_user.emails.find_by(id: params[:email_id])
         not_found!('Email') unless email
 
-        email.destroy
-        current_user.update_secondary_emails!
+        Emails::DestroyService.new(current_user, current_user, email: email.email).execute
       end
 
       desc 'Get a list of user activities'

lib/gitlab/ldap/access.rb

@@ -20,8 +20,8 @@ module Gitlab
           # permissions to keep things clean
           if access.allowed?
             access.update_user
-            user.last_credential_check_at = Time.now
-            user.save
+            Users::UpdateService.new(user, last_credential_check_a: Time.now).execute
+
             true
           else
             false

lib/gitlab/o_auth/user.rb

@@ -32,7 +32,7 @@ module Gitlab
 
         block_after_save = needs_blocking?
 
-        gl_user.save!
+        Users::UpdateService.new(gl_user).execute!
 
         gl_user.block if block_after_save
 

spec/controllers/profiles/preferences_controller_spec.rb

@@ -43,7 +43,8 @@ describe Profiles::PreferencesController do
           dashboard: 'stars'
         }.with_indifferent_access
 
-        expect(user).to receive(:update_attributes).with(prefs)
+        expect(user).to receive(:assign_attributes).with(prefs)
+        expect(user).to receive(:save)
 
         go params: prefs
       end
@@ -51,7 +52,7 @@ describe Profiles::PreferencesController do
 
     context 'on failed update' do
       it 'sets the flash' do
-        expect(user).to receive(:update_attributes).and_return(false)
+        expect(user).to receive(:save).and_return(false)
 
         go
 

spec/features/profiles/password_spec.rb

@@ -25,7 +25,7 @@ describe 'Profile > Password', feature: true do
         end
       end
 
-      it 'does not contains the current password field after an error' do
+      it 'does not contain the current password field after an error' do
         fill_passwords('mypassword', 'mypassword2')
 
         expect(page).to have_no_field('user[current_password]')

spec/requests/api/users_spec.rb

@@ -376,6 +376,7 @@ describe API::Users do
 
     it "updates user with new bio" do
       put api("/users/#{user.id}", admin), { bio: 'new test bio' }
+
       expect(response).to have_http_status(200)
       expect(json_response['bio']).to eq('new test bio')
       expect(user.reload.bio).to eq('new test bio')
@@ -408,13 +409,22 @@ describe API::Users do
 
     it 'updates user with his own email' do
       put api("/users/#{user.id}", admin), email: user.email
+
       expect(response).to have_http_status(200)
       expect(json_response['email']).to eq(user.email)
       expect(user.reload.email).to eq(user.email)
     end
 
+    it 'updates user with a new email' do
+      put api("/users/#{user.id}", admin), email: 'new@email.com'
+
+      expect(response).to have_http_status(200)
+      expect(user.reload.notification_email).to eq('new@email.com')
+    end
+
     it 'updates user with his own username' do
       put api("/users/#{user.id}", admin), username: user.username
+
       expect(response).to have_http_status(200)
       expect(json_response['username']).to eq(user.username)
       expect(user.reload.username).to eq(user.username)
@@ -422,12 +432,14 @@ describe API::Users do
 
     it "updates user's existing identity" do
       put api("/users/#{omniauth_user.id}", admin), provider: 'ldapmain', extern_uid: '654321'
+
       expect(response).to have_http_status(200)
       expect(omniauth_user.reload.identities.first.extern_uid).to eq('654321')
     end
 
     it 'updates user with new identity' do
       put api("/users/#{user.id}", admin), provider: 'github', extern_uid: 'john'
+
       expect(response).to have_http_status(200)
       expect(user.reload.identities.first.extern_uid).to eq('john')
       expect(user.reload.identities.first.provider).to eq('github')
@@ -435,12 +447,14 @@ describe API::Users do
 
     it "updates admin status" do
       put api("/users/#{user.id}", admin), { admin: true }
+
       expect(response).to have_http_status(200)
       expect(user.reload.admin).to eq(true)
     end
 
     it "updates external status" do
       put api("/users/#{user.id}", admin), { external: true }
+
       expect(response.status).to eq 200
       expect(json_response['external']).to eq(true)
       expect(user.reload.external?).to be_truthy
@@ -459,6 +473,7 @@ describe API::Users do
 
     it "does not update admin status" do
       put api("/users/#{admin_user.id}", admin), { can_create_group: false }
+
       expect(response).to have_http_status(200)
       expect(admin_user.reload.admin).to eq(true)
       expect(admin_user.can_create_group).to eq(false)
@@ -466,6 +481,7 @@ describe API::Users do
 
     it "does not allow invalid update" do
       put api("/users/#{user.id}", admin), { email: 'invalid email' }
+
       expect(response).to have_http_status(400)
       expect(user.reload.email).not_to eq('invalid email')
     end
@@ -490,6 +506,7 @@ describe API::Users do
 
     it "returns 404 for non-existing user" do
       put api("/users/999999", admin), { bio: 'update should fail' }
+
       expect(response).to have_http_status(404)
       expect(json_response['message']).to eq('404 User Not Found')
     end
@@ -540,6 +557,7 @@ describe API::Users do
 
       it 'returns 409 conflict error if email address exists' do
         put api("/users/#{@user.id}", admin), email: 'test@example.com'
+
         expect(response).to have_http_status(409)
         expect(@user.reload.email).to eq(@user.email)
       end
@@ -547,6 +565,7 @@ describe API::Users do
       it 'returns 409 conflict error if username taken' do
         @user_id = User.all.last.id
         put api("/users/#{@user.id}", admin), username: 'test'
+
         expect(response).to have_http_status(409)
         expect(@user.reload.username).to eq(@user.username)
       end

spec/services/emails/create_service_spec.rb

+require 'spec_helper'
+
+describe Emails::CreateService, services: true do
+  let(:user) { create(:user) }
+  let(:opts) { { email: 'new@email.com' } }
+
+  subject(:service) { described_class.new(user, user, opts) }
+
+  describe '#execute' do
+    it 'creates an email with valid attributes' do
+      expect { service.execute }.to change { Email.count }.by(1)
+      expect(Email.where(opts)).not_to be_empty
+    end
+
+    it 'has the right user association' do
+      service.execute
+
+      expect(user.emails).to eq(Email.where(opts))
+    end
+  end
+end

spec/services/emails/destroy_service_spec.rb

+require 'spec_helper'
+
+describe Emails::DestroyService, services: true do
+  let!(:user) { create(:user) }
+  let!(:email) { create(:email, user: user) }
+
+  subject(:service) { described_class.new(user, user, email: email.email) }
+
+  describe '#execute' do
+    it 'removes an email' do
+      expect { service.execute }.to change { user.emails.count }.by(-1)
+    end
+  end
+end

spec/services/users/update_service_spec.rb

+require 'spec_helper'
+
+describe Users::UpdateService, services: true do
+  let(:user) { create(:user) }
+
+  describe '#execute' do
+    it 'updates the name' do
+      result = update_user(user, name: 'New Name')
+
+      expect(result).to eq({ status: :success })
+      expect(user.name).to eq('New Name')
+    end
+
+    it 'returns an error result when record cannot be updated' do
+      expect do
+        update_user(user, { email: 'invalid' })
+      end.not_to change { user.reload.email }
+    end
+
+    def update_user(user, opts)
+      described_class.new(user, opts).execute
+    end
+  end
+
+  describe '#execute!' do
+    it 'updates the name' do
+      result = update_user(user, name: 'New Name')
+
+      expect(result).to be true
+      expect(user.name).to eq('New Name')
+    end
+
+    it 'returns an error result when record cannot be updated' do
+      expect do
+        update_user(user, { email: 'invalid' })
+      end.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    def update_user(user, opts)
+      described_class.new(user, opts).execute!
+    end
+  end
+end