Commit 09454b70 authored by Patrick Bajao's avatar Patrick Bajao

Support creating an MR on a fork from an issue

parent 3fe7c990
...@@ -172,6 +172,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -172,6 +172,7 @@ class Projects::IssuesController < Projects::ApplicationController
def create_merge_request def create_merge_request
create_params = params.slice(:branch_name, :ref).merge(issue_iid: issue.iid) create_params = params.slice(:branch_name, :ref).merge(issue_iid: issue.iid)
create_params[:target_project_id] = params[:target_project_id] if Feature.enabled?(:create_confidential_merge_request, @project)
result = ::MergeRequests::CreateFromIssueService.new(project, current_user, create_params).execute result = ::MergeRequests::CreateFromIssueService.new(project, current_user, create_params).execute
if result[:status] == :success if result[:status] == :success
......
...@@ -9,24 +9,27 @@ module MergeRequests ...@@ -9,24 +9,27 @@ module MergeRequests
@branch_name = params[:branch_name] @branch_name = params[:branch_name]
@issue_iid = params[:issue_iid] @issue_iid = params[:issue_iid]
@ref = params[:ref] @ref = params[:ref]
@target_project_id = params[:target_project_id]
super(project, user) super(project, user)
end end
def execute def execute
return error('Project not found') if target_project.blank?
return error('Not allowed to create merge request') unless can_create_merge_request?
return error('Invalid issue iid') unless @issue_iid.present? && issue.present? return error('Invalid issue iid') unless @issue_iid.present? && issue.present?
result = CreateBranchService.new(project, current_user).execute(branch_name, ref) result = CreateBranchService.new(target_project, current_user).execute(branch_name, ref)
return result if result[:status] == :error return result if result[:status] == :error
new_merge_request = create(merge_request) new_merge_request = create(merge_request)
if new_merge_request.valid? if new_merge_request.valid?
SystemNoteService.new_merge_request(issue, project, current_user, new_merge_request) SystemNoteService.new_merge_request(issue, target_project, current_user, new_merge_request)
success(new_merge_request) success(new_merge_request)
else else
SystemNoteService.new_issue_branch(issue, project, current_user, branch_name) SystemNoteService.new_issue_branch(issue, target_project, current_user, branch_name)
error(new_merge_request.errors) error(new_merge_request.errors)
end end
...@@ -34,6 +37,10 @@ module MergeRequests ...@@ -34,6 +37,10 @@ module MergeRequests
private private
def can_create_merge_request?
can?(current_user, :create_merge_request_from, target_project)
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def issue def issue
@issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: @issue_iid) @issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: @issue_iid)
...@@ -45,21 +52,21 @@ module MergeRequests ...@@ -45,21 +52,21 @@ module MergeRequests
end end
def ref def ref
return @ref if project.repository.branch_exists?(@ref) return @ref if target_project.repository.branch_exists?(@ref)
project.default_branch || 'master' target_project.default_branch || 'master'
end end
def merge_request def merge_request
MergeRequests::BuildService.new(project, current_user, merge_request_params).execute MergeRequests::BuildService.new(target_project, current_user, merge_request_params).execute
end end
def merge_request_params def merge_request_params
{ {
issue_iid: @issue_iid, issue_iid: @issue_iid,
source_project_id: project.id, source_project_id: target_project.id,
source_branch: branch_name, source_branch: branch_name,
target_project_id: project.id, target_project_id: target_project.id,
target_branch: ref target_branch: ref
} }
end end
...@@ -67,5 +74,14 @@ module MergeRequests ...@@ -67,5 +74,14 @@ module MergeRequests
def success(merge_request) def success(merge_request)
super().merge(merge_request: merge_request) super().merge(merge_request: merge_request)
end end
def target_project
@target_project ||=
if @target_project_id.present?
project.forks.find_by_id(@target_project_id)
else
project
end
end
end end
end end
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
describe MergeRequests::CreateFromIssueService do describe MergeRequests::CreateFromIssueService do
include ProjectForksHelper
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:label_ids) { create_pair(:label, project: project).map(&:id) } let(:label_ids) { create_pair(:label, project: project).map(&:id) }
...@@ -10,14 +12,17 @@ describe MergeRequests::CreateFromIssueService do ...@@ -10,14 +12,17 @@ describe MergeRequests::CreateFromIssueService do
let(:issue) { create(:issue, project: project, milestone_id: milestone_id) } let(:issue) { create(:issue, project: project, milestone_id: milestone_id) }
let(:custom_source_branch) { 'custom-source-branch' } let(:custom_source_branch) { 'custom-source-branch' }
subject(:service) { described_class.new(project, user, issue_iid: issue.iid) } subject(:service) { described_class.new(project, user, service_params) }
subject(:service_with_custom_source_branch) { described_class.new(project, user, issue_iid: issue.iid, branch_name: custom_source_branch) } subject(:service_with_custom_source_branch) { described_class.new(project, user, service_params.merge(branch_name: custom_source_branch)) }
before do before do
project.add_developer(user) project.add_developer(user)
end end
describe '#execute' do describe '#execute' do
context 'no target_project_id specified' do
let(:service_params) { { issue_iid: issue.iid } }
it 'returns an error with invalid issue iid' do it 'returns an error with invalid issue iid' do
result = described_class.new(project, user, issue_iid: -1).execute result = described_class.new(project, user, issue_iid: -1).execute
...@@ -25,12 +30,6 @@ describe MergeRequests::CreateFromIssueService do ...@@ -25,12 +30,6 @@ describe MergeRequests::CreateFromIssueService do
expect(result[:message]).to eq('Invalid issue iid') expect(result[:message]).to eq('Invalid issue iid')
end end
it 'delegates issue search to IssuesFinder' do
expect_any_instance_of(IssuesFinder).to receive(:find_by).once.and_call_original
described_class.new(project, user, issue_iid: -1).execute
end
it "inherits labels" do it "inherits labels" do
issue.assign_attributes(label_ids: label_ids) issue.assign_attributes(label_ids: label_ids)
...@@ -70,7 +69,7 @@ describe MergeRequests::CreateFromIssueService do ...@@ -70,7 +69,7 @@ describe MergeRequests::CreateFromIssueService do
end end
it 'creates the new_issue_branch system note when the branch could be created but the merge_request cannot be created' do it 'creates the new_issue_branch system note when the branch could be created but the merge_request cannot be created' do
project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED) expect_any_instance_of(MergeRequest).to receive(:valid?).at_least(:once).and_return(false)
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name) expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name)
...@@ -124,7 +123,7 @@ describe MergeRequests::CreateFromIssueService do ...@@ -124,7 +123,7 @@ describe MergeRequests::CreateFromIssueService do
end end
context 'when ref branch is set' do context 'when ref branch is set' do
subject { described_class.new(project, user, issue_iid: issue.iid, ref: 'feature').execute } subject { described_class.new(project, user, service_params.merge(ref: 'feature')).execute }
it 'sets the merge request source branch to the new issue branch' do it 'sets the merge request source branch to the new issue branch' do
expect(subject[:merge_request].source_branch).to eq(issue.to_branch_name) expect(subject[:merge_request].source_branch).to eq(issue.to_branch_name)
...@@ -135,7 +134,7 @@ describe MergeRequests::CreateFromIssueService do ...@@ -135,7 +134,7 @@ describe MergeRequests::CreateFromIssueService do
end end
context 'when ref branch does not exist' do context 'when ref branch does not exist' do
subject { described_class.new(project, user, issue_iid: issue.iid, ref: 'no-such-branch').execute } subject { described_class.new(project, user, service_params.merge(ref: 'no-such-branch')).execute }
it 'creates a merge request' do it 'creates a merge request' do
expect { subject }.to change(project.merge_requests, :count).by(1) expect { subject }.to change(project.merge_requests, :count).by(1)
...@@ -147,4 +146,117 @@ describe MergeRequests::CreateFromIssueService do ...@@ -147,4 +146,117 @@ describe MergeRequests::CreateFromIssueService do
end end
end end
end end
context 'target_project_id is specified' do
let(:service_params) { { issue_iid: issue.iid, target_project_id: target_project.id } }
context 'target project is not a fork of the project' do
let(:target_project) { create(:project, :repository) }
it 'does not create merge request' do
expect { service.execute }.to change(target_project.merge_requests, :count).by(0)
end
end
context 'target project is a fork of project project' do
let(:target_project) { fork_project(project, user, repository: true) }
it 'creates a branch based on issue title' do
service.execute
expect(target_project.repository.branch_exists?(issue.to_branch_name)).to be_truthy
end
it 'creates a branch using passed name' do
service_with_custom_source_branch.execute
expect(target_project.repository.branch_exists?(custom_source_branch)).to be_truthy
end
it 'creates the new_merge_request system note' do
expect(SystemNoteService).to receive(:new_merge_request).with(issue, target_project, user, instance_of(MergeRequest))
service.execute
end
it 'creates the new_issue_branch system note when the branch could be created but the merge_request cannot be created' do
expect_any_instance_of(MergeRequest).to receive(:valid?).at_least(:once).and_return(false)
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, target_project, user, issue.to_branch_name)
service.execute
end
it 'creates a merge request' do
expect { service.execute }.to change(target_project.merge_requests, :count).by(1)
end
it 'sets the merge request title to: "WIP: $issue-branch-name' do
result = service.execute
expect(result[:merge_request].title).to eq("WIP: #{issue.to_branch_name.titleize.humanize}")
end
it 'sets the merge request author to current user' do
result = service.execute
expect(result[:merge_request].author).to eq(user)
end
it 'sets the merge request source branch to the new issue branch' do
result = service.execute
expect(result[:merge_request].source_branch).to eq(issue.to_branch_name)
end
it 'sets the merge request source branch to the passed branch name' do
result = service_with_custom_source_branch.execute
expect(result[:merge_request].source_branch).to eq(custom_source_branch)
end
it 'sets the merge request target branch to the project default branch' do
result = service.execute
expect(result[:merge_request].target_branch).to eq(project.default_branch)
end
it 'executes quick actions if the build service sets them in the description' do
allow(service).to receive(:merge_request).and_wrap_original do |m, *args|
m.call(*args).tap do |merge_request|
merge_request.description = "/assign #{user.to_reference}"
end
end
result = service.execute
expect(result[:merge_request].assignees).to eq([user])
end
context 'when ref branch is set' do
subject { described_class.new(project, user, service_params.merge(ref: 'feature')).execute }
it 'sets the merge request source branch to the new issue branch' do
expect(subject[:merge_request].source_branch).to eq(issue.to_branch_name)
end
it 'sets the merge request target branch to the ref branch' do
expect(subject[:merge_request].target_branch).to eq('feature')
end
context 'when ref branch does not exist' do
subject { described_class.new(project, user, service_params.merge(ref: 'no-such-branch')).execute }
it 'creates a merge request' do
expect { subject }.to change(target_project.merge_requests, :count).by(1)
end
it 'sets the merge request target branch to the project default branch' do
expect(subject[:merge_request].target_branch).to eq(target_project.default_branch)
end
end
end
end
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