From 8bbaf266cc8d1ff03c4f7a4173ce9a7ac60c0cbb Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 8 Mar 2018 17:33:06 +0000 Subject: [PATCH 01/25] Handle empty states for job page --- .../projects/jobs/_empty_state.html.haml | 5 ++-- .../projects/jobs/_empty_status.html.haml | 29 +++++++++++++++++++ app/views/projects/jobs/show.html.haml | 23 +++------------ .../unreleased/42568-pipeline-empty-state.yml | 5 ++++ db/fixtures/development/14_pipelines.rb | 4 +++ spec/features/projects/jobs_spec.rb | 27 +++++++++++++++++ 6 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 app/views/projects/jobs/_empty_status.html.haml create mode 100644 changelogs/unreleased/42568-pipeline-empty-state.yml diff --git a/app/views/projects/jobs/_empty_state.html.haml b/app/views/projects/jobs/_empty_state.html.haml index c66313bdbf3..311934d9c33 100644 --- a/app/views/projects/jobs/_empty_state.html.haml +++ b/app/views/projects/jobs/_empty_state.html.haml @@ -1,7 +1,7 @@ - illustration = local_assigns.fetch(:illustration) - illustration_size = local_assigns.fetch(:illustration_size) - title = local_assigns.fetch(:title) -- content = local_assigns.fetch(:content) +- content = local_assigns.fetch(:content, nil) - action = local_assigns.fetch(:action, nil) .row.empty-state @@ -11,7 +11,8 @@ .col-xs-12 .text-content %h4.text-center= title - %p= content + - if content + %p= content - if action .text-center = action diff --git a/app/views/projects/jobs/_empty_status.html.haml b/app/views/projects/jobs/_empty_status.html.haml new file mode 100644 index 00000000000..707085cddd5 --- /dev/null +++ b/app/views/projects/jobs/_empty_status.html.haml @@ -0,0 +1,29 @@ +- if @build.playable? + = render 'empty_state', + illustration: 'illustrations/manual_action.svg', + illustration_size: 'svg-394', + title: _('This job requires a manual action'), + content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments'), + action: ( link_to _('Trigger this manual action'), play_project_job_path(@project, @build), method: :post, class: 'btn btn-primary', title: _('Trigger this manual action') ) +- elsif @build.created? + = render 'empty_state', + illustration: 'illustrations/job_not_triggered.svg', + illustration_size: 'svg-306', + title: _('This job has not been triggered yet'), + content: _('This job depends on upstream jobs that need to succeed in order for this job to be triggered') +- elsif @build.canceled? + = render 'empty_state', + illustration: 'illustrations/canceled-job_empty.svg', + illustration_size: 'svg-430', + title: _('This job has been canceled') +- elsif @build.skipped? + = render 'empty_state', + illustration: 'illustrations/canceled-job_empty.svg', + illustration_size: 'svg-430', + title: _('This job has been skipped') +- else + = render 'empty_state', + illustration: 'illustrations/pending_job_empty.svg', + illustration_size: 'svg-430', + title: _('This job has not started yet'), + content: _('This job is in pending state and is waiting to be picked by a runner') diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml index 849c273db8c..04c28841511 100644 --- a/app/views/projects/jobs/show.html.haml +++ b/app/views/projects/jobs/show.html.haml @@ -54,7 +54,8 @@ Job has been erased by #{link_to(@build.erased_by_name, user_path(@build.erased_by))} #{time_ago_with_tooltip(@build.erased_at)} - else Job has been erased #{time_ago_with_tooltip(@build.erased_at)} - - if @build.started? + + - if @build.has_trace? .build-trace-container.prepend-top-default .top-bar.js-top-bar .js-truncated-info.truncated-info.hidden-xs.pull-left.hidden< @@ -88,25 +89,9 @@ %pre.build-trace#build-trace %code.bash.js-build-output .build-loader-animation.js-build-refresh - - elsif @build.playable? - = render 'empty_state', - illustration: 'illustrations/manual_action.svg', - illustration_size: 'svg-394', - title: _('This job requires a manual action'), - content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments'), - action: ( link_to _('Trigger this manual action'), play_project_job_path(@project, @build), method: :post, class: 'btn btn-primary', title: _('Trigger this manual action') ) - - elsif @build.created? - = render 'empty_state', - illustration: 'illustrations/job_not_triggered.svg', - illustration_size: 'svg-306', - title: _('This job has not been triggered yet'), - content: _('This job depends on upstream jobs that need to succeed in order for this job to be triggered') - else - = render 'empty_state', - illustration: 'illustrations/pending_job_empty.svg', - illustration_size: 'svg-430', - title: _('This job has not started yet'), - content: _('This job is in pending state and is waiting to be picked by a runner') + = render "empty_status" + = render "sidebar" .js-build-options{ data: javascript_build_options } diff --git a/changelogs/unreleased/42568-pipeline-empty-state.yml b/changelogs/unreleased/42568-pipeline-empty-state.yml new file mode 100644 index 00000000000..d36edcf1b37 --- /dev/null +++ b/changelogs/unreleased/42568-pipeline-empty-state.yml @@ -0,0 +1,5 @@ +--- +title: Improve empty state for canceled job +merge_request: 17646 +author: +type: fixed diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb index d3a63aa2a78..fe21b7aad0e 100644 --- a/db/fixtures/development/14_pipelines.rb +++ b/db/fixtures/development/14_pipelines.rb @@ -30,6 +30,10 @@ class Gitlab::Seeder::Pipelines queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, { name: 'spinach:osx', stage: 'test', status: :failed, allow_failure: true, queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, + { name: 'spinach:osx1', stage: 'test', status: :canceled, + queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, + { name: 'spinach:osx1', stage: 'test', status: :running, + queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, # deploy stage { name: 'staging', stage: 'deploy', environment: 'staging', status_event: :success, diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 5d311f2dde3..aa4c2757a25 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -419,6 +419,33 @@ feature 'Jobs' do expect(page).to have_content('This job is in pending state and is waiting to be picked by a runner') end end + + context 'Canceled job' do + context 'with log' do + let(:job) { create(:ci_build, :canceled, :trace_artifact, pipeline: pipeline) } + + before do + visit project_job_path(project, job) + end + + it 'renders job log' do + expect(page).to have_selector('.js-build-output') + end + end + + context 'without log' do + let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } + + before do + visit project_job_path(project, job) + end + + it 'renders empty state' do + expect(page).not_to have_selector('.js-build-output') + expect(page).to have_content('This job has been canceled') + end + end + end end describe "POST /:project/jobs/:id/cancel", :js do From cd6a58c8100356be95019be6a69724066469bd42 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 9 Mar 2018 17:59:59 +0000 Subject: [PATCH 02/25] Fix broken test --- features/steps/shared/builds.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/shared/builds.rb b/features/steps/shared/builds.rb index a3e4459f169..f5950145348 100644 --- a/features/steps/shared/builds.rb +++ b/features/steps/shared/builds.rb @@ -11,7 +11,7 @@ module SharedBuilds step 'project has a recent build' do @pipeline = create(:ci_empty_pipeline, project: @project, sha: @project.commit.sha, ref: 'master') - @build = create(:ci_build, :running, :coverage, pipeline: @pipeline) + @build = create(:ci_build, :running, :coverage, :trace_artifact, pipeline: @pipeline) end step 'recent build is successful' do From c7776a1d32621740f1eea17ec0385889280df65e Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 12 Mar 2018 19:13:13 +0000 Subject: [PATCH 03/25] Render skipped illustration for skipped state --- app/views/projects/jobs/_empty_status.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/jobs/_empty_status.html.haml b/app/views/projects/jobs/_empty_status.html.haml index 707085cddd5..9e144dbfe32 100644 --- a/app/views/projects/jobs/_empty_status.html.haml +++ b/app/views/projects/jobs/_empty_status.html.haml @@ -18,7 +18,7 @@ title: _('This job has been canceled') - elsif @build.skipped? = render 'empty_state', - illustration: 'illustrations/canceled-job_empty.svg', + illustration: 'illustrations/skipped-job_empty.svg', illustration_size: 'svg-430', title: _('This job has been skipped') - else From 18bce8fcfeaaeeda5aea37f21adedcdc49dd4147 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 19 Mar 2018 10:07:30 +0000 Subject: [PATCH 04/25] Update partial name and remove changes to database fixtures --- .../jobs/{_empty_status.html.haml => _empty_states.html.haml} | 0 app/views/projects/jobs/show.html.haml | 2 +- db/fixtures/development/14_pipelines.rb | 4 ---- 3 files changed, 1 insertion(+), 5 deletions(-) rename app/views/projects/jobs/{_empty_status.html.haml => _empty_states.html.haml} (100%) diff --git a/app/views/projects/jobs/_empty_status.html.haml b/app/views/projects/jobs/_empty_states.html.haml similarity index 100% rename from app/views/projects/jobs/_empty_status.html.haml rename to app/views/projects/jobs/_empty_states.html.haml diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml index 04c28841511..1db01133900 100644 --- a/app/views/projects/jobs/show.html.haml +++ b/app/views/projects/jobs/show.html.haml @@ -90,7 +90,7 @@ %code.bash.js-build-output .build-loader-animation.js-build-refresh - else - = render "empty_status" + = render "empty_states" = render "sidebar" diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb index fe21b7aad0e..d3a63aa2a78 100644 --- a/db/fixtures/development/14_pipelines.rb +++ b/db/fixtures/development/14_pipelines.rb @@ -30,10 +30,6 @@ class Gitlab::Seeder::Pipelines queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, { name: 'spinach:osx', stage: 'test', status: :failed, allow_failure: true, queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, - { name: 'spinach:osx1', stage: 'test', status: :canceled, - queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, - { name: 'spinach:osx1', stage: 'test', status: :running, - queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, # deploy stage { name: 'staging', stage: 'deploy', environment: 'staging', status_event: :success, From d696194f8f80dd6fe4feb9ec7bb42a29ab9e98f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 24 Mar 2018 11:46:04 +0100 Subject: [PATCH 05/25] Add illustrations to build statuses --- lib/gitlab/ci/status/canceled.rb | 4 ++++ lib/gitlab/ci/status/core.rb | 4 ++++ lib/gitlab/ci/status/created.rb | 4 ++++ lib/gitlab/ci/status/manual.rb | 4 ++++ lib/gitlab/ci/status/pending.rb | 4 ++++ lib/gitlab/ci/status/skipped.rb | 4 ++++ spec/lib/gitlab/ci/status/build/factory_spec.rb | 4 ++++ spec/lib/gitlab/ci/status/canceled_spec.rb | 4 ++++ spec/lib/gitlab/ci/status/created_spec.rb | 4 ++++ spec/lib/gitlab/ci/status/manual_spec.rb | 4 ++++ spec/lib/gitlab/ci/status/pending_spec.rb | 4 ++++ spec/lib/gitlab/ci/status/skipped_spec.rb | 4 ++++ 12 files changed, 48 insertions(+) diff --git a/lib/gitlab/ci/status/canceled.rb b/lib/gitlab/ci/status/canceled.rb index e6195a60d4f..9a25375678d 100644 --- a/lib/gitlab/ci/status/canceled.rb +++ b/lib/gitlab/ci/status/canceled.rb @@ -17,6 +17,10 @@ module Gitlab def favicon 'favicon_status_canceled' end + + def illustration + 'canceled-job_empty' + end end end end diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index d4fd83b93f8..89fb9a0b7bc 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -22,6 +22,10 @@ module Gitlab raise NotImplementedError end + def illustration + raise NotImplementedError + end + def label raise NotImplementedError end diff --git a/lib/gitlab/ci/status/created.rb b/lib/gitlab/ci/status/created.rb index 846f00b83dd..033ecbe9380 100644 --- a/lib/gitlab/ci/status/created.rb +++ b/lib/gitlab/ci/status/created.rb @@ -17,6 +17,10 @@ module Gitlab def favicon 'favicon_status_created' end + + def illustration + 'job_not_triggered' + end end end end diff --git a/lib/gitlab/ci/status/manual.rb b/lib/gitlab/ci/status/manual.rb index fc387e2fd25..92c449e2c63 100644 --- a/lib/gitlab/ci/status/manual.rb +++ b/lib/gitlab/ci/status/manual.rb @@ -17,6 +17,10 @@ module Gitlab def favicon 'favicon_status_manual' end + + def illustration + 'manual_action' + end end end end diff --git a/lib/gitlab/ci/status/pending.rb b/lib/gitlab/ci/status/pending.rb index 6780780db32..63c0b5b501f 100644 --- a/lib/gitlab/ci/status/pending.rb +++ b/lib/gitlab/ci/status/pending.rb @@ -17,6 +17,10 @@ module Gitlab def favicon 'favicon_status_pending' end + + def illustration + 'pending_job_empty' + end end end end diff --git a/lib/gitlab/ci/status/skipped.rb b/lib/gitlab/ci/status/skipped.rb index 0dbdc4de426..5dbe2847344 100644 --- a/lib/gitlab/ci/status/skipped.rb +++ b/lib/gitlab/ci/status/skipped.rb @@ -17,6 +17,10 @@ module Gitlab def favicon 'favicon_status_skipped' end + + def illustration + 'skipped-job_empty' + end end end end diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index d196bc6a4c2..04b718c5897 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -115,6 +115,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.text).to eq 'canceled' expect(status.icon).to eq 'status_canceled' expect(status.favicon).to eq 'favicon_status_canceled' + expect(status.illustration).to eq 'canceled-job_empty' expect(status.label).to eq 'canceled' expect(status).to have_details expect(status).to have_action @@ -167,6 +168,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.text).to eq 'pending' expect(status.icon).to eq 'status_pending' expect(status.favicon).to eq 'favicon_status_pending' + expect(status.illustration).to eq 'pending_job_empty' expect(status.label).to eq 'pending' expect(status).to have_details expect(status).to have_action @@ -192,6 +194,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.text).to eq 'skipped' expect(status.icon).to eq 'status_skipped' expect(status.favicon).to eq 'favicon_status_skipped' + expect(status.illustration).to eq 'skipped-job_empty' expect(status.label).to eq 'skipped' expect(status).to have_details expect(status).not_to have_action @@ -221,6 +224,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.group).to eq 'manual' expect(status.icon).to eq 'status_manual' expect(status.favicon).to eq 'favicon_status_manual' + expect(status.illustration).to eq 'manual_action' expect(status.label).to include 'manual play action' expect(status).to have_details expect(status.action_path).to include 'play' diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb index dc74d7e28c5..fc1d9e726ab 100644 --- a/spec/lib/gitlab/ci/status/canceled_spec.rb +++ b/spec/lib/gitlab/ci/status/canceled_spec.rb @@ -21,6 +21,10 @@ describe Gitlab::Ci::Status::Canceled do it { expect(subject.favicon).to eq 'favicon_status_canceled' } end + describe '#illustration' do + it { expect(subject.illustration).to eq 'canceled-job_empty' } + end + describe '#group' do it { expect(subject.group).to eq 'canceled' } end diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb index ce4333f2aca..f7e3b0d8f1a 100644 --- a/spec/lib/gitlab/ci/status/created_spec.rb +++ b/spec/lib/gitlab/ci/status/created_spec.rb @@ -21,6 +21,10 @@ describe Gitlab::Ci::Status::Created do it { expect(subject.favicon).to eq 'favicon_status_created' } end + describe '#illustration' do + it { expect(subject.illustration).to eq 'job_not_triggered' } + end + describe '#group' do it { expect(subject.group).to eq 'created' } end diff --git a/spec/lib/gitlab/ci/status/manual_spec.rb b/spec/lib/gitlab/ci/status/manual_spec.rb index 0463f2e1aff..161c4774ff3 100644 --- a/spec/lib/gitlab/ci/status/manual_spec.rb +++ b/spec/lib/gitlab/ci/status/manual_spec.rb @@ -21,6 +21,10 @@ describe Gitlab::Ci::Status::Manual do it { expect(subject.favicon).to eq 'favicon_status_manual' } end + describe '#illustration' do + it { expect(subject.illustration).to eq 'manual_action' } + end + describe '#group' do it { expect(subject.group).to eq 'manual' } end diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb index 0e25358dd8a..92349308842 100644 --- a/spec/lib/gitlab/ci/status/pending_spec.rb +++ b/spec/lib/gitlab/ci/status/pending_spec.rb @@ -21,6 +21,10 @@ describe Gitlab::Ci::Status::Pending do it { expect(subject.favicon).to eq 'favicon_status_pending' } end + describe '#illustration' do + it { expect(subject.illustration).to eq 'pending_job_empty' } + end + describe '#group' do it { expect(subject.group).to eq 'pending' } end diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb index 63694ca0ea6..993490c6d4e 100644 --- a/spec/lib/gitlab/ci/status/skipped_spec.rb +++ b/spec/lib/gitlab/ci/status/skipped_spec.rb @@ -21,6 +21,10 @@ describe Gitlab::Ci::Status::Skipped do it { expect(subject.favicon).to eq 'favicon_status_skipped' } end + describe '#illustration' do + it { expect(subject.illustration).to eq 'skipped-job_empty' } + end + describe '#group' do it { expect(subject.group).to eq 'skipped' } end From d0349362a7681dc72636b0041a28d58a6e979707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 26 Mar 2018 17:28:53 +0200 Subject: [PATCH 06/25] Extend Gitlab::Ci::Status#illustration to include metadata --- lib/gitlab/ci/status/canceled.rb | 6 +++++- lib/gitlab/ci/status/created.rb | 7 ++++++- lib/gitlab/ci/status/manual.rb | 7 ++++++- lib/gitlab/ci/status/pending.rb | 7 ++++++- lib/gitlab/ci/status/skipped.rb | 6 +++++- spec/lib/gitlab/ci/status/build/factory_spec.rb | 8 ++++---- spec/lib/gitlab/ci/status/canceled_spec.rb | 2 +- spec/lib/gitlab/ci/status/created_spec.rb | 2 +- spec/lib/gitlab/ci/status/manual_spec.rb | 2 +- spec/lib/gitlab/ci/status/pending_spec.rb | 2 +- spec/lib/gitlab/ci/status/skipped_spec.rb | 2 +- 11 files changed, 37 insertions(+), 14 deletions(-) diff --git a/lib/gitlab/ci/status/canceled.rb b/lib/gitlab/ci/status/canceled.rb index 9a25375678d..bea8dcd5587 100644 --- a/lib/gitlab/ci/status/canceled.rb +++ b/lib/gitlab/ci/status/canceled.rb @@ -19,7 +19,11 @@ module Gitlab end def illustration - 'canceled-job_empty' + { + image: 'illustrations/canceled-job_empty.svg', + size: 'svg-430', + title: _('This job has been canceled') + } end end end diff --git a/lib/gitlab/ci/status/created.rb b/lib/gitlab/ci/status/created.rb index 033ecbe9380..3b2a9e5d0ef 100644 --- a/lib/gitlab/ci/status/created.rb +++ b/lib/gitlab/ci/status/created.rb @@ -19,7 +19,12 @@ module Gitlab end def illustration - 'job_not_triggered' + { + image: 'illustrations/job_not_triggered.svg', + size: 'svg-306', + title: _('This job has not been triggered yet'), + content: _('This job depends on upstream jobs that need to succeed in order for this job to be triggered') + } end end end diff --git a/lib/gitlab/ci/status/manual.rb b/lib/gitlab/ci/status/manual.rb index 92c449e2c63..cb587a7815e 100644 --- a/lib/gitlab/ci/status/manual.rb +++ b/lib/gitlab/ci/status/manual.rb @@ -19,7 +19,12 @@ module Gitlab end def illustration - 'manual_action' + { + image: 'illustrations/manual_action.svg', + size: 'svg-394', + title: _('This job requires a manual action'), + content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments') + } end end end diff --git a/lib/gitlab/ci/status/pending.rb b/lib/gitlab/ci/status/pending.rb index 63c0b5b501f..6a27a83696a 100644 --- a/lib/gitlab/ci/status/pending.rb +++ b/lib/gitlab/ci/status/pending.rb @@ -19,7 +19,12 @@ module Gitlab end def illustration - 'pending_job_empty' + { + image: 'illustrations/pending_job_empty.svg', + size: 'svg-430', + title: _('This job has not started yet'), + content: _('This job is in pending state and is waiting to be picked by a runner') + } end end end diff --git a/lib/gitlab/ci/status/skipped.rb b/lib/gitlab/ci/status/skipped.rb index 5dbe2847344..ff7529b5f03 100644 --- a/lib/gitlab/ci/status/skipped.rb +++ b/lib/gitlab/ci/status/skipped.rb @@ -19,7 +19,11 @@ module Gitlab end def illustration - 'skipped-job_empty' + { + image: 'illustrations/skipped-job_empty.svg', + size: 'svg-430', + title: _('This job has been skipped') + } end end end diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index 04b718c5897..0c9a751561f 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -115,7 +115,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.text).to eq 'canceled' expect(status.icon).to eq 'status_canceled' expect(status.favicon).to eq 'favicon_status_canceled' - expect(status.illustration).to eq 'canceled-job_empty' + expect(status.illustration).to include(:image, :size, :title) expect(status.label).to eq 'canceled' expect(status).to have_details expect(status).to have_action @@ -168,7 +168,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.text).to eq 'pending' expect(status.icon).to eq 'status_pending' expect(status.favicon).to eq 'favicon_status_pending' - expect(status.illustration).to eq 'pending_job_empty' + expect(status.illustration).to include(:image, :size, :title, :content) expect(status.label).to eq 'pending' expect(status).to have_details expect(status).to have_action @@ -194,7 +194,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.text).to eq 'skipped' expect(status.icon).to eq 'status_skipped' expect(status.favicon).to eq 'favicon_status_skipped' - expect(status.illustration).to eq 'skipped-job_empty' + expect(status.illustration).to include(:image, :size, :title) expect(status.label).to eq 'skipped' expect(status).to have_details expect(status).not_to have_action @@ -224,7 +224,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.group).to eq 'manual' expect(status.icon).to eq 'status_manual' expect(status.favicon).to eq 'favicon_status_manual' - expect(status.illustration).to eq 'manual_action' + expect(status.illustration).to include(:image, :size, :title, :content) expect(status.label).to include 'manual play action' expect(status).to have_details expect(status.action_path).to include 'play' diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb index fc1d9e726ab..70cf2c1d300 100644 --- a/spec/lib/gitlab/ci/status/canceled_spec.rb +++ b/spec/lib/gitlab/ci/status/canceled_spec.rb @@ -22,7 +22,7 @@ describe Gitlab::Ci::Status::Canceled do end describe '#illustration' do - it { expect(subject.illustration).to eq 'canceled-job_empty' } + it { expect(subject.illustration).to include(:image, :size, :title) } end describe '#group' do diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb index f7e3b0d8f1a..8a7771e0dc8 100644 --- a/spec/lib/gitlab/ci/status/created_spec.rb +++ b/spec/lib/gitlab/ci/status/created_spec.rb @@ -22,7 +22,7 @@ describe Gitlab::Ci::Status::Created do end describe '#illustration' do - it { expect(subject.illustration).to eq 'job_not_triggered' } + it { expect(subject.illustration).to include(:image, :size, :title, :content) } end describe '#group' do diff --git a/spec/lib/gitlab/ci/status/manual_spec.rb b/spec/lib/gitlab/ci/status/manual_spec.rb index 161c4774ff3..9b2dffe53d0 100644 --- a/spec/lib/gitlab/ci/status/manual_spec.rb +++ b/spec/lib/gitlab/ci/status/manual_spec.rb @@ -22,7 +22,7 @@ describe Gitlab::Ci::Status::Manual do end describe '#illustration' do - it { expect(subject.illustration).to eq 'manual_action' } + it { expect(subject.illustration).to include(:image, :size, :title, :content) } end describe '#group' do diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb index 92349308842..d859371df9d 100644 --- a/spec/lib/gitlab/ci/status/pending_spec.rb +++ b/spec/lib/gitlab/ci/status/pending_spec.rb @@ -22,7 +22,7 @@ describe Gitlab::Ci::Status::Pending do end describe '#illustration' do - it { expect(subject.illustration).to eq 'pending_job_empty' } + it { expect(subject.illustration).to include(:image, :size, :title, :content) } end describe '#group' do diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb index 993490c6d4e..f7fa8a035f2 100644 --- a/spec/lib/gitlab/ci/status/skipped_spec.rb +++ b/spec/lib/gitlab/ci/status/skipped_spec.rb @@ -22,7 +22,7 @@ describe Gitlab::Ci::Status::Skipped do end describe '#illustration' do - it { expect(subject.illustration).to eq 'skipped-job_empty' } + it { expect(subject.illustration).to include(:image, :size, :title) } end describe '#group' do From 4b0cbf630629cf1db5285baa0880513e4dd5ca16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 26 Mar 2018 20:23:11 +0200 Subject: [PATCH 07/25] Set up traces in jobs feature spec --- spec/features/projects/jobs_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index aa4c2757a25..d1cee58fce5 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -113,7 +113,7 @@ feature 'Jobs' do describe "GET /:project/jobs/:id" do context "Job from project" do - let(:job) { create(:ci_build, :success, pipeline: pipeline) } + let(:job) { create(:ci_build, :success, :trace_live, pipeline: pipeline) } before do visit project_job_path(project, job) @@ -136,7 +136,7 @@ feature 'Jobs' do end context 'when job is not running', :js do - let(:job) { create(:ci_build, :success, pipeline: pipeline) } + let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) } before do visit project_job_path(project, job) @@ -153,7 +153,7 @@ feature 'Jobs' do end context 'if job failed' do - let(:job) { create(:ci_build, :failed, pipeline: pipeline) } + let(:job) { create(:ci_build, :failed, :trace_artifact, pipeline: pipeline) } before do visit project_job_path(project, job) @@ -339,7 +339,7 @@ feature 'Jobs' do context 'job is successfull and has deployment' do let(:deployment) { create(:deployment) } - let(:job) { create(:ci_build, :success, environment: environment.name, deployments: [deployment], pipeline: pipeline) } + let(:job) { create(:ci_build, :success, :trace_artifact, environment: environment.name, deployments: [deployment], pipeline: pipeline) } it 'shows a link for the job' do visit project_job_path(project, job) @@ -349,7 +349,7 @@ feature 'Jobs' do end context 'job is complete and not successful' do - let(:job) { create(:ci_build, :failed, environment: environment.name, pipeline: pipeline) } + let(:job) { create(:ci_build, :failed, :trace_artifact, environment: environment.name, pipeline: pipeline) } it 'shows a link for the job' do visit project_job_path(project, job) @@ -360,7 +360,7 @@ feature 'Jobs' do context 'job creates a new deployment' do let!(:deployment) { create(:deployment, environment: environment, sha: project.commit.id) } - let(:job) { create(:ci_build, :success, environment: environment.name, pipeline: pipeline) } + let(:job) { create(:ci_build, :success, :trace_artifact, environment: environment.name, pipeline: pipeline) } it 'shows a link to latest deployment' do visit project_job_path(project, job) From 6df1eb14fa02268f16b961d15fc43b7b03586778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 26 Mar 2018 20:47:06 +0200 Subject: [PATCH 08/25] Use Gitlab::Ci::Status#illustration in job empty_states partial --- .../projects/jobs/_empty_states.html.haml | 38 +++++-------------- spec/features/projects/jobs_spec.rb | 18 +++++++++ 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/app/views/projects/jobs/_empty_states.html.haml b/app/views/projects/jobs/_empty_states.html.haml index 9e144dbfe32..6d4a9931923 100644 --- a/app/views/projects/jobs/_empty_states.html.haml +++ b/app/views/projects/jobs/_empty_states.html.haml @@ -1,29 +1,9 @@ -- if @build.playable? - = render 'empty_state', - illustration: 'illustrations/manual_action.svg', - illustration_size: 'svg-394', - title: _('This job requires a manual action'), - content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments'), - action: ( link_to _('Trigger this manual action'), play_project_job_path(@project, @build), method: :post, class: 'btn btn-primary', title: _('Trigger this manual action') ) -- elsif @build.created? - = render 'empty_state', - illustration: 'illustrations/job_not_triggered.svg', - illustration_size: 'svg-306', - title: _('This job has not been triggered yet'), - content: _('This job depends on upstream jobs that need to succeed in order for this job to be triggered') -- elsif @build.canceled? - = render 'empty_state', - illustration: 'illustrations/canceled-job_empty.svg', - illustration_size: 'svg-430', - title: _('This job has been canceled') -- elsif @build.skipped? - = render 'empty_state', - illustration: 'illustrations/skipped-job_empty.svg', - illustration_size: 'svg-430', - title: _('This job has been skipped') -- else - = render 'empty_state', - illustration: 'illustrations/pending_job_empty.svg', - illustration_size: 'svg-430', - title: _('This job has not started yet'), - content: _('This job is in pending state and is waiting to be picked by a runner') +- detailed_status = @build.detailed_status(current_user) +- illustration = detailed_status.illustration + += render 'empty_state', + illustration: illustration[:image], + illustration_size: illustration[:size], + title: illustration[:title], + content: illustration[:content], + action: @build.playable? ? link_to(_("Trigger this manual action"), detailed_status.action_path, method: detailed_status.action_method, class: 'btn btn-primary', title: _("Trigger this manual action")) : nil diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index d1cee58fce5..749a1b81872 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -379,6 +379,7 @@ feature 'Jobs' do end it 'shows manual action empty state' do + expect(page).to have_content(job.detailed_status(user).illustration[:title]) expect(page).to have_content('This job requires a manual action') expect(page).to have_content('This job depends on a user to trigger its process. Often they are used to deploy code to production environments') expect(page).to have_link('Trigger this manual action') @@ -402,6 +403,7 @@ feature 'Jobs' do end it 'shows empty state' do + expect(page).to have_content(job.detailed_status(user).illustration[:title]) expect(page).to have_content('This job has not been triggered yet') expect(page).to have_content('This job depends on upstream jobs that need to succeed in order for this job to be triggered') end @@ -415,6 +417,7 @@ feature 'Jobs' do end it 'shows pending empty state' do + expect(page).to have_content(job.detailed_status(user).illustration[:title]) expect(page).to have_content('This job has not started yet') expect(page).to have_content('This job is in pending state and is waiting to be picked by a runner') end @@ -441,11 +444,26 @@ feature 'Jobs' do end it 'renders empty state' do + expect(page).to have_content(job.detailed_status(user).illustration[:title]) expect(page).not_to have_selector('.js-build-output') expect(page).to have_content('This job has been canceled') end end end + + context 'Skipped job' do + let(:job) { create(:ci_build, :skipped, pipeline: pipeline) } + + before do + visit project_job_path(project, job) + end + + it 'renders empty state' do + expect(page).to have_content(job.detailed_status(user).illustration[:title]) + expect(page).not_to have_selector('.js-build-output') + expect(page).to have_content('This job has been skipped') + end + end end describe "POST /:project/jobs/:id/cancel", :js do From 0969f198496c2ab0b4be6dcd0d9c6434f71e780d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 28 Mar 2018 13:31:53 +0200 Subject: [PATCH 09/25] Move illustration to build specific statuses --- lib/gitlab/ci/status/build/action.rb | 9 +++++++++ lib/gitlab/ci/status/build/canceled.rb | 21 +++++++++++++++++++++ lib/gitlab/ci/status/build/created.rb | 22 ++++++++++++++++++++++ lib/gitlab/ci/status/build/factory.rb | 7 ++++++- lib/gitlab/ci/status/build/pending.rb | 22 ++++++++++++++++++++++ lib/gitlab/ci/status/build/skipped.rb | 21 +++++++++++++++++++++ lib/gitlab/ci/status/canceled.rb | 8 -------- lib/gitlab/ci/status/created.rb | 9 --------- lib/gitlab/ci/status/manual.rb | 9 --------- lib/gitlab/ci/status/pending.rb | 9 --------- lib/gitlab/ci/status/skipped.rb | 8 -------- 11 files changed, 101 insertions(+), 44 deletions(-) create mode 100644 lib/gitlab/ci/status/build/canceled.rb create mode 100644 lib/gitlab/ci/status/build/created.rb create mode 100644 lib/gitlab/ci/status/build/pending.rb create mode 100644 lib/gitlab/ci/status/build/skipped.rb diff --git a/lib/gitlab/ci/status/build/action.rb b/lib/gitlab/ci/status/build/action.rb index 6c9125647ad..17eb37448cb 100644 --- a/lib/gitlab/ci/status/build/action.rb +++ b/lib/gitlab/ci/status/build/action.rb @@ -14,6 +14,15 @@ module Gitlab end end + def illustration + { + image: 'illustrations/manual_action.svg', + size: 'svg-394', + title: _('This job requires a manual action'), + content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments'), + } + end + def self.matches?(build, user) build.playable? end diff --git a/lib/gitlab/ci/status/build/canceled.rb b/lib/gitlab/ci/status/build/canceled.rb new file mode 100644 index 00000000000..c83e2734a73 --- /dev/null +++ b/lib/gitlab/ci/status/build/canceled.rb @@ -0,0 +1,21 @@ +module Gitlab + module Ci + module Status + module Build + class Canceled < Status::Extended + def illustration + { + image: 'illustrations/canceled-job_empty.svg', + size: 'svg-430', + title: _('This job has been canceled') + } + end + + def self.matches?(build, user) + build.canceled? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/created.rb b/lib/gitlab/ci/status/build/created.rb new file mode 100644 index 00000000000..5be8e9de425 --- /dev/null +++ b/lib/gitlab/ci/status/build/created.rb @@ -0,0 +1,22 @@ +module Gitlab + module Ci + module Status + module Build + class Created < Status::Extended + def illustration + { + image: 'illustrations/job_not_triggered.svg', + size: 'svg-306', + title: _('This job has not been triggered yet'), + content: _('This job depends on upstream jobs that need to succeed in order for this job to be triggered') + } + end + + def self.matches?(build, user) + build.created? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index c852d607373..ae4b2bd79bc 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -9,7 +9,12 @@ module Gitlab [Status::Build::FailedAllowed, Status::Build::Play, Status::Build::Stop], - [Status::Build::Action]] + [Status::Build::Action], + [Status::Build::Action, + Status::Build::Canceled, + Status::Build::Created, + Status::Build::Pending, + Status::Build::Skipped]] end def self.common_helpers diff --git a/lib/gitlab/ci/status/build/pending.rb b/lib/gitlab/ci/status/build/pending.rb new file mode 100644 index 00000000000..9dd9a27ad57 --- /dev/null +++ b/lib/gitlab/ci/status/build/pending.rb @@ -0,0 +1,22 @@ +module Gitlab + module Ci + module Status + module Build + class Pending < Status::Extended + def illustration + { + image: 'illustrations/pending_job_empty.svg', + size: 'svg-430', + title: _('This job has not started yet'), + content: _('This job is in pending state and is waiting to be picked by a runner') + } + end + + def self.matches?(build, user) + build.pending? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/skipped.rb b/lib/gitlab/ci/status/build/skipped.rb new file mode 100644 index 00000000000..3e678d0baee --- /dev/null +++ b/lib/gitlab/ci/status/build/skipped.rb @@ -0,0 +1,21 @@ +module Gitlab + module Ci + module Status + module Build + class Skipped < Status::Extended + def illustration + { + image: 'illustrations/skipped-job_empty.svg', + size: 'svg-430', + title: _('This job has been skipped') + } + end + + def self.matches?(build, user) + build.skipped? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/canceled.rb b/lib/gitlab/ci/status/canceled.rb index bea8dcd5587..e6195a60d4f 100644 --- a/lib/gitlab/ci/status/canceled.rb +++ b/lib/gitlab/ci/status/canceled.rb @@ -17,14 +17,6 @@ module Gitlab def favicon 'favicon_status_canceled' end - - def illustration - { - image: 'illustrations/canceled-job_empty.svg', - size: 'svg-430', - title: _('This job has been canceled') - } - end end end end diff --git a/lib/gitlab/ci/status/created.rb b/lib/gitlab/ci/status/created.rb index 3b2a9e5d0ef..846f00b83dd 100644 --- a/lib/gitlab/ci/status/created.rb +++ b/lib/gitlab/ci/status/created.rb @@ -17,15 +17,6 @@ module Gitlab def favicon 'favicon_status_created' end - - def illustration - { - image: 'illustrations/job_not_triggered.svg', - size: 'svg-306', - title: _('This job has not been triggered yet'), - content: _('This job depends on upstream jobs that need to succeed in order for this job to be triggered') - } - end end end end diff --git a/lib/gitlab/ci/status/manual.rb b/lib/gitlab/ci/status/manual.rb index cb587a7815e..fc387e2fd25 100644 --- a/lib/gitlab/ci/status/manual.rb +++ b/lib/gitlab/ci/status/manual.rb @@ -17,15 +17,6 @@ module Gitlab def favicon 'favicon_status_manual' end - - def illustration - { - image: 'illustrations/manual_action.svg', - size: 'svg-394', - title: _('This job requires a manual action'), - content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments') - } - end end end end diff --git a/lib/gitlab/ci/status/pending.rb b/lib/gitlab/ci/status/pending.rb index 6a27a83696a..6780780db32 100644 --- a/lib/gitlab/ci/status/pending.rb +++ b/lib/gitlab/ci/status/pending.rb @@ -17,15 +17,6 @@ module Gitlab def favicon 'favicon_status_pending' end - - def illustration - { - image: 'illustrations/pending_job_empty.svg', - size: 'svg-430', - title: _('This job has not started yet'), - content: _('This job is in pending state and is waiting to be picked by a runner') - } - end end end end diff --git a/lib/gitlab/ci/status/skipped.rb b/lib/gitlab/ci/status/skipped.rb index ff7529b5f03..0dbdc4de426 100644 --- a/lib/gitlab/ci/status/skipped.rb +++ b/lib/gitlab/ci/status/skipped.rb @@ -17,14 +17,6 @@ module Gitlab def favicon 'favicon_status_skipped' end - - def illustration - { - image: 'illustrations/skipped-job_empty.svg', - size: 'svg-430', - title: _('This job has been skipped') - } - end end end end From c48f33c5bec02e8fd49326514023f6b6af66d693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 28 Mar 2018 13:51:30 +0200 Subject: [PATCH 10/25] Move action link to build extended status illustration --- app/views/projects/jobs/_empty_states.html.haml | 2 +- lib/gitlab/ci/status/build/action.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/projects/jobs/_empty_states.html.haml b/app/views/projects/jobs/_empty_states.html.haml index 6d4a9931923..4d2c776e538 100644 --- a/app/views/projects/jobs/_empty_states.html.haml +++ b/app/views/projects/jobs/_empty_states.html.haml @@ -6,4 +6,4 @@ illustration_size: illustration[:size], title: illustration[:title], content: illustration[:content], - action: @build.playable? ? link_to(_("Trigger this manual action"), detailed_status.action_path, method: detailed_status.action_method, class: 'btn btn-primary', title: _("Trigger this manual action")) : nil + action: illustration[:action_path] ? link_to(_("Trigger this manual action"), illustration[:action_path], method: illustration[:action_method], class: 'btn btn-primary', title: _("Trigger this manual action")) : nil diff --git a/lib/gitlab/ci/status/build/action.rb b/lib/gitlab/ci/status/build/action.rb index 17eb37448cb..f7f3d47c098 100644 --- a/lib/gitlab/ci/status/build/action.rb +++ b/lib/gitlab/ci/status/build/action.rb @@ -20,6 +20,8 @@ module Gitlab size: 'svg-394', title: _('This job requires a manual action'), content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments'), + action_path: action_path, + action_method: action_method } end From b57fcbe6162a17b02fc0516af2487ddd57c251bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 28 Mar 2018 14:57:09 +0200 Subject: [PATCH 11/25] Separate the manual empty state from the action empty state --- lib/gitlab/ci/status/build/action.rb | 11 ----------- lib/gitlab/ci/status/build/factory.rb | 2 +- lib/gitlab/ci/status/build/manual.rb | 24 ++++++++++++++++++++++++ 3 files changed, 25 insertions(+), 12 deletions(-) create mode 100644 lib/gitlab/ci/status/build/manual.rb diff --git a/lib/gitlab/ci/status/build/action.rb b/lib/gitlab/ci/status/build/action.rb index f7f3d47c098..6c9125647ad 100644 --- a/lib/gitlab/ci/status/build/action.rb +++ b/lib/gitlab/ci/status/build/action.rb @@ -14,17 +14,6 @@ module Gitlab end end - def illustration - { - image: 'illustrations/manual_action.svg', - size: 'svg-394', - title: _('This job requires a manual action'), - content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments'), - action_path: action_path, - action_method: action_method - } - end - def self.matches?(build, user) build.playable? end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index ae4b2bd79bc..004a9b305b7 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -10,7 +10,7 @@ module Gitlab Status::Build::Play, Status::Build::Stop], [Status::Build::Action], - [Status::Build::Action, + [Status::Build::Manual, Status::Build::Canceled, Status::Build::Created, Status::Build::Pending, diff --git a/lib/gitlab/ci/status/build/manual.rb b/lib/gitlab/ci/status/build/manual.rb new file mode 100644 index 00000000000..01e94e159a1 --- /dev/null +++ b/lib/gitlab/ci/status/build/manual.rb @@ -0,0 +1,24 @@ +module Gitlab + module Ci + module Status + module Build + class Manual < Status::Extended + def illustration + { + image: 'illustrations/manual_action.svg', + size: 'svg-394', + title: _('This job requires a manual action'), + content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments'), + action_path: play_project_job_path(subject.project, subject), + action_method: :post + } + end + + def self.matches?(build, user) + build.playable? + end + end + end + end + end +end From d6509274f32fce8b879051d19e4c9d2bb8f9d09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 28 Mar 2018 14:58:32 +0200 Subject: [PATCH 12/25] Fix tests for build empty state statuses --- spec/lib/gitlab/ci/status/build/canceled_spec.rb | 11 +++++++++++ spec/lib/gitlab/ci/status/build/created_spec.rb | 11 +++++++++++ spec/lib/gitlab/ci/status/build/manual_spec.rb | 13 +++++++++++++ spec/lib/gitlab/ci/status/build/pending_spec.rb | 11 +++++++++++ spec/lib/gitlab/ci/status/build/skipped_spec.rb | 11 +++++++++++ spec/lib/gitlab/ci/status/canceled_spec.rb | 4 ---- spec/lib/gitlab/ci/status/created_spec.rb | 4 ---- spec/lib/gitlab/ci/status/manual_spec.rb | 4 ---- spec/lib/gitlab/ci/status/pending_spec.rb | 4 ---- spec/lib/gitlab/ci/status/skipped_spec.rb | 4 ---- 10 files changed, 57 insertions(+), 20 deletions(-) create mode 100644 spec/lib/gitlab/ci/status/build/canceled_spec.rb create mode 100644 spec/lib/gitlab/ci/status/build/created_spec.rb create mode 100644 spec/lib/gitlab/ci/status/build/manual_spec.rb create mode 100644 spec/lib/gitlab/ci/status/build/pending_spec.rb create mode 100644 spec/lib/gitlab/ci/status/build/skipped_spec.rb diff --git a/spec/lib/gitlab/ci/status/build/canceled_spec.rb b/spec/lib/gitlab/ci/status/build/canceled_spec.rb new file mode 100644 index 00000000000..689b4237b6e --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/canceled_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Canceled do + subject do + described_class.new(double('subject')) + end + + describe '#illustration' do + it { expect(subject.illustration).to include(:image, :size, :title) } + end +end diff --git a/spec/lib/gitlab/ci/status/build/created_spec.rb b/spec/lib/gitlab/ci/status/build/created_spec.rb new file mode 100644 index 00000000000..ace553c67a1 --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/created_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Created do + subject do + described_class.new(double('subject')) + end + + describe '#illustration' do + it { expect(subject.illustration).to include(:image, :size, :title, :content) } + end +end diff --git a/spec/lib/gitlab/ci/status/build/manual_spec.rb b/spec/lib/gitlab/ci/status/build/manual_spec.rb new file mode 100644 index 00000000000..1cbbbfa7570 --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/manual_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Manual do + subject do + user = create(:user) + build = create(:ci_build, :manual) + described_class.new(Gitlab::Ci::Status::Core.new(build, user)) + end + + describe '#illustration' do + it { expect(subject.illustration).to include(:image, :size, :title, :content, :action_path, :action_method) } + end +end diff --git a/spec/lib/gitlab/ci/status/build/pending_spec.rb b/spec/lib/gitlab/ci/status/build/pending_spec.rb new file mode 100644 index 00000000000..2c114313536 --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/pending_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Pending do + subject do + described_class.new(double('subject')) + end + + describe '#illustration' do + it { expect(subject.illustration).to include(:image, :size, :title, :content) } + end +end diff --git a/spec/lib/gitlab/ci/status/build/skipped_spec.rb b/spec/lib/gitlab/ci/status/build/skipped_spec.rb new file mode 100644 index 00000000000..4667d10e6f6 --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/skipped_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Skipped do + subject do + described_class.new(double('subject')) + end + + describe '#illustration' do + it { expect(subject.illustration).to include(:image, :size, :title) } + end +end diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb index 70cf2c1d300..dc74d7e28c5 100644 --- a/spec/lib/gitlab/ci/status/canceled_spec.rb +++ b/spec/lib/gitlab/ci/status/canceled_spec.rb @@ -21,10 +21,6 @@ describe Gitlab::Ci::Status::Canceled do it { expect(subject.favicon).to eq 'favicon_status_canceled' } end - describe '#illustration' do - it { expect(subject.illustration).to include(:image, :size, :title) } - end - describe '#group' do it { expect(subject.group).to eq 'canceled' } end diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb index 8a7771e0dc8..ce4333f2aca 100644 --- a/spec/lib/gitlab/ci/status/created_spec.rb +++ b/spec/lib/gitlab/ci/status/created_spec.rb @@ -21,10 +21,6 @@ describe Gitlab::Ci::Status::Created do it { expect(subject.favicon).to eq 'favicon_status_created' } end - describe '#illustration' do - it { expect(subject.illustration).to include(:image, :size, :title, :content) } - end - describe '#group' do it { expect(subject.group).to eq 'created' } end diff --git a/spec/lib/gitlab/ci/status/manual_spec.rb b/spec/lib/gitlab/ci/status/manual_spec.rb index 9b2dffe53d0..0463f2e1aff 100644 --- a/spec/lib/gitlab/ci/status/manual_spec.rb +++ b/spec/lib/gitlab/ci/status/manual_spec.rb @@ -21,10 +21,6 @@ describe Gitlab::Ci::Status::Manual do it { expect(subject.favicon).to eq 'favicon_status_manual' } end - describe '#illustration' do - it { expect(subject.illustration).to include(:image, :size, :title, :content) } - end - describe '#group' do it { expect(subject.group).to eq 'manual' } end diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb index d859371df9d..0e25358dd8a 100644 --- a/spec/lib/gitlab/ci/status/pending_spec.rb +++ b/spec/lib/gitlab/ci/status/pending_spec.rb @@ -21,10 +21,6 @@ describe Gitlab::Ci::Status::Pending do it { expect(subject.favicon).to eq 'favicon_status_pending' } end - describe '#illustration' do - it { expect(subject.illustration).to include(:image, :size, :title, :content) } - end - describe '#group' do it { expect(subject.group).to eq 'pending' } end diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb index f7fa8a035f2..63694ca0ea6 100644 --- a/spec/lib/gitlab/ci/status/skipped_spec.rb +++ b/spec/lib/gitlab/ci/status/skipped_spec.rb @@ -21,10 +21,6 @@ describe Gitlab::Ci::Status::Skipped do it { expect(subject.favicon).to eq 'favicon_status_skipped' } end - describe '#illustration' do - it { expect(subject.illustration).to include(:image, :size, :title) } - end - describe '#group' do it { expect(subject.group).to eq 'skipped' } end From bb0483dc2f9501461407766f74600e0f3d283686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 28 Mar 2018 16:18:06 +0200 Subject: [PATCH 13/25] Move the empty state extended status to be the beginning --- lib/gitlab/ci/status/build/factory.rb | 14 +++++++------- .../lib/gitlab/ci/status/build/factory_spec.rb | 18 ++++++++++-------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index 004a9b305b7..b809a9cf766 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -4,17 +4,17 @@ module Gitlab module Build class Factory < Status::Factory def self.extended_statuses - [[Status::Build::Cancelable, + [[Status::Build::Manual, + Status::Build::Canceled, + Status::Build::Created, + Status::Build::Pending, + Status::Build::Skipped], + [Status::Build::Cancelable, Status::Build::Retryable], [Status::Build::FailedAllowed, Status::Build::Play, Status::Build::Stop], - [Status::Build::Action], - [Status::Build::Manual, - Status::Build::Canceled, - Status::Build::Created, - Status::Build::Pending, - Status::Build::Skipped]] + [Status::Build::Action]] end def self.common_helpers diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index 0c9a751561f..a68fed0a41c 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -104,7 +104,7 @@ describe Gitlab::Ci::Status::Build::Factory do it 'matches correct extended statuses' do expect(factory.extended_statuses) - .to eq [Gitlab::Ci::Status::Build::Retryable] + .to eq [Gitlab::Ci::Status::Build::Canceled, Gitlab::Ci::Status::Build::Retryable] end it 'fabricates a retryable build status' do @@ -157,7 +157,7 @@ describe Gitlab::Ci::Status::Build::Factory do it 'matches correct extended statuses' do expect(factory.extended_statuses) - .to eq [Gitlab::Ci::Status::Build::Cancelable] + .to eq [Gitlab::Ci::Status::Build::Pending, Gitlab::Ci::Status::Build::Cancelable] end it 'fabricates a cancelable build status' do @@ -182,12 +182,12 @@ describe Gitlab::Ci::Status::Build::Factory do expect(factory.core_status).to be_a Gitlab::Ci::Status::Skipped end - it 'does not match extended statuses' do - expect(factory.extended_statuses).to be_empty + it 'matches correct extended statuses' do + expect(factory.extended_statuses).to eq [Gitlab::Ci::Status::Build::Skipped] end - it 'fabricates a core skipped status' do - expect(status).to be_a Gitlab::Ci::Status::Skipped + it 'fabricates a skipped build status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Skipped end it 'fabricates status with correct details' do @@ -211,7 +211,8 @@ describe Gitlab::Ci::Status::Build::Factory do it 'matches correct extended statuses' do expect(factory.extended_statuses) - .to eq [Gitlab::Ci::Status::Build::Play, + .to eq [Gitlab::Ci::Status::Build::Manual, + Gitlab::Ci::Status::Build::Play, Gitlab::Ci::Status::Build::Action] end @@ -259,7 +260,8 @@ describe Gitlab::Ci::Status::Build::Factory do it 'matches correct extended statuses' do expect(factory.extended_statuses) - .to eq [Gitlab::Ci::Status::Build::Stop, + .to eq [Gitlab::Ci::Status::Build::Manual, + Gitlab::Ci::Status::Build::Stop, Gitlab::Ci::Status::Build::Action] end From ddabac46b918611ecaa82ea5af554a7f0f9b7407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 29 Mar 2018 19:24:19 +0200 Subject: [PATCH 14/25] Move action button titles to respective statuses --- app/views/projects/jobs/_empty_states.html.haml | 2 +- lib/gitlab/ci/status/build/cancelable.rb | 4 ++++ lib/gitlab/ci/status/build/manual.rb | 4 +--- lib/gitlab/ci/status/build/play.rb | 4 ++++ lib/gitlab/ci/status/build/retryable.rb | 4 ++++ lib/gitlab/ci/status/build/stop.rb | 4 ++++ spec/lib/gitlab/ci/status/build/cancelable_spec.rb | 4 ++++ spec/lib/gitlab/ci/status/build/manual_spec.rb | 2 +- spec/lib/gitlab/ci/status/build/play_spec.rb | 4 ++++ spec/lib/gitlab/ci/status/build/retryable_spec.rb | 4 ++++ spec/lib/gitlab/ci/status/build/stop_spec.rb | 4 ++++ 11 files changed, 35 insertions(+), 5 deletions(-) diff --git a/app/views/projects/jobs/_empty_states.html.haml b/app/views/projects/jobs/_empty_states.html.haml index 4d2c776e538..e5198d047df 100644 --- a/app/views/projects/jobs/_empty_states.html.haml +++ b/app/views/projects/jobs/_empty_states.html.haml @@ -6,4 +6,4 @@ illustration_size: illustration[:size], title: illustration[:title], content: illustration[:content], - action: illustration[:action_path] ? link_to(_("Trigger this manual action"), illustration[:action_path], method: illustration[:action_method], class: 'btn btn-primary', title: _("Trigger this manual action")) : nil + action: detailed_status.has_action? ? link_to(detailed_status.action_button_title, detailed_status.action_path, method: detailed_status.action_method, class: 'btn btn-primary', title: detailed_status.action_button_title) : nil diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index 2d9166d6bdd..024047d4983 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -23,6 +23,10 @@ module Gitlab 'Cancel' end + def action_button_title + _('Cancel this job') + end + def self.matches?(build, user) build.cancelable? end diff --git a/lib/gitlab/ci/status/build/manual.rb b/lib/gitlab/ci/status/build/manual.rb index 01e94e159a1..042da6392d3 100644 --- a/lib/gitlab/ci/status/build/manual.rb +++ b/lib/gitlab/ci/status/build/manual.rb @@ -8,9 +8,7 @@ module Gitlab image: 'illustrations/manual_action.svg', size: 'svg-394', title: _('This job requires a manual action'), - content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments'), - action_path: play_project_job_path(subject.project, subject), - action_method: :post + content: _('This job depends on a user to trigger its process. Often they are used to deploy code to production environments') } end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index b7b45466d3b..a8b9ebf0803 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -19,6 +19,10 @@ module Gitlab 'Play' end + def action_button_title + _('Trigger this manual action') + end + def action_path play_project_job_path(subject.project, subject) end diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 44ffe783e50..5aeb8e51480 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -15,6 +15,10 @@ module Gitlab 'Retry' end + def action_button_title + _('Retry this job') + end + def action_path retry_project_job_path(subject.project, subject) end diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 46e730797e4..dea838bfa39 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -19,6 +19,10 @@ module Gitlab 'Stop' end + def action_button_title + _('Stop this environment') + end + def action_path play_project_job_path(subject.project, subject) end diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb index 9cdebaa5cf2..0b88e0c7c9c 100644 --- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb +++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb @@ -72,6 +72,10 @@ describe Gitlab::Ci::Status::Build::Cancelable do describe '#action_title' do it { expect(subject.action_title).to eq 'Cancel' } end + + describe '#action_button_title' do + it { expect(subject.action_button_title).to eq 'Cancel this job' } + end end describe '.matches?' do diff --git a/spec/lib/gitlab/ci/status/build/manual_spec.rb b/spec/lib/gitlab/ci/status/build/manual_spec.rb index 1cbbbfa7570..c8c650d4e56 100644 --- a/spec/lib/gitlab/ci/status/build/manual_spec.rb +++ b/spec/lib/gitlab/ci/status/build/manual_spec.rb @@ -8,6 +8,6 @@ describe Gitlab::Ci::Status::Build::Manual do end describe '#illustration' do - it { expect(subject.illustration).to include(:image, :size, :title, :content, :action_path, :action_method) } + it { expect(subject.illustration).to include(:image, :size, :title, :content) } end end diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index 81d5f553fd1..b0c3a8bb398 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -53,6 +53,10 @@ describe Gitlab::Ci::Status::Build::Play do it { expect(subject.action_title).to eq 'Play' } end + describe '#action_button_title' do + it { expect(subject.action_button_title).to eq 'Trigger this manual action' } + end + describe '.matches?' do subject { described_class.matches?(build, user) } diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb index 14d42e0d70f..d6e909f6268 100644 --- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb +++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb @@ -72,6 +72,10 @@ describe Gitlab::Ci::Status::Build::Retryable do describe '#action_title' do it { expect(subject.action_title).to eq 'Retry' } end + + describe '#action_button_title' do + it { expect(subject.action_button_title).to eq 'Retry this job' } + end end describe '.matches?' do diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb index 18e250772f0..7a72ca2c14d 100644 --- a/spec/lib/gitlab/ci/status/build/stop_spec.rb +++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb @@ -44,6 +44,10 @@ describe Gitlab::Ci::Status::Build::Stop do describe '#action_title' do it { expect(subject.action_title).to eq 'Stop' } end + + describe '#action_button_title' do + it { expect(subject.action_button_title).to eq 'Stop this environment' } + end end describe '.matches?' do From 3447d71e73a538dbcb30fb478474bdd526ff38a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 29 Mar 2018 19:39:30 +0200 Subject: [PATCH 15/25] Add specs for empty states .matches? methods --- .../gitlab/ci/status/build/canceled_spec.rb | 22 ++++++++++++++++++ .../gitlab/ci/status/build/created_spec.rb | 22 ++++++++++++++++++ .../lib/gitlab/ci/status/build/manual_spec.rb | 23 ++++++++++++++++++- .../gitlab/ci/status/build/pending_spec.rb | 22 ++++++++++++++++++ .../gitlab/ci/status/build/skipped_spec.rb | 22 ++++++++++++++++++ 5 files changed, 110 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/ci/status/build/canceled_spec.rb b/spec/lib/gitlab/ci/status/build/canceled_spec.rb index 689b4237b6e..c6b5cc68770 100644 --- a/spec/lib/gitlab/ci/status/build/canceled_spec.rb +++ b/spec/lib/gitlab/ci/status/build/canceled_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Canceled do + let(:user) { create(:user) } + subject do described_class.new(double('subject')) end @@ -8,4 +10,24 @@ describe Gitlab::Ci::Status::Build::Canceled do describe '#illustration' do it { expect(subject.illustration).to include(:image, :size, :title) } end + + describe '.matches?' do + subject {described_class.matches?(build, user) } + + context 'when build is canceled' do + let(:build) { create(:ci_build, :canceled) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not canceled' do + let(:build) { create(:ci_build) } + + it 'does not match' do + expect(subject).to be false + end + end + end end diff --git a/spec/lib/gitlab/ci/status/build/created_spec.rb b/spec/lib/gitlab/ci/status/build/created_spec.rb index ace553c67a1..8bdfe6ef7a2 100644 --- a/spec/lib/gitlab/ci/status/build/created_spec.rb +++ b/spec/lib/gitlab/ci/status/build/created_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Created do + let(:user) { create(:user) } + subject do described_class.new(double('subject')) end @@ -8,4 +10,24 @@ describe Gitlab::Ci::Status::Build::Created do describe '#illustration' do it { expect(subject.illustration).to include(:image, :size, :title, :content) } end + + describe '.matches?' do + subject {described_class.matches?(build, user) } + + context 'when build is created' do + let(:build) { create(:ci_build, :created) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not created' do + let(:build) { create(:ci_build) } + + it 'does not match' do + expect(subject).to be false + end + end + end end diff --git a/spec/lib/gitlab/ci/status/build/manual_spec.rb b/spec/lib/gitlab/ci/status/build/manual_spec.rb index c8c650d4e56..6386296f992 100644 --- a/spec/lib/gitlab/ci/status/build/manual_spec.rb +++ b/spec/lib/gitlab/ci/status/build/manual_spec.rb @@ -1,8 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Manual do + let(:user) { create(:user) } + subject do - user = create(:user) build = create(:ci_build, :manual) described_class.new(Gitlab::Ci::Status::Core.new(build, user)) end @@ -10,4 +11,24 @@ describe Gitlab::Ci::Status::Build::Manual do describe '#illustration' do it { expect(subject.illustration).to include(:image, :size, :title, :content) } end + + describe '.matches?' do + subject {described_class.matches?(build, user) } + + context 'when build is manual' do + let(:build) { create(:ci_build, :manual) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not manual' do + let(:build) { create(:ci_build) } + + it 'does not match' do + expect(subject).to be false + end + end + end end diff --git a/spec/lib/gitlab/ci/status/build/pending_spec.rb b/spec/lib/gitlab/ci/status/build/pending_spec.rb index 2c114313536..4cf70828e53 100644 --- a/spec/lib/gitlab/ci/status/build/pending_spec.rb +++ b/spec/lib/gitlab/ci/status/build/pending_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Pending do + let(:user) { create(:user) } + subject do described_class.new(double('subject')) end @@ -8,4 +10,24 @@ describe Gitlab::Ci::Status::Build::Pending do describe '#illustration' do it { expect(subject.illustration).to include(:image, :size, :title, :content) } end + + describe '.matches?' do + subject {described_class.matches?(build, user) } + + context 'when build is pending' do + let(:build) { create(:ci_build, :pending) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not pending' do + let(:build) { create(:ci_build, :success) } + + it 'does not match' do + expect(subject).to be false + end + end + end end diff --git a/spec/lib/gitlab/ci/status/build/skipped_spec.rb b/spec/lib/gitlab/ci/status/build/skipped_spec.rb index 4667d10e6f6..46f6933025a 100644 --- a/spec/lib/gitlab/ci/status/build/skipped_spec.rb +++ b/spec/lib/gitlab/ci/status/build/skipped_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Skipped do + let(:user) { create(:user) } + subject do described_class.new(double('subject')) end @@ -8,4 +10,24 @@ describe Gitlab::Ci::Status::Build::Skipped do describe '#illustration' do it { expect(subject.illustration).to include(:image, :size, :title) } end + + describe '.matches?' do + subject {described_class.matches?(build, user) } + + context 'when build is skipped' do + let(:build) { create(:ci_build, :skipped) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not skipped' do + let(:build) { create(:ci_build) } + + it 'does not match' do + expect(subject).to be false + end + end + end end From 8a4ec84965bb746c86e7e73aa2f9abb238d85ca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 3 Apr 2018 16:54:03 +0200 Subject: [PATCH 16/25] Add action_button_title Core status method --- lib/gitlab/ci/status/core.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 89fb9a0b7bc..031bd4cd29c 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -61,6 +61,10 @@ module Gitlab def action_title raise NotImplementedError end + + def action_button_title + raise NotImplementedError + end end end end From 8ac7ea4301eb9c1c90a592e878ac32154f6eb354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 3 Apr 2018 19:47:04 +0200 Subject: [PATCH 17/25] Set up traces in build show spec --- spec/views/projects/jobs/show.html.haml_spec.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/spec/views/projects/jobs/show.html.haml_spec.rb b/spec/views/projects/jobs/show.html.haml_spec.rb index 6a67da79ec5..64d6ae6d2a3 100644 --- a/spec/views/projects/jobs/show.html.haml_spec.rb +++ b/spec/views/projects/jobs/show.html.haml_spec.rb @@ -18,7 +18,7 @@ describe 'projects/jobs/show' do describe 'environment info in job view' do context 'job with latest deployment' do let(:build) do - create(:ci_build, :success, environment: 'staging') + create(:ci_build, :success, :trace_artifact, environment: 'staging') end before do @@ -37,11 +37,11 @@ describe 'projects/jobs/show' do context 'job with outdated deployment' do let(:build) do - create(:ci_build, :success, environment: 'staging', pipeline: pipeline) + create(:ci_build, :success, :trace_artifact, environment: 'staging', pipeline: pipeline) end let(:second_build) do - create(:ci_build, :success, environment: 'staging', pipeline: pipeline) + create(:ci_build, :success, :trace_artifact, environment: 'staging', pipeline: pipeline) end let(:environment) do @@ -67,7 +67,7 @@ describe 'projects/jobs/show' do context 'job failed to deploy' do let(:build) do - create(:ci_build, :failed, environment: 'staging', pipeline: pipeline) + create(:ci_build, :failed, :trace_artifact, environment: 'staging', pipeline: pipeline) end let!(:environment) do @@ -85,7 +85,7 @@ describe 'projects/jobs/show' do context 'job will deploy' do let(:build) do - create(:ci_build, :running, environment: 'staging', pipeline: pipeline) + create(:ci_build, :running, :trace_live, environment: 'staging', pipeline: pipeline) end context 'when environment exists' do @@ -133,7 +133,7 @@ describe 'projects/jobs/show' do context 'job that failed to deploy and environment has not been created' do let(:build) do - create(:ci_build, :failed, environment: 'staging', pipeline: pipeline) + create(:ci_build, :failed, :trace_artifact, environment: 'staging', pipeline: pipeline) end let!(:environment) do @@ -151,7 +151,7 @@ describe 'projects/jobs/show' do context 'job that will deploy and environment has not been created' do let(:build) do - create(:ci_build, :running, environment: 'staging', pipeline: pipeline) + create(:ci_build, :running, :trace_live, environment: 'staging', pipeline: pipeline) end let!(:environment) do @@ -171,8 +171,9 @@ describe 'projects/jobs/show' do end context 'when job is running' do + let(:build) { create(:ci_build, :trace_live, :running, pipeline: pipeline) } + before do - build.run! render end From 5af00309d3a4d50e89b278981dcd6485ae4e77f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 3 Apr 2018 20:14:33 +0200 Subject: [PATCH 18/25] Properly set up jobs in mini pipeline graph spec --- .../merge_request/user_sees_mini_pipeline_graph_spec.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb index a43ba05c64c..fd1629746ef 100644 --- a/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb +++ b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb @@ -9,6 +9,7 @@ describe 'Merge request < User sees mini pipeline graph', :js do before do build.run + build.trace.set('hello') sign_in(user) visit_merge_request end @@ -26,15 +27,15 @@ describe 'Merge request < User sees mini pipeline graph', :js do let(:artifacts_file2) { fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'), 'image/png') } before do - create(:ci_build, pipeline: pipeline, legacy_artifacts_file: artifacts_file1) - create(:ci_build, pipeline: pipeline, when: 'manual') + create(:ci_build, :success, :trace_artifact, pipeline: pipeline, legacy_artifacts_file: artifacts_file1) + create(:ci_build, :manual, pipeline: pipeline, when: 'manual') end it 'avoids repeated database queries' do before = ActiveRecord::QueryRecorder.new { visit_merge_request(format: :json, serializer: 'widget') } - create(:ci_build, pipeline: pipeline, legacy_artifacts_file: artifacts_file2) - create(:ci_build, pipeline: pipeline, when: 'manual') + create(:ci_build, :success, :trace_artifact, pipeline: pipeline, legacy_artifacts_file: artifacts_file2) + create(:ci_build, :manual, pipeline: pipeline, when: 'manual') after = ActiveRecord::QueryRecorder.new { visit_merge_request(format: :json, serializer: 'widget') } From ff1383fb11267996bf040260513e6d3d8f468def Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 3 Apr 2018 20:17:26 +0200 Subject: [PATCH 19/25] Properly set up job trace in user_browses_job_spec --- spec/features/projects/jobs/user_browses_job_spec.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb index 4c49cff30d4..8fa902cc386 100644 --- a/spec/features/projects/jobs/user_browses_job_spec.rb +++ b/spec/features/projects/jobs/user_browses_job_spec.rb @@ -1,16 +1,14 @@ require 'spec_helper' describe 'User browses a job', :js do - let!(:build) { create(:ci_build, :running, :coverage, pipeline: pipeline) } - let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.sha, ref: 'master') } - let(:project) { create(:project, :repository, namespace: user.namespace) } let(:user) { create(:user) } + let(:project) { create(:project, :repository, namespace: user.namespace) } + let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.sha, ref: 'master') } + let!(:build) { create(:ci_build, :success, :trace_artifact, :coverage, pipeline: pipeline) } before do project.add_master(user) project.enable_ci - build.success - build.trace.set('job trace') sign_in(user) From 75f8a45f6a202aa5bec00613dcd16573fc1cc46a Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 4 Apr 2018 19:28:53 +0100 Subject: [PATCH 20/25] Scrolls to the top of the page before erase click --- spec/features/projects/jobs/user_browses_job_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb index 8fa902cc386..a728e6eb996 100644 --- a/spec/features/projects/jobs/user_browses_job_spec.rb +++ b/spec/features/projects/jobs/user_browses_job_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe 'User browses a job', :js do let(:user) { create(:user) } + let(:user_access_level) { :developer } let(:project) { create(:project, :repository, namespace: user.namespace) } let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.sha, ref: 'master') } let!(:build) { create(:ci_build, :success, :trace_artifact, :coverage, pipeline: pipeline) } @@ -19,7 +20,9 @@ describe 'User browses a job', :js do expect(page).to have_content("Job ##{build.id}") expect(page).to have_css('#build-trace') - accept_confirm { click_link('Erase') } + # scroll to the top of the page first + execute_script "window.scrollTo(0,0)" + accept_confirm { find('.js-erase-link').click } expect(page).to have_no_css('.artifacts') expect(build).not_to have_trace From d883fe1cce01f53a3acc442e8b94ca20540e0525 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 4 Apr 2018 21:00:32 +0200 Subject: [PATCH 21/25] Add success CI Build empty state status --- lib/gitlab/ci/status/build/factory.rb | 3 +- lib/gitlab/ci/status/build/success.rb | 21 ++++++++++++ .../gitlab/ci/status/build/factory_spec.rb | 16 +++++++++ .../gitlab/ci/status/build/success_spec.rb | 33 +++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 lib/gitlab/ci/status/build/success.rb create mode 100644 spec/lib/gitlab/ci/status/build/success_spec.rb diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index b809a9cf766..d3a34384f6f 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -8,7 +8,8 @@ module Gitlab Status::Build::Canceled, Status::Build::Created, Status::Build::Pending, - Status::Build::Skipped], + Status::Build::Skipped, + Status::Build::Success], [Status::Build::Cancelable, Status::Build::Retryable], [Status::Build::FailedAllowed, diff --git a/lib/gitlab/ci/status/build/success.rb b/lib/gitlab/ci/status/build/success.rb new file mode 100644 index 00000000000..bafc1b2f93a --- /dev/null +++ b/lib/gitlab/ci/status/build/success.rb @@ -0,0 +1,21 @@ +module Gitlab + module Ci + module Status + module Build + class Success < Status::Extended + def illustration + { + image: 'illustrations/skipped-job_empty.svg', + size: 'svg-430', + title: _('Job has been erased') + } + end + + def self.matches?(build, user) + build.success? + end + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index a68fed0a41c..28166d08c02 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -36,6 +36,22 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status).to have_details expect(status).to have_action end + + context 'when job log gets erased' do + before do + build.trace.set(nil) + end + + it 'matches correct extended statuses' do + expect(factory.extended_statuses) + .to eq [Gitlab::Ci::Status::Build::Success, + Gitlab::Ci::Status::Build::Retryable] + end + + it 'fabricates a retryable build status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Retryable + end + end end context 'when build is failed' do diff --git a/spec/lib/gitlab/ci/status/build/success_spec.rb b/spec/lib/gitlab/ci/status/build/success_spec.rb new file mode 100644 index 00000000000..730cd7aefbc --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/success_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Success do + let(:user) { create(:user) } + + subject do + described_class.new(double('subject')) + end + + describe '#illustration' do + it { expect(subject.illustration).to include(:image, :size, :title) } + end + + describe '.matches?' do + subject {described_class.matches?(build, user) } + + context 'when build succeeded' do + let(:build) { create(:ci_build, :success) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build did not succeed' do + let(:build) { create(:ci_build, :skipped) } + + it 'does not match' do + expect(subject).to be false + end + end + end +end From 00b45348b9dca5c394fa64a7f4975232dc712b5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 4 Apr 2018 22:15:18 +0200 Subject: [PATCH 22/25] Fix job setup in success empty state specs --- lib/gitlab/ci/status/build/success.rb | 2 +- spec/lib/gitlab/ci/status/build/factory_spec.rb | 4 ++-- spec/lib/gitlab/ci/status/build/success_spec.rb | 10 ++++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/ci/status/build/success.rb b/lib/gitlab/ci/status/build/success.rb index bafc1b2f93a..daf43315c90 100644 --- a/lib/gitlab/ci/status/build/success.rb +++ b/lib/gitlab/ci/status/build/success.rb @@ -12,7 +12,7 @@ module Gitlab end def self.matches?(build, user) - build.success? + !build.has_trace? && build.success? end end end diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index 28166d08c02..d68d8f6f105 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Ci::Status::Build::Factory do end context 'when build is successful' do - let(:build) { create(:ci_build, :success) } + let(:build) { create(:ci_build, :success, :trace_artifact) } it 'matches correct core status' do expect(factory.core_status).to be_a Gitlab::Ci::Status::Success @@ -39,7 +39,7 @@ describe Gitlab::Ci::Status::Build::Factory do context 'when job log gets erased' do before do - build.trace.set(nil) + build.erase end it 'matches correct extended statuses' do diff --git a/spec/lib/gitlab/ci/status/build/success_spec.rb b/spec/lib/gitlab/ci/status/build/success_spec.rb index 730cd7aefbc..e67ab461463 100644 --- a/spec/lib/gitlab/ci/status/build/success_spec.rb +++ b/spec/lib/gitlab/ci/status/build/success_spec.rb @@ -12,18 +12,20 @@ describe Gitlab::Ci::Status::Build::Success do end describe '.matches?' do - subject {described_class.matches?(build, user) } + subject { described_class.matches?(build, user) } - context 'when build succeeded' do + context 'when build succeeded but does not have trace' do let(:build) { create(:ci_build, :success) } it 'is a correct match' do + build.erase + expect(subject).to be true end end - context 'when build did not succeed' do - let(:build) { create(:ci_build, :skipped) } + context 'when build succeed but has trace' do + let!(:build) { create(:ci_build, :success, :trace_artifact) } it 'does not match' do expect(subject).to be false From f379e1b370d033097c1d0102401eb6156384890c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 5 Apr 2018 13:52:54 +0200 Subject: [PATCH 23/25] Rename success to erased empty state spec --- .../ci/status/build/{success.rb => erased.rb} | 4 +-- lib/gitlab/ci/status/build/factory.rb | 2 +- .../build/{success_spec.rb => erased_spec.rb} | 12 +++---- .../gitlab/ci/status/build/factory_spec.rb | 35 ++++++++++++------- 4 files changed, 31 insertions(+), 22 deletions(-) rename lib/gitlab/ci/status/build/{success.rb => erased.rb} (80%) rename spec/lib/gitlab/ci/status/build/{success_spec.rb => erased_spec.rb} (62%) diff --git a/lib/gitlab/ci/status/build/success.rb b/lib/gitlab/ci/status/build/erased.rb similarity index 80% rename from lib/gitlab/ci/status/build/success.rb rename to lib/gitlab/ci/status/build/erased.rb index daf43315c90..3a5113b16b6 100644 --- a/lib/gitlab/ci/status/build/success.rb +++ b/lib/gitlab/ci/status/build/erased.rb @@ -2,7 +2,7 @@ module Gitlab module Ci module Status module Build - class Success < Status::Extended + class Erased < Status::Extended def illustration { image: 'illustrations/skipped-job_empty.svg', @@ -12,7 +12,7 @@ module Gitlab end def self.matches?(build, user) - !build.has_trace? && build.success? + build.erased? end end end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index d3a34384f6f..466a0a989ad 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -9,7 +9,7 @@ module Gitlab Status::Build::Created, Status::Build::Pending, Status::Build::Skipped, - Status::Build::Success], + Status::Build::Erased], [Status::Build::Cancelable, Status::Build::Retryable], [Status::Build::FailedAllowed, diff --git a/spec/lib/gitlab/ci/status/build/success_spec.rb b/spec/lib/gitlab/ci/status/build/erased_spec.rb similarity index 62% rename from spec/lib/gitlab/ci/status/build/success_spec.rb rename to spec/lib/gitlab/ci/status/build/erased_spec.rb index e67ab461463..0acd271e375 100644 --- a/spec/lib/gitlab/ci/status/build/success_spec.rb +++ b/spec/lib/gitlab/ci/status/build/erased_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Status::Build::Success do +describe Gitlab::Ci::Status::Build::Erased do let(:user) { create(:user) } subject do @@ -14,18 +14,16 @@ describe Gitlab::Ci::Status::Build::Success do describe '.matches?' do subject { described_class.matches?(build, user) } - context 'when build succeeded but does not have trace' do - let(:build) { create(:ci_build, :success) } + context 'when build is erased' do + let(:build) { create(:ci_build, :success, :erased) } it 'is a correct match' do - build.erase - expect(subject).to be true end end - context 'when build succeed but has trace' do - let!(:build) { create(:ci_build, :success, :trace_artifact) } + context 'when build is not erased' do + let(:build) { create(:ci_build, :success, :trace_artifact) } it 'does not match' do expect(subject).to be false diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index d68d8f6f105..94eedc50bb2 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -36,21 +36,32 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status).to have_details expect(status).to have_action end + end - context 'when job log gets erased' do - before do - build.erase - end + context 'when build is erased' do + let(:build) { create(:ci_build, :success, :erased) } - it 'matches correct extended statuses' do - expect(factory.extended_statuses) - .to eq [Gitlab::Ci::Status::Build::Success, - Gitlab::Ci::Status::Build::Retryable] - end + it 'matches correct core status' do + expect(factory.core_status).to be_a Gitlab::Ci::Status::Success + end - it 'fabricates a retryable build status' do - expect(status).to be_a Gitlab::Ci::Status::Build::Retryable - end + it 'matches correct extended statuses' do + expect(factory.extended_statuses) + .to eq [Gitlab::Ci::Status::Build::Erased, + Gitlab::Ci::Status::Build::Retryable] + end + + it 'fabricates a retryable build status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Retryable + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'passed' + expect(status.icon).to eq 'status_success' + expect(status.favicon).to eq 'favicon_status_success' + expect(status.label).to eq 'passed' + expect(status).to have_details + expect(status).to have_action end end From 99edc15127d9d23475c94079d53e2893f58c042a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 5 Apr 2018 17:32:03 +0200 Subject: [PATCH 24/25] Put Erased empty state at the beginning --- lib/gitlab/ci/status/build/factory.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index 466a0a989ad..9d2d4170266 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -4,12 +4,12 @@ module Gitlab module Build class Factory < Status::Factory def self.extended_statuses - [[Status::Build::Manual, + [[Status::Build::Erased, + Status::Build::Manual, Status::Build::Canceled, Status::Build::Created, Status::Build::Pending, - Status::Build::Skipped, - Status::Build::Erased], + Status::Build::Skipped], [Status::Build::Cancelable, Status::Build::Retryable], [Status::Build::FailedAllowed, From 6d0c9c9403483a274dfe55094123a99a1937bbff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 6 Apr 2018 16:47:50 +0200 Subject: [PATCH 25/25] Add missing trace artifacts to jobs in spec --- spec/features/projects/jobs/user_browses_job_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb index 3c8b28e8408..bff5bbe99af 100644 --- a/spec/features/projects/jobs/user_browses_job_spec.rb +++ b/spec/features/projects/jobs/user_browses_job_spec.rb @@ -37,7 +37,7 @@ describe 'User browses a job', :js do end context 'with a failed job' do - let!(:build) { create(:ci_build, :failed, pipeline: pipeline) } + let!(:build) { create(:ci_build, :failed, :trace_artifact, pipeline: pipeline) } it 'displays the failure reason' do within('.builds-container') do @@ -48,7 +48,7 @@ describe 'User browses a job', :js do end context 'when a failed job has been retried' do - let!(:build) { create(:ci_build, :failed, :retried, pipeline: pipeline) } + let!(:build) { create(:ci_build, :failed, :retried, :trace_artifact, pipeline: pipeline) } it 'displays the failure reason and retried label' do within('.builds-container') do