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.

https://gitlab.com/gitlab-org/gitlab-ce/issues/29240 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.

Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/9357

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

ee/spec/helpers/ee/appearances_helper_spec.rb

@@ -60,6 +60,28 @@ describe AppearancesHelper do
     end
   end
 
+  describe '#brand_image' do
+    let!(:appearance) { create(:appearance, :with_logo) }
+
+    context 'when there is a logo' do
+      it 'returns a path' do
+        expect(helper.brand_image).to match(%r(img data-src="/uploads/-/system/appearance/.*png))
+      end
+    end
+
+    context 'when there is a logo but no associated upload' do
+      before do
+        # Legacy attachments were not tracked in the uploads table
+        appearance.logo.upload.destroy
+        appearance.reload
+      end
+
+      it 'falls back to using the original path' do
+        expect(helper.brand_image).to match(%r(img data-src="/uploads/-/system/appearance/.*png))
+      end
+    end
+  end
+
   describe '#brand_title' do
     it 'returns the default EE title when no appearance is present' do
       allow(helper)

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