Merge branch 'use-favicon-full-path-for-pipeline-status-favicon' into 'master'

Use favicon full path

See merge request !10676

18 files changed, 21 insertions, 18 deletions

diff --git a/app/assets/images/ci_favicons/icon_status_canceled.ico b/app/assets/images/ci_favicons/favicon_status_canceled.ico
index 5a19458f2a2..5a19458f2a2 100755
--- a/app/assets/images/ci_favicons/icon_status_canceled.ico
+++ b/app/assets/images/ci_favicons/favicon_status_canceled.ico
diff --git a/app/assets/images/ci_favicons/icon_status_created.ico b/app/assets/images/ci_favicons/favicon_status_created.ico
index 4dca9640cb3..4dca9640cb3 100755
--- a/app/assets/images/ci_favicons/icon_status_created.ico
+++ b/app/assets/images/ci_favicons/favicon_status_created.ico
diff --git a/app/assets/images/ci_favicons/icon_status_failed.ico b/app/assets/images/ci_favicons/favicon_status_failed.ico
index c961ff9a69b..c961ff9a69b 100755
--- a/app/assets/images/ci_favicons/icon_status_failed.ico
+++ b/app/assets/images/ci_favicons/favicon_status_failed.ico
diff --git a/app/assets/images/ci_favicons/icon_status_manual.ico b/app/assets/images/ci_favicons/favicon_status_manual.ico
index 5fbbc99ea7c..5fbbc99ea7c 100755
--- a/app/assets/images/ci_favicons/icon_status_manual.ico
+++ b/app/assets/images/ci_favicons/favicon_status_manual.ico
diff --git a/app/assets/images/ci_favicons/icon_status_not_found.ico b/app/assets/images/ci_favicons/favicon_status_not_found.ico
index 21afa9c72e6..21afa9c72e6 100755
--- a/app/assets/images/ci_favicons/icon_status_not_found.ico
+++ b/app/assets/images/ci_favicons/favicon_status_not_found.ico
diff --git a/app/assets/images/ci_favicons/icon_status_pending.ico b/app/assets/images/ci_favicons/favicon_status_pending.ico
index 8be32dab85a..8be32dab85a 100755
--- a/app/assets/images/ci_favicons/icon_status_pending.ico
+++ b/app/assets/images/ci_favicons/favicon_status_pending.ico
diff --git a/app/assets/images/ci_favicons/icon_status_running.ico b/app/assets/images/ci_favicons/favicon_status_running.ico
index f328ff1a5ed..f328ff1a5ed 100755
--- a/app/assets/images/ci_favicons/icon_status_running.ico
+++ b/app/assets/images/ci_favicons/favicon_status_running.ico
diff --git a/app/assets/images/ci_favicons/icon_status_skipped.ico b/app/assets/images/ci_favicons/favicon_status_skipped.ico
index b4394e1b4af..b4394e1b4af 100755
--- a/app/assets/images/ci_favicons/icon_status_skipped.ico
+++ b/app/assets/images/ci_favicons/favicon_status_skipped.ico
diff --git a/app/assets/images/ci_favicons/icon_status_success.ico b/app/assets/images/ci_favicons/favicon_status_success.ico
index 4f436c95242..4f436c95242 100755
--- a/app/assets/images/ci_favicons/icon_status_success.ico
+++ b/app/assets/images/ci_favicons/favicon_status_success.ico
diff --git a/app/assets/images/ci_favicons/icon_status_warning.ico b/app/assets/images/ci_favicons/favicon_status_warning.ico
index 805cc20cdec..805cc20cdec 100755
--- a/app/assets/images/ci_favicons/icon_status_warning.ico
+++ b/app/assets/images/ci_favicons/favicon_status_warning.ico
diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js
index 33c900e264b..01c4b9821d3 100644
--- a/app/assets/javascripts/lib/utils/common_utils.js
+++ b/app/assets/javascripts/lib/utils/common_utils.js
@@ -368,9 +368,9 @@
       });
     };
 
-    w.gl.utils.setFavicon = (iconName) => {
-      if (faviconEl && iconName) {
-        faviconEl.setAttribute('href', `/assets/${iconName}.ico`);
+    w.gl.utils.setFavicon = (faviconPath) => {
+      if (faviconEl && faviconPath) {
+        faviconEl.setAttribute('href', faviconPath);
       }
     };
 
@@ -385,8 +385,8 @@
         url: pageUrl,
         dataType: 'json',
         success: function(data) {
-          if (data && data.icon) {
-            gl.utils.setFavicon(`ci_favicons/${data.icon}`);
+          if (data && data.favicon) {
+            gl.utils.setFavicon(data.favicon);
           } else {
             gl.utils.resetFavicon();
           }
diff --git a/app/serializers/status_entity.rb b/app/serializers/status_entity.rb
index dfd9d1584a1..944472f3e51 100644
--- a/app/serializers/status_entity.rb
+++ b/app/serializers/status_entity.rb
@@ -1,8 +1,12 @@
 class StatusEntity < Grape::Entity
   include RequestAwareEntity
 
-  expose :icon, :favicon, :text, :label, :group
+  expose :icon, :text, :label, :group
 
   expose :has_details?, as: :has_details
   expose :details_path
+
+  expose :favicon do |status|
+    ActionController::Base.helpers.image_path(File.join('ci_favicons', "#{status.favicon}.ico"))
+  end
 end
diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb
index 13208d21918..faf3770f5e9 100644
--- a/spec/controllers/projects/builds_controller_spec.rb
+++ b/spec/controllers/projects/builds_controller_spec.rb
@@ -60,7 +60,7 @@ describe Projects::BuildsController do
       expect(json_response['text']).to eq status.text
       expect(json_response['label']).to eq status.label
       expect(json_response['icon']).to eq status.icon
-      expect(json_response['favicon']).to eq status.favicon
+      expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico"
     end
   end
 end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 1739d40ab88..cc393bd24f2 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -1208,7 +1208,7 @@ describe Projects::MergeRequestsController do
         expect(json_response['text']).to eq status.text
         expect(json_response['label']).to eq status.label
         expect(json_response['icon']).to eq status.icon
-        expect(json_response['favicon']).to eq status.favicon
+        expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico"
       end
     end
 
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index d8f9bfd0d37..d9192177a06 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -86,7 +86,7 @@ describe Projects::PipelinesController do
       expect(json_response['text']).to eq status.text
       expect(json_response['label']).to eq status.label
       expect(json_response['icon']).to eq status.icon
-      expect(json_response['favicon']).to eq status.favicon
+      expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico"
     end
   end
 end
diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js
index 03f3c206f44..56aabc16382 100644
--- a/spec/javascripts/lib/utils/common_utils_spec.js
+++ b/spec/javascripts/lib/utils/common_utils_spec.js
@@ -313,7 +313,7 @@ require('~/lib/utils/common_utils');
 
     describe('gl.utils.setFavicon', () => {
       it('should set page favicon to provided favicon', () => {
-        const faviconName = 'custom_favicon';
+        const faviconPath = '//custom_favicon';
         const fakeLink = {
           setAttribute() {},
         };
@@ -321,9 +321,9 @@ require('~/lib/utils/common_utils');
         spyOn(window.document, 'getElementById').and.callFake(() => fakeLink);
         spyOn(fakeLink, 'setAttribute').and.callFake((attr, val) => {
           expect(attr).toEqual('href');
-          expect(val.indexOf('/assets/custom_favicon.ico') > -1).toBe(true);
+          expect(val.indexOf(faviconPath) > -1).toBe(true);
         });
-        gl.utils.setFavicon(faviconName);
+        gl.utils.setFavicon(faviconPath);
       });
     });
 
@@ -345,13 +345,12 @@ require('~/lib/utils/common_utils');
     describe('gl.utils.setCiStatusFavicon', () => {
       it('should set page favicon to CI status favicon based on provided status', () => {
         const BUILD_URL = `${gl.TEST_HOST}/frontend-fixtures/builds-project/builds/1/status.json`;
-        const FAVICON_PATH = 'ci_favicons/';
-        const FAVICON = 'icon_status_success';
+        const FAVICON_PATH = '//icon_status_success';
         const spySetFavicon = spyOn(gl.utils, 'setFavicon').and.stub();
         const spyResetFavicon = spyOn(gl.utils, 'resetFavicon').and.stub();
         spyOn($, 'ajax').and.callFake(function (options) {
-          options.success({ icon: FAVICON });
-          expect(spySetFavicon).toHaveBeenCalledWith(FAVICON_PATH + FAVICON);
+          options.success({ favicon: FAVICON_PATH });
+          expect(spySetFavicon).toHaveBeenCalledWith(FAVICON_PATH);
           options.success();
           expect(spyResetFavicon).toHaveBeenCalled();
           options.error();
diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb
index 3cc791bca50..7f1abecfafe 100644
--- a/spec/serializers/build_serializer_spec.rb
+++ b/spec/serializers/build_serializer_spec.rb
@@ -38,7 +38,7 @@ describe BuildSerializer do
         expect(subject[:text]).to eq(status.text)
         expect(subject[:label]).to eq(status.label)
         expect(subject[:icon]).to eq(status.icon)
-        expect(subject[:favicon]).to eq(status.favicon)
+        expect(subject[:favicon]).to eq("/assets/ci_favicons/#{status.favicon}.ico")
       end
     end
   end
diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb
index f6249ab4664..ecde45a6d44 100644
--- a/spec/serializers/pipeline_serializer_spec.rb
+++ b/spec/serializers/pipeline_serializer_spec.rb
@@ -144,7 +144,7 @@ describe PipelineSerializer do
         expect(subject[:text]).to eq(status.text)
         expect(subject[:label]).to eq(status.label)
         expect(subject[:icon]).to eq(status.icon)
-        expect(subject[:favicon]).to eq(status.favicon)
+        expect(subject[:favicon]).to eq("/assets/ci_favicons/#{status.favicon}.ico")
       end
     end
   end