From e42de89a15c858866d78a4d2a5837a0feec922a5 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 15 Dec 2016 17:30:49 +0000 Subject: [PATCH] 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 | 121 +++++++++++------- 5 files changed, 76 insertions(+), 82 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}" + 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='#{title}']") do - expect(page).to have_selector('.ci-status-icon-running') - expect(page).to have_content('deploy') + 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 + 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 + + 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 + + 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="test - failed"] + .ci-action-icon-container') do + expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + end + end + + 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 + + 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="manual build - manual play action"] + .ci-action-icon-container') do + expect(page).to have_selector('.ci-action-icon-container .fa-play') + end + end + + 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 + + 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('build') + expect(page).to have_content('jenkins') 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 - 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 - 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') - end - - 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