Merge allowed_to_create? into CreatePipelineService

4 files changed, 22 insertions, 31 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 7963386bdb1..8d1beca9771 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -164,20 +164,6 @@ module Ci
       where.not(duration: nil).sum(:duration)
     end
 
-    def self.allowed_to_create?(user, project, ref)
-      repo = project.repository
-      access = Gitlab::UserAccess.new(user, project: project)
-
-      Ability.allowed?(user, :create_pipeline, project) &&
-        if repo.ref_exists?("#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref}")
-          access.can_push_or_merge_to_branch?(ref)
-        elsif repo.ref_exists?("#{Gitlab::Git::TAG_REF_PREFIX}#{ref}")
-          access.can_create_tag?(ref)
-        else
-          false
-        end
-    end
-
     def self.internal_sources
       sources.reject { |source| source == "external" }.values
     end
diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb
index eaca2774bf9..49455e79c15 100644
--- a/app/models/ci/pipeline_schedule.rb
+++ b/app/models/ci/pipeline_schedule.rb
@@ -36,10 +36,6 @@ module Ci
       update_attribute(:active, false)
     end
 
-    def runnable_by_owner?
-      Ci::Pipeline.allowed_to_create?(owner, project, ref)
-    end
-
     def set_next_run_at
       self.next_run_at = Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now)
     end
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index e487b7d5f30..485161e5f3f 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -27,7 +27,7 @@ module Ci
         return error('Reference not found')
       end
 
-      unless triggering_user_allowed_for_ref?(trigger_request, ref)
+      unless triggering_user_allowed_for_ref?(trigger_request)
         return error("Insufficient permissions for protected #{ref}")
       end
 
@@ -74,14 +74,26 @@ module Ci
       pipeline.tap(&:process!)
     end
 
-    def triggering_user_allowed_for_ref?(trigger_request, ref)
+    def triggering_user_allowed_for_ref?(trigger_request)
       triggering_user = current_user || trigger_request.trigger.owner
 
-      (triggering_user &&
-        Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) ||
+      (triggering_user && allowed_to_create?(triggering_user)) ||
         !project.protected_for?(ref)
     end
 
+    def allowed_to_create?(triggering_user)
+      access = Gitlab::UserAccess.new(triggering_user, project: project)
+
+      Ability.allowed?(triggering_user, :create_pipeline, project) &&
+        if branch?
+          access.can_push_or_merge_to_branch?(ref)
+        elsif tag?
+          access.can_create_tag?(ref)
+        else
+          false
+        end
+    end
+
     def update_merge_requests_head_pipeline
       return unless pipeline.latest?
 
@@ -145,7 +157,7 @@ module Ci
     end
 
     def ref
-      Gitlab::Git.ref_name(origin_ref)
+      @ref ||= Gitlab::Git.ref_name(origin_ref)
     end
 
     def valid_sha?
diff --git a/app/workers/pipeline_schedule_worker.rb b/app/workers/pipeline_schedule_worker.rb
index 7b485b3363c..d7087f20dfc 100644
--- a/app/workers/pipeline_schedule_worker.rb
+++ b/app/workers/pipeline_schedule_worker.rb
@@ -6,15 +6,12 @@ class PipelineScheduleWorker
     Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now)
       .preload(:owner, :project).find_each do |schedule|
       begin
-        unless schedule.runnable_by_owner?
-          schedule.deactivate!
-          next
-        end
-
-        Ci::CreatePipelineService.new(schedule.project,
-                                      schedule.owner,
-                                      ref: schedule.ref)
+        pipeline = Ci::CreatePipelineService.new(schedule.project,
+                                                 schedule.owner,
+                                                 ref: schedule.ref)
           .execute(:schedule, save_on_errors: false, schedule: schedule)
+
+        schedule.deactivate! unless pipeline.persisted?
       rescue => e
         Rails.logger.error "#{schedule.id}: Failed to create a scheduled pipeline: #{e.message}"
       ensure