From f338ff43c13a6dd5c3bf90bd58c0d5cff52fc79c Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Wed, 7 Feb 2018 18:20:20 +0100
Subject: Refactor and split ApplicationHelper#avatar_icon.

When we don't use the original `ApplicationHelper#avatar_icon` anymore, we
can just remove it (and its specs).

Closes #42800.
---
 app/helpers/application_helper.rb       | 24 ++++++++++++-----
 spec/helpers/application_helper_spec.rb | 48 +++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 6530327698b..77c86be4714 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -69,17 +69,27 @@ module ApplicationHelper
   end
 
   def avatar_icon(user_or_email = nil, size = nil, scale = 2, only_path: true)
-    user =
-      if user_or_email.is_a?(User)
-        user_or_email
-      else
-        User.find_by_any_email(user_or_email.try(:downcase))
-      end
+    if user_or_email.is_a?(User)
+      avatar_icon_for_user(user_or_email, size, scale, only_path: only_path)
+    else
+      avatar_icon_for_email(user_or_email, size, scale, only_path: only_path)
+    end
+  end
+
+  def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
+    user = User.find_by_any_email(email.try(:downcase))
+    if user
+      avatar_icon_for_user(user, size, scale, only_path: only_path)
+    else
+      gravatar_icon(email, size, scale)
+    end
+  end
 
+  def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true)
     if user
       user.avatar_url(size: size, only_path: only_path) || default_avatar
     else
-      gravatar_icon(user_or_email, size, scale)
+      gravatar_icon(nil, size, scale)
     end
   end
 
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index f7a4a7afced..a1fcdeca10e 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -91,6 +91,54 @@ describe ApplicationHelper do
     end
   end
 
+  describe 'avatar_icon_for_email' do
+    let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
+
+    context 'using an email' do
+      context 'when there is a matching user' do
+        it 'returns a relative URL for the avatar' do
+          expect(helper.avatar_icon_for_email(user.email).to_s)
+            .to eq(user.avatar.url)
+        end
+      end
+
+      context 'when no user exists for the email' do
+        it 'calls gravatar_icon' do
+          expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
+
+          helper.avatar_icon_for_email('foo@example.com', 20, 2)
+        end
+      end
+
+      context 'without an email passed' do
+        it 'calls gravatar_icon' do
+          expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
+
+          helper.avatar_icon_for_email(nil, 20, 2)
+        end
+      end
+    end
+  end
+
+  describe 'avatar_icon_for_user' do
+    let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
+
+    context 'with a user object passed' do
+      it 'returns a relative URL for the avatar' do
+        expect(helper.avatar_icon_for_user(user).to_s)
+          .to eq(user.avatar.url)
+      end
+    end
+
+    context 'without a user object passed' do
+      it 'calls gravatar_icon' do
+        expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
+
+        helper.avatar_icon_for_user(nil, 20, 2)
+      end
+    end
+  end
+
   describe 'gravatar_icon' do
     let(:user_email) { 'user@email.com' }
 
-- 
cgit v1.2.1


From be231d21650cc5698a283bf8da58dc433a395fbf Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 11:43:12 +0100
Subject: Use more specific #avatar_icon_for_user.

Whenever we already deal with a User object, let's use the more specific
method avatar_icon_for_user.
---
 app/helpers/projects_helper.rb                                     | 2 +-
 app/views/admin/users/_user.html.haml                              | 2 +-
 app/views/admin/users/show.html.haml                               | 2 +-
 app/views/discussions/_discussion.html.haml                        | 2 +-
 app/views/events/_event.atom.builder                               | 2 +-
 app/views/layouts/header/_default.html.haml                        | 2 +-
 app/views/notify/pipeline_failed_email.html.haml                   | 2 +-
 app/views/notify/pipeline_success_email.html.haml                  | 2 +-
 app/views/profiles/show.html.haml                                  | 4 ++--
 app/views/projects/jobs/_user.html.haml                            | 2 +-
 app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml | 2 +-
 app/views/search/results/_snippet_blob.html.haml                   | 2 +-
 app/views/search/results/_snippet_title.html.haml                  | 2 +-
 app/views/shared/members/_member.html.haml                         | 2 +-
 app/views/shared/milestones/_issuable.html.haml                    | 2 +-
 app/views/shared/milestones/_participants_tab.html.haml            | 2 +-
 app/views/shared/notes/_note.html.haml                             | 2 +-
 app/views/shared/notes/_notes_with_form.html.haml                  | 2 +-
 app/views/shared/snippets/_snippet.html.haml                       | 2 +-
 app/views/users/show.html.haml                                     | 4 ++--
 spec/helpers/projects_helper_spec.rb                               | 2 +-
 21 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 6512617a02d..aeb9b8be722 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -19,7 +19,7 @@ module ProjectsHelper
     classes = %W[avatar avatar-inline s#{opts[:size]}]
     classes << opts[:avatar_class] if opts[:avatar_class]
 
-    avatar = avatar_icon(author, opts[:size])
+    avatar = avatar_icon_for_user(author, opts[:size])
     src = opts[:lazy_load] ? nil : avatar
 
     image_tag(src, width: opts[:size], class: classes, alt: '', "data-src" => avatar)
diff --git a/app/views/admin/users/_user.html.haml b/app/views/admin/users/_user.html.haml
index 3b85586e45b..bbfeceff5b9 100644
--- a/app/views/admin/users/_user.html.haml
+++ b/app/views/admin/users/_user.html.haml
@@ -1,6 +1,6 @@
 %li.flex-row
   .user-avatar
-    = image_tag avatar_icon(user), class: "avatar", alt: ''
+    = image_tag avatar_icon_for_user(user), class: "avatar", alt: ''
   .row-main-content
     .user-name.row-title.str-truncated-100
       = link_to user.name, [:admin, user]
diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml
index 101667508a9..ec3be869797 100644
--- a/app/views/admin/users/show.html.haml
+++ b/app/views/admin/users/show.html.haml
@@ -10,7 +10,7 @@
         = @user.name
       %ul.well-list
         %li
-          = image_tag avatar_icon(@user, 60), class: "avatar s60"
+          = image_tag avatar_icon_for_user(@user, 60), class: "avatar s60"
         %li
           %span.light Profile page:
           %strong
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index 205320ed87c..8b9fa3d6b05 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -3,7 +3,7 @@
   .timeline-entry-inner
     .timeline-icon
       = link_to user_path(discussion.author) do
-        = image_tag avatar_icon(discussion.author), class: "avatar s40"
+        = image_tag avatar_icon_for_user(discussion.author), class: "avatar s40"
     .timeline-content
       .discussion.js-toggle-container{ data: { discussion_id: discussion.id } }
         .discussion-header
diff --git a/app/views/events/_event.atom.builder b/app/views/events/_event.atom.builder
index 38741fe6662..d56234e6c1a 100644
--- a/app/views/events/_event.atom.builder
+++ b/app/views/events/_event.atom.builder
@@ -10,7 +10,7 @@ xml.entry do
   # eager-loaded. This allows us to re-use the user object's Email address,
   # instead of having to run additional queries to figure out what Email to use
   # for the avatar.
-  xml.media   :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(event.author))
+  xml.media   :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_user(event.author))
 
   xml.author do
     xml.username event.author_username
diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml
index e7fc83a8d04..1d00ae928f6 100644
--- a/app/views/layouts/header/_default.html.haml
+++ b/app/views/layouts/header/_default.html.haml
@@ -45,7 +45,7 @@
                   = todos_count_format(todos_pending_count)
             %li.header-user.dropdown
               = link_to current_user, class: user_dropdown_class, data: { toggle: "dropdown" } do
-                = image_tag avatar_icon(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar"
+                = image_tag avatar_icon_for_user(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar"
                 = sprite_icon('angle-down', css_class: 'caret-down')
               .dropdown-menu-nav.dropdown-menu-align-right
                 %ul
diff --git a/app/views/notify/pipeline_failed_email.html.haml b/app/views/notify/pipeline_failed_email.html.haml
index 8eb3f2d5192..13b591533ba 100644
--- a/app/views/notify/pipeline_failed_email.html.haml
+++ b/app/views/notify/pipeline_failed_email.html.haml
@@ -100,7 +100,7 @@
             triggered by
           - if @pipeline.user
             %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" }
-              %img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+              %img.avatar{ height: "24", src: avatar_icon_for_user(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
             %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" }
               %a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" }
                 = @pipeline.user.name
diff --git a/app/views/notify/pipeline_success_email.html.haml b/app/views/notify/pipeline_success_email.html.haml
index bae37292d62..a4ca8269c36 100644
--- a/app/views/notify/pipeline_success_email.html.haml
+++ b/app/views/notify/pipeline_success_email.html.haml
@@ -100,7 +100,7 @@
             triggered by
           - if @pipeline.user
             %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" }
-              %img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+              %img.avatar{ height: "24", src: avatar_icon_for_user(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
             %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" }
               %a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" }
                 = @pipeline.user.name
diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml
index 5c76d2d8f51..110736dc557 100644
--- a/app/views/profiles/show.html.haml
+++ b/app/views/profiles/show.html.haml
@@ -20,8 +20,8 @@
             or change it at #{link_to Gitlab.config.gravatar.host, 'https://' + Gitlab.config.gravatar.host}
     .col-lg-8
       .clearfix.avatar-image.append-bottom-default
-        = link_to avatar_icon(@user, 400), target: '_blank', rel: 'noopener noreferrer' do
-          = image_tag avatar_icon(@user, 160), alt: '', class: 'avatar s160'
+        = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do
+          = image_tag avatar_icon_for_user(@user, 160), alt: '', class: 'avatar s160'
       %h5.prepend-top-0= _("Upload new avatar")
       .prepend-top-5.append-bottom-10
         %button.btn.js-choose-user-avatar-button{ type: 'button' }= _("Choose file...")
diff --git a/app/views/projects/jobs/_user.html.haml b/app/views/projects/jobs/_user.html.haml
index 83f299da651..461d503f95d 100644
--- a/app/views/projects/jobs/_user.html.haml
+++ b/app/views/projects/jobs/_user.html.haml
@@ -1,7 +1,7 @@
 by
 %a{ href: user_path(@build.user) }
   %span.hidden-xs
-    = image_tag avatar_icon(@build.user, 24), class: "avatar s24"
+    = image_tag avatar_icon_for_user(@build.user, 24), class: "avatar s24"
     %strong{ data: { toggle: 'tooltip', placement: 'top', title: @build.user.to_reference } }
       = @build.user.name
   %strong.visible-xs-inline= @build.user.to_reference
diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
index a8692b83b07..55d0e8bb7f9 100644
--- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
+++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
@@ -21,7 +21,7 @@
         = s_("PipelineSchedules|Inactive")
     %td
       - if pipeline_schedule.owner
-        = image_tag avatar_icon(pipeline_schedule.owner, 20), class: "avatar s20"
+        = image_tag avatar_icon_for_user(pipeline_schedule.owner, 20), class: "avatar s20"
         = link_to user_path(pipeline_schedule.owner) do
           = pipeline_schedule.owner&.name
     %td
diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml
index c4a5131c1a7..57a0b64bfd5 100644
--- a/app/views/search/results/_snippet_blob.html.haml
+++ b/app/views/search/results/_snippet_blob.html.haml
@@ -7,7 +7,7 @@
     = snippet.title
     by
     = link_to user_snippets_path(snippet.author) do
-      = image_tag avatar_icon(snippet.author), class: "avatar avatar-inline s16", alt: ''
+      = image_tag avatar_icon_for_user(snippet.author), class: "avatar avatar-inline s16", alt: ''
       = snippet.author_name
     %span.light= time_ago_with_tooltip(snippet.created_at)
   %h4.snippet-title
diff --git a/app/views/search/results/_snippet_title.html.haml b/app/views/search/results/_snippet_title.html.haml
index aef825691e0..65710c09a89 100644
--- a/app/views/search/results/_snippet_title.html.haml
+++ b/app/views/search/results/_snippet_title.html.haml
@@ -18,6 +18,6 @@
     %span
       by
       = link_to user_snippets_path(snippet_title.author) do
-        = image_tag avatar_icon(snippet_title.author), class: "avatar avatar-inline s16", alt: ''
+        = image_tag avatar_icon_for_user(snippet_title.author), class: "avatar avatar-inline s16", alt: ''
         = snippet_title.author_name
       %span.light= time_ago_with_tooltip(snippet_title.created_at)
diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml
index 71878e93255..a6a1ceca640 100644
--- a/app/views/shared/members/_member.html.haml
+++ b/app/views/shared/members/_member.html.haml
@@ -8,7 +8,7 @@
 %li.member{ class: dom_class(member), id: dom_id(member) }
   %span.list-item-name
     - if user
-      = image_tag avatar_icon(user, 40), class: "avatar s40", alt: ''
+      = image_tag avatar_icon_for_user(user, 40), class: "avatar s40", alt: ''
       .user-info
         = link_to user.name, user_path(user), class: 'member'
         %span.cgray= user.to_reference
diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml
index 14395bcc661..479b7270b28 100644
--- a/app/views/shared/milestones/_issuable.html.haml
+++ b/app/views/shared/milestones/_issuable.html.haml
@@ -28,4 +28,4 @@
       - assignees.each do |assignee|
         = link_to polymorphic_path(issuable_type_args, { milestone_title: @milestone.title, assignee_id: assignee.id, state: 'all' }),
                   class: 'has-tooltip', title: "Assigned to #{assignee.name}", data: { container: 'body' } do
-          - image_tag(avatar_icon(assignee, 16), class: "avatar s16", alt: '')
+          - image_tag(avatar_icon_for_user(assignee, 16), class: "avatar s16", alt: '')
diff --git a/app/views/shared/milestones/_participants_tab.html.haml b/app/views/shared/milestones/_participants_tab.html.haml
index 1615871385e..fe83040c168 100644
--- a/app/views/shared/milestones/_participants_tab.html.haml
+++ b/app/views/shared/milestones/_participants_tab.html.haml
@@ -2,7 +2,7 @@
   - users.each do |user|
     %li
       = link_to user, title: user.name, class: "darken" do
-        = image_tag avatar_icon(user, 32), class: "avatar s32"
+        = image_tag avatar_icon_for_user(user, 32), class: "avatar s32"
         %strong= truncate(user.name, length: 40)
         %div
           %small.cgray= user.username
diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml
index 98e0161f7d1..bf359774ead 100644
--- a/app/views/shared/notes/_note.html.haml
+++ b/app/views/shared/notes/_note.html.haml
@@ -16,7 +16,7 @@
         = icon_for_system_note(note)
       - else
         %a.image-diff-avatar-link{ href: user_path(note.author) }
-          = image_tag avatar_icon(note.author), alt: '', class: 'avatar s40'
+          = image_tag avatar_icon_for_user(note.author), alt: '', class: 'avatar s40'
           - if note.is_a?(DiffNote) && note.on_image?
             - if show_image_comment_badge && note_counter == 0
               -# Only show this for the first comment in the discussion
diff --git a/app/views/shared/notes/_notes_with_form.html.haml b/app/views/shared/notes/_notes_with_form.html.haml
index e11f778adf5..b3f865c5b47 100644
--- a/app/views/shared/notes/_notes_with_form.html.haml
+++ b/app/views/shared/notes/_notes_with_form.html.haml
@@ -14,7 +14,7 @@
 
         .timeline-icon.hidden-xs.hidden-sm
           %a.author_link{ href: user_path(current_user) }
-            = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40'
+            = image_tag avatar_icon_for_user(current_user), alt: current_user.to_reference, class: 'avatar s40'
         .timeline-content.timeline-content-form
           = render "shared/notes/form", view: diff_view, supports_autocomplete: autocomplete
 - elsif !current_user
diff --git a/app/views/shared/snippets/_snippet.html.haml b/app/views/shared/snippets/_snippet.html.haml
index 57b445321e2..491a8a41090 100644
--- a/app/views/shared/snippets/_snippet.html.haml
+++ b/app/views/shared/snippets/_snippet.html.haml
@@ -1,7 +1,7 @@
 - link_project = local_assigns.fetch(:link_project, false)
 
 %li.snippet-row
-  = image_tag avatar_icon(snippet.author), class: "avatar s40 hidden-xs", alt: ''
+  = image_tag avatar_icon_for_user(snippet.author), class: "avatar s40 hidden-xs", alt: ''
 
   .title
     = link_to reliable_snippet_path(snippet) do
diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml
index 90aa1be30ac..c9a77d668a2 100644
--- a/app/views/users/show.html.haml
+++ b/app/views/users/show.html.haml
@@ -35,8 +35,8 @@
 
     .profile-header
       .avatar-holder
-        = link_to avatar_icon(@user, 400), target: '_blank', rel: 'noopener noreferrer' do
-          = image_tag avatar_icon(@user, 90), class: "avatar s90", alt: ''
+        = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do
+          = image_tag avatar_icon_for_user(@user, 90), class: "avatar s90", alt: ''
 
       .user-info
         .cover-title
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index c0251bf7dc0..b67fee2fcc0 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -215,7 +215,7 @@ describe ProjectsHelper do
     let(:expected) { double }
 
     before do
-      expect(helper).to receive(:avatar_icon).with(user, 16).and_return(expected)
+      expect(helper).to receive(:avatar_icon_for_user).with(user, 16).and_return(expected)
     end
 
     it 'returns image tag for member avatar' do
-- 
cgit v1.2.1


From a5e271b6843a3c1b2e59a5b470dfef6d0120d38c Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 12:10:53 +0100
Subject: Use more specific #avatar_icon_for_email.

---
 app/views/help/ui.html.haml                | 2 +-
 app/views/projects/network/show.json.erb   | 2 +-
 app/views/shared/members/_member.html.haml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/views/help/ui.html.haml b/app/views/help/ui.html.haml
index 445f0dffbcc..1c4d67a8d2c 100644
--- a/app/views/help/ui.html.haml
+++ b/app/views/help/ui.html.haml
@@ -68,7 +68,7 @@
   .example
     .cover-block
       .avatar-holder
-        = image_tag avatar_icon('admin@example.com', 90), class: "avatar s90", alt: ''
+        = image_tag avatar_icon_for_email('admin@example.com', 90), class: "avatar s90", alt: ''
       .cover-title
         John Smith
 
diff --git a/app/views/projects/network/show.json.erb b/app/views/projects/network/show.json.erb
index 7491b37310d..a0e82e891ff 100644
--- a/app/views/projects/network/show.json.erb
+++ b/app/views/projects/network/show.json.erb
@@ -9,7 +9,7 @@
         author: {
           name: c.author_name,
           email: c.author_email,
-          icon: image_path(avatar_icon(c.author_email, 20))
+          icon: image_path(avatar_icon_for_email(c.author_email, 20))
         },
         time: c.time,
         space: c.spaces.first,
diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml
index a6a1ceca640..ba57d922c6d 100644
--- a/app/views/shared/members/_member.html.haml
+++ b/app/views/shared/members/_member.html.haml
@@ -36,7 +36,7 @@
               Expires in #{distance_of_time_in_words_to_now(member.expires_at)}
 
     - else
-      = image_tag avatar_icon(member.invite_email, 40), class: "avatar s40", alt: ''
+      = image_tag avatar_icon_for_email(member.invite_email, 40), class: "avatar s40", alt: ''
       .user-info
         .member= member.invite_email
         .cgray
-- 
cgit v1.2.1


From 9adafff0b22d7c9593ca470cd08298d45b9cb732 Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 12:24:48 +0100
Subject: Retrieve issue author's avatar by user, not user#email.

---
 app/views/issues/_issue.atom.builder | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/issues/_issue.atom.builder b/app/views/issues/_issue.atom.builder
index 0c113c08526..21cf6d0dd65 100644
--- a/app/views/issues/_issue.atom.builder
+++ b/app/views/issues/_issue.atom.builder
@@ -3,7 +3,7 @@ xml.entry do
   xml.link    href: project_issue_url(issue.project, issue)
   xml.title   truncate(issue.title, length: 80)
   xml.updated issue.updated_at.xmlschema
-  xml.media   :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(issue.author_email))
+  xml.media   :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_user(issue.author))
 
   xml.author do
     xml.name issue.author_name
-- 
cgit v1.2.1


From c015ea818d1c08382c96ed719b7c3d7f8ce78e61 Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 12:30:35 +0100
Subject: Retrieve namespace owner's avatar by owner, not owner#email.

---
 app/helpers/namespaces_helper.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb
index b78d3072186..40ca666f1bf 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -33,7 +33,7 @@ module NamespacesHelper
     if namespace.is_a?(Group)
       group_icon(namespace)
     else
-      avatar_icon(namespace.owner.email, size)
+      avatar_icon_for_user(namespace.owner, size)
     end
   end
 
-- 
cgit v1.2.1


From 2e6dea7aab9daa2c876b7a77f51821b0fca07b7f Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 12:32:08 +0100
Subject: Retrieve project creator's avatar by creator, not creator#email.

---
 app/views/shared/projects/_project.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml
index 2a75b46d376..33435216c14 100644
--- a/app/views/shared/projects/_project.html.haml
+++ b/app/views/shared/projects/_project.html.haml
@@ -17,7 +17,7 @@
       .avatar-container.s40
         = link_to project_path(project), class: dom_class(project) do
           - if project.creator && use_creator_avatar
-            = image_tag avatar_icon(project.creator.email, 40), class: "avatar s40", alt:''
+            = image_tag avatar_icon_for_user(project.creator, 40), class: "avatar s40", alt:''
           - else
             = project_icon(project, alt: '', class: 'avatar project-avatar s40')
     .project-details
-- 
cgit v1.2.1


From d5ffcb81b6f9d0750fac75d1655dc8d77c07bc85 Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 12:36:35 +0100
Subject: Use avatar_icon_for_email for commits.

We need to keep the email lookup as we want to fall back to displaying
the gravatar result for a given email if the user does not exist.
---
 app/views/projects/commits/_commit.atom.builder | 2 +-
 app/views/projects/repositories/_feed.html.haml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/views/projects/commits/_commit.atom.builder b/app/views/projects/commits/_commit.atom.builder
index 04914888763..50f7e7a3a33 100644
--- a/app/views/projects/commits/_commit.atom.builder
+++ b/app/views/projects/commits/_commit.atom.builder
@@ -3,7 +3,7 @@ xml.entry do
   xml.link    href: project_commit_url(@project, id: commit.id)
   xml.title   truncate(commit.title, length: 80, escape: false)
   xml.updated commit.committed_date.xmlschema
-  xml.media   :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(commit.author_email))
+  xml.media   :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_email(commit.author_email))
 
   xml.author do |author|
     xml.name commit.author_name
diff --git a/app/views/projects/repositories/_feed.html.haml b/app/views/projects/repositories/_feed.html.haml
index 170f9e259df..87895a15239 100644
--- a/app/views/projects/repositories/_feed.html.haml
+++ b/app/views/projects/repositories/_feed.html.haml
@@ -11,7 +11,7 @@
     %div
       = link_to project_commits_path(@project, commit.id) do
         %code= commit.short_id
-      = image_tag avatar_icon(commit.author_email), class: "", width: 16, alt: ''
+      = image_tag avatar_icon_for_email(commit.author_email), class: "", width: 16, alt: ''
       = markdown(truncate(commit.title, length: 40), pipeline: :single_line, author: commit.author)
   %td
     %span.pull-right.cgray
-- 
cgit v1.2.1


From c641dff09422522b7de651fc77292d89f137746b Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 13:07:19 +0100
Subject: Explicit use of avatar_icon_* calls depending on situation.

We want to drop the generic #avatar_icon helper that supports both an email
and a user object being passed in. Instead, we want to explicitly use the #avatar_icon_for_user
and #avatar_icon_for_email helpers depending on what we have at hand.
This allows us to avoid unnecessary database queries (e.g. call
User.find_by_any_email if we already have the user).

In situations like here, this makes it less convenient to use.
---
 app/helpers/avatars_helper.rb                     | 10 +++++++++-
 app/views/notify/pipeline_failed_email.html.haml  | 10 ++++++++--
 app/views/notify/pipeline_success_email.html.haml | 10 ++++++++--
 spec/helpers/avatars_helper_spec.rb               | 16 ++++++++--------
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index be11d453898..16e9fbfd3aa 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -11,7 +11,15 @@ module AvatarsHelper
   def user_avatar_without_link(options = {})
     avatar_size = options[:size] || 16
     user_name = options[:user].try(:name) || options[:user_name]
-    avatar_url = options[:url] || avatar_icon(options[:user] || options[:user_email], avatar_size)
+
+    avatar_url = if options[:url]
+                   options[:url]
+                 elsif options[:user]
+                   avatar_icon_for_user(options[:user], avatar_size)
+                 else
+                   avatar_icon_for_email(options[:user_email], avatar_size)
+                 end
+
     has_tooltip = options[:has_tooltip].nil? ? true : options[:has_tooltip]
     data_attributes = options[:data] || {}
     css_class = %W[avatar s#{avatar_size}].push(*options[:css_class])
diff --git a/app/views/notify/pipeline_failed_email.html.haml b/app/views/notify/pipeline_failed_email.html.haml
index 13b591533ba..7ec10982b5b 100644
--- a/app/views/notify/pipeline_failed_email.html.haml
+++ b/app/views/notify/pipeline_failed_email.html.haml
@@ -60,7 +60,10 @@
               %tbody
                 %tr
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
-                    %img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                    - if commit.author
+                      %img.avatar{ height: "24", src: avatar_icon_for_user(commit.author, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                    - else
+                      %img.avatar{ height: "24", src: avatar_icon_for_email(commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                     - if commit.author
                       %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" }
@@ -76,7 +79,10 @@
                 %tbody
                   %tr
                     %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
-                      %img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                      - if commit.commiter
+                        %img.avatar{ height: "24", src: avatar_icon_for_user(commit.committer, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                      - else
+                        %img.avatar{ height: "24", src: avatar_icon_for_email(commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
                     %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                       - if commit.committer
                         %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" }
diff --git a/app/views/notify/pipeline_success_email.html.haml b/app/views/notify/pipeline_success_email.html.haml
index a4ca8269c36..04f40285df9 100644
--- a/app/views/notify/pipeline_success_email.html.haml
+++ b/app/views/notify/pipeline_success_email.html.haml
@@ -60,7 +60,10 @@
               %tbody
                 %tr
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
-                    %img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                    - if commit.author
+                      %img.avatar{ height: "24", src: avatar_icon_for_user(commit.author, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                    - else
+                      %img.avatar{ height: "24", src: avatar_icon_for_email(commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                     - if commit.author
                       %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" }
@@ -76,7 +79,10 @@
                 %tbody
                   %tr
                     %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
-                      %img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                      - if commit.committer
+                        %img.avatar{ height: "24", src: avatar_icon_for_user(commit.committer, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                      - else
+                        %img.avatar{ height: "24", src: avatar_icon_for_email(commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
                     %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                       - if commit.committer
                         %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" }
diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb
index f44e7ef6843..04c6d259135 100644
--- a/spec/helpers/avatars_helper_spec.rb
+++ b/spec/helpers/avatars_helper_spec.rb
@@ -29,7 +29,7 @@ describe AvatarsHelper do
       is_expected.to eq tag(
         :img,
         alt: "#{user.name}'s avatar",
-        src: avatar_icon(user, 16),
+        src: avatar_icon_for_user(user, 16),
         data: { container: 'body' },
         class: 'avatar s16 has-tooltip',
         title: user.name
@@ -43,7 +43,7 @@ describe AvatarsHelper do
         is_expected.to eq tag(
           :img,
           alt: "#{user.name}'s avatar",
-          src: avatar_icon(user, 16),
+          src: avatar_icon_for_user(user, 16),
           data: { container: 'body' },
           class: "avatar s16 #{options[:css_class]} has-tooltip",
           title: user.name
@@ -58,7 +58,7 @@ describe AvatarsHelper do
         is_expected.to eq tag(
           :img,
           alt: "#{user.name}'s avatar",
-          src: avatar_icon(user, options[:size]),
+          src: avatar_icon_for_user(user, options[:size]),
           data: { container: 'body' },
           class: "avatar s#{options[:size]} has-tooltip",
           title: user.name
@@ -89,7 +89,7 @@ describe AvatarsHelper do
           :img,
           alt: "#{user.name}'s avatar",
           src: LazyImageTagHelper.placeholder_image,
-          data: { container: 'body', src: avatar_icon(user, 16) },
+          data: { container: 'body', src: avatar_icon_for_user(user, 16) },
           class: "avatar s16 has-tooltip lazy",
           title: user.name
         )
@@ -104,7 +104,7 @@ describe AvatarsHelper do
           is_expected.to eq tag(
             :img,
             alt: "#{user.name}'s avatar",
-            src: avatar_icon(user, 16),
+            src: avatar_icon_for_user(user, 16),
             data: { container: 'body' },
             class: "avatar s16 has-tooltip",
             title: user.name
@@ -119,7 +119,7 @@ describe AvatarsHelper do
           is_expected.to eq tag(
             :img,
             alt: "#{user.name}'s avatar",
-            src: avatar_icon(user, 16),
+            src: avatar_icon_for_user(user, 16),
             class: "avatar s16",
             title: user.name
           )
@@ -137,7 +137,7 @@ describe AvatarsHelper do
           is_expected.to eq tag(
             :img,
             alt: "#{user.name}'s avatar",
-            src: avatar_icon(user, 16),
+            src: avatar_icon_for_user(user, 16),
             data: { container: 'body' },
             class: "avatar s16 has-tooltip",
             title: user.name
@@ -149,7 +149,7 @@ describe AvatarsHelper do
         is_expected.to eq tag(
           :img,
           alt: "#{options[:user_name]}'s avatar",
-          src: avatar_icon(options[:user_email], 16),
+          src: avatar_icon_for_email(options[:user_email], 16),
           data: { container: 'body' },
           class: "avatar s16 has-tooltip",
           title: options[:user_name]
-- 
cgit v1.2.1


From c4cf7220146f74196ef20b12cf0db3502649ac06 Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 13:28:59 +0100
Subject: Remove generic #avatar_icon helper.

---
 app/helpers/application_helper.rb       |  8 --------
 spec/helpers/application_helper_spec.rb | 28 ----------------------------
 2 files changed, 36 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 77c86be4714..a889decab26 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -68,14 +68,6 @@ module ApplicationHelper
     end
   end
 
-  def avatar_icon(user_or_email = nil, size = nil, scale = 2, only_path: true)
-    if user_or_email.is_a?(User)
-      avatar_icon_for_user(user_or_email, size, scale, only_path: only_path)
-    else
-      avatar_icon_for_email(user_or_email, size, scale, only_path: only_path)
-    end
-  end
-
   def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
     user = User.find_by_any_email(email.try(:downcase))
     if user
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index a1fcdeca10e..96bd4c96ae4 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -63,34 +63,6 @@ describe ApplicationHelper do
     end
   end
 
-  describe 'avatar_icon' do
-    let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
-
-    context 'using an email' do
-      context 'when there is a matching user' do
-        it 'returns a relative URL for the avatar' do
-          expect(helper.avatar_icon(user.email).to_s)
-            .to eq(user.avatar.url)
-        end
-      end
-
-      context 'when no user exists for the email' do
-        it 'calls gravatar_icon' do
-          expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
-
-          helper.avatar_icon('foo@example.com', 20, 2)
-        end
-      end
-    end
-
-    describe 'using a user' do
-      it 'returns a relative URL for the avatar' do
-        expect(helper.avatar_icon(user).to_s)
-          .to eq(user.avatar.url)
-      end
-    end
-  end
-
   describe 'avatar_icon_for_email' do
     let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
 
-- 
cgit v1.2.1


From 4f3d75326cb8a2f94a93bb3a32bf204ddfab806b Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Fri, 9 Feb 2018 22:55:16 +0100
Subject: Extract method to improve readability.

---
 app/helpers/avatars_helper.rb | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index 16e9fbfd3aa..21b6c0a8ad5 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -8,17 +8,21 @@ module AvatarsHelper
     }))
   end
 
+  def user_avatar_url_for(options = {})
+    if options[:url]
+      options[:url]
+    elsif options[:user]
+      avatar_icon_for_user(options[:user], options[:size])
+    else
+      avatar_icon_for_email(options[:user_email], options[:size])
+    end
+  end
+
   def user_avatar_without_link(options = {})
     avatar_size = options[:size] || 16
     user_name = options[:user].try(:name) || options[:user_name]
 
-    avatar_url = if options[:url]
-                   options[:url]
-                 elsif options[:user]
-                   avatar_icon_for_user(options[:user], avatar_size)
-                 else
-                   avatar_icon_for_email(options[:user_email], avatar_size)
-                 end
+    avatar_url = user_avatar_url_for(options.merge(size: avatar_size))
 
     has_tooltip = options[:has_tooltip].nil? ? true : options[:has_tooltip]
     data_attributes = options[:data] || {}
-- 
cgit v1.2.1


From 95738d4b75a132210eb1d7c9ac7d35e4ea4a86bd Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Mon, 12 Feb 2018 18:07:43 +0100
Subject: Always eagerly load an issue's author.

Rather radical but avoids n+1 queries in the rather common case we want
to include information about the author.
---
 app/finders/issues_finder.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb
index 98831f5be4a..83245aadf6e 100644
--- a/app/finders/issues_finder.rb
+++ b/app/finders/issues_finder.rb
@@ -21,7 +21,7 @@ class IssuesFinder < IssuableFinder
   CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::REPORTER
 
   def klass
-    Issue
+    Issue.includes(:author)
   end
 
   def with_confidentiality_access_check
-- 
cgit v1.2.1


From c11e80699783ca2915a59c69f234d8d3aff256d8 Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Mon, 12 Feb 2018 18:20:42 +0100
Subject: Always eagerly load a note's author.

---
 app/finders/notes_finder.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb
index 12157818bcd..33ee1e975b9 100644
--- a/app/finders/notes_finder.rb
+++ b/app/finders/notes_finder.rb
@@ -57,7 +57,7 @@ class NotesFinder
     types = %w(commit issue merge_request snippet)
     note_relations = types.map { |t| notes_for_type(t) }
     note_relations.map! { |notes| search(notes) }
-    UnionFinder.new.find_union(note_relations, Note)
+    UnionFinder.new.find_union(note_relations, Note.includes(:author))
   end
 
   def noteables_for_type(noteable_type)
-- 
cgit v1.2.1


From 6670074c0663be55f28de7cdb04a27432c7304f2 Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Mon, 12 Feb 2018 18:25:38 +0100
Subject: Add changelog.

Closes #42800.
---
 changelogs/unreleased/42800-change-usage-of-avatar_icon.yml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 changelogs/unreleased/42800-change-usage-of-avatar_icon.yml

diff --git a/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml b/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml
new file mode 100644
index 00000000000..00f4b7436a7
--- /dev/null
+++ b/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml
@@ -0,0 +1,6 @@
+---
+title: Use a user object in ApplicationHelper#avatar_icon where possible to avoid
+  N+1 queries.
+merge_request: 42800
+author:
+type: performance
-- 
cgit v1.2.1


From dd1d13b859c4c4cd7b6a64eb93f761c10c9262d4 Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Tue, 13 Feb 2018 17:02:06 +0100
Subject: Extract repeated logic into #avatar_icon_for.

This essentially allows to pass both user and email, so that we can
either prefer the user to retrieve the avatar or (if user is not
present) fall back to the email lookup.
---
 app/helpers/application_helper.rb                 | 12 ++++++++++++
 app/views/notify/pipeline_failed_email.html.haml  | 10 ++--------
 app/views/notify/pipeline_success_email.html.haml | 10 ++--------
 spec/helpers/application_helper_spec.rb           | 16 ++++++++++++++++
 4 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index a889decab26..a6011eb9f30 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -68,6 +68,18 @@ module ApplicationHelper
     end
   end
 
+  # Takes both user and email and returns the avatar_icon by
+  # user (preferred) or email.
+  def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true)
+    if user
+      avatar_icon_for_user(user, size, scale, only_path: only_path)
+    elsif email
+      avatar_icon_for_email(email, size, scale, only_path: only_path)
+    else
+      default_avatar
+    end
+  end
+
   def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
     user = User.find_by_any_email(email.try(:downcase))
     if user
diff --git a/app/views/notify/pipeline_failed_email.html.haml b/app/views/notify/pipeline_failed_email.html.haml
index 7ec10982b5b..38dab104eb5 100644
--- a/app/views/notify/pipeline_failed_email.html.haml
+++ b/app/views/notify/pipeline_failed_email.html.haml
@@ -60,10 +60,7 @@
               %tbody
                 %tr
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
-                    - if commit.author
-                      %img.avatar{ height: "24", src: avatar_icon_for_user(commit.author, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
-                    - else
-                      %img.avatar{ height: "24", src: avatar_icon_for_email(commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                    %img.avatar{ height: "24", src: avatar_icon_for(commit.author, commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                     - if commit.author
                       %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" }
@@ -79,10 +76,7 @@
                 %tbody
                   %tr
                     %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
-                      - if commit.commiter
-                        %img.avatar{ height: "24", src: avatar_icon_for_user(commit.committer, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
-                      - else
-                        %img.avatar{ height: "24", src: avatar_icon_for_email(commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                      %img.avatar{ height: "24", src: avatar_icon_for(commit.committer, commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
                     %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                       - if commit.committer
                         %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" }
diff --git a/app/views/notify/pipeline_success_email.html.haml b/app/views/notify/pipeline_success_email.html.haml
index 04f40285df9..7b06e8afa0b 100644
--- a/app/views/notify/pipeline_success_email.html.haml
+++ b/app/views/notify/pipeline_success_email.html.haml
@@ -60,10 +60,7 @@
               %tbody
                 %tr
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
-                    - if commit.author
-                      %img.avatar{ height: "24", src: avatar_icon_for_user(commit.author, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
-                    - else
-                      %img.avatar{ height: "24", src: avatar_icon_for_email(commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                    %img.avatar{ height: "24", src: avatar_icon_for(commit.author, commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                     - if commit.author
                       %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" }
@@ -79,10 +76,7 @@
                 %tbody
                   %tr
                     %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
-                      - if commit.committer
-                        %img.avatar{ height: "24", src: avatar_icon_for_user(commit.committer, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
-                      - else
-                        %img.avatar{ height: "24", src: avatar_icon_for_email(commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                      %img.avatar{ height: "24", src: avatar_icon_for(commit.committer, commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
                     %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                       - if commit.committer
                         %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" }
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 96bd4c96ae4..43cb0dfe163 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -63,6 +63,22 @@ describe ApplicationHelper do
     end
   end
 
+  describe 'avatar_icon_for' do
+    let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') }
+    let(:email) { 'foo@example.com' }
+    let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) }
+
+    it 'prefers the user to retrieve the avatar_url' do
+      expect(helper.avatar_icon_for(user, email).to_s)
+        .to eq(user.avatar.url)
+    end
+
+    it 'falls back to email lookup if no user given' do
+      expect(helper.avatar_icon_for(nil, email).to_s)
+        .to eq(another_user.avatar.url)
+    end
+  end
+
   describe 'avatar_icon_for_email' do
     let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
 
-- 
cgit v1.2.1