From 0c49c0f76dce95274e409b43b43b5c7d45001e7d Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Mon, 12 Mar 2018 14:28:09 -0600
Subject: [PATCH 01/21] Include failure_reason on BuildSerializer

- Add a method on Ci::Build presenter to map the failure to an
  appropriate description

Closes #44269
---
 app/presenters/ci/build_presenter.rb      | 17 +++++++++++++++++
 app/serializers/job_entity.rb             |  9 ++++++++-
 spec/serializers/build_serializer_spec.rb |  1 +
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 255475e1fe6..655cb817a80 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -28,5 +28,22 @@ def trigger_variables
           trigger_request.user_variables
         end
     end
+
+    def failure_reason_description
+      failure_descriptions[failure_reason]
+    end
+
+    private
+
+    def failure_descriptions
+      {
+        "unknown_failure" => "Unknown failure",
+        "script_failure" => "Script failure",
+        "api_failure" => "API failure",
+        "stuck_or_timeout_failure" => "Stuck or timeout failure",
+        "runner_system_failure" => "Runner system failure",
+        "missing_dependency_failure" => "Missing dependency failure"
+      }
+    end
   end
 end
diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb
index 523b522d449..fea65b5d58f 100644
--- a/app/serializers/job_entity.rb
+++ b/app/serializers/job_entity.rb
@@ -25,7 +25,10 @@ class JobEntity < Grape::Entity
   expose :playable?, as: :playable
   expose :created_at
   expose :updated_at
-  expose :detailed_status, as: :status, with: StatusEntity
+  expose :status do
+    expose :failure_reason
+    expose :detailed_status, merge: true, with: StatusEntity
+  end
 
   private
 
@@ -50,4 +53,8 @@ def detailed_status
   def path_to(route, build)
     send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend
   end
+
+  def failure_reason
+    Ci::BuildPresenter.new(build).failure_reason_description
+  end
 end
diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb
index 9673b11c2a2..147306dc790 100644
--- a/spec/serializers/build_serializer_spec.rb
+++ b/spec/serializers/build_serializer_spec.rb
@@ -39,6 +39,7 @@
         expect(subject[:label]).to eq(status.label)
         expect(subject[:icon]).to eq(status.icon)
         expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
+        expect(subject[:failure_reason]).to eq('Unknown failure')
       end
     end
   end
-- 
GitLab


From 2cf6e48159ea785034a3b4c74edeaaa36148c870 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Thu, 15 Mar 2018 10:35:29 -0600
Subject: [PATCH 02/21] Displays descriptive tooltip

This is implemented on:
- job lists on Pipeline detail view
- global job list view.
- Merge request widget
- Commit widget
- Jobs sidebar
- Pipeline list

Frontend is needed to make the br tag actually work
---
 .../components/graph/job_component.vue        |  7 ++--
 app/helpers/ci_status_helper.rb               |  8 +++++
 app/presenters/ci/build_presenter.rb          | 32 +++++++++++++----
 app/serializers/job_group_entity.rb           | 13 ++++++-
 .../ci/status/_dropdown_graph_badge.html.haml |  2 +-
 app/views/projects/jobs/_sidebar.html.haml    |  2 +-
 ...lure-reason-on-upgrade-tooltip-of-jobs.yml |  5 +++
 spec/factories/ci/builds.rb                   |  4 +++
 spec/presenters/ci/build_presenter_spec.rb    | 34 ++++++++++++++++---
 9 files changed, 90 insertions(+), 17 deletions(-)
 create mode 100644 changelogs/unreleased/44269-show-failure-reason-on-upgrade-tooltip-of-jobs.yml

diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue
index 9b136573135..1a42d86d71a 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -72,9 +72,12 @@
         if (this.job.name && this.status.label) {
           textBuilder.push('-');
         }
-
         if (this.status.label) {
-          textBuilder.push(`${this.job.status.label}`);
+          if (this.status.label === 'failed' && this.status.failure_reason) {
+            textBuilder.push(`${this.job.status.failure_reason}`);
+          } else {
+            textBuilder.push(`${this.job.status.label}`);
+          }
         }
 
         return textBuilder.join(' ');
diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb
index 636316da80a..99808c8cbc2 100644
--- a/app/helpers/ci_status_helper.rb
+++ b/app/helpers/ci_status_helper.rb
@@ -144,4 +144,12 @@ def detailed_status?(status)
       status.respond_to?(:label) &&
       status.respond_to?(:icon)
   end
+
+  def tooltip_for_badge(build, status_label)
+    if build.failed?
+      build.present(current_user: current_user).graph_tooltip_description
+    else
+      "#{build.name} - #{status_label}"
+    end
+  end
 end
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 655cb817a80..39fb3131437 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -15,6 +15,8 @@ def erased_by_name
     def status_title
       if auto_canceled?
         "Job is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}"
+      else
+        tooltip_description
       end
     end
 
@@ -29,20 +31,36 @@ def trigger_variables
         end
     end
 
+    def tooltip_description
+      if failed_or_allowed_to_failed? && failure_reason
+        failure_reason_description
+      else
+        status.titleize
+      end
+    end
+
+    def graph_tooltip_description
+      "#{name} - #{failure_reason_description}"
+    end
+
     def failure_reason_description
-      failure_descriptions[failure_reason]
+      "Failed <br> #{failure_descriptions[failure_reason]}"
     end
 
     private
 
+    def failed_or_allowed_to_failed?
+      failed? || allow_failure?
+    end
+
     def failure_descriptions
       {
-        "unknown_failure" => "Unknown failure",
-        "script_failure" => "Script failure",
-        "api_failure" => "API failure",
-        "stuck_or_timeout_failure" => "Stuck or timeout failure",
-        "runner_system_failure" => "Runner system failure",
-        "missing_dependency_failure" => "Missing dependency failure"
+        'unknown_failure' => 'Unknown failure',
+        'script_failure' => 'Script failure',
+        'api_failure' => 'API failure',
+        'stuck_or_timeout_failure' => 'Stuck or timeout failure',
+        'runner_system_failure' => 'Runner system failure',
+        'missing_dependency_failure' => 'Missing dependency failure'
       }
     end
   end
diff --git a/app/serializers/job_group_entity.rb b/app/serializers/job_group_entity.rb
index 8554de55517..157a59edc6c 100644
--- a/app/serializers/job_group_entity.rb
+++ b/app/serializers/job_group_entity.rb
@@ -3,7 +3,10 @@ class JobGroupEntity < Grape::Entity
 
   expose :name
   expose :size
-  expose :detailed_status, as: :status, with: StatusEntity
+  expose :status do
+    expose :failure_reason
+    expose :detailed_status, merge: true, with: StatusEntity
+  end
   expose :jobs, with: JobEntity
 
   private
@@ -13,4 +16,12 @@ class JobGroupEntity < Grape::Entity
   def detailed_status
     group.detailed_status(request.current_user)
   end
+
+  def failure_reason
+    failure_reason_description if group.jobs.one?
+  end
+
+  def failure_reason_description
+    Ci::BuildPresenter.new(group.jobs.first).failure_reason_description
+  end
 end
diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml
index c5b4439e273..2c3a21ffe75 100644
--- a/app/views/ci/status/_dropdown_graph_badge.html.haml
+++ b/app/views/ci/status/_dropdown_graph_badge.html.haml
@@ -3,7 +3,7 @@
 - subject = local_assigns.fetch(:subject)
 - status = subject.detailed_status(current_user)
 - klass = "ci-status-icon ci-status-icon-#{status.group}"
-- tooltip = "#{subject.name} - #{status.label}"
+- tooltip = tooltip_for_badge(subject, status.label)
 
 - if status.has_details?
   = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, container: 'body' }  do
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index ecf186e3dc8..3032a4df101 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -93,7 +93,7 @@
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
             = link_to project_job_path(@project, build) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
-              %span{ class: "ci-status-icon-#{build.status}" }
+              %span{ class: "ci-status-icon-#{build.status}", data: { toggle: 'tooltip', title: tooltip_for_badge(build, build.detailed_status(current_user)), container: 'body' } }
                 = ci_icon_for_status(build.status)
               %span
                 - if build.name
diff --git a/changelogs/unreleased/44269-show-failure-reason-on-upgrade-tooltip-of-jobs.yml b/changelogs/unreleased/44269-show-failure-reason-on-upgrade-tooltip-of-jobs.yml
new file mode 100644
index 00000000000..b3ae8ca7340
--- /dev/null
+++ b/changelogs/unreleased/44269-show-failure-reason-on-upgrade-tooltip-of-jobs.yml
@@ -0,0 +1,5 @@
+---
+title: Display error message on job's tooltip if this one fails
+merge_request: 17782
+author:
+type: added
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index f6ba3a581ca..8e552aad25b 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -237,5 +237,9 @@
     trait :protected do
       protected true
     end
+
+    trait :script_failure do
+      failure_reason 1
+    end
   end
 end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index 1a8001be6ab..faeae3581ba 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -72,13 +72,25 @@
       end
     end
 
-    context 'when build is not auto-canceled' do
-      before do
-        expect(build).to receive(:auto_canceled?).and_return(false)
+    context 'when build has a failure' do
+      let(:build) { create(:ci_build, :failed, pipeline: pipeline) }
+
+      it 'returns the reason of failure' do
+        status_title = presenter.status_title
+
+        expect(status_title).to include('Failed')
+        expect(status_title).to include('Unknown failure')
       end
+    end
+
+    context 'when build is allowed to failed' do
+      let(:build) { create(:ci_build, :allowed_to_fail, :script_failure, pipeline: pipeline) }
 
-      it 'does not have a status title' do
-        expect(presenter.status_title).to be_nil
+      it 'returns the reason of failure' do
+        status_title = presenter.status_title
+
+        expect(status_title).to include('Failed')
+        expect(status_title).to include('Script failure')
       end
     end
   end
@@ -134,4 +146,16 @@
       end
     end
   end
+
+  describe '#graph_tooltip_descriptoin' do
+    let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
+
+    it 'returns the reason of failure' do
+      graph_tooltip_description = subject.graph_tooltip_description
+
+      expect(graph_tooltip_description).to include(build.name)
+      expect(graph_tooltip_description).to include('Failed')
+      expect(graph_tooltip_description).to include('Script failure')
+    end
+  end
 end
-- 
GitLab


From 42c0ee7b270919a86454808e4a4419cec7e7a68d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Fri, 16 Mar 2018 12:24:52 +0000
Subject: [PATCH 03/21] Allow html within Tooltip for Job status badge

---
 app/views/ci/status/_dropdown_graph_badge.html.haml | 3 ++-
 app/views/projects/jobs/_sidebar.html.haml          | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml
index 2c3a21ffe75..5d4a9383641 100644
--- a/app/views/ci/status/_dropdown_graph_badge.html.haml
+++ b/app/views/ci/status/_dropdown_graph_badge.html.haml
@@ -6,9 +6,10 @@
 - tooltip = tooltip_for_badge(subject, status.label)
 
 - if status.has_details?
-  = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, container: 'body' }  do
+  = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, html: true, container: 'body' }  do
     %span{ class: klass }= sprite_icon(status.icon)
     %span.ci-build-text= subject.name
+
 - else
   .menu-item.mini-pipeline-graph-dropdown-item{ data: { toggle: 'tooltip', title: tooltip, container: 'body' } }
     %span{ class: klass }= sprite_icon(status.icon)
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index 3032a4df101..a534cff17f0 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -93,7 +93,7 @@
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
             = link_to project_job_path(@project, build) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
-              %span{ class: "ci-status-icon-#{build.status}", data: { toggle: 'tooltip', title: tooltip_for_badge(build, build.detailed_status(current_user)), container: 'body' } }
+              %span{ class: "ci-status-icon-#{build.status}", data: { toggle: 'tooltip', title: tooltip_for_badge(build, build.detailed_status(current_user)), html: true, container: 'body' } }
                 = ci_icon_for_status(build.status)
               %span
                 - if build.name
-- 
GitLab


From d014866185e1717e1fa2aff4ff9bfcd7e73853d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Fri, 16 Mar 2018 12:44:28 +0000
Subject: [PATCH 04/21] Make whole row of Job status activate tooltip and
 darker bg

---
 app/assets/stylesheets/pages/builds.scss   | 2 +-
 app/views/projects/jobs/_sidebar.html.haml | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss
index 98d460339cd..7a6352e45f1 100644
--- a/app/assets/stylesheets/pages/builds.scss
+++ b/app/assets/stylesheets/pages/builds.scss
@@ -391,7 +391,7 @@
       }
 
       &:hover {
-        background-color: $row-hover;
+        background-color: $dropdown-item-hover-bg;
       }
 
       .icon-retry {
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index a534cff17f0..1d59bf9908e 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -91,9 +91,9 @@
       - HasStatus::ORDERED_STATUSES.each do |build_status|
         - builds.select{|build| build.status == build_status}.each do |build|
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
-            = link_to project_job_path(@project, build) do
+            = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: true, title: tooltip_for_badge(build, build.detailed_status(current_user)), container: 'body' }) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
-              %span{ class: "ci-status-icon-#{build.status}", data: { toggle: 'tooltip', title: tooltip_for_badge(build, build.detailed_status(current_user)), html: true, container: 'body' } }
+              %span{ class: "ci-status-icon-#{build.status}" }
                 = ci_icon_for_status(build.status)
               %span
                 - if build.name
-- 
GitLab


From 64f38609438e77a2be7355e50c1dd2425d05dbba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Fri, 16 Mar 2018 17:49:51 +0000
Subject: [PATCH 05/21] Allow HTML in tooltip of job vue component

---
 .../javascripts/pipelines/components/graph/job_component.vue    | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue
index 1a42d86d71a..2fdace6e01a 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -103,6 +103,7 @@
       :title="tooltipText"
       :class="cssClassJobName"
       data-container="body"
+      data-html
       class="js-pipeline-graph-job-link"
     >
 
@@ -118,6 +119,7 @@
       class="js-job-component-tooltip"
       :title="tooltipText"
       :class="cssClassJobName"
+      data-html
       data-container="body"
     >
 
-- 
GitLab


From c90707b4c8f05991b73a1e9ad788bf9d8617f47e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Fri, 16 Mar 2018 18:01:32 +0000
Subject: [PATCH 06/21] Allow html in tooltip of ci status badge

---
 app/views/ci/status/_badge.html.haml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/views/ci/status/_badge.html.haml b/app/views/ci/status/_badge.html.haml
index 35a3563dff1..5114387984b 100644
--- a/app/views/ci/status/_badge.html.haml
+++ b/app/views/ci/status/_badge.html.haml
@@ -4,10 +4,10 @@
 - css_classes = "ci-status ci-#{status.group} #{'has-tooltip' if title.present?}"
 
 - if link && status.has_details?
-  = link_to status.details_path, class: css_classes, title: title do
+  = link_to status.details_path, class: css_classes, title: title, data: { html: title.present? } do
     = sprite_icon(status.icon)
     = status.text
 - else
-  %span{ class: css_classes, title: title }
+  %span{ class: css_classes, title: title, data: { html: title.present? } }
     = sprite_icon(status.icon)
     = status.text
-- 
GitLab


From f21c1bb97f2e0641840e499192a6c3c41cbcb5f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Sat, 17 Mar 2018 19:28:03 +0000
Subject: [PATCH 07/21] Unify retry information in tooltip of job status

---
 app/views/projects/jobs/_sidebar.html.haml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index 1d59bf9908e..98ad8bfadd2 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -91,7 +91,9 @@
       - HasStatus::ORDERED_STATUSES.each do |build_status|
         - builds.select{|build| build.status == build_status}.each do |build|
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
-            = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: true, title: tooltip_for_badge(build, build.detailed_status(current_user)), container: 'body' }) do
+            - tooltip = tooltip_for_badge(build, build.detailed_status(current_user))
+            - tooltip += " (retried)" if build.retried?
+            = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: true, title: tooltip, container: 'body' }) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
               %span{ class: "ci-status-icon-#{build.status}" }
                 = ci_icon_for_status(build.status)
@@ -101,5 +103,4 @@
                 - else
                   = build.id
               - if build.retried?
-                %span.has-tooltip{ data: { container: 'body', placement: 'bottom' }, title: 'Job was retried' }
-                  = sprite_icon('retry', size:16, css_class: 'icon-retry')
+                = sprite_icon('retry', size:16, css_class: 'icon-retry')
-- 
GitLab


From 07d8a284bfc90eac60eec0d5b110ee1b1fff7055 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Sat, 17 Mar 2018 19:31:36 +0000
Subject: [PATCH 08/21] Allow html in the tooltip of CI job status in one more
 place

---
 app/views/ci/status/_dropdown_graph_badge.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml
index 5d4a9383641..c51fcc285f5 100644
--- a/app/views/ci/status/_dropdown_graph_badge.html.haml
+++ b/app/views/ci/status/_dropdown_graph_badge.html.haml
@@ -11,7 +11,7 @@
     %span.ci-build-text= subject.name
 
 - else
-  .menu-item.mini-pipeline-graph-dropdown-item{ data: { toggle: 'tooltip', title: tooltip, container: 'body' } }
+  .menu-item.mini-pipeline-graph-dropdown-item{ data: { toggle: 'tooltip', html: true, title: tooltip, container: 'body' } }
     %span{ class: klass }= sprite_icon(status.icon)
     %span.ci-build-text= subject.name
 
-- 
GitLab


From 2ca7592fb1936b7c84b5a94f8ba060d641f86a14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Tue, 20 Mar 2018 11:51:10 +0000
Subject: [PATCH 09/21] Add support for html option on vue triggered tooltips

---
 app/assets/javascripts/vue_shared/directives/tooltip.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/vue_shared/directives/tooltip.js b/app/assets/javascripts/vue_shared/directives/tooltip.js
index b7f7e9fec15..02ac0aedf4f 100644
--- a/app/assets/javascripts/vue_shared/directives/tooltip.js
+++ b/app/assets/javascripts/vue_shared/directives/tooltip.js
@@ -2,7 +2,13 @@ import $ from 'jquery';
 
 export default {
   bind(el) {
-    $(el).tooltip();
+    const tooltipOptions = {};
+
+    if (typeof el.dataset.html !== 'undefined') {
+      tooltipOptions.html = true;
+    }
+
+    $(el).tooltip(tooltipOptions);
   },
 
   componentUpdated(el) {
-- 
GitLab


From c16c7cfbb3419ef9f1cfbf76f690799ecf1dc808 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Tue, 20 Mar 2018 08:44:38 -0600
Subject: [PATCH 10/21] Fix typo on build presenter spec

---
 spec/presenters/ci/build_presenter_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index faeae3581ba..72f8b6d7f3b 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -147,7 +147,7 @@
     end
   end
 
-  describe '#graph_tooltip_descriptoin' do
+  describe '#graph_tooltip_description' do
     let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
 
     it 'returns the reason of failure' do
-- 
GitLab


From 27a556fb7cf5af25cf853f645878cdeb46d9d6e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Wed, 21 Mar 2018 13:53:56 +0000
Subject: [PATCH 11/21] Revert Vue tooltip adaptation and use it correctly

---
 .../pipelines/components/graph/job_component.vue          | 4 ++--
 app/assets/javascripts/vue_shared/directives/tooltip.js   | 8 +-------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue
index 2fdace6e01a..1b6e02929a5 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -103,7 +103,7 @@
       :title="tooltipText"
       :class="cssClassJobName"
       data-container="body"
-      data-html
+      data-html="true"
       class="js-pipeline-graph-job-link"
     >
 
@@ -119,7 +119,7 @@
       class="js-job-component-tooltip"
       :title="tooltipText"
       :class="cssClassJobName"
-      data-html
+      data-html="true"
       data-container="body"
     >
 
diff --git a/app/assets/javascripts/vue_shared/directives/tooltip.js b/app/assets/javascripts/vue_shared/directives/tooltip.js
index 02ac0aedf4f..b7f7e9fec15 100644
--- a/app/assets/javascripts/vue_shared/directives/tooltip.js
+++ b/app/assets/javascripts/vue_shared/directives/tooltip.js
@@ -2,13 +2,7 @@ import $ from 'jquery';
 
 export default {
   bind(el) {
-    const tooltipOptions = {};
-
-    if (typeof el.dataset.html !== 'undefined') {
-      tooltipOptions.html = true;
-    }
-
-    $(el).tooltip(tooltipOptions);
+    $(el).tooltip();
   },
 
   componentUpdated(el) {
-- 
GitLab


From 3467f5ba5ef97c694088c94170f2a5afa5fb934d Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Wed, 21 Mar 2018 10:55:39 -0600
Subject: [PATCH 12/21] Add Gitlab::Ci::Status::.Build::Failed class

This class has two methods:
- label to include the failure_reason
- matches? Returns true if it's a failed job that is not allowed to
  fail
---
 .../components/graph/job_component.vue        |  6 +-
 app/presenters/ci/build_presenter.rb          |  6 +-
 app/serializers/job_entity.rb                 |  9 +--
 app/serializers/job_group_entity.rb           | 13 +---
 lib/gitlab/ci/status/build/factory.rb         |  3 +-
 lib/gitlab/ci/status/build/failed.rb          | 17 +++++
 .../gitlab/ci/status/build/factory_spec.rb    |  8 +--
 .../lib/gitlab/ci/status/build/failed_spec.rb | 66 +++++++++++++++++++
 spec/serializers/build_serializer_spec.rb     | 21 ++++--
 spec/serializers/job_entity_spec.rb           | 21 ++++++
 10 files changed, 135 insertions(+), 35 deletions(-)
 create mode 100644 lib/gitlab/ci/status/build/failed.rb
 create mode 100644 spec/lib/gitlab/ci/status/build/failed_spec.rb

diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue
index 1b6e02929a5..ad21760a3bd 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -73,11 +73,7 @@
           textBuilder.push('-');
         }
         if (this.status.label) {
-          if (this.status.label === 'failed' && this.status.failure_reason) {
-            textBuilder.push(`${this.job.status.failure_reason}`);
-          } else {
-            textBuilder.push(`${this.job.status.label}`);
-          }
+          textBuilder.push(`${this.job.status.label}`);
         }
 
         return textBuilder.join(' ');
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 39fb3131437..0c66e77b1a3 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -44,7 +44,7 @@ def graph_tooltip_description
     end
 
     def failure_reason_description
-      "Failed <br> #{failure_descriptions[failure_reason]}"
+      "#{failed_title} <br> #{failure_descriptions[failure_reason]}"
     end
 
     private
@@ -63,5 +63,9 @@ def failure_descriptions
         'missing_dependency_failure' => 'Missing dependency failure'
       }
     end
+
+    def failed_title
+      s_('CiStatusLabel|failed').titleize
+    end
   end
 end
diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb
index fea65b5d58f..523b522d449 100644
--- a/app/serializers/job_entity.rb
+++ b/app/serializers/job_entity.rb
@@ -25,10 +25,7 @@ class JobEntity < Grape::Entity
   expose :playable?, as: :playable
   expose :created_at
   expose :updated_at
-  expose :status do
-    expose :failure_reason
-    expose :detailed_status, merge: true, with: StatusEntity
-  end
+  expose :detailed_status, as: :status, with: StatusEntity
 
   private
 
@@ -53,8 +50,4 @@ def detailed_status
   def path_to(route, build)
     send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend
   end
-
-  def failure_reason
-    Ci::BuildPresenter.new(build).failure_reason_description
-  end
 end
diff --git a/app/serializers/job_group_entity.rb b/app/serializers/job_group_entity.rb
index 157a59edc6c..8554de55517 100644
--- a/app/serializers/job_group_entity.rb
+++ b/app/serializers/job_group_entity.rb
@@ -3,10 +3,7 @@ class JobGroupEntity < Grape::Entity
 
   expose :name
   expose :size
-  expose :status do
-    expose :failure_reason
-    expose :detailed_status, merge: true, with: StatusEntity
-  end
+  expose :detailed_status, as: :status, with: StatusEntity
   expose :jobs, with: JobEntity
 
   private
@@ -16,12 +13,4 @@ class JobGroupEntity < Grape::Entity
   def detailed_status
     group.detailed_status(request.current_user)
   end
-
-  def failure_reason
-    failure_reason_description if group.jobs.one?
-  end
-
-  def failure_reason_description
-    Ci::BuildPresenter.new(group.jobs.first).failure_reason_description
-  end
 end
diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb
index c852d607373..4f1e5decc66 100644
--- a/lib/gitlab/ci/status/build/factory.rb
+++ b/lib/gitlab/ci/status/build/factory.rb
@@ -9,7 +9,8 @@ def self.extended_statuses
              [Status::Build::FailedAllowed,
               Status::Build::Play,
               Status::Build::Stop],
-             [Status::Build::Action]]
+             [Status::Build::Action],
+             [Status::Build::Failed]]
           end
 
           def self.common_helpers
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
new file mode 100644
index 00000000000..242254dd791
--- /dev/null
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -0,0 +1,17 @@
+module Gitlab
+  module Ci
+    module Status
+      module Build
+        class Failed < Status::Extended
+          def label
+            ::Ci::BuildPresenter.new(subject).failure_reason_description
+          end
+
+          def self.matches?(build, user)
+            build.failed? && !build.allow_failure?
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index d196bc6a4c2..e3c6da60d19 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -48,18 +48,18 @@
 
       it 'matches correct extended statuses' do
         expect(factory.extended_statuses)
-          .to eq [Gitlab::Ci::Status::Build::Retryable]
+          .to eq [Gitlab::Ci::Status::Build::Retryable, Gitlab::Ci::Status::Build::Failed]
       end
 
-      it 'fabricates a retryable build status' do
-        expect(status).to be_a Gitlab::Ci::Status::Build::Retryable
+      it 'fabricates a failed build status' do
+        expect(status).to be_a Gitlab::Ci::Status::Build::Failed
       end
 
       it 'fabricates status with correct details' do
         expect(status.text).to eq 'failed'
         expect(status.icon).to eq 'status_failed'
         expect(status.favicon).to eq 'favicon_status_failed'
-        expect(status.label).to eq 'failed'
+        expect(status.label).to eq 'Failed <br> Unknown failure'
         expect(status).to have_details
         expect(status).to have_action
       end
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
new file mode 100644
index 00000000000..334b2ee1185
--- /dev/null
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -0,0 +1,66 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Status::Build::Failed do
+  let(:build) { create(:ci_build, :failed, :script_failure) }
+  let(:status) { double('core status') }
+  let(:user) { double('user') }
+
+  subject { described_class.new(status) }
+
+  describe '#text' do
+    it 'does not override status text' do
+      expect(status).to receive(:text)
+
+      subject.text
+    end
+  end
+
+  describe '#icon' do
+    it 'does not override status icon' do
+      expect(status).to receive(:icon)
+
+      subject.icon
+    end
+  end
+
+  describe '#group' do
+    it 'does not override status group' do
+      expect(status).to receive(:group)
+
+      subject.group
+    end
+  end
+
+  describe '#favicon' do
+    it 'does not override status label' do
+      expect(status).to receive(:favicon)
+
+      subject.favicon
+    end
+  end
+
+  describe '#label' do
+    let(:user) { create(:user) }
+    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+    it 'does override label' do
+      expect(subject.label).to eq 'Failed <br> Script failure'
+    end
+  end
+
+  describe '.matches?' do
+    context 'with a failed build' do
+      it 'returns true' do
+        expect(described_class.matches?(build, user)).to be_truthy
+      end
+    end
+
+    context 'with any other type of build' do
+      let(:build) { create(:ci_build, :success) }
+
+      it 'returns false' do
+        expect(described_class.matches?(build, user)).to be_falsy
+      end
+    end
+  end
+end
diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb
index 147306dc790..70cc38e2f4f 100644
--- a/spec/serializers/build_serializer_spec.rb
+++ b/spec/serializers/build_serializer_spec.rb
@@ -28,18 +28,31 @@
   end
 
   describe '#represent_status' do
-    context 'when represents only status' do
-      let(:resource) { create(:ci_build) }
+    context 'for a failed build' do
+      let(:resource) { create(:ci_build, :failed) }
+      let(:status) { resource.detailed_status(double('user')) }
+
+      subject { serializer.represent_status(resource) }
+
+      it 'serializes only status' do
+        expect(subject[:text]).to eq(status.text)
+        expect(subject[:label]).to eq('Failed <br> Unknown failure')
+        expect(subject[:icon]).to eq(status.icon)
+        expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
+      end
+    end
+
+    context 'for any other type of build' do
+      let(:resource) { create(:ci_build, :success) }
       let(:status) { resource.detailed_status(double('user')) }
 
       subject { serializer.represent_status(resource) }
 
       it 'serializes only status' do
         expect(subject[:text]).to eq(status.text)
-        expect(subject[:label]).to eq(status.label)
+        expect(subject[:label]).to eq('passed')
         expect(subject[:icon]).to eq(status.icon)
         expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
-        expect(subject[:failure_reason]).to eq('Unknown failure')
       end
     end
   end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index 026360e91a3..371ac01502b 100644
--- a/spec/serializers/job_entity_spec.rb
+++ b/spec/serializers/job_entity_spec.rb
@@ -129,4 +129,25 @@
       expect(subject[:status]).to include :icon, :favicon, :text, :label
     end
   end
+
+  context 'when job failed' do
+    let(:job) { create(:ci_build, :failed, :script_failure) }
+
+    describe 'status' do
+      it 'should contain the failure reason inside label' do
+        expect(subject[:status]).to include :icon, :favicon, :text, :label
+        expect(subject[:status][:label]).to eq('Failed <br> Script failure')
+      end
+    end
+  end
+
+  context 'when job passed' do
+    let(:job) { create(:ci_build, :success) }
+
+    describe 'status' do
+      it 'should not contain the failure reason inside label' do
+        expect(subject[:status][:label]).to eq('passed')
+      end
+    end
+  end
 end
-- 
GitLab


From 0e3a84f5cd969ff737b5c77c53c3ebc5e4e0dac2 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Fri, 23 Mar 2018 12:24:04 -0600
Subject: [PATCH 13/21] Change tooltip format

- Display a lowercase tooltip for pipelines (list, minigraphs and job selector)
- Include failure reason inside parenthesis for all of scenarios
---
 app/presenters/ci/build_presenter.rb          | 20 +++++----
 .../gitlab/ci/status/build/factory_spec.rb    |  2 +-
 .../lib/gitlab/ci/status/build/failed_spec.rb |  2 +-
 spec/presenters/ci/build_presenter_spec.rb    | 41 +++++++++++++++----
 spec/serializers/build_serializer_spec.rb     |  2 +-
 spec/serializers/job_entity_spec.rb           |  2 +-
 6 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 0c66e77b1a3..fa808bc338f 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -33,7 +33,7 @@ def trigger_variables
 
     def tooltip_description
       if failed_or_allowed_to_failed? && failure_reason
-        failure_reason_description
+        failure_reason_description(true)
       else
         status.titleize
       end
@@ -43,8 +43,10 @@ def graph_tooltip_description
       "#{name} - #{failure_reason_description}"
     end
 
-    def failure_reason_description
-      "#{failed_title} <br> #{failure_descriptions[failure_reason]}"
+    def failure_reason_description(titleize = false)
+      title = titleize ? failed_title.titleize : failed_title
+
+      "#{title} <br> (#{failure_descriptions[failure_reason]})"
     end
 
     private
@@ -55,17 +57,17 @@ def failed_or_allowed_to_failed?
 
     def failure_descriptions
       {
-        'unknown_failure' => 'Unknown failure',
-        'script_failure' => 'Script failure',
+        'unknown_failure' => 'unknown failure',
+        'script_failure' => 'script failure',
         'api_failure' => 'API failure',
-        'stuck_or_timeout_failure' => 'Stuck or timeout failure',
-        'runner_system_failure' => 'Runner system failure',
-        'missing_dependency_failure' => 'Missing dependency failure'
+        'stuck_or_timeout_failure' => 'stuck or timeout failure',
+        'runner_system_failure' => 'runner system failure',
+        'missing_dependency_failure' => 'missing dependency failure'
       }
     end
 
     def failed_title
-      s_('CiStatusLabel|failed').titleize
+      s_('CiStatusLabel|failed')
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index e3c6da60d19..a3e4e4f21f6 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -59,7 +59,7 @@
         expect(status.text).to eq 'failed'
         expect(status.icon).to eq 'status_failed'
         expect(status.favicon).to eq 'favicon_status_failed'
-        expect(status.label).to eq 'Failed <br> Unknown failure'
+        expect(status.label).to eq 'failed <br> (unknown failure)'
         expect(status).to have_details
         expect(status).to have_action
       end
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
index 334b2ee1185..1e5f23bbac6 100644
--- a/spec/lib/gitlab/ci/status/build/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -44,7 +44,7 @@
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
     it 'does override label' do
-      expect(subject.label).to eq 'Failed <br> Script failure'
+      expect(subject.label).to eq 'failed <br> (script failure)'
     end
   end
 
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index 72f8b6d7f3b..ffb5fb96c7c 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -79,7 +79,7 @@
         status_title = presenter.status_title
 
         expect(status_title).to include('Failed')
-        expect(status_title).to include('Unknown failure')
+        expect(status_title).to include('(unknown failure)')
       end
     end
 
@@ -90,7 +90,7 @@
         status_title = presenter.status_title
 
         expect(status_title).to include('Failed')
-        expect(status_title).to include('Script failure')
+        expect(status_title).to include('(script failure)')
       end
     end
   end
@@ -147,15 +147,40 @@
     end
   end
 
+  describe '#tooltip_description' do
+    context 'For failed builds' do
+      let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
+
+      it 'returns the reason of failure' do
+        tooltip_description = subject.tooltip_description
+
+        expect(tooltip_description).to eq('Failed <br> (script failure)')
+      end
+    end
+
+    context 'For any other build' do
+      let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+
+      it 'returns the status' do
+        tooltip_description = subject.tooltip_description
+
+        expect(tooltip_description).to eq('Success')
+      end
+    end
+  end
+
   describe '#graph_tooltip_description' do
-    let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
+    context 'For failed builds' do
+      let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
 
-    it 'returns the reason of failure' do
-      graph_tooltip_description = subject.graph_tooltip_description
+      it 'returns the reason of failure' do
+        graph_tooltip_description = subject.graph_tooltip_description
 
-      expect(graph_tooltip_description).to include(build.name)
-      expect(graph_tooltip_description).to include('Failed')
-      expect(graph_tooltip_description).to include('Script failure')
+        expect(graph_tooltip_description).to include(build.name)
+        expect(graph_tooltip_description).to include('failed')
+        expect(graph_tooltip_description).to include('<br>')
+        expect(graph_tooltip_description).to include('(script failure)')
+      end
     end
   end
 end
diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb
index 70cc38e2f4f..0ac23e69d33 100644
--- a/spec/serializers/build_serializer_spec.rb
+++ b/spec/serializers/build_serializer_spec.rb
@@ -36,7 +36,7 @@
 
       it 'serializes only status' do
         expect(subject[:text]).to eq(status.text)
-        expect(subject[:label]).to eq('Failed <br> Unknown failure')
+        expect(subject[:label]).to eq('failed <br> (unknown failure)')
         expect(subject[:icon]).to eq(status.icon)
         expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
       end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index 371ac01502b..6b82249aec5 100644
--- a/spec/serializers/job_entity_spec.rb
+++ b/spec/serializers/job_entity_spec.rb
@@ -136,7 +136,7 @@
     describe 'status' do
       it 'should contain the failure reason inside label' do
         expect(subject[:status]).to include :icon, :favicon, :text, :label
-        expect(subject[:status][:label]).to eq('Failed <br> Script failure')
+        expect(subject[:status][:label]).to eq('failed <br> (script failure)')
       end
     end
   end
-- 
GitLab


From 38eec2dbc14a0b8c60f88791bc79883e64d4a6f3 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Mon, 26 Mar 2018 11:54:50 -0600
Subject: [PATCH 14/21] Merge methods between BuildPresenter and Build::Failed
 status

Gitlab::Ci::Status contain most of the logic, meanwhile BuildPresenter just presents it
---
 app/helpers/ci_status_helper.rb               |  8 ---
 app/presenters/ci/build_presenter.rb          | 43 ++++++++--------
 .../ci/status/_dropdown_graph_badge.html.haml |  2 +-
 app/views/projects/jobs/_sidebar.html.haml    |  3 +-
 lib/gitlab/ci/status/build/failed.rb          | 39 ++++++++++++++-
 spec/factories/ci/builds.rb                   |  1 +
 .../lib/gitlab/ci/status/build/failed_spec.rb | 23 ++++++++-
 spec/presenters/ci/build_presenter_spec.rb    | 49 ++++++++++++++-----
 spec/serializers/job_entity_spec.rb           |  2 +-
 9 files changed, 121 insertions(+), 49 deletions(-)

diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb
index 99808c8cbc2..636316da80a 100644
--- a/app/helpers/ci_status_helper.rb
+++ b/app/helpers/ci_status_helper.rb
@@ -144,12 +144,4 @@ def detailed_status?(status)
       status.respond_to?(:label) &&
       status.respond_to?(:icon)
   end
-
-  def tooltip_for_badge(build, status_label)
-    if build.failed?
-      build.present(current_user: current_user).graph_tooltip_description
-    else
-      "#{build.name} - #{status_label}"
-    end
-  end
 end
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index fa808bc338f..9c77617b911 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -16,7 +16,7 @@ def status_title
       if auto_canceled?
         "Job is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}"
       else
-        tooltip_description
+        tooltip_for_status
       end
     end
 
@@ -31,22 +31,22 @@ def trigger_variables
         end
     end
 
-    def tooltip_description
-      if failed_or_allowed_to_failed? && failure_reason
-        failure_reason_description(true)
+    def tooltip_for_status
+      if failed_or_allowed_to_failed?
+        build_failed_status.titleize_label
       else
         status.titleize
       end
     end
 
-    def graph_tooltip_description
-      "#{name} - #{failure_reason_description}"
-    end
-
-    def failure_reason_description(titleize = false)
-      title = titleize ? failed_title.titleize : failed_title
-
-      "#{title} <br> (#{failure_descriptions[failure_reason]})"
+    def tooltip_for_pipeline
+      if build.failed? && build.retried?
+        build_failed_status.retry_label
+      elsif build.retried?
+        tooltip_pipeline_message + " (retried)"
+      else
+        tooltip_pipeline_message
+      end
     end
 
     private
@@ -55,19 +55,16 @@ def failed_or_allowed_to_failed?
       failed? || allow_failure?
     end
 
-    def failure_descriptions
-      {
-        'unknown_failure' => 'unknown failure',
-        'script_failure' => 'script failure',
-        'api_failure' => 'API failure',
-        'stuck_or_timeout_failure' => 'stuck or timeout failure',
-        'runner_system_failure' => 'runner system failure',
-        'missing_dependency_failure' => 'missing dependency failure'
-      }
+    def build_failed_status
+      Gitlab::Ci::Status::Build::Failed.new(failed_status)
+    end
+
+    def failed_status
+      @failed_status ||= Gitlab::Ci::Status::Failed.new(subject, user)
     end
 
-    def failed_title
-      s_('CiStatusLabel|failed')
+    def tooltip_pipeline_message
+      "#{name} - #{subject.detailed_status(user).label}"
     end
   end
 end
diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml
index c51fcc285f5..453c84604ac 100644
--- a/app/views/ci/status/_dropdown_graph_badge.html.haml
+++ b/app/views/ci/status/_dropdown_graph_badge.html.haml
@@ -3,7 +3,7 @@
 - subject = local_assigns.fetch(:subject)
 - status = subject.detailed_status(current_user)
 - klass = "ci-status-icon ci-status-icon-#{status.group}"
-- tooltip = tooltip_for_badge(subject, status.label)
+- tooltip = subject.present.tooltip_for_pipeline
 
 - if status.has_details?
   = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, html: true, container: 'body' }  do
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index 98ad8bfadd2..4f43cd05cf7 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -91,8 +91,7 @@
       - HasStatus::ORDERED_STATUSES.each do |build_status|
         - builds.select{|build| build.status == build_status}.each do |build|
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
-            - tooltip = tooltip_for_badge(build, build.detailed_status(current_user))
-            - tooltip += " (retried)" if build.retried?
+            - tooltip = build.present.tooltip_for_pipeline
             = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: true, title: tooltip, container: 'body' }) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
               %span{ class: "ci-status-icon-#{build.status}" }
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
index 242254dd791..5a5dcca85c2 100644
--- a/lib/gitlab/ci/status/build/failed.rb
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -3,13 +3,50 @@ module Ci
     module Status
       module Build
         class Failed < Status::Extended
+          FAILURE_REASONS = {
+            'unknown_failure' => 'unknown failure',
+            'script_failure' => 'script failure',
+            'api_failure' => 'API failure',
+            'stuck_or_timeout_failure' => 'stuck or timeout failure',
+            'runner_system_failure' => 'runner system failure',
+            'missing_dependency_failure' => 'missing dependency failure'
+          }.freeze
+
+          def titleize_label
+            failure_reason_description(true)
+          end
+
           def label
-            ::Ci::BuildPresenter.new(subject).failure_reason_description
+            failure_reason_description
           end
 
           def self.matches?(build, user)
             build.failed? && !build.allow_failure?
           end
+
+          def retry_label
+            "#{retry_title} #{description}"
+          end
+
+          private
+
+          def failure_reason_description(titleize = false)
+            title = titleize ? failed_title.titleize : failed_title
+
+            "#{title} #{description}"
+          end
+
+          def failed_title
+            s_('CiStatusLabel|failed')
+          end
+
+          def description
+            "<br> (#{FAILURE_REASONS[subject.failure_reason]})"
+          end
+
+          def retry_title
+            subject.name + " - " + failed_title + " (retried)"
+          end
         end
       end
     end
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 8e552aad25b..bca7e920de4 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -239,6 +239,7 @@
     end
 
     trait :script_failure do
+      failed
       failure_reason 1
     end
   end
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
index 1e5f23bbac6..24f5f15ff2b 100644
--- a/spec/lib/gitlab/ci/status/build/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Status::Build::Failed do
-  let(:build) { create(:ci_build, :failed, :script_failure) }
+  let(:build) { create(:ci_build, :script_failure) }
   let(:status) { double('core status') }
   let(:user) { double('user') }
 
@@ -41,13 +41,22 @@
 
   describe '#label' do
     let(:user) { create(:user) }
-    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
 
     it 'does override label' do
       expect(subject.label).to eq 'failed <br> (script failure)'
     end
   end
 
+  describe '#titleize_label' do
+    let(:user) { create(:user) }
+    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
+
+    it 'returns a titleize label' do
+      expect(subject.titleize_label).to eq 'Failed <br> (script failure)'
+    end
+  end
+
   describe '.matches?' do
     context 'with a failed build' do
       it 'returns true' do
@@ -63,4 +72,14 @@
       end
     end
   end
+
+  describe '#retry_label?' do
+    let(:user) { create(:user) }
+    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
+    let(:build) { create(:ci_build, :failed, :retried) }
+
+    it 'returns retried label and failure reason' do
+      expect(subject.retry_label).to eq "#{build.name} - failed (retried) <br> (unknown failure)"
+    end
+  end
 end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index ffb5fb96c7c..acac9bcc027 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -147,12 +147,12 @@
     end
   end
 
-  describe '#tooltip_description' do
+  describe '#tooltip_for_status' do
     context 'For failed builds' do
-      let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
+      let(:build) { create(:ci_build, :script_failure, pipeline: pipeline) }
 
       it 'returns the reason of failure' do
-        tooltip_description = subject.tooltip_description
+        tooltip_description = subject.tooltip_for_status
 
         expect(tooltip_description).to eq('Failed <br> (script failure)')
       end
@@ -162,24 +162,51 @@
       let(:build) { create(:ci_build, :success, pipeline: pipeline) }
 
       it 'returns the status' do
-        tooltip_description = subject.tooltip_description
+        tooltip_description = subject.tooltip_for_status
 
         expect(tooltip_description).to eq('Success')
       end
     end
   end
 
-  describe '#graph_tooltip_description' do
+  describe '#tooltip_for_pipeline' do
     context 'For failed builds' do
-      let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
+      let(:build) { create(:ci_build, :script_failure, pipeline: pipeline) }
 
       it 'returns the reason of failure' do
-        graph_tooltip_description = subject.graph_tooltip_description
+        tooltip = subject.tooltip_for_pipeline
 
-        expect(graph_tooltip_description).to include(build.name)
-        expect(graph_tooltip_description).to include('failed')
-        expect(graph_tooltip_description).to include('<br>')
-        expect(graph_tooltip_description).to include('(script failure)')
+        expect(tooltip).to eq("#{build.name} - failed <br> (script failure)")
+      end
+    end
+
+    context 'For failed retried build' do
+      let(:build) { create(:ci_build, :script_failure, :retried, pipeline: pipeline) }
+
+      it 'should include the reason of failure and the retried title' do
+        tooltip = subject.tooltip_for_pipeline
+
+        expect(tooltip).to eq("#{build.name} - failed (retried) <br> (script failure)")
+      end
+    end
+
+    context 'For any other build (no retried)' do
+      let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+
+      it 'should include build name and status' do
+        tooltip = subject.tooltip_for_pipeline
+
+        expect(tooltip).to eq("#{build.name} - passed")
+      end
+    end
+
+    context 'For any other build (retried)' do
+      let(:build) { create(:ci_build, :success, :retried, pipeline: pipeline) }
+
+      it 'should include build name and status' do
+        tooltip = subject.tooltip_for_pipeline
+
+        expect(tooltip).to eq("#{build.name} - passed (retried)")
       end
     end
   end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index 6b82249aec5..78841f67af3 100644
--- a/spec/serializers/job_entity_spec.rb
+++ b/spec/serializers/job_entity_spec.rb
@@ -131,7 +131,7 @@
   end
 
   context 'when job failed' do
-    let(:job) { create(:ci_build, :failed, :script_failure) }
+    let(:job) { create(:ci_build, :script_failure) }
 
     describe 'status' do
       it 'should contain the failure reason inside label' do
-- 
GitLab


From 31c6b64df4ed862cc192ce14634f45e184af5f94 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Tue, 27 Mar 2018 09:31:40 -0600
Subject: [PATCH 15/21] Add feature specs for tooltips for pipelines and jobs

Also dry-up logic from job view by creating a BuildsPresenter
---
 .../concerns/builds_presentation.rb           | 11 ++++++++++
 app/controllers/projects/jobs_controller.rb   |  4 ++--
 app/presenters/ci/builds_presenter.rb         | 13 +++++++++++
 app/views/projects/jobs/_sidebar.html.haml    |  4 +---
 app/views/projects/jobs/show.html.haml        |  2 +-
 .../projects/jobs/user_browses_job_spec.rb    | 22 +++++++++++++++++++
 .../projects/jobs/user_browses_jobs_spec.rb   | 11 ++++++++++
 .../projects/pipelines/pipeline_spec.rb       | 16 ++++++++++++++
 .../projects/pipelines/pipelines_spec.rb      | 17 ++++++++++++++
 .../projects/jobs/show.html.haml_spec.rb      | 10 +++++++++
 10 files changed, 104 insertions(+), 6 deletions(-)
 create mode 100644 app/controllers/concerns/builds_presentation.rb
 create mode 100644 app/presenters/ci/builds_presenter.rb

diff --git a/app/controllers/concerns/builds_presentation.rb b/app/controllers/concerns/builds_presentation.rb
new file mode 100644
index 00000000000..fb2eff2c44b
--- /dev/null
+++ b/app/controllers/concerns/builds_presentation.rb
@@ -0,0 +1,11 @@
+module BuildsPresentation
+  extend ActiveSupport::Concern
+
+  def present_builds(builds)
+    Gitlab::View::Presenter::Factory.new(
+      builds,
+      current_user: current_user,
+      presenter_class: Ci::BuildsPresenter
+    ).fabricate!
+  end
+end
diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index 85e972d9731..7755b7a46b1 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -1,8 +1,8 @@
 class Projects::JobsController < Projects::ApplicationController
   include SendFileUpload
+  include BuildsPresentation
 
   before_action :build, except: [:index, :cancel_all]
-
   before_action :authorize_read_build!,
     only: [:index, :show, :status, :raw, :trace]
   before_action :authorize_update_build!,
@@ -46,7 +46,7 @@ def cancel_all
 
   def show
     @builds = @project.pipelines.find_by_sha(@build.sha).builds.order('id DESC')
-    @builds = @builds.where("id not in (?)", @build.id)
+    @builds = present_builds(@builds)
     @pipeline = @build.pipeline
 
     respond_to do |format|
diff --git a/app/presenters/ci/builds_presenter.rb b/app/presenters/ci/builds_presenter.rb
new file mode 100644
index 00000000000..7821290e544
--- /dev/null
+++ b/app/presenters/ci/builds_presenter.rb
@@ -0,0 +1,13 @@
+module Ci
+  class BuildsPresenter < Gitlab::View::Presenter::Delegated
+    include Enumerable
+
+    presents :builds
+
+    def each
+      builds.each do |build|
+        yield build.present(current_user: current_user)
+      end
+    end
+  end
+end
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index 4f43cd05cf7..750d033fdc1 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -1,5 +1,3 @@
-- builds = @build.pipeline.builds.to_a
-
 %aside.right-sidebar.right-sidebar-expanded.build-sidebar.js-build-sidebar.js-right-sidebar{ data: { "offset-top" => "101", "spy" => "affix" } }
   .sidebar-container
     .blocks-container
@@ -91,7 +89,7 @@
       - HasStatus::ORDERED_STATUSES.each do |build_status|
         - builds.select{|build| build.status == build_status}.each do |build|
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
-            - tooltip = build.present.tooltip_for_pipeline
+            - tooltip = build.tooltip_for_pipeline
             = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: true, title: tooltip, container: 'body' }) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
               %span{ class: "ci-status-icon-#{build.status}" }
diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml
index 849c273db8c..996c9a52b5a 100644
--- a/app/views/projects/jobs/show.html.haml
+++ b/app/views/projects/jobs/show.html.haml
@@ -107,7 +107,7 @@
         illustration_size: 'svg-430',
         title: _('This job has not started yet'),
         content: _('This job is in pending state and is waiting to be picked by a runner')
-  = render "sidebar"
+  = render "sidebar", builds: @builds
 
 .js-build-options{ data: javascript_build_options }
 
diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb
index 4c49cff30d4..126746f2f7c 100644
--- a/spec/features/projects/jobs/user_browses_job_spec.rb
+++ b/spec/features/projects/jobs/user_browses_job_spec.rb
@@ -34,4 +34,26 @@
 
     expect(build.project.running_or_pending_build_count).to eq(build.project.builds.running_or_pending.count(:all))
   end
+
+  context 'with a failed job' do
+    let!(:build) { create(:ci_build, :failed, pipeline: pipeline) }
+
+    it 'displays the failure reason' do
+      within('.builds-container') do
+        build_link = first('.build-job > a')
+        expect(build_link['data-title']).to eq('test - failed <br> (unknown failure)')
+      end
+    end
+  end
+
+  context 'when a failed job has been retried' do
+    let!(:build) { create(:ci_build, :failed, :retried, pipeline: pipeline) }
+
+    it 'displays the failure reason and retried label' do
+      within('.builds-container') do
+        build_link = first('.build-job > a')
+        expect(build_link['data-title']).to eq('test - failed (retried) <br> (unknown failure)')
+      end
+    end
+  end
 end
diff --git a/spec/features/projects/jobs/user_browses_jobs_spec.rb b/spec/features/projects/jobs/user_browses_jobs_spec.rb
index 767777f3bf9..36ebbeadd4a 100644
--- a/spec/features/projects/jobs/user_browses_jobs_spec.rb
+++ b/spec/features/projects/jobs/user_browses_jobs_spec.rb
@@ -29,4 +29,15 @@
       expect(ci_lint_tool_link[:href]).to end_with(ci_lint_path)
     end
   end
+
+  context 'with a failed job' do
+    let!(:build) { create(:ci_build, :coverage, :failed, pipeline: pipeline) }
+
+    it 'displays a tooltip with the failure reason' do
+      page.within('.ci-table') do
+        failed_job_link = page.find('.ci-failed')
+        expect(failed_job_link[:title]).to eq('Failed <br> (unknown failure)')
+      end
+    end
+  end
 end
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 266ef693d0b..990e5c4d9df 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -115,6 +115,13 @@
 
           expect(page).not_to have_content('Retry job')
         end
+
+        it 'should include the failure reason' do
+          page.within('#ci-badge-test') do
+            build_link = page.find('.js-pipeline-graph-job-link')
+            expect(build_link['data-original-title']).to eq('test - failed <br> (unknown failure)')
+          end
+        end
       end
 
       context 'when pipeline has manual jobs' do
@@ -289,6 +296,15 @@
 
       it { expect(build_manual.reload).to be_pending }
     end
+
+    context 'failed jobs' do
+      it 'displays a tooltip with the failure reason' do
+        page.within('.ci-table') do
+          failed_job_link = page.find('.ci-failed')
+          expect(failed_job_link[:title]).to eq('Failed <br> (unknown failure)')
+        end
+      end
+    end
   end
 
   describe 'GET /:project/pipelines/:id/failures' do
diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb
index 0e81c6c629a..6e63e0f0b49 100644
--- a/spec/features/projects/pipelines/pipelines_spec.rb
+++ b/spec/features/projects/pipelines/pipelines_spec.rb
@@ -394,6 +394,23 @@
             expect(build.reload).to be_canceled
           end
         end
+
+        context 'for a failed pipeline' do
+          let!(:build) do
+            create(:ci_build, :failed, pipeline: pipeline,
+                                       stage: 'build',
+                                       name: 'build')
+          end
+
+          it 'should display the failure reason' do
+            find('.js-builds-dropdown-button').click
+
+            within('.js-builds-dropdown-list') do
+              build_element = page.find('.mini-pipeline-graph-dropdown-item')
+              expect(build_element['data-title']).to eq('build - failed <br> (unknown failure)')
+            end
+          end
+        end
       end
 
       context 'with pagination' do
diff --git a/spec/views/projects/jobs/show.html.haml_spec.rb b/spec/views/projects/jobs/show.html.haml_spec.rb
index 6a67da79ec5..88ac827e807 100644
--- a/spec/views/projects/jobs/show.html.haml_spec.rb
+++ b/spec/views/projects/jobs/show.html.haml_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe 'projects/jobs/show' do
+  let(:user) { create(:user) }
   let(:project) { create(:project, :repository) }
   let(:build) { create(:ci_build, pipeline: pipeline) }
 
@@ -8,9 +9,18 @@
     create(:ci_pipeline, project: project, sha: project.commit.id)
   end
 
+  let(:builds) do
+    Gitlab::View::Presenter::Factory.new(
+      project.builds,
+      current_user: user,
+      presenter_class: Ci::BuildsPresenter
+    ).fabricate!
+  end
+
   before do
     assign(:build, build.present)
     assign(:project, project)
+    assign(:builds, builds)
 
     allow(view).to receive(:can?).and_return(true)
   end
-- 
GitLab


From e6485973bc274771add00b778eafe94d5610589d Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Wed, 28 Mar 2018 11:44:37 -0600
Subject: [PATCH 16/21] Address comments on MR

- Move retry logic from Ci::Status::Build::Failed to BuildPresenter
- Simplify logic in jobs views
- Rename methods on BuildPresenter to be more descriptive
---
 app/presenters/ci/build_presenter.rb          | 42 +++++++++-------
 .../ci/status/_dropdown_graph_badge.html.haml |  2 +-
 app/views/projects/jobs/_sidebar.html.haml    |  2 +-
 lib/gitlab/ci/status/build/failed.rb          |  8 ----
 .../lib/gitlab/ci/status/build/failed_spec.rb | 10 ----
 spec/presenters/ci/build_presenter_spec.rb    | 48 +++++++------------
 6 files changed, 44 insertions(+), 68 deletions(-)

diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 9c77617b911..410a0116e4e 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -31,40 +31,48 @@ def trigger_variables
         end
     end
 
-    def tooltip_for_status
-      if failed_or_allowed_to_failed?
-        build_failed_status.titleize_label
-      else
-        status.titleize
-      end
-    end
-
-    def tooltip_for_pipeline
+    def tooltip_message
       if build.failed? && build.retried?
-        build_failed_status.retry_label
+        "#{failed_retry_title} #{failed_description}"
       elsif build.retried?
-        tooltip_pipeline_message + " (retried)"
+        tooltip_core_message + " (retried)"
       else
-        tooltip_pipeline_message
+        tooltip_core_message
       end
     end
 
     private
 
+    def tooltip_for_status
+      if failed_or_allowed_to_failed?
+        failed_status.titleize_label
+      else
+        status.titleize
+      end
+    end
+
     def failed_or_allowed_to_failed?
       failed? || allow_failure?
     end
 
-    def build_failed_status
-      Gitlab::Ci::Status::Build::Failed.new(failed_status)
+    def failed_status
+      Gitlab::Ci::Status::Build::Failed.new(core_failed_status)
     end
 
-    def failed_status
-      @failed_status ||= Gitlab::Ci::Status::Failed.new(subject, user)
+    def core_failed_status
+      @core_failed_status ||= Gitlab::Ci::Status::Failed.new(subject, user)
     end
 
-    def tooltip_pipeline_message
+    def tooltip_core_message
       "#{name} - #{subject.detailed_status(user).label}"
     end
+
+    def failed_retry_title
+      subject.name + " - " + s_('CiStatusLabel|failed') + " (retried)"
+    end
+
+    def failed_description
+      "<br> (#{Gitlab::Ci::Status::Build::Failed::FAILURE_REASONS[subject.failure_reason]})"
+    end
   end
 end
diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml
index 453c84604ac..63f546cc9e8 100644
--- a/app/views/ci/status/_dropdown_graph_badge.html.haml
+++ b/app/views/ci/status/_dropdown_graph_badge.html.haml
@@ -3,7 +3,7 @@
 - subject = local_assigns.fetch(:subject)
 - status = subject.detailed_status(current_user)
 - klass = "ci-status-icon ci-status-icon-#{status.group}"
-- tooltip = subject.present.tooltip_for_pipeline
+- tooltip = "#{subject.name} - #{status.label}"
 
 - if status.has_details?
   = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, html: true, container: 'body' }  do
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index 750d033fdc1..0b57ebedebd 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -89,7 +89,7 @@
       - HasStatus::ORDERED_STATUSES.each do |build_status|
         - builds.select{|build| build.status == build_status}.each do |build|
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
-            - tooltip = build.tooltip_for_pipeline
+            - tooltip = build.tooltip_message
             = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: true, title: tooltip, container: 'body' }) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
               %span{ class: "ci-status-icon-#{build.status}" }
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
index 5a5dcca85c2..1986e398d4a 100644
--- a/lib/gitlab/ci/status/build/failed.rb
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -24,10 +24,6 @@ def self.matches?(build, user)
             build.failed? && !build.allow_failure?
           end
 
-          def retry_label
-            "#{retry_title} #{description}"
-          end
-
           private
 
           def failure_reason_description(titleize = false)
@@ -43,10 +39,6 @@ def failed_title
           def description
             "<br> (#{FAILURE_REASONS[subject.failure_reason]})"
           end
-
-          def retry_title
-            subject.name + " - " + failed_title + " (retried)"
-          end
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
index 24f5f15ff2b..73382725e53 100644
--- a/spec/lib/gitlab/ci/status/build/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -72,14 +72,4 @@
       end
     end
   end
-
-  describe '#retry_label?' do
-    let(:user) { create(:user) }
-    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
-    let(:build) { create(:ci_build, :failed, :retried) }
-
-    it 'returns retried label and failure reason' do
-      expect(subject.retry_label).to eq "#{build.name} - failed (retried) <br> (unknown failure)"
-    end
-  end
 end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index acac9bcc027..da44835050f 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -78,8 +78,7 @@
       it 'returns the reason of failure' do
         status_title = presenter.status_title
 
-        expect(status_title).to include('Failed')
-        expect(status_title).to include('(unknown failure)')
+        expect(status_title).to eq('Failed <br> (unknown failure)')
       end
     end
 
@@ -89,8 +88,17 @@
       it 'returns the reason of failure' do
         status_title = presenter.status_title
 
-        expect(status_title).to include('Failed')
-        expect(status_title).to include('(script failure)')
+        expect(status_title).to eq('Failed <br> (script failure)')
+      end
+    end
+
+    context 'For any other build' do
+      let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+
+      it 'returns the status' do
+        tooltip_description = presenter.status_title
+
+        expect(tooltip_description).to eq('Success')
       end
     end
   end
@@ -147,34 +155,12 @@
     end
   end
 
-  describe '#tooltip_for_status' do
-    context 'For failed builds' do
-      let(:build) { create(:ci_build, :script_failure, pipeline: pipeline) }
-
-      it 'returns the reason of failure' do
-        tooltip_description = subject.tooltip_for_status
-
-        expect(tooltip_description).to eq('Failed <br> (script failure)')
-      end
-    end
-
-    context 'For any other build' do
-      let(:build) { create(:ci_build, :success, pipeline: pipeline) }
-
-      it 'returns the status' do
-        tooltip_description = subject.tooltip_for_status
-
-        expect(tooltip_description).to eq('Success')
-      end
-    end
-  end
-
-  describe '#tooltip_for_pipeline' do
+  describe '#tooltip_message' do
     context 'For failed builds' do
       let(:build) { create(:ci_build, :script_failure, pipeline: pipeline) }
 
       it 'returns the reason of failure' do
-        tooltip = subject.tooltip_for_pipeline
+        tooltip = subject.tooltip_message
 
         expect(tooltip).to eq("#{build.name} - failed <br> (script failure)")
       end
@@ -184,7 +170,7 @@
       let(:build) { create(:ci_build, :script_failure, :retried, pipeline: pipeline) }
 
       it 'should include the reason of failure and the retried title' do
-        tooltip = subject.tooltip_for_pipeline
+        tooltip = subject.tooltip_message
 
         expect(tooltip).to eq("#{build.name} - failed (retried) <br> (script failure)")
       end
@@ -194,7 +180,7 @@
       let(:build) { create(:ci_build, :success, pipeline: pipeline) }
 
       it 'should include build name and status' do
-        tooltip = subject.tooltip_for_pipeline
+        tooltip = subject.tooltip_message
 
         expect(tooltip).to eq("#{build.name} - passed")
       end
@@ -204,7 +190,7 @@
       let(:build) { create(:ci_build, :success, :retried, pipeline: pipeline) }
 
       it 'should include build name and status' do
-        tooltip = subject.tooltip_for_pipeline
+        tooltip = subject.tooltip_message
 
         expect(tooltip).to eq("#{build.name} - passed (retried)")
       end
-- 
GitLab


From 2e066f7ac4fa4c28cc0040635ba573b863c0b2b4 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Thu, 29 Mar 2018 10:23:44 -0600
Subject: [PATCH 17/21] Implement 'tooltip' on Gitlab::Ci::Status

This allow us to split responsibilities between label and tooltip. Also
removes extra methods from Gitlab::Ci::Status that are no part of the
abstract class

Also create a 'present' method on Build model, so we can use
'pipeline.builds.present'
---
 .../components/graph/job_component.vue         |  8 +++++---
 .../concerns/builds_presentation.rb            | 11 -----------
 app/controllers/projects/jobs_controller.rb    |  8 +++++---
 app/models/ci/build.rb                         |  4 ++++
 app/presenters/ci/build_presenter.rb           | 18 +++++++-----------
 app/presenters/ci/builds_presenter.rb          | 13 -------------
 app/serializers/status_entity.rb               |  2 +-
 .../ci/status/_dropdown_graph_badge.html.haml  |  2 +-
 lib/gitlab/ci/status/build/failed.rb           | 14 ++------------
 lib/gitlab/ci/status/core.rb                   |  4 ++++
 lib/gitlab/ci/status/success_warning.rb        |  4 ++++
 .../lib/gitlab/ci/status/build/factory_spec.rb |  3 ++-
 spec/lib/gitlab/ci/status/build/failed_spec.rb | 13 ++++++-------
 spec/lib/gitlab/ci/status/canceled_spec.rb     |  4 ++++
 spec/lib/gitlab/ci/status/created_spec.rb      |  4 ++++
 spec/lib/gitlab/ci/status/failed_spec.rb       |  4 ++++
 spec/lib/gitlab/ci/status/manual_spec.rb       |  4 ++++
 spec/lib/gitlab/ci/status/pending_spec.rb      |  4 ++++
 spec/lib/gitlab/ci/status/running_spec.rb      |  4 ++++
 spec/lib/gitlab/ci/status/skipped_spec.rb      |  4 ++++
 spec/lib/gitlab/ci/status/success_spec.rb      |  4 ++++
 .../gitlab/ci/status/success_warning_spec.rb   |  4 ++++
 spec/serializers/build_serializer_spec.rb      |  4 +++-
 spec/serializers/job_entity_spec.rb            |  9 +++++----
 spec/serializers/pipeline_entity_spec.rb       |  2 +-
 spec/serializers/stage_entity_spec.rb          |  2 +-
 spec/serializers/status_entity_spec.rb         |  2 +-
 .../views/projects/jobs/show.html.haml_spec.rb |  9 +--------
 28 files changed, 89 insertions(+), 79 deletions(-)
 delete mode 100644 app/controllers/concerns/builds_presentation.rb
 delete mode 100644 app/presenters/ci/builds_presenter.rb

diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue
index ad21760a3bd..d501c465a96 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -17,6 +17,7 @@
    *     "text": "passed",
    *     "label": "passed",
    *     "group": "success",
+   *     "tooltip": "passed",
    *     "details_path": "/root/ci-mock/builds/4256",
    *     "action": {
    *       "icon": "retry",
@@ -69,11 +70,12 @@
           textBuilder.push(this.job.name);
         }
 
-        if (this.job.name && this.status.label) {
+        if (this.job.name && this.status.tooltip) {
           textBuilder.push('-');
         }
-        if (this.status.label) {
-          textBuilder.push(`${this.job.status.label}`);
+
+        if (this.status.tooltip) {
+          textBuilder.push(`${this.job.status.tooltip}`);
         }
 
         return textBuilder.join(' ');
diff --git a/app/controllers/concerns/builds_presentation.rb b/app/controllers/concerns/builds_presentation.rb
deleted file mode 100644
index fb2eff2c44b..00000000000
--- a/app/controllers/concerns/builds_presentation.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-module BuildsPresentation
-  extend ActiveSupport::Concern
-
-  def present_builds(builds)
-    Gitlab::View::Presenter::Factory.new(
-      builds,
-      current_user: current_user,
-      presenter_class: Ci::BuildsPresenter
-    ).fabricate!
-  end
-end
diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index 7755b7a46b1..278b69f6ae2 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -1,6 +1,5 @@
 class Projects::JobsController < Projects::ApplicationController
   include SendFileUpload
-  include BuildsPresentation
 
   before_action :build, except: [:index, :cancel_all]
   before_action :authorize_read_build!,
@@ -45,8 +44,11 @@ def cancel_all
   end
 
   def show
-    @builds = @project.pipelines.find_by_sha(@build.sha).builds.order('id DESC')
-    @builds = present_builds(@builds)
+    @builds = @project.pipelines
+      .find_by_sha(@build.sha)
+      .builds
+      .order('id DESC')
+      .present(current_user: current_user)
     @pipeline = @build.pipeline
 
     respond_to do |format|
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 08bb5915d10..4b7300ee7d8 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -530,6 +530,10 @@ def serializable_hash(options = {})
       super(options).merge(when: read_attribute(:when))
     end
 
+    def self.present(current_user:)
+      all.map { |build| build.present(current_user: current_user) }
+    end
+
     private
 
     def update_artifacts_size
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 410a0116e4e..1ad3cf335d2 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -45,7 +45,7 @@ def tooltip_message
 
     def tooltip_for_status
       if failed_or_allowed_to_failed?
-        failed_status.titleize_label
+        "#{failed_title.titleize} #{failed_description}"
       else
         status.titleize
       end
@@ -55,24 +55,20 @@ def failed_or_allowed_to_failed?
       failed? || allow_failure?
     end
 
-    def failed_status
-      Gitlab::Ci::Status::Build::Failed.new(core_failed_status)
-    end
-
-    def core_failed_status
-      @core_failed_status ||= Gitlab::Ci::Status::Failed.new(subject, user)
-    end
-
     def tooltip_core_message
-      "#{name} - #{subject.detailed_status(user).label}"
+      "#{name} - #{subject.detailed_status(user).tooltip}"
     end
 
     def failed_retry_title
-      subject.name + " - " + s_('CiStatusLabel|failed') + " (retried)"
+      "#{subject.name} - #{failed_title} (retried)"
     end
 
     def failed_description
       "<br> (#{Gitlab::Ci::Status::Build::Failed::FAILURE_REASONS[subject.failure_reason]})"
     end
+
+    def failed_title
+      s_('CiStatusLabel|failed')
+    end
   end
 end
diff --git a/app/presenters/ci/builds_presenter.rb b/app/presenters/ci/builds_presenter.rb
deleted file mode 100644
index 7821290e544..00000000000
--- a/app/presenters/ci/builds_presenter.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-module Ci
-  class BuildsPresenter < Gitlab::View::Presenter::Delegated
-    include Enumerable
-
-    presents :builds
-
-    def each
-      builds.each do |build|
-        yield build.present(current_user: current_user)
-      end
-    end
-  end
-end
diff --git a/app/serializers/status_entity.rb b/app/serializers/status_entity.rb
index 3e40ecf1c1c..0157a572f28 100644
--- a/app/serializers/status_entity.rb
+++ b/app/serializers/status_entity.rb
@@ -1,7 +1,7 @@
 class StatusEntity < Grape::Entity
   include RequestAwareEntity
 
-  expose :icon, :text, :label, :group
+  expose :icon, :text, :label, :group, :tooltip
 
   expose :has_details?, as: :has_details
   expose :details_path
diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml
index 63f546cc9e8..1245715aa03 100644
--- a/app/views/ci/status/_dropdown_graph_badge.html.haml
+++ b/app/views/ci/status/_dropdown_graph_badge.html.haml
@@ -3,7 +3,7 @@
 - subject = local_assigns.fetch(:subject)
 - status = subject.detailed_status(current_user)
 - klass = "ci-status-icon ci-status-icon-#{status.group}"
-- tooltip = "#{subject.name} - #{status.label}"
+- tooltip = "#{subject.name} - #{status.tooltip}"
 
 - if status.has_details?
   = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, html: true, container: 'body' }  do
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
index 1986e398d4a..5fb1cc506f4 100644
--- a/lib/gitlab/ci/status/build/failed.rb
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -12,12 +12,8 @@ class Failed < Status::Extended
             'missing_dependency_failure' => 'missing dependency failure'
           }.freeze
 
-          def titleize_label
-            failure_reason_description(true)
-          end
-
-          def label
-            failure_reason_description
+          def tooltip
+            "#{failed_title} #{description}"
           end
 
           def self.matches?(build, user)
@@ -26,12 +22,6 @@ def self.matches?(build, user)
 
           private
 
-          def failure_reason_description(titleize = false)
-            title = titleize ? failed_title.titleize : failed_title
-
-            "#{title} #{description}"
-          end
-
           def failed_title
             s_('CiStatusLabel|failed')
           end
diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb
index d4fd83b93f8..8d2db3d25d4 100644
--- a/lib/gitlab/ci/status/core.rb
+++ b/lib/gitlab/ci/status/core.rb
@@ -57,6 +57,10 @@ def action_method
         def action_title
           raise NotImplementedError
         end
+
+        def tooltip
+          label
+        end
       end
     end
   end
diff --git a/lib/gitlab/ci/status/success_warning.rb b/lib/gitlab/ci/status/success_warning.rb
index 32b4cf43e48..61af3526d21 100644
--- a/lib/gitlab/ci/status/success_warning.rb
+++ b/lib/gitlab/ci/status/success_warning.rb
@@ -22,6 +22,10 @@ def group
           'success_with_warnings'
         end
 
+        def tooltip
+          label
+        end
+
         def self.matches?(subject, user)
           subject.success? && subject.has_warnings?
         end
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index a3e4e4f21f6..1e74ac3fa09 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -59,7 +59,8 @@
         expect(status.text).to eq 'failed'
         expect(status.icon).to eq 'status_failed'
         expect(status.favicon).to eq 'favicon_status_failed'
-        expect(status.label).to eq 'failed <br> (unknown failure)'
+        expect(status.label).to eq 'failed'
+        expect(status.tooltip).to eq 'failed <br> (unknown failure)'
         expect(status).to have_details
         expect(status).to have_action
       end
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
index 73382725e53..7ae15ba2893 100644
--- a/spec/lib/gitlab/ci/status/build/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -40,20 +40,19 @@
   end
 
   describe '#label' do
-    let(:user) { create(:user) }
-    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
+    it 'does not override label' do
+      expect(status).to receive(:label)
 
-    it 'does override label' do
-      expect(subject.label).to eq 'failed <br> (script failure)'
+      subject.label
     end
   end
 
-  describe '#titleize_label' do
+  describe '#tooltip' do
     let(:user) { create(:user) }
     let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
 
-    it 'returns a titleize label' do
-      expect(subject.titleize_label).to eq 'Failed <br> (script failure)'
+    it 'does override label' do
+      expect(subject.tooltip).to eq 'failed <br> (script failure)'
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb
index dc74d7e28c5..f622e899d6d 100644
--- a/spec/lib/gitlab/ci/status/canceled_spec.rb
+++ b/spec/lib/gitlab/ci/status/canceled_spec.rb
@@ -24,4 +24,8 @@
   describe '#group' do
     it { expect(subject.group).to eq 'canceled' }
   end
+
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'canceled' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb
index ce4333f2aca..9dc53abebab 100644
--- a/spec/lib/gitlab/ci/status/created_spec.rb
+++ b/spec/lib/gitlab/ci/status/created_spec.rb
@@ -24,4 +24,8 @@
   describe '#group' do
     it { expect(subject.group).to eq 'created' }
   end
+
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'created' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/failed_spec.rb b/spec/lib/gitlab/ci/status/failed_spec.rb
index a4a92117c7f..754a30809f2 100644
--- a/spec/lib/gitlab/ci/status/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/failed_spec.rb
@@ -24,4 +24,8 @@
   describe '#group' do
     it { expect(subject.group).to eq 'failed' }
   end
+
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'failed' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/manual_spec.rb b/spec/lib/gitlab/ci/status/manual_spec.rb
index 0463f2e1aff..af821f51339 100644
--- a/spec/lib/gitlab/ci/status/manual_spec.rb
+++ b/spec/lib/gitlab/ci/status/manual_spec.rb
@@ -24,4 +24,8 @@
   describe '#group' do
     it { expect(subject.group).to eq 'manual' }
   end
+
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'manual action' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb
index 0e25358dd8a..99e26aaec41 100644
--- a/spec/lib/gitlab/ci/status/pending_spec.rb
+++ b/spec/lib/gitlab/ci/status/pending_spec.rb
@@ -24,4 +24,8 @@
   describe '#group' do
     it { expect(subject.group).to eq 'pending' }
   end
+
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'pending' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/running_spec.rb b/spec/lib/gitlab/ci/status/running_spec.rb
index 9c9d431bb5d..b7b61743abf 100644
--- a/spec/lib/gitlab/ci/status/running_spec.rb
+++ b/spec/lib/gitlab/ci/status/running_spec.rb
@@ -24,4 +24,8 @@
   describe '#group' do
     it { expect(subject.group).to eq 'running' }
   end
+
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'running' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb
index 63694ca0ea6..46165a80294 100644
--- a/spec/lib/gitlab/ci/status/skipped_spec.rb
+++ b/spec/lib/gitlab/ci/status/skipped_spec.rb
@@ -24,4 +24,8 @@
   describe '#group' do
     it { expect(subject.group).to eq 'skipped' }
   end
+
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'skipped' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/success_spec.rb b/spec/lib/gitlab/ci/status/success_spec.rb
index 2f67df71c4f..69caa990455 100644
--- a/spec/lib/gitlab/ci/status/success_spec.rb
+++ b/spec/lib/gitlab/ci/status/success_spec.rb
@@ -24,4 +24,8 @@
   describe '#group' do
     it { expect(subject.group).to eq 'success' }
   end
+
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'passed' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/success_warning_spec.rb b/spec/lib/gitlab/ci/status/success_warning_spec.rb
index 4582354e739..05c165640d0 100644
--- a/spec/lib/gitlab/ci/status/success_warning_spec.rb
+++ b/spec/lib/gitlab/ci/status/success_warning_spec.rb
@@ -21,6 +21,10 @@
     it { expect(subject.group).to eq 'success_with_warnings' }
   end
 
+  describe '#tooltip' do
+    it { expect(subject.tooltip).to eq 'passed with warnings' }
+  end
+
   describe '.matches?' do
     let(:matchable) { double('matchable') }
 
diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb
index 0ac23e69d33..98cd15e248b 100644
--- a/spec/serializers/build_serializer_spec.rb
+++ b/spec/serializers/build_serializer_spec.rb
@@ -36,7 +36,8 @@
 
       it 'serializes only status' do
         expect(subject[:text]).to eq(status.text)
-        expect(subject[:label]).to eq('failed <br> (unknown failure)')
+        expect(subject[:label]).to eq('failed')
+        expect(subject[:tooltip]).to eq('failed <br> (unknown failure)')
         expect(subject[:icon]).to eq(status.icon)
         expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
       end
@@ -51,6 +52,7 @@
       it 'serializes only status' do
         expect(subject[:text]).to eq(status.text)
         expect(subject[:label]).to eq('passed')
+        expect(subject[:tooltip]).to eq('passed')
         expect(subject[:icon]).to eq(status.icon)
         expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
       end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index 78841f67af3..24a6f1a2a8a 100644
--- a/spec/serializers/job_entity_spec.rb
+++ b/spec/serializers/job_entity_spec.rb
@@ -38,7 +38,7 @@
 
   it 'contains details' do
     expect(subject).to include :status
-    expect(subject[:status]).to include :icon, :favicon, :text, :label
+    expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
   end
 
   context 'when job is retryable' do
@@ -126,7 +126,7 @@
 
     it 'contains details' do
       expect(subject).to include :status
-      expect(subject[:status]).to include :icon, :favicon, :text, :label
+      expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
     end
   end
 
@@ -135,8 +135,9 @@
 
     describe 'status' do
       it 'should contain the failure reason inside label' do
-        expect(subject[:status]).to include :icon, :favicon, :text, :label
-        expect(subject[:status][:label]).to eq('failed <br> (script failure)')
+        expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
+        expect(subject[:status][:label]).to eq('failed')
+        expect(subject[:status][:tooltip]).to eq('failed <br> (script failure)')
       end
     end
   end
diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb
index 248552d1858..2473c561f4b 100644
--- a/spec/serializers/pipeline_entity_spec.rb
+++ b/spec/serializers/pipeline_entity_spec.rb
@@ -30,7 +30,7 @@
         expect(subject).to include :details
         expect(subject[:details])
           .to include :duration, :finished_at
-        expect(subject[:details][:status]).to include :icon, :favicon, :text, :label
+        expect(subject[:details][:status]).to include :icon, :favicon, :text, :label, :tooltip
       end
 
       it 'contains flags' do
diff --git a/spec/serializers/stage_entity_spec.rb b/spec/serializers/stage_entity_spec.rb
index 40e303f7b89..2034c7891ef 100644
--- a/spec/serializers/stage_entity_spec.rb
+++ b/spec/serializers/stage_entity_spec.rb
@@ -26,7 +26,7 @@
     end
 
     it 'contains detailed status' do
-      expect(subject[:status]).to include :text, :label, :group, :icon
+      expect(subject[:status]).to include :text, :label, :group, :icon, :tooltip
       expect(subject[:status][:label]).to eq 'passed'
     end
 
diff --git a/spec/serializers/status_entity_spec.rb b/spec/serializers/status_entity_spec.rb
index 16431ed4188..4c92e86672b 100644
--- a/spec/serializers/status_entity_spec.rb
+++ b/spec/serializers/status_entity_spec.rb
@@ -16,7 +16,7 @@
     subject { entity.as_json }
 
     it 'contains status details' do
-      expect(subject).to include :text, :icon, :favicon, :label, :group
+      expect(subject).to include :text, :icon, :favicon, :label, :group, :tooltip
       expect(subject).to include :has_details, :details_path
       expect(subject[:favicon]).to match_asset_path('/assets/ci_favicons/favicon_status_success.ico')
     end
diff --git a/spec/views/projects/jobs/show.html.haml_spec.rb b/spec/views/projects/jobs/show.html.haml_spec.rb
index 88ac827e807..9e692159bd0 100644
--- a/spec/views/projects/jobs/show.html.haml_spec.rb
+++ b/spec/views/projects/jobs/show.html.haml_spec.rb
@@ -4,19 +4,12 @@
   let(:user) { create(:user) }
   let(:project) { create(:project, :repository) }
   let(:build) { create(:ci_build, pipeline: pipeline) }
+  let(:builds) { project.builds.present(current_user: user) }
 
   let(:pipeline) do
     create(:ci_pipeline, project: project, sha: project.commit.id)
   end
 
-  let(:builds) do
-    Gitlab::View::Presenter::Factory.new(
-      project.builds,
-      current_user: user,
-      presenter_class: Ci::BuildsPresenter
-    ).fabricate!
-  end
-
   before do
     assign(:build, build.present)
     assign(:project, project)
-- 
GitLab


From 458828cbec33312480a74e67b96a62b481009727 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Fri, 30 Mar 2018 10:12:34 -0600
Subject: [PATCH 18/21] Add three more statuses to Gitlab::Ci::Status::Build

- Failed: Matches if the build has failed, is not allowed to fail and
  hasn't been retried
- FailedRetried: Matches if the build has failed, is not allowed to fail and
  has been retried
- Retried: Matches if the build has been retried and is not a failed
  build
- Also implements ci_badge method, so we can remove the duplication
  between detail_statuses and build presenter
---
 app/models/ci/build.rb                        |  4 -
 app/models/concerns/presentable.rb            | 10 ++
 app/presenters/ci/build_presenter.rb          | 38 ++------
 lib/gitlab/ci/status/build/factory.rb         |  4 +-
 lib/gitlab/ci/status/build/failed.rb          | 19 +---
 lib/gitlab/ci/status/build/failed_allowed.rb  | 10 ++
 lib/gitlab/ci/status/build/failed_retried.rb  | 25 +++++
 lib/gitlab/ci/status/build/failure_data.rb    | 28 ++++++
 lib/gitlab/ci/status/build/retried.rb         | 17 ++++
 lib/gitlab/ci/status/core.rb                  |  4 +
 lib/gitlab/ci/status/success_warning.rb       |  4 -
 .../pipelines/graph/job_component_spec.js     |  2 +
 .../lib/gitlab/ci/status/build/action_spec.rb | 10 ++
 .../gitlab/ci/status/build/cancelable_spec.rb | 18 ++++
 .../ci/status/build/failed_allowed_spec.rb    | 20 ++++
 .../ci/status/build/failed_retried_spec.rb    | 93 +++++++++++++++++++
 .../lib/gitlab/ci/status/build/failed_spec.rb | 11 ++-
 spec/lib/gitlab/ci/status/build/play_spec.rb  | 14 +++
 .../gitlab/ci/status/build/retried_spec.rb    | 83 +++++++++++++++++
 .../gitlab/ci/status/build/retryable_spec.rb  | 18 ++++
 spec/lib/gitlab/ci/status/build/stop_spec.rb  | 18 ++++
 spec/lib/gitlab/ci/status/canceled_spec.rb    |  4 -
 spec/lib/gitlab/ci/status/created_spec.rb     |  4 -
 spec/lib/gitlab/ci/status/failed_spec.rb      |  4 -
 spec/lib/gitlab/ci/status/manual_spec.rb      |  4 -
 spec/lib/gitlab/ci/status/pending_spec.rb     |  4 -
 spec/lib/gitlab/ci/status/running_spec.rb     |  4 -
 spec/lib/gitlab/ci/status/skipped_spec.rb     |  4 -
 spec/lib/gitlab/ci/status/success_spec.rb     |  4 -
 .../gitlab/ci/status/success_warning_spec.rb  |  8 +-
 spec/presenters/ci/build_presenter_spec.rb    | 33 +++++--
 31 files changed, 422 insertions(+), 101 deletions(-)
 create mode 100644 lib/gitlab/ci/status/build/failed_retried.rb
 create mode 100644 lib/gitlab/ci/status/build/failure_data.rb
 create mode 100644 lib/gitlab/ci/status/build/retried.rb
 create mode 100644 spec/lib/gitlab/ci/status/build/failed_retried_spec.rb
 create mode 100644 spec/lib/gitlab/ci/status/build/retried_spec.rb

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 4b7300ee7d8..08bb5915d10 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -530,10 +530,6 @@ def serializable_hash(options = {})
       super(options).merge(when: read_attribute(:when))
     end
 
-    def self.present(current_user:)
-      all.map { |build| build.present(current_user: current_user) }
-    end
-
     private
 
     def update_artifacts_size
diff --git a/app/models/concerns/presentable.rb b/app/models/concerns/presentable.rb
index 7b33b837004..9dd2cddb139 100644
--- a/app/models/concerns/presentable.rb
+++ b/app/models/concerns/presentable.rb
@@ -1,4 +1,14 @@
 module Presentable
+  def self.included(klass)
+    klass.extend(ClassMethods)
+  end
+
+  module ClassMethods
+    def present(attributes)
+      all.map { |klass_object| klass_object.present(attributes) }
+    end
+  end
+
   def present(**attributes)
     Gitlab::View::Presenter::Factory
       .new(self, attributes)
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 1ad3cf335d2..56cfdc9abe3 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -16,7 +16,7 @@ def status_title
       if auto_canceled?
         "Job is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}"
       else
-        tooltip_for_status
+        tooltip_for_ci_badge
       end
     end
 
@@ -32,43 +32,17 @@ def trigger_variables
     end
 
     def tooltip_message
-      if build.failed? && build.retried?
-        "#{failed_retry_title} #{failed_description}"
-      elsif build.retried?
-        tooltip_core_message + " (retried)"
-      else
-        tooltip_core_message
-      end
+      "#{subject.name} - #{detailed_status.tooltip}"
     end
 
     private
 
-    def tooltip_for_status
-      if failed_or_allowed_to_failed?
-        "#{failed_title.titleize} #{failed_description}"
-      else
-        status.titleize
-      end
-    end
-
-    def failed_or_allowed_to_failed?
-      failed? || allow_failure?
-    end
-
-    def tooltip_core_message
-      "#{name} - #{subject.detailed_status(user).tooltip}"
-    end
-
-    def failed_retry_title
-      "#{subject.name} - #{failed_title} (retried)"
-    end
-
-    def failed_description
-      "<br> (#{Gitlab::Ci::Status::Build::Failed::FAILURE_REASONS[subject.failure_reason]})"
+    def tooltip_for_ci_badge
+      detailed_status.ci_badge.capitalize
     end
 
-    def failed_title
-      s_('CiStatusLabel|failed')
+    def detailed_status
+      @detailed_status ||= subject.detailed_status(user)
     end
   end
 end
diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb
index 4f1e5decc66..6caba98efb4 100644
--- a/lib/gitlab/ci/status/build/factory.rb
+++ b/lib/gitlab/ci/status/build/factory.rb
@@ -10,7 +10,9 @@ def self.extended_statuses
               Status::Build::Play,
               Status::Build::Stop],
              [Status::Build::Action],
-             [Status::Build::Failed]]
+             [Status::Build::Failed],
+             [Status::Build::FailedRetried],
+             [Status::Build::Retried]]
           end
 
           def self.common_helpers
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
index 5fb1cc506f4..a5eb961678c 100644
--- a/lib/gitlab/ci/status/build/failed.rb
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -3,14 +3,7 @@ module Ci
     module Status
       module Build
         class Failed < Status::Extended
-          FAILURE_REASONS = {
-            'unknown_failure' => 'unknown failure',
-            'script_failure' => 'script failure',
-            'api_failure' => 'API failure',
-            'stuck_or_timeout_failure' => 'stuck or timeout failure',
-            'runner_system_failure' => 'runner system failure',
-            'missing_dependency_failure' => 'missing dependency failure'
-          }.freeze
+          include Gitlab::Ci::Status::Build::FailureData
 
           def tooltip
             "#{failed_title} #{description}"
@@ -20,14 +13,8 @@ def self.matches?(build, user)
             build.failed? && !build.allow_failure?
           end
 
-          private
-
-          def failed_title
-            s_('CiStatusLabel|failed')
-          end
-
-          def description
-            "<br> (#{FAILURE_REASONS[subject.failure_reason]})"
+          def ci_badge
+            tooltip
           end
         end
       end
diff --git a/lib/gitlab/ci/status/build/failed_allowed.rb b/lib/gitlab/ci/status/build/failed_allowed.rb
index dc90f398c7e..dfd5d4a0f27 100644
--- a/lib/gitlab/ci/status/build/failed_allowed.rb
+++ b/lib/gitlab/ci/status/build/failed_allowed.rb
@@ -3,6 +3,8 @@ module Ci
     module Status
       module Build
         class FailedAllowed < Status::Extended
+          include Gitlab::Ci::Status::Build::FailureData
+
           def label
             'failed (allowed to fail)'
           end
@@ -15,6 +17,14 @@ def group
             'failed_with_warnings'
           end
 
+          def tooltip
+            "#{failed_title} #{description}"
+          end
+
+          def ci_badge
+            tooltip
+          end
+
           def self.matches?(build, user)
             build.failed? && build.allow_failure?
           end
diff --git a/lib/gitlab/ci/status/build/failed_retried.rb b/lib/gitlab/ci/status/build/failed_retried.rb
new file mode 100644
index 00000000000..5afc2ab77ea
--- /dev/null
+++ b/lib/gitlab/ci/status/build/failed_retried.rb
@@ -0,0 +1,25 @@
+module Gitlab
+  module Ci
+    module Status
+      module Build
+        class FailedRetried < Status::Extended
+          include Gitlab::Ci::Status::Build::FailureData
+
+          def tooltip
+            "#{failed_title} (retried) #{description}"
+          end
+
+          def ci_badge
+            "#{failed_title} #{description}"
+          end
+
+          def self.matches?(build, user)
+            build.failed? &&
+              build.retried? &&
+              !build.allow_failure?
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/status/build/failure_data.rb b/lib/gitlab/ci/status/build/failure_data.rb
new file mode 100644
index 00000000000..d03d0bb741c
--- /dev/null
+++ b/lib/gitlab/ci/status/build/failure_data.rb
@@ -0,0 +1,28 @@
+module Gitlab
+  module Ci
+    module Status
+      module Build
+        module FailureData
+          REASONS = {
+            'unknown_failure' => 'unknown failure',
+            'script_failure' => 'script failure',
+            'api_failure' => 'API failure',
+            'stuck_or_timeout_failure' => 'stuck or timeout failure',
+            'runner_system_failure' => 'runner system failure',
+            'missing_dependency_failure' => 'missing dependency failure'
+          }.freeze
+
+          private
+
+          def failed_title
+            s_('CiStatusLabel|failed')
+          end
+
+          def description
+            "<br> (#{REASONS[subject.failure_reason]})"
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/status/build/retried.rb b/lib/gitlab/ci/status/build/retried.rb
new file mode 100644
index 00000000000..9e3b1338452
--- /dev/null
+++ b/lib/gitlab/ci/status/build/retried.rb
@@ -0,0 +1,17 @@
+module Gitlab
+  module Ci
+    module Status
+      module Build
+        class Retried < Status::Extended
+          def tooltip
+            "#{label} (retried)"
+          end
+
+          def self.matches?(build, user)
+            build.retried? && !build.failed?
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb
index 8d2db3d25d4..e152fc83a98 100644
--- a/lib/gitlab/ci/status/core.rb
+++ b/lib/gitlab/ci/status/core.rb
@@ -61,6 +61,10 @@ def action_title
         def tooltip
           label
         end
+
+        def ci_badge
+          subject.status
+        end
       end
     end
   end
diff --git a/lib/gitlab/ci/status/success_warning.rb b/lib/gitlab/ci/status/success_warning.rb
index 61af3526d21..32b4cf43e48 100644
--- a/lib/gitlab/ci/status/success_warning.rb
+++ b/lib/gitlab/ci/status/success_warning.rb
@@ -22,10 +22,6 @@ def group
           'success_with_warnings'
         end
 
-        def tooltip
-          label
-        end
-
         def self.matches?(subject, user)
           subject.success? && subject.has_warnings?
         end
diff --git a/spec/javascripts/pipelines/graph/job_component_spec.js b/spec/javascripts/pipelines/graph/job_component_spec.js
index ce181a1e515..c9677ae209a 100644
--- a/spec/javascripts/pipelines/graph/job_component_spec.js
+++ b/spec/javascripts/pipelines/graph/job_component_spec.js
@@ -13,6 +13,7 @@ describe('pipeline graph job component', () => {
       icon: 'icon_status_success',
       text: 'passed',
       label: 'passed',
+      tooltip: 'passed',
       group: 'success',
       details_path: '/root/ci-mock/builds/4256',
       has_details: true,
@@ -137,6 +138,7 @@ describe('pipeline graph job component', () => {
           status: {
             icon: 'icon_status_success',
             label: 'success',
+            tooltip: 'success',
           },
         },
       });
diff --git a/spec/lib/gitlab/ci/status/build/action_spec.rb b/spec/lib/gitlab/ci/status/build/action_spec.rb
index d612d29e3e0..b2cbc224ccd 100644
--- a/spec/lib/gitlab/ci/status/build/action_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/action_spec.rb
@@ -53,4 +53,14 @@
       end
     end
   end
+
+  describe '#ci_badge' do
+    let(:user) { create(:user) }
+    let(:build) { create(:ci_build, :non_playable) }
+    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+    it 'returns the status' do
+      expect(subject.ci_badge).to eq('created')
+    end
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
index 9cdebaa5cf2..9c6d6da0c0d 100644
--- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
@@ -40,6 +40,24 @@
     end
   end
 
+  describe '#tooltip' do
+    it 'does not override status tooltip' do
+      expect(status).to receive(:tooltip)
+
+      subject.tooltip
+    end
+  end
+
+  describe '#ci_badge' do
+    let(:user) { create(:user) }
+    let(:build) { create(:ci_build) }
+    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+    it 'returns the status' do
+      expect(subject.ci_badge).to eq('pending')
+    end
+  end
+
   describe 'action details' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
diff --git a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
index 99a5a7e4aca..a6c9b7e878a 100644
--- a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
@@ -16,6 +16,26 @@
     end
   end
 
+  describe '#ci_badge' do
+    let(:user) { create(:user) }
+    let(:build) { create(:ci_build, :failed, :allowed_to_fail) }
+    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+    it 'does override ci_badge' do
+      expect(subject.ci_badge).to eq 'failed <br> (unknown failure)'
+    end
+  end
+
+  describe '#tooltip' do
+    let(:user) { create(:user) }
+    let(:build) { create(:ci_build, :failed, :allowed_to_fail) }
+    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+    it 'does override tooltip' do
+      expect(subject.tooltip).to eq 'failed <br> (unknown failure)'
+    end
+  end
+
   describe '#icon' do
     it 'returns a warning icon' do
       expect(subject.icon).to eq 'status_warning'
diff --git a/spec/lib/gitlab/ci/status/build/failed_retried_spec.rb b/spec/lib/gitlab/ci/status/build/failed_retried_spec.rb
new file mode 100644
index 00000000000..80b2d691641
--- /dev/null
+++ b/spec/lib/gitlab/ci/status/build/failed_retried_spec.rb
@@ -0,0 +1,93 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Status::Build::FailedRetried do
+  let(:build) { create(:ci_build, :script_failure, :retried, name: 'test') }
+  let(:status) { double('core status') }
+  let(:user) { double('user') }
+
+  subject { described_class.new(status) }
+
+  describe '#text' do
+    it 'does not override status text' do
+      expect(status).to receive(:text)
+
+      subject.text
+    end
+  end
+
+  describe '#icon' do
+    it 'does not override status icon' do
+      expect(status).to receive(:icon)
+
+      subject.icon
+    end
+  end
+
+  describe '#group' do
+    it 'does not override status group' do
+      expect(status).to receive(:group)
+
+      subject.group
+    end
+  end
+
+  describe '#favicon' do
+    it 'does not override status label' do
+      expect(status).to receive(:favicon)
+
+      subject.favicon
+    end
+  end
+
+  describe '#label' do
+    it 'does not override label' do
+      expect(status).to receive(:label)
+
+      subject.label
+    end
+  end
+
+  describe '#tooltip' do
+    let(:user) { create(:user) }
+    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
+
+    it 'does override tooltip' do
+      expect(subject.tooltip).to eq 'failed (retried) <br> (script failure)'
+    end
+  end
+
+  describe '#ci_badge' do
+    let(:user) { create(:user) }
+    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
+
+    it 'returns failed status with failure reason' do
+      expect(subject.ci_badge).to eq 'failed <br> (script failure)'
+    end
+  end
+
+  describe '.matches?' do
+    subject { described_class.matches?(build, user) }
+
+    context 'with a failed and retried build' do
+      it { is_expected.to be_truthy }
+    end
+
+    context 'with an allow to fail build' do
+      let(:build) { create(:ci_build, :allowed_to_fail) }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'with a failed build' do
+      let(:build) { create(:ci_build, :failed) }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'with any other type of build' do
+      let(:build) { create(:ci_build, :success) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
index 7ae15ba2893..3f3abccc1f4 100644
--- a/spec/lib/gitlab/ci/status/build/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -47,11 +47,20 @@
     end
   end
 
+  describe '#ci_badge' do
+    let(:user) { create(:user) }
+    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
+
+    it 'does override ci_badge' do
+      expect(subject.ci_badge).to eq 'failed <br> (script failure)'
+    end
+  end
+
   describe '#tooltip' do
     let(:user) { create(:user) }
     let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
 
-    it 'does override label' do
+    it 'does override tooltip' do
       expect(subject.tooltip).to eq 'failed <br> (script failure)'
     end
   end
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index 81d5f553fd1..0ef5a99090c 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -14,6 +14,20 @@
     end
   end
 
+  describe '#tooltip' do
+    it 'does not override status tooltip' do
+      expect(status).to receive(:tooltip)
+
+      subject.tooltip
+    end
+  end
+
+  describe '#ci_badge' do
+    it 'returns status' do
+      expect(status.ci_badge).to eq('manual')
+    end
+  end
+
   describe '#has_action?' do
     context 'when user is allowed to update build' do
       context 'when user is allowed to trigger protected action' do
diff --git a/spec/lib/gitlab/ci/status/build/retried_spec.rb b/spec/lib/gitlab/ci/status/build/retried_spec.rb
new file mode 100644
index 00000000000..8a9657e1f3e
--- /dev/null
+++ b/spec/lib/gitlab/ci/status/build/retried_spec.rb
@@ -0,0 +1,83 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Status::Build::Retried do
+  let(:build) { create(:ci_build, :retried) }
+  let(:status) { double('core status') }
+  let(:user) { double('user') }
+
+  subject { described_class.new(status) }
+
+  describe '#text' do
+    it 'does not override status text' do
+      expect(status).to receive(:text)
+
+      subject.text
+    end
+  end
+
+  describe '#icon' do
+    it 'does not override status icon' do
+      expect(status).to receive(:icon)
+
+      subject.icon
+    end
+  end
+
+  describe '#group' do
+    it 'does not override status group' do
+      expect(status).to receive(:group)
+
+      subject.group
+    end
+  end
+
+  describe '#favicon' do
+    it 'does not override status label' do
+      expect(status).to receive(:favicon)
+
+      subject.favicon
+    end
+  end
+
+  describe '#label' do
+    it 'does not override status label' do
+      expect(status).to receive(:label)
+
+      subject.label
+    end
+  end
+
+  describe '#ci_badge' do
+    let(:user) { create(:user) }
+    let(:build) { create(:ci_build, :retried) }
+    let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
+
+    it 'returns status' do
+      expect(status.ci_badge).to eq('pending')
+    end
+  end
+
+  describe '#tooltip' do
+    let(:user) { create(:user) }
+    let(:build) { create(:ci_build, :retried) }
+    let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
+
+    it 'does override tooltip' do
+      expect(subject.tooltip).to eq 'passed (retried)'
+    end
+  end
+
+  describe '.matches?' do
+    subject { described_class.matches?(build, user) }
+
+    context 'with a retried build' do
+      it { is_expected.to be_truthy }
+    end
+
+    context 'with a failed retried build' do
+      let(:build) { create(:ci_build, :failed, :retried) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
index 14d42e0d70f..c9c0176b7be 100644
--- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
@@ -40,6 +40,24 @@
     end
   end
 
+  describe '#tooltip' do
+    it 'does not override status tooltip' do
+      expect(status).to receive(:tooltip)
+
+      subject.tooltip
+    end
+  end
+
+  describe '#ci_badge' do
+    let(:user) { create(:user) }
+    let(:build) { create(:ci_build) }
+    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+    it 'does returns status' do
+      expect(status.ci_badge).to eq('pending')
+    end
+  end
+
   describe 'action details' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb
index 18e250772f0..f1db4304e07 100644
--- a/spec/lib/gitlab/ci/status/build/stop_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb
@@ -77,4 +77,22 @@
       end
     end
   end
+
+  describe '#tooltip' do
+    it 'does not override status tooltip' do
+      expect(status).to receive(:tooltip)
+
+      subject.tooltip
+    end
+  end
+
+  describe '#ci_badge' do
+    let(:user) { create(:user) }
+    let(:build) { create(:ci_build, :playable) }
+    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+    it 'does not override status ci_badge' do
+      expect(status.ci_badge).to eq('manual')
+    end
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb
index f622e899d6d..dc74d7e28c5 100644
--- a/spec/lib/gitlab/ci/status/canceled_spec.rb
+++ b/spec/lib/gitlab/ci/status/canceled_spec.rb
@@ -24,8 +24,4 @@
   describe '#group' do
     it { expect(subject.group).to eq 'canceled' }
   end
-
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'canceled' }
-  end
 end
diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb
index 9dc53abebab..ce4333f2aca 100644
--- a/spec/lib/gitlab/ci/status/created_spec.rb
+++ b/spec/lib/gitlab/ci/status/created_spec.rb
@@ -24,8 +24,4 @@
   describe '#group' do
     it { expect(subject.group).to eq 'created' }
   end
-
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'created' }
-  end
 end
diff --git a/spec/lib/gitlab/ci/status/failed_spec.rb b/spec/lib/gitlab/ci/status/failed_spec.rb
index 754a30809f2..a4a92117c7f 100644
--- a/spec/lib/gitlab/ci/status/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/failed_spec.rb
@@ -24,8 +24,4 @@
   describe '#group' do
     it { expect(subject.group).to eq 'failed' }
   end
-
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'failed' }
-  end
 end
diff --git a/spec/lib/gitlab/ci/status/manual_spec.rb b/spec/lib/gitlab/ci/status/manual_spec.rb
index af821f51339..0463f2e1aff 100644
--- a/spec/lib/gitlab/ci/status/manual_spec.rb
+++ b/spec/lib/gitlab/ci/status/manual_spec.rb
@@ -24,8 +24,4 @@
   describe '#group' do
     it { expect(subject.group).to eq 'manual' }
   end
-
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'manual action' }
-  end
 end
diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb
index 99e26aaec41..0e25358dd8a 100644
--- a/spec/lib/gitlab/ci/status/pending_spec.rb
+++ b/spec/lib/gitlab/ci/status/pending_spec.rb
@@ -24,8 +24,4 @@
   describe '#group' do
     it { expect(subject.group).to eq 'pending' }
   end
-
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'pending' }
-  end
 end
diff --git a/spec/lib/gitlab/ci/status/running_spec.rb b/spec/lib/gitlab/ci/status/running_spec.rb
index b7b61743abf..9c9d431bb5d 100644
--- a/spec/lib/gitlab/ci/status/running_spec.rb
+++ b/spec/lib/gitlab/ci/status/running_spec.rb
@@ -24,8 +24,4 @@
   describe '#group' do
     it { expect(subject.group).to eq 'running' }
   end
-
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'running' }
-  end
 end
diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb
index 46165a80294..63694ca0ea6 100644
--- a/spec/lib/gitlab/ci/status/skipped_spec.rb
+++ b/spec/lib/gitlab/ci/status/skipped_spec.rb
@@ -24,8 +24,4 @@
   describe '#group' do
     it { expect(subject.group).to eq 'skipped' }
   end
-
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'skipped' }
-  end
 end
diff --git a/spec/lib/gitlab/ci/status/success_spec.rb b/spec/lib/gitlab/ci/status/success_spec.rb
index 69caa990455..2f67df71c4f 100644
--- a/spec/lib/gitlab/ci/status/success_spec.rb
+++ b/spec/lib/gitlab/ci/status/success_spec.rb
@@ -24,8 +24,4 @@
   describe '#group' do
     it { expect(subject.group).to eq 'success' }
   end
-
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'passed' }
-  end
 end
diff --git a/spec/lib/gitlab/ci/status/success_warning_spec.rb b/spec/lib/gitlab/ci/status/success_warning_spec.rb
index 05c165640d0..6d05545d1d8 100644
--- a/spec/lib/gitlab/ci/status/success_warning_spec.rb
+++ b/spec/lib/gitlab/ci/status/success_warning_spec.rb
@@ -1,8 +1,10 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Status::SuccessWarning do
+  let(:status) { double('status') }
+
   subject do
-    described_class.new(double('status'))
+    described_class.new(status)
   end
 
   describe '#test' do
@@ -21,10 +23,6 @@
     it { expect(subject.group).to eq 'success_with_warnings' }
   end
 
-  describe '#tooltip' do
-    it { expect(subject.tooltip).to eq 'passed with warnings' }
-  end
-
   describe '.matches?' do
     let(:matchable) { double('matchable') }
 
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index da44835050f..761b6854c93 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -72,7 +72,7 @@
       end
     end
 
-    context 'when build has a failure' do
+    context 'when build failed' do
       let(:build) { create(:ci_build, :failed, pipeline: pipeline) }
 
       it 'returns the reason of failure' do
@@ -82,13 +82,24 @@
       end
     end
 
-    context 'when build is allowed to failed' do
-      let(:build) { create(:ci_build, :allowed_to_fail, :script_failure, pipeline: pipeline) }
+    context 'when build has failed && retried' do
+      let(:build) { create(:ci_build, :failed, :retried, pipeline: pipeline) }
+
+      it 'does not include retried title' do
+        status_title = presenter.status_title
+
+        expect(status_title).not_to include('(retried)')
+        expect(status_title).to eq('Failed <br> (unknown failure)')
+      end
+    end
+
+    context 'when build has failed and is allow to' do
+      let(:build) { create(:ci_build, :failed, :allowed_to_fail, pipeline: pipeline) }
 
       it 'returns the reason of failure' do
         status_title = presenter.status_title
 
-        expect(status_title).to eq('Failed <br> (script failure)')
+        expect(status_title).to eq('Failed <br> (unknown failure)')
       end
     end
 
@@ -156,7 +167,7 @@
   end
 
   describe '#tooltip_message' do
-    context 'For failed builds' do
+    context 'When build has failed' do
       let(:build) { create(:ci_build, :script_failure, pipeline: pipeline) }
 
       it 'returns the reason of failure' do
@@ -166,7 +177,7 @@
       end
     end
 
-    context 'For failed retried build' do
+    context 'When build has failed and retried' do
       let(:build) { create(:ci_build, :script_failure, :retried, pipeline: pipeline) }
 
       it 'should include the reason of failure and the retried title' do
@@ -176,6 +187,16 @@
       end
     end
 
+    context 'When build has failed and is allow to' do
+      let(:build) { create(:ci_build, :script_failure, :allowed_to_fail, pipeline: pipeline) }
+
+      it 'should include the reason of failure' do
+        tooltip = subject.tooltip_message
+
+        expect(tooltip).to eq("#{build.name} - failed <br> (script failure)")
+      end
+    end
+
     context 'For any other build (no retried)' do
       let(:build) { create(:ci_build, :success, pipeline: pipeline) }
 
-- 
GitLab


From f9729d0244d503690bdd3f883058dd36dcff0ee6 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Tue, 3 Apr 2018 11:44:38 -0500
Subject: [PATCH 19/21] Remove failed retried build status class

This makes the tooltip looks misaligned,
but instead give us a more flexible backend, without duplication
---
 app/models/concerns/presentable.rb            |  6 +-
 app/presenters/ci/build_presenter.rb          |  6 +-
 lib/gitlab/ci/status/build/factory.rb         |  1 -
 lib/gitlab/ci/status/build/failed.rb          | 16 +++-
 lib/gitlab/ci/status/build/failed_allowed.rb  |  2 +-
 lib/gitlab/ci/status/build/failed_retried.rb  | 25 -----
 lib/gitlab/ci/status/core.rb                  |  4 +-
 .../projects/jobs/user_browses_job_spec.rb    |  2 +-
 .../lib/gitlab/ci/status/build/action_spec.rb |  4 +-
 .../gitlab/ci/status/build/cancelable_spec.rb |  4 +-
 .../ci/status/build/failed_allowed_spec.rb    |  6 +-
 .../ci/status/build/failed_retried_spec.rb    | 93 -------------------
 .../lib/gitlab/ci/status/build/failed_spec.rb | 20 +++-
 spec/lib/gitlab/ci/status/build/play_spec.rb  |  8 +-
 .../gitlab/ci/status/build/retried_spec.rb    |  4 +-
 .../gitlab/ci/status/build/retryable_spec.rb  |  6 +-
 spec/lib/gitlab/ci/status/build/stop_spec.rb  |  8 +-
 spec/presenters/ci/build_presenter_spec.rb    |  6 +-
 18 files changed, 63 insertions(+), 158 deletions(-)
 delete mode 100644 lib/gitlab/ci/status/build/failed_retried.rb
 delete mode 100644 spec/lib/gitlab/ci/status/build/failed_retried_spec.rb

diff --git a/app/models/concerns/presentable.rb b/app/models/concerns/presentable.rb
index 9dd2cddb139..bc4fbd19a02 100644
--- a/app/models/concerns/presentable.rb
+++ b/app/models/concerns/presentable.rb
@@ -1,9 +1,7 @@
 module Presentable
-  def self.included(klass)
-    klass.extend(ClassMethods)
-  end
+  extend ActiveSupport::Concern
 
-  module ClassMethods
+  class_methods do
     def present(attributes)
       all.map { |klass_object| klass_object.present(attributes) }
     end
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 56cfdc9abe3..ef44d76ac2e 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -16,7 +16,7 @@ def status_title
       if auto_canceled?
         "Job is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}"
       else
-        tooltip_for_ci_badge
+        tooltip_for_badge
       end
     end
 
@@ -37,8 +37,8 @@ def tooltip_message
 
     private
 
-    def tooltip_for_ci_badge
-      detailed_status.ci_badge.capitalize
+    def tooltip_for_badge
+      detailed_status.badge.capitalize
     end
 
     def detailed_status
diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb
index 6caba98efb4..8e48ad1f2f4 100644
--- a/lib/gitlab/ci/status/build/factory.rb
+++ b/lib/gitlab/ci/status/build/factory.rb
@@ -11,7 +11,6 @@ def self.extended_statuses
               Status::Build::Stop],
              [Status::Build::Action],
              [Status::Build::Failed],
-             [Status::Build::FailedRetried],
              [Status::Build::Retried]]
           end
 
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
index a5eb961678c..c5e6daecb7b 100644
--- a/lib/gitlab/ci/status/build/failed.rb
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -6,15 +6,25 @@ class Failed < Status::Extended
           include Gitlab::Ci::Status::Build::FailureData
 
           def tooltip
-            "#{failed_title} #{description}"
+            if subject.retried?
+              "#{base_message} (retried)"
+            else
+              base_message
+            end
           end
 
           def self.matches?(build, user)
             build.failed? && !build.allow_failure?
           end
 
-          def ci_badge
-            tooltip
+          def badge
+            base_message
+          end
+
+          private
+
+          def base_message
+            "#{failed_title} #{description}"
           end
         end
       end
diff --git a/lib/gitlab/ci/status/build/failed_allowed.rb b/lib/gitlab/ci/status/build/failed_allowed.rb
index dfd5d4a0f27..1f25bd45168 100644
--- a/lib/gitlab/ci/status/build/failed_allowed.rb
+++ b/lib/gitlab/ci/status/build/failed_allowed.rb
@@ -21,7 +21,7 @@ def tooltip
             "#{failed_title} #{description}"
           end
 
-          def ci_badge
+          def badge
             tooltip
           end
 
diff --git a/lib/gitlab/ci/status/build/failed_retried.rb b/lib/gitlab/ci/status/build/failed_retried.rb
deleted file mode 100644
index 5afc2ab77ea..00000000000
--- a/lib/gitlab/ci/status/build/failed_retried.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-module Gitlab
-  module Ci
-    module Status
-      module Build
-        class FailedRetried < Status::Extended
-          include Gitlab::Ci::Status::Build::FailureData
-
-          def tooltip
-            "#{failed_title} (retried) #{description}"
-          end
-
-          def ci_badge
-            "#{failed_title} #{description}"
-          end
-
-          def self.matches?(build, user)
-            build.failed? &&
-              build.retried? &&
-              !build.allow_failure?
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb
index e152fc83a98..e0b1710dda6 100644
--- a/lib/gitlab/ci/status/core.rb
+++ b/lib/gitlab/ci/status/core.rb
@@ -58,11 +58,13 @@ def action_title
           raise NotImplementedError
         end
 
+        # Hint that appears on pipeline graph tooltips and builds on the right sidebar in Job detail view
         def tooltip
           label
         end
 
-        def ci_badge
+        # Hint that appears on the build badges
+        def badge
           subject.status
         end
       end
diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb
index 126746f2f7c..b7eee39052a 100644
--- a/spec/features/projects/jobs/user_browses_job_spec.rb
+++ b/spec/features/projects/jobs/user_browses_job_spec.rb
@@ -52,7 +52,7 @@
     it 'displays the failure reason and retried label' do
       within('.builds-container') do
         build_link = first('.build-job > a')
-        expect(build_link['data-title']).to eq('test - failed (retried) <br> (unknown failure)')
+        expect(build_link['data-title']).to eq('test - failed <br> (unknown failure) (retried)')
       end
     end
   end
diff --git a/spec/lib/gitlab/ci/status/build/action_spec.rb b/spec/lib/gitlab/ci/status/build/action_spec.rb
index b2cbc224ccd..be7b8508439 100644
--- a/spec/lib/gitlab/ci/status/build/action_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/action_spec.rb
@@ -54,13 +54,13 @@
     end
   end
 
-  describe '#ci_badge' do
+  describe '#badge' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build, :non_playable) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
     it 'returns the status' do
-      expect(subject.ci_badge).to eq('created')
+      expect(subject.badge).to eq('created')
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
index 9c6d6da0c0d..0bb6cfd996b 100644
--- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
@@ -48,13 +48,13 @@
     end
   end
 
-  describe '#ci_badge' do
+  describe '#badge' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
     it 'returns the status' do
-      expect(subject.ci_badge).to eq('pending')
+      expect(subject.badge).to eq('pending')
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
index a6c9b7e878a..31fd4cefff9 100644
--- a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
@@ -16,13 +16,13 @@
     end
   end
 
-  describe '#ci_badge' do
+  describe '#badge' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build, :failed, :allowed_to_fail) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
-    it 'does override ci_badge' do
-      expect(subject.ci_badge).to eq 'failed <br> (unknown failure)'
+    it 'does override badge' do
+      expect(subject.badge).to eq 'failed <br> (unknown failure)'
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/failed_retried_spec.rb b/spec/lib/gitlab/ci/status/build/failed_retried_spec.rb
deleted file mode 100644
index 80b2d691641..00000000000
--- a/spec/lib/gitlab/ci/status/build/failed_retried_spec.rb
+++ /dev/null
@@ -1,93 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Ci::Status::Build::FailedRetried do
-  let(:build) { create(:ci_build, :script_failure, :retried, name: 'test') }
-  let(:status) { double('core status') }
-  let(:user) { double('user') }
-
-  subject { described_class.new(status) }
-
-  describe '#text' do
-    it 'does not override status text' do
-      expect(status).to receive(:text)
-
-      subject.text
-    end
-  end
-
-  describe '#icon' do
-    it 'does not override status icon' do
-      expect(status).to receive(:icon)
-
-      subject.icon
-    end
-  end
-
-  describe '#group' do
-    it 'does not override status group' do
-      expect(status).to receive(:group)
-
-      subject.group
-    end
-  end
-
-  describe '#favicon' do
-    it 'does not override status label' do
-      expect(status).to receive(:favicon)
-
-      subject.favicon
-    end
-  end
-
-  describe '#label' do
-    it 'does not override label' do
-      expect(status).to receive(:label)
-
-      subject.label
-    end
-  end
-
-  describe '#tooltip' do
-    let(:user) { create(:user) }
-    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
-
-    it 'does override tooltip' do
-      expect(subject.tooltip).to eq 'failed (retried) <br> (script failure)'
-    end
-  end
-
-  describe '#ci_badge' do
-    let(:user) { create(:user) }
-    let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
-
-    it 'returns failed status with failure reason' do
-      expect(subject.ci_badge).to eq 'failed <br> (script failure)'
-    end
-  end
-
-  describe '.matches?' do
-    subject { described_class.matches?(build, user) }
-
-    context 'with a failed and retried build' do
-      it { is_expected.to be_truthy }
-    end
-
-    context 'with an allow to fail build' do
-      let(:build) { create(:ci_build, :allowed_to_fail) }
-
-      it { is_expected.to be_falsy }
-    end
-
-    context 'with a failed build' do
-      let(:build) { create(:ci_build, :failed) }
-
-      it { is_expected.to be_falsy }
-    end
-
-    context 'with any other type of build' do
-      let(:build) { create(:ci_build, :success) }
-
-      it { is_expected.to be_falsy }
-    end
-  end
-end
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
index 3f3abccc1f4..55c22e7fb33 100644
--- a/spec/lib/gitlab/ci/status/build/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -47,12 +47,12 @@
     end
   end
 
-  describe '#ci_badge' do
+  describe '#badge' do
     let(:user) { create(:user) }
     let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
 
-    it 'does override ci_badge' do
-      expect(subject.ci_badge).to eq 'failed <br> (script failure)'
+    it 'does override badge' do
+      expect(subject.badge).to eq 'failed <br> (script failure)'
     end
   end
 
@@ -60,8 +60,18 @@
     let(:user) { create(:user) }
     let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
 
-    it 'does override tooltip' do
-      expect(subject.tooltip).to eq 'failed <br> (script failure)'
+    context 'when the build has not been retried' do
+      it 'does override tooltip' do
+        expect(subject.tooltip).to eq 'failed <br> (script failure)'
+      end
+    end
+
+    context 'when the build has been retried' do
+      let(:build) { create(:ci_build, :failed, :retried) }
+
+      it 'does override tooltip' do
+        expect(subject.tooltip).to eq 'failed <br> (unknown failure) (retried)'
+      end
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index 0ef5a99090c..36dccb137cf 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -22,9 +22,11 @@
     end
   end
 
-  describe '#ci_badge' do
-    it 'returns status' do
-      expect(status.ci_badge).to eq('manual')
+  describe '#badge' do
+    it 'does not override status badge' do
+      expect(status).to receive(:badge)
+
+      subject.badge
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/retried_spec.rb b/spec/lib/gitlab/ci/status/build/retried_spec.rb
index 8a9657e1f3e..c94bfd135b6 100644
--- a/spec/lib/gitlab/ci/status/build/retried_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/retried_spec.rb
@@ -47,13 +47,13 @@
     end
   end
 
-  describe '#ci_badge' do
+  describe '#badge' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build, :retried) }
     let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
 
     it 'returns status' do
-      expect(status.ci_badge).to eq('pending')
+      expect(status.badge).to eq('pending')
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
index c9c0176b7be..e3f2e853f78 100644
--- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
@@ -48,13 +48,13 @@
     end
   end
 
-  describe '#ci_badge' do
+  describe '#badge' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
-    it 'does returns status' do
-      expect(status.ci_badge).to eq('pending')
+    it 'does return status' do
+      expect(status.badge).to eq('pending')
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb
index f1db4304e07..d2675c922a4 100644
--- a/spec/lib/gitlab/ci/status/build/stop_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb
@@ -86,13 +86,15 @@
     end
   end
 
-  describe '#ci_badge' do
+  describe '#badge' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build, :playable) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
-    it 'does not override status ci_badge' do
-      expect(status.ci_badge).to eq('manual')
+    it 'does not override status badge' do
+      expect(status).to receive(:badge)
+
+      subject.badge
     end
   end
 end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index 761b6854c93..782d2d7b82e 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -93,7 +93,7 @@
       end
     end
 
-    context 'when build has failed and is allow to' do
+    context 'when build has failed and is allowed to' do
       let(:build) { create(:ci_build, :failed, :allowed_to_fail, pipeline: pipeline) }
 
       it 'returns the reason of failure' do
@@ -183,11 +183,11 @@
       it 'should include the reason of failure and the retried title' do
         tooltip = subject.tooltip_message
 
-        expect(tooltip).to eq("#{build.name} - failed (retried) <br> (script failure)")
+        expect(tooltip).to eq("#{build.name} - failed <br> (script failure) (retried)")
       end
     end
 
-    context 'When build has failed and is allow to' do
+    context 'When build has failed and is allowed to' do
       let(:build) { create(:ci_build, :script_failure, :allowed_to_fail, pipeline: pipeline) }
 
       it 'should include the reason of failure' do
-- 
GitLab


From 0bc5f2c03fddd6d25a09328361cc91440b68f9db Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Wed, 4 Apr 2018 12:30:03 -0500
Subject: [PATCH 20/21] Addresses backend review

- Removes FailureData as we're using stack extended statuses
- Make retried to be failure agnostic so it can be decorated with
  Failure and then with Retried
---
 lib/gitlab/ci/status/build/factory.rb         |  2 +-
 lib/gitlab/ci/status/build/failed.rb          | 29 ++++++++-----
 lib/gitlab/ci/status/build/failed_allowed.rb  | 16 +++----
 lib/gitlab/ci/status/build/failure_data.rb    | 28 ------------
 lib/gitlab/ci/status/build/retried.rb         |  4 +-
 .../gitlab/ci/status/build/factory_spec.rb    |  1 +
 .../ci/status/build/failed_allowed_spec.rb    | 43 ++++++++++---------
 .../lib/gitlab/ci/status/build/failed_spec.rb | 14 +-----
 .../gitlab/ci/status/build/retried_spec.rb    | 25 ++++++++---
 spec/presenters/ci/build_presenter_spec.rb    |  2 +-
 10 files changed, 75 insertions(+), 89 deletions(-)
 delete mode 100644 lib/gitlab/ci/status/build/failure_data.rb

diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb
index 8e48ad1f2f4..20a319caf86 100644
--- a/lib/gitlab/ci/status/build/factory.rb
+++ b/lib/gitlab/ci/status/build/factory.rb
@@ -6,11 +6,11 @@ class Factory < Status::Factory
           def self.extended_statuses
             [[Status::Build::Cancelable,
               Status::Build::Retryable],
+             [Status::Build::Failed],
              [Status::Build::FailedAllowed,
               Status::Build::Play,
               Status::Build::Stop],
              [Status::Build::Action],
-             [Status::Build::Failed],
              [Status::Build::Retried]]
           end
 
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
index c5e6daecb7b..f14d0989925 100644
--- a/lib/gitlab/ci/status/build/failed.rb
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -3,28 +3,35 @@ module Ci
     module Status
       module Build
         class Failed < Status::Extended
-          include Gitlab::Ci::Status::Build::FailureData
+          REASONS = {
+            'unknown_failure' => 'unknown failure',
+            'script_failure' => 'script failure',
+            'api_failure' => 'API failure',
+            'stuck_or_timeout_failure' => 'stuck or timeout failure',
+            'runner_system_failure' => 'runner system failure',
+            'missing_dependency_failure' => 'missing dependency failure'
+          }.freeze
 
           def tooltip
-            if subject.retried?
-              "#{base_message} (retried)"
-            else
-              base_message
-            end
-          end
-
-          def self.matches?(build, user)
-            build.failed? && !build.allow_failure?
+            base_message
           end
 
           def badge
             base_message
           end
 
+          def self.matches?(build, user)
+            build.failed?
+          end
+
           private
 
           def base_message
-            "#{failed_title} #{description}"
+            "#{s_('CiStatusLabel|failed')} #{description}"
+          end
+
+          def description
+            "<br> (#{REASONS[subject.failure_reason]})"
           end
         end
       end
diff --git a/lib/gitlab/ci/status/build/failed_allowed.rb b/lib/gitlab/ci/status/build/failed_allowed.rb
index 1f25bd45168..ac8a04836f4 100644
--- a/lib/gitlab/ci/status/build/failed_allowed.rb
+++ b/lib/gitlab/ci/status/build/failed_allowed.rb
@@ -3,10 +3,8 @@ module Ci
     module Status
       module Build
         class FailedAllowed < Status::Extended
-          include Gitlab::Ci::Status::Build::FailureData
-
           def label
-            'failed (allowed to fail)'
+            "failed #{allowed_to_fail_title}"
           end
 
           def icon
@@ -18,16 +16,18 @@ def group
           end
 
           def tooltip
-            "#{failed_title} #{description}"
-          end
-
-          def badge
-            tooltip
+            "#{@status.tooltip} #{allowed_to_fail_title}"
           end
 
           def self.matches?(build, user)
             build.failed? && build.allow_failure?
           end
+
+          private
+
+          def allowed_to_fail_title
+            "(allowed to fail)"
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/status/build/failure_data.rb b/lib/gitlab/ci/status/build/failure_data.rb
deleted file mode 100644
index d03d0bb741c..00000000000
--- a/lib/gitlab/ci/status/build/failure_data.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-module Gitlab
-  module Ci
-    module Status
-      module Build
-        module FailureData
-          REASONS = {
-            'unknown_failure' => 'unknown failure',
-            'script_failure' => 'script failure',
-            'api_failure' => 'API failure',
-            'stuck_or_timeout_failure' => 'stuck or timeout failure',
-            'runner_system_failure' => 'runner system failure',
-            'missing_dependency_failure' => 'missing dependency failure'
-          }.freeze
-
-          private
-
-          def failed_title
-            s_('CiStatusLabel|failed')
-          end
-
-          def description
-            "<br> (#{REASONS[subject.failure_reason]})"
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/ci/status/build/retried.rb b/lib/gitlab/ci/status/build/retried.rb
index 9e3b1338452..c80fa848058 100644
--- a/lib/gitlab/ci/status/build/retried.rb
+++ b/lib/gitlab/ci/status/build/retried.rb
@@ -4,11 +4,11 @@ module Status
       module Build
         class Retried < Status::Extended
           def tooltip
-            "#{label} (retried)"
+            @status.tooltip + " (retried)"
           end
 
           def self.matches?(build, user)
-            build.retried? && !build.failed?
+            build.retried?
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index 1e74ac3fa09..8fd8d5e4035 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -76,6 +76,7 @@
       it 'matches correct extended statuses' do
         expect(factory.extended_statuses)
           .to eq [Gitlab::Ci::Status::Build::Retryable,
+                  Gitlab::Ci::Status::Build::Failed,
                   Gitlab::Ci::Status::Build::FailedAllowed]
       end
 
diff --git a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
index 31fd4cefff9..35dc2d02c1a 100644
--- a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
@@ -3,6 +3,7 @@
 describe Gitlab::Ci::Status::Build::FailedAllowed do
   let(:status) { double('core status') }
   let(:user) { double('user') }
+  let(:build) { create(:ci_build, :failed, :allowed_to_fail) }
 
   subject do
     described_class.new(status)
@@ -16,26 +17,6 @@
     end
   end
 
-  describe '#badge' do
-    let(:user) { create(:user) }
-    let(:build) { create(:ci_build, :failed, :allowed_to_fail) }
-    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
-
-    it 'does override badge' do
-      expect(subject.badge).to eq 'failed <br> (unknown failure)'
-    end
-  end
-
-  describe '#tooltip' do
-    let(:user) { create(:user) }
-    let(:build) { create(:ci_build, :failed, :allowed_to_fail) }
-    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
-
-    it 'does override tooltip' do
-      expect(subject.tooltip).to eq 'failed <br> (unknown failure)'
-    end
-  end
-
   describe '#icon' do
     it 'returns a warning icon' do
       expect(subject.icon).to eq 'status_warning'
@@ -88,6 +69,28 @@
     end
   end
 
+  describe '#badge' do
+    let(:user) { create(:user) }
+    let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
+    let(:build_status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
+    let(:status) { described_class.new(build_status) }
+
+    it 'does override badge' do
+      expect(status.badge).to eq('failed <br> (unknown failure)')
+    end
+  end
+
+  describe '#tooltip' do
+    let(:user) { create(:user) }
+    let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
+    let(:build_status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
+    let(:status) { described_class.new(build_status) }
+
+    it 'does override tooltip' do
+      expect(status.tooltip).to eq 'failed <br> (unknown failure) (allowed to fail)'
+    end
+  end
+
   describe '.matches?' do
     subject { described_class.matches?(build, user) }
 
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
index 55c22e7fb33..142fa5d6c90 100644
--- a/spec/lib/gitlab/ci/status/build/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -60,18 +60,8 @@
     let(:user) { create(:user) }
     let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
 
-    context 'when the build has not been retried' do
-      it 'does override tooltip' do
-        expect(subject.tooltip).to eq 'failed <br> (script failure)'
-      end
-    end
-
-    context 'when the build has been retried' do
-      let(:build) { create(:ci_build, :failed, :retried) }
-
-      it 'does override tooltip' do
-        expect(subject.tooltip).to eq 'failed <br> (unknown failure) (retried)'
-      end
+    it 'does override tooltip' do
+      expect(subject.tooltip).to eq 'failed <br> (script failure)'
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/retried_spec.rb b/spec/lib/gitlab/ci/status/build/retried_spec.rb
index c94bfd135b6..7bbb7fa7d93 100644
--- a/spec/lib/gitlab/ci/status/build/retried_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/retried_spec.rb
@@ -59,11 +59,24 @@
 
   describe '#tooltip' do
     let(:user) { create(:user) }
-    let(:build) { create(:ci_build, :retried) }
-    let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
 
-    it 'does override tooltip' do
-      expect(subject.tooltip).to eq 'passed (retried)'
+    context 'with a failed build' do
+      let(:build) { create(:ci_build, :failed, :retried) }
+      let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
+      let(:status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
+
+      it 'does override tooltip' do
+        expect(subject.tooltip).to eq 'failed <br> (unknown failure) (retried)'
+      end
+    end
+
+    context 'with another build' do
+      let(:build) { create(:ci_build, :retried) }
+      let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
+
+      it 'does override tooltip' do
+        expect(subject.tooltip).to eq 'passed (retried)'
+      end
     end
   end
 
@@ -74,8 +87,8 @@
       it { is_expected.to be_truthy }
     end
 
-    context 'with a failed retried build' do
-      let(:build) { create(:ci_build, :failed, :retried) }
+    context 'with a build that has not been retried' do
+      let(:build) { create(:ci_build, :success) }
 
       it { is_expected.to be_falsy }
     end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index 782d2d7b82e..cc16d0f156b 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -193,7 +193,7 @@
       it 'should include the reason of failure' do
         tooltip = subject.tooltip_message
 
-        expect(tooltip).to eq("#{build.name} - failed <br> (script failure)")
+        expect(tooltip).to eq("#{build.name} - failed <br> (script failure) (allowed to fail)")
       end
     end
 
-- 
GitLab


From 1ab8ca7e204da79caadb64dd4e3c8af23e379b8b Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Thu, 5 Apr 2018 08:48:54 -0500
Subject: [PATCH 21/21] Rename methods on Ci::Status::Core

From 'tooltip' to 'status_tooltip' and,
from 'badge' to 'badge_tooltip'
---
 app/presenters/ci/build_presenter.rb             |  4 ++--
 app/serializers/status_entity.rb                 |  4 ++--
 .../ci/status/_dropdown_graph_badge.html.haml    |  2 +-
 lib/gitlab/ci/status/build/failed.rb             |  4 ++--
 lib/gitlab/ci/status/build/failed_allowed.rb     |  4 ++--
 lib/gitlab/ci/status/build/retried.rb            |  4 ++--
 lib/gitlab/ci/status/core.rb                     |  6 +++---
 spec/lib/gitlab/ci/status/build/action_spec.rb   |  4 ++--
 .../gitlab/ci/status/build/cancelable_spec.rb    | 12 ++++++------
 spec/lib/gitlab/ci/status/build/factory_spec.rb  |  2 +-
 .../ci/status/build/failed_allowed_spec.rb       | 12 ++++++------
 spec/lib/gitlab/ci/status/build/failed_spec.rb   | 12 ++++++------
 spec/lib/gitlab/ci/status/build/play_spec.rb     | 16 ++++++++--------
 spec/lib/gitlab/ci/status/build/retried_spec.rb  | 14 +++++++-------
 .../lib/gitlab/ci/status/build/retryable_spec.rb | 12 ++++++------
 spec/lib/gitlab/ci/status/build/stop_spec.rb     | 16 ++++++++--------
 16 files changed, 64 insertions(+), 64 deletions(-)

diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index ef44d76ac2e..9afebda19be 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -32,13 +32,13 @@ def trigger_variables
     end
 
     def tooltip_message
-      "#{subject.name} - #{detailed_status.tooltip}"
+      "#{subject.name} - #{detailed_status.status_tooltip}"
     end
 
     private
 
     def tooltip_for_badge
-      detailed_status.badge.capitalize
+      detailed_status.badge_tooltip.capitalize
     end
 
     def detailed_status
diff --git a/app/serializers/status_entity.rb b/app/serializers/status_entity.rb
index 0157a572f28..81b461afb7a 100644
--- a/app/serializers/status_entity.rb
+++ b/app/serializers/status_entity.rb
@@ -1,8 +1,8 @@
 class StatusEntity < Grape::Entity
   include RequestAwareEntity
 
-  expose :icon, :text, :label, :group, :tooltip
-
+  expose :icon, :text, :label, :group
+  expose :status_tooltip, as: :tooltip
   expose :has_details?, as: :has_details
   expose :details_path
 
diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml
index 1245715aa03..db2040110fa 100644
--- a/app/views/ci/status/_dropdown_graph_badge.html.haml
+++ b/app/views/ci/status/_dropdown_graph_badge.html.haml
@@ -3,7 +3,7 @@
 - subject = local_assigns.fetch(:subject)
 - status = subject.detailed_status(current_user)
 - klass = "ci-status-icon ci-status-icon-#{status.group}"
-- tooltip = "#{subject.name} - #{status.tooltip}"
+- tooltip = "#{subject.name} - #{status.status_tooltip}"
 
 - if status.has_details?
   = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, html: true, container: 'body' }  do
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
index f14d0989925..155f4fc1343 100644
--- a/lib/gitlab/ci/status/build/failed.rb
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -12,11 +12,11 @@ class Failed < Status::Extended
             'missing_dependency_failure' => 'missing dependency failure'
           }.freeze
 
-          def tooltip
+          def status_tooltip
             base_message
           end
 
-          def badge
+          def badge_tooltip
             base_message
           end
 
diff --git a/lib/gitlab/ci/status/build/failed_allowed.rb b/lib/gitlab/ci/status/build/failed_allowed.rb
index ac8a04836f4..ca0046fb1f7 100644
--- a/lib/gitlab/ci/status/build/failed_allowed.rb
+++ b/lib/gitlab/ci/status/build/failed_allowed.rb
@@ -15,8 +15,8 @@ def group
             'failed_with_warnings'
           end
 
-          def tooltip
-            "#{@status.tooltip} #{allowed_to_fail_title}"
+          def status_tooltip
+            "#{@status.status_tooltip} #{allowed_to_fail_title}"
           end
 
           def self.matches?(build, user)
diff --git a/lib/gitlab/ci/status/build/retried.rb b/lib/gitlab/ci/status/build/retried.rb
index c80fa848058..6e190e4ee3c 100644
--- a/lib/gitlab/ci/status/build/retried.rb
+++ b/lib/gitlab/ci/status/build/retried.rb
@@ -3,8 +3,8 @@ module Ci
     module Status
       module Build
         class Retried < Status::Extended
-          def tooltip
-            @status.tooltip + " (retried)"
+          def status_tooltip
+            @status.status_tooltip + " (retried)"
           end
 
           def self.matches?(build, user)
diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb
index e0b1710dda6..daab6bb2de5 100644
--- a/lib/gitlab/ci/status/core.rb
+++ b/lib/gitlab/ci/status/core.rb
@@ -58,13 +58,13 @@ def action_title
           raise NotImplementedError
         end
 
-        # Hint that appears on pipeline graph tooltips and builds on the right sidebar in Job detail view
-        def tooltip
+        # Hint that appears on all the pipeline graph tooltips and builds on the right sidebar in Job detail view
+        def status_tooltip
           label
         end
 
         # Hint that appears on the build badges
-        def badge
+        def badge_tooltip
           subject.status
         end
       end
diff --git a/spec/lib/gitlab/ci/status/build/action_spec.rb b/spec/lib/gitlab/ci/status/build/action_spec.rb
index be7b8508439..bdec582b57b 100644
--- a/spec/lib/gitlab/ci/status/build/action_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/action_spec.rb
@@ -54,13 +54,13 @@
     end
   end
 
-  describe '#badge' do
+  describe '#badge_tooltip' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build, :non_playable) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
     it 'returns the status' do
-      expect(subject.badge).to eq('created')
+      expect(subject.badge_tooltip).to eq('created')
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
index 0bb6cfd996b..3ef0b6817e9 100644
--- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
@@ -40,21 +40,21 @@
     end
   end
 
-  describe '#tooltip' do
-    it 'does not override status tooltip' do
-      expect(status).to receive(:tooltip)
+  describe '#status_tooltip' do
+    it 'does not override status status_tooltip' do
+      expect(status).to receive(:status_tooltip)
 
-      subject.tooltip
+      subject.status_tooltip
     end
   end
 
-  describe '#badge' do
+  describe '#badge_tooltip' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
     it 'returns the status' do
-      expect(subject.badge).to eq('pending')
+      expect(subject.badge_tooltip).to eq('pending')
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index 8fd8d5e4035..bbfa60169a1 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -60,7 +60,7 @@
         expect(status.icon).to eq 'status_failed'
         expect(status.favicon).to eq 'favicon_status_failed'
         expect(status.label).to eq 'failed'
-        expect(status.tooltip).to eq 'failed <br> (unknown failure)'
+        expect(status.status_tooltip).to eq 'failed <br> (unknown failure)'
         expect(status).to have_details
         expect(status).to have_action
       end
diff --git a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
index 35dc2d02c1a..bfaa508785e 100644
--- a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
@@ -69,25 +69,25 @@
     end
   end
 
-  describe '#badge' do
+  describe '#badge_tooltip' do
     let(:user) { create(:user) }
     let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
     let(:build_status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
     let(:status) { described_class.new(build_status) }
 
-    it 'does override badge' do
-      expect(status.badge).to eq('failed <br> (unknown failure)')
+    it 'does override badge_tooltip' do
+      expect(status.badge_tooltip).to eq('failed <br> (unknown failure)')
     end
   end
 
-  describe '#tooltip' do
+  describe '#status_tooltip' do
     let(:user) { create(:user) }
     let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
     let(:build_status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
     let(:status) { described_class.new(build_status) }
 
-    it 'does override tooltip' do
-      expect(status.tooltip).to eq 'failed <br> (unknown failure) (allowed to fail)'
+    it 'does override status_tooltip' do
+      expect(status.status_tooltip).to eq 'failed <br> (unknown failure) (allowed to fail)'
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
index 142fa5d6c90..cadb424ea2c 100644
--- a/spec/lib/gitlab/ci/status/build/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -47,21 +47,21 @@
     end
   end
 
-  describe '#badge' do
+  describe '#badge_tooltip' do
     let(:user) { create(:user) }
     let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
 
-    it 'does override badge' do
-      expect(subject.badge).to eq 'failed <br> (script failure)'
+    it 'does override badge_tooltip' do
+      expect(subject.badge_tooltip).to eq 'failed <br> (script failure)'
     end
   end
 
-  describe '#tooltip' do
+  describe '#status_tooltip' do
     let(:user) { create(:user) }
     let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
 
-    it 'does override tooltip' do
-      expect(subject.tooltip).to eq 'failed <br> (script failure)'
+    it 'does override status_tooltip' do
+      expect(subject.status_tooltip).to eq 'failed <br> (script failure)'
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index 36dccb137cf..35e47cd2526 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -14,19 +14,19 @@
     end
   end
 
-  describe '#tooltip' do
-    it 'does not override status tooltip' do
-      expect(status).to receive(:tooltip)
+  describe '#status_tooltip' do
+    it 'does not override status status_tooltip' do
+      expect(status).to receive(:status_tooltip)
 
-      subject.tooltip
+      subject.status_tooltip
     end
   end
 
-  describe '#badge' do
-    it 'does not override status badge' do
-      expect(status).to receive(:badge)
+  describe '#badge_tooltip' do
+    it 'does not override status badge_tooltip' do
+      expect(status).to receive(:badge_tooltip)
 
-      subject.badge
+      subject.badge_tooltip
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/retried_spec.rb b/spec/lib/gitlab/ci/status/build/retried_spec.rb
index 7bbb7fa7d93..ee9acaf1c21 100644
--- a/spec/lib/gitlab/ci/status/build/retried_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/retried_spec.rb
@@ -47,17 +47,17 @@
     end
   end
 
-  describe '#badge' do
+  describe '#badge_tooltip' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build, :retried) }
     let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
 
     it 'returns status' do
-      expect(status.badge).to eq('pending')
+      expect(status.badge_tooltip).to eq('pending')
     end
   end
 
-  describe '#tooltip' do
+  describe '#status_tooltip' do
     let(:user) { create(:user) }
 
     context 'with a failed build' do
@@ -65,8 +65,8 @@
       let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
       let(:status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
 
-      it 'does override tooltip' do
-        expect(subject.tooltip).to eq 'failed <br> (unknown failure) (retried)'
+      it 'does override status_tooltip' do
+        expect(subject.status_tooltip).to eq 'failed <br> (unknown failure) (retried)'
       end
     end
 
@@ -74,8 +74,8 @@
       let(:build) { create(:ci_build, :retried) }
       let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
 
-      it 'does override tooltip' do
-        expect(subject.tooltip).to eq 'passed (retried)'
+      it 'does override status_tooltip' do
+        expect(subject.status_tooltip).to eq 'passed (retried)'
       end
     end
   end
diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
index e3f2e853f78..0c5099b7da5 100644
--- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
@@ -40,21 +40,21 @@
     end
   end
 
-  describe '#tooltip' do
-    it 'does not override status tooltip' do
-      expect(status).to receive(:tooltip)
+  describe '#status_tooltip' do
+    it 'does not override status status_tooltip' do
+      expect(status).to receive(:status_tooltip)
 
-      subject.tooltip
+      subject.status_tooltip
     end
   end
 
-  describe '#badge' do
+  describe '#badge_tooltip' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
     it 'does return status' do
-      expect(status.badge).to eq('pending')
+      expect(status.badge_tooltip).to eq('pending')
     end
   end
 
diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb
index d2675c922a4..f16fc5c9205 100644
--- a/spec/lib/gitlab/ci/status/build/stop_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb
@@ -78,23 +78,23 @@
     end
   end
 
-  describe '#tooltip' do
-    it 'does not override status tooltip' do
-      expect(status).to receive(:tooltip)
+  describe '#status_tooltip' do
+    it 'does not override status status_tooltip' do
+      expect(status).to receive(:status_tooltip)
 
-      subject.tooltip
+      subject.status_tooltip
     end
   end
 
-  describe '#badge' do
+  describe '#badge_tooltip' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build, :playable) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
-    it 'does not override status badge' do
-      expect(status).to receive(:badge)
+    it 'does not override status badge_tooltip' do
+      expect(status).to receive(:badge_tooltip)
 
-      subject.badge
+      subject.badge_tooltip
     end
   end
 end
-- 
GitLab