From 19f9d998700592be0d40d6727d6e990ef39ada68 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 23 Jan 2019 10:28:26 -0800 Subject: 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. This is a CE backport of https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9277. Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/9357 --- app/models/appearance.rb | 6 +++++- changelogs/unreleased/sh-fix-issue-9357.yml | 5 +++++ spec/models/appearance_spec.rb | 7 +++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-fix-issue-9357.yml diff --git a/app/models/appearance.rb b/app/models/appearance.rb index e114c435b67..ff1ecfda684 100644 --- a/app/models/appearance.rb +++ b/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 diff --git a/changelogs/unreleased/sh-fix-issue-9357.yml b/changelogs/unreleased/sh-fix-issue-9357.yml new file mode 100644 index 00000000000..756cd6047b8 --- /dev/null +++ b/changelogs/unreleased/sh-fix-issue-9357.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 errors with legacy appearance logos +merge_request: 24615 +author: +type: fixed diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index ec2e7d672f0..cc76a2019ec 100644 --- a/spec/models/appearance_spec.rb +++ b/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 -- cgit v1.2.1