diff options
| -rw-r--r-- | app/finders/merge_request_target_project_finder.rb | 18 | ||||
| -rw-r--r-- | app/helpers/merge_requests_helper.rb | 3 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 2 | ||||
| -rw-r--r-- | app/models/project.rb | 15 | ||||
| -rw-r--r-- | changelogs/unreleased/bvl-fork-network-schema.yml | 5 | ||||
| -rw-r--r-- | db/migrate/20170928133643_create_fork_network_members.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/closing_issue_extractor.rb | 3 | ||||
| -rw-r--r-- | spec/features/merge_requests/create_new_mr_from_fork_spec.rb | 60 | ||||
| -rw-r--r-- | spec/finders/merge_request_target_project_finder_spec.rb | 54 | ||||
| -rw-r--r-- | spec/models/merge_request_spec.rb | 17 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 59 | ||||
| -rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 10 | ||||
| -rw-r--r-- | spec/requests/api/v3/merge_requests_spec.rb | 10 |
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', |