From 9d6e8e995a7528b66ee5410f01730bd644118aec 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 11:22:04 +0000
Subject: [PATCH 01/22] Add JobStatusReason to Job view content section

---
 .../javascripts/jobs/components/header.vue    | 46 ++++++++++++-------
 .../jobs/components/job_status_reason.vue     | 24 ++++++++++
 spec/javascripts/jobs/header_spec.js          | 24 ++++++++--
 3 files changed, 72 insertions(+), 22 deletions(-)
 create mode 100644 app/assets/javascripts/jobs/components/job_status_reason.vue

diff --git a/app/assets/javascripts/jobs/components/header.vue b/app/assets/javascripts/jobs/components/header.vue
index 357bc9aab17..cc4d53f8209 100644
--- a/app/assets/javascripts/jobs/components/header.vue
+++ b/app/assets/javascripts/jobs/components/header.vue
@@ -1,12 +1,14 @@
 <script>
   import ciHeader from '../../vue_shared/components/header_ci_component.vue';
   import loadingIcon from '../../vue_shared/components/loading_icon.vue';
+  import jobStatusReason from './job_status_reason.vue';
 
   export default {
     name: 'JobHeaderSection',
     components: {
       ciHeader,
       loadingIcon,
+      jobStatusReason,
     },
     props: {
       job: {
@@ -30,6 +32,9 @@
       shouldRenderContent() {
         return !this.isLoading && Object.keys(this.job).length;
       },
+      shouldRenderReason() {
+        return !!(this.job.status && this.job.status.failure_description);
+      },
       /**
        * When job has not started the key will be `false`
        * When job started the key will be a string with a date.
@@ -61,22 +66,29 @@
   };
 </script>
 <template>
-  <div class="js-build-header build-header top-area">
-    <ci-header
-      v-if="shouldRenderContent"
-      :status="status"
-      item-name="Job"
-      :item-id="job.id"
-      :time="job.created_at"
-      :user="job.user"
-      :actions="actions"
-      :has-sidebar-button="true"
-      :should-render-triggered-label="jobStarted"
-    />
-    <loading-icon
-      v-if="isLoading"
-      size="2"
-      class="prepend-top-default append-bottom-default"
+  <header>
+    <div class="js-build-header build-header top-area">
+      <ci-header
+        v-if="shouldRenderContent"
+        :status="status"
+        item-name="Job"
+        :item-id="job.id"
+        :time="job.created_at"
+        :user="job.user"
+        :actions="actions"
+        :has-sidebar-button="true"
+        :should-render-triggered-label="jobStarted"
+      />
+      <loading-icon
+        v-if="isLoading"
+        size="2"
+        class="prepend-top-default append-bottom-default"
+      />
+    </div>
+
+    <job-status-reason
+      v-if="shouldRenderReason"
+      :job="job"
     />
-  </div>
+  </header>
 </template>
diff --git a/app/assets/javascripts/jobs/components/job_status_reason.vue b/app/assets/javascripts/jobs/components/job_status_reason.vue
new file mode 100644
index 00000000000..03128aa8451
--- /dev/null
+++ b/app/assets/javascripts/jobs/components/job_status_reason.vue
@@ -0,0 +1,24 @@
+<script>
+  import icon from '../../vue_shared/components/icon.vue';
+
+  export default {
+    components: {
+      icon,
+    },
+    props: {
+      job: {
+        type: Object,
+        required: true,
+      },
+    },
+  };
+</script>
+<template>
+  <div
+    class="bs-callout bs-callout-danger"
+    role="alert"
+    aria-live="assertive"
+  >
+    {{ job.status.callout_message }}
+  </div>
+</template>
diff --git a/spec/javascripts/jobs/header_spec.js b/spec/javascripts/jobs/header_spec.js
index 0961605ce5c..191760c1fcb 100644
--- a/spec/javascripts/jobs/header_spec.js
+++ b/spec/javascripts/jobs/header_spec.js
@@ -36,14 +36,28 @@ describe('Job details header', () => {
       },
       isLoading: false,
     };
-
-    vm = mountComponent(HeaderComponent, props);
   });
 
   afterEach(() => {
     vm.$destroy();
   });
 
+  describe('job reason', () => {
+    it('should not render the reason when reason is absent', () => {
+      vm = mountComponent(HeaderComponent, props);
+
+      expect(vm.shouldRenderReason).toBe(false);
+    });
+
+    it('should render the reason when reason is present', () => {
+      props.job.status.callout_message = 'There is an unknown failure, please try again';
+
+      vm = mountComponent(HeaderComponent, props);
+
+      expect(vm.shouldRenderReason).toBe(true);
+    });
+  });
+
   describe('triggered job', () => {
     beforeEach(() => {
       vm = mountComponent(HeaderComponent, props);
@@ -56,9 +70,9 @@ describe('Job details header', () => {
     });
 
     it('should render new issue link', () => {
-      expect(
-        vm.$el.querySelector('.js-new-issue').getAttribute('href'),
-      ).toEqual(props.job.new_issue_path);
+      expect(vm.$el.querySelector('.js-new-issue').getAttribute('href')).toEqual(
+        props.job.new_issue_path,
+      );
     });
   });
 
-- 
2.24.1


From c18e5b9515c1a856d0a2a8ca60c0e8cedc961ee6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Fri, 30 Mar 2018 10:43:53 +0100
Subject: [PATCH 02/22] Prettify header vue + spec files

---
 .../javascripts/jobs/components/header.vue    | 115 +++++++++---------
 spec/javascripts/jobs/header_spec.js          |  10 +-
 2 files changed, 66 insertions(+), 59 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/header.vue b/app/assets/javascripts/jobs/components/header.vue
index cc4d53f8209..2e0fa599b7e 100644
--- a/app/assets/javascripts/jobs/components/header.vue
+++ b/app/assets/javascripts/jobs/components/header.vue
@@ -1,69 +1,70 @@
 <script>
-  import ciHeader from '../../vue_shared/components/header_ci_component.vue';
-  import loadingIcon from '../../vue_shared/components/loading_icon.vue';
-  import jobStatusReason from './job_status_reason.vue';
+import ciHeader from '../../vue_shared/components/header_ci_component.vue';
+import loadingIcon from '../../vue_shared/components/loading_icon.vue';
+import jobStatusReason from './job_status_reason.vue';
 
-  export default {
-    name: 'JobHeaderSection',
-    components: {
-      ciHeader,
-      loadingIcon,
-      jobStatusReason,
+export default {
+  name: 'JobHeaderSection',
+  components: {
+    ciHeader,
+    loadingIcon,
+    jobStatusReason,
+  },
+  props: {
+    job: {
+      type: Object,
+      required: true,
     },
-    props: {
-      job: {
-        type: Object,
-        required: true,
-      },
-      isLoading: {
-        type: Boolean,
-        required: true,
-      },
+    isLoading: {
+      type: Boolean,
+      required: true,
     },
-    data() {
-      return {
-        actions: this.getActions(),
-      };
+  },
+  data() {
+    return {
+      actions: this.getActions(),
+    };
+  },
+  computed: {
+    status() {
+      return this.job && this.job.status;
     },
-    computed: {
-      status() {
-        return this.job && this.job.status;
-      },
-      shouldRenderContent() {
-        return !this.isLoading && Object.keys(this.job).length;
-      },
-      shouldRenderReason() {
-        return !!(this.job.status && this.job.status.failure_description);
-      },
-      /**
-       * When job has not started the key will be `false`
-       * When job started the key will be a string with a date.
-       */
-      jobStarted() {
-        return !this.job.started === false;
-      },
+    shouldRenderContent() {
+      return !this.isLoading && Object.keys(this.job).length;
     },
-    watch: {
-      job() {
-        this.actions = this.getActions();
-      },
+    shouldRenderReason() {
+      return !!(this.job.status && this.job.status.failure_description);
     },
-    methods: {
-      getActions() {
-        const actions = [];
+    /**
+     * When job has not started the key will be `false`
+     * When job started the key will be a string with a date.
+     */
+    jobStarted() {
+      return !this.job.started === false;
+    },
+  },
+  watch: {
+    job() {
+      this.actions = this.getActions();
+    },
+  },
+  methods: {
+    getActions() {
+      const actions = [];
 
-        if (this.job.new_issue_path) {
-          actions.push({
-            label: 'New issue',
-            path: this.job.new_issue_path,
-            cssClass: 'js-new-issue btn btn-new btn-inverted visible-md-block visible-lg-block',
-            type: 'link',
-          });
-        }
-        return actions;
-      },
+      if (this.job.new_issue_path) {
+        actions.push({
+          label: 'New issue',
+          path: this.job.new_issue_path,
+          cssClass:
+            'js-new-issue btn btn-new btn-inverted visible-md-block visible-lg-block',
+          type: 'link',
+        });
+      }
+      return actions;
     },
-  };
+  },
+};
 </script>
 <template>
   <header>
diff --git a/spec/javascripts/jobs/header_spec.js b/spec/javascripts/jobs/header_spec.js
index 191760c1fcb..ac5dd33ab13 100644
--- a/spec/javascripts/jobs/header_spec.js
+++ b/spec/javascripts/jobs/header_spec.js
@@ -65,7 +65,10 @@ describe('Job details header', () => {
 
     it('should render provided job information', () => {
       expect(
-        vm.$el.querySelector('.header-main-content').textContent.replace(/\s+/g, ' ').trim(),
+        vm.$el
+          .querySelector('.header-main-content')
+          .textContent.replace(/\s+/g, ' ')
+          .trim(),
       ).toEqual('failed Job #123 triggered 3 weeks ago by Foo');
     });
 
@@ -82,7 +85,10 @@ describe('Job details header', () => {
       vm = mountComponent(HeaderComponent, props);
 
       expect(
-        vm.$el.querySelector('.header-main-content').textContent.replace(/\s+/g, ' ').trim(),
+        vm.$el
+          .querySelector('.header-main-content')
+          .textContent.replace(/\s+/g, ' ')
+          .trim(),
       ).toEqual('failed Job #123 created 3 weeks ago by Foo');
     });
   });
-- 
2.24.1


From d23b7c86cf7244778f13da5473ddea875d60c621 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Fri, 30 Mar 2018 11:04:19 +0100
Subject: [PATCH 03/22] Move the job sidebar top block into vue component

---
 .../jobs/components/sidebar_details_block.vue | 34 +++++++++++++++++++
 app/views/projects/jobs/_sidebar.html.haml    |  7 ----
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index af47056d98f..237aaf8c456 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -44,6 +44,14 @@
       runnerId() {
         return `#${this.job.runner.id}`;
       },
+      retryButtonClass() {
+        let className = 'js-retry-button pull-right btn btn-retry visible-md-block visible-lg-block';
+        className +=
+          this.job.status && this.job.status.retry_button
+            ? ' btn-primary'
+            : ' btn-inverted-secondary';
+        return className;
+      },
       hasTimeout() {
         return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
       },
@@ -75,6 +83,32 @@
 </script>
 <template>
   <div>
+    <div class="block">
+      <strong class="inline prepend-top-8">
+        {{ job.name }}
+      </strong>
+      <a
+        v-if="job.retry_path"
+        :class="retryButtonClass"
+        :href="job.retry_path"
+        data-method="post"
+        rel="nofollow"
+      >
+        Retry
+      </a>
+      <a
+        aria-label="Toggle Sidebar"
+        class="gutter-toggle pull-right visible-xs-block visible-sm-block js-sidebar-build-toggle"
+        href="#"
+        role="button"
+      >
+        <i
+          aria-hidden="true"
+          data-hidden="true"
+          class="fa fa-angle-double-right"
+        ></i>
+      </a>
+    </div>
     <template v-if="shouldRenderContent">
       <div
         class="block retry-link"
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index 0b57ebedebd..4394d2bd2ee 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -1,13 +1,6 @@
 %aside.right-sidebar.right-sidebar-expanded.build-sidebar.js-build-sidebar.js-right-sidebar{ data: { "offset-top" => "101", "spy" => "affix" } }
   .sidebar-container
     .blocks-container
-      .block
-        %strong.inline.prepend-top-8
-          = @build.name
-        - if can?(current_user, :update_build, @build) && @build.retryable?
-          = link_to "Retry", retry_namespace_project_job_path(@project.namespace, @project, @build), class: 'js-retry-button pull-right btn btn-inverted-secondary btn-retry visible-md-block visible-lg-block', method: :post
-        %a.gutter-toggle.pull-right.visible-xs-block.visible-sm-block.js-sidebar-build-toggle{ href: "#", 'aria-label': 'Toggle Sidebar', role: 'button' }
-          = icon('angle-double-right')
 
       #js-details-block-vue
 
-- 
2.24.1


From 20e6bf7826bfd34a7d4d5b1fc078aec369e00e05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Fri, 30 Mar 2018 11:23:29 +0100
Subject: [PATCH 04/22] Prettify builds.scss

---
 app/assets/stylesheets/pages/builds.scss | 41 +++++++++++++++++-------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss
index 7a6352e45f1..50f32660445 100644
--- a/app/assets/stylesheets/pages/builds.scss
+++ b/app/assets/stylesheets/pages/builds.scss
@@ -1,39 +1,56 @@
 @keyframes fade-out-status {
-  0%, 50% { opacity: 1; }
-  100% { opacity: 0; }
+  0%,
+  50% {
+    opacity: 1;
+  }
+
+  100% {
+    opacity: 0;
+  }
 }
 
 @keyframes blinking-dots {
   0% {
     background-color: rgba($white-light, 1);
     box-shadow: 12px 0 0 0 rgba($white-light, 0.2),
-                24px 0 0 0 rgba($white-light, 0.2);
+      24px 0 0 0 rgba($white-light, 0.2);
   }
 
   25% {
     background-color: rgba($white-light, 0.4);
     box-shadow: 12px 0 0 0 rgba($white-light, 2),
-                24px 0 0 0 rgba($white-light, 0.2);
+      24px 0 0 0 rgba($white-light, 0.2);
   }
 
   75% {
     background-color: rgba($white-light, 0.4);
     box-shadow: 12px 0 0 0 rgba($white-light, 0.2),
-                24px 0 0 0 rgba($white-light, 1);
+      24px 0 0 0 rgba($white-light, 1);
   }
 
   100% {
     background-color: rgba($white-light, 1);
     box-shadow: 12px 0 0 0 rgba($white-light, 0.2),
-                24px 0 0 0 rgba($white-light, 0.2);
+      24px 0 0 0 rgba($white-light, 0.2);
   }
 }
 
 @keyframes blinking-scroll-button {
-  0% { opacity: 0.2; }
-  25% { opacity: 0.5; }
-  50% { opacity: 0.7; }
-  100% { opacity: 1; }
+  0% {
+    opacity: 0.2;
+  }
+
+  25% {
+    opacity: 0.5;
+  }
+
+  50% {
+    opacity: 0.7;
+  }
+
+  100% {
+    opacity: 1;
+  }
 }
 
 .build-page {
@@ -125,12 +142,12 @@
       .btn-scroll.animate {
         .first-triangle {
           animation: blinking-scroll-button 1s ease infinite;
-          animation-delay: .3s;
+          animation-delay: 0.3s;
         }
 
         .second-triangle {
           animation: blinking-scroll-button 1s ease infinite;
-          animation-delay: .2s;
+          animation-delay: 0.2s;
         }
 
         .third-triangle {
-- 
2.24.1


From 25f2afc7a8c576371f1915b0d675daf47e05915e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Fri, 30 Mar 2018 15:28:34 +0100
Subject: [PATCH 05/22] Remove icon from callout of job status reason

---
 .../jobs/components/job_status_reason.vue     | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/job_status_reason.vue b/app/assets/javascripts/jobs/components/job_status_reason.vue
index 03128aa8451..2ff5bf09831 100644
--- a/app/assets/javascripts/jobs/components/job_status_reason.vue
+++ b/app/assets/javascripts/jobs/components/job_status_reason.vue
@@ -1,17 +1,12 @@
 <script>
-  import icon from '../../vue_shared/components/icon.vue';
-
-  export default {
-    components: {
-      icon,
+export default {
+  props: {
+    job: {
+      type: Object,
+      required: true,
     },
-    props: {
-      job: {
-        type: Object,
-        required: true,
-      },
-    },
-  };
+  },
+};
 </script>
 <template>
   <div
-- 
2.24.1


From 06d40df75bd0118714acdee2326baf230a2b5cb9 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Sun, 1 Apr 2018 13:00:58 -0500
Subject: [PATCH 06/22] Include callout message and retry button into Job
 Entity

---
 .../javascripts/jobs/components/header.vue    |  2 +-
 app/presenters/ci/build_presenter.rb          | 23 ++++++++++++
 app/serializers/job_entity.rb                 | 22 +++++++++++-
 spec/presenters/ci/build_presenter_spec.rb    | 35 +++++++++++++++++++
 spec/serializers/job_entity_spec.rb           | 34 +++++++++++++-----
 5 files changed, 105 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/header.vue b/app/assets/javascripts/jobs/components/header.vue
index 2e0fa599b7e..fc6e9ff0663 100644
--- a/app/assets/javascripts/jobs/components/header.vue
+++ b/app/assets/javascripts/jobs/components/header.vue
@@ -33,7 +33,7 @@ export default {
       return !this.isLoading && Object.keys(this.job).length;
     },
     shouldRenderReason() {
-      return !!(this.job.status && this.job.status.failure_description);
+      return !!(this.job.status && this.job.status.callout_message);
     },
     /**
      * When job has not started the key will be `false`
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 9afebda19be..16765cdae66 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -35,6 +35,14 @@ module Ci
       "#{subject.name} - #{detailed_status.status_tooltip}"
     end
 
+    def callout_failure_message
+      failure_messages[failure_reason]
+    end
+
+    def show_retry_button?
+      !not_retryable_failures.include?(failure_reason)
+    end
+
     private
 
     def tooltip_for_badge
@@ -44,5 +52,20 @@ module Ci
     def detailed_status
       @detailed_status ||= subject.detailed_status(user)
     end
+
+    def failure_messages
+      {
+        'unknown_failure' => 'There is an unknown failure, please try again',
+        'script_failure' => 'There has been a script failure. Check the job log for more information',
+        'api_failure' => 'There has been an API failure, please try again',
+        'stuck_or_timeout_failure' => 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again',
+        'runner_system_failure' => 'There has been a runner system failure, please try again',
+        'missing_dependency_failure' => 'There has been a missing dependency failure, check the job log for more information'
+      }
+    end
+
+    def not_retryable_failures
+      %w(script_failure missing_dependency_failure)
+    end
   end
 end
diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb
index 523b522d449..7b1fe101aea 100644
--- a/app/serializers/job_entity.rb
+++ b/app/serializers/job_entity.rb
@@ -25,7 +25,11 @@ 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 :callout_message, if: -> (*) { failed_or_allowed_to_fail? }
+    expose :retry_button, if: -> (*) { failed_or_allowed_to_fail? }
+    expose :detailed_status, merge: true, as: :status, with: StatusEntity
+  end
 
   private
 
@@ -47,6 +51,22 @@ class JobEntity < Grape::Entity
     build.detailed_status(request.current_user)
   end
 
+  def failed_or_allowed_to_fail?
+    build.failed? || build.allow_failure?
+  end
+
+  def callout_message
+    build_presenter.callout_failure_message
+  end
+
+  def retry_button
+    build_presenter.show_retry_button?
+  end
+
+  def build_presenter
+    @build_presenter ||= Ci::BuildPresenter.new(build)
+  end
+
   def path_to(route, build)
     send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend
   end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index cc16d0f156b..7539491d0fb 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -217,4 +217,39 @@ describe Ci::BuildPresenter do
       end
     end
   end
+
+  describe '#callout_failure_message' do
+    let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
+
+    it 'returns a verbose reason of failure' do
+      description = subject.callout_failure_message
+      expect(description).to eq('There has been a script failure. Check the job log for more information')
+    end
+  end
+
+  describe '#show_retry_button' do
+    context 'when is a script or missing dependency failure' do
+      let(:build) { create(:ci_build, :failed, :script_failure) }
+      let(:failure_reasons) { %w(script_failure missing_dependency_failure) }
+
+      it 'should return false' do
+        failure_reasons.each do |failure_reason|
+          build.update_attribute(:failure_reason, failure_reason)
+          expect(presenter.show_retry_button?).to be_falsy
+        end
+      end
+    end
+
+    context 'when is any other failure type' do
+      let(:build) { create(:ci_build, :failed) }
+      let(:failure_reasons) { %w(unknown_failure api_failure stuck_or_timeout_failure runner_system_failure) }
+
+      it 'should return true' do
+        failure_reasons.each do |failure_reason|
+          build.update_attribute(:failure_reason, failure_reason)
+          expect(presenter.show_retry_button?).to be_truthy
+        end
+      end
+    end
+  end
 end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index 24a6f1a2a8a..1db7bc36a33 100644
--- a/spec/serializers/job_entity_spec.rb
+++ b/spec/serializers/job_entity_spec.rb
@@ -130,24 +130,40 @@ describe JobEntity do
     end
   end
 
-  context 'when job failed' do
-    let(:job) { create(:ci_build, :script_failure) }
+  describe '#status' do
+    context 'when job failed' do
+      let(:job) { create(:ci_build, :script_failure) }
 
-    describe 'status' do
       it 'should contain the failure reason inside label' do
         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)')
+        expect(subject[:status][:callout_message]).to eq('There has been a script failure. Check the job log for more information')
+        expect(subject[:status][:retry_button]).to eq(false)
+      end
+    end
+
+    context 'when job is allowed to fail' do
+      let(:job) { create(:ci_build, :script_failure, :allowed_to_fail) }
+
+      it 'should contain the failure reason inside label' do
+        expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
+        expect(subject[:status][:label]).to eq('failed (allowed to fail)')
+        expect(subject[:status][:tooltip]).to eq('failed <br> (script failure)')
+        expect(subject[:status][:callout_message]).to eq('There has been a script failure. Check the job log for more information')
+        expect(subject[:status][:retry_button]).to eq(false)
       end
     end
-  end
 
-  context 'when job passed' do
-    let(:job) { create(:ci_build, :success) }
+    context 'when job has any other status' 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')
+      describe 'status' do
+        it 'should not contain the failure reason inside label' do
+          expect(subject[:status][:tooltip]).to eq('passed')
+          expect(subject[:status].keys).not_to include('callout_message')
+          expect(subject[:status].keys).not_to include('retry_button')
+        end
       end
     end
   end
-- 
2.24.1


From b3acc5a5e034a6e61ea58676576ca30382de351d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Mon, 2 Apr 2018 19:07:21 +0100
Subject: [PATCH 07/22] Make vue aware of whether use can retry build from ruby

---
 .../jobs/components/sidebar_details_block.vue            | 9 +++++++--
 app/assets/javascripts/jobs/job_details_bundle.js        | 5 ++++-
 app/views/projects/jobs/_sidebar.html.haml               | 2 +-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index 237aaf8c456..731c3bbfca1 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -22,6 +22,11 @@
         type: Boolean,
         required: true,
       },
+      canUserRetry: {
+        type: Boolean,
+        required: false,
+        default: false,
+      },
       runnerHelpUrl: {
         type: String,
         required: false,
@@ -88,7 +93,7 @@
         {{ job.name }}
       </strong>
       <a
-        v-if="job.retry_path"
+        v-if="canUserRetry"
         :class="retryButtonClass"
         :href="job.retry_path"
         data-method="post"
@@ -122,7 +127,7 @@
           New issue
         </a>
         <a
-          v-if="job.retry_path"
+          v-if="canUserRetry"
           class="js-retry-job btn btn-inverted-secondary"
           :href="job.retry_path"
           data-method="post"
diff --git a/app/assets/javascripts/jobs/job_details_bundle.js b/app/assets/javascripts/jobs/job_details_bundle.js
index 656676ead91..7a32b572711 100644
--- a/app/assets/javascripts/jobs/job_details_bundle.js
+++ b/app/assets/javascripts/jobs/job_details_bundle.js
@@ -35,9 +35,11 @@ export default () => {
   });
 
   // Sidebar information block
+  const detailsBlockId = 'js-details-block-vue';
+  const detailsBlockDataset = document.getElementById(detailsBlockId).dataset;
   // eslint-disable-next-line
   new Vue({
-    el: '#js-details-block-vue',
+    el: `#${detailsBlockId}`,
     components: {
       detailsBlock,
     },
@@ -50,6 +52,7 @@ export default () => {
       return createElement('details-block', {
         props: {
           isLoading: this.mediator.state.isLoading,
+          canUserRetry: !!('canUserRetry' in detailsBlockDataset),
           job: this.mediator.store.state.job,
           runnerHelpUrl: dataset.runnerHelpUrl,
         },
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index 4394d2bd2ee..7f0bef5ede0 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -2,7 +2,7 @@
   .sidebar-container
     .blocks-container
 
-      #js-details-block-vue
+      #js-details-block-vue{ data: { can_user_retry: can?(current_user, :update_build, @build) && @build.retryable? } }
 
       - if can?(current_user, :read_build, @project) && (@build.artifacts? || @build.artifacts_expired?)
         .block
-- 
2.24.1


From 4195fbe6a77b810c80134a81c270a68bf88bbe5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Mon, 2 Apr 2018 19:08:59 +0100
Subject: [PATCH 08/22] Make toggle button for job sidebar use a button element

---
 .../jobs/components/sidebar_details_block.vue    | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index 731c3bbfca1..341dc7e69c4 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -57,6 +57,14 @@
             : ' btn-inverted-secondary';
         return className;
       },
+      toggleButtonClass() {
+        return [
+          'btn btn-blank',
+          'gutter-toggle pull-right',
+          'visible-xs-block visible-sm-block',
+          'js-sidebar-build-toggle',
+        ].join(' ');
+      },
       hasTimeout() {
         return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
       },
@@ -101,18 +109,16 @@
       >
         Retry
       </a>
-      <a
+      <button
         aria-label="Toggle Sidebar"
-        class="gutter-toggle pull-right visible-xs-block visible-sm-block js-sidebar-build-toggle"
-        href="#"
-        role="button"
+        :class="toggleButtonClass"
       >
         <i
           aria-hidden="true"
           data-hidden="true"
           class="fa fa-angle-double-right"
         ></i>
-      </a>
+      </button>
     </div>
     <template v-if="shouldRenderContent">
       <div
-- 
2.24.1


From e9752658018782539a031006ea8e73b5b60cf189 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Mon, 2 Apr 2018 19:13:47 +0100
Subject: [PATCH 09/22] Add i18n to sidebar_details_block vue componennt

---
 .../jobs/components/sidebar_details_block.vue      | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index 341dc7e69c4..93432aa4c12 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -107,10 +107,10 @@
         data-method="post"
         rel="nofollow"
       >
-        Retry
+        {{ __('Retry') }}
       </a>
       <button
-        aria-label="Toggle Sidebar"
+        :aria-label="__('Toggle Sidebar')"
         :class="toggleButtonClass"
       >
         <i
@@ -130,7 +130,7 @@
           class="js-new-issue btn btn-new btn-inverted"
           :href="job.new_issue_path"
         >
-          New issue
+          {{ __('New issue') }}
         </a>
         <a
           v-if="canUserRetry"
@@ -139,7 +139,7 @@
           data-method="post"
           rel="nofollow"
         >
-          Retry
+          {{ __('Retry') }}
         </a>
       </div>
       <div :class="{block : renderBlock }">
@@ -148,7 +148,7 @@
           v-if="job.merge_request"
         >
           <span class="build-light-text">
-            Merge Request:
+            {{ __('Merge Request:') }}
           </span>
           <a :href="job.merge_request.path">
             !{{ job.merge_request.iid }}
@@ -203,7 +203,7 @@
           v-if="job.tags.length"
         >
           <span class="build-light-text">
-            Tags:
+            {{ __('Tags:') }}
           </span>
           <span
             v-for="(tag, i) in job.tags"
@@ -223,7 +223,7 @@
             data-method="post"
             rel="nofollow"
           >
-            Cancel
+            {{ __('Cancel') }}
           </a>
         </div>
       </div>
-- 
2.24.1


From 4cba119397c4206474b70a021c60cc55b4511520 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Mon, 2 Apr 2018 15:21:47 -0500
Subject: [PATCH 10/22] Print 'no job log' on job trace if there's no log

---
 app/controllers/projects/jobs_controller.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index dd12d30a085..545f49eb4cf 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -75,6 +75,7 @@ class Projects::JobsController < Projects::ApplicationController
             stream.limit
             state = params[:state].presence
             trace = stream.html_with_state(state)
+            trace[:html] = "No job log" if trace[:html].blank?
             result.merge!(trace.to_h)
           end
 
-- 
2.24.1


From e47907b94d29dfa9acf3774c41b8dcb1b09a63e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andre=CC=81=20Lui=CC=81s?= <aluis@gitlab.com>
Date: Tue, 3 Apr 2018 14:09:05 +0100
Subject: [PATCH 11/22] Update karma for sidebar_details_block vue

---
 .../jobs/sidebar_details_block_spec.js        | 61 ++++++++++---------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/spec/javascripts/jobs/sidebar_details_block_spec.js b/spec/javascripts/jobs/sidebar_details_block_spec.js
index 602dae514b1..6b397c22fb9 100644
--- a/spec/javascripts/jobs/sidebar_details_block_spec.js
+++ b/spec/javascripts/jobs/sidebar_details_block_spec.js
@@ -31,10 +31,25 @@ describe('Sidebar details block', () => {
     });
   });
 
+  describe("when user can't retry", () => {
+    it('should not render a retry button', () => {
+      vm = new SidebarComponent({
+        propsData: {
+          job: {},
+          canUserRetry: false,
+          isLoading: true,
+        },
+      }).$mount();
+
+      expect(vm.$el.querySelector('.js-retry-job')).toBeNull();
+    });
+  });
+
   beforeEach(() => {
     vm = new SidebarComponent({
       propsData: {
         job,
+        canUserRetry: true,
         isLoading: false,
       },
     }).$mount();
@@ -42,7 +57,9 @@ describe('Sidebar details block', () => {
 
   describe('actions', () => {
     it('should render link to new issue', () => {
-      expect(vm.$el.querySelector('.js-new-issue').getAttribute('href')).toEqual(job.new_issue_path);
+      expect(vm.$el.querySelector('.js-new-issue').getAttribute('href')).toEqual(
+        job.new_issue_path,
+      );
       expect(vm.$el.querySelector('.js-new-issue').textContent.trim()).toEqual('New issue');
     });
 
@@ -57,43 +74,35 @@ describe('Sidebar details block', () => {
 
   describe('information', () => {
     it('should render merge request link', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-mr')),
-      ).toEqual('Merge Request: !2');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-mr'))).toEqual('Merge Request: !2');
 
-      expect(
-        vm.$el.querySelector('.js-job-mr a').getAttribute('href'),
-      ).toEqual(job.merge_request.path);
+      expect(vm.$el.querySelector('.js-job-mr a').getAttribute('href')).toEqual(
+        job.merge_request.path,
+      );
     });
 
     it('should render job duration', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-duration')),
-      ).toEqual('Duration: 6 seconds');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-duration'))).toEqual(
+        'Duration: 6 seconds',
+      );
     });
 
     it('should render erased date', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-erased')),
-      ).toEqual('Erased: 3 weeks ago');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-erased'))).toEqual('Erased: 3 weeks ago');
     });
 
     it('should render finished date', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-finished')),
-      ).toEqual('Finished: 3 weeks ago');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-finished'))).toEqual(
+        'Finished: 3 weeks ago',
+      );
     });
 
     it('should render queued date', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-queued')),
-      ).toEqual('Queued: 9 seconds');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-queued'))).toEqual('Queued: 9 seconds');
     });
 
     it('should render runner ID', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-runner')),
-      ).toEqual('Runner: #1');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-runner'))).toEqual('Runner: #1');
     });
 
     it('should render timeout information', () => {
@@ -103,15 +112,11 @@ describe('Sidebar details block', () => {
     });
 
     it('should render coverage', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-coverage')),
-      ).toEqual('Coverage: 20%');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-coverage'))).toEqual('Coverage: 20%');
     });
 
     it('should render tags', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-tags')),
-      ).toEqual('Tags: tag');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-tags'))).toEqual('Tags: tag');
     });
   });
 });
-- 
2.24.1


From 480ea2b7669058761a3870fe308ee60f5634fbd6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Lu=C3=ADs?= <aluis@gitlab.com>
Date: Tue, 3 Apr 2018 19:33:56 +0100
Subject: [PATCH 12/22] Improve code style of sidebar_details_block

Improves a few tweaks that were added to conform with eslint.
---
 .../jobs/components/sidebar_details_block.vue        | 12 +++---------
 app/assets/javascripts/jobs/job_details_bundle.js    |  6 +++---
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index 93432aa4c12..591a216366a 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -57,14 +57,6 @@
             : ' btn-inverted-secondary';
         return className;
       },
-      toggleButtonClass() {
-        return [
-          'btn btn-blank',
-          'gutter-toggle pull-right',
-          'visible-xs-block visible-sm-block',
-          'js-sidebar-build-toggle',
-        ].join(' ');
-      },
       hasTimeout() {
         return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
       },
@@ -110,8 +102,10 @@
         {{ __('Retry') }}
       </a>
       <button
+        type="button"
         :aria-label="__('Toggle Sidebar')"
-        :class="toggleButtonClass"
+        class="btn btn-blank gutter-toggle pull-right
+          visible-xs-block visible-sm-block js-sidebar-build-toggle"
       >
         <i
           aria-hidden="true"
diff --git a/app/assets/javascripts/jobs/job_details_bundle.js b/app/assets/javascripts/jobs/job_details_bundle.js
index 7a32b572711..f2939ad4dbe 100644
--- a/app/assets/javascripts/jobs/job_details_bundle.js
+++ b/app/assets/javascripts/jobs/job_details_bundle.js
@@ -35,11 +35,11 @@ export default () => {
   });
 
   // Sidebar information block
-  const detailsBlockId = 'js-details-block-vue';
-  const detailsBlockDataset = document.getElementById(detailsBlockId).dataset;
+  const detailsBlockElement = document.getElementById('js-details-block-vue');
+  const detailsBlockDataset = detailsBlockElement.dataset;
   // eslint-disable-next-line
   new Vue({
-    el: `#${detailsBlockId}`,
+    el: detailsBlockElement,
     components: {
       detailsBlock,
     },
-- 
2.24.1


From a4e28adc77c76efa1faaa9c6987177d3d1d0dd9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Lu=C3=ADs?= <aluis@gitlab.com>
Date: Fri, 6 Apr 2018 12:12:16 +0100
Subject: [PATCH 13/22] Prettify sidebar_details_block.vue

---
 .../jobs/components/sidebar_details_block.vue | 160 +++++++++---------
 1 file changed, 80 insertions(+), 80 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index 591a216366a..23c2dedcd76 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -1,90 +1,90 @@
 <script>
-  import detailRow from './sidebar_detail_row.vue';
-  import loadingIcon from '../../vue_shared/components/loading_icon.vue';
-  import timeagoMixin from '../../vue_shared/mixins/timeago';
-  import { timeIntervalInWords } from '../../lib/utils/datetime_utility';
+import detailRow from './sidebar_detail_row.vue';
+import loadingIcon from '../../vue_shared/components/loading_icon.vue';
+import timeagoMixin from '../../vue_shared/mixins/timeago';
+import { timeIntervalInWords } from '../../lib/utils/datetime_utility';
 
-  export default {
-    name: 'SidebarDetailsBlock',
-    components: {
-      detailRow,
-      loadingIcon,
+export default {
+  name: 'SidebarDetailsBlock',
+  components: {
+    detailRow,
+    loadingIcon,
+  },
+  mixins: [timeagoMixin],
+  props: {
+    job: {
+      type: Object,
+      required: true,
     },
-    mixins: [
-      timeagoMixin,
-    ],
-    props: {
-      job: {
-        type: Object,
-        required: true,
-      },
-      isLoading: {
-        type: Boolean,
-        required: true,
-      },
-      canUserRetry: {
-        type: Boolean,
-        required: false,
-        default: false,
-      },
-      runnerHelpUrl: {
-        type: String,
-        required: false,
-        default: '',
-      },
+    isLoading: {
+      type: Boolean,
+      required: true,
     },
-    computed: {
-      shouldRenderContent() {
-        return !this.isLoading && Object.keys(this.job).length > 0;
-      },
-      coverage() {
-        return `${this.job.coverage}%`;
-      },
-      duration() {
-        return timeIntervalInWords(this.job.duration);
-      },
-      queued() {
-        return timeIntervalInWords(this.job.queued);
-      },
-      runnerId() {
-        return `#${this.job.runner.id}`;
-      },
-      retryButtonClass() {
-        let className = 'js-retry-button pull-right btn btn-retry visible-md-block visible-lg-block';
-        className +=
-          this.job.status && this.job.status.retry_button
-            ? ' btn-primary'
-            : ' btn-inverted-secondary';
-        return className;
-      },
-      hasTimeout() {
-        return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
-      },
-      timeout() {
-        if (this.job.metadata == null) {
-          return '';
-        }
+    canUserRetry: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    runnerHelpUrl: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  computed: {
+    shouldRenderContent() {
+      return !this.isLoading && Object.keys(this.job).length > 0;
+    },
+    coverage() {
+      return `${this.job.coverage}%`;
+    },
+    duration() {
+      return timeIntervalInWords(this.job.duration);
+    },
+    queued() {
+      return timeIntervalInWords(this.job.queued);
+    },
+    runnerId() {
+      return `#${this.job.runner.id}`;
+    },
+    retryButtonClass() {
+      let className = 'js-retry-button pull-right btn btn-retry visible-md-block visible-lg-block';
+      className +=
+        this.job.status && this.job.status.retry_button
+          ? ' btn-primary'
+          : ' btn-inverted-secondary';
+      return className;
+    },
+    hasTimeout() {
+      return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
+    },
+    timeout() {
+      if (this.job.metadata == null) {
+        return '';
+      }
 
-        let t = this.job.metadata.timeout_human_readable;
-        if (this.job.metadata.timeout_source !== '') {
-          t += ` (from ${this.job.metadata.timeout_source})`;
-        }
+      let t = this.job.metadata.timeout_human_readable;
+      if (this.job.metadata.timeout_source !== '') {
+        t += ` (from ${this.job.metadata.timeout_source})`;
+      }
 
-        return t;
-      },
-      renderBlock() {
-        return this.job.merge_request ||
-          this.job.duration ||
-          this.job.finished_data ||
-          this.job.erased_at ||
-          this.job.queued ||
-          this.job.runner ||
-          this.job.coverage ||
-          this.job.tags.length ||
-          this.job.cancel_path;
-      },
+      return t;
+    },
+    renderBlock() {
+      return (
+        this.job.merge_request ||
+        this.job.duration ||
+        this.job.finished_data ||
+        this.job.erased_at ||
+        this.job.queued ||
+        this.job.runner ||
+        this.job.coverage ||
+        this.job.tags.length ||
+        this.job.cancel_path
+      );
     },
-  };
+  },
+};
 </script>
 <template>
   <div>
-- 
2.24.1


From 0a311abd240d7b25411b9a593fa0b87bf164e586 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Tue, 10 Apr 2018 11:41:49 -0500
Subject: [PATCH 14/22] Moves callout logic to a different presenter

- This methods are only used on Job detail view, so it doesn't make
  sense to have it in the regular presenter
- Readjust code and specs for this purpose
---
 app/controllers/projects/jobs_controller.rb   |  3 +-
 app/presenters/ci/build_detail_presenter.rb   | 28 +++++++++++++
 app/presenters/ci/build_presenter.rb          | 23 -----------
 app/serializers/job_entity.rb                 | 16 ++++----
 .../projects/jobs_controller_spec.rb          |  5 ++-
 .../ci/build_detail_presenter_spec.rb         | 40 +++++++++++++++++++
 spec/presenters/ci/build_presenter_spec.rb    | 35 ----------------
 spec/serializers/job_entity_spec.rb           | 34 ++++++++--------
 8 files changed, 100 insertions(+), 84 deletions(-)
 create mode 100644 app/presenters/ci/build_detail_presenter.rb
 create mode 100644 spec/presenters/ci/build_detail_presenter_spec.rb

diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index 545f49eb4cf..7497b5012ec 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -75,10 +75,11 @@ class Projects::JobsController < Projects::ApplicationController
             stream.limit
             state = params[:state].presence
             trace = stream.html_with_state(state)
-            trace[:html] = "No job log" if trace[:html].blank?
             result.merge!(trace.to_h)
           end
 
+          result[:html] = result[:html].presence || 'No job log'
+
           render json: result
         end
       end
diff --git a/app/presenters/ci/build_detail_presenter.rb b/app/presenters/ci/build_detail_presenter.rb
new file mode 100644
index 00000000000..7c43b2db67c
--- /dev/null
+++ b/app/presenters/ci/build_detail_presenter.rb
@@ -0,0 +1,28 @@
+module Ci
+  class BuildDetailPresenter < Gitlab::View::Presenter::Delegated
+    CALLOUT_FAILURE_MESSAGES = {
+      unknown_failure: 'There is an unknown failure, please try again',
+      script_failure: 'There has been a script failure. Check the job log for more information',
+      api_failure: 'There has been an API failure, please try again',
+      stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again',
+      runner_system_failure: 'There has been a runner system failure, please try again',
+      missing_dependency_failure: 'There has been a missing dependency failure, check the job log for more information'
+    }.freeze
+
+    presents :build
+
+    def callout_failure_message
+      CALLOUT_FAILURE_MESSAGES[failure_reason.to_sym]
+    end
+
+    def show_retry_button?
+      try_again_failures.include?(failure_reason.to_sym)
+    end
+
+    private
+
+    def try_again_failures
+      CALLOUT_FAILURE_MESSAGES.keys - %i(script_failure missing_dependency_failure)
+    end
+  end
+end
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 16765cdae66..9afebda19be 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -35,14 +35,6 @@ module Ci
       "#{subject.name} - #{detailed_status.status_tooltip}"
     end
 
-    def callout_failure_message
-      failure_messages[failure_reason]
-    end
-
-    def show_retry_button?
-      !not_retryable_failures.include?(failure_reason)
-    end
-
     private
 
     def tooltip_for_badge
@@ -52,20 +44,5 @@ module Ci
     def detailed_status
       @detailed_status ||= subject.detailed_status(user)
     end
-
-    def failure_messages
-      {
-        'unknown_failure' => 'There is an unknown failure, please try again',
-        'script_failure' => 'There has been a script failure. Check the job log for more information',
-        'api_failure' => 'There has been an API failure, please try again',
-        'stuck_or_timeout_failure' => 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again',
-        'runner_system_failure' => 'There has been a runner system failure, please try again',
-        'missing_dependency_failure' => 'There has been a missing dependency failure, check the job log for more information'
-      }
-    end
-
-    def not_retryable_failures
-      %w(script_failure missing_dependency_failure)
-    end
   end
 end
diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb
index 7b1fe101aea..aa0955b1e27 100644
--- a/app/serializers/job_entity.rb
+++ b/app/serializers/job_entity.rb
@@ -51,23 +51,23 @@ class JobEntity < Grape::Entity
     build.detailed_status(request.current_user)
   end
 
+  def path_to(route, build)
+    send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend
+  end
+
   def failed_or_allowed_to_fail?
     build.failed? || build.allow_failure?
   end
 
   def callout_message
-    build_presenter.callout_failure_message
+    build_detail_presenter.callout_failure_message
   end
 
   def retry_button
-    build_presenter.show_retry_button?
+    build_detail_presenter.show_retry_button?
   end
 
-  def build_presenter
-    @build_presenter ||= Ci::BuildPresenter.new(build)
-  end
-
-  def path_to(route, build)
-    send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend
+  def build_detail_presenter
+    @build_detail_presenter ||= Ci::BuildDetailPresenter.new(build)
   end
 end
diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb
index b9a979044fe..f677cec3408 100644
--- a/spec/controllers/projects/jobs_controller_spec.rb
+++ b/spec/controllers/projects/jobs_controller_spec.rb
@@ -190,7 +190,10 @@ describe Projects::JobsController do
         expect(response).to have_gitlab_http_status(:ok)
         expect(json_response['id']).to eq job.id
         expect(json_response['status']).to eq job.status
-        expect(json_response['html']).to be_nil
+      end
+
+      it 'returns no job log message' do
+        expect(json_response['html']).to eq('No job log')
       end
     end
 
diff --git a/spec/presenters/ci/build_detail_presenter_spec.rb b/spec/presenters/ci/build_detail_presenter_spec.rb
new file mode 100644
index 00000000000..60f6ac39306
--- /dev/null
+++ b/spec/presenters/ci/build_detail_presenter_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+
+describe Ci::BuildDetailPresenter do
+  let(:build) { create(:ci_build, :failed, :script_failure) }
+
+  subject(:presenter) do
+    described_class.new(build)
+  end
+
+  describe '#callout_failure_message' do
+    it 'returns a verbose failure reason' do
+      description = subject.callout_failure_message
+      expect(description).to eq('There has been a script failure. Check the job log for more information')
+    end
+  end
+
+  describe '#show_retry_button' do
+    context 'when is a script or missing dependency failure' do
+      let(:failure_reasons) { %w(script_failure missing_dependency_failure) }
+
+      it 'should return false' do
+        failure_reasons.each do |failure_reason|
+          build.update_attribute(:failure_reason, failure_reason)
+          expect(presenter.show_retry_button?).to be_falsy
+        end
+      end
+    end
+
+    context 'when is any other failure type' do
+      let(:failure_reasons) { %w(unknown_failure api_failure stuck_or_timeout_failure runner_system_failure) }
+
+      it 'should return true' do
+        failure_reasons.each do |failure_reason|
+          build.update_attribute(:failure_reason, failure_reason)
+          expect(presenter.show_retry_button?).to be_truthy
+        end
+      end
+    end
+  end
+end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index 7539491d0fb..cc16d0f156b 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -217,39 +217,4 @@ describe Ci::BuildPresenter do
       end
     end
   end
-
-  describe '#callout_failure_message' do
-    let(:build) { create(:ci_build, :failed, :script_failure, pipeline: pipeline) }
-
-    it 'returns a verbose reason of failure' do
-      description = subject.callout_failure_message
-      expect(description).to eq('There has been a script failure. Check the job log for more information')
-    end
-  end
-
-  describe '#show_retry_button' do
-    context 'when is a script or missing dependency failure' do
-      let(:build) { create(:ci_build, :failed, :script_failure) }
-      let(:failure_reasons) { %w(script_failure missing_dependency_failure) }
-
-      it 'should return false' do
-        failure_reasons.each do |failure_reason|
-          build.update_attribute(:failure_reason, failure_reason)
-          expect(presenter.show_retry_button?).to be_falsy
-        end
-      end
-    end
-
-    context 'when is any other failure type' do
-      let(:build) { create(:ci_build, :failed) }
-      let(:failure_reasons) { %w(unknown_failure api_failure stuck_or_timeout_failure runner_system_failure) }
-
-      it 'should return true' do
-        failure_reasons.each do |failure_reason|
-          build.update_attribute(:failure_reason, failure_reason)
-          expect(presenter.show_retry_button?).to be_truthy
-        end
-      end
-    end
-  end
 end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index 1db7bc36a33..8a0a645dced 100644
--- a/spec/serializers/job_entity_spec.rb
+++ b/spec/serializers/job_entity_spec.rb
@@ -130,40 +130,42 @@ describe JobEntity do
     end
   end
 
-  describe '#status' do
-    context 'when job failed' do
-      let(:job) { create(:ci_build, :script_failure) }
+  context 'when job failed' do
+    let(:job) { create(:ci_build, :script_failure) }
 
+    describe 'status' do
       it 'should contain the failure reason inside label' do
         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)')
         expect(subject[:status][:callout_message]).to eq('There has been a script failure. Check the job log for more information')
-        expect(subject[:status][:retry_button]).to eq(false)
+        expect(subject[:status][:retry_button]).to be_falsy
       end
     end
+  end
 
-    context 'when job is allowed to fail' do
-      let(:job) { create(:ci_build, :script_failure, :allowed_to_fail) }
+  context 'when job is allowed to fail' do
+    let(:job) { create(:ci_build, :allowed_to_fail, :script_failure) }
 
+    describe 'status' do
       it 'should contain the failure reason inside label' do
         expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
         expect(subject[:status][:label]).to eq('failed (allowed to fail)')
-        expect(subject[:status][:tooltip]).to eq('failed <br> (script failure)')
+        expect(subject[:status][:tooltip]).to eq('failed <br> (script failure) (allowed to fail)')
         expect(subject[:status][:callout_message]).to eq('There has been a script failure. Check the job log for more information')
-        expect(subject[:status][:retry_button]).to eq(false)
+        expect(subject[:status][:retry_button]).to be_falsy
       end
     end
+  end
 
-    context 'when job has any other status' do
-      let(:job) { create(:ci_build, :success) }
+  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][:tooltip]).to eq('passed')
-          expect(subject[:status].keys).not_to include('callout_message')
-          expect(subject[:status].keys).not_to include('retry_button')
-        end
+    describe 'status' do
+      it 'should not contain the failure reason inside label' do
+        expect(subject[:status][:label]).to eq('passed')
+        expect(subject[:status]).not_to include('callout_message')
+        expect(subject[:status]).not_to include('retry_button')
       end
     end
   end
-- 
2.24.1


From 0847b7f38a2e637bf695f979176d5742dc10ff60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Lu=C3=ADs?= <aluis@gitlab.com>
Date: Thu, 12 Apr 2018 17:51:51 +0100
Subject: [PATCH 15/22] Refactor job status reason vue into generic callout

---
 .../javascripts/jobs/components/header.vue    | 11 ++++----
 .../jobs/components/job_status_reason.vue     | 19 -------------
 .../vue_shared/components/callout.vue         | 27 +++++++++++++++++++
 3 files changed, 32 insertions(+), 25 deletions(-)
 delete mode 100644 app/assets/javascripts/jobs/components/job_status_reason.vue
 create mode 100644 app/assets/javascripts/vue_shared/components/callout.vue

diff --git a/app/assets/javascripts/jobs/components/header.vue b/app/assets/javascripts/jobs/components/header.vue
index fc6e9ff0663..154cf54eb73 100644
--- a/app/assets/javascripts/jobs/components/header.vue
+++ b/app/assets/javascripts/jobs/components/header.vue
@@ -1,14 +1,14 @@
 <script>
 import ciHeader from '../../vue_shared/components/header_ci_component.vue';
 import loadingIcon from '../../vue_shared/components/loading_icon.vue';
-import jobStatusReason from './job_status_reason.vue';
+import callout from '../../vue_shared/components/callout.vue';
 
 export default {
   name: 'JobHeaderSection',
   components: {
     ciHeader,
     loadingIcon,
-    jobStatusReason,
+    callout,
   },
   props: {
     job: {
@@ -56,8 +56,7 @@ export default {
         actions.push({
           label: 'New issue',
           path: this.job.new_issue_path,
-          cssClass:
-            'js-new-issue btn btn-new btn-inverted visible-md-block visible-lg-block',
+          cssClass: 'js-new-issue btn btn-new btn-inverted visible-md-block visible-lg-block',
           type: 'link',
         });
       }
@@ -87,9 +86,9 @@ export default {
       />
     </div>
 
-    <job-status-reason
+    <callout
       v-if="shouldRenderReason"
-      :job="job"
+      :message="job.status.callout_message"
     />
   </header>
 </template>
diff --git a/app/assets/javascripts/jobs/components/job_status_reason.vue b/app/assets/javascripts/jobs/components/job_status_reason.vue
deleted file mode 100644
index 2ff5bf09831..00000000000
--- a/app/assets/javascripts/jobs/components/job_status_reason.vue
+++ /dev/null
@@ -1,19 +0,0 @@
-<script>
-export default {
-  props: {
-    job: {
-      type: Object,
-      required: true,
-    },
-  },
-};
-</script>
-<template>
-  <div
-    class="bs-callout bs-callout-danger"
-    role="alert"
-    aria-live="assertive"
-  >
-    {{ job.status.callout_message }}
-  </div>
-</template>
diff --git a/app/assets/javascripts/vue_shared/components/callout.vue b/app/assets/javascripts/vue_shared/components/callout.vue
new file mode 100644
index 00000000000..ccf802c456c
--- /dev/null
+++ b/app/assets/javascripts/vue_shared/components/callout.vue
@@ -0,0 +1,27 @@
+<script>
+const calloutVariants = ['danger', 'success', 'info', 'warning'];
+
+export default {
+  props: {
+    category: {
+      type: String,
+      required: false,
+      default: calloutVariants[0],
+      validator: value => calloutVariants.includes(value),
+    },
+    message: {
+      type: String,
+      required: true,
+    },
+  },
+};
+</script>
+<template>
+  <div
+    :class="`bs-callout bs-callout-${category}`"
+    role="alert"
+    aria-live="assertive"
+  >
+    {{ message }}
+  </div>
+</template>
-- 
2.24.1


From aab3854b764ac30453021600d4126ed3ffe9ccc3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Lu=C3=ADs?= <aluis@gitlab.com>
Date: Fri, 13 Apr 2018 12:17:23 +0100
Subject: [PATCH 16/22] Add karma tests for callout vue component

---
 .../vue_shared/components/callout_spec.js     | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 spec/javascripts/vue_shared/components/callout_spec.js

diff --git a/spec/javascripts/vue_shared/components/callout_spec.js b/spec/javascripts/vue_shared/components/callout_spec.js
new file mode 100644
index 00000000000..5e452a20b09
--- /dev/null
+++ b/spec/javascripts/vue_shared/components/callout_spec.js
@@ -0,0 +1,45 @@
+import Vue from 'vue';
+import callout from '~/vue_shared/components/callout.vue';
+
+describe('Callout Component', () => {
+  let CalloutComponent;
+  const exampleMessage = 'This is a callout message!';
+
+  beforeEach(() => {
+    CalloutComponent = Vue.extend(callout);
+  });
+
+  it('should render the appropriate variant of callout', () => {
+    const component = new CalloutComponent({
+      propsData: {
+        category: 'info',
+        message: exampleMessage,
+      },
+    }).$mount();
+
+    expect(component.$el.getAttribute('class')).toEqual('bs-callout bs-callout-info');
+
+    expect(component.$el.tagName).toEqual('DIV');
+  });
+
+  it('should render accessibility attributes', () => {
+    const component = new CalloutComponent({
+      propsData: {
+        message: exampleMessage,
+      },
+    }).$mount();
+
+    expect(component.$el.getAttribute('role')).toEqual('alert');
+    expect(component.$el.getAttribute('aria-live')).toEqual('assertive');
+  });
+
+  it('should render the provided message', () => {
+    const component = new CalloutComponent({
+      propsData: {
+        message: exampleMessage,
+      },
+    }).$mount();
+
+    expect(component.$el.innerHTML.trim()).toEqual(exampleMessage);
+  });
+});
-- 
2.24.1


From 0000468eeb9ec80387fc3eae7df0fe52417ecc78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Lu=C3=ADs?= <aluis@gitlab.com>
Date: Fri, 13 Apr 2018 14:11:00 +0100
Subject: [PATCH 17/22] Fix karma tests of callout vue component

---
 .../vue_shared/components/callout_spec.js     | 44 +++++++++----------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/spec/javascripts/vue_shared/components/callout_spec.js b/spec/javascripts/vue_shared/components/callout_spec.js
index 5e452a20b09..e62bd86f4ca 100644
--- a/spec/javascripts/vue_shared/components/callout_spec.js
+++ b/spec/javascripts/vue_shared/components/callout_spec.js
@@ -1,45 +1,45 @@
 import Vue from 'vue';
 import callout from '~/vue_shared/components/callout.vue';
+import createComponent from 'spec/helpers/vue_mount_component_helper';
 
 describe('Callout Component', () => {
   let CalloutComponent;
+  let vm;
   const exampleMessage = 'This is a callout message!';
 
   beforeEach(() => {
     CalloutComponent = Vue.extend(callout);
   });
 
+  afterEach(() => {
+    vm.$destroy();
+  });
+
   it('should render the appropriate variant of callout', () => {
-    const component = new CalloutComponent({
-      propsData: {
-        category: 'info',
-        message: exampleMessage,
-      },
-    }).$mount();
+    vm = createComponent(CalloutComponent, {
+      category: 'info',
+      message: exampleMessage,
+    });
 
-    expect(component.$el.getAttribute('class')).toEqual('bs-callout bs-callout-info');
+    expect(vm.$el.getAttribute('class')).toEqual('bs-callout bs-callout-info');
 
-    expect(component.$el.tagName).toEqual('DIV');
+    expect(vm.$el.tagName).toEqual('DIV');
   });
 
   it('should render accessibility attributes', () => {
-    const component = new CalloutComponent({
-      propsData: {
-        message: exampleMessage,
-      },
-    }).$mount();
-
-    expect(component.$el.getAttribute('role')).toEqual('alert');
-    expect(component.$el.getAttribute('aria-live')).toEqual('assertive');
+    vm = createComponent(CalloutComponent, {
+      message: exampleMessage,
+    });
+
+    expect(vm.$el.getAttribute('role')).toEqual('alert');
+    expect(vm.$el.getAttribute('aria-live')).toEqual('assertive');
   });
 
   it('should render the provided message', () => {
-    const component = new CalloutComponent({
-      propsData: {
-        message: exampleMessage,
-      },
-    }).$mount();
+    vm = createComponent(CalloutComponent, {
+      message: exampleMessage,
+    });
 
-    expect(component.$el.innerHTML.trim()).toEqual(exampleMessage);
+    expect(vm.$el.innerHTML.trim()).toEqual(exampleMessage);
   });
 });
-- 
2.24.1


From 9b9b6334f84e899c4d78266230e56db5dd54c81b Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Fri, 13 Apr 2018 12:46:57 -0500
Subject: [PATCH 18/22] Moves callout_message and retry method outside status

- They were not describing status, rather than giving information about
  the build elements, so it's a better design to have them outside
JobSerializer#status
- Changes vue components respectively
- Also changes presenter methods to be more accurate
---
 .../javascripts/jobs/components/header.vue    |  4 +-
 .../jobs/components/sidebar_details_block.vue |  2 +-
 app/presenters/ci/build_detail_presenter.rb   |  8 +--
 app/serializers/job_entity.rb                 | 12 ++--
 spec/factories/ci/builds.rb                   |  5 ++
 spec/features/projects/jobs_spec.rb           |  8 ++-
 spec/javascripts/jobs/header_spec.js          |  2 +-
 .../ci/build_detail_presenter_spec.rb         |  6 +-
 spec/serializers/job_entity_spec.rb           | 69 +++++++++++++------
 9 files changed, 73 insertions(+), 43 deletions(-)

diff --git a/app/assets/javascripts/jobs/components/header.vue b/app/assets/javascripts/jobs/components/header.vue
index 154cf54eb73..21b545d6cab 100644
--- a/app/assets/javascripts/jobs/components/header.vue
+++ b/app/assets/javascripts/jobs/components/header.vue
@@ -33,7 +33,7 @@ export default {
       return !this.isLoading && Object.keys(this.job).length;
     },
     shouldRenderReason() {
-      return !!(this.job.status && this.job.status.callout_message);
+      return !!(this.job.status && this.job.callout_message);
     },
     /**
      * When job has not started the key will be `false`
@@ -88,7 +88,7 @@ export default {
 
     <callout
       v-if="shouldRenderReason"
-      :message="job.status.callout_message"
+      :message="job.callout_message"
     />
   </header>
 </template>
diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index 23c2dedcd76..4cd44bf7a76 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -50,7 +50,7 @@ export default {
     retryButtonClass() {
       let className = 'js-retry-button pull-right btn btn-retry visible-md-block visible-lg-block';
       className +=
-        this.job.status && this.job.status.retry_button
+        this.job.status && this.job.recoverable
           ? ' btn-primary'
           : ' btn-inverted-secondary';
       return className;
diff --git a/app/presenters/ci/build_detail_presenter.rb b/app/presenters/ci/build_detail_presenter.rb
index 7c43b2db67c..9111b9960da 100644
--- a/app/presenters/ci/build_detail_presenter.rb
+++ b/app/presenters/ci/build_detail_presenter.rb
@@ -15,14 +15,14 @@ module Ci
       CALLOUT_FAILURE_MESSAGES[failure_reason.to_sym]
     end
 
-    def show_retry_button?
-      try_again_failures.include?(failure_reason.to_sym)
+    def recoverable?
+      failed? && !unrecoverable?
     end
 
     private
 
-    def try_again_failures
-      CALLOUT_FAILURE_MESSAGES.keys - %i(script_failure missing_dependency_failure)
+    def unrecoverable?
+      script_failure? || missing_dependency_failure?
     end
   end
 end
diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb
index aa0955b1e27..e61c8aa1a8d 100644
--- a/app/serializers/job_entity.rb
+++ b/app/serializers/job_entity.rb
@@ -25,11 +25,9 @@ class JobEntity < Grape::Entity
   expose :playable?, as: :playable
   expose :created_at
   expose :updated_at
-  expose :status do
-    expose :callout_message, if: -> (*) { failed_or_allowed_to_fail? }
-    expose :retry_button, if: -> (*) { failed_or_allowed_to_fail? }
-    expose :detailed_status, merge: true, as: :status, with: StatusEntity
-  end
+  expose :detailed_status, as: :status, with: StatusEntity
+  expose :callout_message, if: -> (*) { failed_or_allowed_to_fail? }
+  expose :recoverable, if: -> (*) { failed_or_allowed_to_fail? }
 
   private
 
@@ -63,8 +61,8 @@ class JobEntity < Grape::Entity
     build_detail_presenter.callout_failure_message
   end
 
-  def retry_button
-    build_detail_presenter.show_retry_button?
+  def recoverable
+    build_detail_presenter.recoverable?
   end
 
   def build_detail_presenter
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index bca7e920de4..31f9f307fd5 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -242,5 +242,10 @@ FactoryBot.define do
       failed
       failure_reason 1
     end
+
+    trait :api_failure do
+      failed
+      failure_reason 2
+    end
   end
 end
diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb
index a460024542c..a00db6dd161 100644
--- a/spec/features/projects/jobs_spec.rb
+++ b/spec/features/projects/jobs_spec.rb
@@ -491,16 +491,18 @@ feature 'Jobs' do
     end
   end
 
-  describe "POST /:project/jobs/:id/retry" do
+  describe "POST /:project/jobs/:id/retry", :js do
     context "Job from project", :js do
       before do
         job.run!
+        job.cancel!
         visit project_job_path(project, job)
-        find('.js-cancel-job').click()
+        wait_for_requests
+
         find('.js-retry-button').click
       end
 
-      it 'shows the right status and buttons', :js do
+      it 'shows the right status and buttons' do
         page.within('aside.right-sidebar') do
           expect(page).to have_content 'Cancel'
         end
diff --git a/spec/javascripts/jobs/header_spec.js b/spec/javascripts/jobs/header_spec.js
index ac5dd33ab13..4f861c39d3f 100644
--- a/spec/javascripts/jobs/header_spec.js
+++ b/spec/javascripts/jobs/header_spec.js
@@ -50,7 +50,7 @@ describe('Job details header', () => {
     });
 
     it('should render the reason when reason is present', () => {
-      props.job.status.callout_message = 'There is an unknown failure, please try again';
+      props.job.callout_message = 'There is an unknown failure, please try again';
 
       vm = mountComponent(HeaderComponent, props);
 
diff --git a/spec/presenters/ci/build_detail_presenter_spec.rb b/spec/presenters/ci/build_detail_presenter_spec.rb
index 60f6ac39306..ad4ba9848a7 100644
--- a/spec/presenters/ci/build_detail_presenter_spec.rb
+++ b/spec/presenters/ci/build_detail_presenter_spec.rb
@@ -14,14 +14,14 @@ describe Ci::BuildDetailPresenter do
     end
   end
 
-  describe '#show_retry_button' do
+  describe '#recoverable?' do
     context 'when is a script or missing dependency failure' do
       let(:failure_reasons) { %w(script_failure missing_dependency_failure) }
 
       it 'should return false' do
         failure_reasons.each do |failure_reason|
           build.update_attribute(:failure_reason, failure_reason)
-          expect(presenter.show_retry_button?).to be_falsy
+          expect(presenter.recoverable?).to be_falsy
         end
       end
     end
@@ -32,7 +32,7 @@ describe Ci::BuildDetailPresenter do
       it 'should return true' do
         failure_reasons.each do |failure_reason|
           build.update_attribute(:failure_reason, failure_reason)
-          expect(presenter.show_retry_button?).to be_truthy
+          expect(presenter.recoverable?).to be_truthy
         end
       end
     end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index 8a0a645dced..c90396ebb28 100644
--- a/spec/serializers/job_entity_spec.rb
+++ b/spec/serializers/job_entity_spec.rb
@@ -133,40 +133,65 @@ describe JobEntity do
   context 'when job failed' do
     let(:job) { create(:ci_build, :script_failure) }
 
-    describe 'status' do
-      it 'should contain the failure reason inside label' do
-        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)')
-        expect(subject[:status][:callout_message]).to eq('There has been a script failure. Check the job log for more information')
-        expect(subject[:status][:retry_button]).to be_falsy
-      end
+    it 'contains details' do
+      expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
+    end
+
+    it 'states that it failed' do
+      expect(subject[:status][:label]).to eq('failed')
+    end
+
+    it 'should indicate the failure reason on tooltip' do
+      expect(subject[:status][:tooltip]).to eq('failed <br> (script failure)')
+    end
+
+    it 'should include a callout message with a verbose output' do
+      expect(subject[:callout_message]).to eq('There has been a script failure. Check the job log for more information')
+    end
+
+    it 'should state that it is not recoverable' do
+      expect(subject[:recoverable]).to be_falsy
     end
   end
 
   context 'when job is allowed to fail' do
     let(:job) { create(:ci_build, :allowed_to_fail, :script_failure) }
 
-    describe 'status' do
-      it 'should contain the failure reason inside label' do
-        expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
-        expect(subject[:status][:label]).to eq('failed (allowed to fail)')
-        expect(subject[:status][:tooltip]).to eq('failed <br> (script failure) (allowed to fail)')
-        expect(subject[:status][:callout_message]).to eq('There has been a script failure. Check the job log for more information')
-        expect(subject[:status][:retry_button]).to be_falsy
-      end
+    it 'contains details' do
+      expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
+    end
+
+    it 'states that it failed' do
+      expect(subject[:status][:label]).to eq('failed (allowed to fail)')
+    end
+
+    it 'should indicate the failure reason on tooltip' do
+      expect(subject[:status][:tooltip]).to eq('failed <br> (script failure) (allowed to fail)')
+    end
+
+    it 'should include a callout message with a verbose output' do
+      expect(subject[:callout_message]).to eq('There has been a script failure. Check the job log for more information')
+    end
+
+    it 'should state that it is not recoverable' do
+      expect(subject[:recoverable]).to be_falsy
+    end
+  end
+
+  context 'when job failed and is recoverable' do
+    let(:job) { create(:ci_build, :api_failure) }
+
+    it 'should state it is recoverable' do
+      expect(subject[:recoverable]).to be_truthy
     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')
-        expect(subject[:status]).not_to include('callout_message')
-        expect(subject[:status]).not_to include('retry_button')
-      end
+    it 'should not include callout message or recoverable keys' do
+      expect(subject).not_to include('callout_message')
+      expect(subject).not_to include('recoverable')
     end
   end
 end
-- 
2.24.1


From f7f4b65892dced4f84223507ec212d3750932117 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Mon, 16 Apr 2018 12:20:16 -0500
Subject: [PATCH 19/22] Removes BuildDetailPresenter

- This will allow us to keep a single source of truth
- Also display the callout message only to failed builds
---
 app/presenters/ci/build_detail_presenter.rb   | 28 -------------
 app/presenters/ci/build_presenter.rb          | 21 ++++++++++
 app/serializers/job_entity.rb                 | 10 ++---
 .../ci/build_detail_presenter_spec.rb         | 40 -------------------
 spec/presenters/ci/build_presenter_spec.rb    | 35 ++++++++++++++++
 5 files changed, 61 insertions(+), 73 deletions(-)
 delete mode 100644 app/presenters/ci/build_detail_presenter.rb
 delete mode 100644 spec/presenters/ci/build_detail_presenter_spec.rb

diff --git a/app/presenters/ci/build_detail_presenter.rb b/app/presenters/ci/build_detail_presenter.rb
deleted file mode 100644
index 9111b9960da..00000000000
--- a/app/presenters/ci/build_detail_presenter.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-module Ci
-  class BuildDetailPresenter < Gitlab::View::Presenter::Delegated
-    CALLOUT_FAILURE_MESSAGES = {
-      unknown_failure: 'There is an unknown failure, please try again',
-      script_failure: 'There has been a script failure. Check the job log for more information',
-      api_failure: 'There has been an API failure, please try again',
-      stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again',
-      runner_system_failure: 'There has been a runner system failure, please try again',
-      missing_dependency_failure: 'There has been a missing dependency failure, check the job log for more information'
-    }.freeze
-
-    presents :build
-
-    def callout_failure_message
-      CALLOUT_FAILURE_MESSAGES[failure_reason.to_sym]
-    end
-
-    def recoverable?
-      failed? && !unrecoverable?
-    end
-
-    private
-
-    def unrecoverable?
-      script_failure? || missing_dependency_failure?
-    end
-  end
-end
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 9afebda19be..4873d7ce662 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -1,5 +1,14 @@
 module Ci
   class BuildPresenter < Gitlab::View::Presenter::Delegated
+    CALLOUT_FAILURE_MESSAGES = {
+      unknown_failure: 'There is an unknown failure, please try again',
+      script_failure: 'There has been a script failure. Check the job log for more information',
+      api_failure: 'There has been an API failure, please try again',
+      stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again',
+      runner_system_failure: 'There has been a runner system failure, please try again',
+      missing_dependency_failure: 'There has been a missing dependency failure, check the job log for more information'
+    }.freeze
+
     presents :build
 
     def erased_by_user?
@@ -35,6 +44,14 @@ module Ci
       "#{subject.name} - #{detailed_status.status_tooltip}"
     end
 
+    def callout_failure_message
+      CALLOUT_FAILURE_MESSAGES[failure_reason.to_sym]
+    end
+
+    def recoverable?
+      failed? && !unrecoverable?
+    end
+
     private
 
     def tooltip_for_badge
@@ -44,5 +61,9 @@ module Ci
     def detailed_status
       @detailed_status ||= subject.detailed_status(user)
     end
+
+    def unrecoverable?
+      script_failure? || missing_dependency_failure?
+    end
   end
 end
diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb
index e61c8aa1a8d..9e2a4843559 100644
--- a/app/serializers/job_entity.rb
+++ b/app/serializers/job_entity.rb
@@ -26,8 +26,8 @@ class JobEntity < Grape::Entity
   expose :created_at
   expose :updated_at
   expose :detailed_status, as: :status, with: StatusEntity
-  expose :callout_message, if: -> (*) { failed_or_allowed_to_fail? }
-  expose :recoverable, if: -> (*) { failed_or_allowed_to_fail? }
+  expose :callout_message, if: -> (*) { failed? }
+  expose :recoverable, if: -> (*) { failed? }
 
   private
 
@@ -53,8 +53,8 @@ class JobEntity < Grape::Entity
     send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend
   end
 
-  def failed_or_allowed_to_fail?
-    build.failed? || build.allow_failure?
+  def failed?
+    build.failed?
   end
 
   def callout_message
@@ -66,6 +66,6 @@ class JobEntity < Grape::Entity
   end
 
   def build_detail_presenter
-    @build_detail_presenter ||= Ci::BuildDetailPresenter.new(build)
+    @build_detail_presenter ||= build.present
   end
 end
diff --git a/spec/presenters/ci/build_detail_presenter_spec.rb b/spec/presenters/ci/build_detail_presenter_spec.rb
deleted file mode 100644
index ad4ba9848a7..00000000000
--- a/spec/presenters/ci/build_detail_presenter_spec.rb
+++ /dev/null
@@ -1,40 +0,0 @@
-require 'spec_helper'
-
-describe Ci::BuildDetailPresenter do
-  let(:build) { create(:ci_build, :failed, :script_failure) }
-
-  subject(:presenter) do
-    described_class.new(build)
-  end
-
-  describe '#callout_failure_message' do
-    it 'returns a verbose failure reason' do
-      description = subject.callout_failure_message
-      expect(description).to eq('There has been a script failure. Check the job log for more information')
-    end
-  end
-
-  describe '#recoverable?' do
-    context 'when is a script or missing dependency failure' do
-      let(:failure_reasons) { %w(script_failure missing_dependency_failure) }
-
-      it 'should return false' do
-        failure_reasons.each do |failure_reason|
-          build.update_attribute(:failure_reason, failure_reason)
-          expect(presenter.recoverable?).to be_falsy
-        end
-      end
-    end
-
-    context 'when is any other failure type' do
-      let(:failure_reasons) { %w(unknown_failure api_failure stuck_or_timeout_failure runner_system_failure) }
-
-      it 'should return true' do
-        failure_reasons.each do |failure_reason|
-          build.update_attribute(:failure_reason, failure_reason)
-          expect(presenter.recoverable?).to be_truthy
-        end
-      end
-    end
-  end
-end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index cc16d0f156b..4bc005df2fc 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -217,4 +217,39 @@ describe Ci::BuildPresenter do
       end
     end
   end
+
+  describe '#callout_failure_message' do
+    let(:build) { create(:ci_build, :failed, :script_failure) }
+
+    it 'returns a verbose failure reason' do
+      description = subject.callout_failure_message
+      expect(description).to eq('There has been a script failure. Check the job log for more information')
+    end
+  end
+
+  describe '#recoverable?' do
+    let(:build) { create(:ci_build, :failed, :script_failure) }
+
+    context 'when is a script or missing dependency failure' do
+      let(:failure_reasons) { %w(script_failure missing_dependency_failure) }
+
+      it 'should return false' do
+        failure_reasons.each do |failure_reason|
+          build.update_attribute(:failure_reason, failure_reason)
+          expect(presenter.recoverable?).to be_falsy
+        end
+      end
+    end
+
+    context 'when is any other failure type' do
+      let(:failure_reasons) { %w(unknown_failure api_failure stuck_or_timeout_failure runner_system_failure) }
+
+      it 'should return true' do
+        failure_reasons.each do |failure_reason|
+          build.update_attribute(:failure_reason, failure_reason)
+          expect(presenter.recoverable?).to be_truthy
+        end
+      end
+    end
+  end
 end
-- 
2.24.1


From 673c874fe8d2fde301e0b51e9c86d93fbf7990bf Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Mon, 16 Apr 2018 12:39:59 -0500
Subject: [PATCH 20/22] Mention new form on seeing the failure reason on doc

---
 doc/ci/pipelines.md | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md
index f14e0527998..b16cbc61d14 100644
--- a/doc/ci/pipelines.md
+++ b/doc/ci/pipelines.md
@@ -75,7 +75,7 @@ cancel the job, retry it, or erase the job trace.
 
 ## Seeing the failure reason for jobs
 
-> [Introduced][ce-5742] in GitLab 10.7.
+> [Introduced][ce-17782] in GitLab 10.7.
 
 When a pipeline fails or is allowed to fail, there are several places where you
 can quickly check the reason it failed:
@@ -88,6 +88,8 @@ In any case, if you hover over the failed job you can see the reason it failed.
 
 ![Pipeline detail](img/job_failure_reason.png)
 
+From [GitLab 10.8][ce-17814] you can also see the reason it failed on the Job detail page.
+
 ## Pipeline graphs
 
 > [Introduced][ce-5742] in GitLab 8.11.
@@ -279,4 +281,5 @@ runners will not use regular runners, they must be tagged accordingly.
 [ce-7931]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7931
 [ce-9760]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9760
 [ce-17782]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17782
+[ce-17814]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17814
 [regexp]: https://gitlab.com/gitlab-org/gitlab-ce/blob/2f3dc314f42dbd79813e6251792853bc231e69dd/app/models/commit_status.rb#L99
-- 
2.24.1


From 2868a43fb68649f9e4321393ee80c6b6bf251736 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Tue, 17 Apr 2018 09:51:08 -0500
Subject: [PATCH 21/22] Only call present if the Build is not already presented

In some scenarios, build object is already presented so there's no need
to call the method again
---
 app/serializers/job_entity.rb       | 9 +++++----
 spec/serializers/job_entity_spec.rb | 4 ++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb
index 9e2a4843559..7f567435172 100644
--- a/app/serializers/job_entity.rb
+++ b/app/serializers/job_entity.rb
@@ -58,14 +58,15 @@ class JobEntity < Grape::Entity
   end
 
   def callout_message
-    build_detail_presenter.callout_failure_message
+    build_presenter.callout_failure_message
   end
 
   def recoverable
-    build_detail_presenter.recoverable?
+    build_presenter.recoverable?
   end
 
-  def build_detail_presenter
-    @build_detail_presenter ||= build.present
+  def build_presenter
+    @build_presenter ||=
+      build.is_a?(Ci::BuildPresenter) ? build : build.present(current_user: request.current_user)
   end
 end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index c90396ebb28..fb6d5915cd3 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, :script_failure) }
+    let(:job) { create(:ci_build, :script_failure).present }
 
     it 'contains details' do
       expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
@@ -155,7 +155,7 @@ describe JobEntity do
   end
 
   context 'when job is allowed to fail' do
-    let(:job) { create(:ci_build, :allowed_to_fail, :script_failure) }
+    let(:job) { create(:ci_build, :allowed_to_fail, :script_failure).present }
 
     it 'contains details' do
       expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
-- 
2.24.1


From 28f10c2eda21da59eb0ca16962bef619109a3e58 Mon Sep 17 00:00:00 2001
From: Mayra Cabrera <mcabrera@gitlab.com>
Date: Wed, 18 Apr 2018 10:28:32 -0500
Subject: [PATCH 22/22] Augments presenters factory with 'present' method

It'll simply return self for presented objects
---
 app/serializers/job_entity.rb               | 3 +--
 lib/gitlab/view/presenter/base.rb           | 4 ++++
 spec/lib/gitlab/view/presenter/base_spec.rb | 7 +++++++
 spec/serializers/job_entity_spec.rb         | 4 ++--
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb
index 7f567435172..3076fed1674 100644
--- a/app/serializers/job_entity.rb
+++ b/app/serializers/job_entity.rb
@@ -66,7 +66,6 @@ class JobEntity < Grape::Entity
   end
 
   def build_presenter
-    @build_presenter ||=
-      build.is_a?(Ci::BuildPresenter) ? build : build.present(current_user: request.current_user)
+    @build_presenter ||= build.present
   end
 end
diff --git a/lib/gitlab/view/presenter/base.rb b/lib/gitlab/view/presenter/base.rb
index 841fb681435..36162faa1eb 100644
--- a/lib/gitlab/view/presenter/base.rb
+++ b/lib/gitlab/view/presenter/base.rb
@@ -20,6 +20,10 @@ module Gitlab
           subject
         end
 
+        def present(**attributes)
+          self
+        end
+
         class_methods do
           def presenter?
             true
diff --git a/spec/lib/gitlab/view/presenter/base_spec.rb b/spec/lib/gitlab/view/presenter/base_spec.rb
index 32a946ca034..4eca53032a2 100644
--- a/spec/lib/gitlab/view/presenter/base_spec.rb
+++ b/spec/lib/gitlab/view/presenter/base_spec.rb
@@ -48,4 +48,11 @@ describe Gitlab::View::Presenter::Base do
       end
     end
   end
+
+  describe '#present' do
+    it 'returns self' do
+      presenter = presenter_class.new(build_stubbed(:project))
+      expect(presenter.present).to eq(presenter)
+    end
+  end
 end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index fb6d5915cd3..c90396ebb28 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, :script_failure).present }
+    let(:job) { create(:ci_build, :script_failure) }
 
     it 'contains details' do
       expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
@@ -155,7 +155,7 @@ describe JobEntity do
   end
 
   context 'when job is allowed to fail' do
-    let(:job) { create(:ci_build, :allowed_to_fail, :script_failure).present }
+    let(:job) { create(:ci_build, :allowed_to_fail, :script_failure) }
 
     it 'contains details' do
       expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
-- 
2.24.1