diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 73b5a40c7fc..8dba28b8d97 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -1,6 +1,6 @@ module Ci class PipelinePolicy < BasePolicy - delegate { pipeline.project } + delegate { @subject.project } condition(:user_cannot_update) do !::Gitlab::UserAccess diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index db12116b3ae..e487b7d5f30 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -51,19 +51,13 @@ module Ci return error('No stages / jobs for this pipeline.') end - process! + process! do + pipeline_created_counter.increment(source: source) + end end private - def triggering_user_allowed_for_ref?(trigger_request, ref) - triggering_user = current_user || trigger_request.trigger.owner - - (triggering_user && - Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) || - !project.protected_for?(ref) - end - def process! Ci::Pipeline.transaction do update_merge_requests_head_pipeline if pipeline.save @@ -75,11 +69,19 @@ module Ci cancel_pending_pipelines if project.auto_cancel_pending_pipelines? - pipeline_created_counter.increment(source: source) + yield pipeline.tap(&:process!) end + def triggering_user_allowed_for_ref?(trigger_request, ref) + triggering_user = current_user || trigger_request.trigger.owner + + (triggering_user && + Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) || + !project.protected_for?(ref) + end + def update_merge_requests_head_pipeline return unless pipeline.latest? diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index 2a8e6653eb8..9e2b0506bf3 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -110,7 +110,7 @@ describe Ci::BuildPolicy, :models do let(:branch_policy) { :no_one_can_push } it 'does not include ability to update build' do - expect(policies).to be_disallowed :update_build + expect(policy).to be_disallowed :update_build end end @@ -118,7 +118,7 @@ describe Ci::BuildPolicy, :models do let(:branch_policy) { :developers_can_push } it 'includes ability to update build' do - expect(policies).to be_allowed :update_build + expect(policy).to be_allowed :update_build end end @@ -126,7 +126,7 @@ describe Ci::BuildPolicy, :models do let(:branch_policy) { :developers_can_merge } it 'includes ability to update build' do - expect(policies).to be_allowed :update_build + expect(policy).to be_allowed :update_build end end end diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb index db09be96875..cc04230411f 100644 --- a/spec/policies/ci/pipeline_policy_spec.rb +++ b/spec/policies/ci/pipeline_policy_spec.rb @@ -4,8 +4,8 @@ describe Ci::PipelinePolicy, :models do let(:user) { create(:user) } let(:pipeline) { create(:ci_empty_pipeline, project: project) } - let(:policies) do - described_class.abilities(user, pipeline).to_set + let(:policy) do + described_class.new(user, pipeline) end describe 'rules' do @@ -23,7 +23,7 @@ describe Ci::PipelinePolicy, :models do let(:branch_policy) { :no_one_can_push } it 'does not include ability to update pipeline' do - expect(policies).to be_disallowed :update_pipeline + expect(policy).to be_disallowed :update_pipeline end end @@ -31,7 +31,7 @@ describe Ci::PipelinePolicy, :models do let(:branch_policy) { :developers_can_push } it 'includes ability to update pipeline' do - expect(policies).to be_allowed :update_pipeline + expect(policy).to be_allowed :update_pipeline end end @@ -39,7 +39,7 @@ describe Ci::PipelinePolicy, :models do let(:branch_policy) { :developers_can_merge } it 'includes ability to update pipeline' do - expect(policies).to be_allowed :update_pipeline + expect(policy).to be_allowed :update_pipeline end end end