Merge methods between BuildPresenter and Build::Failed status44269-show-failure-reason-on-upgrade-tooltip-of-jobs

Gitlab::Ci::Status contain most of the logic, meanwhile BuildPresenter just presents it

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 @@ module CiStatusHelper
       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 @@ module Ci
       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 @@ module Ci
         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 @@ module Ci
       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 4aa29f8067e..24488089cb9 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 Gitlab
     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 @@ FactoryBot.define do
     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 Gitlab::Ci::Status::Build::Failed do
 
   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 @@ describe Gitlab::Ci::Status::Build::Failed do
       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 @@ describe Ci::BuildPresenter do
     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 @@ describe Ci::BuildPresenter do
       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 @@ describe JobEntity do
   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