Commit 72c92942 authored by Bob Van Landuyt's avatar Bob Van Landuyt Committed by Nick Thomas

Set the SHA to be merged from push options

When we set `merge_when_pipeline_succeeds` in the push options, we
also need to provide the sha that we'll be merging.

We take the cha from the changes list that was passed into the push
options, and pass it onwards to the Create or Update service.
parent 7ae78719
...@@ -4,14 +4,14 @@ module MergeRequests ...@@ -4,14 +4,14 @@ module MergeRequests
class PushOptionsHandlerService class PushOptionsHandlerService
LIMIT = 10 LIMIT = 10
attr_reader :branches, :changes_by_branch, :current_user, :errors, attr_reader :current_user, :errors, :changes,
:project, :push_options, :target_project :project, :push_options, :target_project
def initialize(project, current_user, changes, push_options) def initialize(project, current_user, changes, push_options)
@project = project @project = project
@target_project = @project.default_merge_request_target @target_project = @project.default_merge_request_target
@current_user = current_user @current_user = current_user
@branches = get_branches(changes) @changes = Gitlab::ChangesList.new(changes)
@push_options = push_options @push_options = push_options
@errors = [] @errors = []
end end
...@@ -34,8 +34,12 @@ module MergeRequests ...@@ -34,8 +34,12 @@ module MergeRequests
private private
def get_branches(raw_changes) def branches
Gitlab::ChangesList.new(raw_changes).map do |changes| changes_by_branch.keys
end
def changes_by_branch
@changes_by_branch ||= changes.each_with_object({}) do |changes, result|
next unless Gitlab::Git.branch_ref?(changes[:ref]) next unless Gitlab::Git.branch_ref?(changes[:ref])
# Deleted branch # Deleted branch
...@@ -45,8 +49,8 @@ module MergeRequests ...@@ -45,8 +49,8 @@ module MergeRequests
branch_name = Gitlab::Git.branch_name(changes[:ref]) branch_name = Gitlab::Git.branch_name(changes[:ref])
next if branch_name == target_project.default_branch next if branch_name == target_project.default_branch
branch_name result[branch_name] = changes
end.compact.uniq end
end end
def validate_service def validate_service
...@@ -101,7 +105,7 @@ module MergeRequests ...@@ -101,7 +105,7 @@ module MergeRequests
project, project,
current_user, current_user,
merge_request.attributes.merge(assignees: merge_request.assignees, merge_request.attributes.merge(assignees: merge_request.assignees,
label_ids: merge_request.label_ids) label_ids: merge_request.label_ids)
).execute ).execute
end end
...@@ -112,7 +116,7 @@ module MergeRequests ...@@ -112,7 +116,7 @@ module MergeRequests
merge_request = ::MergeRequests::UpdateService.new( merge_request = ::MergeRequests::UpdateService.new(
target_project, target_project,
current_user, current_user,
update_params update_params(merge_request)
).execute(merge_request) ).execute(merge_request)
collect_errors_from_merge_request(merge_request) unless merge_request.valid? collect_errors_from_merge_request(merge_request) unless merge_request.valid?
...@@ -130,19 +134,22 @@ module MergeRequests ...@@ -130,19 +134,22 @@ module MergeRequests
params.compact! params.compact!
if push_options.key?(:merge_when_pipeline_succeeds)
params.merge!(
merge_when_pipeline_succeeds: push_options[:merge_when_pipeline_succeeds],
merge_user: current_user
)
end
params[:add_labels] = params.delete(:label).keys if params.has_key?(:label) params[:add_labels] = params.delete(:label).keys if params.has_key?(:label)
params[:remove_labels] = params.delete(:unlabel).keys if params.has_key?(:unlabel) params[:remove_labels] = params.delete(:unlabel).keys if params.has_key?(:unlabel)
params params
end end
def merge_params(branch)
return {} unless push_options.key?(:merge_when_pipeline_succeeds)
{
merge_when_pipeline_succeeds: push_options[:merge_when_pipeline_succeeds],
merge_user: current_user,
sha: changes_by_branch.dig(branch, :newrev)
}
end
def create_params(branch) def create_params(branch)
params = base_params params = base_params
...@@ -153,13 +160,15 @@ module MergeRequests ...@@ -153,13 +160,15 @@ module MergeRequests
target_project: target_project target_project: target_project
) )
params.merge!(merge_params(branch))
params[:target_branch] ||= target_project.default_branch params[:target_branch] ||= target_project.default_branch
params params
end end
def update_params def update_params(merge_request)
base_params base_params.merge(merge_params(merge_request.source_branch))
end end
def collect_errors_from_merge_request(merge_request) def collect_errors_from_merge_request(merge_request)
......
---
title: Fix merging merge requests from push options
merge_request: 20639
author:
type: fixed
...@@ -101,17 +101,15 @@ describe MergeRequests::PushOptionsHandlerService do ...@@ -101,17 +101,15 @@ describe MergeRequests::PushOptionsHandlerService do
shared_examples_for 'a service that can set the merge request to merge when pipeline succeeds' do shared_examples_for 'a service that can set the merge request to merge when pipeline succeeds' do
subject(:last_mr) { MergeRequest.last } subject(:last_mr) { MergeRequest.last }
let(:change) { Gitlab::ChangesList.new(changes).changes.first }
it 'sets auto_merge_enabled' do it 'sets auto_merge_enabled' do
service.execute service.execute
expect(last_mr.auto_merge_enabled).to eq(true) expect(last_mr.auto_merge_enabled).to eq(true)
expect(last_mr.auto_merge_strategy).to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) expect(last_mr.auto_merge_strategy).to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
end
it 'sets merge_user to the user' do
service.execute
expect(last_mr.merge_user).to eq(user) expect(last_mr.merge_user).to eq(user)
expect(last_mr.merge_params['sha']).to eq(change[:newrev])
end end
end end
......
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