GitLab steht Mittwoch, den 23. September, zwischen 10:00 und 12:00 Uhr aufgrund von Wartungsarbeiten nicht zur Verfügung.

Commit 6109daf4 authored by Scott Le's avatar Scott Le

api for generating new merge request

DRY code + fix rubocop

Add more test cases

Append to changelog

DRY changes list

find_url service for merge_requests

use GET for getting merge request links

remove files

rename to get_url_service

reduce loop

add test case for cross project

refactor tiny thing

update changelog
parent 5a33bc98
......@@ -102,6 +102,7 @@ v 8.11.0 (unreleased)
- Fix importing GitLab projects with an invalid MR source project
- Sort folders with submodules in Files view !5521
- Each `File::exists?` replaced to `File::exist?` because of deprecate since ruby version 2.2.0
- Print urls to create (or view) merge requests after git push !5542 (Scott Le)
v 8.10.5
- Add a data migration to fix some missing timestamps in the members table. !5670
......
......@@ -104,6 +104,7 @@ class MergeRequest < ActiveRecord::Base
scope :from_project, ->(project) { where(source_project_id: project.id) }
scope :merged, -> { with_state(:merged) }
scope :closed_and_merged, -> { with_states(:closed, :merged) }
scope :from_source_branches, ->(branches) { where(source_branch: branches) }
scope :join_project, -> { joins(:target_project) }
scope :references_project, -> { references(:target_project) }
......
module MergeRequests
class GetUrlsService < BaseService
attr_reader :project
def initialize(project)
@project = project
end
def execute(changes)
branches = get_branches(changes)
merge_requests_map = opened_merge_requests_from_source_branches(branches)
branches.map do |branch|
existing_merge_request = merge_requests_map[branch]
if existing_merge_request
url_for_existing_merge_request(existing_merge_request)
else
url_for_new_merge_request(branch)
end
end
end
private
def opened_merge_requests_from_source_branches(branches)
merge_requests = MergeRequest.from_project(project).opened.from_source_branches(branches)
merge_requests.inject({}) do |hash, mr|
hash[mr.source_branch] = mr
hash
end
end
def get_branches(changes)
changes_list = Gitlab::ChangesList.new(changes)
changes_list.map do |change|
next unless Gitlab::Git.branch_ref?(change[:ref])
Gitlab::Git.branch_name(change[:ref])
end.compact
end
def url_for_new_merge_request(branch_name)
merge_request_params = { source_branch: branch_name }
url = Gitlab::Routing.url_helpers.new_namespace_project_merge_request_url(project.namespace, project, merge_request: merge_request_params)
{ branch_name: branch_name, url: url, new_merge_request: true }
end
def url_for_existing_merge_request(merge_request)
target_project = merge_request.target_project
url = Gitlab::Routing.url_helpers.namespace_project_merge_request_url(target_project.namespace, target_project, merge_request)
{ branch_name: merge_request.source_branch, url: url, new_merge_request: false }
end
end
end
......@@ -74,6 +74,10 @@ module API
response
end
get "/merge_request_urls" do
::MergeRequests::GetUrlsService.new(project).execute(params[:changes])
end
#
# Discover user by ssh key
#
......
module Gitlab
class ChangesList
include Enumerable
attr_reader :raw_changes
def initialize(changes)
@raw_changes = changes.kind_of?(String) ? changes.lines : changes
end
def each(&block)
changes.each(&block)
end
def changes
@changes ||= begin
@raw_changes.map do |change|
next if change.blank?
oldrev, newrev, ref = change.strip.split(' ')
{ oldrev: oldrev, newrev: newrev, ref: ref }
end.compact
end
end
end
end
......@@ -4,8 +4,8 @@ module Gitlab
attr_reader :user_access, :project
def initialize(change, user_access:, project:)
@oldrev, @newrev, @ref = change.split(' ')
@branch_name = branch_name(@ref)
@oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
@branch_name = Gitlab::Git.branch_name(@ref)
@user_access = user_access
@project = project
end
......@@ -47,7 +47,7 @@ module Gitlab
end
def tag_checks
tag_ref = tag_name(@ref)
tag_ref = Gitlab::Git.tag_name(@ref)
if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project)
"You are not allowed to change existing tags on this project."
......@@ -73,24 +73,6 @@ module Gitlab
def matching_merge_request?
Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match?
end
def branch_name(ref)
ref = @ref.to_s
if Gitlab::Git.branch_ref?(ref)
Gitlab::Git.ref_name(ref)
else
nil
end
end
def tag_name(ref)
ref = @ref.to_s
if Gitlab::Git.tag_ref?(ref)
Gitlab::Git.ref_name(ref)
else
nil
end
end
end
end
end
......@@ -9,6 +9,24 @@ module Gitlab
ref.gsub(/\Arefs\/(tags|heads)\//, '')
end
def branch_name(ref)
ref = ref.to_s
if self.branch_ref?(ref)
self.ref_name(ref)
else
nil
end
end
def tag_name(ref)
ref = ref.to_s
if self.tag_ref?(ref)
self.ref_name(ref)
else
nil
end
end
def tag_ref?(ref)
ref.start_with?(TAG_REF_PREFIX)
end
......
......@@ -76,10 +76,10 @@ module Gitlab
return build_status_object(false, "A repository for this project does not exist yet.")
end
changes = changes.lines if changes.kind_of?(String)
changes_list = Gitlab::ChangesList.new(changes)
# Iterate over all changes to find if user allowed all of them to be applied
changes.map(&:strip).reject(&:blank?).each do |change|
changes_list.each do |change|
status = change_access_check(change)
unless status.allowed?
# If user does not have access to make at least one change - cancel all push
......
require "spec_helper"
describe Gitlab::ChangesList do
let(:valid_changes_string) { "\n000000 570e7b2 refs/heads/my_branch\nd14d6c 6fd24d refs/heads/master" }
let(:invalid_changes) { 1 }
context 'when changes is a valid string' do
let(:changes_list) { Gitlab::ChangesList.new(valid_changes_string) }
it 'splits elements by newline character' do
expect(changes_list).to contain_exactly({
oldrev: "000000",
newrev: "570e7b2",
ref: "refs/heads/my_branch"
}, {
oldrev: "d14d6c",
newrev: "6fd24d",
ref: "refs/heads/master"
})
end
it 'behaves like a list' do
expect(changes_list.first).to eq({
oldrev: "000000",
newrev: "570e7b2",
ref: "refs/heads/my_branch"
})
end
end
end
......@@ -275,6 +275,24 @@ describe API::API, api: true do
end
end
describe 'GET /internal/merge_request_urls' do
let(:repo_name) { "#{project.namespace.name}/#{project.path}" }
let(:changes) { URI.escape("#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch") }
before do
project.team << [user, :developer]
get api("/internal/merge_request_urls?project=#{repo_name}&changes=#{changes}"), secret_token: secret_token
end
it 'returns link to create new merge request' do
expect(json_response).to match [{
"branch_name" => "new_branch",
"url" => "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch",
"new_merge_request" => true
}]
end
end
def pull(key, project, protocol = 'ssh')
post(
api("/internal/allowed"),
......
require "spec_helper"
describe MergeRequests::GetUrlsService do
let(:project) { create(:project, :public) }
let(:service) { MergeRequests::GetUrlsService.new(project) }
let(:source_branch) { "my_branch" }
let(:new_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" }
let(:show_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" }
let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
describe "#execute" do
shared_examples 'new_merge_request_link' do
it 'returns url to create new merge request' do
result = service.execute(changes)
expect(result).to match([{
branch_name: source_branch,
url: new_merge_request_url,
new_merge_request: true
}])
end
end
shared_examples 'show_merge_request_url' do
it 'returns url to view merge request' do
result = service.execute(changes)
expect(result).to match([{
branch_name: source_branch,
url: show_merge_request_url,
new_merge_request: false
}])
end
end
context 'pushing one completely new branch' do
let(:changes) { new_branch_changes }
it_behaves_like 'new_merge_request_link'
end
context 'pushing to existing branch but no merge request' do
let(:changes) { existing_branch_changes }
it_behaves_like 'new_merge_request_link'
end
context 'pushing to existing branch and merge request opened' do
let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch) }
let(:changes) { existing_branch_changes }
it_behaves_like 'show_merge_request_url'
end
context 'pushing to existing branch and merge request is reopened' do
let!(:merge_request) { create(:merge_request, :reopened, source_project: project, source_branch: source_branch) }
let(:changes) { existing_branch_changes }
it_behaves_like 'show_merge_request_url'
end
context 'pushing to existing branch from forked project' do
let(:user) { create(:user) }
let!(:forked_project) { Projects::ForkService.new(project, user).execute }
let!(:merge_request) { create(:merge_request, source_project: forked_project, target_project: project, source_branch: source_branch) }
let(:changes) { existing_branch_changes }
# Source project is now the forked one
let(:service) { MergeRequests::GetUrlsService.new(forked_project) }
it_behaves_like 'show_merge_request_url'
end
context 'pushing to existing branch and merge request is closed' do
let!(:merge_request) { create(:merge_request, :closed, source_project: project, source_branch: source_branch) }
let(:changes) { existing_branch_changes }
it_behaves_like 'new_merge_request_link'
end
context 'pushing to existing branch and merge request is merged' do
let!(:merge_request) { create(:merge_request, :merged, source_project: project, source_branch: source_branch) }
let(:changes) { existing_branch_changes }
it_behaves_like 'new_merge_request_link'
end
context 'pushing new branch and existing branch (with merge request created) at once' do
let!(:merge_request) { create(:merge_request, source_project: project, source_branch: "existing_branch") }
let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" }
let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/existing_branch" }
let(:changes) { "#{new_branch_changes}\n#{existing_branch_changes}" }
let(:new_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" }
it 'returns 2 urls for both creating new and showing merge request' do
result = service.execute(changes)
expect(result).to match([{
branch_name: "new_branch",
url: new_merge_request_url,
new_merge_request: true
}, {
branch_name: "existing_branch",
url: show_merge_request_url,
new_merge_request: false
}])
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