Resolve "Show `failure_reason` and improve failed jobs tab in pipeline detail view"

6 files changed, 220 insertions, 25 deletions

diff --git a/app/assets/stylesheets/framework/tables.scss b/app/assets/stylesheets/framework/tables.scss
index 10c23f6c407..6e1758d7677 100644
--- a/app/assets/stylesheets/framework/tables.scss
+++ b/app/assets/stylesheets/framework/tables.scss
@@ -39,6 +39,11 @@ table {
         &.wide {
           width: 55%;
         }
+
+        &.table-th-transparent {
+          background: none;
+          color: $gl-text-color-secondary;
+        }
       }
 
       td {
@@ -46,9 +51,86 @@ table {
       }
     }
   }
+
+  &.responsive-table {
+    @include media-breakpoint-down(sm) {
+      thead {
+        display: none;
+      }
+
+      td {
+        display: block;
+        color: $gl-text-color-secondary;
+      }
+
+      tbody td.responsive-table-cell {
+        padding: $gl-padding 0;
+        width: 100%;
+        display: flex;
+        text-align: right;
+        align-items: center;
+        justify-content: space-between;
+
+        &[data-column]::before {
+          content: attr(data-column);
+          display: block;
+          text-align: left;
+          padding-right: $gl-padding;
+          color: $gl-text-color-secondary;
+        }
+
+        &:not([data-column]) {
+          flex-direction: row-reverse;
+        }
+      }
+
+      tr.responsive-table-border-start,
+      tr.responsive-table-border-end {
+        display: block;
+        border: solid $gl-text-color-quaternary;
+        padding-left: 0;
+        padding-right: 0;
+
+        > td {
+          border-color: $gl-text-color-quaternary;
+
+          &,
+          &:last-child {
+            padding-left: $gl-padding;
+            padding-right: $gl-padding;
+          }
+        }
+      }
+
+      tr.responsive-table-border-start {
+        border-width: 1px 1px 0;
+        border-radius: $border-radius-default $border-radius-default 0 0;
+        padding-top: 0;
+        padding-bottom: 0;
+
+        > td:first-child {
+          border-top: 0; // always have the <table> top border
+        }
+
+        > td:last-child {
+          border-bottom: 1px solid $gl-text-color-quaternary;
+        }
+      }
+
+      tr.responsive-table-border-end {
+        border-width: 0 1px 1px;
+        border-radius: 0 0 $border-radius-default $border-radius-default;
+        margin-bottom: 2 * $gl-padding;
+
+        > :last-child {
+          border-bottom: 0;
+        }
+      }
+    }
+  }
 }
 
-.responsive-table {
+.responsive-table:not(table) {
   @include media-breakpoint-down(sm) {
     th {
       width: 100%;
diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss
index 9213ccd4cdf..f030189af06 100644
--- a/app/assets/stylesheets/pages/builds.scss
+++ b/app/assets/stylesheets/pages/builds.scss
@@ -12,26 +12,22 @@
 @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);
+    box-shadow: 12px 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);
+    box-shadow: 12px 0 0 0 rgba($white-light, 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);
+    box-shadow: 12px 0 0 0 rgba($white-light, 0.2), 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);
+    box-shadow: 12px 0 0 0 rgba($white-light, 0.2), 24px 0 0 0 rgba($white-light, 0.2);
   }
 }
 
@@ -71,6 +67,10 @@
     .bash {
       display: block;
     }
+
+    &.build-trace-rounded {
+      border-radius: $border-radius-base;
+    }
   }
 
   .top-bar {
diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss
index f85f66b9c0b..30428fd198d 100644
--- a/app/assets/stylesheets/pages/pipelines.scss
+++ b/app/assets/stylesheets/pages/pipelines.scss
@@ -321,18 +321,17 @@
 }
 
 .build-failures {
+  th {
+    border-top: 0;
+  }
+
   .build-state {
     padding: 20px 2px;
 
     .build-name {
-      float: right;
       font-weight: $gl-font-weight-normal;
     }
 
-    .ci-status-icon-failed svg {
-      vertical-align: middle;
-    }
-
     .stage {
       color: $gl-text-color-secondary;
       font-weight: $gl-font-weight-normal;
@@ -344,6 +343,81 @@
     border: 0;
     line-height: initial;
   }
+
+  .build-trace-row td {
+    border-top: 0;
+    border-bottom-width: 1px;
+    border-bottom-style: solid;
+    padding-top: 0;
+  }
+
+  .build-trace {
+    width: 100%;
+    text-align: left;
+    margin-top: $gl-padding;
+  }
+
+  .build-name {
+    width: 196px;
+
+    a {
+      font-weight: $gl-font-weight-bold;
+      color: $gl-text-color;
+      text-decoration: none;
+
+      &:focus,
+      &:hover {
+        text-decoration: underline;
+      }
+    }
+  }
+
+  .build-actions {
+    width: 70px;
+    text-align: right;
+  }
+
+  .build-stage {
+    width: 140px;
+  }
+
+  .ci-status-icon-failed {
+    padding: 10px 0 10px 12px;
+    width: 12px + 24px; // padding-left + svg width
+  }
+
+  .build-icon svg {
+    width: 24px;
+    height: 24px;
+    vertical-align: middle;
+  }
+
+  .build-state,
+  .build-trace-row {
+    > td:last-child {
+      padding-right: 0;
+    }
+  }
+
+  @include media-breakpoint-down(sm) {
+    td:empty {
+      display: none;
+    }
+
+    .ci-table {
+      margin-top: 2 * $gl-padding;
+    }
+
+    .build-trace-container {
+      padding-top: $gl-padding;
+      padding-bottom: $gl-padding;
+    }
+
+    .build-trace {
+      margin-bottom: 0;
+      margin-top: 0;
+    }
+  }
 }
 
 .pipeline-tab-content {
@@ -929,7 +1003,7 @@ button.mini-pipeline-graph-dropdown-toggle {
   &.dropdown-menu {
     transform: translate(-80%, 0);
 
-    @media(min-width: map-get($grid-breakpoints, md)) {
+    @media (min-width: map-get($grid-breakpoints, md)) {
       transform: translate(-50%, 0);
       right: auto;
       left: 50%;
diff --git a/app/views/projects/pipelines/_with_tabs.html.haml b/app/views/projects/pipelines/_with_tabs.html.haml
index 118391aac64..951f80b378d 100644
--- a/app/views/projects/pipelines/_with_tabs.html.haml
+++ b/app/views/projects/pipelines/_with_tabs.html.haml
@@ -1,7 +1,7 @@
 .tabs-holder
   %ul.pipelines-tabs.nav-links.no-top.no-bottom.mobile-separator.nav.nav-tabs
     %li.js-pipeline-tab-link
-      = link_to project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-pipeline', action: 'pipelines', toggle: 'tab' },  class: 'pipeline-tab' do
+      = link_to project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-pipeline', action: 'pipelines', toggle: 'tab' }, class: 'pipeline-tab' do
         = _("Pipeline")
     %li.js-builds-tab-link
       = link_to builds_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-builds', action: 'builds', toggle: 'tab' }, class: 'builds-tab' do
@@ -43,12 +43,36 @@
         = render partial: "projects/stage/stage", collection: pipeline.legacy_stages, as: :stage
 
   - if @pipeline.failed_builds.present?
-    #js-tab-failures.build-failures.tab-pane
-      - @pipeline.failed_builds.each_with_index do |build, index|
-        .build-state
-          %span.ci-status-icon-failed= custom_icon('icon_status_failed')
-          %span.stage
-            = build.stage.titleize
-          %span.build-name
-            = link_to build.name, pipeline_job_url(pipeline, build)
-        %pre.build-log= build_summary(build, skip: index >= 10)
+    #js-tab-failures.build-failures.tab-pane.build-page
+      %table.table.responsive-table.ci-table.responsive-table-sm-rounded
+        %thead
+          %th.table-th-transparent
+          %th.table-th-transparent= _("Name")
+          %th.table-th-transparent= _("Stage")
+          %th.table-th-transparent= _("Failure")
+
+        %tbody
+          - @pipeline.failed_builds.each_with_index do |build, index|
+            - job = build.present(current_user: current_user)
+            %tr.build-state.responsive-table-border-start
+              %td.responsive-table-cell.ci-status-icon-failed{ data: { column: "Status"} }
+                .d-none.d-md-block.build-icon
+                  = custom_icon("icon_status_#{build.status}")
+                .d-md-none.build-badge
+                  = render "ci/status/badge", link: false, status: job.detailed_status(current_user)
+              %td.responsive-table-cell.build-name{ data: { column: _("Name")} }
+                = link_to build.name, pipeline_job_url(pipeline, build)
+              %td.responsive-table-cell.build-stage{ data: { column: _("Stage")} }
+                = build.stage.titleize
+              %td.responsive-table-cell.build-failure{ data: { column: _("Failure")} }
+                = build.present.callout_failure_message
+              %td.responsive-table-cell.build-actions
+                = link_to retry_project_job_path(build.project, build, return_to: request.original_url), method: :post, title: _('Retry'), class: 'btn btn-build' do
+                  = icon('repeat')
+            %tr.build-trace-row.responsive-table-border-end
+              %td
+              %td.responsive-table-cell.build-trace-container{ colspan: 4 }
+                %pre.build-trace.build-trace-rounded
+                  %code.bash.js-build-output
+                    = build_summary(build)
+
diff --git a/changelogs/unreleased/44267-improve-failed-jobs-tab.yml b/changelogs/unreleased/44267-improve-failed-jobs-tab.yml
new file mode 100644
index 00000000000..9743704e23d
--- /dev/null
+++ b/changelogs/unreleased/44267-improve-failed-jobs-tab.yml
@@ -0,0 +1,5 @@
+---
+title: Improve Failed Jobs tab in the Pipeline detail page
+merge_request:
+author:
+type: changed
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 35776a5f23b..ecc7cf84138 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -344,6 +344,16 @@ describe 'Pipeline', :js do
       it 'shows build failure logs' do
         expect(page).to have_content('4 examples, 1 failure')
       end
+
+      it 'shows the failure reason' do
+        expect(page).to have_content('There is an unknown failure, please try again')
+      end
+
+      it 'shows retry button for failed build' do
+        page.within(find('.build-failures', match: :first)) do
+          expect(page).to have_link('Retry')
+        end
+      end
     end
 
     context 'when missing build logs' do