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

4 files changed, 51 insertions, 26 deletions

diff --git a/spec/models/concerns/avatarable_spec.rb b/spec/models/concerns/avatarable_spec.rb
new file mode 100644
index 00000000000..cbdc438be0b
--- /dev/null
+++ b/spec/models/concerns/avatarable_spec.rb
@@ -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
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index d4052a64570..5e82a2988ce 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -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
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index e684389af5d..4db417c8793 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -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
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index a3ebf649aa8..65dccf9638d 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -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