Commit a7fded9b authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Huge refactoring for accepting merge requests

Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 26f5d604
...@@ -875,3 +875,6 @@ DEPENDENCIES ...@@ -875,3 +875,6 @@ DEPENDENCIES
virtus virtus
webmock (~> 1.21.0) webmock (~> 1.21.0)
wikicloth (= 0.8.1) wikicloth (= 0.8.1)
BUNDLED WITH
1.10.4
...@@ -19,7 +19,7 @@ class @MergeRequestWidget ...@@ -19,7 +19,7 @@ class @MergeRequestWidget
when 'merged' when 'merged'
location.reload() location.reload()
else else
setTimeout(merge_request_widget.mergeInProgress, 3000) setTimeout(merge_request_widget.mergeInProgress, 2000)
dataType: 'json' dataType: 'json'
getMergeStatus: -> getMergeStatus: ->
......
class Projects::MergeRequestsController < Projects::ApplicationController class Projects::MergeRequestsController < Projects::ApplicationController
before_action :module_enabled before_action :module_enabled
before_action :merge_request, only: [ before_action :merge_request, only: [
:edit, :update, :show, :diffs, :commits, :automerge, :automerge_check, :edit, :update, :show, :diffs, :commits, :merge, :merge_check,
:ci_status, :toggle_subscription :ci_status, :toggle_subscription
] ]
before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits] before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits]
...@@ -135,7 +135,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -135,7 +135,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
end end
def automerge_check def merge_check
if @merge_request.unchecked? if @merge_request.unchecked?
@merge_request.check_if_can_be_merged @merge_request.check_if_can_be_merged
end end
...@@ -145,11 +145,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -145,11 +145,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
render partial: "projects/merge_requests/widget/show.html.haml", layout: false render partial: "projects/merge_requests/widget/show.html.haml", layout: false
end end
def automerge def merge
return access_denied! unless @merge_request.can_be_merged_by?(current_user) return access_denied! unless @merge_request.can_be_merged_by?(current_user)
if @merge_request.automergeable? if @merge_request.mergeable?
AutoMergeWorker.perform_async(@merge_request.id, current_user.id, params) MergeWorker.perform_async(@merge_request.id, current_user.id, params)
@status = true @status = true
else else
@status = false @status = false
......
...@@ -41,8 +41,6 @@ class MergeRequest < ActiveRecord::Base ...@@ -41,8 +41,6 @@ class MergeRequest < ActiveRecord::Base
delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil
attr_accessor :should_remove_source_branch
# When this attribute is true some MR validation is ignored # When this attribute is true some MR validation is ignored
# It allows us to close or modify broken merge requests # It allows us to close or modify broken merge requests
attr_accessor :allow_broken attr_accessor :allow_broken
...@@ -57,7 +55,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -57,7 +55,7 @@ class MergeRequest < ActiveRecord::Base
transition [:reopened, :opened] => :closed transition [:reopened, :opened] => :closed
end end
event :merge do event :mark_as_merged do
transition [:reopened, :opened, :locked] => :merged transition [:reopened, :opened, :locked] => :merged
end end
...@@ -223,14 +221,6 @@ class MergeRequest < ActiveRecord::Base ...@@ -223,14 +221,6 @@ class MergeRequest < ActiveRecord::Base
self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
end end
def automerge!(current_user, commit_message = nil)
return unless automergeable?
MergeRequests::AutoMergeService.
new(target_project, current_user).
execute(self, commit_message)
end
def open? def open?
opened? || reopened? opened? || reopened?
end end
...@@ -239,11 +229,11 @@ class MergeRequest < ActiveRecord::Base ...@@ -239,11 +229,11 @@ class MergeRequest < ActiveRecord::Base
title =~ /\A\[?WIP\]?:? /i title =~ /\A\[?WIP\]?:? /i
end end
def automergeable? def mergeable?
open? && !work_in_progress? && can_be_merged? open? && !work_in_progress? && can_be_merged?
end end
def automerge_status def gitlab_merge_status
if work_in_progress? if work_in_progress?
"work_in_progress" "work_in_progress"
else else
...@@ -445,4 +435,13 @@ class MergeRequest < ActiveRecord::Base ...@@ -445,4 +435,13 @@ class MergeRequest < ActiveRecord::Base
"refs/merge-requests/#{id}/head" "refs/merge-requests/#{id}/head"
) )
end end
def in_locked_state
begin
lock_mr
yield
ensure
unlock_mr if locked?
end
end
end end
...@@ -70,6 +70,8 @@ class GitlabCiService < CiService ...@@ -70,6 +70,8 @@ class GitlabCiService < CiService
else else
:error :error
end end
rescue Errno::ECONNREFUSED
:error
end end
def fork_registration(new_project, private_token) def fork_registration(new_project, private_token)
...@@ -99,6 +101,8 @@ class GitlabCiService < CiService ...@@ -99,6 +101,8 @@ class GitlabCiService < CiService
if response.code == 200 and response["coverage"] if response.code == 200 and response["coverage"]
response["coverage"] response["coverage"]
end end
rescue Errno::ECONNREFUSED
nil
end end
def build_page(sha, ref) def build_page(sha, ref)
......
...@@ -31,6 +31,10 @@ class BaseService ...@@ -31,6 +31,10 @@ class BaseService
SystemHooksService.new SystemHooksService.new
end end
def repository
project.repository
end
# Add an error to the specified model for restricted visibility levels # Add an error to the specified model for restricted visibility levels
def deny_visibility_level(model, denied_visibility_level = nil) def deny_visibility_level(model, denied_visibility_level = nil)
denied_visibility_level ||= model.visibility_level denied_visibility_level ||= model.visibility_level
......
...@@ -33,15 +33,8 @@ module Files ...@@ -33,15 +33,8 @@ module Files
private private
def repository
project.repository
end
def after_commit(sha, branch) def after_commit(sha, branch)
commit = repository.commit(sha) PostCommitService.new(project, current_user).execute(sha, branch)
full_ref = 'refs/heads/' + branch
old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA
GitPushService.new.execute(project, current_user, old_sha, sha, full_ref)
end end
def current_branch def current_branch
......
module MergeRequests
# AutoMergeService class
#
# Do git merge and in case of success
# mark merge request as merged and execute all hooks and notifications
# Called when you do merge via GitLab UI
class AutoMergeService < BaseMergeService
attr_reader :merge_request, :commit_message
def execute(merge_request, commit_message)
@commit_message = commit_message
@merge_request = merge_request
merge_request.lock_mr
if merge!
merge_request.merge
create_merge_event(merge_request, current_user)
create_note(merge_request)
notification_service.merge_mr(merge_request, current_user)
execute_hooks(merge_request, 'merge')
true
else
merge_request.unlock_mr
false
end
rescue
merge_request.unlock_mr if merge_request.locked?
merge_request.mark_as_unmergeable
false
end
def merge!
if sha = commit
after_commit(sha, merge_request.target_branch)
end
end
def commit
committer = repository.user_to_comitter(current_user)
options = {
message: commit_message,
author: committer,
committer: committer
}
repository.merge(merge_request.source_sha, merge_request.target_branch, options)
end
def after_commit(sha, branch)
commit = repository.commit(sha)
full_ref = 'refs/heads/' + branch
old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA
GitPushService.new.execute(project, current_user, old_sha, sha, full_ref)
end
def repository
project.repository
end
end
end
module MergeRequests
class BaseMergeService < MergeRequests::BaseService
private
def create_merge_event(merge_request, current_user)
EventCreateService.new.merge_mr(merge_request, current_user)
end
end
end
module MergeRequests module MergeRequests
# MergeService class # MergeService class
# #
# Mark existing merge request as merged # Do git merge and in case of success
# and execute all hooks and notifications # mark merge request as merged and execute all hooks and notifications
# Called when you do merge via command line and push code # Executed when you do merge via GitLab UI
# to target branch #
class MergeService < BaseMergeService class MergeService < MergeRequests::BaseService
attr_reader :merge_request, :commit_message
def execute(merge_request, commit_message) def execute(merge_request, commit_message)
merge_request.merge @commit_message = commit_message
@merge_request = merge_request
unless @merge_request.mergeable?
return error('Merge request is not mergeable')
end
merge_request.in_locked_state do
if merge_changes
after_merge
success
else
error('Can not merge changes')
end
end
end
private
create_merge_event(merge_request, current_user) def merge_changes
create_note(merge_request) if sha = commit
notification_service.merge_mr(merge_request, current_user) after_commit(sha, merge_request.target_branch)
execute_hooks(merge_request, 'merge') end
end
def commit
committer = repository.user_to_comitter(current_user)
options = {
message: commit_message,
author: committer,
committer: committer
}
repository.merge(merge_request.source_sha, merge_request.target_branch, options)
end
def after_commit(sha, branch)
PostCommitService.new(project, current_user).execute(sha, branch)
end
true def after_merge
rescue MergeRequests::PostMergeService.new(project, current_user).execute(merge_request)
false
end end
end end
end end
module MergeRequests
# PostMergeService class
#
# Mark existing merge request as merged
# and execute all hooks and notifications
#
class PostMergeService < MergeRequests::BaseService
def execute(merge_request)
merge_request.mark_as_merged
create_merge_event(merge_request, current_user)
create_note(merge_request)
notification_service.merge_mr(merge_request, current_user)
execute_hooks(merge_request, 'merge')
end
private
def create_merge_event(merge_request, current_user)
EventCreateService.new.merge_mr(merge_request, current_user)
end
end
end
...@@ -33,9 +33,9 @@ module MergeRequests ...@@ -33,9 +33,9 @@ module MergeRequests
merge_requests.uniq.select(&:source_project).each do |merge_request| merge_requests.uniq.select(&:source_project).each do |merge_request|
MergeRequests::MergeService. MergeRequests::PostMergeService
new(merge_request.target_project, @current_user). new(merge_request.target_project, @current_user).
execute(merge_request, nil) execute(merge_request)
end end
end end
......
class PostCommitService < BaseService
def execute(sha, branch)
commit = repository.commit(sha)
full_ref = 'refs/heads/' + branch
old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA
GitPushService.new.execute(project, current_user, old_sha, sha, full_ref)
end
end
...@@ -11,10 +11,10 @@ ...@@ -11,10 +11,10 @@
var merge_request_widget; var merge_request_widget;
merge_request_widget = new MergeRequestWidget({ merge_request_widget = new MergeRequestWidget({
url_to_automerge_check: "#{automerge_check_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", url_to_automerge_check: "#{merge_check_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}",
check_enable: #{@merge_request.unchecked? ? "true" : "false"}, check_enable: #{@merge_request.unchecked? ? "true" : "false"},
url_to_ci_check: "#{ci_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", url_to_ci_check: "#{ci_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}",
ci_enable: #{@project.ci_service ? "true" : "false"}, ci_enable: #{@project.ci_service ? "true" : "false"},
current_status: "#{@merge_request.automerge_status}", current_status: "#{@merge_request.gitlab_merge_status}",
}); });
= form_for [:automerge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-requires-input' } do |f| = form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-requires-input' } do |f|
= hidden_field_tag :authenticity_token, form_authenticity_token = hidden_field_tag :authenticity_token, form_authenticity_token
.accept-merge-holder.clearfix.js-toggle-container .accept-merge-holder.clearfix.js-toggle-container
.accept-action .accept-action
......
class AutoMergeWorker class MergeWorker
include Sidekiq::Worker include Sidekiq::Worker
sidekiq_options queue: :default sidekiq_options queue: :default
...@@ -7,7 +7,13 @@ class AutoMergeWorker ...@@ -7,7 +7,13 @@ class AutoMergeWorker
params = params.with_indifferent_access params = params.with_indifferent_access
current_user = User.find(current_user_id) current_user = User.find(current_user_id)
merge_request = MergeRequest.find(merge_request_id) merge_request = MergeRequest.find(merge_request_id)
merge_request.should_remove_source_branch = params[:should_remove_source_branch]
merge_request.automerge!(current_user, params[:commit_message]) result = MergeRequests::MergeService.new(merge_request.target_project, current_user).
execute(merge_request, params[:commit_message])
if result[:status] == :success && params[:should_remove_source_branch].present?
DeleteBranchService.new(merge_request.source_project, current_user).
execute(merge_request.source_branch)
end
end end
end end
...@@ -458,8 +458,8 @@ Gitlab::Application.routes.draw do ...@@ -458,8 +458,8 @@ Gitlab::Application.routes.draw do
member do member do
get :diffs get :diffs
get :commits get :commits
post :automerge post :merge
get :automerge_check get :merge_check
get :ci_status get :ci_status
post :toggle_subscription post :toggle_subscription
end end
......
...@@ -198,7 +198,11 @@ module API ...@@ -198,7 +198,11 @@ module API
if merge_request.open? && !merge_request.work_in_progress? if merge_request.open? && !merge_request.work_in_progress?
if merge_request.can_be_merged? if merge_request.can_be_merged?
merge_request.automerge!(current_user, params[:merge_commit_message] || merge_request.merge_commit_message) commit_message = params[:merge_commit_message] || merge_request.merge_commit_message
MergeRequests::MergeService.new(merge_request.target_project, current_user).
execute(merge_request, commit_message)
present merge_request, with: Entities::MergeRequest present merge_request, with: Entities::MergeRequest
else else
render_api_error!('Branch cannot be merged', 405) render_api_error!('Branch cannot be merged', 405)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment