Create Backend Services for Cancel and Unschedule job actions

app/controllers/projects/jobs_controller.rb

@@ -9,7 +9,7 @@ class Projects::JobsController < Projects::ApplicationController
   before_action :authorize_read_build_trace!, only: [:trace, :raw]
   before_action :authorize_read_build!
   before_action :authorize_update_build!,
-    except: [:index, :show, :status, :raw, :trace, :erase]
+    except: [:index, :show, :status, :raw, :trace, :erase, :cancel, :unschedule]
   before_action :authorize_erase_build!, only: [:erase]
   before_action :authorize_use_build_terminal!, only: [:terminal, :terminal_websocket_authorize]
   before_action :verify_api_request!, only: :terminal_websocket_authorize
@@ -93,22 +93,28 @@ class Projects::JobsController < Projects::ApplicationController
   end
 
   def cancel
-    return respond_422 unless @build.cancelable?
+    service_response = Ci::BuildCancelService.new(@build, current_user).execute
 
-    @build.cancel
-
-    if continue_params[:to]
-      redirect_to continue_params[:to]
+    if service_response.success?
+      destination = continue_params[:to].presence || builds_project_pipeline_path(@project, @build.pipeline.id)
+      redirect_to destination
+    elsif service_response.http_status == :forbidden
+      access_denied!
     else
-      redirect_to builds_project_pipeline_path(@project, @build.pipeline.id)
+      head service_response.http_status
     end
   end
 
   def unschedule
-    return respond_422 unless @build.scheduled?
+    service_response = Ci::BuildUnscheduleService.new(@build, current_user).execute
 
-    @build.unschedule!
-    redirect_to build_path(@build)
+    if service_response.success?
+      redirect_to build_path(@build)
+    elsif service_response.http_status == :forbidden
+      access_denied!
+    else
+      head service_response.http_status
+    end
   end
 
   def status

app/services/ci/build_cancel_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class BuildCancelService
+    def initialize(build, user)
+      @build = build
+      @user = user
+    end
+
+    def execute
+      return forbidden unless allowed?
+      return unprocessable_entity unless build.cancelable?
+
+      build.cancel
+
+      ServiceResponse.success(payload: build)
+    end
+
+    private
+
+    attr_reader :build, :user
+
+    def allowed?
+      user.can?(:update_build, build)
+    end
+
+    def forbidden
+      ServiceResponse.error(message: 'Forbidden', http_status: :forbidden)
+    end
+
+    def unprocessable_entity
+      ServiceResponse.error(message: 'Unprocessable entity', http_status: :unprocessable_entity)
+    end
+  end
+end

app/services/ci/build_unschedule_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class BuildUnscheduleService
+    def initialize(build, user)
+      @build = build
+      @user = user
+    end
+
+    def execute
+      return forbidden unless allowed?
+      return unprocessable_entity unless build.scheduled?
+
+      build.unschedule!
+
+      ServiceResponse.success(payload: build)
+    end
+
+    private
+
+    attr_reader :build, :user
+
+    def allowed?
+      user.can?(:update_build, build)
+    end
+
+    def forbidden
+      ServiceResponse.error(message: 'Forbidden', http_status: :forbidden)
+    end
+
+    def unprocessable_entity
+      ServiceResponse.error(message: 'Unprocessable entity', http_status: :unprocessable_entity)
+    end
+  end
+end

spec/controllers/projects/jobs_controller_spec.rb

@@ -868,64 +868,85 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
   end
 
   describe 'POST cancel' do
-    before do
-      project.add_developer(user)
-      sign_in(user)
-    end
+    context 'when user is authorized to cancel the build' do
+      before do
+        project.add_developer(user)
+        sign_in(user)
+      end
 
-    context 'when continue url is present' do
-      let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) }
+      context 'when continue url is present' do
+        let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) }
 
-      context 'when continue to is a safe url' do
-        let(:url) { '/test' }
+        context 'when continue to is a safe url' do
+          let(:url) { '/test' }
 
-        before do
-          post_cancel(continue: { to: url })
-        end
+          before do
+            post_cancel(continue: { to: url })
+          end
 
-        it 'redirects to the continue url' do
-          expect(response).to have_gitlab_http_status(:found)
-          expect(response).to redirect_to(url)
+          it 'redirects to the continue url' do
+            expect(response).to have_gitlab_http_status(:found)
+            expect(response).to redirect_to(url)
+          end
+
+          it 'transits to canceled' do
+            expect(job.reload).to be_canceled
+          end
         end
 
-        it 'transits to canceled' do
-          expect(job.reload).to be_canceled
+        context 'when continue to is not a safe url' do
+          let(:url) { 'http://example.com' }
+
+          it 'raises an error' do
+            expect { cancel_with_redirect(url) }.to raise_error
+          end
         end
       end
 
-      context 'when continue to is not a safe url' do
-        let(:url) { 'http://example.com' }
+      context 'when continue url is not present' do
+        before do
+          post_cancel
+        end
+
+        context 'when job is cancelable' do
+          let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) }
+
+          it 'redirects to the builds page' do
+            expect(response).to have_gitlab_http_status(:found)
+            expect(response).to redirect_to(builds_namespace_project_pipeline_path(id: pipeline.id))
+          end
+
+          it 'transits to canceled' do
+            expect(job.reload).to be_canceled
+          end
+        end
+
+        context 'when job is not cancelable' do
+          let(:job) { create(:ci_build, :canceled, pipeline: pipeline) }
 
-        it 'raises an error' do
-          expect { cancel_with_redirect(url) }.to raise_error
+          it 'returns unprocessable_entity' do
+            expect(response).to have_gitlab_http_status(:unprocessable_entity)
+          end
         end
       end
     end
 
-    context 'when continue url is not present' do
+    context 'when user is not authorized to cancel the build' do
+      let!(:job) { create(:ci_build, :cancelable, pipeline: pipeline) }
+
       before do
+        project.add_reporter(user)
+        sign_in(user)
+
         post_cancel
       end
 
-      context 'when job is cancelable' do
-        let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) }
-
-        it 'redirects to the builds page' do
-          expect(response).to have_gitlab_http_status(:found)
-          expect(response).to redirect_to(builds_namespace_project_pipeline_path(id: pipeline.id))
-        end
-
-        it 'transits to canceled' do
-          expect(job.reload).to be_canceled
-        end
+      it 'responds with not_found' do
+        expect(response).to have_gitlab_http_status(:not_found)
       end
 
-      context 'when job is not cancelable' do
-        let(:job) { create(:ci_build, :canceled, pipeline: pipeline) }
-
-        it 'returns unprocessable_entity' do
-          expect(response).to have_gitlab_http_status(:unprocessable_entity)
-        end
+      it 'does not transit to canceled' do
+        expect(job.reload).not_to be_canceled
       end
     end
 
@@ -938,43 +959,60 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
 
   describe 'POST unschedule' do
     before do
-      project.add_developer(user)
+      create(:protected_branch, :developers_can_merge, name: 'master', project: project)
+    end
 
-      create(:protected_branch, :developers_can_merge,
-             name: 'master', project: project)
+    context 'when user is authorized to unschedule the build' do
+      before do
+        project.add_developer(user)
+        sign_in(user)
 
-      sign_in(user)
+        post_unschedule
+      end
 
-      post_unschedule
-    end
+      context 'when job is scheduled' do
+        let(:job) { create(:ci_build, :scheduled, pipeline: pipeline) }
 
-    context 'when job is scheduled' do
-      let(:job) { create(:ci_build, :scheduled, pipeline: pipeline) }
+        it 'redirects to the unscheduled job page' do
+          expect(response).to have_gitlab_http_status(:found)
+          expect(response).to redirect_to(namespace_project_job_path(id: job.id))
+        end
 
-      it 'redirects to the unscheduled job page' do
-        expect(response).to have_gitlab_http_status(:found)
-        expect(response).to redirect_to(namespace_project_job_path(id: job.id))
+        it 'transits to manual' do
+          expect(job.reload).to be_manual
+        end
       end
 
-      it 'transits to manual' do
-        expect(job.reload).to be_manual
+      context 'when job is not scheduled' do
+        let(:job) { create(:ci_build, pipeline: pipeline) }
+
+        it 'renders unprocessable_entity' do
+          expect(response).to have_gitlab_http_status(:unprocessable_entity)
+        end
       end
     end
 
-    context 'when job is not scheduled' do
-      let(:job) { create(:ci_build, pipeline: pipeline) }
+    context 'when user is not authorized to unschedule the build' do
+      let(:job) { create(:ci_build, :scheduled, pipeline: pipeline) }
 
-      it 'renders unprocessable_entity' do
-        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+      before do
+        project.add_reporter(user)
+        sign_in(user)
+
+        post_unschedule
+      end
+
+      it 'responds with not_found' do
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+
+      it 'does not transit to scheduled' do
+        expect(job.reload).not_to be_manual
       end
     end
 
     def post_unschedule
-      post :unschedule, params: {
-                          namespace_id: project.namespace,
-                          project_id: project,
-                          id: job.id
-                        }
+      post :unschedule, params: { namespace_id: project.namespace, project_id: project, id: job.id }
     end
   end
 

spec/services/ci/build_cancel_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::BuildCancelService do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+
+  describe '#execute' do
+    subject(:execute) { described_class.new(build, user).execute }
+
+    context 'when user is authorized to cancel the build' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      context 'when build is cancelable' do
+        let!(:build) { create(:ci_build, :cancelable, pipeline: pipeline) }
+
+        it 'transits build to canceled', :aggregate_failures do
+          response = execute
+
+          expect(response).to be_success
+          expect(response.payload.reload).to be_canceled
+        end
+      end
+
+      context 'when build is not cancelable' do
+        let!(:build) { create(:ci_build, :canceled, pipeline: pipeline) }
+
+        it 'responds with unprocessable entity', :aggregate_failures do
+          response = execute
+
+          expect(response).to be_error
+          expect(response.http_status).to eq(:unprocessable_entity)
+        end
+      end
+    end
+
+    context 'when user is not authorized to cancel the build' do
+      let!(:build) { create(:ci_build, :cancelable, pipeline: pipeline) }
+
+      it 'responds with forbidden', :aggregate_failures do
+        response = execute
+
+        expect(response).to be_error
+        expect(response.http_status).to eq(:forbidden)
+      end
+    end
+  end
+end

spec/services/ci/build_unschedule_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::BuildUnscheduleService do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+
+  describe '#execute' do
+    subject(:execute) { described_class.new(build, user).execute }
+
+    context 'when user is authorized to unschedule the build' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      context 'when build is scheduled' do
+        let!(:build) { create(:ci_build, :scheduled, pipeline: pipeline) }
+
+        it 'transits build to manual' do
+          response = execute
+
+          expect(response).to be_success
+          expect(response.payload.reload).to be_manual
+        end
+      end
+
+      context 'when build is not scheduled' do
+        let!(:build) { create(:ci_build, pipeline: pipeline) }
+
+        it 'responds with unprocessable entity', :aggregate_failures do
+          response = execute
+
+          expect(response).to be_error
+          expect(response.http_status).to eq(:unprocessable_entity)
+        end
+      end
+    end
+
+    context 'when user is not authorized to unschedule the build' do
+      let!(:build) { create(:ci_build, :scheduled, pipeline: pipeline) }
+
+      it 'responds with forbidden', :aggregate_failures do
+        response = execute
+
+        expect(response).to be_error
+        expect(response.http_status).to eq(:forbidden)
+      end
+    end
+  end
+end