Merge pull request #1543 from railsaholic/small_users_controller_refactoring

refactor UsersController to reduce complexity

app/controllers/users_controller.rb

@@ -64,11 +64,7 @@ class UsersController < ApplicationController
         end
       end
 
-      if u.save
+      u.save ? u : nil
-        u
-      else
-        nil
-      end
     end
   end
 
@@ -139,41 +135,16 @@ class UsersController < ApplicationController
     auth = authenticate_user(user, params)
     register_nickname(user)
 
-    if user.save
+    user.save ? user_create_successful(user, auth) : user_create_failed(user)
-      activator = UserActivator.new(user, request, session, cookies)
-      message = activator.activation_message
-      create_third_party_auth_records(user, auth)
 
-      # Clear authentication session.
-      session[:authentication] = nil
-
-      render json: { success: true, active: user.active?, message: message }
-    else
-      render json: {
-        success: false,
-        message: I18n.t("login.errors", errors: user.errors.full_messages.join("\n")),
-        errors: user.errors.to_hash,
-        values: user.attributes.slice("name", "username", "email")
-      }
-    end
   rescue ActiveRecord::StatementInvalid
     render json: { success: false, message: I18n.t("login.something_already_taken") }
-  rescue DiscourseHub::NicknameUnavailable=> e
+  rescue DiscourseHub::NicknameUnavailable => e
     render json: e.response_message
   rescue RestClient::Forbidden
     render json: { errors: [I18n.t("discourse_hub.access_token_problem")] }
   end
 
-  def authenticate_user(user, params)
-    auth = session[:authentication]
-    if valid_session_authentication?(auth, params[:email])
-      user.active = true
-    end
-    user.password_required! unless auth
-
-    auth
-  end
-
   def get_honeypot_value
     render json: {value: honeypot_value, challenge: challenge_value}
   end
@@ -313,11 +284,7 @@ class UsersController < ApplicationController
     return render status: 413, text: I18n.t("upload.images.too_large", max_size_kb: max_size_kb) if filesize > max_size_kb
 
     upload = Upload.create_for(user.id, file, filesize)
-
+    user.update_avatar(upload)
-    user.uploaded_avatar_template = nil
-    user.uploaded_avatar = upload
-    user.use_uploaded_avatar = true
-    user.save!
 
     Jobs.enqueue(:generate_avatars, user_id: user.id, upload_id: upload.id)
 
@@ -397,4 +364,30 @@ class UsersController < ApplicationController
         DiscourseHub.register_nickname(user.username, user.email)
       end
     end
+
+    def user_create_successful(user, auth)
+      activator = UserActivator.new(user, request, session, cookies)
+      create_third_party_auth_records(user, auth)
+
+      # Clear authentication session.
+      session[:authentication] = nil
+      render json: { success: true, active: user.active?, message: activator.activation_message }
+    end
+
+    def user_create_failed(user)
+      render json: {
+        success: false,
+        message: I18n.t("login.errors", errors: user.errors.full_messages.join("\n")),
+        errors: user.errors.to_hash,
+        values: user.attributes.slice("name", "username", "email")
+      }
+    end
+
+    def authenticate_user(user, params)
+      auth = session[:authentication]
+      user.active = true if valid_session_authentication?(auth, params[:email])
+      user.password_required! unless auth
+      auth
+    end
+
 end

app/models/user.rb

@@ -472,6 +472,13 @@ class User < ActiveRecord::Base
     created_at > 1.day.ago
   end
 
+  def update_avatar(upload)
+    self.uploaded_avatar_template = nil
+    self.uploaded_avatar = upload
+    self.use_uploaded_avatar = true
+    self.save!
+  end
+
   protected
 
   def cook

spec/models/user_spec.rb

@@ -853,6 +853,14 @@ describe User do
         expect(user).to be_added_a_day_ago
       end
     end
+  end
 
+  describe "#update_avatar" do
+    let(:upload) { Fabricate(:upload) }
+    let(:user)   { Fabricate(:user) }
+
+    it "should update use's avatar" do
+      expect(user.update_avatar(upload)).to be_true
+    end
   end
 end