diff options
author | Phil Hughes <me@iamphill.com> | 2018-10-04 15:59:57 +0300 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-10-04 15:59:57 +0300 |
commit | 5f3c90cd0af995e96a754d168d9c0e1e38e5df32 (patch) | |
tree | de8ca2bab58ac104d530982bf157617245e98cf0 | |
parent | 2090f2d0d6e2f034f631dbbe3c7e9cf48f9135d2 (diff) | |
parent | e13baff004810fba3b7fd6eeffc3e4de5af952a1 (diff) |
Merge branch '50904-empty-states' into 'master'
Renders empty states in the Vue app in Job page
See merge request gitlab-org/gitlab-ce!22087
-rw-r--r-- | app/assets/javascripts/jobs/components/empty_state.vue | 4 | ||||
-rw-r--r-- | app/assets/javascripts/jobs/components/job_app.vue | 15 | ||||
-rw-r--r-- | app/assets/javascripts/jobs/store/getters.js | 13 | ||||
-rw-r--r-- | app/serializers/detailed_status_entity.rb | 8 | ||||
-rw-r--r-- | app/views/projects/jobs/_empty_state.html.haml | 18 | ||||
-rw-r--r-- | app/views/projects/jobs/_empty_states.html.haml | 9 | ||||
-rw-r--r-- | app/views/projects/jobs/show.html.haml | 2 | ||||
-rw-r--r-- | spec/features/projects/jobs_spec.rb | 12 | ||||
-rw-r--r-- | spec/javascripts/jobs/components/empty_state_spec.js | 2 | ||||
-rw-r--r-- | spec/javascripts/jobs/components/job_app_spec.js | 139 | ||||
-rw-r--r-- | spec/javascripts/jobs/store/getters_spec.js | 92 |
11 files changed, 274 insertions, 40 deletions
diff --git a/app/assets/javascripts/jobs/components/empty_state.vue b/app/assets/javascripts/jobs/components/empty_state.vue index ff45a5b05f8..ff500eddddb 100644 --- a/app/assets/javascripts/jobs/components/empty_state.vue +++ b/app/assets/javascripts/jobs/components/empty_state.vue @@ -27,7 +27,7 @@ value === null || (Object.prototype.hasOwnProperty.call(value, 'path') && Object.prototype.hasOwnProperty.call(value, 'method') && - Object.prototype.hasOwnProperty.call(value, 'title')) + Object.prototype.hasOwnProperty.call(value, 'button_title')) ); }, }, @@ -67,7 +67,7 @@ :data-method="action.method" class="js-job-empty-state-action btn btn-primary" > - {{ action.title }} + {{ action.button_title }} </a> </div> </div> diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue index bac8bd71d64..047e55866ce 100644 --- a/app/assets/javascripts/jobs/components/job_app.vue +++ b/app/assets/javascripts/jobs/components/job_app.vue @@ -2,6 +2,7 @@ import { mapGetters, mapState } from 'vuex'; import CiHeader from '~/vue_shared/components/header_ci_component.vue'; import Callout from '~/vue_shared/components/callout.vue'; + import EmptyState from './empty_state.vue'; import EnvironmentsBlock from './environments_block.vue'; import ErasedBlock from './erased_block.vue'; import StuckBlock from './stuck_block.vue'; @@ -11,6 +12,7 @@ components: { CiHeader, Callout, + EmptyState, EnvironmentsBlock, ErasedBlock, StuckBlock, @@ -31,6 +33,8 @@ 'jobHasStarted', 'hasEnvironment', 'isJobStuck', + 'hasTrace', + 'emptyStateIllustration', ]), }, }; @@ -77,12 +81,14 @@ <environments-block v-if="hasEnvironment" + class="js-job-environment" :deployment-status="job.deployment_status" :icon-status="job.status" /> <erased-block v-if="job.erased" + class="js-job-erased" :user="job.erased_by" :erased-at="job.erased_at" /> @@ -91,6 +97,15 @@ <!-- EO job log --> <!--empty state --> + <empty-state + v-if="!hasTrace" + class="js-job-empty-state" + :illustration-path="emptyStateIllustration.image" + :illustration-size-class="emptyStateIllustration.size" + :title="emptyStateIllustration.title" + :content="emptyStateIllustration.content" + :action="job.status.action" + /> <!-- EO empty state --> <!-- EO Body Section --> diff --git a/app/assets/javascripts/jobs/store/getters.js b/app/assets/javascripts/jobs/store/getters.js index 62d154ff584..afe5f88b292 100644 --- a/app/assets/javascripts/jobs/store/getters.js +++ b/app/assets/javascripts/jobs/store/getters.js @@ -30,13 +30,24 @@ export const jobHasStarted = state => !(state.job.started === false); export const hasEnvironment = state => !_.isEmpty(state.job.deployment_status); /** + * Checks if it the job has trace. + * Used to check if it should render the job log or the empty state + * @returns {Boolean} + */ +export const hasTrace = state => state.job.has_trace || state.job.status.group === 'running'; + +export const emptyStateIllustration = state => + (state.job && state.job.status && state.job.status.illustration) || {}; + +/** * When the job is pending and there are no available runners * we need to render the stuck block; * * @returns {Boolean} */ export const isJobStuck = state => - state.job.status.group === 'pending' && state.job.runners && state.job.runners.available === false; + state.job.status.group === 'pending' && + (!_.isEmpty(state.job.runners) && state.job.runners.available === false); // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/serializers/detailed_status_entity.rb b/app/serializers/detailed_status_entity.rb index c772c807f76..da994d78286 100644 --- a/app/serializers/detailed_status_entity.rb +++ b/app/serializers/detailed_status_entity.rb @@ -10,7 +10,12 @@ class DetailedStatusEntity < Grape::Entity expose :illustration do |status| begin - status.illustration + illustration = { + image: ActionController::Base.helpers.image_path(status.illustration[:image]) + } + illustration = status.illustration.merge(illustration) + + illustration rescue NotImplementedError # ignored end @@ -25,5 +30,6 @@ class DetailedStatusEntity < Grape::Entity expose :action_title, as: :title expose :action_path, as: :path expose :action_method, as: :method + expose :action_button_title, as: :button_title end end diff --git a/app/views/projects/jobs/_empty_state.html.haml b/app/views/projects/jobs/_empty_state.html.haml deleted file mode 100644 index ea552c73c92..00000000000 --- a/app/views/projects/jobs/_empty_state.html.haml +++ /dev/null @@ -1,18 +0,0 @@ -- illustration = local_assigns.fetch(:illustration) -- illustration_size = local_assigns.fetch(:illustration_size) -- title = local_assigns.fetch(:title) -- content = local_assigns.fetch(:content, nil) -- action = local_assigns.fetch(:action, nil) - -.row.empty-state - .col-12 - .svg-content{ class: illustration_size } - = image_tag illustration - .col-12 - .text-content - %h4.text-center= title - - if content - %p= content - - if action - .text-center - = action diff --git a/app/views/projects/jobs/_empty_states.html.haml b/app/views/projects/jobs/_empty_states.html.haml deleted file mode 100644 index e5198d047df..00000000000 --- a/app/views/projects/jobs/_empty_states.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -- 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: 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/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml index ab7963737ca..a5f814b722d 100644 --- a/app/views/projects/jobs/show.html.haml +++ b/app/views/projects/jobs/show.html.haml @@ -42,8 +42,6 @@ = custom_icon('scroll_down') = render 'shared/builds/build_output' - - else - = render "empty_states" #js-details-block-vue{ data: { terminal_path: can?(current_user, :create_build_terminal, @build) && @build.has_terminal? ? terminal_project_job_path(@project, @build) : nil } } diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 6213cabfaad..9fe56d840e1 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -542,7 +542,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do visit project_job_path(project, job) end - it 'shows manual action empty state' do + it 'shows manual action empty state', :js 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') @@ -566,14 +566,14 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do visit project_job_path(project, job) end - it 'shows empty state' do + it 'shows empty state', :js 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 end - context 'Pending job' do + context 'Pending job', :js do let(:job) { create(:ci_build, :pending, pipeline: pipeline) } before do @@ -600,7 +600,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do end end - context 'without log' do + context 'without log', :js do let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } before do @@ -615,7 +615,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do end end - context 'Skipped job' do + context 'Skipped job', :js do let(:job) { create(:ci_build, :skipped, pipeline: pipeline) } before do @@ -629,7 +629,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do end end - context 'when job is failed but has no trace' do + context 'when job is failed but has no trace', :js do let(:job) { create(:ci_build, :failed, pipeline: pipeline) } it 'renders empty state' do diff --git a/spec/javascripts/jobs/components/empty_state_spec.js b/spec/javascripts/jobs/components/empty_state_spec.js index 872cc1e3864..73488eaab9b 100644 --- a/spec/javascripts/jobs/components/empty_state_spec.js +++ b/spec/javascripts/jobs/components/empty_state_spec.js @@ -67,7 +67,7 @@ describe('Empty State', () => { content, action: { path: 'runner', - title: 'Check runner', + button_title: 'Check runner', method: 'post', }, }); diff --git a/spec/javascripts/jobs/components/job_app_spec.js b/spec/javascripts/jobs/components/job_app_spec.js index c31fa6f9887..e02eb9723fe 100644 --- a/spec/javascripts/jobs/components/job_app_spec.js +++ b/spec/javascripts/jobs/components/job_app_spec.js @@ -37,6 +37,7 @@ describe('Job App ', () => { available: false, }, tags: ['docker'], + has_trace: true, }; const props = { @@ -182,4 +183,142 @@ describe('Job App ', () => { expect(vm.$el.querySelector('.js-job-stuck')).toBeNull(); }); }); + + describe('environments block', () => { + it('renders environment block when job has environment', () => { + store.dispatch( + 'receiveJobSuccess', + Object.assign({}, job, { + deployment_status: { + environment: { + environment_path: '/path', + name: 'foo', + }, + }, + }), + ); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + expect(vm.$el.querySelector('.js-job-environment')).not.toBeNull(); + }); + + it('does not render environment block when job has environment', () => { + store.dispatch('receiveJobSuccess', job); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + expect(vm.$el.querySelector('.js-job-environment')).toBeNull(); + }); + }); + + describe('erased block', () => { + it('renders erased block when `erased` is true', () => { + store.dispatch( + 'receiveJobSuccess', + Object.assign({}, job, { + erased: true, + erased_by: { + username: 'root', + web_url: 'gitlab.com/root', + }, + erased_at: '2016-11-07T11:11:16.525Z', + }), + ); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + expect(vm.$el.querySelector('.js-job-erased')).not.toBeNull(); + }); + + it('does not render erased block when `erased` is false', () => { + store.dispatch('receiveJobSuccess', Object.assign({}, job, { erased: false })); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + expect(vm.$el.querySelector('.js-job-erased')).toBeNull(); + }); + }); + + describe('empty states block', () => { + it('renders empty state when job does not have trace and is not running', () => { + store.dispatch( + 'receiveJobSuccess', + Object.assign({}, job, { + has_trace: false, + status: { + group: 'pending', + icon: 'status_pending', + label: 'pending', + text: 'pending', + details_path: 'path', + illustration: { + image: 'path', + size: '340', + title: 'Empty State', + content: 'This is an empty state', + }, + action: { + button_title: 'Retry job', + method: 'post', + path: '/path', + }, + }, + }), + ); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + expect(vm.$el.querySelector('.js-job-empty-state')).not.toBeNull(); + }); + + it('does not render empty state when job does not have trace but it is running', () => { + store.dispatch( + 'receiveJobSuccess', + Object.assign({}, job, { + has_trace: false, + status: { + group: 'running', + icon: 'status_running', + label: 'running', + text: 'running', + details_path: 'path', + }, + }), + ); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + expect(vm.$el.querySelector('.js-job-empty-state')).toBeNull(); + }); + + it('does not render empty state when job has trace but it is not running', () => { + store.dispatch('receiveJobSuccess', Object.assign({}, job, { has_trace: true })); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + expect(vm.$el.querySelector('.js-job-empty-state')).toBeNull(); + }); + }); }); diff --git a/spec/javascripts/jobs/store/getters_spec.js b/spec/javascripts/jobs/store/getters_spec.js index 63ef4135d83..160b2f4b34a 100644 --- a/spec/javascripts/jobs/store/getters_spec.js +++ b/spec/javascripts/jobs/store/getters_spec.js @@ -99,12 +99,14 @@ describe('Job Store Getters', () => { expect(getters.hasEnvironment(localState)).toEqual(false); }); }); + describe('with an empty object for `deployment_status`', () => { it('returns false', () => { localState.job.deployment_status = {}; expect(getters.hasEnvironment(localState)).toEqual(false); }); }); + describe('when `deployment_status` is defined and not empty', () => { it('returns true', () => { localState.job.deployment_status = { @@ -118,4 +120,94 @@ describe('Job Store Getters', () => { }); }); }); + + describe('hasTrace', () => { + describe('when has_trace is true', () => { + it('returns true', () => { + localState.job.has_trace = true; + localState.job.status = {}; + + expect(getters.hasTrace(localState)).toEqual(true); + }); + }); + + describe('when job is running', () => { + it('returns true', () => { + localState.job.has_trace = false; + localState.job.status = { group: 'running' }; + + expect(getters.hasTrace(localState)).toEqual(true); + }); + }); + + describe('when has_trace is false and job is not running', () => { + it('returns false', () => { + localState.job.has_trace = false; + localState.job.status = { group: 'pending' }; + + expect(getters.hasTrace(localState)).toEqual(false); + }); + }); + }); + + describe('emptyStateIllustration', () => { + describe('with defined illustration', () => { + it('returns the state illustration object', () => { + localState.job.status = { + illustration: { + path: 'foo', + }, + }; + + expect(getters.emptyStateIllustration(localState)).toEqual({ path: 'foo' }); + }); + }); + + describe('when illustration is not defined', () => { + it('returns an empty object', () => { + expect(getters.emptyStateIllustration(localState)).toEqual({}); + }); + }); + }); + + describe('isJobStuck', () => { + describe('when job is pending and runners are not available', () => { + it('returns true', () => { + localState.job.status = { + group: 'pending', + }; + localState.job.runners = { + available: false, + }; + + expect(getters.isJobStuck(localState)).toEqual(true); + }); + }); + + describe('when job is not pending', () => { + it('returns false', () => { + localState.job.status = { + group: 'running', + }; + localState.job.runners = { + available: false, + }; + + expect(getters.isJobStuck(localState)).toEqual(false); + }); + }); + + describe('when runners are available', () => { + it('returns false', () => { + localState.job.status = { + group: 'pending', + }; + localState.job.runners = { + available: true, + }; + + expect(getters.isJobStuck(localState)).toEqual(false); + }); + }); + }); }); |