From 217244074fbf78e2aabe1a5019ae0e9c0501e258 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 4 Oct 2016 15:59:11 +0200 Subject: [PATCH] Move MWBS trigger from build to pipeline event --- app/models/ci/pipeline.rb | 16 +++++++------- app/models/commit_status.rb | 4 ---- .../add_todo_when_build_fails_service.rb | 4 ++-- app/services/merge_requests/base_service.rb | 21 +++++++++---------- .../merge_when_build_succeeds_service.rb | 9 ++++---- 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 + def pipeline_merge_requests(pipeline) + merge_requests_for(pipeline.ref, pipeline.sha).each do |merge_request| + yield merge_request + end + end - next unless pipeline - next unless pipeline.sha == commit_status.sha - - 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