From 79c80de99d511d084cff072bd90192ffe8ba4cda Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Fri, 22 Sep 2017 15:49:56 +0200
Subject: [PATCH 01/12] Making private project avatars use local paths + Some
 Group Icons

---
 app/helpers/application_helper.rb    |  7 ++++++-
 app/helpers/groups_helper.rb         |  9 +++++++--
 app/helpers/lazy_image_tag_helper.rb |  6 +++++-
 app/models/concerns/avatarable.rb    | 14 +++++++++-----
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 8d02d5de5c3..cfdb95e2498 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -41,7 +41,12 @@ module ApplicationHelper
       end
 
     if project.avatar_url
-      image_tag project.avatar_url, options
+      if project.private?
+        options[:use_original_source] = true
+        image_tag project.avatar_url(use_asset_path: false), options
+      else
+        image_tag project.avatar_url, options
+      end
     else # generated icon
       project_identicon(project, options)
     end
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 82bceddf1f0..091d98a2c94 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -12,7 +12,7 @@ module GroupsHelper
       group = Group.find_by_full_path(group)
     end
 
-    group.try(:avatar_url) || ActionController::Base.helpers.image_path('no_group_avatar.png')
+    group.try(:avatar_url, use_asset_path: false) || ActionController::Base.helpers.image_path('no_group_avatar.png')
   end
 
   def group_title(group, name = nil, url = nil)
@@ -89,7 +89,12 @@ module GroupsHelper
     link_to(group_path(group), class: "group-path #{'breadcrumb-item-text' unless for_dropdown} js-breadcrumb-item-text #{'hidable' if hidable}") do
       output =
         if (group.try(:avatar_url) || show_avatar) && !Rails.env.test?
-          image_tag(group_icon(group), class: "avatar-tile", width: 15, height: 15)
+          if group.private?
+            puts "GROUP IS PRIVATE : " + group_icon(group)
+            image_tag(group_icon(group), class: "avatar-tile", width: 15, height: 15, use_original_source: true)
+          else
+            image_tag(group_icon(group), class: "avatar-tile", width: 15, height: 15)
+          end
         else
           ""
         end
diff --git a/app/helpers/lazy_image_tag_helper.rb b/app/helpers/lazy_image_tag_helper.rb
index 2c5619ac41b..aea1fcc7e91 100644
--- a/app/helpers/lazy_image_tag_helper.rb
+++ b/app/helpers/lazy_image_tag_helper.rb
@@ -9,7 +9,11 @@ module LazyImageTagHelper
 
     unless options.delete(:lazy) == false
       options[:data] ||= {}
-      options[:data][:src] = path_to_image(source)
+      unless options.delete(:use_original_source) == true
+        options[:data][:src] = path_to_image(source)
+      else
+        options[:data][:src] = source
+      end
       options[:class] ||= ""
       options[:class] << " lazy"
 
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index 8fbfed11bdf..f8c0327e190 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -1,7 +1,7 @@
 module Avatarable
   extend ActiveSupport::Concern
 
-  def avatar_path(only_path: true)
+  def avatar_path(only_path: true, use_asset_path: true)
     return unless self[:avatar].present?
 
     # If only_path is true then use the relative path of avatar.
@@ -9,10 +9,14 @@ module Avatarable
     asset_host = ActionController::Base.asset_host
     gitlab_host = only_path ? gitlab_config.relative_url_root : gitlab_config.url
 
-    # If asset_host is set then it is expected that assets are handled by a standalone host.
-    # That means we do not want to get GitLab's relative_url_root option anymore.
-    host = asset_host.present? ? asset_host : gitlab_host
+    if use_asset_path
+      # If asset_host is set then it is expected that assets are handled by a standalone host.
+      # That means we do not want to get GitLab's relative_url_root option anymore.
+      host = asset_host.present? ? asset_host : gitlab_host
 
-    [host, avatar.url].join
+      [host, avatar.url].join
+    else
+      avatar.url
+    end
   end
 end

From 892b02e890be539a95e6b52feb14f5d188513700 Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Mon, 2 Oct 2017 13:35:01 +0200
Subject: [PATCH 02/12] Created group_icon and group_icon_url Tests for these
 new helper methods

---
 app/helpers/groups_helper.rb                  | 29 ++++++++++--
 app/models/concerns/avatarable.rb             |  2 +-
 app/serializers/group_entity.rb               |  2 +-
 app/views/admin/groups/_group.html.haml       |  2 +-
 app/views/admin/groups/show.html.haml         |  2 +-
 app/views/groups/_home_panel.html.haml        |  2 +-
 app/views/groups/edit.html.haml               |  2 +-
 .../layouts/nav/sidebar/_group.html.haml      |  2 +-
 app/views/shared/groups/_group.html.haml      |  2 +-
 app/views/shared/members/_group.html.haml     |  2 +-
 app/views/users/_groups.html.haml             |  2 +-
 spec/helpers/groups_helper_spec.rb            | 47 ++++++++++++++++++-
 12 files changed, 79 insertions(+), 17 deletions(-)

diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 091d98a2c94..60ac4c63e62 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -1,4 +1,8 @@
+require 'uri'
+
 module GroupsHelper
+  include Gitlab::CurrentSettings
+
   def can_change_group_visibility_level?(group)
     can?(current_user, :change_visibility_level, group)
   end
@@ -7,12 +11,28 @@ module GroupsHelper
     can?(current_user, :change_share_with_group_lock, group)
   end
 
-  def group_icon(group)
+  # = project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile')
+  # = image_tag group_icon(@group), alt: '', class: 'avatar group-avatar s160'
+  def group_icon(group, options = {})
+    img_path = group_icon_url(group, options)
+    image_tag img_path, options
+  end
+
+  def group_icon_url(group, options = {})
     if group.is_a?(String)
       group = Group.find_by_full_path(group)
     end
 
-    group.try(:avatar_url, use_asset_path: false) || ActionController::Base.helpers.image_path('no_group_avatar.png')
+    if group.avatar_url
+      if group.private?
+        options[:use_original_source] = true
+        group.avatar_url(use_asset_path: false)
+      else
+        group.avatar_url
+      end
+    else # No Avatar Icon
+      ActionController::Base.helpers.image_path('no_group_avatar.png')
+    end
   end
 
   def group_title(group, name = nil, url = nil)
@@ -90,10 +110,9 @@ module GroupsHelper
       output =
         if (group.try(:avatar_url) || show_avatar) && !Rails.env.test?
           if group.private?
-            puts "GROUP IS PRIVATE : " + group_icon(group)
-            image_tag(group_icon(group), class: "avatar-tile", width: 15, height: 15, use_original_source: true)
+            group_icon(group, class: "avatar-tile", width: 15, height: 15, use_original_source: true)
           else
-            image_tag(group_icon(group), class: "avatar-tile", width: 15, height: 15)
+            group_icon(group, class: "avatar-tile", width: 15, height: 15)
           end
         else
           ""
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index f8c0327e190..ddaa15e6a0d 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -16,7 +16,7 @@ module Avatarable
 
       [host, avatar.url].join
     else
-      avatar.url
+      [host, avatar.url].join
     end
   end
 end
diff --git a/app/serializers/group_entity.rb b/app/serializers/group_entity.rb
index 7c872a3e986..6d8466da902 100644
--- a/app/serializers/group_entity.rb
+++ b/app/serializers/group_entity.rb
@@ -45,6 +45,6 @@ class GroupEntity < Grape::Entity
   end
 
   expose :avatar_url do |group|
-    group_icon(group)
+    group_icon_url(group)
   end
 end
diff --git a/app/views/admin/groups/_group.html.haml b/app/views/admin/groups/_group.html.haml
index e3a77dfdf10..47cc2d4d27e 100644
--- a/app/views/admin/groups/_group.html.haml
+++ b/app/views/admin/groups/_group.html.haml
@@ -20,7 +20,7 @@
       = visibility_level_icon(group.visibility_level, fw: false)
 
   .avatar-container.s40
-    = image_tag group_icon(group), class: "avatar s40 hidden-xs"
+    = group_icon(group, class: "avatar s40 hidden-xs")
   .title
     = link_to [:admin, group], class: 'group-name' do
       = group.full_name
diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml
index 3e02f7b1e16..2545cecc721 100644
--- a/app/views/admin/groups/show.html.haml
+++ b/app/views/admin/groups/show.html.haml
@@ -16,7 +16,7 @@
       %ul.well-list
         %li
           .avatar-container.s60
-            = image_tag group_icon(@group), class: "avatar s60"
+            = group_icon(@group, class: "avatar s60")
         %li
           %span.light Name:
           %strong= @group.name
diff --git a/app/views/groups/_home_panel.html.haml b/app/views/groups/_home_panel.html.haml
index 181c7bee702..a0760c2073b 100644
--- a/app/views/groups/_home_panel.html.haml
+++ b/app/views/groups/_home_panel.html.haml
@@ -1,7 +1,7 @@
 .group-home-panel.text-center
   %div{ class: container_class }
     .avatar-container.s70.group-avatar
-      = image_tag group_icon(@group), class: "avatar s70 avatar-tile"
+      = group_icon(@group, class: "avatar s70 avatar-tile")
     %h1.group-title
       = @group.name
       %span.visibility-icon.has-tooltip{ data: { container: 'body' }, title: visibility_icon_description(@group) }
diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml
index 15606dd30fd..16038ef2f79 100644
--- a/app/views/groups/edit.html.haml
+++ b/app/views/groups/edit.html.haml
@@ -10,7 +10,7 @@
       .form-group
         .col-sm-offset-2.col-sm-10
           .avatar-container.s160
-            = image_tag group_icon(@group), alt: '', class: 'avatar group-avatar s160'
+            = group_icon(@group, alt: '', class: 'avatar group-avatar s160')
           %p.light
             - if @group.avatar?
               You can change your group avatar here
diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml
index 8cba495f7e4..0bf318b0b66 100644
--- a/app/views/layouts/nav/sidebar/_group.html.haml
+++ b/app/views/layouts/nav/sidebar/_group.html.haml
@@ -6,7 +6,7 @@
     .context-header
       = link_to group_path(@group), title: @group.name do
         .avatar-container.s40.group-avatar
-          = image_tag group_icon(@group), class: "avatar s40 avatar-tile"
+          = group_icon(@group, class: "avatar s40 avatar-tile")
         .sidebar-context-title
           = @group.name
     %ul.sidebar-top-level-items
diff --git a/app/views/shared/groups/_group.html.haml b/app/views/shared/groups/_group.html.haml
index b361ec86ced..63f62eb476e 100644
--- a/app/views/shared/groups/_group.html.haml
+++ b/app/views/shared/groups/_group.html.haml
@@ -28,7 +28,7 @@
 
   .avatar-container.s40
     = link_to group do
-      = image_tag group_icon(group), class: "avatar s40 hidden-xs"
+      = group_icon(group, class: "avatar s40 hidden-xs")
   .title
     = link_to group_name, group, class: 'group-name'
 
diff --git a/app/views/shared/members/_group.html.haml b/app/views/shared/members/_group.html.haml
index bcdad3c153a..5868c52566d 100644
--- a/app/views/shared/members/_group.html.haml
+++ b/app/views/shared/members/_group.html.haml
@@ -4,7 +4,7 @@
 - dom_id = "group_member_#{group_link.id}"
 %li.member.group_member{ id: dom_id }
   %span.list-item-name
-    = image_tag group_icon(group), class: "avatar s40", alt: ''
+    = group_icon(group, class: "avatar s40", alt: '')
     %strong
       = link_to group.full_name, group_path(group)
     .cgray
diff --git a/app/views/users/_groups.html.haml b/app/views/users/_groups.html.haml
index eff6c80d144..55799e10a46 100644
--- a/app/views/users/_groups.html.haml
+++ b/app/views/users/_groups.html.haml
@@ -2,4 +2,4 @@
   - groups.each do |group|
     = link_to group, class: 'profile-groups-avatars inline', title: group.name do
       .avatar-container.s40
-        = image_tag group_icon(group), class: 'avatar group-avatar s40'
+        = group_icon(group, class: 'avatar group-avatar s40')
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 76e5964ccf7..40c26b6e1d5 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
 describe GroupsHelper do
   include ApplicationHelper
 
+
   describe 'group_icon' do
     avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
 
@@ -10,14 +11,56 @@ describe GroupsHelper do
       group = create(:group)
       group.avatar = fixture_file_upload(avatar_file_path)
       group.save!
-      expect(group_icon(group.path).to_s)
+
+      avatar_url = "/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif"
+
+      expect(group_icon(group).to_s)
+        .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
+
+      allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
+      avatar_url = "#{gitlab_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif"
+
+      expect(group_icon(group).to_s)
+        .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
+    end
+  end
+
+
+
+  describe 'group_icon_url' do
+    avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
+
+    it 'returns an url for the avatar' do
+      group = create(:group)
+      group.avatar = fixture_file_upload(avatar_file_path)
+      group.save!
+      expect(group_icon_url(group.path).to_s)
+        .to match("/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif")
+    end
+
+    it 'returns an CDN url for the avatar' do
+      allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
+      group = create(:group)
+      group.avatar = fixture_file_upload(avatar_file_path)
+      group.save!
+      expect(group_icon_url(group.path).to_s)
+        .to match("#{gitlab_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif")
+    end
+
+    it 'returns an based url for the avatar if private' do
+      allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
+      group = create(:group)
+      group.avatar = fixture_file_upload(avatar_file_path)
+      group.private = true
+      group.save!
+      expect(group_icon_url(group.path).to_s)
         .to match("/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif")
     end
 
     it 'gives default avatar_icon when no avatar is present' do
       group = create(:group)
       group.save!
-      expect(group_icon(group.path)).to match_asset_path('group_avatar.png')
+      expect(group_icon_url(group.path)).to match_asset_path('group_avatar.png')
     end
   end
 

From 1b752968757873434404a2bf82ea360ce829ec87 Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Mon, 2 Oct 2017 15:44:58 +0200
Subject: [PATCH 03/12] Fixed Linting errors + tests

---
 app/helpers/lazy_image_tag_helper.rb    |  2 +-
 app/models/concerns/avatarable.rb       |  6 ++----
 spec/helpers/application_helper_spec.rb |  6 ++++--
 spec/helpers/groups_helper_spec.rb      | 13 ++++++-------
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/app/helpers/lazy_image_tag_helper.rb b/app/helpers/lazy_image_tag_helper.rb
index aea1fcc7e91..60df16b3373 100644
--- a/app/helpers/lazy_image_tag_helper.rb
+++ b/app/helpers/lazy_image_tag_helper.rb
@@ -9,7 +9,7 @@ module LazyImageTagHelper
 
     unless options.delete(:lazy) == false
       options[:data] ||= {}
-      unless options.delete(:use_original_source) == true
+      unless options.delete(:use_original_source)
         options[:data][:src] = path_to_image(source)
       else
         options[:data][:src] = source
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index ddaa15e6a0d..6ed57dea858 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -13,10 +13,8 @@ module Avatarable
       # If asset_host is set then it is expected that assets are handled by a standalone host.
       # That means we do not want to get GitLab's relative_url_root option anymore.
       host = asset_host.present? ? asset_host : gitlab_host
-
-      [host, avatar.url].join
-    else
-      [host, avatar.url].join
     end
+
+    [host, avatar.url].join
   end
 end
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 10bc5f2ecd2..6dafd73d337 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -57,6 +57,8 @@ describe ApplicationHelper do
   end
 
   describe 'project_icon' do
+    let(:asset_host) { 'http://assets' }
+
     it 'returns an url for the avatar' do
       project = create(:project, avatar: File.open(uploaded_image_temp_path))
       avatar_url = "/uploads/-/system/project/avatar/#{project.id}/banana_sample.gif"
@@ -64,8 +66,8 @@ describe ApplicationHelper do
       expect(helper.project_icon(project.full_path).to_s)
         .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
 
-      allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
-      avatar_url = "#{gitlab_host}/uploads/-/system/project/avatar/#{project.id}/banana_sample.gif"
+      allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
+      avatar_url = "#{asset_host}/uploads/-/system/project/avatar/#{project.id}/banana_sample.gif"
 
       expect(helper.project_icon(project.full_path).to_s)
         .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 40c26b6e1d5..55de03c8ed2 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
 describe GroupsHelper do
   include ApplicationHelper
 
+  let(:asset_host) { 'http://assets' }
 
   describe 'group_icon' do
     avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
@@ -17,16 +18,14 @@ describe GroupsHelper do
       expect(group_icon(group).to_s)
         .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
 
-      allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
-      avatar_url = "#{gitlab_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif"
+      allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
+      avatar_url = "#{asset_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif"
 
       expect(group_icon(group).to_s)
         .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
     end
   end
 
-
-
   describe 'group_icon_url' do
     avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
 
@@ -39,16 +38,16 @@ describe GroupsHelper do
     end
 
     it 'returns an CDN url for the avatar' do
-      allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
+      allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
       group = create(:group)
       group.avatar = fixture_file_upload(avatar_file_path)
       group.save!
       expect(group_icon_url(group.path).to_s)
-        .to match("#{gitlab_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif")
+        .to match("#{asset_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif")
     end
 
     it 'returns an based url for the avatar if private' do
-      allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
+      allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
       group = create(:group)
       group.avatar = fixture_file_upload(avatar_file_path)
       group.private = true

From 6168e33e9e2f327f078e07b71c466dcc9fa9813a Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Tue, 3 Oct 2017 12:21:02 +0200
Subject: [PATCH 04/12] Fixed Test

---
 app/helpers/lazy_image_tag_helper.rb | 6 +++---
 spec/helpers/groups_helper_spec.rb   | 3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/app/helpers/lazy_image_tag_helper.rb b/app/helpers/lazy_image_tag_helper.rb
index 60df16b3373..4def806f1c0 100644
--- a/app/helpers/lazy_image_tag_helper.rb
+++ b/app/helpers/lazy_image_tag_helper.rb
@@ -9,10 +9,10 @@ module LazyImageTagHelper
 
     unless options.delete(:lazy) == false
       options[:data] ||= {}
-      unless options.delete(:use_original_source)
-        options[:data][:src] = path_to_image(source)
-      else
+      if options.delete(:use_original_source)
         options[:data][:src] = source
+      else
+        options[:data][:src] = path_to_image(source)        
       end
       options[:class] ||= ""
       options[:class] << " lazy"
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 55de03c8ed2..4fe50d120d7 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -48,9 +48,8 @@ describe GroupsHelper do
 
     it 'returns an based url for the avatar if private' do
       allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
-      group = create(:group)
+      group = create(:group, :private)
       group.avatar = fixture_file_upload(avatar_file_path)
-      group.private = true
       group.save!
       expect(group_icon_url(group.path).to_s)
         .to match("/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif")

From 27577e8e2b7bf18c0dbc402e5efad4905c5a154c Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Wed, 4 Oct 2017 13:58:32 +0200
Subject: [PATCH 05/12] Fixed Tests

---
 app/helpers/lazy_image_tag_helper.rb    | 11 ++++++-----
 spec/helpers/application_helper_spec.rb |  2 +-
 spec/helpers/groups_helper_spec.rb      |  4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/app/helpers/lazy_image_tag_helper.rb b/app/helpers/lazy_image_tag_helper.rb
index 4def806f1c0..2b5e9a9ef57 100644
--- a/app/helpers/lazy_image_tag_helper.rb
+++ b/app/helpers/lazy_image_tag_helper.rb
@@ -9,11 +9,12 @@ module LazyImageTagHelper
 
     unless options.delete(:lazy) == false
       options[:data] ||= {}
-      if options.delete(:use_original_source)
-        options[:data][:src] = source
-      else
-        options[:data][:src] = path_to_image(source)        
-      end
+      options[:data][:src] = if options.delete(:use_original_source)
+                               source
+                             else
+                               path_to_image(source)
+                             end
+
       options[:class] ||= ""
       options[:class] << " lazy"
 
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 6dafd73d337..87ae1fa5660 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -60,7 +60,7 @@ describe ApplicationHelper do
     let(:asset_host) { 'http://assets' }
 
     it 'returns an url for the avatar' do
-      project = create(:project, avatar: File.open(uploaded_image_temp_path))
+      project = create(:project, :public, avatar: File.open(uploaded_image_temp_path))
       avatar_url = "/uploads/-/system/project/avatar/#{project.id}/banana_sample.gif"
 
       expect(helper.project_icon(project.full_path).to_s)
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 4fe50d120d7..97f0ed4904e 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -15,13 +15,13 @@ describe GroupsHelper do
 
       avatar_url = "/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif"
 
-      expect(group_icon(group).to_s)
+      expect(helper.group_icon(group).to_s)
         .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
 
       allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
       avatar_url = "#{asset_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif"
 
-      expect(group_icon(group).to_s)
+      expect(helper.group_icon(group).to_s)
         .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
     end
   end

From d2620faff92f235d5659ccdac0825cf18c7e2389 Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Thu, 5 Oct 2017 16:29:45 +0200
Subject: [PATCH 06/12] Updates based on MR comments

---
 app/helpers/application_helper.rb    | 15 +++++++++------
 app/helpers/lazy_image_tag_helper.rb | 12 +++++++-----
 app/models/concerns/avatarable.rb    |  8 +++-----
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index cfdb95e2498..264f21a1466 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -41,12 +41,15 @@ module ApplicationHelper
       end
 
     if project.avatar_url
-      if project.private?
-        options[:use_original_source] = true
-        image_tag project.avatar_url(use_asset_path: false), options
-      else
-        image_tag project.avatar_url, options
-      end
+      #if project.private?
+      #  options[:use_original_source] = true
+      #  image_tag project.avatar_url(use_asset_path: false), options
+      #else
+      #  image_tag project.avatar_url, options
+      #end
+
+      image_tag project.avatar_url(use_asset_path: project.public?), options
+
     else # generated icon
       project_identicon(project, options)
     end
diff --git a/app/helpers/lazy_image_tag_helper.rb b/app/helpers/lazy_image_tag_helper.rb
index 2b5e9a9ef57..c150c2927a9 100644
--- a/app/helpers/lazy_image_tag_helper.rb
+++ b/app/helpers/lazy_image_tag_helper.rb
@@ -9,11 +9,13 @@ module LazyImageTagHelper
 
     unless options.delete(:lazy) == false
       options[:data] ||= {}
-      options[:data][:src] = if options.delete(:use_original_source)
-                               source
-                             else
-                               path_to_image(source)
-                             end
+      #options[:data][:src] = if options.delete(:use_original_source)
+      #                         source
+      #                       else
+      #                         path_to_image(source)
+      #                       end
+
+      options[:data][:src] = path_to_image(source)
 
       options[:class] ||= ""
       options[:class] << " lazy"
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index 6ed57dea858..dee7e47e2aa 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -9,11 +9,9 @@ module Avatarable
     asset_host = ActionController::Base.asset_host
     gitlab_host = only_path ? gitlab_config.relative_url_root : gitlab_config.url
 
-    if use_asset_path
-      # If asset_host is set then it is expected that assets are handled by a standalone host.
-      # That means we do not want to get GitLab's relative_url_root option anymore.
-      host = asset_host.present? ? asset_host : gitlab_host
-    end
+    # If asset_host is set then it is expected that assets are handled by a standalone host.
+    # That means we do not want to get GitLab's relative_url_root option anymore.
+    host = (asset_host.present? && use_asset_path) ? asset_host : gitlab_host
 
     [host, avatar.url].join
   end

From c786c8dcb334fbacf5b6e4fbf2da1910f94d80c4 Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Thu, 5 Oct 2017 17:49:11 +0200
Subject: [PATCH 07/12] Another Change for cleanup

---
 app/helpers/lazy_image_tag_helper.rb | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/app/helpers/lazy_image_tag_helper.rb b/app/helpers/lazy_image_tag_helper.rb
index c150c2927a9..603b9438e35 100644
--- a/app/helpers/lazy_image_tag_helper.rb
+++ b/app/helpers/lazy_image_tag_helper.rb
@@ -9,12 +9,6 @@ module LazyImageTagHelper
 
     unless options.delete(:lazy) == false
       options[:data] ||= {}
-      #options[:data][:src] = if options.delete(:use_original_source)
-      #                         source
-      #                       else
-      #                         path_to_image(source)
-      #                       end
-
       options[:data][:src] = path_to_image(source)
 
       options[:class] ||= ""

From 3d152cd2058a928ec26efe90240b66e5e62ed677 Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Thu, 5 Oct 2017 23:17:52 +0200
Subject: [PATCH 08/12] Fixed Linting Error

---
 app/helpers/application_helper.rb | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 264f21a1466..466e1170181 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -41,15 +41,7 @@ module ApplicationHelper
       end
 
     if project.avatar_url
-      #if project.private?
-      #  options[:use_original_source] = true
-      #  image_tag project.avatar_url(use_asset_path: false), options
-      #else
-      #  image_tag project.avatar_url, options
-      #end
-
       image_tag project.avatar_url(use_asset_path: project.public?), options
-
     else # generated icon
       project_identicon(project, options)
     end

From 9f228449a53e1cc5661aba2645a49ecbbf4d5794 Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Fri, 6 Oct 2017 08:14:52 +0200
Subject: [PATCH 09/12] Removed 2 uncommented lines

---
 app/helpers/groups_helper.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 60ac4c63e62..b366e627e01 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -11,8 +11,6 @@ module GroupsHelper
     can?(current_user, :change_share_with_group_lock, group)
   end
 
-  # = project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile')
-  # = image_tag group_icon(@group), alt: '', class: 'avatar group-avatar s160'
   def group_icon(group, options = {})
     img_path = group_icon_url(group, options)
     image_tag img_path, options

From 5f41cddf80b5bcf7b409e200b21cd26761ad9afd Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Mon, 9 Oct 2017 10:45:23 +0200
Subject: [PATCH 10/12] Based on MR simplified the logic

---
 app/helpers/application_helper.rb |  2 +-
 app/helpers/groups_helper.rb      | 16 ++--------------
 app/models/concerns/avatarable.rb |  4 ++--
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 466e1170181..8d02d5de5c3 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -41,7 +41,7 @@ module ApplicationHelper
       end
 
     if project.avatar_url
-      image_tag project.avatar_url(use_asset_path: project.public?), options
+      image_tag project.avatar_url, options
     else # generated icon
       project_identicon(project, options)
     end
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index b366e627e01..655b9d02c1e 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -1,7 +1,4 @@
-require 'uri'
-
 module GroupsHelper
-  include Gitlab::CurrentSettings
 
   def can_change_group_visibility_level?(group)
     can?(current_user, :change_visibility_level, group)
@@ -22,12 +19,7 @@ module GroupsHelper
     end
 
     if group.avatar_url
-      if group.private?
-        options[:use_original_source] = true
-        group.avatar_url(use_asset_path: false)
-      else
-        group.avatar_url
-      end
+      group.avatar_url
     else # No Avatar Icon
       ActionController::Base.helpers.image_path('no_group_avatar.png')
     end
@@ -107,11 +99,7 @@ module GroupsHelper
     link_to(group_path(group), class: "group-path #{'breadcrumb-item-text' unless for_dropdown} js-breadcrumb-item-text #{'hidable' if hidable}") do
       output =
         if (group.try(:avatar_url) || show_avatar) && !Rails.env.test?
-          if group.private?
-            group_icon(group, class: "avatar-tile", width: 15, height: 15, use_original_source: true)
-          else
-            group_icon(group, class: "avatar-tile", width: 15, height: 15)
-          end
+          group_icon(group, class: "avatar-tile", width: 15, height: 15)
         else
           ""
         end
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index dee7e47e2aa..2ec70203710 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -1,7 +1,7 @@
 module Avatarable
   extend ActiveSupport::Concern
 
-  def avatar_path(only_path: true, use_asset_path: true)
+  def avatar_path(only_path: true)
     return unless self[:avatar].present?
 
     # If only_path is true then use the relative path of avatar.
@@ -11,7 +11,7 @@ module Avatarable
 
     # If asset_host is set then it is expected that assets are handled by a standalone host.
     # That means we do not want to get GitLab's relative_url_root option anymore.
-    host = (asset_host.present? && use_asset_path) ? asset_host : gitlab_host
+    host = (asset_host.present? && (!respond_to?(:public?) || public?)) ? asset_host : gitlab_host
 
     [host, avatar.url].join
   end

From 59c6ddcdefb813487d552499ebcb5504b5fde548 Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Mon, 9 Oct 2017 17:05:51 +0200
Subject: [PATCH 11/12] Fix for Lint Error + Upload Test

---
 app/helpers/groups_helper.rb | 1 -
 spec/models/project_spec.rb  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 655b9d02c1e..f5ef9c53a7a 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -1,5 +1,4 @@
 module GroupsHelper
-
   def can_change_group_visibility_level?(group)
     can?(current_user, :change_visibility_level, group)
   end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 176bb568cbe..fef89999b85 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -844,7 +844,7 @@ describe Project do
     let(:project) { create(:project) }
 
     context 'when avatar file is uploaded' do
-      let(:project) { create(:project, :with_avatar) }
+      let(:project) { create(:project, :public, :with_avatar) }
       let(:avatar_path) { "/uploads/-/system/project/avatar/#{project.id}/dk.png" }
       let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
 

From 75c6953fc76422b138335680b227e7303d20c136 Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Tue, 10 Oct 2017 10:59:07 +0200
Subject: [PATCH 12/12] Changed Group Icon URL Back to try option

---
 app/helpers/groups_helper.rb | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index f5ef9c53a7a..676c1d1988b 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -17,11 +17,7 @@ module GroupsHelper
       group = Group.find_by_full_path(group)
     end
 
-    if group.avatar_url
-      group.avatar_url
-    else # No Avatar Icon
-      ActionController::Base.helpers.image_path('no_group_avatar.png')
-    end
+    group.try(:avatar_url) || ActionController::Base.helpers.image_path('no_group_avatar.png')
   end
 
   def group_title(group, name = nil, url = nil)