From 868e4ffe6d163c7b04ea0d5672469cdf66a77804 Mon Sep 17 00:00:00 2001
From: Manoj <manoj.mk27@gmail.com>
Date: Sat, 19 Oct 2013 15:18:11 +0530
Subject: [PATCH] refactor UsersController to reduce complexity

Refactored: UsersController#create
---
 app/controllers/users_controller.rb | 67 +++++++++++++----------------
 app/models/user.rb                  |  7 +++
 spec/models/user_spec.rb            |  8 ++++
 3 files changed, 45 insertions(+), 37 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 2eca5e70334..a6a318a7c7f 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -64,11 +64,7 @@ class UsersController < ApplicationController
         end
       end
 
-      if u.save
-        u
-      else
-        nil
-      end
+      u.save ? u : nil
     end
   end
 
@@ -138,41 +134,16 @@ class UsersController < ApplicationController
     auth = authenticate_user(user, params)
     register_nickname(user)
 
-    if user.save
-      activator = UserActivator.new(user, request, session, cookies)
-      message = activator.activation_message
-      create_third_party_auth_records(user, auth)
+    user.save ? user_create_successful(user, auth) : user_create_failed(user)
 
-      # 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
@@ -312,11 +283,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.uploaded_avatar_template = nil
-    user.uploaded_avatar = upload
-    user.use_uploaded_avatar = true
-    user.save!
+    user.update_avatar(upload)
 
     Jobs.enqueue(:generate_avatars, user_id: user.id, upload_id: upload.id)
 
@@ -396,4 +363,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
diff --git a/app/models/user.rb b/app/models/user.rb
index aeab21cb254..fd4cef6acf4 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -471,6 +471,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
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 253d51b4948..a1f4e2b958a 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -839,6 +839,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