Merge branch 'sh-fix-issue-9357' into 'master'

Fix 500 errors with legacy appearance logos

Closes gitlab-ee#9357

See merge request gitlab-org/gitlab-ce!24615

app/models/appearance.rb

@@ -44,7 +44,11 @@ class Appearance < ActiveRecord::Base
   private
 
   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

changelogs/unreleased/sh-fix-issue-9357.yml

+---
+title: Fix 500 errors with legacy appearance logos
+merge_request: 24615
+author:
+type: fixed

spec/models/appearance_spec.rb

@@ -36,6 +36,13 @@ describe Appearance do
       expect(subject.send("#{logo_type}_path")).to be_nil
     end
 
+    it 'returns the path when the upload has been orphaned' do
+      appearance.send(logo_type).upload.destroy
+      appearance.reload
+
+      expect(appearance.send("#{logo_type}_path")).to eq(expected_path)
+    end
+
     it 'returns a local path using the system route' do
       expect(appearance.send("#{logo_type}_path")).to eq(expected_path)
     end