diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index df19e15d18d..d237587b305 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -43,28 +43,8 @@ class UsersController < ApplicationController
user = User.where(username_lower: params[:username].downcase).first
guardian.ensure_can_edit!(user)
json_result(user, serializer: UserSerializer) do |u|
-
- website = params[:website]
- if website
- website = "http://" + website unless website =~ /^http/
- end
-
- u.bio_raw = params[:bio_raw] || u.bio_raw
- u.name = params[:name] || u.name
- u.website = website || u.website
- u.digest_after_days = params[:digest_after_days] || u.digest_after_days
- u.auto_track_topics_after_msecs = params[:auto_track_topics_after_msecs].to_i if params[:auto_track_topics_after_msecs]
- u.new_topic_duration_minutes = params[:new_topic_duration_minutes].to_i if params[:new_topic_duration_minutes]
- u.title = params[:title] || u.title if guardian.can_grant_title?(u)
-
- [:email_digests, :email_always, :email_direct, :email_private_messages,
- :external_links_in_new_tab, :enable_quoting, :dynamic_favicon].each do |i|
- if params[i].present?
- u.send("#{i.to_s}=", params[i] == 'true')
- end
- end
-
- u.save ? u : nil
+ updater = UserUpdater.new(user)
+ updater.update(params)
end
end
diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb
new file mode 100644
index 00000000000..d8c1e5047e0
--- /dev/null
+++ b/app/services/user_updater.rb
@@ -0,0 +1,54 @@
+class UserUpdater
+ def initialize(user)
+ @user = user
+ @guardian = Guardian.new(user)
+ end
+
+ def update(attributes = {})
+ user.website = format_url(attributes[:website]) || user.website
+
+ user.bio_raw = attributes[:bio_raw] || user.bio_raw
+ user.name = attributes[:name] || user.name
+ user.digest_after_days = attributes[:digest_after_days] || user.digest_after_days
+
+ if attributes[:auto_track_topics_after_msecs]
+ user.auto_track_topics_after_msecs = attributes[:auto_track_topics_after_msecs].to_i
+ end
+
+ if attributes[:new_topic_duration_minutes]
+ user.new_topic_duration_minutes = attributes[:new_topic_duration_minutes].to_i
+ end
+
+ if guardian.can_grant_title?(user)
+ user.title = attributes[:title] || user.title
+ end
+
+ [
+ :email_digests,
+ :email_always,
+ :email_direct,
+ :email_private_messages,
+ :external_links_in_new_tab,
+ :enable_quoting,
+ :dynamic_favicon
+ ].each do |attribute|
+ if attributes[attribute].present?
+ user.send("#{attribute.to_s}=", attributes[attribute] == 'true')
+ end
+ end
+
+ user.save
+ end
+
+ private
+
+ attr_reader :user, :guardian
+
+ def format_url(website)
+ if website =~ /^http/
+ website
+ else
+ "http://#{website}"
+ end
+ end
+end
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index d5d2a24f0b6..46b76cdafa6 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -853,29 +853,9 @@ describe UsersController do
json = JSON.parse(response.body)
expect(json['user']['id']).to eq user.id
end
-
- context 'when website includes http' do
- it 'does not add http before updating' do
- user = log_in
-
- put :update, username: user.username, website: 'http://example.com'
-
- expect(user.reload.website).to eq 'http://example.com'
- end
- end
-
- context 'when website does not include http' do
- it 'adds http before updating' do
- user = log_in
-
- put :update, username: user.username, website: 'example.com'
-
- expect(user.reload.website).to eq 'http://example.com'
- end
- end
end
- context 'without permission to update any attributes' do
+ context 'without permission to update' do
it 'does not allow the update' do
user = Fabricate(:user, name: 'Billy Bob')
log_in_user(user)
@@ -889,20 +869,6 @@ describe UsersController do
expect(user.reload.name).not_to eq 'Jim Tom'
end
end
-
- context 'without permission to update title' do
- it 'does not allow the user to update their title' do
- user = Fabricate(:user, title: 'Emperor')
- log_in_user(user)
- guardian = Guardian.new(user)
- guardian.stubs(can_grant_title?: false).with(user)
- Guardian.stubs(new: guardian).with(user)
-
- put :update, username: user.username, title: 'Minion'
-
- expect(user.reload.title).not_to eq 'Minion'
- end
- end
end
end
diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb
new file mode 100644
index 00000000000..4524c967432
--- /dev/null
+++ b/spec/services/user_updater_spec.rb
@@ -0,0 +1,83 @@
+require 'spec_helper'
+
+describe UserUpdater do
+ describe '#update' do
+ it 'saves user' do
+ user = Fabricate(:user, name: 'Billy Bob')
+ updater = UserUpdater.new(user)
+
+ updater.update(name: 'Jim Tom')
+
+ expect(user.reload.name).to eq 'Jim Tom'
+ end
+
+ context 'when update succeeds' do
+ it 'returns true' do
+ user = Fabricate(:user)
+ updater = UserUpdater.new(user)
+
+ expect(updater.update).to be_true
+ end
+ end
+
+ context 'when update fails' do
+ it 'returns false' do
+ user = Fabricate(:user)
+ user.stubs(save: false)
+ updater = UserUpdater.new(user)
+
+ expect(updater.update).to be_false
+ end
+ end
+
+ context 'with permission to update title' do
+ it 'allows user to change title' do
+ user = Fabricate(:user, title: 'Emperor')
+ guardian = stub
+ guardian.stubs(:can_grant_title?).with(user).returns(true)
+ Guardian.stubs(:new).with(user).returns(guardian)
+ updater = UserUpdater.new(user)
+
+ updater.update(title: 'Minion')
+
+ expect(user.reload.title).to eq 'Minion'
+ end
+ end
+
+ context 'without permission to update title' do
+ it 'does not allow user to change title' do
+ user = Fabricate(:user, title: 'Emperor')
+ guardian = stub
+ guardian.stubs(:can_grant_title?).with(user).returns(false)
+ Guardian.stubs(:new).with(user).returns(guardian)
+ updater = UserUpdater.new(user)
+
+ updater.update(title: 'Minion')
+
+ expect(user.reload.title).not_to eq 'Minion'
+ end
+ end
+
+ context 'when website includes http' do
+ it 'does not add http before updating' do
+ user = Fabricate(:user)
+ updater = UserUpdater.new(user)
+
+ updater.update(website: 'http://example.com')
+
+ expect(user.reload.website).to eq 'http://example.com'
+ end
+ end
+
+ context 'when website does not include http' do
+ it 'adds http before updating' do
+ user = Fabricate(:user)
+ updater = UserUpdater.new(user)
+
+ updater.update(website: 'example.com')
+
+ expect(user.reload.website).to eq 'http://example.com'
+ end
+ end
+ end
+end