Allow creating merge requests across forks of a project

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',