From 11dca1fd9231283f9f458fa7e1ebc3a01ba372f9 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Tue, 6 Aug 2013 06:25:44 +1000
Subject: [PATCH] make code climate a bit happier

---
 app/controllers/application_controller.rb | 37 ++++++++++++-----------
 app/serializers/user_action_serializer.rb | 35 +++++++++++++--------
 2 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 0a2e9e133..740dd502e 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -106,23 +106,15 @@ class ApplicationController < ActionController::Base
 
   # If we are rendering HTML, preload the session data
   def preload_json
-
     # We don't preload JSON on xhr or JSON request
     return if request.xhr?
 
-    if guardian.current_user
-      guardian.current_user.sync_notification_channel_position
+    preload_anonymous_data
+
+    if current_user
+      preload_current_user_data
+      current_user.sync_notification_channel_position
     end
-
-    store_preloaded("site", Site.cached_json(guardian))
-
-    if current_user.present?
-      store_preloaded("currentUser", MultiJson.dump(CurrentUserSerializer.new(current_user, root: false)))
-
-      serializer = ActiveModel::ArraySerializer.new(TopicTrackingState.report([current_user.id]), each_serializer: TopicTrackingStateSerializer)
-      store_preloaded("topicTrackingStates", MultiJson.dump(serializer))
-    end
-    store_preloaded("siteSettings", SiteSetting.client_settings_json)
   end
 
 
@@ -201,6 +193,17 @@ class ApplicationController < ActionController::Base
 
   private
 
+    def preload_anonymous_data
+      store_preloaded("site", Site.cached_json(guardian))
+      store_preloaded("siteSettings", SiteSetting.client_settings_json)
+    end
+
+    def preload_current_user_data
+      store_preloaded("currentUser", MultiJson.dump(CurrentUserSerializer.new(current_user, root: false)))
+      serializer = ActiveModel::ArraySerializer.new(TopicTrackingState.report([current_user.id]), each_serializer: TopicTrackingStateSerializer)
+      store_preloaded("topicTrackingStates", MultiJson.dump(serializer))
+    end
+
     def render_json_error(obj)
       if obj.present?
         render json: MultiJson.dump(errors: obj.errors.full_messages), status: 422
@@ -257,11 +260,9 @@ class ApplicationController < ActionController::Base
     end
 
     def check_xhr
-      unless (controller_name == 'forums' || controller_name == 'user_open_ids')
-        # bypass xhr check on PUT / POST / DELETE provided api key is there, otherwise calling api is annoying
-        return if !request.get? && api_key_valid?
-        raise RenderEmpty.new unless ((request.format && request.format.json?) || request.xhr?)
-      end
+      # bypass xhr check on PUT / POST / DELETE provided api key is there, otherwise calling api is annoying
+      return if !request.get? && api_key_valid?
+      raise RenderEmpty.new unless ((request.format && request.format.json?) || request.xhr?)
     end
 
     def ensure_logged_in
diff --git a/app/serializers/user_action_serializer.rb b/app/serializers/user_action_serializer.rb
index a77d2bb78..0fa62156c 100644
--- a/app/serializers/user_action_serializer.rb
+++ b/app/serializers/user_action_serializer.rb
@@ -28,21 +28,21 @@ class UserActionSerializer < ApplicationSerializer
   end
 
   def avatar_template
-    user = User.new
-    user[:email] = object.email
-    user[:use_uploaded_avatar] = object.use_uploaded_avatar
-    user[:uploaded_avatar_template] = object.uploaded_avatar_template
-    user[:uploaded_avatar_id] = object.uploaded_avatar_id
-    user.avatar_template
+    avatar_for(
+      object.email,
+      object.use_uploaded_avatar,
+      object.uploaded_avatar_template,
+      object.uploaded_avatar_id
+    )
   end
 
   def acting_avatar_template
-    acting_user = User.new
-    acting_user[:email] = object.acting_email
-    acting_user[:use_uploaded_avatar] = object.acting_use_uploaded_avatar
-    acting_user[:uploaded_avatar_template] = object.acting_uploaded_avatar_template
-    acting_user[:uploaded_avatar_id] = object.acting_uploaded_avatar_id
-    acting_user.avatar_template
+    avatar_for(
+                object.acting_email,
+                object.acting_use_uploaded_avatar,
+                object.acting_uploaded_avatar_template,
+                object.acting_uploaded_avatar_id
+    )
   end
 
   def slug
@@ -53,4 +53,15 @@ class UserActionSerializer < ApplicationSerializer
     object.post_type == Post.types[:moderator_action]
   end
 
+  private
+  def avatar_for(email, use_uploaded_avatar, uploaded_avatar_template, uploaded_avatar_id)
+    # NOTE: id is required for cases where the template is blank (during initial population)
+    User.new(
+      email: email,
+      use_uploaded_avatar: use_uploaded_avatar,
+      uploaded_avatar_template: uploaded_avatar_template,
+      uploaded_avatar_id: uploaded_avatar_id
+    ).avatar_template
+  end
+
 end