From c3c9122d1e6aa532ad213394c4758653d4cf3874 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 5 Dec 2016 17:04:36 +0000 Subject: [PATCH 01/47] Adds new hover state Fixes the line that connects the dots Adds style for the badge Add new style for status text Fix badge style Adjust font weight --- .../stylesheets/framework/variables.scss | 7 ++++ app/assets/stylesheets/pages/pipelines.scss | 34 ++++++++++++++----- .../commit/_pipeline_status_group.html.haml | 2 +- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 18716813c48..0591801d259 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -571,3 +571,10 @@ $body-text-shadow: rgba(255,255,255,0.01); */ $ui-dev-kit-example-color: #bbb; $ui-dev-kit-example-border: #ddd; + +/* +Pipeline Graph +*/ +$stage-hover-bg: #eaf3fc; +$stage-hover-border: #d1e7fc; +$stage-bagde-text: #d4d4d4; diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 08062b85504..cc8d4dd9544 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -363,15 +363,22 @@ .build { border: 1px solid $border-color; + border-radius: 30px; background-color: $white-light; + color: $gl-text-color; position: relative; padding: 7px 10px 8px; - border-radius: 30px; width: 186px; margin-bottom: 10px; &:hover { - background-color: $gray-lighter; + background-color: $stage-hover-bg; + border: 1px solid $stage-hover-border; + + .ci-status-text, + .dropdown-counter-bagde { + color: $gl-text-color; + } } &.playable { @@ -411,14 +418,14 @@ } .ci-status-text { - width: 135px; + max-width: 110px; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; - vertical-align: middle; + vertical-align: bottom; display: inline-block; position: relative; - top: -1px; + font-weight: 100; } a { @@ -435,7 +442,7 @@ flex-grow: 1; .ci-status-text { - max-width: 112px; + max-width: 110px; width: auto; } } @@ -480,7 +487,7 @@ } .ci-status-text { - width: 112px; + width: 110px; } .arrow { @@ -520,9 +527,18 @@ } } + .dropdown-counter-bagde { + float: right; + color: $stage-bagde-text; + font-weight: 100; + font-size: 13px; + margin-top: 1px; + margin-right: 2px; + } + svg { vertical-align: middle; - margin-right: 5px; + margin-right: 3px; } // Connect first build in each stage with right horizontal line @@ -531,7 +547,7 @@ content: ''; position: absolute; top: 48%; - right: -48px; + right: -49px; border-top: 2px solid $border-color; width: 48px; height: 1px; diff --git a/app/views/projects/commit/_pipeline_status_group.html.haml b/app/views/projects/commit/_pipeline_status_group.html.haml index 2b26ad9d6fa..1e91e249fe9 100644 --- a/app/views/projects/commit/_pipeline_status_group.html.haml +++ b/app/views/projects/commit/_pipeline_status_group.html.haml @@ -4,7 +4,7 @@ = ci_icon_for_status(group_status) %span.ci-status-text = name - %span.badge= subject.size + %span.dropdown-counter-bagde= subject.size .dropdown-menu.grouped-pipeline-dropdown .arrow %ul -- 2.24.1 From f9c103c2f314a2f9edbdfb93a26ace597d62e7e6 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 6 Dec 2016 11:56:50 +0000 Subject: [PATCH 02/47] Fix tooltip to show the all name CSS - Changes to look like newest mockups - Simplifies nested elements - Divides nodes from lines Remove is playable from left side Remove nested elements in scss Improve dropdown Focus state Fix scss linter Remove not used css Fix typo --- app/assets/stylesheets/pages/pipelines.scss | 498 ++++++++---------- .../ci/builds/_build_pipeline.html.haml | 6 +- .../commit/_pipeline_status_group.html.haml | 2 +- 3 files changed, 231 insertions(+), 275 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index cc8d4dd9544..cd7df7beda8 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -287,15 +287,40 @@ } // Pipeline visualization +.pipeline-actions { + border-bottom: none; +} -.toggle-pipeline-btn { - background-color: $gray-dark; +.tab-pane { + &.pipelines { + .ci-table { + min-width: 900px; + } - &.graph-collapsed { - background-color: $white-light; + .content-list.pipelines { + overflow: auto; + } + + .stage { + max-width: 100px; + width: 100px; + } + + .pipeline-actions { + min-width: initial; + } + } + + &.builds { + .ci-table { + tr { + height: 71px; + } + } } } +// Pipeline graph .pipeline-graph { width: 100%; background-color: $background-color; @@ -304,52 +329,120 @@ white-space: nowrap; transition: max-height 0.3s, padding 0.3s; - &.graph-collapsed { - max-height: 0; - padding: 0 16px; - } -} - -.pipeline-visualization { - position: relative; - ul { padding: 0; } -} -.stage-column { - display: inline-block; - vertical-align: top; + a { + text-decoration: none; + color: $gl-text-color-light; + } - &:not(:last-child) { - margin-right: 44px; + svg { + vertical-align: middle; + margin-right: 3px; } - &.left-margin { - &:not(:first-child) { - margin-left: 44px; + .stage-column { + display: inline-block; + vertical-align: top; - .left-connector { - &::before { - content: ''; - position: absolute; - top: 48%; - left: -48px; - border-top: 2px solid $border-color; - width: 48px; - height: 1px; + &:not(:last-child) { + margin-right: 44px; + } + + &.left-margin { + &:not(:first-child) { + margin-left: 44px; + + .left-connector { + &::before { + content: ''; + position: absolute; + top: 48%; + left: -48px; + border-top: 2px solid $border-color; + width: 48px; + height: 1px; + } } } } - } - &.no-margin { - margin: 0; - } + &.no-margin { + margin: 0; + } - li { - list-style: none; + li { + list-style: none; + } + + &:last-child { + .build { + // Remove right connecting horizontal line from first build in last stage + &:first-child { + &::after { + border: none; + } + } + // Remove right curved connectors from all builds in last stage + &:not(:first-child) { + &::after { + border: none; + } + } + // Remove opposite curve + .curve { + &::before { + display: none; + } + } + } + } + + &:first-child { + .build { + // Remove left curved connectors from all builds in first stage + &:not(:first-child) { + &::before { + border: none; + } + } + // Remove opposite curve + .curve { + &::after { + display: none; + } + } + } + } + + // Curve first child connecting lines in opposite direction + .curve { + display: none; + + &::before, + &::after { + content: ''; + width: 21px; + height: 25px; + position: absolute; + top: -31px; + border-top: 2px solid $border-color; + } + + &::after { + left: -44px; + border-right: 2px solid $border-color; + border-radius: 0 20px; + } + + &::before { + right: -44px; + border-left: 2px solid $border-color; + border-radius: 20px 0 0; + } + } } .stage-name { @@ -365,24 +458,12 @@ border: 1px solid $border-color; border-radius: 30px; background-color: $white-light; - color: $gl-text-color; position: relative; - padding: 7px 10px 8px; + padding: 8px 10px 9px; width: 186px; margin-bottom: 10px; - &:hover { - background-color: $stage-hover-bg; - border: 1px solid $stage-hover-border; - - .ci-status-text, - .dropdown-counter-bagde { - color: $gl-text-color; - } - } - &.playable { - svg { height: 13px; width: 20px; @@ -395,150 +476,56 @@ } } - .build-content { - display: -ms-flexbox; - display: -webkit-flex; - display: flex; - width: 164px; - - .ci-status-icon { - svg { - height: 20px; - width: 20px; - } - } - - .tooltip { - white-space: nowrap; - - .tooltip-inner { - overflow: hidden; - text-overflow: ellipsis; - } - } - - .ci-status-text { - max-width: 110px; - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - vertical-align: bottom; - display: inline-block; - position: relative; - font-weight: 100; - } - - a { - color: $gl-text-color-light; - text-decoration: none; - } + &:hover { + background-color: $stage-hover-bg; + border: 1px solid $stage-hover-border; + a, + .dropdown-counter-badge, .dropdown-menu-toggle { - background-color: transparent; - border: none; - width: auto; - padding: 0; - color: $gl-text-color-light; - flex-grow: 1; - - .ci-status-text { - max-width: 110px; - width: auto; - } + color: $gl-text-color; } - .grouped-pipeline-dropdown { - padding: 0; - width: 186px; - left: auto; - right: -197px; - top: -9px; - - ul { - max-height: 245px; - overflow: auto; - - li:first-child { - padding-top: 8px; - } - - li:last-child { - padding-bottom: 8px; - } - } + .grouped-pipeline-dropdown a { + color: $gl-text-color-light; - a { + &:hover { color: $gl-text-color; - padding: 7px 8px 8px; - - &:hover { - background-color: $blue-light-transparent; - border-radius: 3px; - - .ci-status-text { - text-decoration: none; - } - } - } - - svg { - width: 14px; - height: 14px; - } - - .ci-status-text { - width: 110px; } + } + } - .arrow { - &::before, - &::after { - content: ''; - display: inline-block; - position: absolute; - width: 0; - height: 0; - border-color: transparent; - border-style: solid; - top: 18px; - } - - &::before { - left: -5px; - margin-top: -6px; - border-width: 7px 5px 7px 0; - border-right-color: $border-color; - } + .ci-status-icon svg { + height: 20px; + width: 20px; + } - &::after { - left: -4px; - margin-top: -9px; - border-width: 10px 7px 10px 0; - border-right-color: $white-light; - } - } + .arrow { + &::before, + &::after { + content: ''; + display: inline-block; + position: absolute; + width: 0; + height: 0; + border-color: transparent; + border-style: solid; + top: 18px; } - .badge { - background-color: $gray-darker; - color: $gl-text-color-light; - font-weight: normal; - margin-left: $btn-xs-side-margin; + &::before { + left: -5px; + margin-top: -6px; + border-width: 7px 5px 7px 0; + border-right-color: $border-color; } - } - - .dropdown-counter-bagde { - float: right; - color: $stage-bagde-text; - font-weight: 100; - font-size: 13px; - margin-top: 1px; - margin-right: 2px; - } - svg { - vertical-align: middle; - margin-right: 3px; + &::after { + left: -4px; + margin-top: -9px; + border-width: 10px 7px 10px 0; + border-right-color: $white-light; + } } // Connect first build in each stage with right horizontal line @@ -595,113 +582,86 @@ } } - &:last-child { - .build { - // Remove right connecting horizontal line from first build in last stage - &:first-child { - &::after { - border: none; - } - } - // Remove right curved connectors from all builds in last stage - &:not(:first-child) { - &::after { - border: none; - } - } - // Remove opposite curve - .curve { - &::before { - display: none; - } - } - } - } - - &:first-child { - .build { - // Remove left curved connectors from all builds in first stage - &:not(:first-child) { - &::before { - border: none; - } - } - // Remove opposite curve - .curve { - &::after { - display: none; - } - } - } + .ci-status-text { + max-width: 110px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + vertical-align: bottom; + display: inline-block; + position: relative; + font-weight: 100; } - // Curve first child connecting lines in opposite direction - .curve { - display: none; + .dropdown-menu-toggle { + background-color: transparent; + border: none; + padding: 0; + color: $gl-text-color-light; + flex-grow: 1; - &::before, - &::after { - content: ''; - width: 21px; - height: 25px; - position: absolute; - top: -32px; - border-top: 2px solid $border-color; + &:focus { + outline: none; } - &::after { - left: -44px; - border-right: 2px solid $border-color; - border-radius: 0 20px; - } + &:hover { + color: $gl-text-color; - &::before { - right: -44px; - border-left: 2px solid $border-color; - border-radius: 20px 0 0; + .dropdown-counter-bagde { + color: $gl-text-color; + } } } -} -.pipeline-actions { - border-bottom: none; -} - -.toggle-pipeline-btn { - - .fa { - color: $dropdown-header-color; + .dropdown-counter-bagde { + float: right; + color: $stage-bagde-text; + font-weight: 100; + font-size: 13px; + margin-top: 1px; + margin-right: 2px; } -} -.tab-pane { + .grouped-pipeline-dropdown { + padding: 0; + width: 191px; + left: auto; + right: -206px; + top: -11px; + box-shadow: 0 1px 5px $black-transparent; + + ul { + max-height: 245px; + overflow: auto; - &.pipelines { + li { + padding-top: 1px; + padding-bottom: 1px; + } - .ci-table { - min-width: 900px; - } + li:first-child { + padding-top: 9px; + } - .content-list.pipelines { - overflow: auto; + li:last-child { + padding-bottom: 9px; + } } - .stage { - max-width: 100px; - width: 100px; - } + a { + color: $gl-text-color-light; + padding: 7px 8px 8px; - .pipeline-actions { - min-width: initial; + &:hover { + background-color: $stage-hover-bg; + border-radius: 3px; + color: $gl-text-color; + } } - } - - &.builds { - .ci-table { - tr { - height: 71px; - } + .ci-status-icon svg { + height: 18px; + width: 18px; } } } diff --git a/app/views/projects/ci/builds/_build_pipeline.html.haml b/app/views/projects/ci/builds/_build_pipeline.html.haml index 423a1282eb2..ac90d3278e2 100644 --- a/app/views/projects/ci/builds/_build_pipeline.html.haml +++ b/app/views/projects/ci/builds/_build_pipeline.html.haml @@ -1,9 +1,5 @@ - is_playable = subject.playable? && can?(current_user, :update_build, @project) -- if is_playable - = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, data: { toggle: 'tooltip', title: "#{subject.name} - play", container: '.pipeline-graph', placement: 'bottom' } do - = ci_icon_for_status('play') - .ci-status-text= subject.name -- elsif can?(current_user, :read_build, @project) +- if can?(current_user, :read_build, @project) = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject), data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } do %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} = ci_icon_for_status(subject.status) diff --git a/app/views/projects/commit/_pipeline_status_group.html.haml b/app/views/projects/commit/_pipeline_status_group.html.haml index 1e91e249fe9..8b782d38193 100644 --- a/app/views/projects/commit/_pipeline_status_group.html.haml +++ b/app/views/projects/commit/_pipeline_status_group.html.haml @@ -4,7 +4,7 @@ = ci_icon_for_status(group_status) %span.ci-status-text = name - %span.dropdown-counter-bagde= subject.size + %span.dropdown-counter-badge= subject.size .dropdown-menu.grouped-pipeline-dropdown .arrow %ul -- 2.24.1 From bd30f75af67240d4069d4a4559faecf4ad7fab5a Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 7 Dec 2016 18:03:40 +0000 Subject: [PATCH 03/47] Adds actions to the nodes Improve CSS for dropdown actions --- .../stylesheets/framework/variables.scss | 2 +- app/assets/stylesheets/pages/pipelines.scss | 84 ++++++++++++++----- .../ci/builds/_build_pipeline.html.haml | 22 +++++ .../commit/_pipeline_status_group.html.haml | 2 +- 4 files changed, 86 insertions(+), 24 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 0591801d259..cfb2ed9321b 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -577,4 +577,4 @@ Pipeline Graph */ $stage-hover-bg: #eaf3fc; $stage-hover-border: #d1e7fc; -$stage-bagde-text: #d4d4d4; +$stage-badge-text: #d4d4d4; diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index cd7df7beda8..81cd397ef14 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -463,19 +463,6 @@ width: 186px; margin-bottom: 10px; - &.playable { - svg { - height: 13px; - width: 20px; - position: relative; - top: 1px; - - path { - fill: $layout-link-gray; - } - } - } - &:hover { background-color: $stage-hover-bg; border: 1px solid $stage-hover-border; @@ -607,15 +594,15 @@ &:hover { color: $gl-text-color; - .dropdown-counter-bagde { + .dropdown-counter-badge { color: $gl-text-color; } } } - .dropdown-counter-bagde { + .dropdown-counter-badge { float: right; - color: $stage-bagde-text; + color: $stage-badge-text; font-weight: 100; font-size: 13px; margin-top: 1px; @@ -630,27 +617,49 @@ top: -11px; box-shadow: 0 1px 5px $black-transparent; + a { + display: inline-block; + + &:hover { + background-color: $stage-hover-bg; + } + } + ul { max-height: 245px; overflow: auto; li { - padding-top: 1px; - padding-bottom: 1px; + padding-top: 2px; + margin: 0 5px; } li:first-child { - padding-top: 9px; + padding-top: 6px; } li:last-child { - padding-bottom: 9px; + padding-bottom: 6px; } } - a { + .dropdown-build { color: $gl-text-color-light; - padding: 7px 8px 8px; + + a { + padding: 7px 8px 8px; + } + + a.ci-action-icon-container { + padding: 0; + font-size: 11px; + float: right; + margin-top: 5px; + + i { + font-size: 11px; + } + } &:hover { background-color: $stage-hover-bg; @@ -665,3 +674,34 @@ } } } + +// Action Icons +.ci-action-icon-container { + padding: 0; + + .ci-action-icon-wrapper { + display: inline-block; + float: right; + + i { + color: $stage-badge-text; + border-radius: 100%; + border: 1px solid $stage-badge-text; + text-align: center; + display: table-cell; + vertical-align: middle; + padding: 5px; + font-size: 13px; + + &:hover { + color: $gl-text-color; + background-color: $stage-hover-bg; + border: 1px solid $gl-text-color; + } + } + + .ci-play-icon { + padding: 5px 4px 5px 7px; + } + } +} diff --git a/app/views/projects/ci/builds/_build_pipeline.html.haml b/app/views/projects/ci/builds/_build_pipeline.html.haml index ac90d3278e2..41b9265fe5e 100644 --- a/app/views/projects/ci/builds/_build_pipeline.html.haml +++ b/app/views/projects/ci/builds/_build_pipeline.html.haml @@ -1,9 +1,31 @@ - is_playable = subject.playable? && can?(current_user, :update_build, @project) +- can_cancel = subject.active? && can?(current_user, :update_build, @project) +- can_retry = subject.retryable? && can?(current_user, :update_build, @project) +- can_stop = subject.complete? && subject.stops_environment? && can?(current_user, :update_build, @project) + - if can?(current_user, :read_build, @project) = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject), data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } do %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} = ci_icon_for_status(subject.status) .ci-status-text= subject.name + + - if is_playable + = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: "#{subject.name} - Play", class: 'ci-action-icon-container' do + %i.ci-action-icon-wrapper + = icon('play', class: 'ci-play-icon') + - elsif can_cancel + = link_to cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: "#{subject.name} - Cancel", class: 'ci-action-icon-container' do + %i.ci-action-icon-wrapper + = icon('ban') + - elsif can_retry + = link_to retry_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: "#{subject.name} - Retry", class: 'ci-action-icon-container' do + %i.ci-action-icon-wrapper + = icon('refresh') + - elsif can_stop + = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: "#{subject.name} - Stop", class: 'ci-action-icon-container' do + %i.ci-action-icon-wrapper + = icon('stop') + - else %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} = ci_icon_for_status(subject.status) diff --git a/app/views/projects/commit/_pipeline_status_group.html.haml b/app/views/projects/commit/_pipeline_status_group.html.haml index 8b782d38193..2d198d1b389 100644 --- a/app/views/projects/commit/_pipeline_status_group.html.haml +++ b/app/views/projects/commit/_pipeline_status_group.html.haml @@ -9,5 +9,5 @@ .arrow %ul - subject.each do |status| - %li + %li.dropdown-build = render "projects/#{status.to_partial_path}_pipeline", subject: status -- 2.24.1 From 2320a5c7118084ed999b9fa0966c3315b94edd67 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 7 Dec 2016 20:28:29 +0000 Subject: [PATCH 04/47] Adds new icons for pipeline graph Fix padding Fix hover state of icon --- app/assets/stylesheets/pages/pipelines.scss | 5 +- app/helpers/ci_status_helper.rb | 66 ++++++++++++------- .../icons/_icon_graph_job_cancelled.svg | 1 + .../shared/icons/_icon_graph_job_created.svg | 1 + .../shared/icons/_icon_graph_job_failed.svg | 1 + .../shared/icons/_icon_graph_job_manual.svg | 1 + .../shared/icons/_icon_graph_job_pending.svg | 1 + .../shared/icons/_icon_graph_job_running.svg | 1 + .../shared/icons/_icon_graph_job_skipped.svg | 1 + .../shared/icons/_icon_graph_job_success.svg | 1 + .../shared/icons/_icon_graph_job_warning.svg | 1 + 11 files changed, 56 insertions(+), 24 deletions(-) create mode 100755 app/views/shared/icons/_icon_graph_job_cancelled.svg create mode 100755 app/views/shared/icons/_icon_graph_job_created.svg create mode 100755 app/views/shared/icons/_icon_graph_job_failed.svg create mode 100755 app/views/shared/icons/_icon_graph_job_manual.svg create mode 100755 app/views/shared/icons/_icon_graph_job_pending.svg create mode 100755 app/views/shared/icons/_icon_graph_job_running.svg create mode 100755 app/views/shared/icons/_icon_graph_job_skipped.svg create mode 100755 app/views/shared/icons/_icon_graph_job_success.svg create mode 100755 app/views/shared/icons/_icon_graph_job_warning.svg diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 81cd397ef14..06b078d5345 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -647,7 +647,7 @@ color: $gl-text-color-light; a { - padding: 7px 8px 8px; + padding: 6px 8px 7px; } a.ci-action-icon-container { @@ -692,11 +692,12 @@ vertical-align: middle; padding: 5px; font-size: 13px; + background: $white-light; &:hover { color: $gl-text-color; background-color: $stage-hover-bg; - border: 1px solid $gl-text-color; + border: 1px solid $stage-hover-bg; } } diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 8e19752a8a1..999d279e5b9 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -43,32 +43,54 @@ module CiStatusHelper status.humanize end - def ci_icon_for_status(status) + def ci_icon_for_status(status, graph: nil) if detailed_status?(status) return custom_icon(status.icon) end - icon_name = - case status - when 'success' - 'icon_status_success' - when 'success_with_warnings' - 'icon_status_warning' - when 'failed' - 'icon_status_failed' - when 'pending' - 'icon_status_pending' - when 'running' - 'icon_status_running' - when 'play' - 'icon_play' - when 'created' - 'icon_status_created' - when 'skipped' - 'icon_status_skipped' - else - 'icon_status_canceled' - end + if graph + icon_name = + case status + when 'success' + 'icon_graph_job_success' + when 'success_with_warnings' + 'icon_graph_job_warning' + when 'failed' + 'icon_graph_job_failed' + when 'pending' + 'icon_graph_job_pending' + when 'running' + 'icon_graph_job_running' + when 'created' + 'icon_graph_job_created' + when 'skipped' + 'icon_graph_job_skipped' + else + 'icon_graph_job_canceled' + end + else + icon_name = + case status + when 'success' + 'icon_status_success' + when 'success_with_warnings' + 'icon_status_warning' + when 'failed' + 'icon_status_failed' + when 'pending' + 'icon_status_pending' + when 'running' + 'icon_status_running' + when 'play' + 'icon_play' + when 'created' + 'icon_status_created' + when 'skipped' + 'icon_status_skipped' + else + 'icon_status_canceled' + end + end custom_icon(icon_name) end diff --git a/app/views/shared/icons/_icon_graph_job_cancelled.svg b/app/views/shared/icons/_icon_graph_job_cancelled.svg new file mode 100755 index 00000000000..bd5d04e1cd7 --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_cancelled.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_graph_job_created.svg b/app/views/shared/icons/_icon_graph_job_created.svg new file mode 100755 index 00000000000..326ad04e017 --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_created.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_graph_job_failed.svg b/app/views/shared/icons/_icon_graph_job_failed.svg new file mode 100755 index 00000000000..64da5aa31fc --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_failed.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_graph_job_manual.svg b/app/views/shared/icons/_icon_graph_job_manual.svg new file mode 100755 index 00000000000..c98839f51a9 --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_manual.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_graph_job_pending.svg b/app/views/shared/icons/_icon_graph_job_pending.svg new file mode 100755 index 00000000000..02d5da407e3 --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_pending.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_graph_job_running.svg b/app/views/shared/icons/_icon_graph_job_running.svg new file mode 100755 index 00000000000..532f4fee33c --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_running.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_graph_job_skipped.svg b/app/views/shared/icons/_icon_graph_job_skipped.svg new file mode 100755 index 00000000000..1998dfef9ea --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_skipped.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_graph_job_success.svg b/app/views/shared/icons/_icon_graph_job_success.svg new file mode 100755 index 00000000000..eed5006bebe --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_success.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_graph_job_warning.svg b/app/views/shared/icons/_icon_graph_job_warning.svg new file mode 100755 index 00000000000..cb785635b7e --- /dev/null +++ b/app/views/shared/icons/_icon_graph_job_warning.svg @@ -0,0 +1 @@ + -- 2.24.1 From b4f18a30fb776e28fac405922cb5dfcfdc8ac5d7 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 7 Dec 2016 22:56:47 +0000 Subject: [PATCH 05/47] Adds tests for the status and actions icons rendered in the pipeline graph Fix padding in dropdown --- app/assets/stylesheets/pages/pipelines.scss | 4 -- .../projects/pipelines/pipeline_spec.rb | 41 +++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 06b078d5345..304a7932a06 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -646,10 +646,6 @@ .dropdown-build { color: $gl-text-color-light; - a { - padding: 6px 8px 7px; - } - a.ci-action-icon-container { padding: 0; font-size: 11px; diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 3350a3aeefc..e21de05ac64 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -38,6 +38,47 @@ describe "Pipelines", feature: true, js: true do expect(page).to have_css('#js-tab-pipeline.active') end + context 'pipeline graph' do + it 'shows a running icon and a cancel action for the running build' do + page.within('.stage-column:first-child .build:first-child') do + expect(page).to have_selector('.ci-status-icon-running') + expect(page).to have_content('deploy') + expect(page).to have_selector('.ci-action-icon-container .fa-ban') + end + end + + it 'shows the success icon and a retry action for the successfull build' do + page.within('.stage-column:nth-child(3)') do + expect(page).to have_selector('.ci-status-icon-success') + expect(page).to have_content('build') + expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + end + end + + it 'shows the failed icon and a retry action for the failed build' do + page.within('.stage-column:nth-child(2) .build') do + expect(page).to have_selector('.ci-status-icon-failed') + expect(page).to have_content('test') + expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + end + end + + it 'shows the skipped icon and a play action for the manual build' do + page.within('.stage-column:first-child .build:nth-child(2)') do + expect(page).to have_selector('.ci-status-icon-skipped') + expect(page).to have_content('manual') + expect(page).to have_selector('.ci-action-icon-container .ci-play-icon') + end + end + + it 'shows the success icon for the generic comit status build' do + page.within('.stage-column:nth-child(4) .build') do + expect(page).to have_selector('.ci-status-icon-success') + expect(page).to have_content('jenkins') + end + end + end + context 'page tabs' do it 'shows Pipeline and Builds tabs with link' do expect(page).to have_link('Pipeline') -- 2.24.1 From f7335aa5cb1ed368f0583d7dca8fcfa7574d6ad9 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 7 Dec 2016 23:09:55 +0000 Subject: [PATCH 06/47] Adds changelog entry --- changelogs/unreleased/22604-manual-actions.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/22604-manual-actions.yml diff --git a/changelogs/unreleased/22604-manual-actions.yml b/changelogs/unreleased/22604-manual-actions.yml new file mode 100644 index 00000000000..7335e597292 --- /dev/null +++ b/changelogs/unreleased/22604-manual-actions.yml @@ -0,0 +1,4 @@ +--- +title: Resolve "Manual actions on pipeline graph" +merge_request: 7931 +author: -- 2.24.1 From 007255ed5833bdd7bec8927d9f197002dc519186 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 09:51:36 +0100 Subject: [PATCH 07/47] Added Ci::Status::Build --- app/models/ci/build.rb | 4 ++ app/models/commit_status.rb | 4 ++ lib/gitlab/ci/status/build/common.rb | 54 +++++++++++++++++++++++++++ lib/gitlab/ci/status/build/factory.rb | 15 ++++++++ lib/gitlab/ci/status/core.rb | 10 +++-- 5 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 lib/gitlab/ci/status/build/common.rb create mode 100644 lib/gitlab/ci/status/build/factory.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 88c46076df6..0f4c498c266 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -100,6 +100,10 @@ module Ci end end + def detailed_status + Gitlab::Ci::Status::Build::Factory.new(self).fabricate! + end + def manual? self.when == 'manual' end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index cf90475f4d4..fce16174e22 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -131,4 +131,8 @@ class CommitStatus < ActiveRecord::Base def has_trace? false end + + def detailed_status + Gitlab::Ci::Status::Factory.new(self).fabricate! + end end diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb new file mode 100644 index 00000000000..d3d7e03ee3f --- /dev/null +++ b/lib/gitlab/ci/status/build/common.rb @@ -0,0 +1,54 @@ +module Gitlab + module Ci + module Status + module Build + module Common + def has_details? + true + end + + def details_path + namespace_project_build_path(@subject.project.namespace, + @subject.project, + @subject.pipeline) + end + + def action_type + case + when @subject.playable? then :playable + when @subject.active? then :cancel + when @subject.retryable? then :retry + end + end + + def has_action?(current_user) + action_type && can?(current_user, :update_build, @subject) + end + + def action_icon + case action_type + when :playable then 'remove' + when :cancel then 'icon_play' + when :retry then 'repeat' + end + end + + def action_path + case action_type + when :playable + play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + when :cancel + cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) + when :retry + retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + end + + def action_method + :post + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb new file mode 100644 index 00000000000..dd38e1418b6 --- /dev/null +++ b/lib/gitlab/ci/status/build/factory.rb @@ -0,0 +1,15 @@ +module Gitlab + module Ci + module Status + module Build + class Factory < Status::Factory + private + + def core_status + super.extend(Status::Build::Common) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index ce4108fdcf2..60c559248aa 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -34,15 +34,15 @@ module Gitlab end def has_details? - raise NotImplementedError + false end def details_path raise NotImplementedError end - def has_action? - raise NotImplementedError + def has_action?(_user = nil) + false end def action_icon @@ -52,6 +52,10 @@ module Gitlab def action_path raise NotImplementedError end + + def action_method + raise NotImplementedError + end end end end -- 2.24.1 From cd4a2270c5ccbdbd9e57e0c625eee2d80357d6be Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 10:40:56 +0100 Subject: [PATCH 08/47] Improve actions --- app/models/ci/build.rb | 4 +++ lib/gitlab/ci/status/build/common.rb | 26 +++++---------- lib/gitlab/ci/status/build/factory.rb | 4 +++ lib/gitlab/ci/status/build/play.rb | 47 +++++++++++++++++++++++++++ lib/gitlab/ci/status/build/stop.rb | 47 +++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 18 deletions(-) create mode 100644 lib/gitlab/ci/status/build/play.rb create mode 100644 lib/gitlab/ci/status/build/stop.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 0f4c498c266..73564dd2aa0 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -127,6 +127,10 @@ module Ci end end + def cancelable? + active? + end + def retryable? project.builds_enabled? && commands.present? && complete? end diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index d3d7e03ee3f..2bed68d1a11 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -13,33 +13,23 @@ module Gitlab @subject.pipeline) end - def action_type - case - when @subject.playable? then :playable - when @subject.active? then :cancel - when @subject.retryable? then :retry - end - end - def has_action?(current_user) - action_type && can?(current_user, :update_build, @subject) + (subject.cancelable? || subject.retryable?) && + can?(current_user, :update_build, @subject) end def action_icon - case action_type - when :playable then 'remove' - when :cancel then 'icon_play' - when :retry then 'repeat' + case + when subject.cancelable? then 'icon_play' + when subject.retryable? then 'repeat' end end def action_path - case action_type - when :playable - play_namespace_project_build_path(subject.project.namespace, subject.project, subject) - when :cancel + case + when subject.cancelable? cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) - when :retry + when subject.retryable? retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) end end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index dd38e1418b6..d8a9f53f236 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -5,6 +5,10 @@ module Gitlab class Factory < Status::Factory private + def extended_statuses + [Stop, Play] + end + def core_status super.extend(Status::Build::Common) end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb new file mode 100644 index 00000000000..581c81d0175 --- /dev/null +++ b/lib/gitlab/ci/status/build/play.rb @@ -0,0 +1,47 @@ +module Gitlab + module Ci + module Status + module Status + class Play < SimpleDelegator + extend Status::Extended + + def text + 'play' + end + + def label + 'play' + end + + def icon + 'icon_status_skipped' + end + + def to_s + 'play' + end + + def has_action?(current_user) + can?(current_user, :update_build, subject) + end + + def action_icon + :play + end + + def action_path + play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + + def action_method + :post + end + + def self.matches?(build) + build.playable? && !build.stops_environment? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb new file mode 100644 index 00000000000..261de9695c5 --- /dev/null +++ b/lib/gitlab/ci/status/build/stop.rb @@ -0,0 +1,47 @@ +module Gitlab + module Ci + module Status + module Status + class Play < SimpleDelegator + extend Status::Extended + + def text + 'stop' + end + + def label + 'stop' + end + + def icon + 'icon_status_skipped' + end + + def to_s + 'stop' + end + + def has_action?(current_user) + can?(current_user, :update_build, subject) + end + + def action_icon + :play + end + + def action_path + play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + + def action_method + :post + end + + def self.matches?(build) + build.playable? && build.stops_environment? + end + end + end + end + end +end -- 2.24.1 From 82ee1d29fd3806982afe98678e056194059c64ce Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 10:48:08 +0100 Subject: [PATCH 09/47] Introduce `cancelable` and `returnable` [ci skip] --- lib/gitlab/ci/status/build/cancelable.rb | 31 ++++++++++++++++++++++++ lib/gitlab/ci/status/build/common.rb | 25 ------------------- lib/gitlab/ci/status/build/factory.rb | 2 +- lib/gitlab/ci/status/build/play.rb | 10 +------- lib/gitlab/ci/status/build/retryable.rb | 31 ++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 35 deletions(-) create mode 100644 lib/gitlab/ci/status/build/cancelable.rb create mode 100644 lib/gitlab/ci/status/build/retryable.rb diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb new file mode 100644 index 00000000000..bff0464ef0c --- /dev/null +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -0,0 +1,31 @@ +module Gitlab + module Ci + module Status + module Status + class Cancelable < SimpleDelegator + extend Status::Extended + + def has_action?(current_user) + can?(current_user, :update_build, subject) + end + + def action_icon + 'remove' + end + + def action_path + cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + + def action_method + :post + end + + def self.matches?(build) + build.cancelable? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index 2bed68d1a11..3e47d7dfd20 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -12,31 +12,6 @@ module Gitlab @subject.project, @subject.pipeline) end - - def has_action?(current_user) - (subject.cancelable? || subject.retryable?) && - can?(current_user, :update_build, @subject) - end - - def action_icon - case - when subject.cancelable? then 'icon_play' - when subject.retryable? then 'repeat' - end - end - - def action_path - case - when subject.cancelable? - cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) - when subject.retryable? - retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) - end - end - - def action_method - :post - end end end end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index d8a9f53f236..8f420a93954 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -6,7 +6,7 @@ module Gitlab private def extended_statuses - [Stop, Play] + [Stop, Play, Cancelable, Retryable] end def core_status diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 581c81d0175..d295850137b 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -13,20 +13,12 @@ module Gitlab 'play' end - def icon - 'icon_status_skipped' - end - - def to_s - 'play' - end - def has_action?(current_user) can?(current_user, :update_build, subject) end def action_icon - :play + 'play' end def action_path diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb new file mode 100644 index 00000000000..b3c0eedadf8 --- /dev/null +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -0,0 +1,31 @@ +module Gitlab + module Ci + module Status + module Status + class Retryable < SimpleDelegator + extend Status::Extended + + def has_action?(current_user) + can?(current_user, :update_build, subject) + end + + def action_icon + 'repeat' + end + + def action_path + retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + + def action_method + :post + end + + def self.matches?(build) + build.retryable? + end + end + end + end + end +end -- 2.24.1 From 609bc0a0b67354d6e3df0eba11da1acde6a9d033 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 10:52:44 +0100 Subject: [PATCH 10/47] Check permission of details --- lib/gitlab/ci/status/build/common.rb | 4 ++-- lib/gitlab/ci/status/core.rb | 2 +- lib/gitlab/ci/status/pipeline/common.rb | 4 ++-- lib/gitlab/ci/status/stage/common.rb | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index 3e47d7dfd20..2fb79afa3d3 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -3,8 +3,8 @@ module Gitlab module Status module Build module Common - def has_details? - true + def has_details?(current_user) + can?(current_user, :read_build, subject) end def details_path diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 60c559248aa..6b47096f811 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -33,7 +33,7 @@ module Gitlab self.class.name.demodulize.downcase.underscore end - def has_details? + def has_details?(_user = nil) false end diff --git a/lib/gitlab/ci/status/pipeline/common.rb b/lib/gitlab/ci/status/pipeline/common.rb index 25e52bec3da..5f79044a496 100644 --- a/lib/gitlab/ci/status/pipeline/common.rb +++ b/lib/gitlab/ci/status/pipeline/common.rb @@ -3,8 +3,8 @@ module Gitlab module Status module Pipeline module Common - def has_details? - true + def has_details?(current_user) + can?(current_user, :read_pipeline, subject) end def details_path diff --git a/lib/gitlab/ci/status/stage/common.rb b/lib/gitlab/ci/status/stage/common.rb index 14c437d2b98..e6ee2f92341 100644 --- a/lib/gitlab/ci/status/stage/common.rb +++ b/lib/gitlab/ci/status/stage/common.rb @@ -3,8 +3,8 @@ module Gitlab module Status module Stage module Common - def has_details? - true + def has_details?(current_user) + can?(current_user, :read_pipeline, subject) end def details_path -- 2.24.1 From 65f3206024778b934171c9d9ece8ab627ba6c4c5 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 11:03:01 +0100 Subject: [PATCH 11/47] Remove ci_status_with_icon helper and replace it with partial [ci skip] --- app/helpers/ci_status_helper.rb | 20 +------------------ app/views/admin/runners/show.html.haml | 2 +- .../ci/status/_icon_with_label.html.haml | 10 ++++++++++ app/views/projects/builds/_header.html.haml | 2 +- app/views/projects/ci/builds/_build.html.haml | 5 +---- .../projects/ci/pipelines/_pipeline.html.haml | 5 +---- .../_generic_commit_status.html.haml | 5 +---- app/views/projects/pipelines/_info.html.haml | 2 +- 8 files changed, 17 insertions(+), 34 deletions(-) create mode 100644 app/views/ci/status/_icon_with_label.html.haml diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 8e19752a8a1..d9f5e01f0dc 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -4,25 +4,7 @@ module CiStatusHelper builds_namespace_project_commit_path(project.namespace, project, pipeline.sha) end - def ci_status_with_icon(status, target = nil) - content = ci_icon_for_status(status) + ci_text_for_status(status) - klass = "ci-status ci-#{status}" - - if target - link_to content, target, class: klass - else - content_tag :span, content, class: klass - end - end - - def ci_text_for_status(status) - if detailed_status?(status) - status.text - else - status - end - end - + # Is used by Commit and Merge Request Widget def ci_label_for_status(status) if detailed_status?(status) return status.label diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index 73038164056..fa8be25ffa8 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -91,7 +91,7 @@ %strong ##{build.id} %td.status - = ci_status_with_icon(build.status) + = render "ci/status/icon_with_label", subject: build %td.status - if project diff --git a/app/views/ci/status/_icon_with_label.html.haml b/app/views/ci/status/_icon_with_label.html.haml new file mode 100644 index 00000000000..65a74e88444 --- /dev/null +++ b/app/views/ci/status/_icon_with_label.html.haml @@ -0,0 +1,10 @@ +- details_path = subject.details_path if subject.has_details?(current_user) +- klass = "ci-status ci-#{subject.status}" +- if details_path + = link_to details_path, class: klass do + = custom_icon(status.icon) + = status.text +- else + %span{ class: klass } + = custom_icon(status.icon) + = status.text diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index f6aa20c4579..5e4e30f08d5 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -1,6 +1,6 @@ .content-block.build-header .header-content - = ci_status_with_icon(@build.status) + = render "ci/status/icon_with_label", subject: build Build %strong ##{@build.id} in pipeline diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 18b3b04154f..6b0cd3e49a0 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -9,10 +9,7 @@ %tr.build.commit{class: ('retried' if retried)} %td.status - - if can?(current_user, :read_build, build) - = ci_status_with_icon(build.status, namespace_project_build_url(build.project.namespace, build.project, build)) - - else - = ci_status_with_icon(build.status) + = render "ci/status/icon_with_label", subject: build %td.branch-commit - if can?(current_user, :read_build, build) diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index b58dceb58c9..84243e4306d 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -1,13 +1,10 @@ - status = pipeline.status -- detailed_status = pipeline.detailed_status - show_commit = local_assigns.fetch(:show_commit, true) - show_branch = local_assigns.fetch(:show_branch, true) %tr.commit %td.commit-link - = link_to namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id), class: "ci-status ci-#{detailed_status}" do - = ci_icon_for_status(detailed_status) - = ci_text_for_status(detailed_status) + = render "ci/status/icon_with_label", subject: pipeline %td = link_to namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) do diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index 7f751d9ae2e..69cb1631ee8 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -8,10 +8,7 @@ %tr.generic_commit_status{class: ('retried' if retried)} %td.status - - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url - = ci_status_with_icon(generic_commit_status.status, generic_commit_status.target_url) - - else - = ci_status_with_icon(generic_commit_status.status) + = render "ci/status/icon_with_label", subject: generic_commit_status %td.generic_commit_status-link - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index 229bdfb0e8d..f7385184a2b 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,6 +1,6 @@ .page-content-header .header-main-content - = ci_status_with_icon(@pipeline.detailed_status) + = render "ci/status/icon_with_label", subject: @pipeline %strong Pipeline ##{@commit.pipelines.last.id} triggered #{time_ago_with_tooltip(@commit.authored_date)} by = author_avatar(@commit, size: 24) -- 2.24.1 From 3acf4323797346437f74e6bf5950fcac917ae56c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Dec 2016 13:51:06 +0100 Subject: [PATCH 12/47] Refactor ci status factories to DRY code a little --- lib/gitlab/ci/status/build/factory.rb | 11 ++++----- lib/gitlab/ci/status/extended.rb | 2 +- lib/gitlab/ci/status/factory.rb | 30 ++++++++++++++---------- lib/gitlab/ci/status/pipeline/factory.rb | 8 +++---- lib/gitlab/ci/status/stage/factory.rb | 6 ++--- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index 8f420a93954..eee9a64120b 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -3,14 +3,13 @@ module Gitlab module Status module Build class Factory < Status::Factory - private - - def extended_statuses - [Stop, Play, Cancelable, Retryable] + def self.extended_statuses + [Status::Build::Stop, Status::Build::Play, + Status::Build::Cancelable, Status::Build::Retryable] end - def core_status - super.extend(Status::Build::Common) + def self.common_helpers + Status::Build::Common end end end diff --git a/lib/gitlab/ci/status/extended.rb b/lib/gitlab/ci/status/extended.rb index 6bfb5d38c1f..93e6eff1c94 100644 --- a/lib/gitlab/ci/status/extended.rb +++ b/lib/gitlab/ci/status/extended.rb @@ -2,7 +2,7 @@ module Gitlab module Ci module Status module Extended - def matches?(_subject) + def matches?(_subject, _user) raise NotImplementedError end end diff --git a/lib/gitlab/ci/status/factory.rb b/lib/gitlab/ci/status/factory.rb index b2f896f2211..944e0fdde2d 100644 --- a/lib/gitlab/ci/status/factory.rb +++ b/lib/gitlab/ci/status/factory.rb @@ -2,10 +2,9 @@ module Gitlab module Ci module Status class Factory - attr_reader :subject - - def initialize(subject) + def initialize(subject, user = nil) @subject = subject + @user = user end def fabricate! @@ -16,27 +15,32 @@ module Gitlab end end + def self.extended_statuses + [] + end + + def self.common_helpers + Module.new + end + private - def subject_status - @subject_status ||= subject.status + def simple_status + @simple_status ||= @subject.status || :created end def core_status Gitlab::Ci::Status - .const_get(subject_status.capitalize) - .new(subject) + .const_get(simple_status.capitalize) + .new(@subject) + .extend(self.class.common_helpers) end def extended_status - @extended ||= extended_statuses.find do |status| - status.matches?(subject) + @extended ||= self.class.extended_statuses.find do |status| + status.matches?(@subject, @user) end end - - def extended_statuses - [] - end end end end diff --git a/lib/gitlab/ci/status/pipeline/factory.rb b/lib/gitlab/ci/status/pipeline/factory.rb index 4ac4ec671d0..16dcb326be9 100644 --- a/lib/gitlab/ci/status/pipeline/factory.rb +++ b/lib/gitlab/ci/status/pipeline/factory.rb @@ -3,14 +3,12 @@ module Gitlab module Status module Pipeline class Factory < Status::Factory - private - - def extended_statuses + def self.extended_statuses [Pipeline::SuccessWithWarnings] end - def core_status - super.extend(Status::Pipeline::Common) + def self.common_helpers + Status::Pipeline::Common end end end diff --git a/lib/gitlab/ci/status/stage/factory.rb b/lib/gitlab/ci/status/stage/factory.rb index c6522d5ada1..689a5dd45bc 100644 --- a/lib/gitlab/ci/status/stage/factory.rb +++ b/lib/gitlab/ci/status/stage/factory.rb @@ -3,10 +3,8 @@ module Gitlab module Status module Stage class Factory < Status::Factory - private - - def core_status - super.extend(Status::Stage::Common) + def self.common_helpers + Status::Stage::Common end end end -- 2.24.1 From 9c0a8cb5b6311c217f7c5c98721da78eae09391f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Dec 2016 14:28:49 +0100 Subject: [PATCH 13/47] Incorporate permission checks into new CI statuses [ci skip] --- lib/gitlab/ci/status/build/cancelable.rb | 12 +++++++----- lib/gitlab/ci/status/build/common.rb | 10 +++++----- lib/gitlab/ci/status/build/play.rb | 12 +++++++----- lib/gitlab/ci/status/build/retryable.rb | 12 +++++++----- lib/gitlab/ci/status/build/stop.rb | 12 +++++++----- lib/gitlab/ci/status/core.rb | 13 ++++++------- lib/gitlab/ci/status/extended.rb | 8 ++++++-- lib/gitlab/ci/status/factory.rb | 4 ++-- lib/gitlab/ci/status/pipeline/common.rb | 10 +++++----- .../ci/status/pipeline/success_with_warnings.rb | 4 ++-- lib/gitlab/ci/status/stage/common.rb | 12 ++++++------ 11 files changed, 60 insertions(+), 49 deletions(-) diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index bff0464ef0c..a8830b04715 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -3,10 +3,10 @@ module Gitlab module Status module Status class Cancelable < SimpleDelegator - extend Status::Extended + include Status::Extended - def has_action?(current_user) - can?(current_user, :update_build, subject) + def has_action? + can?(user, :update_build, subject) end def action_icon @@ -14,14 +14,16 @@ module Gitlab end def action_path - cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) + cancel_namespace_project_build_path(subject.project.namespace, + subject.project, + subject) end def action_method :post end - def self.matches?(build) + def self.matches?(build, user) build.cancelable? end end diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index 2fb79afa3d3..2b602f1e247 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -3,14 +3,14 @@ module Gitlab module Status module Build module Common - def has_details?(current_user) - can?(current_user, :read_build, subject) + def has_details? + can?(user, :read_build, subject) end def details_path - namespace_project_build_path(@subject.project.namespace, - @subject.project, - @subject.pipeline) + namespace_project_build_path(subject.project.namespace, + subject.project, + subject.pipeline) end end end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index d295850137b..70c08197ea1 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -3,7 +3,7 @@ module Gitlab module Status module Status class Play < SimpleDelegator - extend Status::Extended + include Status::Extended def text 'play' @@ -13,8 +13,8 @@ module Gitlab 'play' end - def has_action?(current_user) - can?(current_user, :update_build, subject) + def has_action? + can?(user, :update_build, subject) end def action_icon @@ -22,14 +22,16 @@ module Gitlab end def action_path - play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + play_namespace_project_build_path(subject.project.namespace, + subject.project, + subject) end def action_method :post end - def self.matches?(build) + def self.matches?(build, user) build.playable? && !build.stops_environment? end end diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index b3c0eedadf8..3309e8808e1 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -3,10 +3,10 @@ module Gitlab module Status module Status class Retryable < SimpleDelegator - extend Status::Extended + include Status::Extended - def has_action?(current_user) - can?(current_user, :update_build, subject) + def has_action? + can?(user, :update_build, subject) end def action_icon @@ -14,14 +14,16 @@ module Gitlab end def action_path - retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) + retry_namespace_project_build_path(subject.project.namespace, + subject.project, + subject) end def action_method :post end - def self.matches?(build) + def self.matches?(build, user) build.retryable? end end diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 261de9695c5..6fb51890bec 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -3,7 +3,7 @@ module Gitlab module Status module Status class Play < SimpleDelegator - extend Status::Extended + include Status::Extended def text 'stop' @@ -21,8 +21,8 @@ module Gitlab 'stop' end - def has_action?(current_user) - can?(current_user, :update_build, subject) + def has_action? + can?(user, :update_build, subject) end def action_icon @@ -30,14 +30,16 @@ module Gitlab end def action_path - play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + play_namespace_project_build_path(subject.project.namespace, + subject.project, + subject) end def action_method :post end - def self.matches?(build) + def self.matches?(build, user) build.playable? && build.stops_environment? end end diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 6b47096f811..df371363736 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -6,8 +6,11 @@ module Gitlab class Core include Gitlab::Routing.url_helpers - def initialize(subject) + attr_reader :subject, :user + + def initialize(subject, user) @subject = subject + @user = user end def icon @@ -18,10 +21,6 @@ module Gitlab raise NotImplementedError end - def title - "#{@subject.class.name.demodulize}: #{label}" - end - # Deprecation warning: this method is here because we need to maintain # backwards compatibility with legacy statuses. We often do something # like "ci-status ci-status-#{status}" to set CSS class. @@ -33,7 +32,7 @@ module Gitlab self.class.name.demodulize.downcase.underscore end - def has_details?(_user = nil) + def has_details? false end @@ -41,7 +40,7 @@ module Gitlab raise NotImplementedError end - def has_action?(_user = nil) + def has_action? false end diff --git a/lib/gitlab/ci/status/extended.rb b/lib/gitlab/ci/status/extended.rb index 93e6eff1c94..d367c9bda69 100644 --- a/lib/gitlab/ci/status/extended.rb +++ b/lib/gitlab/ci/status/extended.rb @@ -2,8 +2,12 @@ module Gitlab module Ci module Status module Extended - def matches?(_subject, _user) - raise NotImplementedError + extend ActiveSupport::Concern + + class_methods do + def matches?(_subject, _user) + raise NotImplementedError + end end end end diff --git a/lib/gitlab/ci/status/factory.rb b/lib/gitlab/ci/status/factory.rb index 944e0fdde2d..ae9ef895df4 100644 --- a/lib/gitlab/ci/status/factory.rb +++ b/lib/gitlab/ci/status/factory.rb @@ -2,7 +2,7 @@ module Gitlab module Ci module Status class Factory - def initialize(subject, user = nil) + def initialize(subject, user) @subject = subject @user = user end @@ -32,7 +32,7 @@ module Gitlab def core_status Gitlab::Ci::Status .const_get(simple_status.capitalize) - .new(@subject) + .new(@subject, @user) .extend(self.class.common_helpers) end diff --git a/lib/gitlab/ci/status/pipeline/common.rb b/lib/gitlab/ci/status/pipeline/common.rb index 5f79044a496..76bfd18bf40 100644 --- a/lib/gitlab/ci/status/pipeline/common.rb +++ b/lib/gitlab/ci/status/pipeline/common.rb @@ -3,14 +3,14 @@ module Gitlab module Status module Pipeline module Common - def has_details?(current_user) - can?(current_user, :read_pipeline, subject) + def has_details? + can?(user, :read_pipeline, subject) end def details_path - namespace_project_pipeline_path(@subject.project.namespace, - @subject.project, - @subject) + namespace_project_pipeline_path(subject.project.namespace, + subject.project, + subject) end def has_action? diff --git a/lib/gitlab/ci/status/pipeline/success_with_warnings.rb b/lib/gitlab/ci/status/pipeline/success_with_warnings.rb index 4b040d60df8..a7c98f9e909 100644 --- a/lib/gitlab/ci/status/pipeline/success_with_warnings.rb +++ b/lib/gitlab/ci/status/pipeline/success_with_warnings.rb @@ -3,7 +3,7 @@ module Gitlab module Status module Pipeline class SuccessWithWarnings < SimpleDelegator - extend Status::Extended + include Status::Extended def text 'passed' @@ -21,7 +21,7 @@ module Gitlab 'success_with_warnings' end - def self.matches?(pipeline) + def self.matches?(pipeline, user) pipeline.success? && pipeline.has_warnings? end end diff --git a/lib/gitlab/ci/status/stage/common.rb b/lib/gitlab/ci/status/stage/common.rb index e6ee2f92341..6851ceda317 100644 --- a/lib/gitlab/ci/status/stage/common.rb +++ b/lib/gitlab/ci/status/stage/common.rb @@ -3,15 +3,15 @@ module Gitlab module Status module Stage module Common - def has_details?(current_user) - can?(current_user, :read_pipeline, subject) + def has_details? + can?(user, :read_pipeline, subject) end def details_path - namespace_project_pipeline_path(@subject.project.namespace, - @subject.project, - @subject.pipeline, - anchor: @subject.name) + namespace_project_pipeline_path(subject.project.namespace, + subject.project, + subject.pipeline, + anchor: subject.name) end def has_action? -- 2.24.1 From 7d88399454b5c71c9af84561c858001d1a733c41 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Dec 2016 14:40:21 +0100 Subject: [PATCH 14/47] Fix tests related to detailed statuses and permissions [ci skip] --- spec/lib/gitlab/ci/status/canceled_spec.rb | 4 +++- spec/lib/gitlab/ci/status/created_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/extended_spec.rb | 2 +- spec/lib/gitlab/ci/status/factory_spec.rb | 6 ++++-- spec/lib/gitlab/ci/status/failed_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/pending_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/pipeline/common_spec.rb | 4 +++- spec/lib/gitlab/ci/status/pipeline/factory_spec.rb | 4 +++- .../ci/status/pipeline/success_with_warnings_spec.rb | 2 +- spec/lib/gitlab/ci/status/running_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/skipped_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/stage/common_spec.rb | 8 ++++++-- spec/lib/gitlab/ci/status/stage/factory_spec.rb | 8 ++++++-- spec/lib/gitlab/ci/status/success_spec.rb | 8 +++----- 14 files changed, 45 insertions(+), 41 deletions(-) diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb index 619ecbcba67..eaf974bb953 100644 --- a/spec/lib/gitlab/ci/status/canceled_spec.rb +++ b/spec/lib/gitlab/ci/status/canceled_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Canceled do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'canceled' } diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb index 157302c65a8..2ce176a29d6 100644 --- a/spec/lib/gitlab/ci/status/created_spec.rb +++ b/spec/lib/gitlab/ci/status/created_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Created do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'created' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Created do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_created' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: created' } - end end diff --git a/spec/lib/gitlab/ci/status/extended_spec.rb b/spec/lib/gitlab/ci/status/extended_spec.rb index 120e121aae5..864121dec4b 100644 --- a/spec/lib/gitlab/ci/status/extended_spec.rb +++ b/spec/lib/gitlab/ci/status/extended_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Ci::Status::Extended do end it 'requires subclass to implement matcher' do - expect { subject.matches?(double) } + expect { subject.matches?(double, double) } .to raise_error(NotImplementedError) end end diff --git a/spec/lib/gitlab/ci/status/factory_spec.rb b/spec/lib/gitlab/ci/status/factory_spec.rb index d5bd7f7102b..f92a1c149bf 100644 --- a/spec/lib/gitlab/ci/status/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/factory_spec.rb @@ -2,15 +2,17 @@ require 'spec_helper' describe Gitlab::Ci::Status::Factory do subject do - described_class.new(object) + described_class.new(resource, user) end + let(:user) { create(:user) } + let(:status) { subject.fabricate! } context 'when object has a core status' do HasStatus::AVAILABLE_STATUSES.each do |core_status| context "when core status is #{core_status}" do - let(:object) { double(status: core_status) } + let(:resource) { double(status: core_status) } it "fabricates a core status #{core_status}" do expect(status).to be_a( diff --git a/spec/lib/gitlab/ci/status/failed_spec.rb b/spec/lib/gitlab/ci/status/failed_spec.rb index 0b3cb8168e6..9d527e6a7ef 100644 --- a/spec/lib/gitlab/ci/status/failed_spec.rb +++ b/spec/lib/gitlab/ci/status/failed_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Failed do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'failed' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Failed do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_failed' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: failed' } - end end diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb index 57c901c1202..d03f595d3c7 100644 --- a/spec/lib/gitlab/ci/status/pending_spec.rb +++ b/spec/lib/gitlab/ci/status/pending_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pending do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'pending' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Pending do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_pending' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: pending' } - end end diff --git a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb index 21adee3f8e7..4f32ae5d809 100644 --- a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb @@ -1,11 +1,13 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Common do + let(:user) { create(:user) } let(:pipeline) { create(:ci_pipeline) } subject do Class.new(Gitlab::Ci::Status::Core) - .new(pipeline).extend(described_class) + .new(pipeline, user) + .extend(described_class) end it 'does not have action' do diff --git a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb index d6243940f2e..c6b2582652d 100644 --- a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb @@ -1,8 +1,10 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Factory do + let(:user) { create(:user) } + subject do - described_class.new(pipeline) + described_class.new(pipeline, user) end let(:status) do diff --git a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb index 02e526e3de2..634f80088d5 100644 --- a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do subject do - described_class.new(double('status')) + described_class.new(double('status'), double('user')) end describe '#test' do diff --git a/spec/lib/gitlab/ci/status/running_spec.rb b/spec/lib/gitlab/ci/status/running_spec.rb index c023f1872cc..9f47090d396 100644 --- a/spec/lib/gitlab/ci/status/running_spec.rb +++ b/spec/lib/gitlab/ci/status/running_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Running do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'running' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Running do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_running' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: running' } - end end diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb index d4f7f4b3b70..94601648a8d 100644 --- a/spec/lib/gitlab/ci/status/skipped_spec.rb +++ b/spec/lib/gitlab/ci/status/skipped_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Skipped do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'skipped' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Skipped do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_skipped' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: skipped' } - end end diff --git a/spec/lib/gitlab/ci/status/stage/common_spec.rb b/spec/lib/gitlab/ci/status/stage/common_spec.rb index f3259c6f23e..9b7e6777dc1 100644 --- a/spec/lib/gitlab/ci/status/stage/common_spec.rb +++ b/spec/lib/gitlab/ci/status/stage/common_spec.rb @@ -1,12 +1,16 @@ require 'spec_helper' describe Gitlab::Ci::Status::Stage::Common do + let(:user) { create(:user) } let(:pipeline) { create(:ci_empty_pipeline) } - let(:stage) { build(:ci_stage, pipeline: pipeline, name: 'test') } + + let(:stage) do + build(:ci_stage, pipeline: pipeline, name: 'test') + end subject do Class.new(Gitlab::Ci::Status::Core) - .new(stage).extend(described_class) + .new(stage, user).extend(described_class) end it 'does not have action' do diff --git a/spec/lib/gitlab/ci/status/stage/factory_spec.rb b/spec/lib/gitlab/ci/status/stage/factory_spec.rb index 17929665c83..5a281564415 100644 --- a/spec/lib/gitlab/ci/status/stage/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/stage/factory_spec.rb @@ -1,11 +1,15 @@ require 'spec_helper' describe Gitlab::Ci::Status::Stage::Factory do + let(:user) { create(:user) } let(:pipeline) { create(:ci_empty_pipeline) } - let(:stage) { build(:ci_stage, pipeline: pipeline, name: 'test') } + + let(:stage) do + build(:ci_stage, pipeline: pipeline, name: 'test') + end subject do - described_class.new(stage) + described_class.new(stage, user) end let(:status) do diff --git a/spec/lib/gitlab/ci/status/success_spec.rb b/spec/lib/gitlab/ci/status/success_spec.rb index 9e261a3aa5f..90f9f615e0d 100644 --- a/spec/lib/gitlab/ci/status/success_spec.rb +++ b/spec/lib/gitlab/ci/status/success_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Success do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'passed' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Success do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_success' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: passed' } - end end -- 2.24.1 From c1db5b91207f4e3f7144c5cb62ce9160cf2e32e9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Dec 2016 14:51:38 +0100 Subject: [PATCH 15/47] Fix some detailed statuses specs related to abilities --- app/models/ability.rb | 6 +++++ lib/gitlab/ci/status/core.rb | 1 + lib/gitlab/ci/status/stage/common.rb | 2 +- spec/lib/gitlab/ci/status/canceled_spec.rb | 4 ---- spec/lib/gitlab/ci/status/extended_spec.rb | 2 +- .../gitlab/ci/status/pipeline/common_spec.rb | 7 +++++- .../gitlab/ci/status/pipeline/factory_spec.rb | 5 ++++ .../pipeline/success_with_warnings_spec.rb | 10 ++++---- .../lib/gitlab/ci/status/stage/common_spec.rb | 23 +++++++++++++++---- .../gitlab/ci/status/stage/factory_spec.rb | 7 +++++- 10 files changed, 49 insertions(+), 18 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index fa8f8bc3a5f..ce461caf686 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,4 +1,10 @@ class Ability + module Allowable + def can?(user, action, subject) + Ability.allowed?(user, action, subject) + end + end + class << self # Given a list of users and a project this method returns the users that can # read the given project. diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index df371363736..7e9f6e35012 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -5,6 +5,7 @@ module Gitlab # class Core include Gitlab::Routing.url_helpers + include Ability::Allowable attr_reader :subject, :user diff --git a/lib/gitlab/ci/status/stage/common.rb b/lib/gitlab/ci/status/stage/common.rb index 6851ceda317..7852f492e1d 100644 --- a/lib/gitlab/ci/status/stage/common.rb +++ b/lib/gitlab/ci/status/stage/common.rb @@ -4,7 +4,7 @@ module Gitlab module Stage module Common def has_details? - can?(user, :read_pipeline, subject) + can?(user, :read_pipeline, subject.pipeline) end def details_path diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb index eaf974bb953..4639278ad45 100644 --- a/spec/lib/gitlab/ci/status/canceled_spec.rb +++ b/spec/lib/gitlab/ci/status/canceled_spec.rb @@ -16,8 +16,4 @@ describe Gitlab::Ci::Status::Canceled do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_canceled' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: canceled' } - end end diff --git a/spec/lib/gitlab/ci/status/extended_spec.rb b/spec/lib/gitlab/ci/status/extended_spec.rb index 864121dec4b..c2d74ca5cde 100644 --- a/spec/lib/gitlab/ci/status/extended_spec.rb +++ b/spec/lib/gitlab/ci/status/extended_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Extended do subject do - Class.new.extend(described_class) + Class.new.include(described_class) end it 'requires subclass to implement matcher' do diff --git a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb index 4f32ae5d809..2df9d574677 100644 --- a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Common do let(:user) { create(:user) } - let(:pipeline) { create(:ci_pipeline) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } subject do Class.new(Gitlab::Ci::Status::Core) @@ -10,6 +11,10 @@ describe Gitlab::Ci::Status::Pipeline::Common do .extend(described_class) end + before do + project.team << [user, :developer] + end + it 'does not have action' do expect(subject).not_to have_action end diff --git a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb index c6b2582652d..d4a2dc7fcc1 100644 --- a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Factory do let(:user) { create(:user) } + let(:project) { pipeline.project } subject do described_class.new(pipeline, user) @@ -11,6 +12,10 @@ describe Gitlab::Ci::Status::Pipeline::Factory do subject.fabricate! end + before do + project.team << [user, :developer] + end + context 'when pipeline has a core status' do HasStatus::AVAILABLE_STATUSES.each do |core_status| context "when core status is #{core_status}" do diff --git a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb index 634f80088d5..7e3383c307f 100644 --- a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do subject do - described_class.new(double('status'), double('user')) + described_class.new(double('status')) end describe '#test' do @@ -29,13 +29,13 @@ describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do end it 'is a correct match' do - expect(described_class.matches?(pipeline)).to eq true + expect(described_class.matches?(pipeline, double)).to eq true end end context 'when pipeline does not have warnings' do it 'does not match' do - expect(described_class.matches?(pipeline)).to eq false + expect(described_class.matches?(pipeline, double)).to eq false end end end @@ -51,13 +51,13 @@ describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do end it 'does not match' do - expect(described_class.matches?(pipeline)).to eq false + expect(described_class.matches?(pipeline, double)).to eq false end end context 'when pipeline does not have warnings' do it 'does not match' do - expect(described_class.matches?(pipeline)).to eq false + expect(described_class.matches?(pipeline, double)).to eq false end end end diff --git a/spec/lib/gitlab/ci/status/stage/common_spec.rb b/spec/lib/gitlab/ci/status/stage/common_spec.rb index 9b7e6777dc1..8814a7614a0 100644 --- a/spec/lib/gitlab/ci/status/stage/common_spec.rb +++ b/spec/lib/gitlab/ci/status/stage/common_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Stage::Common do let(:user) { create(:user) } - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:stage) do build(:ci_stage, pipeline: pipeline, name: 'test') @@ -17,14 +18,26 @@ describe Gitlab::Ci::Status::Stage::Common do expect(subject).not_to have_action end - it 'has details' do - expect(subject).to have_details - end - it 'links to the pipeline details page' do expect(subject.details_path) .to include "pipelines/#{pipeline.id}" expect(subject.details_path) .to include "##{stage.name}" end + + context 'when user has permission to read pipeline' do + before do + project.team << [user, :master] + end + + it 'has details' do + expect(subject).to have_details + end + end + + context 'when user does not have permission to read pipeline' do + it 'does not have details' do + expect(subject).not_to have_details + end + end end diff --git a/spec/lib/gitlab/ci/status/stage/factory_spec.rb b/spec/lib/gitlab/ci/status/stage/factory_spec.rb index 5a281564415..6f8721d30c2 100644 --- a/spec/lib/gitlab/ci/status/stage/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/stage/factory_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Stage::Factory do let(:user) { create(:user) } - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:stage) do build(:ci_stage, pipeline: pipeline, name: 'test') @@ -16,6 +17,10 @@ describe Gitlab::Ci::Status::Stage::Factory do subject.fabricate! end + before do + project.team << [user, :developer] + end + context 'when stage has a core status' do HasStatus::AVAILABLE_STATUSES.each do |core_status| context "when core status is #{core_status}" do -- 2.24.1 From 5b89ec0dbf76784ccc1e41b66498dd0dc4f05042 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 17:52:24 +0100 Subject: [PATCH 16/47] Fix auto loading of constants for Ci Statuses --- app/models/ci/build.rb | 6 +++--- app/models/ci/pipeline.rb | 4 ++-- app/models/ci/stage.rb | 4 ++-- app/models/commit_status.rb | 4 ++-- app/views/ci/status/_icon_with_label.html.haml | 13 +++++++------ lib/gitlab/ci/status/build/cancelable.rb | 2 +- lib/gitlab/ci/status/build/play.rb | 2 +- lib/gitlab/ci/status/build/retryable.rb | 2 +- lib/gitlab/ci/status/build/stop.rb | 4 ++-- 9 files changed, 21 insertions(+), 20 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 73564dd2aa0..65ee327a8e5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -100,8 +100,8 @@ module Ci end end - def detailed_status - Gitlab::Ci::Status::Build::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Build::Factory.new(self, current_user).fabricate! end def manual? @@ -156,7 +156,7 @@ module Ci end def environment_action - self.options.fetch(:environment, {}).fetch(:action, 'start') + self.options.fetch(:environment, {}).fetch(:action, 'start') if self.options end def outdated_deployment? diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index fda8228a1e9..1f33106d358 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -336,8 +336,8 @@ module Ci .select { |merge_request| merge_request.head_pipeline.try(:id) == self.id } end - def detailed_status - Gitlab::Ci::Status::Pipeline::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Pipeline::Factory.new(self, current_user).fabricate! end private diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index d2a37c0a827..be52cce20f1 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -22,8 +22,8 @@ module Ci @status ||= statuses.latest.status end - def detailed_status - Gitlab::Ci::Status::Stage::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Stage::Factory.new(self, current_user).fabricate! end def statuses diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index fce16174e22..6548a7dda2c 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -132,7 +132,7 @@ class CommitStatus < ActiveRecord::Base false end - def detailed_status - Gitlab::Ci::Status::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Factory.new(self, current_user).fabricate! end end diff --git a/app/views/ci/status/_icon_with_label.html.haml b/app/views/ci/status/_icon_with_label.html.haml index 65a74e88444..d3fe332cc78 100644 --- a/app/views/ci/status/_icon_with_label.html.haml +++ b/app/views/ci/status/_icon_with_label.html.haml @@ -1,10 +1,11 @@ -- details_path = subject.details_path if subject.has_details?(current_user) -- klass = "ci-status ci-#{subject.status}" +- detailed_status = subject.detailed_status(current_user) +- details_path = detailed_status.details_path if detailed_status.has_details? +- klass = "ci-status ci-#{detailed_status}" - if details_path = link_to details_path, class: klass do - = custom_icon(status.icon) - = status.text + = custom_icon(detailed_status.icon) + = detailed_status.text - else %span{ class: klass } - = custom_icon(status.icon) - = status.text + = custom_icon(detailed_status.icon) + = detailed_status.text diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index a8830b04715..88be0cd924b 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Cancelable < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 70c08197ea1..57c7058fe84 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Play < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 3309e8808e1..69f2ad1d277 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Retryable < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 6fb51890bec..cd9bd959a7c 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -1,8 +1,8 @@ module Gitlab module Ci module Status - module Status - class Play < SimpleDelegator + module Build + class Stop < SimpleDelegator include Status::Extended def text -- 2.24.1 From f6240862c884a0290f4ffcce9071946c785b4027 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:16:23 +0100 Subject: [PATCH 17/47] Rename icon_with_label to icon_with_description --- app/views/admin/runners/show.html.haml | 2 +- ..._with_label.html.haml => _icon_with_description.html.haml} | 1 + app/views/projects/builds/_header.html.haml | 2 +- app/views/projects/ci/builds/_build.html.haml | 2 +- app/views/projects/ci/pipelines/_pipeline.html.haml | 2 +- .../generic_commit_statuses/_generic_commit_status.html.haml | 2 +- app/views/projects/pipelines/_info.html.haml | 2 +- app/views/projects/stage/_graph.html.haml | 4 ++-- app/views/projects/stage/_in_stage_group.html.haml | 2 +- 9 files changed, 10 insertions(+), 9 deletions(-) rename app/views/ci/status/{_icon_with_label.html.haml => _icon_with_description.html.haml} (99%) diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index fa8be25ffa8..badeb11b208 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -91,7 +91,7 @@ %strong ##{build.id} %td.status - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build %td.status - if project diff --git a/app/views/ci/status/_icon_with_label.html.haml b/app/views/ci/status/_icon_with_description.html.haml similarity index 99% rename from app/views/ci/status/_icon_with_label.html.haml rename to app/views/ci/status/_icon_with_description.html.haml index d3fe332cc78..34c923440d0 100644 --- a/app/views/ci/status/_icon_with_label.html.haml +++ b/app/views/ci/status/_icon_with_description.html.haml @@ -1,6 +1,7 @@ - detailed_status = subject.detailed_status(current_user) - details_path = detailed_status.details_path if detailed_status.has_details? - klass = "ci-status ci-#{detailed_status}" + - if details_path = link_to details_path, class: klass do = custom_icon(detailed_status.icon) diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index 5e4e30f08d5..85d1793ecb9 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -1,6 +1,6 @@ .content-block.build-header .header-content - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build Build %strong ##{@build.id} in pipeline diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 6b0cd3e49a0..4257bb86859 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -9,7 +9,7 @@ %tr.build.commit{class: ('retried' if retried)} %td.status - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build %td.branch-commit - if can?(current_user, :read_build, build) diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 84243e4306d..6dff955ea3d 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -4,7 +4,7 @@ %tr.commit %td.commit-link - = render "ci/status/icon_with_label", subject: pipeline + = render "ci/status/icon_with_description", subject: pipeline %td = link_to namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) do diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index 69cb1631ee8..1dd07ae1a2a 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -8,7 +8,7 @@ %tr.generic_commit_status{class: ('retried' if retried)} %td.status - = render "ci/status/icon_with_label", subject: generic_commit_status + = render "ci/status/icon_with_description", subject: generic_commit_status %td.generic_commit_status-link - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index f7385184a2b..d05697b4ee3 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,6 +1,6 @@ .page-content-header .header-main-content - = render "ci/status/icon_with_label", subject: @pipeline + = render "ci/status/icon_with_description", subject: @pipeline %strong Pipeline ##{@commit.pipelines.last.id} triggered #{time_ago_with_tooltip(@commit.authored_date)} by = author_avatar(@commit, size: 24) diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index d8c87fae5a1..cb845ae7f92 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -15,9 +15,9 @@ %li.build{ class: ("playable" if is_playable) } .curve .build-content - = render "projects/#{status.to_partial_path}_pipeline", subject: status + = render 'ci/status/icon_with_name_and_action', subject: status - else %li.build .curve .dropdown.inline.build-content - = render "projects/stage/in_stage_group", name: group_name, subject: grouped_statuses + = render 'projects/stage/in_stage_group', name: group_name, subject: grouped_statuses diff --git a/app/views/projects/stage/_in_stage_group.html.haml b/app/views/projects/stage/_in_stage_group.html.haml index 2d198d1b389..5c9b6549b37 100644 --- a/app/views/projects/stage/_in_stage_group.html.haml +++ b/app/views/projects/stage/_in_stage_group.html.haml @@ -10,4 +10,4 @@ %ul - subject.each do |status| %li.dropdown-build - = render "projects/#{status.to_partial_path}_pipeline", subject: status + = render 'ci/status/icon_with_name_and_action', subject: status -- 2.24.1 From 7c6984752fb6557ac754bb13a554f9b76ee39989 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:18:30 +0100 Subject: [PATCH 18/47] Add action_class/action_title --- lib/gitlab/ci/status/build/cancelable.rb | 6 +++++- lib/gitlab/ci/status/build/play.rb | 8 ++++++++ lib/gitlab/ci/status/build/retryable.rb | 6 +++++- lib/gitlab/ci/status/build/stop.rb | 6 +++++- lib/gitlab/ci/status/core.rb | 7 +++++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index 88be0cd924b..a979fe7d573 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -10,7 +10,7 @@ module Gitlab end def action_icon - 'remove' + 'ban' end def action_path @@ -23,6 +23,10 @@ module Gitlab :post end + def action_title + 'Cancel' + end + def self.matches?(build, user) build.cancelable? end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 57c7058fe84..e3066d40a37 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -17,10 +17,18 @@ module Gitlab can?(user, :update_build, subject) end + def action_title + 'Play' + end + def action_icon 'play' end + def action_class + 'ci-play-icon' + end + def action_path play_namespace_project_build_path(subject.project.namespace, subject.project, diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 69f2ad1d277..8e38d6a8523 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -10,7 +10,11 @@ module Gitlab end def action_icon - 'repeat' + 'refresh' + end + + def action_title + 'Retry' end def action_path diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index cd9bd959a7c..487fd033960 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -26,7 +26,11 @@ module Gitlab end def action_icon - :play + 'stop' + end + + def action_title + 'Stop' end def action_path diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 7e9f6e35012..dd3a824e486 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -49,6 +49,9 @@ module Gitlab raise NotImplementedError end + def action_class + end + def action_path raise NotImplementedError end @@ -56,6 +59,10 @@ module Gitlab def action_method raise NotImplementedError end + + def action_title + raise NotImplementedError + end end end end -- 2.24.1 From 203c9040571944f573d18db2bd477521b6c76535 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:19:17 +0100 Subject: [PATCH 19/47] Add icon_with_name_and_action and use it everywhere --- app/views/ci/status/_icon_with_name.html.haml | 11 +++++++ .../_icon_with_name_and_action.html.haml | 8 +++++ .../ci/builds/_build_pipeline.html.haml | 31 ------------------- .../_generic_commit_status_pipeline.html.haml | 10 ------ 4 files changed, 19 insertions(+), 41 deletions(-) create mode 100644 app/views/ci/status/_icon_with_name.html.haml create mode 100644 app/views/ci/status/_icon_with_name_and_action.html.haml delete mode 100644 app/views/projects/ci/builds/_build_pipeline.html.haml delete mode 100644 app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml diff --git a/app/views/ci/status/_icon_with_name.html.haml b/app/views/ci/status/_icon_with_name.html.haml new file mode 100644 index 00000000000..028e1fe9402 --- /dev/null +++ b/app/views/ci/status/_icon_with_name.html.haml @@ -0,0 +1,11 @@ +- detailed_status = subject.detailed_status(current_user) +- details_path = detailed_status.details_path if detailed_status.has_details? +- klass = "ci-status-icon ci-status-icon-#{detailed_status}" + +- if details_path + = link_to details_path, class: klass, data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do + %span{ class: klass }= custom_icon(detailed_status.icon) + .ci-status-text= subject.name +- else + %span{ class: klass }= custom_icon(detailed_status.icon) + .ci-status-text= subject.name diff --git a/app/views/ci/status/_icon_with_name_and_action.html.haml b/app/views/ci/status/_icon_with_name_and_action.html.haml new file mode 100644 index 00000000000..76db3b7f38a --- /dev/null +++ b/app/views/ci/status/_icon_with_name_and_action.html.haml @@ -0,0 +1,8 @@ += render "ci/status/icon_with_name", subject: subject + +- detailed_status = subject.detailed_status(current_user) +- if detailed_status.has_action? + = link_to detailed_status.action_path, method: detailed_status.action_method, + title: "#{subject.name}: #{detailed_status.action_title}", class: 'ci-action-icon-container' do + %i.ci-action-icon-wrapper + = icon(detailed_status.action_icon, class: detailed_status.action_class) diff --git a/app/views/projects/ci/builds/_build_pipeline.html.haml b/app/views/projects/ci/builds/_build_pipeline.html.haml deleted file mode 100644 index 41b9265fe5e..00000000000 --- a/app/views/projects/ci/builds/_build_pipeline.html.haml +++ /dev/null @@ -1,31 +0,0 @@ -- is_playable = subject.playable? && can?(current_user, :update_build, @project) -- can_cancel = subject.active? && can?(current_user, :update_build, @project) -- can_retry = subject.retryable? && can?(current_user, :update_build, @project) -- can_stop = subject.complete? && subject.stops_environment? && can?(current_user, :update_build, @project) - -- if can?(current_user, :read_build, @project) - = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject), data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } do - %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} - = ci_icon_for_status(subject.status) - .ci-status-text= subject.name - - - if is_playable - = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: "#{subject.name} - Play", class: 'ci-action-icon-container' do - %i.ci-action-icon-wrapper - = icon('play', class: 'ci-play-icon') - - elsif can_cancel - = link_to cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: "#{subject.name} - Cancel", class: 'ci-action-icon-container' do - %i.ci-action-icon-wrapper - = icon('ban') - - elsif can_retry - = link_to retry_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: "#{subject.name} - Retry", class: 'ci-action-icon-container' do - %i.ci-action-icon-wrapper - = icon('refresh') - - elsif can_stop - = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: "#{subject.name} - Stop", class: 'ci-action-icon-container' do - %i.ci-action-icon-wrapper - = icon('stop') - -- else - %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} - = ci_icon_for_status(subject.status) diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml deleted file mode 100644 index 7b82d913d29..00000000000 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml +++ /dev/null @@ -1,10 +0,0 @@ -%a{ data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } } - - if subject.target_url - = link_to subject.target_url do - %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} - = ci_icon_for_status(subject.status) - %span.ci-status-text= subject.name - - else - %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} - = ci_icon_for_status(subject.status) - %span.ci-status-text= subject.name -- 2.24.1 From 6296a1cc5d70e2e32a0a98e903415832ca29f281 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 17:52:24 +0100 Subject: [PATCH 20/47] Fix auto loading of constants for Ci Statuses --- app/models/ci/build.rb | 6 +++--- app/models/ci/pipeline.rb | 4 ++-- app/models/ci/stage.rb | 4 ++-- app/models/commit_status.rb | 4 ++-- app/views/ci/status/_icon_with_label.html.haml | 13 +++++++------ lib/gitlab/ci/status/build/cancelable.rb | 2 +- lib/gitlab/ci/status/build/play.rb | 2 +- lib/gitlab/ci/status/build/retryable.rb | 2 +- lib/gitlab/ci/status/build/stop.rb | 4 ++-- 9 files changed, 21 insertions(+), 20 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 73564dd2aa0..65ee327a8e5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -100,8 +100,8 @@ module Ci end end - def detailed_status - Gitlab::Ci::Status::Build::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Build::Factory.new(self, current_user).fabricate! end def manual? @@ -156,7 +156,7 @@ module Ci end def environment_action - self.options.fetch(:environment, {}).fetch(:action, 'start') + self.options.fetch(:environment, {}).fetch(:action, 'start') if self.options end def outdated_deployment? diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index fda8228a1e9..1f33106d358 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -336,8 +336,8 @@ module Ci .select { |merge_request| merge_request.head_pipeline.try(:id) == self.id } end - def detailed_status - Gitlab::Ci::Status::Pipeline::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Pipeline::Factory.new(self, current_user).fabricate! end private diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index d2a37c0a827..be52cce20f1 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -22,8 +22,8 @@ module Ci @status ||= statuses.latest.status end - def detailed_status - Gitlab::Ci::Status::Stage::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Stage::Factory.new(self, current_user).fabricate! end def statuses diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index fce16174e22..6548a7dda2c 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -132,7 +132,7 @@ class CommitStatus < ActiveRecord::Base false end - def detailed_status - Gitlab::Ci::Status::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Factory.new(self, current_user).fabricate! end end diff --git a/app/views/ci/status/_icon_with_label.html.haml b/app/views/ci/status/_icon_with_label.html.haml index 65a74e88444..d3fe332cc78 100644 --- a/app/views/ci/status/_icon_with_label.html.haml +++ b/app/views/ci/status/_icon_with_label.html.haml @@ -1,10 +1,11 @@ -- details_path = subject.details_path if subject.has_details?(current_user) -- klass = "ci-status ci-#{subject.status}" +- detailed_status = subject.detailed_status(current_user) +- details_path = detailed_status.details_path if detailed_status.has_details? +- klass = "ci-status ci-#{detailed_status}" - if details_path = link_to details_path, class: klass do - = custom_icon(status.icon) - = status.text + = custom_icon(detailed_status.icon) + = detailed_status.text - else %span{ class: klass } - = custom_icon(status.icon) - = status.text + = custom_icon(detailed_status.icon) + = detailed_status.text diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index a8830b04715..88be0cd924b 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Cancelable < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 70c08197ea1..57c7058fe84 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Play < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 3309e8808e1..69f2ad1d277 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Retryable < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 6fb51890bec..cd9bd959a7c 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -1,8 +1,8 @@ module Gitlab module Ci module Status - module Status - class Play < SimpleDelegator + module Build + class Stop < SimpleDelegator include Status::Extended def text -- 2.24.1 From a30dcc6771ec9244223ddb1de9293fe3fefeedf0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:16:23 +0100 Subject: [PATCH 21/47] Rename icon_with_label to icon_with_description --- app/views/admin/runners/show.html.haml | 2 +- ..._with_label.html.haml => _icon_with_description.html.haml} | 1 + app/views/projects/builds/_header.html.haml | 2 +- app/views/projects/ci/builds/_build.html.haml | 2 +- app/views/projects/ci/pipelines/_pipeline.html.haml | 2 +- .../generic_commit_statuses/_generic_commit_status.html.haml | 2 +- app/views/projects/pipelines/_info.html.haml | 2 +- app/views/projects/stage/_graph.html.haml | 4 ++-- 8 files changed, 9 insertions(+), 8 deletions(-) rename app/views/ci/status/{_icon_with_label.html.haml => _icon_with_description.html.haml} (99%) diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index fa8be25ffa8..badeb11b208 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -91,7 +91,7 @@ %strong ##{build.id} %td.status - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build %td.status - if project diff --git a/app/views/ci/status/_icon_with_label.html.haml b/app/views/ci/status/_icon_with_description.html.haml similarity index 99% rename from app/views/ci/status/_icon_with_label.html.haml rename to app/views/ci/status/_icon_with_description.html.haml index d3fe332cc78..34c923440d0 100644 --- a/app/views/ci/status/_icon_with_label.html.haml +++ b/app/views/ci/status/_icon_with_description.html.haml @@ -1,6 +1,7 @@ - detailed_status = subject.detailed_status(current_user) - details_path = detailed_status.details_path if detailed_status.has_details? - klass = "ci-status ci-#{detailed_status}" + - if details_path = link_to details_path, class: klass do = custom_icon(detailed_status.icon) diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index 5e4e30f08d5..85d1793ecb9 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -1,6 +1,6 @@ .content-block.build-header .header-content - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build Build %strong ##{@build.id} in pipeline diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 6b0cd3e49a0..4257bb86859 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -9,7 +9,7 @@ %tr.build.commit{class: ('retried' if retried)} %td.status - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build %td.branch-commit - if can?(current_user, :read_build, build) diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 84243e4306d..6dff955ea3d 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -4,7 +4,7 @@ %tr.commit %td.commit-link - = render "ci/status/icon_with_label", subject: pipeline + = render "ci/status/icon_with_description", subject: pipeline %td = link_to namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) do diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index 69cb1631ee8..1dd07ae1a2a 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -8,7 +8,7 @@ %tr.generic_commit_status{class: ('retried' if retried)} %td.status - = render "ci/status/icon_with_label", subject: generic_commit_status + = render "ci/status/icon_with_description", subject: generic_commit_status %td.generic_commit_status-link - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index f7385184a2b..d05697b4ee3 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,6 +1,6 @@ .page-content-header .header-main-content - = render "ci/status/icon_with_label", subject: @pipeline + = render "ci/status/icon_with_description", subject: @pipeline %strong Pipeline ##{@commit.pipelines.last.id} triggered #{time_ago_with_tooltip(@commit.authored_date)} by = author_avatar(@commit, size: 24) diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index d8c87fae5a1..cb845ae7f92 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -15,9 +15,9 @@ %li.build{ class: ("playable" if is_playable) } .curve .build-content - = render "projects/#{status.to_partial_path}_pipeline", subject: status + = render 'ci/status/icon_with_name_and_action', subject: status - else %li.build .curve .dropdown.inline.build-content - = render "projects/stage/in_stage_group", name: group_name, subject: grouped_statuses + = render 'projects/stage/in_stage_group', name: group_name, subject: grouped_statuses -- 2.24.1 From e8aae61ad07c3fc3d62394ba620c3b8654c29f65 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:18:30 +0100 Subject: [PATCH 22/47] Add action_class/action_title --- lib/gitlab/ci/status/build/cancelable.rb | 6 +++++- lib/gitlab/ci/status/build/play.rb | 8 ++++++++ lib/gitlab/ci/status/build/retryable.rb | 6 +++++- lib/gitlab/ci/status/build/stop.rb | 6 +++++- lib/gitlab/ci/status/core.rb | 7 +++++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index 88be0cd924b..a979fe7d573 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -10,7 +10,7 @@ module Gitlab end def action_icon - 'remove' + 'ban' end def action_path @@ -23,6 +23,10 @@ module Gitlab :post end + def action_title + 'Cancel' + end + def self.matches?(build, user) build.cancelable? end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 57c7058fe84..e3066d40a37 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -17,10 +17,18 @@ module Gitlab can?(user, :update_build, subject) end + def action_title + 'Play' + end + def action_icon 'play' end + def action_class + 'ci-play-icon' + end + def action_path play_namespace_project_build_path(subject.project.namespace, subject.project, diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 69f2ad1d277..8e38d6a8523 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -10,7 +10,11 @@ module Gitlab end def action_icon - 'repeat' + 'refresh' + end + + def action_title + 'Retry' end def action_path diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index cd9bd959a7c..487fd033960 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -26,7 +26,11 @@ module Gitlab end def action_icon - :play + 'stop' + end + + def action_title + 'Stop' end def action_path diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 7e9f6e35012..dd3a824e486 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -49,6 +49,9 @@ module Gitlab raise NotImplementedError end + def action_class + end + def action_path raise NotImplementedError end @@ -56,6 +59,10 @@ module Gitlab def action_method raise NotImplementedError end + + def action_title + raise NotImplementedError + end end end end -- 2.24.1 From be1d3376c6525f4ad24a6050d776fc3967e201a1 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:21:11 +0100 Subject: [PATCH 23/47] Revert some unneeded changes --- app/views/projects/stage/_graph.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index cb845ae7f92..c0ad7b07c77 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -15,7 +15,7 @@ %li.build{ class: ("playable" if is_playable) } .curve .build-content - = render 'ci/status/icon_with_name_and_action', subject: status + = render "projects/#{status.to_partial_path}_pipeline", subject: status - else %li.build .curve -- 2.24.1 From b64cf8405c551e79520e8ce4eef2dc17259c41e3 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 8 Dec 2016 17:58:32 +0000 Subject: [PATCH 24/47] Renders new icons for the pipeline graph --- app/helpers/ci_status_helper.rb | 66 +++++++------------ app/views/ci/status/_icon_with_name.html.haml | 3 +- .../_icon_with_name_and_action.html.haml | 2 +- ...ed.svg => _icon_status_canceled_graph.svg} | 0 ...ted.svg => _icon_status_created_graph.svg} | 0 ...iled.svg => _icon_status_failed_graph.svg} | 0 ...nual.svg => _icon_status_manual_graph.svg} | 0 ...ing.svg => _icon_status_pending_graph.svg} | 0 ...ing.svg => _icon_status_running_graph.svg} | 0 ...ped.svg => _icon_status_skipped_graph.svg} | 0 ...ess.svg => _icon_status_success_graph.svg} | 0 ...ing.svg => _icon_status_warning_graph.svg} | 0 12 files changed, 25 insertions(+), 46 deletions(-) rename app/views/shared/icons/{_icon_graph_job_cancelled.svg => _icon_status_canceled_graph.svg} (100%) rename app/views/shared/icons/{_icon_graph_job_created.svg => _icon_status_created_graph.svg} (100%) rename app/views/shared/icons/{_icon_graph_job_failed.svg => _icon_status_failed_graph.svg} (100%) rename app/views/shared/icons/{_icon_graph_job_manual.svg => _icon_status_manual_graph.svg} (100%) rename app/views/shared/icons/{_icon_graph_job_pending.svg => _icon_status_pending_graph.svg} (100%) rename app/views/shared/icons/{_icon_graph_job_running.svg => _icon_status_running_graph.svg} (100%) rename app/views/shared/icons/{_icon_graph_job_skipped.svg => _icon_status_skipped_graph.svg} (100%) rename app/views/shared/icons/{_icon_graph_job_success.svg => _icon_status_success_graph.svg} (100%) rename app/views/shared/icons/{_icon_graph_job_warning.svg => _icon_status_warning_graph.svg} (100%) diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 7ddef2cdd5f..d9f5e01f0dc 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -25,54 +25,32 @@ module CiStatusHelper status.humanize end - def ci_icon_for_status(status, graph: nil) + def ci_icon_for_status(status) if detailed_status?(status) return custom_icon(status.icon) end - if graph - icon_name = - case status - when 'success' - 'icon_graph_job_success' - when 'success_with_warnings' - 'icon_graph_job_warning' - when 'failed' - 'icon_graph_job_failed' - when 'pending' - 'icon_graph_job_pending' - when 'running' - 'icon_graph_job_running' - when 'created' - 'icon_graph_job_created' - when 'skipped' - 'icon_graph_job_skipped' - else - 'icon_graph_job_canceled' - end - else - icon_name = - case status - when 'success' - 'icon_status_success' - when 'success_with_warnings' - 'icon_status_warning' - when 'failed' - 'icon_status_failed' - when 'pending' - 'icon_status_pending' - when 'running' - 'icon_status_running' - when 'play' - 'icon_play' - when 'created' - 'icon_status_created' - when 'skipped' - 'icon_status_skipped' - else - 'icon_status_canceled' - end - end + icon_name = + case status + when 'success' + 'icon_status_success' + when 'success_with_warnings' + 'icon_status_warning' + when 'failed' + 'icon_status_failed' + when 'pending' + 'icon_status_pending' + when 'running' + 'icon_status_running' + when 'play' + 'icon_play' + when 'created' + 'icon_status_created' + when 'skipped' + 'icon_status_skipped' + else + 'icon_status_canceled' + end custom_icon(icon_name) end diff --git a/app/views/ci/status/_icon_with_name.html.haml b/app/views/ci/status/_icon_with_name.html.haml index 028e1fe9402..a467316ef47 100644 --- a/app/views/ci/status/_icon_with_name.html.haml +++ b/app/views/ci/status/_icon_with_name.html.haml @@ -1,10 +1,11 @@ - detailed_status = subject.detailed_status(current_user) - details_path = detailed_status.details_path if detailed_status.has_details? - klass = "ci-status-icon ci-status-icon-#{detailed_status}" +- status_icon = graph ? "#{detailed_status.icon}_graph" : detailed_status.icon - if details_path = link_to details_path, class: klass, data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do - %span{ class: klass }= custom_icon(detailed_status.icon) + %span{ class: klass }= custom_icon(status_icon) .ci-status-text= subject.name - else %span{ class: klass }= custom_icon(detailed_status.icon) diff --git a/app/views/ci/status/_icon_with_name_and_action.html.haml b/app/views/ci/status/_icon_with_name_and_action.html.haml index 76db3b7f38a..b912c212534 100644 --- a/app/views/ci/status/_icon_with_name_and_action.html.haml +++ b/app/views/ci/status/_icon_with_name_and_action.html.haml @@ -1,4 +1,4 @@ -= render "ci/status/icon_with_name", subject: subject += render "ci/status/icon_with_name", subject: subject, graph: true - detailed_status = subject.detailed_status(current_user) - if detailed_status.has_action? diff --git a/app/views/shared/icons/_icon_graph_job_cancelled.svg b/app/views/shared/icons/_icon_status_canceled_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_cancelled.svg rename to app/views/shared/icons/_icon_status_canceled_graph.svg diff --git a/app/views/shared/icons/_icon_graph_job_created.svg b/app/views/shared/icons/_icon_status_created_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_created.svg rename to app/views/shared/icons/_icon_status_created_graph.svg diff --git a/app/views/shared/icons/_icon_graph_job_failed.svg b/app/views/shared/icons/_icon_status_failed_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_failed.svg rename to app/views/shared/icons/_icon_status_failed_graph.svg diff --git a/app/views/shared/icons/_icon_graph_job_manual.svg b/app/views/shared/icons/_icon_status_manual_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_manual.svg rename to app/views/shared/icons/_icon_status_manual_graph.svg diff --git a/app/views/shared/icons/_icon_graph_job_pending.svg b/app/views/shared/icons/_icon_status_pending_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_pending.svg rename to app/views/shared/icons/_icon_status_pending_graph.svg diff --git a/app/views/shared/icons/_icon_graph_job_running.svg b/app/views/shared/icons/_icon_status_running_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_running.svg rename to app/views/shared/icons/_icon_status_running_graph.svg diff --git a/app/views/shared/icons/_icon_graph_job_skipped.svg b/app/views/shared/icons/_icon_status_skipped_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_skipped.svg rename to app/views/shared/icons/_icon_status_skipped_graph.svg diff --git a/app/views/shared/icons/_icon_graph_job_success.svg b/app/views/shared/icons/_icon_status_success_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_success.svg rename to app/views/shared/icons/_icon_status_success_graph.svg diff --git a/app/views/shared/icons/_icon_graph_job_warning.svg b/app/views/shared/icons/_icon_status_warning_graph.svg similarity index 100% rename from app/views/shared/icons/_icon_graph_job_warning.svg rename to app/views/shared/icons/_icon_status_warning_graph.svg -- 2.24.1 From 1e8271b60bcd16b6fcc20d574c9583d593084eef Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 9 Dec 2016 17:24:21 +0000 Subject: [PATCH 25/47] Adds new partial for graph icons. Fix tests --- app/views/ci/status/_graph_icon_with_name.html.haml | 12 ++++++++++++ .../_graph_icon_with_name_and_action.html.haml | 8 ++++++++ app/views/ci/status/_icon_with_name.html.haml | 3 +-- .../ci/status/_icon_with_name_and_action.html.haml | 2 +- app/views/projects/stage/_graph.html.haml | 2 +- app/views/projects/stage/_in_stage_group.html.haml | 2 +- spec/features/projects/pipelines/pipeline_spec.rb | 6 +++--- 7 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 app/views/ci/status/_graph_icon_with_name.html.haml create mode 100644 app/views/ci/status/_graph_icon_with_name_and_action.html.haml diff --git a/app/views/ci/status/_graph_icon_with_name.html.haml b/app/views/ci/status/_graph_icon_with_name.html.haml new file mode 100644 index 00000000000..51037a3bd20 --- /dev/null +++ b/app/views/ci/status/_graph_icon_with_name.html.haml @@ -0,0 +1,12 @@ +- detailed_status = subject.detailed_status(current_user) +- details_path = detailed_status.details_path if detailed_status.has_details? +- klass = "ci-status-icon ci-status-icon-#{detailed_status}" +- graph_status_icon = "#{detailed_status.icon}_graph" + +- if details_path + = link_to details_path, class: klass, data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do + %span{ class: klass }= custom_icon(graph_status_icon) + .ci-status-text= subject.name +- else + %span{ class: klass }= custom_icon(graph_status_icon) + .ci-status-text= subject.name diff --git a/app/views/ci/status/_graph_icon_with_name_and_action.html.haml b/app/views/ci/status/_graph_icon_with_name_and_action.html.haml new file mode 100644 index 00000000000..525075ced70 --- /dev/null +++ b/app/views/ci/status/_graph_icon_with_name_and_action.html.haml @@ -0,0 +1,8 @@ += render "ci/status/graph_icon_with_name", subject: subject + +- detailed_status = subject.detailed_status(current_user) +- if detailed_status.has_action? + = link_to detailed_status.action_path, method: detailed_status.action_method, + title: "#{subject.name}: #{detailed_status.action_title}", class: 'ci-action-icon-container' do + %i.ci-action-icon-wrapper + = icon(detailed_status.action_icon, class: detailed_status.action_class) diff --git a/app/views/ci/status/_icon_with_name.html.haml b/app/views/ci/status/_icon_with_name.html.haml index a467316ef47..028e1fe9402 100644 --- a/app/views/ci/status/_icon_with_name.html.haml +++ b/app/views/ci/status/_icon_with_name.html.haml @@ -1,11 +1,10 @@ - detailed_status = subject.detailed_status(current_user) - details_path = detailed_status.details_path if detailed_status.has_details? - klass = "ci-status-icon ci-status-icon-#{detailed_status}" -- status_icon = graph ? "#{detailed_status.icon}_graph" : detailed_status.icon - if details_path = link_to details_path, class: klass, data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do - %span{ class: klass }= custom_icon(status_icon) + %span{ class: klass }= custom_icon(detailed_status.icon) .ci-status-text= subject.name - else %span{ class: klass }= custom_icon(detailed_status.icon) diff --git a/app/views/ci/status/_icon_with_name_and_action.html.haml b/app/views/ci/status/_icon_with_name_and_action.html.haml index b912c212534..76db3b7f38a 100644 --- a/app/views/ci/status/_icon_with_name_and_action.html.haml +++ b/app/views/ci/status/_icon_with_name_and_action.html.haml @@ -1,4 +1,4 @@ -= render "ci/status/icon_with_name", subject: subject, graph: true += render "ci/status/icon_with_name", subject: subject - detailed_status = subject.detailed_status(current_user) - if detailed_status.has_action? diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index 745b6d143f4..255091cbfe8 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -14,7 +14,7 @@ %li.build{ class: ("playable" if is_playable) } .curve .build-content - = render 'ci/status/icon_with_name_and_action', subject: status + = render 'ci/status/graph_icon_with_name_and_action', subject: status - else %li.build .curve diff --git a/app/views/projects/stage/_in_stage_group.html.haml b/app/views/projects/stage/_in_stage_group.html.haml index 5c9b6549b37..70101ccf806 100644 --- a/app/views/projects/stage/_in_stage_group.html.haml +++ b/app/views/projects/stage/_in_stage_group.html.haml @@ -10,4 +10,4 @@ %ul - subject.each do |status| %li.dropdown-build - = render 'ci/status/icon_with_name_and_action', subject: status + = render 'ci/status/graph_icon_with_name_and_action', subject: status diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index e21de05ac64..7358931b9f0 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -40,7 +40,7 @@ describe "Pipelines", feature: true, js: true do context 'pipeline graph' do it 'shows a running icon and a cancel action for the running build' do - page.within('.stage-column:first-child .build:first-child') do + page.within('.stage-column:nth-child(2) .build:first-child') do expect(page).to have_selector('.ci-status-icon-running') expect(page).to have_content('deploy') expect(page).to have_selector('.ci-action-icon-container .fa-ban') @@ -56,7 +56,7 @@ describe "Pipelines", feature: true, js: true do end it 'shows the failed icon and a retry action for the failed build' do - page.within('.stage-column:nth-child(2) .build') do + page.within('.stage-column:first-child .build') do expect(page).to have_selector('.ci-status-icon-failed') expect(page).to have_content('test') expect(page).to have_selector('.ci-action-icon-container .fa-refresh') @@ -64,7 +64,7 @@ describe "Pipelines", feature: true, js: true do end it 'shows the skipped icon and a play action for the manual build' do - page.within('.stage-column:first-child .build:nth-child(2)') do + page.within('.stage-column:nth-child(2) .build:nth-child(2)') do expect(page).to have_selector('.ci-status-icon-skipped') expect(page).to have_content('manual') expect(page).to have_selector('.ci-action-icon-container .ci-play-icon') -- 2.24.1 From a2a16503a8ecfe1b38f6ef8e31c96b53537dfe02 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 12 Dec 2016 12:12:37 +0000 Subject: [PATCH 26/47] Fix vertical alignment of action icon with status icon --- app/assets/stylesheets/pages/pipelines.scss | 42 +++++++++------------ app/views/projects/stage/_graph.html.haml | 3 +- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 304a7932a06..75b127eb7f6 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -654,6 +654,7 @@ i { font-size: 11px; + margin-top: 0; } } @@ -672,33 +673,26 @@ } // Action Icons -.ci-action-icon-container { - padding: 0; +.ci-action-icon-container .ci-action-icon-wrapper { + float: right; + margin-top: -1px; - .ci-action-icon-wrapper { - display: inline-block; - float: right; - - i { - color: $stage-badge-text; - border-radius: 100%; - border: 1px solid $stage-badge-text; - text-align: center; - display: table-cell; - vertical-align: middle; - padding: 5px; - font-size: 13px; - background: $white-light; + i { + color: $stage-badge-text; + border-radius: 100%; + border: 1px solid $stage-badge-text; + padding: 5px 6px; + font-size: 13px; + background: $white-light; - &:hover { - color: $gl-text-color; - background-color: $stage-hover-bg; - border: 1px solid $stage-hover-bg; - } + &:hover { + color: $gl-text-color; + background-color: $stage-hover-bg; + border: 1px solid $stage-hover-bg; } + } - .ci-play-icon { - padding: 5px 4px 5px 7px; - } + .ci-play-icon { + padding: 5px 5px 5px 7px; } } diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index 255091cbfe8..cf3050eea63 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -10,8 +10,7 @@ - status_groups.each do |group_name, grouped_statuses| - if grouped_statuses.one? - status = grouped_statuses.first - - is_playable = status.playable? && can?(current_user, :update_build, @project) - %li.build{ class: ("playable" if is_playable) } + %li.build .curve .build-content = render 'ci/status/graph_icon_with_name_and_action', subject: status -- 2.24.1 From 1de2edc1f14a805afa23c6fde79bdd499af75f33 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 12 Dec 2016 21:09:17 +0000 Subject: [PATCH 27/47] Fix pipeline stroke position --- app/assets/stylesheets/pages/pipelines.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 75b127eb7f6..4a5b0b5922c 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -427,7 +427,7 @@ width: 21px; height: 25px; position: absolute; - top: -31px; + top: -30px; border-top: 2px solid $border-color; } -- 2.24.1 From 0b9df1bd93f9b6b6a5714431d8b815b0225917b5 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 13 Dec 2016 11:30:12 +0000 Subject: [PATCH 28/47] Replace old icons with new ones --- app/views/shared/icons/_icon_status_canceled.svg | 2 +- app/views/shared/icons/_icon_status_canceled_graph.svg | 1 - app/views/shared/icons/_icon_status_created.svg | 2 +- app/views/shared/icons/_icon_status_created_graph.svg | 1 - app/views/shared/icons/_icon_status_failed.svg | 2 +- app/views/shared/icons/_icon_status_failed_graph.svg | 1 - .../{_icon_status_manual_graph.svg => _icon_status_manual.svg} | 0 app/views/shared/icons/_icon_status_pending.svg | 2 +- app/views/shared/icons/_icon_status_pending_graph.svg | 1 - app/views/shared/icons/_icon_status_running.svg | 2 +- app/views/shared/icons/_icon_status_running_graph.svg | 1 - app/views/shared/icons/_icon_status_skipped.svg | 2 +- app/views/shared/icons/_icon_status_skipped_graph.svg | 1 - app/views/shared/icons/_icon_status_success.svg | 2 +- app/views/shared/icons/_icon_status_success_graph.svg | 1 - app/views/shared/icons/_icon_status_warning.svg | 2 +- app/views/shared/icons/_icon_status_warning_graph.svg | 1 - 17 files changed, 8 insertions(+), 16 deletions(-) mode change 100644 => 100755 app/views/shared/icons/_icon_status_canceled.svg delete mode 100755 app/views/shared/icons/_icon_status_canceled_graph.svg mode change 100644 => 100755 app/views/shared/icons/_icon_status_created.svg delete mode 100755 app/views/shared/icons/_icon_status_created_graph.svg mode change 100644 => 100755 app/views/shared/icons/_icon_status_failed.svg delete mode 100755 app/views/shared/icons/_icon_status_failed_graph.svg rename app/views/shared/icons/{_icon_status_manual_graph.svg => _icon_status_manual.svg} (100%) mode change 100644 => 100755 app/views/shared/icons/_icon_status_pending.svg delete mode 100755 app/views/shared/icons/_icon_status_pending_graph.svg mode change 100644 => 100755 app/views/shared/icons/_icon_status_running.svg delete mode 100755 app/views/shared/icons/_icon_status_running_graph.svg mode change 100644 => 100755 app/views/shared/icons/_icon_status_skipped.svg delete mode 100755 app/views/shared/icons/_icon_status_skipped_graph.svg mode change 100644 => 100755 app/views/shared/icons/_icon_status_success.svg delete mode 100755 app/views/shared/icons/_icon_status_success_graph.svg mode change 100644 => 100755 app/views/shared/icons/_icon_status_warning.svg delete mode 100755 app/views/shared/icons/_icon_status_warning_graph.svg diff --git a/app/views/shared/icons/_icon_status_canceled.svg b/app/views/shared/icons/_icon_status_canceled.svg old mode 100644 new mode 100755 index 41a210a8ed9..bd5d04e1cd7 --- a/app/views/shared/icons/_icon_status_canceled.svg +++ b/app/views/shared/icons/_icon_status_canceled.svg @@ -1 +1 @@ - + diff --git a/app/views/shared/icons/_icon_status_canceled_graph.svg b/app/views/shared/icons/_icon_status_canceled_graph.svg deleted file mode 100755 index bd5d04e1cd7..00000000000 --- a/app/views/shared/icons/_icon_status_canceled_graph.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/app/views/shared/icons/_icon_status_created.svg b/app/views/shared/icons/_icon_status_created.svg old mode 100644 new mode 100755 index 1f5c3b51b03..326ad04e017 --- a/app/views/shared/icons/_icon_status_created.svg +++ b/app/views/shared/icons/_icon_status_created.svg @@ -1 +1 @@ - + diff --git a/app/views/shared/icons/_icon_status_created_graph.svg b/app/views/shared/icons/_icon_status_created_graph.svg deleted file mode 100755 index 326ad04e017..00000000000 --- a/app/views/shared/icons/_icon_status_created_graph.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/app/views/shared/icons/_icon_status_failed.svg b/app/views/shared/icons/_icon_status_failed.svg old mode 100644 new mode 100755 index af267b8938a..64da5aa31fc --- a/app/views/shared/icons/_icon_status_failed.svg +++ b/app/views/shared/icons/_icon_status_failed.svg @@ -1 +1 @@ - + diff --git a/app/views/shared/icons/_icon_status_failed_graph.svg b/app/views/shared/icons/_icon_status_failed_graph.svg deleted file mode 100755 index 64da5aa31fc..00000000000 --- a/app/views/shared/icons/_icon_status_failed_graph.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/app/views/shared/icons/_icon_status_manual_graph.svg b/app/views/shared/icons/_icon_status_manual.svg similarity index 100% rename from app/views/shared/icons/_icon_status_manual_graph.svg rename to app/views/shared/icons/_icon_status_manual.svg diff --git a/app/views/shared/icons/_icon_status_pending.svg b/app/views/shared/icons/_icon_status_pending.svg old mode 100644 new mode 100755 index 516231d1b44..02d5da407e3 --- a/app/views/shared/icons/_icon_status_pending.svg +++ b/app/views/shared/icons/_icon_status_pending.svg @@ -1 +1 @@ - + diff --git a/app/views/shared/icons/_icon_status_pending_graph.svg b/app/views/shared/icons/_icon_status_pending_graph.svg deleted file mode 100755 index 02d5da407e3..00000000000 --- a/app/views/shared/icons/_icon_status_pending_graph.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/app/views/shared/icons/_icon_status_running.svg b/app/views/shared/icons/_icon_status_running.svg old mode 100644 new mode 100755 index d2618bce200..532f4fee33c --- a/app/views/shared/icons/_icon_status_running.svg +++ b/app/views/shared/icons/_icon_status_running.svg @@ -1 +1 @@ - + diff --git a/app/views/shared/icons/_icon_status_running_graph.svg b/app/views/shared/icons/_icon_status_running_graph.svg deleted file mode 100755 index 532f4fee33c..00000000000 --- a/app/views/shared/icons/_icon_status_running_graph.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/app/views/shared/icons/_icon_status_skipped.svg b/app/views/shared/icons/_icon_status_skipped.svg old mode 100644 new mode 100755 index 701f33bcbea..1998dfef9ea --- a/app/views/shared/icons/_icon_status_skipped.svg +++ b/app/views/shared/icons/_icon_status_skipped.svg @@ -1 +1 @@ - + diff --git a/app/views/shared/icons/_icon_status_skipped_graph.svg b/app/views/shared/icons/_icon_status_skipped_graph.svg deleted file mode 100755 index 1998dfef9ea..00000000000 --- a/app/views/shared/icons/_icon_status_skipped_graph.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/app/views/shared/icons/_icon_status_success.svg b/app/views/shared/icons/_icon_status_success.svg old mode 100644 new mode 100755 index b7c21ba6971..eed5006bebe --- a/app/views/shared/icons/_icon_status_success.svg +++ b/app/views/shared/icons/_icon_status_success.svg @@ -1 +1 @@ - + diff --git a/app/views/shared/icons/_icon_status_success_graph.svg b/app/views/shared/icons/_icon_status_success_graph.svg deleted file mode 100755 index eed5006bebe..00000000000 --- a/app/views/shared/icons/_icon_status_success_graph.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/app/views/shared/icons/_icon_status_warning.svg b/app/views/shared/icons/_icon_status_warning.svg old mode 100644 new mode 100755 index 9191e0050a6..cb785635b7e --- a/app/views/shared/icons/_icon_status_warning.svg +++ b/app/views/shared/icons/_icon_status_warning.svg @@ -1 +1 @@ - + diff --git a/app/views/shared/icons/_icon_status_warning_graph.svg b/app/views/shared/icons/_icon_status_warning_graph.svg deleted file mode 100755 index cb785635b7e..00000000000 --- a/app/views/shared/icons/_icon_status_warning_graph.svg +++ /dev/null @@ -1 +0,0 @@ - -- 2.24.1 From a06016d00cd363eec7ef748e5fda6459c911eaba Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 13 Dec 2016 11:38:34 +0000 Subject: [PATCH 29/47] Remove unneeded partial --- app/views/ci/status/_graph_icon_with_name.html.haml | 12 ------------ .../_graph_icon_with_name_and_action.html.haml | 8 -------- app/views/projects/stage/_graph.html.haml | 2 +- app/views/projects/stage/_in_stage_group.html.haml | 2 +- 4 files changed, 2 insertions(+), 22 deletions(-) delete mode 100644 app/views/ci/status/_graph_icon_with_name.html.haml delete mode 100644 app/views/ci/status/_graph_icon_with_name_and_action.html.haml diff --git a/app/views/ci/status/_graph_icon_with_name.html.haml b/app/views/ci/status/_graph_icon_with_name.html.haml deleted file mode 100644 index 51037a3bd20..00000000000 --- a/app/views/ci/status/_graph_icon_with_name.html.haml +++ /dev/null @@ -1,12 +0,0 @@ -- detailed_status = subject.detailed_status(current_user) -- details_path = detailed_status.details_path if detailed_status.has_details? -- klass = "ci-status-icon ci-status-icon-#{detailed_status}" -- graph_status_icon = "#{detailed_status.icon}_graph" - -- if details_path - = link_to details_path, class: klass, data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do - %span{ class: klass }= custom_icon(graph_status_icon) - .ci-status-text= subject.name -- else - %span{ class: klass }= custom_icon(graph_status_icon) - .ci-status-text= subject.name diff --git a/app/views/ci/status/_graph_icon_with_name_and_action.html.haml b/app/views/ci/status/_graph_icon_with_name_and_action.html.haml deleted file mode 100644 index 525075ced70..00000000000 --- a/app/views/ci/status/_graph_icon_with_name_and_action.html.haml +++ /dev/null @@ -1,8 +0,0 @@ -= render "ci/status/graph_icon_with_name", subject: subject - -- detailed_status = subject.detailed_status(current_user) -- if detailed_status.has_action? - = link_to detailed_status.action_path, method: detailed_status.action_method, - title: "#{subject.name}: #{detailed_status.action_title}", class: 'ci-action-icon-container' do - %i.ci-action-icon-wrapper - = icon(detailed_status.action_icon, class: detailed_status.action_class) diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index cf3050eea63..6d280468262 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -13,7 +13,7 @@ %li.build .curve .build-content - = render 'ci/status/graph_icon_with_name_and_action', subject: status + = render 'ci/status/icon_with_name_and_action', subject: status - else %li.build .curve diff --git a/app/views/projects/stage/_in_stage_group.html.haml b/app/views/projects/stage/_in_stage_group.html.haml index 70101ccf806..5c9b6549b37 100644 --- a/app/views/projects/stage/_in_stage_group.html.haml +++ b/app/views/projects/stage/_in_stage_group.html.haml @@ -10,4 +10,4 @@ %ul - subject.each do |status| %li.dropdown-build - = render 'ci/status/graph_icon_with_name_and_action', subject: status + = render 'ci/status/icon_with_name_and_action', subject: status -- 2.24.1 From 92b0f54ea222ce4c4437a50683c972bacc1fee06 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 13 Dec 2016 11:39:13 +0000 Subject: [PATCH 30/47] Fix graph stroke position --- app/assets/stylesheets/pages/pipelines.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 4a5b0b5922c..15da30dda2b 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -427,7 +427,7 @@ width: 21px; height: 25px; position: absolute; - top: -30px; + top: -32px; border-top: 2px solid $border-color; } -- 2.24.1 From 863146d42ed8fd44ab0c0f11c873fcdbbfcb28fc Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 14:16:28 +0000 Subject: [PATCH 31/47] Remove unused file --- app/views/ci/status/_icon_with_description.html.haml | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 app/views/ci/status/_icon_with_description.html.haml diff --git a/app/views/ci/status/_icon_with_description.html.haml b/app/views/ci/status/_icon_with_description.html.haml deleted file mode 100644 index 34c923440d0..00000000000 --- a/app/views/ci/status/_icon_with_description.html.haml +++ /dev/null @@ -1,12 +0,0 @@ -- detailed_status = subject.detailed_status(current_user) -- details_path = detailed_status.details_path if detailed_status.has_details? -- klass = "ci-status ci-#{detailed_status}" - -- if details_path - = link_to details_path, class: klass do - = custom_icon(detailed_status.icon) - = detailed_status.text -- else - %span{ class: klass } - = custom_icon(detailed_status.icon) - = detailed_status.text -- 2.24.1 From 4ba48d6b05f66dfccc829db588312109b8b015a1 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 14:17:02 +0000 Subject: [PATCH 32/47] Merge two partials into one. Rename it to match the other partials --- ...e_graph_icon_with_name_and_action.html.haml | 18 ++++++++++++++++++ app/views/ci/status/_icon_with_name.html.haml | 11 ----------- .../_icon_with_name_and_action.html.haml | 8 -------- app/views/projects/stage/_graph.html.haml | 2 +- .../projects/stage/_in_stage_group.html.haml | 2 +- 5 files changed, 20 insertions(+), 21 deletions(-) create mode 100644 app/views/ci/status/_badge_graph_icon_with_name_and_action.html.haml delete mode 100644 app/views/ci/status/_icon_with_name.html.haml delete mode 100644 app/views/ci/status/_icon_with_name_and_action.html.haml diff --git a/app/views/ci/status/_badge_graph_icon_with_name_and_action.html.haml b/app/views/ci/status/_badge_graph_icon_with_name_and_action.html.haml new file mode 100644 index 00000000000..12a55735559 --- /dev/null +++ b/app/views/ci/status/_badge_graph_icon_with_name_and_action.html.haml @@ -0,0 +1,18 @@ +- detailed_status = subject.detailed_status(current_user) +- details_path = detailed_status.details_path if detailed_status.has_details? +- klass = "ci-status-icon ci-status-icon-#{detailed_status}" + +- if details_path + = link_to details_path, class: klass, + data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do + %span{ class: klass }= custom_icon(detailed_status.icon) + .ci-status-text= subject.name +- else + %span{ class: klass }= custom_icon(detailed_status.icon) + .ci-status-text= subject.name + +- if detailed_status.has_action? + = link_to detailed_status.action_path, method: detailed_status.action_method, + title: "#{subject.name}: #{detailed_status.action_title}", class: 'ci-action-icon-container' do + %i.ci-action-icon-wrapper + = icon(detailed_status.action_icon, class: detailed_status.action_class) diff --git a/app/views/ci/status/_icon_with_name.html.haml b/app/views/ci/status/_icon_with_name.html.haml deleted file mode 100644 index 028e1fe9402..00000000000 --- a/app/views/ci/status/_icon_with_name.html.haml +++ /dev/null @@ -1,11 +0,0 @@ -- detailed_status = subject.detailed_status(current_user) -- details_path = detailed_status.details_path if detailed_status.has_details? -- klass = "ci-status-icon ci-status-icon-#{detailed_status}" - -- if details_path - = link_to details_path, class: klass, data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do - %span{ class: klass }= custom_icon(detailed_status.icon) - .ci-status-text= subject.name -- else - %span{ class: klass }= custom_icon(detailed_status.icon) - .ci-status-text= subject.name diff --git a/app/views/ci/status/_icon_with_name_and_action.html.haml b/app/views/ci/status/_icon_with_name_and_action.html.haml deleted file mode 100644 index 76db3b7f38a..00000000000 --- a/app/views/ci/status/_icon_with_name_and_action.html.haml +++ /dev/null @@ -1,8 +0,0 @@ -= render "ci/status/icon_with_name", subject: subject - -- detailed_status = subject.detailed_status(current_user) -- if detailed_status.has_action? - = link_to detailed_status.action_path, method: detailed_status.action_method, - title: "#{subject.name}: #{detailed_status.action_title}", class: 'ci-action-icon-container' do - %i.ci-action-icon-wrapper - = icon(detailed_status.action_icon, class: detailed_status.action_class) diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index 6d280468262..ff86d93f354 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -13,7 +13,7 @@ %li.build .curve .build-content - = render 'ci/status/icon_with_name_and_action', subject: status + = render 'ci/status/bagde_graph_icon_with_name_and_action', subject: status - else %li.build .curve diff --git a/app/views/projects/stage/_in_stage_group.html.haml b/app/views/projects/stage/_in_stage_group.html.haml index 5c9b6549b37..e4a7613998d 100644 --- a/app/views/projects/stage/_in_stage_group.html.haml +++ b/app/views/projects/stage/_in_stage_group.html.haml @@ -10,4 +10,4 @@ %ul - subject.each do |status| %li.dropdown-build - = render 'ci/status/icon_with_name_and_action', subject: status + = render 'ci/status/bagde_graph_icon_with_name_and_action', subject: status -- 2.24.1 From 5b1a38564efdba1850d7195b01146c6b49ffb29a Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 14:19:18 +0000 Subject: [PATCH 33/47] Fix scss error --- app/assets/stylesheets/pages/pipelines.scss | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 23312259f73..26487b2acf9 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -665,14 +665,15 @@ color: $gl-text-color; } - .stage { - max-width: 100px; - width: 100px; - } + .stage { + max-width: 100px; + width: 100px; + } - .ci-status-icon svg { - height: 18px; - width: 18px; + .ci-status-icon svg { + height: 18px; + width: 18px; + } } } } -- 2.24.1 From 268a201cce4da7225841b651336835876be43fc2 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 14:24:25 +0000 Subject: [PATCH 34/47] Rename file according to review --- ...ph_icon_with_name_and_action.html.haml => _graph_badge.haml} | 2 ++ app/views/projects/stage/_graph.html.haml | 2 +- app/views/projects/stage/_in_stage_group.html.haml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) rename app/views/ci/status/{_badge_graph_icon_with_name_and_action.html.haml => _graph_badge.haml} (91%) diff --git a/app/views/ci/status/_badge_graph_icon_with_name_and_action.html.haml b/app/views/ci/status/_graph_badge.haml similarity index 91% rename from app/views/ci/status/_badge_graph_icon_with_name_and_action.html.haml rename to app/views/ci/status/_graph_badge.haml index 12a55735559..7c6d36217f0 100644 --- a/app/views/ci/status/_badge_graph_icon_with_name_and_action.html.haml +++ b/app/views/ci/status/_graph_badge.haml @@ -1,3 +1,5 @@ +-# Renders the graph node with both the status icon, status name and action icon + - detailed_status = subject.detailed_status(current_user) - details_path = detailed_status.details_path if detailed_status.has_details? - klass = "ci-status-icon ci-status-icon-#{detailed_status}" diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index ff86d93f354..b70b574e687 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -13,7 +13,7 @@ %li.build .curve .build-content - = render 'ci/status/bagde_graph_icon_with_name_and_action', subject: status + = render 'ci/status/graph_badge', subject: status - else %li.build .curve diff --git a/app/views/projects/stage/_in_stage_group.html.haml b/app/views/projects/stage/_in_stage_group.html.haml index e4a7613998d..b03837d1211 100644 --- a/app/views/projects/stage/_in_stage_group.html.haml +++ b/app/views/projects/stage/_in_stage_group.html.haml @@ -10,4 +10,4 @@ %ul - subject.each do |status| %li.dropdown-build - = render 'ci/status/bagde_graph_icon_with_name_and_action', subject: status + = render 'ci/status/graph_badge', subject: status -- 2.24.1 From 281237d7731cfa6669dd650fdfabfe612082d47f Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 15:04:26 +0000 Subject: [PATCH 35/47] Changes after review Remove empty line --- app/models/ability.rb | 6 ------ app/views/ci/status/_graph_badge.haml | 3 +-- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index ce461caf686..fa8f8bc3a5f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,10 +1,4 @@ class Ability - module Allowable - def can?(user, action, subject) - Ability.allowed?(user, action, subject) - end - end - class << self # Given a list of users and a project this method returns the users that can # read the given project. diff --git a/app/views/ci/status/_graph_badge.haml b/app/views/ci/status/_graph_badge.haml index 7c6d36217f0..839b4334713 100644 --- a/app/views/ci/status/_graph_badge.haml +++ b/app/views/ci/status/_graph_badge.haml @@ -5,8 +5,7 @@ - klass = "ci-status-icon ci-status-icon-#{detailed_status}" - if details_path - = link_to details_path, class: klass, - data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do + = link_to details_path, data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do %span{ class: klass }= custom_icon(detailed_status.icon) .ci-status-text= subject.name - else -- 2.24.1 From e138646aad4a836459999a399cdd9eca1e09847d Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 15:25:38 +0000 Subject: [PATCH 36/47] Fix broken tests --- spec/features/projects/pipelines/pipeline_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 7358931b9f0..5094fcf33e8 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -40,7 +40,7 @@ describe "Pipelines", feature: true, js: true do context 'pipeline graph' do it 'shows a running icon and a cancel action for the running build' do - page.within('.stage-column:nth-child(2) .build:first-child') do + page.within('.stage-column:nth-child(3) .build:first-child') do expect(page).to have_selector('.ci-status-icon-running') expect(page).to have_content('deploy') expect(page).to have_selector('.ci-action-icon-container .fa-ban') @@ -48,7 +48,7 @@ describe "Pipelines", feature: true, js: true do end it 'shows the success icon and a retry action for the successfull build' do - page.within('.stage-column:nth-child(3)') do + page.within('.stage-column:nth-child(2) .build:first-child') do expect(page).to have_selector('.ci-status-icon-success') expect(page).to have_content('build') expect(page).to have_selector('.ci-action-icon-container .fa-refresh') @@ -64,7 +64,7 @@ describe "Pipelines", feature: true, js: true do end it 'shows the skipped icon and a play action for the manual build' do - page.within('.stage-column:nth-child(2) .build:nth-child(2)') do + page.within('.stage-column:nth-child(3) .build:nth-child(2)') do expect(page).to have_selector('.ci-status-icon-skipped') expect(page).to have_content('manual') expect(page).to have_selector('.ci-action-icon-container .ci-play-icon') -- 2.24.1 From a0ba72ee28ff666e651d3a9cdef19a9d35c22616 Mon Sep 17 00:00:00 2001 From: Dimitrie Hoekstra Date: Wed, 14 Dec 2016 17:53:39 +0100 Subject: [PATCH 37/47] css changes @dimitrieh v1 --- .../stylesheets/framework/dropdowns.scss | 2 +- .../stylesheets/framework/variables.scss | 2 +- app/assets/stylesheets/pages/pipelines.scss | 27 ++++++++++++++++--- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index d5914b900e2..21df80c01f7 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -98,7 +98,7 @@ @extend .dropdown-toggle; padding-right: 20px; position: relative; - width: 160px; + width: 163px; text-overflow: ellipsis; overflow: hidden; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 03fbfa5f1bd..3ed19672ec1 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -578,4 +578,4 @@ Pipeline Graph */ $stage-hover-bg: #eaf3fc; $stage-hover-border: #d1e7fc; -$stage-badge-text: #d4d4d4; +$stage-badge-text: #e5e5e5; diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 26487b2acf9..ac18c39dfc4 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -460,7 +460,7 @@ border-radius: 30px; background-color: $white-light; position: relative; - padding: 8px 10px 9px; + padding: 8px 4px 9px 10px; width: 186px; margin-bottom: 10px; @@ -605,7 +605,7 @@ float: right; color: $stage-badge-text; font-weight: 100; - font-size: 13px; + font-size: 15px; margin-top: 1px; margin-right: 2px; } @@ -629,6 +629,7 @@ ul { max-height: 245px; overflow: auto; + margin: 5px 0; li { padding-top: 2px; @@ -665,6 +666,18 @@ color: $gl-text-color; } + .ci-action-icon-container { + i { + width: 25px; + height: 25px; + + &:before{ + top: 1px; + left: 1px; + } + } + } + .stage { max-width: 100px; width: 100px; @@ -681,7 +694,7 @@ // Action Icons .ci-action-icon-container .ci-action-icon-wrapper { float: right; - margin-top: -1px; + margin-top: -4px; i { color: $stage-badge-text; @@ -690,6 +703,14 @@ padding: 5px 6px; font-size: 13px; background: $white-light; + height: 30px; + width: 30px; + + &:before { + position: relative; + top: 3px; + left: 3px; + } &:hover { color: $gl-text-color; -- 2.24.1 From 98a12e535f612339f1af59c9d479b0cdab69df3a Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 17:18:00 +0000 Subject: [PATCH 38/47] Fix extension in file --- app/views/ci/status/{_graph_badge.haml => _graph_badge.html.haml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/views/ci/status/{_graph_badge.haml => _graph_badge.html.haml} (100%) diff --git a/app/views/ci/status/_graph_badge.haml b/app/views/ci/status/_graph_badge.html.haml similarity index 100% rename from app/views/ci/status/_graph_badge.haml rename to app/views/ci/status/_graph_badge.html.haml -- 2.24.1 From acc4b73cb5e3edfed1fa25d461d36b82fe2aacb2 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 17:31:43 +0000 Subject: [PATCH 39/47] Remove duplicate color variable --- app/assets/stylesheets/framework/variables.scss | 1 - app/assets/stylesheets/pages/pipelines.scss | 11 +++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 3ed19672ec1..710b971615b 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -578,4 +578,3 @@ Pipeline Graph */ $stage-hover-bg: #eaf3fc; $stage-hover-border: #d1e7fc; -$stage-badge-text: #e5e5e5; diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index ac18c39dfc4..0124940408a 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -603,10 +603,9 @@ .dropdown-counter-badge { float: right; - color: $stage-badge-text; + color: $border-color; font-weight: 100; font-size: 15px; - margin-top: 1px; margin-right: 2px; } @@ -687,6 +686,10 @@ height: 18px; width: 18px; } + + .ci-status-text { + max-width: 95px; + } } } } @@ -697,9 +700,9 @@ margin-top: -4px; i { - color: $stage-badge-text; + color: $border-color; border-radius: 100%; - border: 1px solid $stage-badge-text; + border: 1px solid $border-color; padding: 5px 6px; font-size: 13px; background: $white-light; -- 2.24.1 From e998d6fb59e331cf000fa1c7463f79610b87be7c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 14 Dec 2016 20:06:24 +0100 Subject: [PATCH 40/47] Simplify graph status badge partial and require locals --- app/views/ci/status/_graph_badge.html.haml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/app/views/ci/status/_graph_badge.html.haml b/app/views/ci/status/_graph_badge.html.haml index 839b4334713..a7e8544e7d4 100644 --- a/app/views/ci/status/_graph_badge.html.haml +++ b/app/views/ci/status/_graph_badge.html.haml @@ -1,19 +1,19 @@ -# Renders the graph node with both the status icon, status name and action icon -- detailed_status = subject.detailed_status(current_user) -- details_path = detailed_status.details_path if detailed_status.has_details? -- klass = "ci-status-icon ci-status-icon-#{detailed_status}" +- subject = local_assigns.fetch(:subject) +- status = subject.detailed_status(current_user) +- klass = "ci-status-icon ci-status-icon-#{status}" -- if details_path - = link_to details_path, data: { toggle: 'tooltip', title: "#{subject.name} - #{detailed_status}" } do - %span{ class: klass }= custom_icon(detailed_status.icon) +- if status.has_details? + = link_to status.details_path, data: { toggle: 'tooltip', title: "#{subject.name} - #{status}" } do + %span{ class: klass }= custom_icon(status.icon) .ci-status-text= subject.name - else - %span{ class: klass }= custom_icon(detailed_status.icon) + %span{ class: klass }= custom_icon(status.icon) .ci-status-text= subject.name -- if detailed_status.has_action? - = link_to detailed_status.action_path, method: detailed_status.action_method, - title: "#{subject.name}: #{detailed_status.action_title}", class: 'ci-action-icon-container' do +- if status.has_action? + = link_to status.action_path, method: status.action_method, + title: "#{subject.name}: #{status.action_title}", class: 'ci-action-icon-container' do %i.ci-action-icon-wrapper - = icon(detailed_status.action_icon, class: detailed_status.action_class) + = icon(status.action_icon, class: status.action_class) -- 2.24.1 From bdae2df31065365283720c1e29d9e5e5d495bcf3 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 23:27:26 +0000 Subject: [PATCH 41/47] Fix firefox bug --- app/assets/stylesheets/pages/pipelines.scss | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 0124940408a..ee08d9d3ca0 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -463,6 +463,7 @@ padding: 8px 4px 9px 10px; width: 186px; margin-bottom: 10px; + white-space: normal; &:hover { background-color: $stage-hover-bg; @@ -586,7 +587,7 @@ border: none; padding: 0; color: $gl-text-color-light; - flex-grow: 1; + white-space: normal; &:focus { outline: none; @@ -603,6 +604,7 @@ .dropdown-counter-badge { float: right; + clear: right; color: $border-color; font-weight: 100; font-size: 15px; @@ -651,7 +653,10 @@ padding: 0; font-size: 11px; float: right; - margin-top: 5px; + clear: right; + margin-top: 3px; + display: inline-block; + position: relative; i { font-size: 11px; -- 2.24.1 From 63e5e009b3fb7c85a8471d2e15951bd5d04870b5 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 14 Dec 2016 23:33:25 +0000 Subject: [PATCH 42/47] Changes after review --- app/assets/stylesheets/pages/pipelines.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index ee08d9d3ca0..e879c2495fb 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -484,6 +484,10 @@ } } + .ci-status-icon { + position: relative; + top: 1px; + } .ci-status-icon svg { height: 20px; width: 20px; @@ -588,6 +592,7 @@ padding: 0; color: $gl-text-color-light; white-space: normal; + overflow: visible; &:focus { outline: none; -- 2.24.1 From 4564492a84b91b820ef0bcc69a84563ebeed9cb2 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 15 Dec 2016 00:04:22 +0000 Subject: [PATCH 43/47] Fix dropdown hover --- app/assets/stylesheets/pages/pipelines.scss | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index e879c2495fb..4569b91383f 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -488,6 +488,7 @@ position: relative; top: 1px; } + .ci-status-icon svg { height: 20px; width: 20px; @@ -638,8 +639,11 @@ margin: 5px 0; li { - padding-top: 2px; margin: 0 5px; + padding-left: 0; + padding-bottom: 0; + margin-bottom: 0; + line-height: 1.2; } li:first-child { @@ -658,8 +662,7 @@ padding: 0; font-size: 11px; float: right; - clear: right; - margin-top: 3px; + margin-top: 4px; display: inline-block; position: relative; @@ -699,6 +702,9 @@ .ci-status-text { max-width: 95px; + padding-bottom: 3px; + position: relative; + top: 3px; } } } -- 2.24.1 From 48fed57b05b0394ca824bed90c670505b3a8a273 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 15 Dec 2016 10:55:34 +0000 Subject: [PATCH 44/47] Fix scss linter error --- app/assets/stylesheets/pages/pipelines.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 4569b91383f..3054f0655db 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -683,7 +683,7 @@ width: 25px; height: 25px; - &:before{ + &::before { top: 1px; left: 1px; } @@ -725,7 +725,7 @@ height: 30px; width: 30px; - &:before { + &::before { position: relative; top: 3px; left: 3px; -- 2.24.1 From 03a95c7070938e0fde67f2def334c08cf72d9d29 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 15 Dec 2016 11:48:05 +0000 Subject: [PATCH 45/47] Fix tests --- .../projects/pipelines/pipeline_spec.rb | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 5094fcf33e8..80a596d34c9 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -40,43 +40,62 @@ describe "Pipelines", feature: true, js: true do context 'pipeline graph' do it 'shows a running icon and a cancel action for the running build' do - page.within('.stage-column:nth-child(3) .build:first-child') do + title = "#{@running.name} - #{@running.status}" + + page.within("a[data-title='#{title}']") do expect(page).to have_selector('.ci-status-icon-running') expect(page).to have_content('deploy') + end + + page.within("a[data-title='#{title}'] + .ci-action-icon-container") do expect(page).to have_selector('.ci-action-icon-container .fa-ban') end + end it 'shows the success icon and a retry action for the successfull build' do - page.within('.stage-column:nth-child(2) .build:first-child') do + title = "#{@success.name} - #{@success.status}" + + page.within("a[data-title='#{title}']") do expect(page).to have_selector('.ci-status-icon-success') expect(page).to have_content('build') + end + + page.within("a[data-title='#{title}'] + .ci-action-icon-container") do expect(page).to have_selector('.ci-action-icon-container .fa-refresh') end end it 'shows the failed icon and a retry action for the failed build' do - page.within('.stage-column:first-child .build') do + title = "#{@failed.name} - #{@failed.status}" + + page.within("a[data-title='#{title}']") do expect(page).to have_selector('.ci-status-icon-failed') expect(page).to have_content('test') + end + + page.within("a[data-title='#{title}'] + .ci-action-icon-container") do expect(page).to have_selector('.ci-action-icon-container .fa-refresh') end end it 'shows the skipped icon and a play action for the manual build' do - page.within('.stage-column:nth-child(3) .build:nth-child(2)') do + title = "#{@manual.name} - #{@manual.status}" + + page.within("a[data-title='#{title}']") do expect(page).to have_selector('.ci-status-icon-skipped') expect(page).to have_content('manual') - expect(page).to have_selector('.ci-action-icon-container .ci-play-icon') end - end - it 'shows the success icon for the generic comit status build' do - page.within('.stage-column:nth-child(4) .build') do - expect(page).to have_selector('.ci-status-icon-success') - expect(page).to have_content('jenkins') + page.within("a[data-title='#{title}'] + .ci-action-icon-container") do + expect(page).to have_selector('.ci-action-icon-container .fa-play') end end + + it 'shows the success icon and the generic comit status build' do + expect(page).to have_selector('.ci-status-icon-success') + expect(page).to have_content('jenkins') + end end context 'page tabs' do -- 2.24.1 From e42de89a15c858866d78a4d2a5837a0feec922a5 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 15 Dec 2016 17:30:49 +0000 Subject: [PATCH 46/47] Changes after review Changes after review Fix tooltip title Remove unneeded string interpolation --- app/assets/stylesheets/pages/pipelines.scss | 10 +- app/views/ci/status/_graph_badge.html.haml | 4 +- .../ci/builds/_build_pipeline.html.haml | 13 --- .../_generic_commit_status_pipeline.html.haml | 10 -- .../projects/pipelines/pipeline_spec.rb | 93 ++++++++++++------- 5 files changed, 62 insertions(+), 68 deletions(-) delete mode 100644 app/views/projects/ci/builds/_build_pipeline.html.haml delete mode 100644 app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index d3f39570f11..be22e7bdc79 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -645,14 +645,6 @@ margin-bottom: 0; line-height: 1.2; } - - li:first-child { - padding-top: 6px; - } - - li:last-child { - padding-bottom: 6px; - } } .dropdown-build { @@ -741,4 +733,4 @@ .ci-play-icon { padding: 5px 5px 5px 7px; } -} \ No newline at end of file +} diff --git a/app/views/ci/status/_graph_badge.html.haml b/app/views/ci/status/_graph_badge.html.haml index a7e8544e7d4..c7d04ab61e9 100644 --- a/app/views/ci/status/_graph_badge.html.haml +++ b/app/views/ci/status/_graph_badge.html.haml @@ -5,7 +5,7 @@ - klass = "ci-status-icon ci-status-icon-#{status}" - if status.has_details? - = link_to status.details_path, data: { toggle: 'tooltip', title: "#{subject.name} - #{status}" } do + = link_to status.details_path, data: { toggle: 'tooltip', title: "#{subject.name} - #{status.label}" } do %span{ class: klass }= custom_icon(status.icon) .ci-status-text= subject.name - else @@ -14,6 +14,6 @@ - if status.has_action? = link_to status.action_path, method: status.action_method, - title: "#{subject.name}: #{status.action_title}", class: 'ci-action-icon-container' do + title: status.action_title, class: 'ci-action-icon-container' do %i.ci-action-icon-wrapper = icon(status.action_icon, class: status.action_class) diff --git a/app/views/projects/ci/builds/_build_pipeline.html.haml b/app/views/projects/ci/builds/_build_pipeline.html.haml deleted file mode 100644 index ad1a7360a8b..00000000000 --- a/app/views/projects/ci/builds/_build_pipeline.html.haml +++ /dev/null @@ -1,13 +0,0 @@ -- is_playable = subject.playable? && can?(current_user, :update_build, @project) -- if is_playable - = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, data: { toggle: 'tooltip', title: "#{subject.name} - play", container: '.js-pipeline-graph', placement: 'bottom' } do - = ci_icon_for_status('play') - .ci-status-text= subject.name -- elsif can?(current_user, :read_build, @project) - = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject), data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.js-pipeline-graph', placement: 'bottom' } do - %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} - = ci_icon_for_status(subject.status) - .ci-status-text= subject.name -- else - %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} - = ci_icon_for_status(subject.status) diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml deleted file mode 100644 index 1bba0443154..00000000000 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml +++ /dev/null @@ -1,10 +0,0 @@ -%a{ data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.js-pipeline-graph', placement: 'bottom' } } - - if subject.target_url - = link_to subject.target_url do - %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} - = ci_icon_for_status(subject.status) - %span.ci-status-text= subject.name - - else - %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} - = ci_icon_for_status(subject.status) - %span.ci-status-text= subject.name diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 80a596d34c9..0a77eaa123c 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -38,63 +38,88 @@ describe "Pipelines", feature: true, js: true do expect(page).to have_css('#js-tab-pipeline.active') end - context 'pipeline graph' do - it 'shows a running icon and a cancel action for the running build' do - title = "#{@running.name} - #{@running.status}" - - page.within("a[data-title='#{title}']") do - expect(page).to have_selector('.ci-status-icon-running') - expect(page).to have_content('deploy') + describe 'pipeline graph' do + context 'when pipeline has running builds' do + it 'shows a running icon and a cancel action for the running build' do + page.within('a[data-title="deploy - running"]') do + expect(page).to have_selector('.ci-status-icon-running') + expect(page).to have_content('deploy') + end + + page.within('a[data-title="deploy - running"] + .ci-action-icon-container') do + expect(page).to have_selector('.ci-action-icon-container .fa-ban') + end end - page.within("a[data-title='#{title}'] + .ci-action-icon-container") do - expect(page).to have_selector('.ci-action-icon-container .fa-ban') - end + it 'should be possible to cancel the running build' do + find('a[data-title="deploy - running"] + .ci-action-icon-container').trigger('click') + expect(page).not_to have_content('Cancel running') + end end - it 'shows the success icon and a retry action for the successfull build' do - title = "#{@success.name} - #{@success.status}" + context 'when pipeline has successful builds' do + it 'shows the success icon and a retry action for the successfull build' do + page.within('a[data-title="build - passed"]') do + expect(page).to have_selector('.ci-status-icon-success') + expect(page).to have_content('build') + end - page.within("a[data-title='#{title}']") do - expect(page).to have_selector('.ci-status-icon-success') - expect(page).to have_content('build') + page.within('a[data-title="build - passed"] + .ci-action-icon-container') do + expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + end end - page.within("a[data-title='#{title}'] + .ci-action-icon-container") do - expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + it 'should be possible to retry the success build' do + find('a[data-title="build - passed"] + .ci-action-icon-container').trigger('click') + + expect(page).not_to have_content('Retry build') end end - it 'shows the failed icon and a retry action for the failed build' do - title = "#{@failed.name} - #{@failed.status}" + context 'when pipeline has failed builds' do + it 'shows the failed icon and a retry action for the failed build' do + page.within('a[data-title="test - failed"]') do + expect(page).to have_selector('.ci-status-icon-failed') + expect(page).to have_content('test') + end - page.within("a[data-title='#{title}']") do - expect(page).to have_selector('.ci-status-icon-failed') - expect(page).to have_content('test') + page.within('a[data-title="test - failed"] + .ci-action-icon-container') do + expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + end end - page.within("a[data-title='#{title}'] + .ci-action-icon-container") do - expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + it 'should be possible to retry the failed build' do + find('a[data-title="test - failed"] + .ci-action-icon-container').trigger('click') + + expect(page).not_to have_content('Retry build') end end - it 'shows the skipped icon and a play action for the manual build' do - title = "#{@manual.name} - #{@manual.status}" + context 'when pipeline has manual builds' do + it 'shows the skipped icon and a play action for the manual build' do + page.within('a[data-title="manual build - manual play action"]') do + expect(page).to have_selector('.ci-status-icon-skipped') + expect(page).to have_content('manual') + end - page.within("a[data-title='#{title}']") do - expect(page).to have_selector('.ci-status-icon-skipped') - expect(page).to have_content('manual') + page.within('a[data-title="manual build - manual play action"] + .ci-action-icon-container') do + expect(page).to have_selector('.ci-action-icon-container .fa-play') + end end - page.within("a[data-title='#{title}'] + .ci-action-icon-container") do - expect(page).to have_selector('.ci-action-icon-container .fa-play') + it 'should be possible to play the manual build' do + find('a[data-title="manual build - manual play action"] + .ci-action-icon-container').trigger('click') + + expect(page).not_to have_content('Play build') end end - it 'shows the success icon and the generic comit status build' do - expect(page).to have_selector('.ci-status-icon-success') - expect(page).to have_content('jenkins') + context 'when pipeline has external build' do + it 'shows the success icon and the generic comit status build' do + expect(page).to have_selector('.ci-status-icon-success') + expect(page).to have_content('jenkins') + end end end -- 2.24.1 From ca16a6bdf8feb92ae2b24cff86143fdaf668ce7d Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 16 Dec 2016 11:30:22 +0000 Subject: [PATCH 47/47] Fix broken test --- spec/features/commits_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 23a504ff965..8f561c8f90b 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -107,7 +107,7 @@ describe 'Commits' do describe 'Cancel build' do it 'cancels build' do visit ci_status_path(pipeline) - click_on 'Cancel' + find('a.btn[title="Cancel"]').click expect(page).to have_content 'canceled' end end -- 2.24.1