Merge branch '43263-git-push-option-to-create-mr' into 'master'
Git push options to create a merge request, set target_branch and set merge when pipeline succeeds Closes #53198 and #43263 See merge request gitlab-org/gitlab-ce!26752
This commit is contained in:
commit
d95889b8a4
|
@ -37,7 +37,7 @@ module Ci
|
|||
variables_attributes: params[:variables_attributes],
|
||||
project: project,
|
||||
current_user: current_user,
|
||||
push_options: params[:push_options],
|
||||
push_options: params[:push_options] || {},
|
||||
chat_data: params[:chat_data],
|
||||
**extra_options(options))
|
||||
|
||||
|
|
|
@ -79,7 +79,7 @@ module Git
|
|||
limited_commits,
|
||||
event_message,
|
||||
commits_count: commits_count,
|
||||
push_options: params[:push_options] || []
|
||||
push_options: params[:push_options] || {}
|
||||
)
|
||||
|
||||
# Dependent code may modify the push data, so return a duplicate each time
|
||||
|
|
|
@ -0,0 +1,162 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module MergeRequests
|
||||
class PushOptionsHandlerService
|
||||
LIMIT = 10
|
||||
|
||||
attr_reader :branches, :changes_by_branch, :current_user, :errors,
|
||||
:project, :push_options, :target_project
|
||||
|
||||
def initialize(project, current_user, changes, push_options)
|
||||
@project = project
|
||||
@target_project = @project.default_merge_request_target
|
||||
@current_user = current_user
|
||||
@branches = get_branches(changes)
|
||||
@push_options = push_options
|
||||
@errors = []
|
||||
end
|
||||
|
||||
def execute
|
||||
validate_service
|
||||
return self if errors.present?
|
||||
|
||||
branches.each do |branch|
|
||||
execute_for_branch(branch)
|
||||
rescue Gitlab::Access::AccessDeniedError
|
||||
errors << 'User access was denied'
|
||||
rescue StandardError => e
|
||||
Gitlab::AppLogger.error(e)
|
||||
errors << 'An unknown error occurred'
|
||||
end
|
||||
|
||||
self
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def get_branches(raw_changes)
|
||||
Gitlab::ChangesList.new(raw_changes).map do |changes|
|
||||
next unless Gitlab::Git.branch_ref?(changes[:ref])
|
||||
|
||||
# Deleted branch
|
||||
next if Gitlab::Git.blank_ref?(changes[:newrev])
|
||||
|
||||
# Default branch
|
||||
branch_name = Gitlab::Git.branch_name(changes[:ref])
|
||||
next if branch_name == target_project.default_branch
|
||||
|
||||
branch_name
|
||||
end.compact.uniq
|
||||
end
|
||||
|
||||
def validate_service
|
||||
errors << 'User is required' if current_user.nil?
|
||||
|
||||
unless target_project.merge_requests_enabled?
|
||||
errors << "Merge requests are not enabled for project #{target_project.full_path}"
|
||||
end
|
||||
|
||||
if branches.size > LIMIT
|
||||
errors << "Too many branches pushed (#{branches.size} were pushed, limit is #{LIMIT})"
|
||||
end
|
||||
|
||||
if push_options[:target] && !target_project.repository.branch_exists?(push_options[:target])
|
||||
errors << "Branch #{push_options[:target]} does not exist"
|
||||
end
|
||||
end
|
||||
|
||||
# Returns a Hash of branch => MergeRequest
|
||||
def merge_requests
|
||||
@merge_requests ||= MergeRequest.from_project(target_project)
|
||||
.opened
|
||||
.from_source_branches(branches)
|
||||
.index_by(&:source_branch)
|
||||
end
|
||||
|
||||
def execute_for_branch(branch)
|
||||
merge_request = merge_requests[branch]
|
||||
|
||||
if merge_request
|
||||
update!(merge_request)
|
||||
else
|
||||
create!(branch)
|
||||
end
|
||||
end
|
||||
|
||||
def create!(branch)
|
||||
unless push_options[:create]
|
||||
errors << "A merge_request.create push option is required to create a merge request for branch #{branch}"
|
||||
return
|
||||
end
|
||||
|
||||
# Use BuildService to assign the standard attributes of a merge request
|
||||
merge_request = ::MergeRequests::BuildService.new(
|
||||
project,
|
||||
current_user,
|
||||
create_params(branch)
|
||||
).execute
|
||||
|
||||
unless merge_request.errors.present?
|
||||
merge_request = ::MergeRequests::CreateService.new(
|
||||
project,
|
||||
current_user,
|
||||
merge_request.attributes
|
||||
).execute
|
||||
end
|
||||
|
||||
collect_errors_from_merge_request(merge_request) unless merge_request.persisted?
|
||||
end
|
||||
|
||||
def update!(merge_request)
|
||||
merge_request = ::MergeRequests::UpdateService.new(
|
||||
target_project,
|
||||
current_user,
|
||||
update_params
|
||||
).execute(merge_request)
|
||||
|
||||
collect_errors_from_merge_request(merge_request) unless merge_request.valid?
|
||||
end
|
||||
|
||||
def create_params(branch)
|
||||
params = {
|
||||
assignee: current_user,
|
||||
source_branch: branch,
|
||||
source_project: project,
|
||||
target_branch: push_options[:target] || target_project.default_branch,
|
||||
target_project: target_project
|
||||
}
|
||||
|
||||
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
|
||||
end
|
||||
|
||||
def update_params
|
||||
params = {}
|
||||
|
||||
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
|
||||
|
||||
if push_options.key?(:target)
|
||||
params[:target_branch] = push_options[:target]
|
||||
end
|
||||
|
||||
params
|
||||
end
|
||||
|
||||
def collect_errors_from_merge_request(merge_request)
|
||||
merge_request.errors.full_messages.each do |error|
|
||||
errors << error
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -3,7 +3,7 @@
|
|||
class PostReceive
|
||||
include ApplicationWorker
|
||||
|
||||
def perform(gl_repository, identifier, changes, push_options = [])
|
||||
def perform(gl_repository, identifier, changes, push_options = {})
|
||||
project, repo_type = Gitlab::GlRepository.parse(gl_repository)
|
||||
|
||||
if project.nil?
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Allow merge requests to be created via git push options
|
||||
merge_request: 26752
|
||||
author:
|
||||
type: added
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
title: Allow merge requests to be set to merge when pipeline succeeds via git push
|
||||
options
|
||||
merge_request: 26842
|
||||
author:
|
||||
type: added
|
|
@ -219,6 +219,64 @@ apply the patches. The target branch can be specified using the
|
|||
[`/target_branch` quick action](../quick_actions.md). If the source
|
||||
branch already exists, the patches will be applied on top of it.
|
||||
|
||||
## Git push options
|
||||
|
||||
> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/26752) in GitLab 11.10.
|
||||
|
||||
NOTE: **Note:**
|
||||
Git push options are only available with Git 2.10 or newer.
|
||||
|
||||
GitLab supports using
|
||||
[Git push options](https://git-scm.com/docs/git-push#Documentation/git-push.txt--oltoptiongt)
|
||||
to perform the following actions against merge requests at the same time
|
||||
as pushing changes:
|
||||
|
||||
- Create a new merge request for the pushed branch.
|
||||
- Set the target of the merge request to a particular branch.
|
||||
- Set the merge request to merge when its pipeline succeeds.
|
||||
|
||||
### Create a new merge request using git push options
|
||||
|
||||
To create a new merge request for a branch, use the
|
||||
`merge_request.create` push option:
|
||||
|
||||
```sh
|
||||
git push -o merge_request.create
|
||||
```
|
||||
|
||||
### Set the target branch of a merge request using git push options
|
||||
|
||||
To update an existing merge request's target branch, use the
|
||||
`merge_request.target=<branch_name>` push option:
|
||||
|
||||
```sh
|
||||
git push -o merge_request.target=branch_name
|
||||
```
|
||||
|
||||
You can also create a merge request and set its target branch at the
|
||||
same time using a `-o` flag per push option:
|
||||
|
||||
```sh
|
||||
git push -o merge_request.create -o merge_request.target=branch_name
|
||||
```
|
||||
|
||||
### Set merge when pipeline succeeds using git push options
|
||||
|
||||
To set an existing merge request to
|
||||
[merge when its pipeline succeeds](merge_when_pipeline_succeeds.md), use
|
||||
the `merge_request.merge_when_pipeline_succeeds` push option:
|
||||
|
||||
```sh
|
||||
git push -o merge_request.merge_when_pipeline_succeeds
|
||||
```
|
||||
|
||||
You can also create a merge request and set it to merge when its
|
||||
pipeline succeeds at the same time using a `-o` flag per push option:
|
||||
|
||||
```sh
|
||||
git push -o merge_request.create -o merge_request.merge_when_pipeline_succeeds
|
||||
```
|
||||
|
||||
## Find the merge request that introduced a change
|
||||
|
||||
> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/2383) in GitLab 10.5.
|
||||
|
|
|
@ -43,6 +43,28 @@ module API
|
|||
::MergeRequests::GetUrlsService.new(project).execute(params[:changes])
|
||||
end
|
||||
|
||||
def process_mr_push_options(push_options, project, user, changes)
|
||||
output = {}
|
||||
|
||||
service = ::MergeRequests::PushOptionsHandlerService.new(
|
||||
project,
|
||||
user,
|
||||
changes,
|
||||
push_options
|
||||
).execute
|
||||
|
||||
if service.errors.present?
|
||||
output[:warnings] = push_options_warning(service.errors.join("\n\n"))
|
||||
end
|
||||
|
||||
output
|
||||
end
|
||||
|
||||
def push_options_warning(warning)
|
||||
options = Array.wrap(params[:push_options]).map { |p| "'#{p}'" }.join(' ')
|
||||
"Error encountered with push options #{options}: #{warning}"
|
||||
end
|
||||
|
||||
def redis_ping
|
||||
result = Gitlab::Redis::SharedState.with { |redis| redis.ping }
|
||||
|
||||
|
|
|
@ -256,19 +256,27 @@ module API
|
|||
post '/post_receive' do
|
||||
status 200
|
||||
|
||||
output = {} # Messages to gitlab-shell
|
||||
user = identify(params[:identifier])
|
||||
project = Gitlab::GlRepository.parse(params[:gl_repository]).first
|
||||
push_options = Gitlab::PushOptions.new(params[:push_options])
|
||||
|
||||
PostReceive.perform_async(params[:gl_repository], params[:identifier],
|
||||
params[:changes], params[:push_options].to_a)
|
||||
params[:changes], push_options.as_json)
|
||||
|
||||
if Feature.enabled?(:mr_push_options, default_enabled: true)
|
||||
mr_options = push_options.get(:merge_request)
|
||||
output.merge!(process_mr_push_options(mr_options, project, user, params[:changes])) if mr_options.present?
|
||||
end
|
||||
|
||||
broadcast_message = BroadcastMessage.current&.last&.message
|
||||
reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease
|
||||
|
||||
output = {
|
||||
merge_request_urls: merge_request_urls,
|
||||
output.merge!(
|
||||
broadcast_message: broadcast_message,
|
||||
reference_counter_decreased: reference_counter_decreased
|
||||
}
|
||||
|
||||
project = Gitlab::GlRepository.parse(params[:gl_repository]).first
|
||||
user = identify(params[:identifier])
|
||||
reference_counter_decreased: reference_counter_decreased,
|
||||
merge_request_urls: merge_request_urls
|
||||
)
|
||||
|
||||
# A user is not guaranteed to be returned; an orphaned write deploy
|
||||
# key could be used
|
||||
|
|
|
@ -8,7 +8,6 @@ module Gitlab
|
|||
include ::Gitlab::Utils::StrongMemoize
|
||||
|
||||
SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i
|
||||
SKIP_PUSH_OPTION = 'ci.skip'
|
||||
|
||||
def perform!
|
||||
if skipped?
|
||||
|
@ -35,7 +34,8 @@ module Gitlab
|
|||
end
|
||||
|
||||
def push_option_skips_ci?
|
||||
!!(@command.push_options&.include?(SKIP_PUSH_OPTION))
|
||||
@command.push_options.present? &&
|
||||
@command.push_options.deep_symbolize_keys.dig(:ci, :skip).present?
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -32,10 +32,7 @@ module Gitlab
|
|||
}
|
||||
],
|
||||
total_commits_count: 1,
|
||||
push_options: [
|
||||
"ci.skip",
|
||||
"custom option"
|
||||
]
|
||||
push_options: { ci: { skip: true } }
|
||||
}.freeze
|
||||
|
||||
# Produce a hash of post-receive data
|
||||
|
@ -57,11 +54,11 @@ module Gitlab
|
|||
# },
|
||||
# commits: Array,
|
||||
# total_commits_count: Fixnum,
|
||||
# push_options: Array
|
||||
# push_options: Hash
|
||||
# }
|
||||
#
|
||||
# rubocop:disable Metrics/ParameterLists
|
||||
def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: [])
|
||||
def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: {})
|
||||
commits = Array(commits)
|
||||
|
||||
# Total commits count
|
||||
|
|
|
@ -5,7 +5,7 @@ module Gitlab
|
|||
include Gitlab::Identifier
|
||||
attr_reader :project, :identifier, :changes, :push_options
|
||||
|
||||
def initialize(project, identifier, changes, push_options)
|
||||
def initialize(project, identifier, changes, push_options = {})
|
||||
@project = project
|
||||
@identifier = identifier
|
||||
@changes = deserialize_changes(changes)
|
||||
|
|
|
@ -0,0 +1,70 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module Gitlab
|
||||
class PushOptions
|
||||
VALID_OPTIONS = HashWithIndifferentAccess.new({
|
||||
merge_request: {
|
||||
keys: [:create, :merge_when_pipeline_succeeds, :target]
|
||||
},
|
||||
ci: {
|
||||
keys: [:skip]
|
||||
}
|
||||
}).freeze
|
||||
|
||||
NAMESPACE_ALIASES = HashWithIndifferentAccess.new({
|
||||
mr: :merge_request
|
||||
}).freeze
|
||||
|
||||
OPTION_MATCHER = /(?<namespace>[^\.]+)\.(?<key>[^=]+)=?(?<value>.*)/
|
||||
|
||||
attr_reader :options
|
||||
|
||||
def initialize(options = [])
|
||||
@options = parse_options(options)
|
||||
end
|
||||
|
||||
def get(*args)
|
||||
options.dig(*args)
|
||||
end
|
||||
|
||||
# Allow #to_json serialization
|
||||
def as_json(*_args)
|
||||
options
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def parse_options(raw_options)
|
||||
options = HashWithIndifferentAccess.new
|
||||
|
||||
Array.wrap(raw_options).each do |option|
|
||||
namespace, key, value = parse_option(option)
|
||||
|
||||
next if [namespace, key].any?(&:nil?)
|
||||
|
||||
options[namespace] ||= HashWithIndifferentAccess.new
|
||||
options[namespace][key] = value
|
||||
end
|
||||
|
||||
options
|
||||
end
|
||||
|
||||
def parse_option(option)
|
||||
parts = OPTION_MATCHER.match(option)
|
||||
return unless parts
|
||||
|
||||
namespace, key, value = parts.values_at(:namespace, :key, :value).map(&:strip)
|
||||
namespace = NAMESPACE_ALIASES[namespace] if NAMESPACE_ALIASES[namespace]
|
||||
value = value.presence || true
|
||||
|
||||
return unless valid_option?(namespace, key)
|
||||
|
||||
[namespace, key, value]
|
||||
end
|
||||
|
||||
def valid_option?(namespace, key)
|
||||
keys = VALID_OPTIONS.dig(namespace, :keys)
|
||||
keys && keys.include?(key.to_sym)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,103 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::PushOptions do
|
||||
describe 'namespace and key validation' do
|
||||
it 'ignores unrecognised namespaces' do
|
||||
options = described_class.new(['invalid.key=value'])
|
||||
|
||||
expect(options.get(:invalid)).to eq(nil)
|
||||
end
|
||||
|
||||
it 'ignores unrecognised keys' do
|
||||
options = described_class.new(['merge_request.key=value'])
|
||||
|
||||
expect(options.get(:merge_request)).to eq(nil)
|
||||
end
|
||||
|
||||
it 'ignores blank keys' do
|
||||
options = described_class.new(['merge_request'])
|
||||
|
||||
expect(options.get(:merge_request)).to eq(nil)
|
||||
end
|
||||
|
||||
it 'parses recognised namespace and key pairs' do
|
||||
options = described_class.new(['merge_request.target=value'])
|
||||
|
||||
expect(options.get(:merge_request)).to include({
|
||||
target: 'value'
|
||||
})
|
||||
end
|
||||
end
|
||||
|
||||
describe '#get' do
|
||||
it 'can emulate Hash#dig' do
|
||||
options = described_class.new(['merge_request.target=value'])
|
||||
|
||||
expect(options.get(:merge_request, :target)).to eq('value')
|
||||
end
|
||||
end
|
||||
|
||||
describe '#as_json' do
|
||||
it 'returns all options' do
|
||||
options = described_class.new(['merge_request.target=value'])
|
||||
|
||||
expect(options.as_json).to include(
|
||||
merge_request: {
|
||||
target: 'value'
|
||||
}
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
it 'can parse multiple push options' do
|
||||
options = described_class.new([
|
||||
'merge_request.create',
|
||||
'merge_request.target=value'
|
||||
])
|
||||
|
||||
expect(options.get(:merge_request)).to include({
|
||||
create: true,
|
||||
target: 'value'
|
||||
})
|
||||
expect(options.get(:merge_request, :create)).to eq(true)
|
||||
expect(options.get(:merge_request, :target)).to eq('value')
|
||||
end
|
||||
|
||||
it 'stores options internally as a HashWithIndifferentAccess' do
|
||||
options = described_class.new([
|
||||
'merge_request.create'
|
||||
])
|
||||
|
||||
expect(options.get('merge_request', 'create')).to eq(true)
|
||||
expect(options.get(:merge_request, :create)).to eq(true)
|
||||
end
|
||||
|
||||
it 'selects the last option when options contain duplicate namespace and key pairs' do
|
||||
options = described_class.new([
|
||||
'merge_request.target=value1',
|
||||
'merge_request.target=value2'
|
||||
])
|
||||
|
||||
expect(options.get(:merge_request, :target)).to eq('value2')
|
||||
end
|
||||
|
||||
it 'defaults values to true' do
|
||||
options = described_class.new(['merge_request.create'])
|
||||
|
||||
expect(options.get(:merge_request, :create)).to eq(true)
|
||||
end
|
||||
|
||||
it 'expands aliases' do
|
||||
options = described_class.new(['mr.target=value'])
|
||||
|
||||
expect(options.get(:merge_request, :target)).to eq('value')
|
||||
end
|
||||
|
||||
it 'forgives broken push options' do
|
||||
options = described_class.new(['merge_request . target = value'])
|
||||
|
||||
expect(options.get(:merge_request, :target)).to eq('value')
|
||||
end
|
||||
end
|
|
@ -888,8 +888,10 @@ describe API::Internal do
|
|||
}
|
||||
end
|
||||
|
||||
let(:branch_name) { 'feature' }
|
||||
|
||||
let(:changes) do
|
||||
"#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch"
|
||||
"#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{branch_name}"
|
||||
end
|
||||
|
||||
let(:push_options) do
|
||||
|
@ -905,9 +907,9 @@ describe API::Internal do
|
|||
|
||||
it 'enqueues a PostReceive worker job' do
|
||||
expect(PostReceive).to receive(:perform_async)
|
||||
.with(gl_repository, identifier, changes, push_options)
|
||||
.with(gl_repository, identifier, changes, { ci: { skip: true } })
|
||||
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
end
|
||||
|
||||
it 'decreases the reference counter and returns the result' do
|
||||
|
@ -915,17 +917,17 @@ describe API::Internal do
|
|||
.and_return(reference_counter)
|
||||
expect(reference_counter).to receive(:decrease).and_return(true)
|
||||
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(json_response['reference_counter_decreased']).to be(true)
|
||||
end
|
||||
|
||||
it 'returns link to create new merge request' do
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(json_response['merge_request_urls']).to match [{
|
||||
"branch_name" => "new_branch",
|
||||
"url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch",
|
||||
"branch_name" => branch_name,
|
||||
"url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{branch_name}",
|
||||
"new_merge_request" => true
|
||||
}]
|
||||
end
|
||||
|
@ -933,16 +935,87 @@ describe API::Internal do
|
|||
it 'returns empty array if printing_merge_request_link_enabled is false' do
|
||||
project.update!(printing_merge_request_link_enabled: false)
|
||||
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(json_response['merge_request_urls']).to eq([])
|
||||
end
|
||||
|
||||
it 'does not invoke MergeRequests::PushOptionsHandlerService' do
|
||||
expect(MergeRequests::PushOptionsHandlerService).not_to receive(:new)
|
||||
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
end
|
||||
|
||||
context 'when there are merge_request push options' do
|
||||
before do
|
||||
valid_params[:push_options] = ['merge_request.create']
|
||||
end
|
||||
|
||||
it 'invokes MergeRequests::PushOptionsHandlerService' do
|
||||
expect(MergeRequests::PushOptionsHandlerService).to receive(:new)
|
||||
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
end
|
||||
|
||||
it 'creates a new merge request' do
|
||||
expect do
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
end.to change { MergeRequest.count }.by(1)
|
||||
end
|
||||
|
||||
it 'links to the newly created merge request' do
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(json_response['merge_request_urls']).to match [{
|
||||
'branch_name' => branch_name,
|
||||
'url' => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/1",
|
||||
'new_merge_request' => false
|
||||
}]
|
||||
end
|
||||
|
||||
it 'adds errors on the service instance to warnings' do
|
||||
expect_any_instance_of(
|
||||
MergeRequests::PushOptionsHandlerService
|
||||
).to receive(:errors).at_least(:once).and_return(['my error'])
|
||||
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(json_response['warnings']).to eq('Error encountered with push options \'merge_request.create\': my error')
|
||||
end
|
||||
|
||||
it 'adds ActiveRecord errors on invalid MergeRequest records to warnings' do
|
||||
invalid_merge_request = MergeRequest.new
|
||||
invalid_merge_request.errors.add(:base, 'my error')
|
||||
|
||||
expect_any_instance_of(
|
||||
MergeRequests::CreateService
|
||||
).to receive(:execute).and_return(invalid_merge_request)
|
||||
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(json_response['warnings']).to eq('Error encountered with push options \'merge_request.create\': my error')
|
||||
end
|
||||
|
||||
context 'when the feature is disabled' do
|
||||
it 'does not invoke MergeRequests::PushOptionsHandlerService' do
|
||||
Feature.disable(:mr_push_options)
|
||||
|
||||
expect(MergeRequests::PushOptionsHandlerService).to receive(:new)
|
||||
|
||||
expect do
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
end.not_to change { MergeRequest.count }
|
||||
|
||||
Feature.enable(:mr_push_options)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'broadcast message exists' do
|
||||
let!(:broadcast_message) { create(:broadcast_message, starts_at: 1.day.ago, ends_at: 1.day.from_now ) }
|
||||
|
||||
it 'returns one broadcast message' do
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(json_response['broadcast_message']).to eq(broadcast_message.message)
|
||||
|
@ -951,7 +1024,7 @@ describe API::Internal do
|
|||
|
||||
context 'broadcast message does not exist' do
|
||||
it 'returns empty string' do
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(json_response['broadcast_message']).to eq(nil)
|
||||
|
@ -962,7 +1035,7 @@ describe API::Internal do
|
|||
it 'returns empty string' do
|
||||
allow(BroadcastMessage).to receive(:current).and_return(nil)
|
||||
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(json_response['broadcast_message']).to eq(nil)
|
||||
|
@ -974,7 +1047,7 @@ describe API::Internal do
|
|||
project_moved = Gitlab::Checks::ProjectMoved.new(project, user, 'http', 'foo/baz')
|
||||
project_moved.add_message
|
||||
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(json_response["redirected_message"]).to be_present
|
||||
|
@ -987,7 +1060,7 @@ describe API::Internal do
|
|||
project_created = Gitlab::Checks::ProjectCreated.new(project, user, 'http')
|
||||
project_created.add_message
|
||||
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(json_response["project_created_message"]).to be_present
|
||||
|
@ -999,7 +1072,7 @@ describe API::Internal do
|
|||
it 'does not try to notify that project moved' do
|
||||
allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(nil)
|
||||
|
||||
post api("/internal/post_receive"), params: valid_params
|
||||
post api('/internal/post_receive'), params: valid_params
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
end
|
||||
|
|
|
@ -418,8 +418,7 @@ describe Ci::CreatePipelineService do
|
|||
|
||||
context 'when push options contain ci.skip' do
|
||||
let(:push_options) do
|
||||
['ci.skip',
|
||||
'another push option']
|
||||
{ 'ci' => { 'skip' => true } }
|
||||
end
|
||||
|
||||
it 'creates a pipline in the skipped state' do
|
||||
|
|
|
@ -0,0 +1,404 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
|
||||
describe MergeRequests::PushOptionsHandlerService do
|
||||
include ProjectForksHelper
|
||||
|
||||
let(:user) { create(:user) }
|
||||
let(:project) { create(:project, :public, :repository) }
|
||||
let(:forked_project) { fork_project(project, user, repository: true) }
|
||||
let(:service) { described_class.new(project, user, changes, push_options) }
|
||||
let(:source_branch) { 'fix' }
|
||||
let(:target_branch) { 'feature' }
|
||||
let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
|
||||
let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
|
||||
let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" }
|
||||
let(:default_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{project.default_branch}" }
|
||||
|
||||
before do
|
||||
project.add_developer(user)
|
||||
end
|
||||
|
||||
shared_examples_for 'a service that can create a merge request' do
|
||||
subject(:last_mr) { MergeRequest.last }
|
||||
|
||||
it 'creates a merge request' do
|
||||
expect { service.execute }.to change { MergeRequest.count }.by(1)
|
||||
end
|
||||
|
||||
it 'sets the correct target branch' do
|
||||
branch = push_options[:target] || project.default_branch
|
||||
|
||||
service.execute
|
||||
|
||||
expect(last_mr.target_branch).to eq(branch)
|
||||
end
|
||||
|
||||
it 'assigns the MR to the user' do
|
||||
service.execute
|
||||
|
||||
expect(last_mr.assignee).to eq(user)
|
||||
end
|
||||
|
||||
context 'when project has been forked' do
|
||||
let(:forked_project) { fork_project(project, user, repository: true) }
|
||||
let(:service) { described_class.new(forked_project, user, changes, push_options) }
|
||||
|
||||
before do
|
||||
allow(forked_project).to receive(:empty_repo?).and_return(false)
|
||||
end
|
||||
|
||||
it 'sets the correct source project' do
|
||||
service.execute
|
||||
|
||||
expect(last_mr.source_project).to eq(forked_project)
|
||||
end
|
||||
|
||||
it 'sets the correct target project' do
|
||||
service.execute
|
||||
|
||||
expect(last_mr.target_project).to eq(project)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'a service that can set the target of a merge request' do
|
||||
subject(:last_mr) { MergeRequest.last }
|
||||
|
||||
it 'sets the target_branch' do
|
||||
service.execute
|
||||
|
||||
expect(last_mr.target_branch).to eq(target_branch)
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'a service that can set the merge request to merge when pipeline succeeds' do
|
||||
subject(:last_mr) { MergeRequest.last }
|
||||
|
||||
it 'sets merge_when_pipeline_succeeds' do
|
||||
service.execute
|
||||
|
||||
expect(last_mr.merge_when_pipeline_succeeds).to eq(true)
|
||||
end
|
||||
|
||||
it 'sets merge_user to the user' do
|
||||
service.execute
|
||||
|
||||
expect(last_mr.merge_user).to eq(user)
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'a service that does not create a merge request' do
|
||||
it do
|
||||
expect { service.execute }.not_to change { MergeRequest.count }
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'a service that does not update a merge request' do
|
||||
it do
|
||||
expect { service.execute }.not_to change { MergeRequest.maximum(:updated_at) }
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'a service that does nothing' do
|
||||
include_examples 'a service that does not create a merge request'
|
||||
include_examples 'a service that does not update a merge request'
|
||||
end
|
||||
|
||||
describe '`create` push option' do
|
||||
let(:push_options) { { create: true } }
|
||||
|
||||
context 'with a new branch' do
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that can create a merge request'
|
||||
end
|
||||
|
||||
context 'with an existing branch but no open MR' do
|
||||
let(:changes) { existing_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that can create a merge request'
|
||||
end
|
||||
|
||||
context 'with an existing branch that has a merge request open' do
|
||||
let(:changes) { existing_branch_changes }
|
||||
let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)}
|
||||
|
||||
it_behaves_like 'a service that does not create a merge request'
|
||||
end
|
||||
|
||||
context 'with a deleted branch' do
|
||||
let(:changes) { deleted_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does nothing'
|
||||
end
|
||||
|
||||
context 'with the project default branch' do
|
||||
let(:changes) { default_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does nothing'
|
||||
end
|
||||
end
|
||||
|
||||
describe '`merge_when_pipeline_succeeds` push option' do
|
||||
let(:push_options) { { merge_when_pipeline_succeeds: true } }
|
||||
|
||||
context 'with a new branch' do
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does not create a merge request'
|
||||
|
||||
it 'adds an error to the service' do
|
||||
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
|
||||
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to include(error)
|
||||
end
|
||||
|
||||
context 'when coupled with the `create` push option' do
|
||||
let(:push_options) { { create: true, merge_when_pipeline_succeeds: true } }
|
||||
|
||||
it_behaves_like 'a service that can create a merge request'
|
||||
it_behaves_like 'a service that can set the merge request to merge when pipeline succeeds'
|
||||
end
|
||||
end
|
||||
|
||||
context 'with an existing branch but no open MR' do
|
||||
let(:changes) { existing_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does not create a merge request'
|
||||
|
||||
it 'adds an error to the service' do
|
||||
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
|
||||
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to include(error)
|
||||
end
|
||||
|
||||
context 'when coupled with the `create` push option' do
|
||||
let(:push_options) { { create: true, merge_when_pipeline_succeeds: true } }
|
||||
|
||||
it_behaves_like 'a service that can create a merge request'
|
||||
it_behaves_like 'a service that can set the merge request to merge when pipeline succeeds'
|
||||
end
|
||||
end
|
||||
|
||||
context 'with an existing branch that has a merge request open' do
|
||||
let(:changes) { existing_branch_changes }
|
||||
let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)}
|
||||
|
||||
it_behaves_like 'a service that does not create a merge request'
|
||||
it_behaves_like 'a service that can set the merge request to merge when pipeline succeeds'
|
||||
end
|
||||
|
||||
context 'with a deleted branch' do
|
||||
let(:changes) { deleted_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does nothing'
|
||||
end
|
||||
|
||||
context 'with the project default branch' do
|
||||
let(:changes) { default_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does nothing'
|
||||
end
|
||||
end
|
||||
|
||||
describe '`target` push option' do
|
||||
let(:push_options) { { target: target_branch } }
|
||||
|
||||
context 'with a new branch' do
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does not create a merge request'
|
||||
|
||||
it 'adds an error to the service' do
|
||||
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
|
||||
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to include(error)
|
||||
end
|
||||
|
||||
context 'when coupled with the `create` push option' do
|
||||
let(:push_options) { { create: true, target: target_branch } }
|
||||
|
||||
it_behaves_like 'a service that can create a merge request'
|
||||
it_behaves_like 'a service that can set the target of a merge request'
|
||||
end
|
||||
end
|
||||
|
||||
context 'with an existing branch but no open MR' do
|
||||
let(:changes) { existing_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does not create a merge request'
|
||||
|
||||
it 'adds an error to the service' do
|
||||
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
|
||||
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to include(error)
|
||||
end
|
||||
|
||||
context 'when coupled with the `create` push option' do
|
||||
let(:push_options) { { create: true, target: target_branch } }
|
||||
|
||||
it_behaves_like 'a service that can create a merge request'
|
||||
it_behaves_like 'a service that can set the target of a merge request'
|
||||
end
|
||||
end
|
||||
|
||||
context 'with an existing branch that has a merge request open' do
|
||||
let(:changes) { existing_branch_changes }
|
||||
let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)}
|
||||
|
||||
it_behaves_like 'a service that does not create a merge request'
|
||||
it_behaves_like 'a service that can set the target of a merge request'
|
||||
end
|
||||
|
||||
context 'with a deleted branch' do
|
||||
let(:changes) { deleted_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does nothing'
|
||||
end
|
||||
|
||||
context 'with the project default branch' do
|
||||
let(:changes) { default_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does nothing'
|
||||
end
|
||||
end
|
||||
|
||||
describe 'multiple pushed branches' do
|
||||
let(:push_options) { { create: true } }
|
||||
let(:changes) do
|
||||
[
|
||||
new_branch_changes,
|
||||
"#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/feature_conflict"
|
||||
]
|
||||
end
|
||||
|
||||
it 'creates a merge request per branch' do
|
||||
expect { service.execute }.to change { MergeRequest.count }.by(2)
|
||||
end
|
||||
|
||||
context 'when there are too many pushed branches' do
|
||||
let(:limit) { MergeRequests::PushOptionsHandlerService::LIMIT }
|
||||
let(:changes) do
|
||||
TestEnv::BRANCH_SHA.to_a[0..limit].map do |x|
|
||||
"#{Gitlab::Git::BLANK_SHA} #{x.first} refs/heads/#{x.last}"
|
||||
end
|
||||
end
|
||||
|
||||
it 'records an error' do
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to eq(["Too many branches pushed (#{limit + 1} were pushed, limit is #{limit})"])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'no push options' do
|
||||
let(:push_options) { {} }
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it_behaves_like 'a service that does nothing'
|
||||
end
|
||||
|
||||
describe 'no user' do
|
||||
let(:user) { nil }
|
||||
let(:push_options) { { create: true } }
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it 'records an error' do
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to eq(['User is required'])
|
||||
end
|
||||
end
|
||||
|
||||
describe 'unauthorized user' do
|
||||
let(:push_options) { { create: true } }
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it 'records an error' do
|
||||
Members::DestroyService.new(user).execute(ProjectMember.find_by!(user_id: user.id))
|
||||
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to eq(['User access was denied'])
|
||||
end
|
||||
end
|
||||
|
||||
describe 'handling unexpected exceptions' do
|
||||
let(:push_options) { { create: true } }
|
||||
let(:changes) { new_branch_changes }
|
||||
let(:exception) { StandardError.new('My standard error') }
|
||||
|
||||
def run_service_with_exception
|
||||
allow_any_instance_of(
|
||||
MergeRequests::BuildService
|
||||
).to receive(:execute).and_raise(exception)
|
||||
|
||||
service.execute
|
||||
end
|
||||
|
||||
it 'records an error' do
|
||||
run_service_with_exception
|
||||
|
||||
expect(service.errors).to eq(['An unknown error occurred'])
|
||||
end
|
||||
|
||||
it 'writes to Gitlab::AppLogger' do
|
||||
expect(Gitlab::AppLogger).to receive(:error).with(exception)
|
||||
|
||||
run_service_with_exception
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when target is not a valid branch name' do
|
||||
let(:push_options) { { create: true, target: 'my-branch' } }
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it 'records an error' do
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to eq(['Branch my-branch does not exist'])
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when MRs are not enabled' do
|
||||
let(:push_options) { { create: true } }
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it 'records an error' do
|
||||
expect(project).to receive(:merge_requests_enabled?).and_return(false)
|
||||
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to eq(["Merge requests are not enabled for project #{project.full_path}"])
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when MR has ActiveRecord errors' do
|
||||
let(:push_options) { { create: true } }
|
||||
let(:changes) { new_branch_changes }
|
||||
|
||||
it 'adds the error to its errors property' do
|
||||
invalid_merge_request = MergeRequest.new
|
||||
invalid_merge_request.errors.add(:base, 'my error')
|
||||
|
||||
expect_any_instance_of(
|
||||
MergeRequests::CreateService
|
||||
).to receive(:execute).and_return(invalid_merge_request)
|
||||
|
||||
service.execute
|
||||
|
||||
expect(service.errors).to eq(['my error'])
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue