summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/finders/merge_request_target_project_finder.rb18
-rw-r--r--app/helpers/merge_requests_helper.rb3
-rw-r--r--app/models/merge_request.rb2
-rw-r--r--app/models/project.rb15
-rw-r--r--changelogs/unreleased/bvl-fork-network-schema.yml5
-rw-r--r--db/migrate/20170928133643_create_fork_network_members.rb2
-rw-r--r--lib/gitlab/closing_issue_extractor.rb3
-rw-r--r--spec/features/merge_requests/create_new_mr_from_fork_spec.rb60
-rw-r--r--spec/finders/merge_request_target_project_finder_spec.rb54
-rw-r--r--spec/models/merge_request_spec.rb17
-rw-r--r--spec/models/project_spec.rb59
-rw-r--r--spec/requests/api/merge_requests_spec.rb10
-rw-r--r--spec/requests/api/v3/merge_requests_spec.rb10
13 files changed, 230 insertions, 28 deletions
diff --git a/app/finders/merge_request_target_project_finder.rb b/app/finders/merge_request_target_project_finder.rb
new file mode 100644
index 00000000000..508b53a52c1
--- /dev/null
+++ b/app/finders/merge_request_target_project_finder.rb
@@ -0,0 +1,18 @@
+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
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index c31023f2d9a..5b2c58d193d 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -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)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 086226618e6..9b312f7db6c 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -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?
diff --git a/app/models/project.rb b/app/models/project.rb
index aff8fd768f5..4a883552a8d 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -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.
+ 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
diff --git a/changelogs/unreleased/bvl-fork-network-schema.yml b/changelogs/unreleased/bvl-fork-network-schema.yml
new file mode 100644
index 00000000000..97b2d5acada
--- /dev/null
+++ b/changelogs/unreleased/bvl-fork-network-schema.yml
@@ -0,0 +1,5 @@
+---
+title: Allow creating merge requests across a fork network
+merge_request: 14422
+author:
+type: changed
diff --git a/db/migrate/20170928133643_create_fork_network_members.rb b/db/migrate/20170928133643_create_fork_network_members.rb
index c1d94e7d52e..836f023efdc 100644
--- a/db/migrate/20170928133643_create_fork_network_members.rb
+++ b/db/migrate/20170928133643_create_fork_network_members.rb
@@ -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
diff --git a/lib/gitlab/closing_issue_extractor.rb b/lib/gitlab/closing_issue_extractor.rb
index 243c1f1394d..7e7aaeeaa17 100644
--- a/lib/gitlab/closing_issue_extractor.rb
+++ b/lib/gitlab/closing_issue_extractor.rb
@@ -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
diff --git a/spec/features/merge_requests/create_new_mr_from_fork_spec.rb b/spec/features/merge_requests/create_new_mr_from_fork_spec.rb
new file mode 100644
index 00000000000..515818c5d42
--- /dev/null
+++ b/spec/features/merge_requests/create_new_mr_from_fork_spec.rb
@@ -0,0 +1,60 @@
+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
diff --git a/spec/finders/merge_request_target_project_finder_spec.rb b/spec/finders/merge_request_target_project_finder_spec.rb
new file mode 100644
index 00000000000..c81bfd7932c
--- /dev/null
+++ b/spec/finders/merge_request_target_project_finder_spec.rb
@@ -0,0 +1,54 @@
+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
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 188a0a98ec3..5f3e3c05d78 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -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)
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 760ea7da322..40b64cad475 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -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) }
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index c4f6e97b915..377d1a8f877 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -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',
diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb
index 86f38dd4ec1..acd53fbff66 100644
--- a/spec/requests/api/v3/merge_requests_spec.rb
+++ b/spec/requests/api/v3/merge_requests_spec.rb
@@ -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',