diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 6be0b4654020355f29701e7193fd57a8698e8eae..133055cc3172572f84251b949af0407f194f5545 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -6,6 +6,7 @@ class Projects::CompareController < Projects::ApplicationController include DiffForPath include DiffHelper include RendersCommits + include CompareHelper # Authorize before_action :require_non_empty_project @@ -37,16 +38,18 @@ def diff_for_path end def create - if params[:from].blank? || params[:to].blank? + from_to_vars = { + from: params[:from].presence, + to: params[:to].presence, + from_project_id: params[:from_project_id].presence + } + + if from_to_vars[:from].blank? || from_to_vars[:to].blank? flash[:alert] = "You must select a Source and a Target revision" - from_to_vars = { - from: params[:from].presence, - to: params[:to].presence - } - redirect_to project_compare_index_path(@project, from_to_vars) + + redirect_to project_compare_index_path(source_project, from_to_vars) else - redirect_to project_compare_path(@project, - params[:from], params[:to]) + redirect_to project_compare_path(source_project, from_to_vars) end end @@ -73,13 +76,34 @@ def validate_refs! return if valid.all? flash[:alert] = "Invalid branch name" - redirect_to project_compare_index_path(@project) + redirect_to project_compare_index_path(source_project) + end + + # target == start_ref == from + def target_project + strong_memoize(:target_project) do + next source_project unless params.key?(:from_project_id) + next source_project unless Feature.enabled?(:compare_repo_dropdown, source_project, default_enabled: :yaml) + next source_project if params[:from_project_id].to_i == source_project.id + + target_project = target_projects(source_project).find_by_id(params[:from_project_id]) + + # Just ignore the field if it points at a non-existent or hidden project + next source_project unless target_project && can?(current_user, :download_code, target_project) + + target_project + end + end + + # source == head_ref == to + def source_project + project end def compare return @compare if defined?(@compare) - @compare = CompareService.new(@project, head_ref).execute(@project, start_ref) + @compare = CompareService.new(source_project, head_ref).execute(target_project, start_ref) end def start_ref @@ -102,9 +126,9 @@ def define_diffs def define_environment if compare - environment_params = @repository.branch_exists?(head_ref) ? { ref: head_ref } : { commit: compare.commit } + environment_params = source_project.repository.branch_exists?(head_ref) ? { ref: head_ref } : { commit: compare.commit } environment_params[:find_latest] = true - @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last + @environment = EnvironmentsFinder.new(source_project, current_user, environment_params).execute.last end end @@ -114,8 +138,8 @@ def define_diff_notes_disabled # rubocop: disable CodeReuse/ActiveRecord def merge_request - @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened - .find_by(source_project: @project, source_branch: head_ref, target_branch: start_ref) + @merge_request ||= MergeRequestsFinder.new(current_user, project_id: target_project.id).execute.opened + .find_by(source_project: source_project, source_branch: head_ref, target_branch: start_ref) end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/finders/merge_request_target_project_finder.rb b/app/finders/merge_request_target_project_finder.rb index 85a73e0c6ffc08595e22c2f35fc970187696fdf8..dc9b28ab0a04ca5e5447d97e5028f28a5ff72e33 100644 --- a/app/finders/merge_request_target_project_finder.rb +++ b/app/finders/merge_request_target_project_finder.rb @@ -5,29 +5,30 @@ class MergeRequestTargetProjectFinder attr_reader :current_user, :source_project - def initialize(current_user: nil, source_project:) + def initialize(current_user: nil, source_project:, project_feature: :merge_requests) @current_user = current_user @source_project = source_project + @project_feature = project_feature end - # rubocop: disable CodeReuse/ActiveRecord def execute(include_routes: false) if source_project.fork_network include_routes ? projects.inc_routes : projects else - Project.where(id: source_project) + Project.id_in(source_project.id) end end - # rubocop: enable CodeReuse/ActiveRecord private + attr_reader :project_feature + def projects source_project .fork_network .projects .public_or_visible_to_user(current_user) .non_archived - .with_feature_available_for_user(:merge_requests, current_user) + .with_feature_available_for_user(project_feature, current_user) end end diff --git a/app/helpers/compare_helper.rb b/app/helpers/compare_helper.rb index 9ece8b0bc5bb81d294c130fce51d614238f08ac5..c14aa44ced166aa6f42535f6599fe553b99d8d7c 100644 --- a/app/helpers/compare_helper.rb +++ b/app/helpers/compare_helper.rb @@ -1,22 +1,31 @@ # frozen_string_literal: true module CompareHelper - def create_mr_button?(from = params[:from], to = params[:to], project = @project) + def create_mr_button?(from: params[:from], to: params[:to], source_project: @project, target_project: @target_project) from.present? && to.present? && from != to && - can?(current_user, :create_merge_request_from, project) && - project.repository.branch_exists?(from) && - project.repository.branch_exists?(to) + can?(current_user, :create_merge_request_from, source_project) && + can?(current_user, :create_merge_request_in, target_project) && + target_project.repository.branch_exists?(from) && + source_project.repository.branch_exists?(to) end - def create_mr_path(from = params[:from], to = params[:to], project = @project) + def create_mr_path(from: params[:from], to: params[:to], source_project: @project, target_project: @target_project) project_new_merge_request_path( - project, + target_project, merge_request: { + source_project_id: source_project.id, source_branch: to, + target_project_id: target_project.id, target_branch: from } ) end + + def target_projects(source_project) + MergeRequestTargetProjectFinder + .new(current_user: current_user, source_project: source_project, project_feature: :repository) + .execute(include_routes: true) + end end diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index d6662e1fc318130473c0d445766e90123763faa5..97dd606855b4462fcd27c287cf3f7a03b1fa2bec 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -21,7 +21,8 @@ %ul.content-list.event_commits = render "events/commit", project: project, event: event - - create_mr = event.new_ref? && create_mr_button?(project.default_branch, event.ref_name, project) && event.authored_by?(current_user) + - create_mr = event.new_ref? && create_mr_button?(from: project.default_branch, to: event.ref_name, source_project: project, target_project: project) && event.authored_by?(current_user) + - create_mr_path = create_mr_path(from: project.default_branch, to: event.ref_name, source_project: project, target_project: project) if create_mr - if event.commits_count > 1 %li.commits-stat %span ... and #{pluralize(event.commits_count - 1, 'more commit')}. @@ -40,9 +41,9 @@ - if create_mr %span or - = link_to create_mr_path(project.default_branch, event.ref_name, project) do + = link_to create_mr_path do create a merge request - elsif create_mr %li.commits-stat - = link_to create_mr_path(project.default_branch, event.ref_name, project) do + = link_to create_mr_path do Create Merge Request diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index fcf073e1e094cba41f67ec1e0a4db78fa12d1eb1..6f86ccd7824f1f48b299adf33a859809e278a339 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -35,8 +35,8 @@ .gl-display-inline-flex.gl-vertical-align-middle.gl-mr-5 %svg.s24 - - if merge_project && create_mr_button?(@repository.root_ref, branch.name) - = link_to create_mr_path(@repository.root_ref, branch.name), class: 'gl-button btn btn-default' do + - if merge_project && create_mr_button?(from: @repository.root_ref, to: branch.name, source_project: @project, target_project: @project) + = link_to create_mr_path(from: @repository.root_ref, to: branch.name, source_project: @project, target_project: @project), class: 'gl-button btn btn-default' do = _('Merge request') - if branch.name != @repository.root_ref diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml index 802df664241532f99e18c5f008eb92d6ba2990df..463984a13a2a65b2643067b268fdd9acf7605e75 100644 --- a/app/views/projects/commits/show.html.haml +++ b/app/views/projects/commits/show.html.haml @@ -18,9 +18,9 @@ - if @merge_request.present? .control.d-none.d-md-block = link_to _("View open merge request"), project_merge_request_path(@project, @merge_request), class: 'btn gl-button' - - elsif create_mr_button?(@repository.root_ref, @ref) + - elsif create_mr_button?(from: @repository.root_ref, to: @ref, source_project: @project, target_project: @project) .control.d-none.d-md-block - = link_to _("Create merge request"), create_mr_path(@repository.root_ref, @ref), class: 'btn gl-button btn-success' + = link_to _("Create merge request"), create_mr_path(from: @repository.root_ref, to: @ref, source_project: @project, target_project: @project), class: 'btn gl-button btn-success' .control = form_tag(project_commits_path(@project, @id), method: :get, class: 'commits-search-form js-signature-container', data: { 'signatures-path' => namespace_project_signatures_path }) do diff --git a/config/feature_flags/development/compare_repo_dropdown.yml b/config/feature_flags/development/compare_repo_dropdown.yml new file mode 100644 index 0000000000000000000000000000000000000000..b47bce8c521ea1d05fd2d60dea85cc20c4c18653 --- /dev/null +++ b/config/feature_flags/development/compare_repo_dropdown.yml @@ -0,0 +1,8 @@ +--- +name: compare_repo_dropdown +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/14615 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/322141 +milestone: '13.9' +type: development +group: group::source code +default_enabled: false diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 6aa4bfe235bd1571b82bb040d5994671f84b6405..80a6d3960cd69e96506c7516381be9f909afd5dd 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -3,8 +3,21 @@ require 'spec_helper' RSpec.describe Projects::CompareController do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } + include ProjectForksHelper + + using RSpec::Parameterized::TableSyntax + + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:user) { create(:user) } + + let(:private_fork) { fork_project(project, nil, repository: true).tap { |fork| fork.update!(visibility: 'private') } } + let(:public_fork) do + fork_project(project, nil, repository: true).tap do |fork| + fork.update!(visibility: 'public') + # Create a reference that only exists in this project + fork.repository.create_ref('refs/heads/improve/awesome', 'refs/heads/improve/more-awesome') + end + end before do sign_in(user) @@ -32,18 +45,20 @@ { namespace_id: project.namespace, project_id: project, - from: source_ref, - to: target_ref, + from_project_id: from_project_id, + from: from_ref, + to: to_ref, w: whitespace } end let(:whitespace) { nil } - context 'when the refs exist' do + context 'when the refs exist in the same project' do context 'when we set the white space param' do - let(:source_ref) { "08f22f25" } - let(:target_ref) { "66eceea0" } + let(:from_project_id) { nil } + let(:from_ref) { '08f22f25' } + let(:to_ref) { '66eceea0' } let(:whitespace) { 1 } it 'shows some diffs with ignore whitespace change option' do @@ -60,8 +75,9 @@ end context 'when we do not set the white space param' do - let(:source_ref) { "improve%2Fawesome" } - let(:target_ref) { "feature" } + let(:from_project_id) { nil } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'feature' } let(:whitespace) { nil } it 'sets the diffs and commits ivars' do @@ -74,9 +90,40 @@ end end + context 'when the refs exist in different projects that the user can see' do + let(:from_project_id) { public_fork.id } + let(:from_ref) { 'improve%2Fmore-awesome' } + let(:to_ref) { 'feature' } + let(:whitespace) { nil } + + it 'shows the diff' do + show_request + + expect(response).to be_successful + expect(assigns(:diffs).diff_files.first).not_to be_nil + expect(assigns(:commits).length).to be >= 1 + end + end + + context 'when the refs exist in different projects but the user cannot see' do + let(:from_project_id) { private_fork.id } + let(:from_ref) { 'improve%2Fmore-awesome' } + let(:to_ref) { 'feature' } + let(:whitespace) { nil } + + it 'does not show the diff' do + show_request + + expect(response).to be_successful + expect(assigns(:diffs)).to be_empty + expect(assigns(:commits)).to be_empty + end + end + context 'when the source ref does not exist' do - let(:source_ref) { 'non-existent-source-ref' } - let(:target_ref) { "feature" } + let(:from_project_id) { nil } + let(:from_ref) { 'non-existent-source-ref' } + let(:to_ref) { 'feature' } it 'sets empty diff and commit ivars' do show_request @@ -88,8 +135,9 @@ end context 'when the target ref does not exist' do - let(:target_ref) { 'non-existent-target-ref' } - let(:source_ref) { "improve%2Fawesome" } + let(:from_project_id) { nil } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'non-existent-target-ref' } it 'sets empty diff and commit ivars' do show_request @@ -101,8 +149,9 @@ end context 'when the target ref is invalid' do - let(:target_ref) { "master%' AND 2554=4423 AND '%'='" } - let(:source_ref) { "improve%2Fawesome" } + let(:from_project_id) { nil } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { "master%' AND 2554=4423 AND '%'='" } it 'shows a flash message and redirects' do show_request @@ -113,8 +162,9 @@ end context 'when the source ref is invalid' do - let(:source_ref) { "master%' AND 2554=4423 AND '%'='" } - let(:target_ref) { "improve%2Fawesome" } + let(:from_project_id) { nil } + let(:from_ref) { "master%' AND 2554=4423 AND '%'='" } + let(:to_ref) { 'improve%2Fawesome' } it 'shows a flash message and redirects' do show_request @@ -126,24 +176,33 @@ end describe 'GET diff_for_path' do - def diff_for_path(extra_params = {}) - params = { + subject(:diff_for_path_request) { get :diff_for_path, params: request_params } + + let(:request_params) do + { + from_project_id: from_project_id, + from: from_ref, + to: to_ref, namespace_id: project.namespace, - project_id: project + project_id: project, + old_path: old_path, + new_path: new_path } - - get :diff_for_path, params: params.merge(extra_params) end let(:existing_path) { 'files/ruby/feature.rb' } - let(:source_ref) { "improve%2Fawesome" } - let(:target_ref) { "feature" } - context 'when the source and target refs exist' do + let(:from_project_id) { nil } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'feature' } + let(:old_path) { existing_path } + let(:new_path) { existing_path } + + context 'when the source and target refs exist in the same project' do context 'when the user has access target the project' do context 'when the path exists in the diff' do it 'disables diff notes' do - diff_for_path(from: source_ref, to: target_ref, old_path: existing_path, new_path: existing_path) + diff_for_path_request expect(assigns(:diff_notes_disabled)).to be_truthy end @@ -154,16 +213,17 @@ def diff_for_path(extra_params = {}) meth.call(diffs) end - diff_for_path(from: source_ref, to: target_ref, old_path: existing_path, new_path: existing_path) + diff_for_path_request end end context 'when the path does not exist in the diff' do - before do - diff_for_path(from: source_ref, to: target_ref, old_path: existing_path.succ, new_path: existing_path.succ) - end + let(:old_path) { existing_path.succ } + let(:new_path) { existing_path.succ } it 'returns a 404' do + diff_for_path_request + expect(response).to have_gitlab_http_status(:not_found) end end @@ -172,31 +232,56 @@ def diff_for_path(extra_params = {}) context 'when the user does not have access target the project' do before do project.team.truncate - diff_for_path(from: source_ref, to: target_ref, old_path: existing_path, new_path: existing_path) end it 'returns a 404' do + diff_for_path_request + expect(response).to have_gitlab_http_status(:not_found) end end end - context 'when the source ref does not exist' do - before do - diff_for_path(from: source_ref.succ, to: target_ref, old_path: existing_path, new_path: existing_path) + context 'when the source and target refs exist in different projects and the user can see' do + let(:from_project_id) { public_fork.id } + let(:from_ref) { 'improve%2Fmore-awesome' } + + it 'shows the diff for that path' do + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs| + expect(diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) + meth.call(diffs) + end + + diff_for_path_request + end + end + + context 'when the source and target refs exist in different projects and the user cannot see' do + let(:from_project_id) { private_fork.id } + + it 'does not show the diff for that path' do + diff_for_path_request + + expect(response).to have_gitlab_http_status(:not_found) end + end + + context 'when the source ref does not exist' do + let(:from_ref) { 'this-ref-does-not-exist' } it 'returns a 404' do + diff_for_path_request + expect(response).to have_gitlab_http_status(:not_found) end end context 'when the target ref does not exist' do - before do - diff_for_path(from: source_ref, to: target_ref.succ, old_path: existing_path, new_path: existing_path) - end + let(:to_ref) { 'this-ref-does-not-exist' } it 'returns a 404' do + diff_for_path_request + expect(response).to have_gitlab_http_status(:not_found) end end @@ -209,53 +294,54 @@ def diff_for_path(extra_params = {}) { namespace_id: project.namespace, project_id: project, - from: source_ref, - to: target_ref + from_project_id: from_project_id, + from: from_ref, + to: to_ref } end context 'when sending valid params' do - let(:source_ref) { "improve%2Fawesome" } - let(:target_ref) { "feature" } + let(:from_ref) { 'awesome%2Ffeature' } + let(:to_ref) { 'feature' } - it 'redirects back to show' do - create_request - - expect(response).to redirect_to(project_compare_path(project, to: target_ref, from: source_ref)) - end - end + context 'without a from_project_id' do + let(:from_project_id) { nil } - context 'when sending invalid params' do - context 'when the source ref is empty and target ref is set' do - let(:source_ref) { '' } - let(:target_ref) { 'master' } - - it 'redirects back to index and preserves the target ref' do + it 'redirects to the show page' do create_request - expect(response).to redirect_to(project_compare_index_path(project, to: target_ref)) + expect(response).to redirect_to(project_compare_path(project, from: from_ref, to: to_ref)) end end - context 'when the target ref is empty and source ref is set' do - let(:source_ref) { 'master' } - let(:target_ref) { '' } + context 'with a from_project_id' do + let(:from_project_id) { 'something or another' } - it 'redirects back to index and preserves source ref' do + it 'redirects to the show page without interpreting from_project_id' do create_request - expect(response).to redirect_to(project_compare_index_path(project, from: source_ref)) + expect(response).to redirect_to(project_compare_path(project, from: from_ref, to: to_ref, from_project_id: from_project_id)) end end + end + + context 'when sending invalid params' do + where(:from_ref, :to_ref, :from_project_id, :expected_redirect_params) do + '' | '' | '' | {} + 'main' | '' | '' | { from: 'main' } + '' | 'main' | '' | { to: 'main' } + '' | '' | '1' | { from_project_id: 1 } + 'main' | '' | '1' | { from: 'main', from_project_id: 1 } + '' | 'main' | '1' | { to: 'main', from_project_id: 1 } + end - context 'when the target and source ref are empty' do - let(:source_ref) { '' } - let(:target_ref) { '' } + with_them do + let(:expected_redirect) { project_compare_index_path(project, expected_redirect_params) } - it 'redirects back to index' do + it 'redirects back to the index' do create_request - expect(response).to redirect_to(namespace_project_compare_index_path) + expect(response).to redirect_to(expected_redirect) end end end @@ -268,15 +354,15 @@ def diff_for_path(extra_params = {}) { namespace_id: project.namespace, project_id: project, - from: source_ref, - to: target_ref, + from: from_ref, + to: to_ref, format: :json } end context 'when the source and target refs exist' do - let(:source_ref) { "improve%2Fawesome" } - let(:target_ref) { "feature" } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'feature' } context 'when the user has access to the project' do render_views @@ -285,14 +371,14 @@ def diff_for_path(extra_params = {}) let(:non_signature_commit) { build(:commit, project: project, safe_message: "message", sha: 'non_signature_commit') } before do - escaped_source_ref = Addressable::URI.unescape(source_ref) - escaped_target_ref = Addressable::URI.unescape(target_ref) + escaped_from_ref = Addressable::URI.unescape(from_ref) + escaped_to_ref = Addressable::URI.unescape(to_ref) - compare_service = CompareService.new(project, escaped_target_ref) - compare = compare_service.execute(project, escaped_source_ref) + compare_service = CompareService.new(project, escaped_to_ref) + compare = compare_service.execute(project, escaped_from_ref) - expect(CompareService).to receive(:new).with(project, escaped_target_ref).and_return(compare_service) - expect(compare_service).to receive(:execute).with(project, escaped_source_ref).and_return(compare) + expect(CompareService).to receive(:new).with(project, escaped_to_ref).and_return(compare_service) + expect(compare_service).to receive(:execute).with(project, escaped_from_ref).and_return(compare) expect(compare).to receive(:commits).and_return([signature_commit, non_signature_commit]) expect(non_signature_commit).to receive(:has_signature?).and_return(false) @@ -313,6 +399,7 @@ def diff_for_path(extra_params = {}) context 'when the user does not have access to the project' do before do project.team.truncate + project.update!(visibility: 'private') end it 'returns a 404' do @@ -324,8 +411,8 @@ def diff_for_path(extra_params = {}) end context 'when the source ref does not exist' do - let(:source_ref) { 'non-existent-ref-source' } - let(:target_ref) { "feature" } + let(:from_ref) { 'non-existent-ref-source' } + let(:to_ref) { 'feature' } it 'returns no signatures' do signatures_request @@ -336,8 +423,8 @@ def diff_for_path(extra_params = {}) end context 'when the target ref does not exist' do - let(:target_ref) { 'non-existent-ref-target' } - let(:source_ref) { "improve%2Fawesome" } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'non-existent-ref-target' } it 'returns no signatures' do signatures_request diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb index e3d8534ace96f23f2bd115bfe6a4c56b07b61c30..2a486e6e5e143633142f2c88afdd55de2d72588e 100644 --- a/spec/features/projects/merge_request_button_spec.rb +++ b/spec/features/projects/merge_request_button_spec.rb @@ -3,11 +3,13 @@ require 'spec_helper' RSpec.describe 'Merge Request button' do - shared_examples 'Merge request button only shown when allowed' do - let(:user) { create(:user) } - let(:project) { create(:project, :public, :repository) } - let(:forked_project) { create(:project, :public, :repository, forked_from_project: project) } + include ProjectForksHelper + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + let(:forked_project) { fork_project(project, user, repository: true) } + shared_examples 'Merge request button only shown when allowed' do context 'not logged in' do it 'does not show Create merge request button' do visit url @@ -23,9 +25,15 @@ end it 'shows Create merge request button' do - href = project_new_merge_request_path(project, - merge_request: { source_branch: 'feature', - target_branch: 'master' }) + href = project_new_merge_request_path( + project, + merge_request: { + source_project_id: project.id, + source_branch: 'feature', + target_project_id: project.id, + target_branch: 'master' + } + ) visit url @@ -75,12 +83,16 @@ end context 'on own fork of project' do - let(:user) { forked_project.owner } - it 'shows Create merge request button' do - href = project_new_merge_request_path(forked_project, - merge_request: { source_branch: 'feature', - target_branch: 'master' }) + href = project_new_merge_request_path( + forked_project, + merge_request: { + source_project_id: forked_project.id, + source_branch: 'feature', + target_project_id: forked_project.id, + target_branch: 'master' + } + ) visit fork_url @@ -101,11 +113,33 @@ end context 'on compare page' do + let(:label) { 'Create merge request' } + it_behaves_like 'Merge request button only shown when allowed' do - let(:label) { 'Create merge request' } let(:url) { project_compare_path(project, from: 'master', to: 'feature') } let(:fork_url) { project_compare_path(forked_project, from: 'master', to: 'feature') } end + + it 'shows the correct merge request button when viewing across forks' do + sign_in(user) + project.add_developer(user) + + href = project_new_merge_request_path( + project, + merge_request: { + source_project_id: forked_project.id, + source_branch: 'feature', + target_project_id: project.id, + target_branch: 'master' + } + ) + + visit project_compare_path(forked_project, from: 'master', to: 'feature', from_project_id: project.id) + + within("#content-body") do + expect(page).to have_link(label, href: href) + end + end end context 'on commits page' do diff --git a/spec/finders/merge_request_target_project_finder_spec.rb b/spec/finders/merge_request_target_project_finder_spec.rb index dfb4d86fbb60ab5de3ffd0f424d16cabe5b0a81e..08fbfd7229aec3ff9bd9278d904aa36f263bc99e 100644 --- a/spec/finders/merge_request_target_project_finder_spec.rb +++ b/spec/finders/merge_request_target_project_finder_spec.rb @@ -16,13 +16,22 @@ 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 + it 'does not include projects that have merge requests turned off by default' 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 + it 'includes projects that have merge requests turned off by default with a more-permissive project feature' do + finder = described_class.new(current_user: user, source_project: forked_project, project_feature: :repository) + + 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(base_project, other_fork, forked_project) + end + it 'does not contain archived projects' do base_project.update!(archived: true)