ensures that the avatar helper always returns a string, might be empty

app/helpers/avatar_helper.rb

@@ -16,17 +16,24 @@ module AvatarHelper
   # Returns the avatar image tag for the given +user+ if avatars are enabled
   # +user+ can be a User or a string that will be scanned for an email address (eg. 'joe <joe@foo.bar>')
   def avatar(user, options = { })
-    if Setting.gravatar_enabled?
-      options.merge!({:ssl => (defined?(request) && request.ssl?), :default => Setting.gravatar_default})
-      email = nil
-      if user.respond_to?(:mail)
-        email = user.mail
-      elsif user.to_s =~ %r{<(.+?)>}
-        email = $1
-      end
-      return gravatar(email.to_s.downcase, options) unless email.blank? rescue nil
-    else
-      ''.html_safe
+    avatar = if Setting.gravatar_enabled? && (email = extract_email_address(user)).present?
+               options.merge!({ :ssl => (defined?(request) && request.ssl?),
+                                :default => Setting.gravatar_default })
+
+               gravatar(email.to_s.downcase, options)
+             end
+  ensure
+    # return is actually needed here
+    return (avatar || ''.html_safe)
+  end
+
+  private
+
+  def extract_email_address(object)
+    if object.respond_to?(:mail)
+      object.mail
+    elsif object.to_s =~ %r{<(.+?)>}
+      $1
     end
   end
 end

doc/CHANGELOG.md

@@ -1,5 +1,6 @@
 # Changelog
 
+* `#1406` Creating a work package w/o responsible or assignee results in 500
 * `#1391` Opening the new issue form in a project with an issue category defined produces 500 response
 * `#1063` Added helper to format the time as a date in the current user or the system time zone
 * `#1024` Add 'assign random password' option to user settings

spec/helpers/avatar_helper_spec.rb

@@ -15,19 +15,77 @@ require 'spec_helper'
 describe AvatarHelper do
   let(:user) { FactoryGirl.build_stubbed(:user) }
 
+  def expected_image_tag(digest, options = {})
+    # there are some attributes in here that are wrong but are returned by the
+    # bundled plugin.  I will not fix the lib at the given moment. I would rather, we
+    # remove the bundled gem and reference one in the Gemfile or
+    # implement the thing ourselves.
+
+    host = options[:ssl] ?
+             "https://secure.gravatar.com" :
+             "http://www.gravatar.com"
+
+    expected_src = "#{host}/avatar/#{digest}?rating=PG&size=50&default="
+
+    image_tag expected_src, :class => "gravatar",
+                            :title => '',
+                            :ssl => options[:ssl] || false,
+                            :alt => '',
+                            :default => '',
+                            :rating => 'PG'
+  end
+
   describe :avatar do
-    it "should produce an image tag pointing to gravatar" do
-      # turn on avatars
-      Setting.gravatar_enabled = '1'
-      mail = user.mail
-      assert helper.avatar(user).include?(Digest::MD5.hexdigest(mail))
-      assert helper.avatar("#{user.name} <#{mail}>").include?(Digest::MD5.hexdigest(mail))
-      assert_nil helper.avatar('admin')
-      assert_nil helper.avatar(nil)
-
-      # turn off avatars
-      Setting.gravatar_enabled = '0'
-      assert_equal '', helper.avatar(user)
+    it "should return a gravatar image tag if a user is provided" do
+      digest = Digest::MD5.hexdigest(user.mail)
+
+      with_settings :gravatar_enabled => '1' do
+        helper.avatar(user).should == expected_image_tag(digest)
+      end
+    end
+
+    it "should return a gravatar image tag with ssl if the request was ssl required" do
+      digest = Digest::MD5.hexdigest(user.mail)
+      helper.request.stub!(:ssl?).and_return(true)
+
+      with_settings :gravatar_enabled => '1' do
+        helper.avatar(user).should == expected_image_tag(digest, :ssl => true)
+      end
+    end
+
+    it "should return a gravatar image tag if a parsable e-mail string is provided" do
+      with_settings :gravatar_enabled => '1' do
+        mail = "<e-mail@mail.de>"
+        digest = Digest::MD5.hexdigest("e-mail@mail.de")
+
+        helper.avatar(mail).should == expected_image_tag(digest)
+      end
+    end
+
+    it "should return an empty string if a non parsable (e-mail) string is provided" do
+      with_settings :gravatar_enabled => '1' do
+        helper.avatar('just the name').should == ''
+      end
+    end
+
+    it "should return an empty string if nil is provided" do
+      with_settings :gravatar_enabled => '1' do
+        helper.avatar(nil).should == ''
+      end
+    end
+
+    it "should return an empty string if gravatar is disabled" do
+      with_settings :gravatar_enabled => '0' do
+        helper.avatar(user).should == ''
+      end
+    end
+
+    it "should return an empty string if any error is produced in the lib" do
+      helper.stub!(:gravatar).and_raise(ArgumentError)
+
+      with_settings :gravatar_enabled => '1' do
+        helper.avatar(user).should == ''
+      end
     end
   end
 end