Commit d0848c08 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'sh-use-system-path-for-appearance-logos' into 'master'

Use system paths for appearance logos

Closes gitlab-ee#6778

See merge request gitlab-org/gitlab-ce!24024
parents 5fabc1fd 1a248006
Pipeline #41433119 passed with stages
in 51 minutes and 19 seconds
......@@ -11,7 +11,7 @@ module AppearancesHelper
end
def brand_image
image_tag(current_appearance.logo) if current_appearance&.logo?
image_tag(current_appearance.logo_path) if current_appearance&.logo?
end
def brand_text
......@@ -28,7 +28,7 @@ module AppearancesHelper
def brand_header_logo
if current_appearance&.header_logo?
image_tag current_appearance.header_logo
image_tag current_appearance.header_logo_path
else
render 'shared/logo.svg'
end
......
......@@ -58,7 +58,7 @@ module EmailsHelper
def header_logo
if current_appearance&.header_logo?
image_tag(
current_appearance.header_logo,
current_appearance.header_logo_path,
style: 'height: 50px'
)
else
......
......@@ -28,4 +28,32 @@ class Appearance < ActiveRecord::Base
errors.add(:single_appearance_row, 'Only 1 appearances row can exist')
end
end
def logo_path
logo_system_path(logo, 'logo')
end
def header_logo_path
logo_system_path(header_logo, 'header_logo')
end
def favicon_path
logo_system_path(favicon, 'favicon')
end
private
def logo_system_path(logo, mount_type)
return unless logo&.upload
# If we're using a CDN, we need to use the full URL
asset_host = ActionController::Base.asset_host
local_path = Gitlab::Routing.url_helpers.appearance_upload_path(
filename: logo.filename,
id: logo.upload.model_id,
model: 'appearance',
mounted_as: mount_type)
Gitlab::Utils.append_path(asset_host, local_path)
end
end
......@@ -8,7 +8,7 @@
= f.label :header_logo, 'Header logo', class: 'col-sm-2 col-form-label pt-0'
.col-sm-10
- if @appearance.header_logo?
= image_tag @appearance.header_logo_url, class: 'appearance-light-logo-preview'
= image_tag @appearance.header_logo_path, class: 'appearance-light-logo-preview'
- if @appearance.persisted?
%br
= link_to 'Remove header logo', header_logos_admin_appearances_path, data: { confirm: "Header logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo"
......@@ -25,7 +25,7 @@
= f.label :favicon, 'Favicon', class: 'col-sm-2 col-form-label pt-0'
.col-sm-10
- if @appearance.favicon?
= image_tag @appearance.favicon_url, class: 'appearance-light-logo-preview'
= image_tag @appearance.favicon_path, class: 'appearance-light-logo-preview'
- if @appearance.persisted?
%br
= link_to 'Remove favicon', favicon_admin_appearances_path, data: { confirm: "Favicon will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo"
......@@ -54,7 +54,7 @@
= f.label :logo, class: 'col-sm-2 col-form-label pt-0'
.col-sm-10
- if @appearance.logo?
= image_tag @appearance.logo_url, class: 'appearance-logo-preview'
= image_tag @appearance.logo_path, class: 'appearance-logo-preview'
- if @appearance.persisted?
%br
= link_to 'Remove logo', logo_admin_appearances_path, data: { confirm: "Logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo"
......
......@@ -17,7 +17,8 @@ scope path: :uploads do
# Appearance
get "-/system/:model/:mounted_as/:id/:filename",
to: "uploads#show",
constraints: { model: /appearance/, mounted_as: /logo|header_logo|favicon/, filename: /.+/ }
constraints: { model: /appearance/, mounted_as: /logo|header_logo|favicon/, filename: /.+/ },
as: 'appearance_upload'
# Project markdown uploads
get ":namespace_id/:project_id/:secret/:filename",
......
......@@ -15,6 +15,10 @@ FactoryBot.define do
header_logo { fixture_file_upload('spec/fixtures/dk.png') }
end
trait :with_favicon do
favicon { fixture_file_upload('spec/fixtures/dk.png') }
end
trait :with_logos do
with_logo
with_header_logo
......
......@@ -26,4 +26,34 @@ describe Appearance do
let(:uploader_class) { AttachmentUploader }
end
end
shared_examples 'logo paths' do |logo_type|
let(:appearance) { create(:appearance, "with_#{logo_type}".to_sym) }
let(:filename) { 'dk.png' }
let(:expected_path) { "/uploads/-/system/appearance/#{logo_type}/#{appearance.id}/#{filename}" }
it 'returns nil when there is no upload' do
expect(subject.send("#{logo_type}_path")).to be_nil
end
it 'returns a local path using the system route' do
expect(appearance.send("#{logo_type}_path")).to eq(expected_path)
end
describe 'with asset host configured' do
let(:asset_host) { 'https://gitlab-assets.example.com' }
before do
allow(ActionController::Base).to receive(:asset_host) { asset_host }
end
it 'returns a full URL with the system path' do
expect(appearance.send("#{logo_type}_path")).to eq("#{asset_host}#{expected_path}")
end
end
end
%i(logo header_logo favicon).each do |logo_type|
it_behaves_like 'logo paths', logo_type
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment