Destroy pipeline in service

Move all logic for destroying a Pipeline into a service so it's easily
reusable.

5 files changed, 93 insertions, 16 deletions

diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 1c082945299..221826121da 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -144,6 +144,7 @@ class ProjectPolicy < BasePolicy
     enable :destroy_merge_request
     enable :destroy_issue
     enable :remove_pages
+    enable :destroy_pipeline
 
     enable :set_issue_iid
     enable :set_issue_created_at
diff --git a/app/services/ci/destroy_pipeline_service.rb b/app/services/ci/destroy_pipeline_service.rb
new file mode 100644
index 00000000000..059e871f20e
--- /dev/null
+++ b/app/services/ci/destroy_pipeline_service.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module Ci
+  class DestroyPipelineService < BaseService
+    def execute(pipeline)
+      return false unless can?(current_user, :destroy_pipeline, project)
+
+      AuditEventService.new(current_user, pipeline).security_event
+
+      pipeline.destroy
+    end
+  end
+end
diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb
index d4bd4e71343..39d693bb9e9 100644
--- a/lib/api/pipelines.rb
+++ b/lib/api/pipelines.rb
@@ -89,11 +89,11 @@ module API
         requires :pipeline_id, type: Integer, desc: 'The pipeline ID'
       end
       delete ':id/pipelines/:pipeline_id' do
-        authorize! :admin_pipeline, user_project
+        authorize! :destroy_pipeline, user_project
 
-        AuditEventService.new(current_user, user_project).security_event
-
-        destroy_conditionally!(pipeline)
+        destroy_conditionally!(pipeline) do
+          ::Ci::DestroyPipelineService.new(user_project, current_user).execute(pipeline)
+        end
       end
 
       desc 'Retry builds in the pipeline' do
diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb
index 68de3068568..e786b7531a9 100644
--- a/spec/requests/api/pipelines_spec.rb
+++ b/spec/requests/api/pipelines_spec.rb
@@ -440,34 +440,31 @@ describe API::Pipelines do
 
   describe 'DELETE /projects/:id/pipelines/:pipeline_id' do
     context 'authorized user' do
-      it 'deletes the pipeline' do
-        delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", user)
+      let(:owner) { project.owner }
+
+      it 'destroys the pipeline' do
+        delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner)
 
         expect(response).to have_gitlab_http_status(204)
         expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound)
       end
 
       it 'returns 404 when it does not exist' do
-        delete api("/projects/#{project.id}/pipelines/123456", user)
+        delete api("/projects/#{project.id}/pipelines/123456", owner)
 
         expect(response).to have_gitlab_http_status(404)
         expect(json_response['message']).to eq '404 Not found'
       end
 
       it 'logs an audit event' do
-        expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) }.to change { SecurityEvent.count }.by(1)
+        expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.to change { SecurityEvent.count }.by(1)
       end
 
       context 'when the pipeline has jobs' do
-        let!(:pipeline) do
-          create(:ci_pipeline, project: project, sha: project.commit.id,
-                               ref: project.default_branch, user: user)
-        end
-
         let!(:build) { create(:ci_build, project: project, pipeline: pipeline) }
 
-        it 'deletes associated jobs' do
-          delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", user)
+        it 'destroys associated jobs' do
+          delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner)
 
           expect(response).to have_gitlab_http_status(204)
           expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound)
@@ -476,7 +473,7 @@ describe API::Pipelines do
     end
 
     context 'unauthorized user' do
-      it 'should not return a project pipeline' do
+      it 'should return a 404' do
         get api("/projects/#{project.id}/pipelines/#{pipeline.id}", non_member)
 
         expect(response).to have_gitlab_http_status(404)
diff --git a/spec/services/ci/destroy_pipeline_service_spec.rb b/spec/services/ci/destroy_pipeline_service_spec.rb
new file mode 100644
index 00000000000..9f449dd73e8
--- /dev/null
+++ b/spec/services/ci/destroy_pipeline_service_spec.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Ci::DestroyPipelineService do
+  let(:project) { create(:project) }
+  let!(:pipeline) { create(:ci_pipeline, project: project) }
+
+  subject { described_class.new(project, user).execute(pipeline) }
+
+  context 'user is owner' do
+    let(:user) { project.owner }
+
+    it 'destroys the pipeline' do
+      subject
+
+      expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    it 'logs an audit event' do
+      expect { subject }.to change { SecurityEvent.count }.by(1)
+    end
+
+    context 'when the pipeline has jobs' do
+      let!(:build) { create(:ci_build, project: project, pipeline: pipeline) }
+
+      it 'destroys associated jobs' do
+        subject
+
+        expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it 'destroys associated stages' do
+        stages = pipeline.stages
+
+        subject
+
+        expect(stages).to all(raise_error(ActiveRecord::RecordNotFound))
+      end
+
+      context 'when job has artifacts' do
+        let!(:artifact) { create(:ci_job_artifact, :archive, job: build) }
+
+        it 'destroys associated artifacts' do
+          subject
+
+          expect { artifact.reload }.to raise_error(ActiveRecord::RecordNotFound)
+        end
+      end
+    end
+  end
+
+  context 'user is not owner' do
+    let(:user) { create(:user) }
+
+    it 'returns false' do
+      is_expected.to eq(false)
+    end
+
+    it 'does not destroy the pipeline' do
+      subject
+
+      expect { pipeline.reload }.not_to raise_error
+    end
+  end
+end