Extract repeated logic into #avatar_icon_for.42800-change-usage-of-avatar_icon

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.

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)) }