diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index a51c52b3f91a47aaa6b9a433b1c49c9a3b069cf0..b3dbb5484543f5f59de298b74472231f2a5d1fd0 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -23,7 +23,7 @@ module Ci return error('Insufficient permissions to create a new pipeline') end - unless trigger_request && trigger_request.trigger.owner + if trigger_request && !trigger_request.trigger.owner return error('Legacy trigger without a owner is not allowed') end diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index beb27a5a5974a7b3e52be46d6f9d0dfba10cf886..e4f55c27f617824ec984c83856e98fcd0a684d8d 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -6,7 +6,8 @@ module Ci pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref). execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request) - trigger_request if pipeline.persisted? + trigger_request.pipeline = pipeline + trigger_request end end end diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index a9f2ca2608ea526c3b395fa96daaaee5b9e53c01..9e444563fdf13d5a8aa616cfc6db6c50a868a854 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -28,11 +28,12 @@ module API # create request and trigger builds trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables) - if trigger_request - present trigger_request.pipeline, with: Entities::Pipeline + pipeline = trigger_request.pipeline + + if pipeline.persisted? + present pipeline, with: Entities::Pipeline else - errors = 'No pipeline created' - render_api_error!(errors, 400) + render_validation_error!(pipeline) end end diff --git a/lib/api/v3/triggers.rb b/lib/api/v3/triggers.rb index a23d6b6b48cb1546c7a6dd696c7cc53a00a03e46..7e75c579528fa073e1a93ee5b2fc771a1eebad0d 100644 --- a/lib/api/v3/triggers.rb +++ b/lib/api/v3/triggers.rb @@ -29,11 +29,12 @@ module API # create request and trigger builds trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables) - if trigger_request + pipeline = trigger_request.pipeline + + if pipeline.persisted? present trigger_request, with: ::API::V3::Entities::TriggerRequest else - errors = 'No builds created' - render_api_error!(errors, 400) + render_validation_error!(pipeline) end end diff --git a/lib/ci/api/triggers.rb b/lib/ci/api/triggers.rb index 6e622601680ff02f772d9e36c454e012d8783560..0e5174e13ab486231429da8765f4b63528a78755 100644 --- a/lib/ci/api/triggers.rb +++ b/lib/ci/api/triggers.rb @@ -25,11 +25,12 @@ module Ci # create request and trigger builds trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref], variables) - if trigger_request + pipeline = trigger_request.pipeline + + if pipeline.persisted? present trigger_request, with: Entities::TriggerRequest else - errors = 'No builds created' - render_api_error!(errors, 400) + render_validation_error!(pipeline) end end end diff --git a/spec/requests/ci/api/triggers_spec.rb b/spec/requests/ci/api/triggers_spec.rb index 26b03c0f14820ed5cff4cd5697f1cd625edbe629..e481ca916aba87b076aa916150a1286c279579ef 100644 --- a/spec/requests/ci/api/triggers_spec.rb +++ b/spec/requests/ci/api/triggers_spec.rb @@ -5,7 +5,14 @@ describe Ci::API::Triggers do let!(:trigger_token) { 'secure token' } let!(:project) { create(:project, :repository, ci_id: 10) } let!(:project2) { create(:empty_project, ci_id: 11) } - let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } + + let!(:trigger) do + create(:ci_trigger, + project: project, + token: trigger_token, + owner: create(:user)) + end + let(:options) do { token: trigger_token @@ -14,6 +21,8 @@ describe Ci::API::Triggers do before do stub_ci_pipeline_to_return_yaml_file + + project.add_developer(trigger.owner) end context 'Handles errors' do @@ -47,7 +56,8 @@ describe Ci::API::Triggers do it 'returns bad request with no builds created if there\'s no commit for that ref' do post ci_api("/projects/#{project.ci_id}/refs/other-branch/trigger"), options expect(response).to have_http_status(400) - expect(json_response['message']).to eq('No builds created') + expect(json_response['message']['base']) + .to contain_exactly('Reference not found') end context 'Validates variables' do diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index f2956262f4b0d3fc8ceef2b99ecef1eb37340d59..8582c74e734c1711ba343aa35cf7970d0fab4ada 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -3,10 +3,13 @@ require 'spec_helper' describe Ci::CreateTriggerRequestService, services: true do let(:service) { described_class.new } let(:project) { create(:project, :repository) } - let(:trigger) { create(:ci_trigger, project: project) } + let(:trigger) { create(:ci_trigger, project: project, owner: owner) } + let(:owner) { create(:user) } before do stub_ci_pipeline_to_return_yaml_file + + project.add_developer(owner) end describe '#execute' do @@ -21,9 +24,6 @@ describe Ci::CreateTriggerRequestService, services: true do end context 'with owner' do - let(:owner) { create(:user) } - let(:trigger) { create(:ci_trigger, project: project, owner: owner) } - it { expect(subject).to be_kind_of(Ci::TriggerRequest) } it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } it { expect(subject.pipeline).to be_trigger } @@ -36,7 +36,7 @@ describe Ci::CreateTriggerRequestService, services: true do context 'no commit for ref' do subject { service.execute(project, trigger, 'other-branch') } - it { expect(subject).to be_nil } + it { expect(subject.pipeline).not_to be_persisted } end context 'no builds created' do @@ -46,7 +46,7 @@ describe Ci::CreateTriggerRequestService, services: true do stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }') end - it { expect(subject).to be_nil } + it { expect(subject.pipeline).not_to be_persisted } end end end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index f4bc63bcc6a1c92dd22e2198ffa73acd7407408c..7da48647bb55fe0d1a7318c1b59ce00b77f6f0a2 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -82,6 +82,7 @@ describe PostReceive do OpenStruct.new(id: '123456') end allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true) + allow_any_instance_of(Repository).to receive(:ref_exists?).and_return(true) stub_ci_pipeline_to_return_yaml_file end