Move MWBS trigger from build to pipeline event

5 files changed, 26 insertions, 28 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 2cf9892edc5..040ba20eb09 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -56,6 +56,10 @@ module Ci
         pipeline.finished_at = Time.now
       end
 
+      before_transition do |pipeline|
+        pipeline.update_duration
+      end
+
       after_transition [:created, :pending] => :running do |pipeline|
         MergeRequest::Metrics.where(merge_request_id: pipeline.merge_requests.map(&:id)).
           update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: nil)
@@ -66,8 +70,8 @@ module Ci
           update_all(latest_build_finished_at: pipeline.finished_at)
       end
 
-      before_transition do |pipeline|
-        pipeline.update_duration
+      after_transition [:created, :pending, :running] => :success do |pipeline|
+        MergeRequests::MergeWhenBuildSucceedsService.new(pipeline.project, nil).trigger(pipeline)
       end
 
       after_transition do |pipeline, transition|
@@ -292,11 +296,9 @@ module Ci
     # Merge requests for which the current pipeline is running against
     # the merge request's latest commit.
     def merge_requests
-      @merge_requests ||=
-        begin
-          project.merge_requests.where(source_branch: self.ref).
-            select { |merge_request| merge_request.pipeline.try(:id) == self.id }
-        end
+      @merge_requests ||= project.merge_requests
+        .where(source_branch: self.ref)
+        .select { |merge_request| merge_request.pipeline.try(:id) == self.id }
     end
 
     private
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 9fa8d17e74e..5a240bcf2c6 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -98,10 +98,6 @@ class CommitStatus < ActiveRecord::Base
       true
     end
 
-    after_transition [:created, :pending, :running] => :success do |commit_status|
-      MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.pipeline.project, nil).trigger(commit_status)
-    end
-
     after_transition any => :failed do |commit_status|
       MergeRequests::AddTodoWhenBuildFailsService.new(commit_status.pipeline.project, nil).execute(commit_status)
     end
diff --git a/app/services/merge_requests/add_todo_when_build_fails_service.rb b/app/services/merge_requests/add_todo_when_build_fails_service.rb
index 566049525cb..d572a928a42 100644
--- a/app/services/merge_requests/add_todo_when_build_fails_service.rb
+++ b/app/services/merge_requests/add_todo_when_build_fails_service.rb
@@ -2,14 +2,14 @@ module MergeRequests
   class AddTodoWhenBuildFailsService < MergeRequests::BaseService
     # Adds a todo to the parent merge_request when a CI build fails
     def execute(commit_status)
-      each_merge_request(commit_status) do |merge_request|
+      commit_status_merge_requests(commit_status) do |merge_request|
         todo_service.merge_request_build_failed(merge_request)
       end
     end
 
     # Closes any pending build failed todos for the parent MRs when a build is retried
     def close(commit_status)
-      each_merge_request(commit_status) do |merge_request|
+      commit_status_merge_requests(commit_status) do |merge_request|
         todo_service.merge_request_build_retried(merge_request)
       end
     end
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index d0d155b7ee1..a726dd98697 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -42,11 +42,9 @@ module MergeRequests
       super(:merge_request)
     end
 
-    def merge_request_from(commit_status)
-      branches = commit_status.ref
-
+    def merge_requests_for(branches, sha)
       # This is for ref-less builds
-      branches ||= @project.repository.branch_names_contains(commit_status.sha)
+      branches ||= @project.repository.branch_names_contains(sha)
 
       return [] if branches.blank?
 
@@ -56,14 +54,15 @@ module MergeRequests
       merge_requests.uniq.select(&:source_project)
     end
 
-    def each_merge_request(commit_status)
-      merge_request_from(commit_status).each do |merge_request|
-        pipeline = merge_request.pipeline
-
-        next unless pipeline
-        next unless pipeline.sha == commit_status.sha
+    def pipeline_merge_requests(pipeline)
+      merge_requests_for(pipeline.ref, pipeline.sha).each do |merge_request|
+        yield merge_request
+      end
+    end
 
-        yield merge_request, pipeline
+    def commit_status_merge_requests(commit_status)
+      merge_requests_for(commit_status.ref, commit_status.sha).each do |merge_request|
+        yield merge_request
       end
     end
   end
diff --git a/app/services/merge_requests/merge_when_build_succeeds_service.rb b/app/services/merge_requests/merge_when_build_succeeds_service.rb
index 4ad5fb08311..dc159de0058 100644
--- a/app/services/merge_requests/merge_when_build_succeeds_service.rb
+++ b/app/services/merge_requests/merge_when_build_succeeds_service.rb
@@ -18,12 +18,13 @@ module MergeRequests
       merge_request.save
     end
 
-    # Triggers the automatic merge of merge_request once the build succeeds
-    def trigger(commit_status)
-      each_merge_request(commit_status) do |merge_request, pipeline|
+    # Triggers the automatic merge of merge_request once the pipeline succeeds
+    def trigger(pipeline)
+      return unless pipeline.success?
+
+      pipeline_merge_requests(pipeline) do |merge_request|
         next unless merge_request.merge_when_build_succeeds?
         next unless merge_request.mergeable?
-        next unless pipeline.success?
 
         MergeWorker.perform_async(merge_request.id, merge_request.merge_user_id, merge_request.merge_params)
       end