Merge branch 'dz-project-setting-default-target-project' into 'master'

Add setting to change default MR target project

See merge request gitlab-org/gitlab!58093

app/controllers/projects/merge_requests/creations_controller.rb

@@ -121,13 +121,15 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
 
   # rubocop: disable CodeReuse/ActiveRecord
   def selected_target_project
-    if @project.id.to_s == params[:target_project_id] || !@project.forked?
-      @project
-    elsif params[:target_project_id].present?
+    return @project unless @project.forked?
+
+    if params[:target_project_id].present?
+      return @project if @project.id.to_s == params[:target_project_id]
+
       MergeRequestTargetProjectFinder.new(current_user: current_user, source_project: @project)
         .find_by(id: params[:target_project_id])
     else
-      @project.forked_from_project
+      @project.default_merge_request_target
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/controllers/projects_controller.rb

@@ -404,6 +404,7 @@ class ProjectsController < Projects::ApplicationController
       show_default_award_emojis
       squash_option
       allow_editing_commit_messages
+      mr_default_target_self
     ]
   end
 

app/models/project.rb

@@ -2172,17 +2172,18 @@ class Project < ApplicationRecord
   end
 
   def default_merge_request_target
-    return self unless forked_from_project
-    return self unless forked_from_project.merge_requests_enabled?
-
-    # When our current visibility is more restrictive than the source project,
-    # (e.g., the fork is `private` but the parent is `public`), target the less
-    # permissive project
-    if visibility_level_value < forked_from_project.visibility_level_value
-      self
-    else
-      forked_from_project
-    end
+    return self if project_setting.mr_default_target_self
+    return self unless mr_can_target_upstream?
+
+    forked_from_project
+  end
+
+  def mr_can_target_upstream?
+    # When our current visibility is more restrictive than the upstream project,
+    # (e.g., the fork is `private` but the parent is `public`), don't allow target upstream
+    forked_from_project &&
+      forked_from_project.merge_requests_enabled? &&
+      forked_from_project.visibility_level_value <= visibility_level_value
   end
 
   def multiple_issue_boards_available?

app/views/projects/_merge_request_settings.html.haml

@@ -9,3 +9,6 @@
 = render 'projects/merge_request_merge_checks_settings', project: @project, form: form
 
 = render 'projects/merge_request_merge_suggestions_settings', project: @project, form: form
+
+- if @project.forked?
+  = render 'projects/merge_request_target_project_settings', project: @project, form: form

app/views/projects/_merge_request_target_project_settings.html.haml

+- return unless @project.mr_can_target_upstream? && can?(current_user, :read_project, @project.forked_from_project)
+
+- form = local_assigns.fetch(:form)
+
+= form.fields_for :project_setting do |settings|
+  .form-group
+    %b= s_('ProjectSettings|Target project')
+    %p.text-secondary
+      = s_('ProjectSettings|The default target project for merge requests created in this fork project.')
+
+    .form-check.gl-mb-2
+      = settings.radio_button :mr_default_target_self, false, class: "form-check-input"
+      = label_tag :project_project_setting_attributes_mr_default_target_self_false, class: 'form-check-label' do
+        .gl-font-weight-bold
+          = s_('ProjectSettings|Upstream project')
+        = @project.forked_from_project.full_name
+
+    .form-check.gl-mb-2
+      = settings.radio_button :mr_default_target_self, true, class: "form-check-input"
+      = label_tag :project_project_setting_attributes_mr_default_target_self_true, class: 'form-check-label' do
+        .gl-font-weight-bold
+          = s_('ProjectSettings|This project')
+        = @project.full_name

changelogs/unreleased/dz-project-setting-default-target-project.yml

+---
+title: Add setting to change default target project for merge requests from forks
+merge_request: 58093
+author:
+type: added

db/migrate/20210331125111_add_default_target_project.rb

+# frozen_string_literal: true
+
+class AddDefaultTargetProject < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  def up
+    with_lock_retries do
+      add_column :project_settings, :mr_default_target_self, :boolean, default: false, null: false
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :project_settings, :mr_default_target_self
+    end
+  end
+end

db/schema_migrations/20210331125111

+8c7343dafaa036115e85f30d2d096d14279c80de99f49b969039ed3afa5acdf6
\ No newline at end of file

db/structure.sql

@@ -16520,6 +16520,7 @@ CREATE TABLE project_settings (
     allow_editing_commit_messages boolean DEFAULT false NOT NULL,
     prevent_merge_without_jira_issue boolean DEFAULT false NOT NULL,
     cve_id_request_enabled boolean DEFAULT true NOT NULL,
+    mr_default_target_self boolean DEFAULT false NOT NULL,
     CONSTRAINT check_bde223416c CHECK ((show_default_award_emojis IS NOT NULL))
 );
 

ee/app/views/projects/_merge_request_settings.html.haml

@@ -10,6 +10,9 @@
 
 = render 'projects/merge_request_merge_suggestions_settings', project: @project, form: form
 
+- if @project.forked?
+  = render_ce 'projects/merge_request_target_project_settings', project: @project, form: form
+
 - if @project.feature_available?(:issuable_default_templates)
   .form-group
     %b= _('Default description template for merge requests')

locale/gitlab.pot

@@ -24696,12 +24696,21 @@ msgstr ""
 msgid "ProjectSettings|Supported variables:"
 msgstr ""
 
+msgid "ProjectSettings|Target project"
+msgstr ""
+
 msgid "ProjectSettings|The commit message used when applying merge request suggestions. %{link_start}Learn more about suggestions.%{link_end}"
 msgstr ""
 
+msgid "ProjectSettings|The default target project for merge requests created in this fork project."
+msgstr ""
+
 msgid "ProjectSettings|These checks must pass before merge requests can be merged."
 msgstr ""
 
+msgid "ProjectSettings|This project"
+msgstr ""
+
 msgid "ProjectSettings|This setting is applied on the server level and can be overridden by an admin."
 msgstr ""
 
@@ -24717,6 +24726,9 @@ msgstr ""
 msgid "ProjectSettings|Transfer project"
 msgstr ""
 
+msgid "ProjectSettings|Upstream project"
+msgstr ""
+
 msgid "ProjectSettings|Used for every new merge request."
 msgstr ""
 

spec/controllers/projects/merge_requests/creations_controller_spec.rb

@@ -213,6 +213,38 @@ RSpec.describe Projects::MergeRequests::CreationsController do
       expect(assigns(:commit)).to be_nil
       expect(response).to have_gitlab_http_status(:ok)
     end
+
+    context 'no target_project_id provided' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      it 'selects itself as a target project' do
+        get :branch_to,
+          params: {
+          namespace_id: project.namespace,
+          project_id: project,
+          ref: 'master'
+        }
+
+        expect(assigns(:target_project)).to eq(project)
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+
+      context 'project is a fork' do
+        it 'calls to project defaults to selects a correct target project' do
+          get :branch_to,
+            params: {
+            namespace_id: fork_project.namespace,
+            project_id: fork_project,
+            ref: 'master'
+          }
+
+          expect(assigns(:target_project)).to eq(project)
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+    end
   end
 
   describe 'POST create' do

spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb

@@ -2,6 +2,8 @@
 require 'spec_helper'
 
 RSpec.describe 'Projects > Settings > User manages merge request settings' do
+  include ProjectForksHelper
+
   let(:user) { create(:user) }
   let(:project) { create(:project, :public, namespace: user.namespace, path: 'gitlab', name: 'sample') }
 
@@ -198,4 +200,36 @@ RSpec.describe 'Projects > Settings > User manages merge request settings' do
       expect(project.reload.project_setting.squash_option).to eq('never')
     end
   end
+
+  describe 'target project settings' do
+    context 'when project is a fork' do
+      let_it_be(:upstream) { create(:project, :public) }
+
+      let(:project) { fork_project(upstream, user) }
+
+      it 'allows to change merge request target project behavior' do
+        expect(page).to have_content 'The default target project for merge requests'
+
+        radio = find_field('project_project_setting_attributes_mr_default_target_self_false')
+        expect(radio).to be_checked
+
+        choose('project_project_setting_attributes_mr_default_target_self_true')
+
+        within('.merge-request-settings-form') do
+          find('.rspec-save-merge-request-changes')
+          click_on('Save changes')
+        end
+
+        find('.flash-notice')
+        radio = find_field('project_project_setting_attributes_mr_default_target_self_true')
+
+        expect(radio).to be_checked
+        expect(project.reload.project_setting.mr_default_target_self).to be_truthy
+      end
+    end
+
+    it 'does not show target project section' do
+      expect(page).not_to have_content 'The default target project for merge requests'
+    end
+  end
 end

spec/models/project_spec.rb

@@ -3803,49 +3803,74 @@ RSpec.describe Project, factory_default: :keep do
   end
 
   describe '#default_merge_request_target' do
+    let_it_be(:project) { create(:project, :public) }
+
+    let!(:forked) { fork_project(project) }
+
+    context 'when mr_default_target_self is set to true' do
+      it 'returns the current project' do
+        expect(forked.project_setting).to receive(:mr_default_target_self)
+          .and_return(true)
+
+        expect(forked.default_merge_request_target).to eq(forked)
+      end
+    end
+
+    context 'when merge request can not target upstream' do
+      it 'returns the current project' do
+        expect(forked).to receive(:mr_can_target_upstream?).and_return(false)
+
+        expect(forked.default_merge_request_target).to eq(forked)
+      end
+    end
+
+    context 'when merge request can target upstream' do
+      it 'returns the source project' do
+        expect(forked).to receive(:mr_can_target_upstream?).and_return(true)
+
+        expect(forked.default_merge_request_target).to eq(project)
+      end
+    end
+  end
+
+  describe '#mr_can_target_upstream?' do
+    let_it_be(:project) { create(:project, :public) }
+
+    let!(:forked) { fork_project(project) }
+
     context 'when forked from a more visible project' do
-      it 'returns the more restrictive project' do
-        project = create(:project, :public)
-        forked = fork_project(project)
+      it 'can not target the upstream project' do
         forked.visibility = Gitlab::VisibilityLevel::PRIVATE
         forked.save!
 
         expect(project.visibility).to eq 'public'
         expect(forked.visibility).to eq 'private'
 
-        expect(forked.default_merge_request_target).to eq(forked)
+        expect(forked.mr_can_target_upstream?).to be_falsey
       end
     end
 
     context 'when forked from a project with disabled merge requests' do
-      it 'returns the current project' do
-        project = create(:project, :merge_requests_disabled)
-        forked = fork_project(project)
+      it 'can not target the upstream project' do
+        project.project_feature
+          .update!(merge_requests_access_level: ProjectFeature::DISABLED)
 
         expect(forked.forked_from_project).to receive(:merge_requests_enabled?)
           .and_call_original
 
-        expect(forked.default_merge_request_target).to eq(forked)
+        expect(forked.mr_can_target_upstream?).to be_falsey
       end
     end
 
     context 'when forked from a project with enabled merge requests' do
-      it 'returns the source project' do
-        project = create(:project, :public)
-        forked = fork_project(project)
-
-        expect(project.visibility).to eq 'public'
-        expect(forked.visibility).to eq 'public'
-
-        expect(forked.default_merge_request_target).to eq(project)
+      it 'can target the upstream project' do
+        expect(forked.mr_can_target_upstream?).to be_truthy
       end
     end
 
     context 'when not forked' do
-      it 'returns the current project' do
-        project = build_stubbed(:project)
-
-        expect(project.default_merge_request_target).to eq(project)
+      it 'can not target the upstream project' do
+        expect(project.mr_can_target_upstream?).to be_falsey
       end
     end
   end

spec/requests/api/project_attributes.yml

@@ -140,6 +140,7 @@ project_setting:
     - squash_option
     - updated_at
     - cve_id_request_enabled
+    - mr_default_target_self
 
 build_service_desk_setting: # service_desk_setting
   unexposed_attributes: