call Gitlab::Favicon.status in serializer

this ways we can keep the `lib/gitlab/ci/status/*` classes to return the
bare favicon name as it was before.

also the favicon uploader versions are now have the same names as the
stock favicons (+ `favicon_` prefix), which makes working with the
status names easier.

15 files changed, 45 insertions, 42 deletions

diff --git a/app/helpers/page_layout_helper.rb b/app/helpers/page_layout_helper.rb
index c3dd204181d..68d892393ef 100644
--- a/app/helpers/page_layout_helper.rb
+++ b/app/helpers/page_layout_helper.rb
@@ -39,7 +39,7 @@ module PageLayoutHelper
   end
 
   def favicon
-    Gitlab::Favicon.default
+    Gitlab::Favicon.main
   end
 
   def page_image
diff --git a/app/serializers/status_entity.rb b/app/serializers/status_entity.rb
index f4d0a33d466..2f3d4b80565 100644
--- a/app/serializers/status_entity.rb
+++ b/app/serializers/status_entity.rb
@@ -7,7 +7,7 @@ class StatusEntity < Grape::Entity
   expose :details_path
 
   expose :favicon do |status|
-    ActionController::Base.helpers.image_path(status.favicon)
+    Gitlab::Favicon.status(status.favicon)
   end
 
   expose :action, if: -> (status, _) { status.has_action? } do
diff --git a/app/uploaders/favicon_uploader.rb b/app/uploaders/favicon_uploader.rb
index d3debc16fa9..7697f5fe885 100644
--- a/app/uploaders/favicon_uploader.rb
+++ b/app/uploaders/favicon_uploader.rb
@@ -2,19 +2,19 @@ class FaviconUploader < AttachmentUploader
   include CarrierWave::MiniMagick
 
   STATUS_ICON_NAMES = [
-    :status_canceled,
-    :status_created,
-    :status_failed,
-    :status_manual,
-    :status_not_found,
-    :status_pending,
-    :status_running,
-    :status_skipped,
-    :status_success,
-    :status_warning
+    :favicon_status_canceled,
+    :favicon_status_created,
+    :favicon_status_failed,
+    :favicon_status_manual,
+    :favicon_status_not_found,
+    :favicon_status_pending,
+    :favicon_status_running,
+    :favicon_status_skipped,
+    :favicon_status_success,
+    :favicon_status_warning
   ].freeze
 
-  version :default do
+  version :favicon_main do
     process resize_to_fill: [32, 32]
     process convert: 'ico'
 
@@ -24,7 +24,7 @@ class FaviconUploader < AttachmentUploader
   end
 
   STATUS_ICON_NAMES.each do |status_name|
-    version status_name, from_version: :default do
+    version status_name, from_version: :favicon_main do
       process status_favicon: status_name
 
       def full_filename(filename)
@@ -41,7 +41,7 @@ class FaviconUploader < AttachmentUploader
 
   def status_favicon(status_name)
     manipulate! do |img|
-      overlay_path = Rails.root.join("app/assets/images/ci_favicons/overlays/favicon_#{status_name}.png")
+      overlay_path = Rails.root.join("app/assets/images/ci_favicons/overlays/#{status_name}.png")
       overlay = MiniMagick::Image.open(overlay_path)
       img.composite(overlay) do |c|
         c.compose 'over'
diff --git a/app/views/admin/appearances/_form.html.haml b/app/views/admin/appearances/_form.html.haml
index 8d73bd8fb55..1119c75637c 100644
--- a/app/views/admin/appearances/_form.html.haml
+++ b/app/views/admin/appearances/_form.html.haml
@@ -62,7 +62,7 @@
     = f.label :favicon, 'Favicon', class: 'control-label'
     .col-sm-10
       - if @appearance.favicon?
-        = image_tag @appearance.favicon.default.url, class: 'appearance-light-logo-preview'
+        = image_tag @appearance.favicon.favicon_main.url, class: 'appearance-light-logo-preview'
     - if @appearance.favicon?
       = f.label :favicon, 'Generated status icons', class: 'control-label'
     .col-sm-10
diff --git a/lib/gitlab/ci/status/canceled.rb b/lib/gitlab/ci/status/canceled.rb
index 0d71ff03163..e6195a60d4f 100644
--- a/lib/gitlab/ci/status/canceled.rb
+++ b/lib/gitlab/ci/status/canceled.rb
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def favicon
-          Gitlab::Favicon.status('canceled')
+          'favicon_status_canceled'
         end
       end
     end
diff --git a/lib/gitlab/ci/status/created.rb b/lib/gitlab/ci/status/created.rb
index de86191dfeb..846f00b83dd 100644
--- a/lib/gitlab/ci/status/created.rb
+++ b/lib/gitlab/ci/status/created.rb
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def favicon
-          Gitlab::Favicon.status('created')
+          'favicon_status_created'
         end
       end
     end
diff --git a/lib/gitlab/ci/status/failed.rb b/lib/gitlab/ci/status/failed.rb
index 20e2050108c..27ce85bd3ed 100644
--- a/lib/gitlab/ci/status/failed.rb
+++ b/lib/gitlab/ci/status/failed.rb
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def favicon
-          Gitlab::Favicon.status('failed')
+          'favicon_status_failed'
         end
       end
     end
diff --git a/lib/gitlab/ci/status/manual.rb b/lib/gitlab/ci/status/manual.rb
index 2c02ce6e870..fc387e2fd25 100644
--- a/lib/gitlab/ci/status/manual.rb
+++ b/lib/gitlab/ci/status/manual.rb
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def favicon
-          Gitlab::Favicon.status('manual')
+          'favicon_status_manual'
         end
       end
     end
diff --git a/lib/gitlab/ci/status/pending.rb b/lib/gitlab/ci/status/pending.rb
index 9122d11cfed..6780780db32 100644
--- a/lib/gitlab/ci/status/pending.rb
+++ b/lib/gitlab/ci/status/pending.rb
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def favicon
-          Gitlab::Favicon.status('pending')
+          'favicon_status_pending'
         end
       end
     end
diff --git a/lib/gitlab/ci/status/running.rb b/lib/gitlab/ci/status/running.rb
index 9bc48ec2c29..ee13905e46d 100644
--- a/lib/gitlab/ci/status/running.rb
+++ b/lib/gitlab/ci/status/running.rb
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def favicon
-          Gitlab::Favicon.status('running')
+          'favicon_status_running'
         end
       end
     end
diff --git a/lib/gitlab/ci/status/skipped.rb b/lib/gitlab/ci/status/skipped.rb
index b404118cd3b..0dbdc4de426 100644
--- a/lib/gitlab/ci/status/skipped.rb
+++ b/lib/gitlab/ci/status/skipped.rb
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def favicon
-          Gitlab::Favicon.status('skipped')
+          'favicon_status_skipped'
         end
       end
     end
diff --git a/lib/gitlab/ci/status/success.rb b/lib/gitlab/ci/status/success.rb
index be7e5d60b26..731013ec017 100644
--- a/lib/gitlab/ci/status/success.rb
+++ b/lib/gitlab/ci/status/success.rb
@@ -15,7 +15,7 @@ module Gitlab
         end
 
         def favicon
-          Gitlab::Favicon.status('success')
+          'favicon_status_success'
         end
       end
     end
diff --git a/lib/gitlab/favicon.rb b/lib/gitlab/favicon.rb
index 8802f58e31c..51a25b408ee 100644
--- a/lib/gitlab/favicon.rb
+++ b/lib/gitlab/favicon.rb
@@ -1,8 +1,8 @@
 module Gitlab
   class Favicon
     class << self
-      def default
-        return custom_favicon_url(appearance_favicon.default.url) if appearance_favicon.exists?
+      def main
+        return custom_favicon_url(appearance_favicon.favicon_main.url) if appearance_favicon.exists?
         return 'favicon-yellow.ico' if Gitlab::Utils.to_boolean(ENV['CANARY'])
         return 'favicon-blue.ico' if Rails.env.development?
 
@@ -11,13 +11,16 @@ module Gitlab
 
       def status(status_name)
         if appearance_favicon.exists?
-          custom_favicon_url(appearance_favicon.public_send("status_#{status_name}").url) # rubocop:disable GitlabSecurity/PublicSend
+          custom_favicon_url(appearance_favicon.public_send("#{status_name}").url) # rubocop:disable GitlabSecurity/PublicSend
         else
-          dir = 'ci_favicons'
-          dir = File.join(dir, 'dev') if Rails.env.development?
-          dir = File.join(dir, 'canary') if Gitlab::Utils.to_boolean(ENV['CANARY'])
+          path = File.join(
+            'ci_favicons',
+            Rails.env.development? ? 'dev' : '',
+            Gitlab::Utils.to_boolean(ENV['CANARY']) ? 'canary' : '',
+            "#{status_name}.ico"
+          )
 
-          File.join(dir, "favicon_status_#{status_name}.ico")
+          ActionController::Base.helpers.image_path(path)
         end
       end
 
diff --git a/spec/lib/gitlab/favicon_spec.rb b/spec/lib/gitlab/favicon_spec.rb
index ddfa81c0b5d..51b8fda81d1 100644
--- a/spec/lib/gitlab/favicon_spec.rb
+++ b/spec/lib/gitlab/favicon_spec.rb
@@ -1,38 +1,38 @@
 require 'rails_helper'
 
 RSpec.describe Gitlab::Favicon, :request_store do
-  describe '.default' do
+  describe '.main' do
     it 'defaults to favicon.ico' do
       allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
-      expect(described_class.default).to eq 'favicon.ico'
+      expect(described_class.main).to eq 'favicon.ico'
     end
 
     it 'has blue favicon for development' do
       allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('development'))
-      expect(described_class.default).to eq 'favicon-blue.ico'
+      expect(described_class.main).to eq 'favicon-blue.ico'
     end
 
     it 'has yellow favicon for canary' do
       stub_env('CANARY', 'true')
-      expect(described_class.favicon).to eq 'favicon-yellow.ico'
+      expect(described_class.main).to eq 'favicon-yellow.ico'
     end
 
     it 'uses the custom favicon if a favicon appearance is present' do
       create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'))
-      expect(described_class.default).to match %r{/uploads/-/system/appearance/favicon/\d+/default_dk.ico\?}
+      expect(described_class.main).to match %r{/uploads/-/system/appearance/favicon/\d+/favicon_main_dk.ico}
     end
   end
 
   describe '.status' do
-    subject { described_class.status('created') }
+    subject { described_class.status('favicon_status_created') }
 
     it 'defaults to the stock icon' do
-      expect(subject).to eq 'ci_favicons/favicon_status_created.ico'
+      expect(subject).to eq '/assets/ci_favicons/favicon_status_created.ico'
     end
 
     it 'uses the custom favicon if a favicon appearance is present' do
       create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'))
-      expect(subject).to match(%r{/uploads/-/system/appearance/favicon/\d+/status_created_dk.ico\?})
+      expect(subject).to match(%r{/uploads/-/system/appearance/favicon/\d+/favicon_status_created_dk.ico})
     end
   end
 end
diff --git a/spec/uploaders/favicon_uploader_spec.rb b/spec/uploaders/favicon_uploader_spec.rb
index 5989d294112..b521670addb 100644
--- a/spec/uploaders/favicon_uploader_spec.rb
+++ b/spec/uploaders/favicon_uploader_spec.rb
@@ -19,11 +19,11 @@ RSpec.describe FaviconUploader do
     end
 
     it 'has the correct format' do
-      expect(uploader.default).to be_format('ico')
+      expect(uploader.favicon_main).to be_format('ico')
     end
 
     it 'has the correct dimensions' do
-      expect(uploader.default).to have_dimensions(32, 32)
+      expect(uploader.favicon_main).to have_dimensions(32, 32)
     end
 
     it 'generates all the status icons' do
@@ -31,7 +31,7 @@ RSpec.describe FaviconUploader do
       expect(FaviconUploader::STATUS_ICON_NAMES.count).to eq 10
 
       FaviconUploader::STATUS_ICON_NAMES.each do |status_name|
-        expect(File.exist?(uploader.status_not_found.file.file)).to be true
+        expect(File.exist?(uploader.favicon_status_not_found.file.file)).to be true
       end
     end
   end