Merge branch 'dm-gravatar-username' into 'master'

Add username parameter to gravatar URL

Closes #24472

See merge request !11741

app/models/user.rb

@@ -781,7 +781,7 @@ class User < ActiveRecord::Base
   def avatar_url(size: nil, scale: 2, **args)
     # We use avatar_path instead of overriding avatar_url because of carrierwave.
     # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864
-    avatar_path(args) || GravatarService.new.execute(email, size, scale)
+    avatar_path(args) || GravatarService.new.execute(email, size, scale, username: username)
   end
 
   def all_emails

app/services/gravatar_service.rb

@@ -1,15 +1,20 @@
 class GravatarService
   include Gitlab::CurrentSettings
 
-  def execute(email, size = nil, scale = 2)
-    if current_application_settings.gravatar_enabled? && email.present?
-      size = 40 if size.nil? || size <= 0
+  def execute(email, size = nil, scale = 2, username: nil)
+    return unless current_application_settings.gravatar_enabled?
 
-      sprintf gravatar_url,
-        hash: Digest::MD5.hexdigest(email.strip.downcase),
-        size: size * scale,
-        email: email.strip
-    end
+    identifier = email.presence || username.presence
+    return unless identifier
+
+    hash = Digest::MD5.hexdigest(identifier.strip.downcase)
+    size = 40 unless size && size > 0
+
+    sprintf gravatar_url,
+      hash: hash,
+      size: size * scale,
+      email: ERB::Util.url_encode(email&.strip || ''),
+      username: ERB::Util.url_encode(username&.strip || '')
   end
 
   def gitlab_config

changelogs/unreleased/dm-gravatar-username.yml

@@ -0,0 +1,4 @@
+---
+title: Add username parameter to gravatar URL
+merge_request:
+author:

config/gitlab.yml.example

@@ -169,7 +169,7 @@ production: &base
   ## Gravatar
   ## For Libravatar see: http://doc.gitlab.com/ce/customization/libravatar.html
   gravatar:
-    # gravatar urls: possible placeholders: %{hash} %{size} %{email}
+    # gravatar urls: possible placeholders: %{hash} %{size} %{email} %{username}
     # plain_url: "http://..."     # default: http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon
     # ssl_url:   "https://..."    # default: https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon
 

doc/customization/libravatar.md

@@ -16,7 +16,7 @@ the configuration options as follows:
 ```yml
   gravatar:
     enabled: true
-    # gravatar URLs: possible placeholders: %{hash} %{size} %{email}
+    # gravatar URLs: possible placeholders: %{hash} %{size} %{email} %{username}
     plain_url: "http://cdn.libravatar.org/avatar/%{hash}?s=%{size}&d=identicon"
 ```
 
@@ -25,7 +25,7 @@ the configuration options as follows:
 ```yml
   gravatar:
     enabled: true
-    # gravatar URLs: possible placeholders: %{hash} %{size} %{email}
+    # gravatar URLs: possible placeholders: %{hash} %{size} %{email} %{username}
     ssl_url: "https://seccdn.libravatar.org/avatar/%{hash}?s=%{size}&d=identicon"
 ```
 

spec/services/gravatar_service_spec.rb

@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe GravatarService, service: true do
+  describe '#execute' do
+    let(:url) { 'http://example.com/avatar?hash=%{hash}&size=%{size}&email=%{email}&username=%{username}' }
+
+    before do
+      allow(Gitlab.config.gravatar).to receive(:plain_url).and_return(url)
+    end
+
+    it 'replaces the placeholders' do
+      avatar_url = described_class.new.execute('user@example.com', 100, 2, username: 'user')
+
+      expect(avatar_url).to include("hash=#{Digest::MD5.hexdigest('user@example.com')}")
+      expect(avatar_url).to include("size=200")
+      expect(avatar_url).to include("email=user%40example.com")
+      expect(avatar_url).to include("username=user")
+    end
+  end
+end