summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@mcgivern.me.uk>2017-06-26 11:42:44 +0000
committerSean McGivern <sean@mcgivern.me.uk>2017-06-26 11:42:44 +0000
commitbff82b784bac7af8cf3a3d8b49fa10ebd1ef67f0 (patch)
tree2b294a4f5f947843ea6afdb8f31338e2901f5e8f
parent739a03a825e5c04c7566bc3bb26314b049e403ec (diff)
parent4d70c479107fc502668a83fc369159062d8efbb8 (diff)
downloadgitlab-ce-bff82b784bac7af8cf3a3d8b49fa10ebd1ef67f0.tar.gz
Merge branch 'feature/add-new-services' into 'master'
Add additional user and email services See merge request !12125
-rw-r--r--app/controllers/admin/users_controller.rb39
-rw-r--r--app/controllers/profiles/avatars_controller.rb3
-rw-r--r--app/controllers/profiles/emails_controller.rb7
-rw-r--r--app/controllers/profiles/notifications_controller.rb4
-rw-r--r--app/controllers/profiles/passwords_controller.rb22
-rw-r--r--app/controllers/profiles/preferences_controller.rb4
-rw-r--r--app/controllers/profiles/two_factor_auths_controller.rb13
-rw-r--r--app/controllers/profiles_controller.rb41
-rw-r--r--app/controllers/sessions_controller.rb7
-rw-r--r--app/models/user.rb13
-rw-r--r--app/services/emails/base_service.rb8
-rw-r--r--app/services/emails/create_service.rb7
-rw-r--r--app/services/emails/destroy_service.rb17
-rw-r--r--app/services/users/build_service.rb1
-rw-r--r--app/services/users/create_service.rb1
-rw-r--r--app/services/users/update_service.rb34
-rw-r--r--lib/api/internal.rb7
-rw-r--r--lib/api/notification_settings.rb5
-rw-r--r--lib/api/users.rb20
-rw-r--r--lib/gitlab/ldap/access.rb4
-rw-r--r--lib/gitlab/o_auth/user.rb2
-rw-r--r--spec/controllers/profiles/preferences_controller_spec.rb5
-rw-r--r--spec/features/profiles/password_spec.rb2
-rw-r--r--spec/requests/api/users_spec.rb19
-rw-r--r--spec/services/emails/create_service_spec.rb21
-rw-r--r--spec/services/emails/destroy_service_spec.rb14
-rw-r--r--spec/services/users/update_service_spec.rb43
27 files changed, 279 insertions, 84 deletions
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index b09eef17c23..fa1bc72560e 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/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(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
@@ -202,4 +209,10 @@ class Admin::UsersController < Admin::ApplicationController
:website_url
]
end
+
+ def update_user(&block)
+ result = Users::UpdateService.new(user).execute(&block)
+
+ result[:status] == :success
+ end
end
diff --git a/app/controllers/profiles/avatars_controller.rb b/app/controllers/profiles/avatars_controller.rb
index 933e0f3bceb..408650aac54 100644
--- a/app/controllers/profiles/avatars_controller.rb
+++ b/app/controllers/profiles/avatars_controller.rb
@@ -1,9 +1,8 @@
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
diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb
index 5655fb2ba0e..17b66df43e7 100644
--- a/app/controllers/profiles/emails_controller.rb
+++ b/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, 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, 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 a271e2dfc4b..960b7512602 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/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"
diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb
index 6217ec5ecef..10145bae0d3 100644
--- a/app/controllers/profiles/passwords_controller.rb
+++ b/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
diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb
index 5414142e2df..1e557c47638 100644
--- a/app/controllers/profiles/preferences_controller.rb
+++ b/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.'
diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb
index 313cdcd1c15..1a4f77639e7 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
- 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
diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb
index f98a9e24de1..076076fd1b3 100644
--- a/app/controllers/profiles_controller.rb
+++ b/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! 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! 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! 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
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 0d8186dce02..f39441a281e 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -60,10 +60,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
diff --git a/app/models/user.rb b/app/models/user.rb
index 9971e43146a..650b64e7551 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -53,7 +53,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
@@ -494,10 +494,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, email: email).execute
+ Emails::CreateService.new(self, email: email_was).execute
end
end
@@ -965,7 +963,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
@@ -1129,7 +1127,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)
diff --git a/app/services/emails/base_service.rb b/app/services/emails/base_service.rb
new file mode 100644
index 00000000000..ace49889097
--- /dev/null
+++ b/app/services/emails/base_service.rb
@@ -0,0 +1,8 @@
+module Emails
+ class BaseService
+ def initialize(user, opts)
+ @user = user
+ @email = opts[:email]
+ end
+ end
+end
diff --git a/app/services/emails/create_service.rb b/app/services/emails/create_service.rb
new file mode 100644
index 00000000000..b6491ee9804
--- /dev/null
+++ b/app/services/emails/create_service.rb
@@ -0,0 +1,7 @@
+module Emails
+ class CreateService < ::Emails::BaseService
+ def execute
+ @user.emails.create(email: @email)
+ end
+ end
+end
diff --git a/app/services/emails/destroy_service.rb b/app/services/emails/destroy_service.rb
new file mode 100644
index 00000000000..d586b9dfe0c
--- /dev/null
+++ b/app/services/emails/destroy_service.rb
@@ -0,0 +1,17 @@
+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(@user).execute do |user|
+ user.update_secondary_emails!
+ end
+
+ result[:status] == 'success'
+ end
+ end
+end
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
new file mode 100644
index 00000000000..dfbd6016c3f
--- /dev/null
+++ b/app/services/users/update_service.rb
@@ -0,0 +1,34 @@
+module Users
+ class UpdateService < BaseService
+ def initialize(user, params = {})
+ @user = user
+ @params = params.dup
+ end
+
+ def execute(validate: true, &block)
+ yield(@user) if block_given?
+
+ 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)
+ @user.assign_attributes(params) if params.any?
+ end
+ end
+end
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 479ee16a611..f1c79970ba4 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -132,8 +132,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!
+ codes = nil
+
+ ::Users::UpdateService.new(user).execute! do |user|
+ codes = user.generate_otp_backup_codes!
+ end
{ success: true, recovery_codes: codes }
end
diff --git a/lib/api/notification_settings.rb b/lib/api/notification_settings.rb
index 992ea5dc24d..5d113c94b22 100644
--- a/lib/api/notification_settings.rb
+++ b/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
diff --git a/lib/api/users.rb b/lib/api/users.rb
index c10e3364382..f9555842daf 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -98,7 +98,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
@@ -156,7 +156,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)
@@ -234,9 +236,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(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
@@ -274,8 +276,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(user, email: email.email).execute
end
desc 'Delete a user. Available only for admins.' do
@@ -487,9 +488,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, declared_params).execute
- if email.save
+ if email.errors.blank?
NotificationService.new.new_email(email)
present email, with: Entities::Email
else
@@ -505,8 +506,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, email: email.email).execute
end
desc 'Get a list of user activities'
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index 54a5b1d31cd..8779577258b 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -16,8 +16,8 @@ module Gitlab
def self.allowed?(user)
self.open(user) do |access|
if access.allowed?
- user.last_credential_check_at = Time.now
- user.save
+ Users::UpdateService.new(user, last_credential_check_a: Time.now).execute
+
true
else
false
diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb
index 7307f8c2c87..b3f453e506d 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?
- gl_user.save!
+ Users::UpdateService.new(gl_user).execute!
gl_user.block if block_after_save
diff --git a/spec/controllers/profiles/preferences_controller_spec.rb b/spec/controllers/profiles/preferences_controller_spec.rb
index 7b3aa0491c7..a5f544b4f92 100644
--- a/spec/controllers/profiles/preferences_controller_spec.rb
+++ b/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
diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb
index 2d36f3d020f..86c9df5ff86 100644
--- a/spec/features/profiles/password_spec.rb
+++ b/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]')
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 18000d91795..c0174b304c8 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -364,6 +364,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')
@@ -396,13 +397,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)
@@ -410,12 +420,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')
@@ -423,12 +435,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
@@ -436,6 +450,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)
@@ -443,6 +458,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
@@ -459,6 +475,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
@@ -509,6 +526,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
@@ -516,6 +534,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
diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb
new file mode 100644
index 00000000000..c1f477f551e
--- /dev/null
+++ b/spec/services/emails/create_service_spec.rb
@@ -0,0 +1,21 @@
+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, 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
diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb
new file mode 100644
index 00000000000..5e7ab4a40af
--- /dev/null
+++ b/spec/services/emails/destroy_service_spec.rb
@@ -0,0 +1,14 @@
+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, email: email.email) }
+
+ describe '#execute' do
+ it 'removes an email' do
+ expect { service.execute }.to change { user.emails.count }.by(-1)
+ end
+ end
+end
diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb
new file mode 100644
index 00000000000..0b2f840c462
--- /dev/null
+++ b/spec/services/users/update_service_spec.rb
@@ -0,0 +1,43 @@
+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 'raises an error 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