Commit 70716a12 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Allow creating merge requests across forks of a project

parent d3280072
class MergeRequestTargetProjectFinder
attr_reader :current_user, :source_project
def initialize(current_user: nil, source_project:, params: {})
@current_user = current_user
@source_project = source_project
end
def execute
if @source_project.fork_network
@source_project.fork_network.projects
.public_or_visible_to_user(current_user)
.with_feature_available_for_user(:merge_requests, current_user)
else
Project.where(id: source_project)
end
end
end
......@@ -73,7 +73,8 @@ module MergeRequestsHelper
end
def target_projects(project)
[project, project.default_merge_request_target].uniq
MergeRequestTargetProjectFinder.new(current_user: current_user, source_project: project)
.execute
end
def merge_request_button_visibility(merge_request, closed)
......
......@@ -403,7 +403,7 @@ class MergeRequest < ActiveRecord::Base
return false unless for_fork?
return true unless source_project
!source_project.forked_from?(target_project)
!source_project.in_fork_network_of?(target_project)
end
def reopenable?
......
......@@ -1128,8 +1128,19 @@ class Project < ActiveRecord::Base
end
end
def in_fork_network_of?(project)
forked? && project.fork_network == fork_network
def forked_from?(other_project)
forked? && forked_from_project == other_project
end
def in_fork_network_of?(other_project)
# TODO: Remove this in a next release when all fork_networks are populated
# This makes sure all MergeRequests remain valid while the projects don't
# have a fork_network yet.
  • @reprazent hey, I was going through the code and saw this comment (in master). Do you think we can address it now?

Please register or sign in to reply
return true if forked_from?(other_project)
return false if fork_network.nil? || other_project.fork_network.nil?
fork_network == other_project.fork_network
end
def origin_merge_requests
......
---
title: Allow creating merge requests across a fork network
merge_request: 14422
author:
type: changed
......@@ -18,7 +18,7 @@ class CreateForkNetworkMembers < ActiveRecord::Migration
end
def down
if foreign_key_exists?(:fork_network_members, column: :forked_from_project_id)
if foreign_keys_for(:fork_network_members, :forked_from_project_id).any?
remove_foreign_key :fork_network_members, column: :forked_from_project_id
end
drop_table :fork_network_members
......
......@@ -23,7 +23,8 @@ module Gitlab
@extractor.analyze(closing_statements.join(" "))
@extractor.issues.reject do |issue|
@extractor.project.forked_from?(issue.project) # Don't extract issues on original project
# Don't extract issues from the project this project was forked from
@extractor.project.forked_from?(issue.project)
end
end
end
......
require 'spec_helper'
feature 'Creating a merge request from a fork', :js do
let(:user) { create(:user) }
let(:project) { create(:project, :public, :repository) }
let!(:source_project) { ::Projects::ForkService.new(project, user).execute }
before do
source_project.add_master(user)
sign_in(user)
end
shared_examples 'create merge request to other project' do
it 'has all possible target projects' do
visit project_new_merge_request_path(source_project)
first('.js-target-project').click
within('.dropdown-target-project .dropdown-content') do
expect(page).to have_content(project.full_path)
expect(page).to have_content(target_project.full_path)
expect(page).to have_content(source_project.full_path)
end
end
it 'allows creating the merge request to another target project' do
visit project_merge_requests_path(source_project)
page.within '.content' do
click_link 'New merge request'
end
find('.js-source-branch', match: :first).click
find('.dropdown-source-branch .dropdown-content a', match: :first).click
first('.js-target-project').click
find('.dropdown-target-project .dropdown-content a', text: target_project.full_path).click
click_button 'Compare branches and continue'
wait_for_requests
expect { click_button 'Submit merge request' }
.to change { target_project.merge_requests.reload.size }.by(1)
end
end
context 'creating to the source of a fork' do
let(:target_project) { project }
it_behaves_like('create merge request to other project')
end
context 'creating to a sibling of a fork' do
let!(:target_project) { ::Projects::ForkService.new(project, create(:user)).execute }
it_behaves_like('create merge request to other project')
end
end
require 'spec_helper'
describe MergeRequestTargetProjectFinder do
include ProjectForksHelper
let(:user) { create(:user) }
subject(:finder) { described_class.new(current_user: user, source_project: forked_project) }
shared_examples 'finding related projects' do
it 'finds sibling projects and base project' do
other_fork
expect(finder.execute).to contain_exactly(base_project, other_fork, forked_project)
end
it 'does not include projects that have merge requests turned off' do
other_fork.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
base_project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
expect(finder.execute).to contain_exactly(forked_project)
end
end
context 'public projects' do
let(:base_project) { create(:project, :public, path: 'base') }
let(:forked_project) { fork_project(base_project) }
let(:other_fork) { fork_project(base_project) }
it_behaves_like 'finding related projects'
end
context 'private projects' do
let(:base_project) { create(:project, :private, path: 'base') }
let(:forked_project) { fork_project(base_project, base_project.owner) }
let(:other_fork) { fork_project(base_project, base_project.owner) }
context 'when the user is a member of all projects' do
before do
base_project.add_developer(user)
forked_project.add_developer(user)
other_fork.add_developer(user)
end
it_behaves_like 'finding related projects'
end
it 'only finds the projects the user is a member of' do
other_fork.add_developer(user)
base_project.add_developer(user)
expect(finder.execute).to contain_exactly(other_fork, base_project)
end
end
end
......@@ -49,6 +49,19 @@ describe MergeRequest do
expect(subject).to be_valid
end
end
context 'for forks' do
let(:project) { create(:project) }
let(:fork1) { create(:forked_project_link, forked_from_project: project).forked_to_project }
let(:fork2) { create(:forked_project_link, forked_from_project: project).forked_to_project }
it 'allows merge requests for sibling-forks' do
subject.source_project = fork1
subject.target_project = fork2
expect(subject).to be_valid
end
end
end
describe 'respond to' do
......@@ -1425,7 +1438,7 @@ describe MergeRequest do
describe "#source_project_missing?" do
let(:project) { create(:project) }
let(:fork_project) { create(:project, forked_from_project: project) }
let(:fork_project) { create(:forked_project_link, forked_from_project: project).forked_to_project }
let(:user) { create(:user) }
let(:unlink_project) { Projects::UnlinkForkService.new(fork_project, user) }
......@@ -1446,7 +1459,7 @@ describe MergeRequest do
end
context "when the fork does not exist" do
let(:merge_request) do
let!(:merge_request) do
create(:merge_request,
source_project: fork_project,
target_project: project)
......
......@@ -1818,6 +1818,65 @@ describe Project do
end
end
context 'forks' do
let(:project) { create(:project, :public) }
let!(:forked_project) { fork_project(project) }
def fork_project(project)
Projects::ForkService.new(project, create(:user)).execute
end
describe '#fork_network' do
it 'includes a fork of the project' do
expect(project.fork_network).to include(forked_project)
end
it 'includes a fork of a fork' do
other_fork = fork_project(forked_project)
expect(project.fork_network).to include(other_fork)
end
it 'includes sibling forks' do
other_fork = fork_project(project)
expect(forked_project.fork_network).to include(other_fork)
end
it 'includes the base project' do
expect(forked_project.fork_network).to include(project)
end
end
describe '#in_fork_network_of?' do
it 'is false when the project is not a fork' do
expect(project.in_fork_network_of?(double)).to be_falsy
end
it 'is true for a real fork' do
expect(forked_project.in_fork_network_of?(project)).to be_truthy
end
it 'is true for a fork of a fork', :postgresql do
other_fork = fork_project(forked_project)
expect(other_fork.in_fork_network_of?(project)).to be_truthy
end
it 'is true for sibling forks' do
sibling = fork_project(project)
expect(sibling.in_fork_network_of?(forked_project)).to be_truthy
end
it 'is false when another project is given' do
other_project = build_stubbed(:project)
expect(forked_project.in_fork_network_of?(other_project)).to be_falsy
end
end
end
describe '#pushes_since_gc' do
let(:project) { create(:project) }
......
......@@ -676,16 +676,6 @@ describe API::MergeRequests do
end
context 'when target_branch is specified' do
it 'returns 422 if not a forked project' do
post api("/projects/#{project.id}/merge_requests", user),
title: 'Test merge_request',
target_branch: 'master',
source_branch: 'markdown',
author: user,
target_project_id: fork_project.id
expect(response).to have_gitlab_http_status(422)
end
it 'returns 422 if targeting a different fork' do
post api("/projects/#{fork_project.id}/merge_requests", user2),
title: 'Test merge_request',
......
......@@ -372,16 +372,6 @@ describe API::MergeRequests do
end
context 'when target_branch is specified' do
it 'returns 422 if not a forked project' do
post v3_api("/projects/#{project.id}/merge_requests", user),
title: 'Test merge_request',
target_branch: 'master',
source_branch: 'markdown',
author: user,
target_project_id: fork_project.id
expect(response).to have_gitlab_http_status(422)
end
it 'returns 422 if targeting a different fork' do
post v3_api("/projects/#{fork_project.id}/merge_requests", user2),
title: 'Test merge_request',
......
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