Commit 838bd54a authored by lauraMon's avatar lauraMon

Adds better error handling and refactors specs

* Applies suggestions
* Fixes rubocop formatting
parent a5953474
......@@ -3,7 +3,6 @@
module Mutations
module Ci
class Base < BaseMutation
argument :id, GraphQL::ID_TYPE,
required: true,
description: 'The id of the pipeline to mutate'
......
......@@ -8,13 +8,12 @@ module Mutations
authorize :update_pipeline
def resolve
::Ci::CancelUserPipelinesService.new.execute(current_user)
result = ::Ci::CancelUserPipelinesService.new.execute(current_user)
{
errors: []
errors: [result&.message]
}
end
end
end
end
......@@ -12,11 +12,11 @@ module Mutations
project = pipeline.project
::Ci::DestroyPipelineService.new(project, current_user).execute(pipeline)
rescue ActiveRecord::RecordNotFound
{
errors: []
errors: ['Pipeline not found']
}
end
end
end
end
......@@ -17,7 +17,6 @@ module Mutations
errors: errors_on_object(pipeline)
}
end
end
end
end
......@@ -7,6 +7,8 @@ module Ci
# https://gitlab.com/gitlab-org/gitlab/issues/32332
def execute(user)
user.pipelines.cancelable.find_each(&:cancel_running)
rescue ActiveRecord::StaleObjectError
ServiceResponse.error(message: 'Error canceling pipeline')
end
# rubocop: enable CodeReuse/ActiveRecord
end
......
......@@ -7,7 +7,7 @@ RSpec.describe 'PipelineCancel' do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, :running, project: project, user: user) }
let_it_be(:pipeline) { create(:ci_pipeline, :running, project: project, user: user) }
let(:mutation) do
graphql_mutation(:pipeline_cancel, {},
......@@ -17,10 +17,21 @@ RSpec.describe 'PipelineCancel' do
)
end
before do
let(:mutation_response) { graphql_mutation_response(:pipeline_cancel) }
before(:all) do
project.add_maintainer(user)
end
it 'reports the service-level error' do
service = double(execute: ServiceResponse.error(message: 'Error canceling pipeline'))
allow(::Ci::CancelUserPipelinesService).to receive(:new).and_return(service)
post_graphql_mutation(mutation, current_user: create(:user))
expect(mutation_response).to include('errors' => ['Error canceling pipeline'])
end
it 'does not change any pipelines not owned by the current user' do
build = create(:ci_build, :running, pipeline: pipeline)
......
......@@ -7,7 +7,7 @@ RSpec.describe 'PipelineDestroy' do
let_it_be(:project) { create(:project) }
let_it_be(:user) { project.owner }
let(:pipeline) { create(:ci_pipeline, :success, project: project, user: user) }
let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project, user: user) }
let(:mutation) do
variables = {
......
......@@ -7,7 +7,7 @@ RSpec.describe 'PipelineRetry' do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: project, user: user) }
let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) }
let(:mutation) do
variables = {
......@@ -25,7 +25,7 @@ RSpec.describe 'PipelineRetry' do
let(:mutation_response) { graphql_mutation_response(:pipeline_retry) }
before do
before(:all) do
project.add_maintainer(user)
end
......
......@@ -19,5 +19,17 @@ RSpec.describe Ci::CancelUserPipelinesService do
expect(build.reload).to be_canceled
end
end
context 'when an error ocurrs' do
it 'raises a service level error' do
service = double(execute: ServiceResponse.error(message: 'Error canceling pipeline'))
allow(::Ci::CancelUserPipelinesService).to receive(:new).and_return(service)
result = subject
expect(result).to be_a(ServiceResponse)
expect(result).to be_error
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment