Merge branch 'dm-remove-favicon-main-version' into 'master'

Remove FaviconUploader favicon_main version

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

6 files changed, 10 insertions, 33 deletions

diff --git a/app/uploaders/favicon_uploader.rb b/app/uploaders/favicon_uploader.rb
index 09afc63a5aa..3639375d474 100644
--- a/app/uploaders/favicon_uploader.rb
+++ b/app/uploaders/favicon_uploader.rb
@@ -1,17 +1,6 @@
 class FaviconUploader < AttachmentUploader
   EXTENSION_WHITELIST = %w[png ico].freeze
 
-  include CarrierWave::MiniMagick
-
-  version :favicon_main do
-    process resize_to_fill: [32, 32]
-    process convert: 'png'
-
-    def full_filename(filename)
-      filename_for_different_format(super(filename), 'png')
-    end
-  end
-
   def extension_whitelist
     EXTENSION_WHITELIST
   end
diff --git a/app/views/admin/appearances/_form.html.haml b/app/views/admin/appearances/_form.html.haml
index 94db374040c..a0861870ba4 100644
--- a/app/views/admin/appearances/_form.html.haml
+++ b/app/views/admin/appearances/_form.html.haml
@@ -25,7 +25,7 @@
     = f.label :favicon, 'Favicon', class: 'col-sm-2 col-form-label'
     .col-sm-10
       - if @appearance.favicon?
-        = image_tag @appearance.favicon.favicon_main.url, class: 'appearance-light-logo-preview'
+        = image_tag @appearance.favicon_url, 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"
@@ -33,9 +33,9 @@
       = f.hidden_field :favicon_cache
       = f.file_field :favicon, class: ''
       .hint
-        Maximum file size is 1MB. Allowed image formats are #{favicon_extension_whitelist}.
+        Maximum file size is 1MB. Image size must be 32x32px. Allowed image formats are #{favicon_extension_whitelist}.
         %br
-        The resulting favicons will be cropped to be square and scaled down to a size of 32x32 px.
+        Images with incorrect dimensions are not resized automatically, and may result in unexpected behavior.
 
   %fieldset.sign-in
     %legend
diff --git a/lib/gitlab/favicon.rb b/lib/gitlab/favicon.rb
index 451c9daf761..faf7016d73a 100644
--- a/lib/gitlab/favicon.rb
+++ b/lib/gitlab/favicon.rb
@@ -2,7 +2,7 @@ module Gitlab
   class Favicon
     class << self
       def main
-        return appearance_favicon.favicon_main.url if appearance_favicon.exists?
+        return appearance_favicon.url if appearance_favicon.exists?
 
         image_name =
           if Gitlab::Utils.to_boolean(ENV['CANARY'])
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 912aa82526a..444e6759694 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -580,23 +580,6 @@ describe UploadsController do
           expect(response).to have_gitlab_http_status(404)
         end
       end
-
-      context 'has a valid filename on the version file' do
-        it 'successfully returns the file' do
-          get :show, model: 'appearance', mounted_as: 'favicon', id: appearance.id, filename: 'favicon_main_dk.png'
-
-          expect(response).to have_gitlab_http_status(200)
-          expect(response.header['Content-Disposition']).to end_with 'filename="favicon_main_dk.png"'
-        end
-      end
-
-      context 'has an invalid filename on the version file' do
-        it 'returns a 404' do
-          get :show, model: 'appearance', mounted_as: 'favicon', id: appearance.id, filename: 'favicon_bogusversion_dk.png'
-
-          expect(response).to have_gitlab_http_status(404)
-        end
-      end
     end
   end
 end
diff --git a/spec/lib/gitlab/favicon_spec.rb b/spec/lib/gitlab/favicon_spec.rb
index fdc5c0180e4..ad3e7c6b5f7 100644
--- a/spec/lib/gitlab/favicon_spec.rb
+++ b/spec/lib/gitlab/favicon_spec.rb
@@ -18,8 +18,13 @@ RSpec.describe Gitlab::Favicon, :request_store do
     end
 
     it 'uses the custom favicon if a favicon appearance is present' do
+<<<<<<< HEAD
       create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'))
       expect(described_class.main).to match %r{/uploads/-/system/appearance/favicon/\d+/favicon_main_dk.png}
+=======
+      create :appearance, favicon: fixture_file_upload('spec/fixtures/dk.png')
+      expect(described_class.main).to match %r{/uploads/-/system/appearance/favicon/\d+/dk.png}
+>>>>>>> 26810dd771... Merge branch 'dm-remove-favicon-main-version' into 'master'
     end
   end
 
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index c3b4eb17a5c..7e4fbc3ed40 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -478,7 +478,7 @@ describe JiraService do
       create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'))
 
       props = described_class.new.send(:build_remote_link_props, url: 'http://example.com', title: 'title')
-      expect(props[:object][:icon][:url16x16]).to match %r{^http://localhost/uploads/-/system/appearance/favicon/\d+/favicon_main_dk.png$}
+      expect(props[:object][:icon][:url16x16]).to match %r{^http://localhost/uploads/-/system/appearance/favicon/\d+/dk.png$}
     end
   end
 end