Don’t pointlessly pull stuff from reds in context_user_name

It looks like there was no reason we were Rails.cache.fetching
this most of the time. Becaus inside the rails.cache.fetch
we would not actually cause any db other expensive reads unless 
We were passed a `user` that was not an actual user model
(Or something else that .respond_to?(:short_name))

Reviewers: am I missing something here? Is there some reason we
Need to always cache this/read it from cache?

Change-Id: I6f8d3db7b691aea79785566444a253f99a8572ce
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/216275
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>

app/helpers/application_helper.rb

@@ -23,19 +23,22 @@ module ApplicationHelper
   include LocaleSelection
   include Canvas::LockExplanation
 
+  def context_user_name_display(user)
+    name = user.try(:short_name) || user.try(:name)
+    if user.try(:pronouns)
+      "#{name} (#{user.pronouns})"
+    else
+      name
+    end
+  end
+
   def context_user_name(context, user)
     return nil unless user
-    return user.short_name if !context && user.respond_to?(:short_name)
-    user_id = user
-    user_id = user.id if user.is_a?(User) || user.is_a?(OpenObject)
+    return context_user_name_display(user) if user.respond_to?(:short_name)
+
+    user_id = user.is_a?(OpenObject) ? user.id : user
     Rails.cache.fetch(['context_user_name', context, user_id].cache_key, {:expires_in=>15.minutes}) do
-      user = user.respond_to?(:short_name) ? user : User.find(user_id)
-      name = user.short_name || user.name
-      if user.pronouns
-        "#{name} (#{user.pronouns})"
-      else
-        name
-      end
+      context_user_name_display(User.find(user_id))
     end
   end