From 93d3a4d277a368ddaaee17195af010db3624ab3c Mon Sep 17 00:00:00 2001 From: Fabio Huser Date: Thu, 5 Dec 2019 13:19:06 +0100 Subject: [PATCH 1/7] feat(pipelines): add pipeline delete button to UI --- .../pipelines/components/header_component.vue | 12 +++++ .../components/header_ci_component.vue | 3 +- .../projects/pipelines_controller.rb | 8 ++++ app/serializers/pipeline_entity.rb | 9 ++++ config/routes/project.rb | 2 +- doc/ci/pipelines.md | 8 ++++ locale/gitlab.pot | 6 +++ .../projects/pipelines_controller_spec.rb | 47 +++++++++++++++++++ .../pipelines/header_component_spec.js | 12 ++++- .../components/header_ci_component_spec.js | 27 +++++++++-- spec/serializers/pipeline_entity_spec.rb | 20 ++++++++ 11 files changed, 145 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/pipelines/components/header_component.vue b/app/assets/javascripts/pipelines/components/header_component.vue index 39afa87afc37cf..396af0bce398f2 100644 --- a/app/assets/javascripts/pipelines/components/header_component.vue +++ b/app/assets/javascripts/pipelines/components/header_component.vue @@ -73,6 +73,18 @@ export default { }); } + if (this.pipeline.delete_path) { + actions.push({ + label: __('Delete'), + path: this.pipeline.delete_path, + method: 'delete', + confirm: __('Are you sure you want to delete this pipeline?'), + cssClass: 'js-btn-delete-pipeline btn btn-danger btn-inverted', + type: 'ujs-link', + isLoading: false, + }); + } + return actions; }, }, diff --git a/app/assets/javascripts/vue_shared/components/header_ci_component.vue b/app/assets/javascripts/vue_shared/components/header_ci_component.vue index c652a684d7ce8d..a6c86548855a5b 100644 --- a/app/assets/javascripts/vue_shared/components/header_ci_component.vue +++ b/app/assets/javascripts/vue_shared/components/header_ci_component.vue @@ -131,7 +131,8 @@ export default { :key="i" :href="action.path" :class="action.cssClass" - data-method="post" + :data-method="action.method" + :data-confirm="action.confirm" rel="nofollow" > {{ action.label }} diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index e3ef8f3f2ffbf6..688281d7fd058f 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -80,6 +80,14 @@ def show end end + def destroy + ::Ci::DestroyPipelineService.new(project, current_user).execute(pipeline) + + redirect_to project_pipelines_path(project), + status: :found, + notice: _("Pipeline has been successfully deleted!") + end + def builds render_show end diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index 6b2a1bfe666c90..c9bfb646c919f8 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -29,6 +29,7 @@ class PipelineEntity < Grape::Entity expose :has_yaml_errors?, as: :yaml_errors expose :can_retry?, as: :retryable expose :can_cancel?, as: :cancelable + expose :can_delete?, as: :deleteable expose :failure_reason?, as: :failure_reason expose :detached_merge_request_pipeline?, as: :detached_merge_request_pipeline expose :merge_request_pipeline?, as: :merge_request_pipeline @@ -77,6 +78,10 @@ class PipelineEntity < Grape::Entity cancel_project_pipeline_path(pipeline.project, pipeline) end + expose :delete_path, if: -> (*) { can_delete? } do |pipeline| + project_pipeline_path(pipeline.project, pipeline) + end + expose :failed_builds, if: -> (*) { can_retry? }, using: JobEntity do |pipeline| pipeline.failed_builds end @@ -95,6 +100,10 @@ def can_cancel? pipeline.cancelable? end + def can_delete? + can?(request.current_user, :destroy_pipeline, pipeline) + end + def has_presentable_merge_request? pipeline.triggered_by_merge_request? && can?(request.current_user, :read_merge_request, pipeline.merge_request) diff --git a/config/routes/project.rb b/config/routes/project.rb index df1b6cd5032257..f339be7d0f55ae 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -343,7 +343,7 @@ draw :merge_requests end - resources :pipelines, only: [:index, :new, :create, :show] do + resources :pipelines, only: [:index, :new, :create, :show, :destroy] do collection do resource :pipelines_settings, path: 'settings', only: [:show, :update] get :charts diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index d1e50039417a7f..fc0c0bce6e092e 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -410,6 +410,14 @@ This functionality is only available: - For users with at least Developer access. - If the the stage contains [manual actions](#manual-actions-from-pipeline-graphs). +### Deleting a single pipeline + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/21365) in GitLab 12.6. + +Users with at least Owner access can delete a pipeline and all related child objects, +such as jobs and artifacts. Deleting a pipeline is done via **Delete** button on the +pipeline detail view. + ## Most Recent Pipeline > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/issues/50499) in GitLab 12.3. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8967ce8b994d07..9b638aabc8a7e0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2092,6 +2092,9 @@ msgstr "" msgid "Are you sure you want to delete this pipeline schedule?" msgstr "" +msgid "Are you sure you want to delete this pipeline?" +msgstr "" + msgid "Are you sure you want to erase this build?" msgstr "" @@ -12867,6 +12870,9 @@ msgstr "" msgid "Pipeline Schedules" msgstr "" +msgid "Pipeline has been successfully deleted!" +msgstr "" + msgid "Pipeline minutes quota" msgstr "" diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 902a84a843b7fb..d3a985f5d5f2cf 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -740,4 +740,51 @@ def get_stage_ajax(name) expect(response).to have_gitlab_http_status(404) end end + + describe 'DELETE #destroy' do + let!(:project) { create(:project, :private, :repository) } + let!(:pipeline) { create(:ci_pipeline, :failed, project: project) } + let!(:build) { create(:ci_build, :failed, pipeline: pipeline) } + + context 'when user has ability to delete pipeline' do + before do + sign_in(project.owner) + end + + it 'deletes pipeline and redirects' do + delete_pipeline + + expect(response).to have_gitlab_http_status(302) + + expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + context 'and builds are disabled' do + let(:feature) { ProjectFeature::DISABLED } + + it 'fails to delete pipeline' do + delete_pipeline + + expect(response).to have_gitlab_http_status(404) + end + end + end + + context 'when user has no privileges' do + it 'fails to delete pipeline' do + delete_pipeline + + expect(response).to have_gitlab_http_status(403) + end + end + + def delete_pipeline + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: pipeline.id + } + end + end end diff --git a/spec/javascripts/pipelines/header_component_spec.js b/spec/javascripts/pipelines/header_component_spec.js index 556a0976b293be..2e68d1aa62480f 100644 --- a/spec/javascripts/pipelines/header_component_spec.js +++ b/spec/javascripts/pipelines/header_component_spec.js @@ -34,6 +34,7 @@ describe('Pipeline details header', () => { avatar_url: 'link', }, retry_path: 'path', + delete_path: 'path', }, isLoading: false, }; @@ -55,12 +56,19 @@ describe('Pipeline details header', () => { }); describe('action buttons', () => { - it('should call postAction when button action is clicked', () => { + it('should call postAction when retry button action is clicked', () => { eventHub.$on('headerPostAction', action => { expect(action.path).toEqual('path'); }); - vm.$el.querySelector('button').click(); + vm.$el.querySelector('.js-retry-button').click(); + }); + + it('should contain delete button with proper path and method', () => { + const link = vm.$el.querySelector('.js-btn-delete-pipeline'); + + expect(link.getAttribute('href')).toContain('path'); + expect(link.getAttribute('data-method')).toContain('delete'); }); }); }); diff --git a/spec/javascripts/vue_shared/components/header_ci_component_spec.js b/spec/javascripts/vue_shared/components/header_ci_component_spec.js index 7bd5e5a64b1ab8..15dea6a8a69943 100644 --- a/spec/javascripts/vue_shared/components/header_ci_component_spec.js +++ b/spec/javascripts/vue_shared/components/header_ci_component_spec.js @@ -42,6 +42,14 @@ describe('Header CI Component', () => { cssClass: 'link', isLoading: false, }, + { + label: 'Delete', + path: 'path', + type: 'ujs-link', + cssClass: 'ujs-link', + method: 'delete', + confirm: 'Are you sure?', + }, ], hasSidebarButton: true, }; @@ -77,11 +85,20 @@ describe('Header CI Component', () => { }); it('should render provided actions', () => { - expect(vm.$el.querySelector('.btn').tagName).toEqual('BUTTON'); - expect(vm.$el.querySelector('.btn').textContent.trim()).toEqual(props.actions[0].label); - expect(vm.$el.querySelector('.link').tagName).toEqual('A'); - expect(vm.$el.querySelector('.link').textContent.trim()).toEqual(props.actions[1].label); - expect(vm.$el.querySelector('.link').getAttribute('href')).toEqual(props.actions[0].path); + const btn = vm.$el.querySelector('.btn'); + const link = vm.$el.querySelector('.link'); + const ujsLink = vm.$el.querySelector('.ujs-link'); + + expect(btn.tagName).toEqual('BUTTON'); + expect(btn.textContent.trim()).toEqual(props.actions[0].label); + expect(link.tagName).toEqual('A'); + expect(link.textContent.trim()).toEqual(props.actions[1].label); + expect(link.getAttribute('href')).toEqual(props.actions[0].path); + expect(ujsLink.tagName).toEqual('A'); + expect(ujsLink.textContent.trim()).toEqual(props.actions[2].label); + expect(ujsLink.getAttribute('href')).toEqual(props.actions[2].path); + expect(ujsLink.getAttribute('data-method')).toEqual(props.actions[2].method); + expect(ujsLink.getAttribute('data-confirm')).toEqual(props.actions[2].confirm); }); it('should show loading icon', done => { diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index d95aaf3d104539..3a7104c029321c 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -123,6 +123,26 @@ end end + context 'when pipeline is deleteable' do + context 'user has ability to delete pipeline' do + let(:project) { create(:project, namespace: user.namespace) } + let(:pipeline) { create(:ci_pipeline, project: project) } + + it 'contains cancel path' do + expect(subject[:delete_path]).to be_present + end + end + + context 'user does not have ability to delete pipeline' do + let(:project) { create(:project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + + it 'does not contain cancel path' do + expect(subject).not_to have_key(:delete_path) + end + end + end + context 'when pipeline ref is empty' do let(:pipeline) { create(:ci_empty_pipeline) } -- GitLab From 52716b396f19d512be4ff6a3b3e3ebb9c2f24dc0 Mon Sep 17 00:00:00 2001 From: Fabio Huser Date: Tue, 17 Dec 2019 16:47:35 +0100 Subject: [PATCH 2/7] refactor(pipelines): rework pipeline deletion to use AJAX --- .../pipelines/components/header_component.vue | 32 +++++++++++++++--- .../javascripts/pipelines/mixins/pipelines.js | 12 ++++++- .../pipelines/pipeline_details_bundle.js | 12 +++++++ .../pipelines/pipeline_details_mediator.js | 4 +++ .../pipelines/services/pipeline_service.js | 5 +++ .../components/header_ci_component.vue | 33 +++++++------------ .../projects/pipelines_controller.rb | 4 +-- locale/gitlab.pot | 14 +++++--- .../projects/pipelines_controller_spec.rb | 2 +- .../pipelines/header_component_spec.js | 9 ++--- .../components/header_ci_component_spec.js | 27 ++++----------- 11 files changed, 95 insertions(+), 59 deletions(-) diff --git a/app/assets/javascripts/pipelines/components/header_component.vue b/app/assets/javascripts/pipelines/components/header_component.vue index 396af0bce398f2..10fdae3619f887 100644 --- a/app/assets/javascripts/pipelines/components/header_component.vue +++ b/app/assets/javascripts/pipelines/components/header_component.vue @@ -1,5 +1,5 @@