Commit 19f9d998 authored by Stan Hu's avatar Stan Hu

Fix 500 errors with legacy appearance logos

Prior to GitLab 9.0, attachments were not tracked the `uploads` table,
so it was possible that the appearance logos were just stored in the
database as a string and mounted via CarrierWave. implemented in
GitLab 10.3 was supposed to cover populating the `uploads` table for all
attachments, including all the logos from appearances. However, it's
possible that didn't work for logos or the `uploads` entry was orphaned.

GitLab instances that had a customized logo with no associated `uploads`
entry would see Error 500s. The only way to fix this is to delete the
`logo` column from the `appearances` table and re-upload the attachment.

This change makes things more robust by falling back to the original
behavior if the upload is not available.

This is a CE backport of

parent 768475bd
......@@ -44,7 +44,11 @@ def favicon_path
def logo_system_path(logo, mount_type)
return unless logo&.upload
# Legacy attachments may not have have an associated Upload record,
# so fallback to the AttachmentUploader#url if this is the
# case. AttachmentUploader#path doesn't work because for a local
# file, this is an absolute path to the file.
return logo&.url unless logo&.upload
# If we're using a CDN, we need to use the full URL
asset_host = ActionController::Base.asset_host
title: Fix 500 errors with legacy appearance logos
merge_request: 24615
type: fixed
......@@ -36,6 +36,13 @@
expect(subject.send("#{logo_type}_path")).to be_nil
it 'returns the path when the upload has been orphaned' do
expect(appearance.send("#{logo_type}_path")).to eq(expected_path)
it 'returns a local path using the system route' do
expect(appearance.send("#{logo_type}_path")).to eq(expected_path)
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