kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Always return full avatar URL for private/internal groups/projects when asset host is set

Browse source
This commit is contained in:
Douwe Maan 2017-11-09 15:40:41 +00:00 committed by Rémy Coutable
parent 9a56496d44
commit 2fbbba9a29
8 changed files with 82 additions and 131 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
app/models/concerns/avatarable.rb
View file

@ -4,15 +4,26 @@ module Avatarable
def avatar_path(only_path: true)
return unless self[:avatar].present?
# If only_path is true then use the relative path of avatar.
# Otherwise use full path (including host).
asset_host = ActionController::Base.asset_host
gitlab_host = only_path ? gitlab_config.relative_url_root : gitlab_config.url
use_asset_host = asset_host.present?
# 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? && (!respond_to?(:public?) || public?)) ? asset_host : gitlab_host
# Avatars for private and internal groups and projects require authentication to be viewed,
# which means they can only be served by Rails, on the regular GitLab host.
# If an asset host is configured, we need to return the fully qualified URL
# instead of only the avatar path, so that Rails doesn't prefix it with the asset host.
if use_asset_host && respond_to?(:public?) && !public?
use_asset_host = false
only_path = false
end
[host, avatar.url].join
url_base = ""
if use_asset_host
url_base << asset_host unless only_path
else
url_base << gitlab_config.base_url unless only_path
url_base << gitlab_config.relative_url_root
end
url_base + avatar.url
end
end

6
changelogs/unreleased/dm-avatarable-with-asset-host.yml Normal file
View file

@ -0,0 +1,6 @@
---
title: Always return full avatar URL for private/internal groups/projects when asset
host is set
merge_request:
author:
type: fixed

73
spec/helpers/application_helper_spec.rb
View file

@ -4,8 +4,6 @@ require 'spec_helper'
describe ApplicationHelper do
include UploadHelpers
let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
describe 'current_controller?' do
it 'returns true when controller matches argument' do
stub_controller_name('foo')
@ -57,30 +55,11 @@ describe ApplicationHelper do
end
describe 'project_icon' do
let(:asset_host) { 'http://assets' }
it 'returns an url for the avatar' do
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)
.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/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}\" />"
end
it 'gives uploaded icon when present' do
project = create(:project)
allow_any_instance_of(Project).to receive(:avatar_in_git).and_return(true)
avatar_url = "#{gitlab_host}#{project_avatar_path(project)}"
expect(helper.project_icon(project.full_path).to_s)
.to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
.to eq "<img data-src=\"#{project.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
end
end
@ -91,40 +70,7 @@ describe ApplicationHelper 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("/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif")
end
context 'when an asset_host is set in the config' do
let(:asset_host) { 'http://assets' }
before do
allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
end
it 'returns an absolute URL on that asset host' do
expect(helper.avatar_icon(user.email, only_path: false).to_s)
.to eq("#{asset_host}/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
context 'when only_path is set to false' do
it 'returns an absolute URL for the avatar' do
expect(helper.avatar_icon(user.email, only_path: false).to_s)
.to eq("#{gitlab_host}/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
context 'when the GitLab instance is at a relative URL' do
before do
stub_config_setting(relative_url_root: '/gitlab')
# Must be stubbed after the stub above, and separately
stub_config_setting(url: Settings.send(:build_gitlab_url))
end
it 'returns a relative URL with the correct prefix' do
expect(helper.avatar_icon(user.email).to_s)
.to eq("/gitlab/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif")
end
.to eq(user.avatar.url)
end
end
@ -138,18 +84,9 @@ describe ApplicationHelper do
end
describe 'using a user' do
context 'when only_path is true' do
it 'returns a relative URL for the avatar' do
expect(helper.avatar_icon(user, only_path: true).to_s)
.to eq("/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
context 'when only_path is false' do
it 'returns an absolute URL for the avatar' do
expect(helper.avatar_icon(user, only_path: false).to_s)
.to eq("#{gitlab_host}/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif")
end
it 'returns a relative URL for the avatar' do
expect(helper.avatar_icon(user).to_s)
.to eq(user.avatar.url)
end
end
end

32
spec/helpers/groups_helper_spec.rb
View file

@ -3,8 +3,6 @@ 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')
@ -13,16 +11,8 @@ describe GroupsHelper do
group.avatar = fixture_file_upload(avatar_file_path)
group.save!
avatar_url = "/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif"
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(helper.group_icon(group).to_s)
.to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
.to eq "<img data-src=\"#{group.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
end
end
@ -34,25 +24,7 @@ describe GroupsHelper do
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(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("#{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(asset_host)
group = create(:group, :private)
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")
.to match(group.avatar.url)
end
it 'gives default avatar_icon when no avatar is present' do

44
spec/models/concerns/avatarable_spec.rb Normal file
View file

@ -0,0 +1,44 @@
require 'spec_helper'
describe Avatarable do
subject { create(:project, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) }
let(:gitlab_host) { "https://gitlab.example.com" }
let(:relative_url_root) { "/gitlab" }
let(:asset_host) { "https://gitlab-assets.example.com" }
before do
stub_config_setting(base_url: gitlab_host)
stub_config_setting(relative_url_root: relative_url_root)
end
describe '#avatar_path' do
using RSpec::Parameterized::TableSyntax
where(:has_asset_host, :visibility_level, :only_path, :avatar_path) do
true | Project::PRIVATE | true | [gitlab_host, relative_url_root, subject.avatar.url]
true | Project::PRIVATE | false | [gitlab_host, relative_url_root, subject.avatar.url]
true | Project::INTERNAL | true | [gitlab_host, relative_url_root, subject.avatar.url]
true | Project::INTERNAL | false | [gitlab_host, relative_url_root, subject.avatar.url]
true | Project::PUBLIC | true | [subject.avatar.url]
true | Project::PUBLIC | false | [asset_host, subject.avatar.url]
false | Project::PRIVATE | true | [relative_url_root, subject.avatar.url]
false | Project::PRIVATE | false | [gitlab_host, relative_url_root, subject.avatar.url]
false | Project::INTERNAL | true | [relative_url_root, subject.avatar.url]
false | Project::INTERNAL | false | [gitlab_host, relative_url_root, subject.avatar.url]
false | Project::PUBLIC | true | [relative_url_root, subject.avatar.url]
false | Project::PUBLIC | false | [gitlab_host, relative_url_root, subject.avatar.url]
end
with_them do
before do
allow(ActionController::Base).to receive(:asset_host).and_return(has_asset_host ? asset_host : nil)
subject.visibility_level = visibility_level
end
it 'returns the expected avatar path' do
expect(subject.avatar_path(only_path: only_path)).to eq(avatar_path.join)
end
end
end
end

10
spec/models/group_spec.rb
View file

@ -247,8 +247,6 @@ describe Group do
describe '#avatar_url' do
let!(:group) { create(:group, :access_requestable, :with_avatar) }
let(:user) { create(:user) }
let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
let(:avatar_path) { "/uploads/-/system/group/avatar/#{group.id}/dk.png" }
context 'when avatar file is uploaded' do
before do
@ -256,12 +254,8 @@ describe Group do
end
it 'shows correct avatar url' do
expect(group.avatar_url).to eq(avatar_path)
expect(group.avatar_url(only_path: false)).to eq([gitlab_host, avatar_path].join)
allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
expect(group.avatar_url).to eq([gitlab_host, avatar_path].join)
expect(group.avatar_url).to eq(group.avatar.url)
expect(group.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, group.avatar.url].join)
end
end
end

12
spec/models/project_spec.rb
View file

@ -883,20 +883,14 @@ describe Project do
context 'when avatar file is uploaded' do
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}" }
it 'shows correct url' do
expect(project.avatar_url).to eq(avatar_path)
expect(project.avatar_url(only_path: false)).to eq([gitlab_host, avatar_path].join)
allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
expect(project.avatar_url).to eq([gitlab_host, avatar_path].join)
expect(project.avatar_url).to eq(project.avatar.url)
expect(project.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, project.avatar.url].join)
end
end
context 'When avatar file in git' do
context 'when avatar file in git' do
before do
allow(project).to receive(:avatar_in_git) { true }
end

11
spec/models/user_spec.rb
View file

@ -1149,16 +1149,9 @@ describe User do
let(:user) { create(:user, :with_avatar) }
context 'when avatar file is uploaded' do
let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
let(:avatar_path) { "/uploads/-/system/user/avatar/#{user.id}/dk.png" }
it 'shows correct avatar url' do
expect(user.avatar_url).to eq(avatar_path)
expect(user.avatar_url(only_path: false)).to eq([gitlab_host, avatar_path].join)
allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
expect(user.avatar_url).to eq([gitlab_host, avatar_path].join)
expect(user.avatar_url).to eq(user.avatar.url)
expect(user.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, user.avatar.url].join)
end
end
end