Avoid setting merge request target branch when source if default branchfj-62807-not-prefill-target-branch

In case the source and the target project are the same, the
source branch is the default branch, and the target branch
is not present, we will avoid prefilling the target branch
with the repository default branch. Letting the user decide.

4 files changed, 83 insertions, 53 deletions

diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index b28f80939ae..a1937d24294 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -21,7 +21,8 @@ module MergeRequests
       merge_request.assign_attributes(params.to_h.compact)
 
       merge_request.compare_commits = []
-      merge_request.target_branch = find_target_branch
+      set_merge_request_target_branch
+
       merge_request.can_be_created = projects_and_branches_valid?
 
       # compare branches only if branches are valid, otherwise
@@ -81,8 +82,12 @@ module MergeRequests
       project_from_params
     end
 
-    def find_target_branch
-      target_branch || target_project.default_branch
+    def set_merge_request_target_branch
+      if source_branch_default? && !target_branch_specified?
+        merge_request.target_branch = nil
+      else
+        merge_request.target_branch ||= target_project.default_branch
+      end
     end
 
     def source_branch_specified?
@@ -137,7 +142,15 @@ module MergeRequests
     end
 
     def same_source_and_target?
-      source_project == target_project && target_branch == source_branch
+      same_source_and_target_project? && target_branch == source_branch
+    end
+
+    def source_branch_default?
+      same_source_and_target_project? && source_branch == target_project.default_branch
+    end
+
+    def same_source_and_target_project?
+      source_project == target_project
     end
 
     def source_branch_exists?
diff --git a/app/views/projects/merge_requests/creations/_new_compare.html.haml b/app/views/projects/merge_requests/creations/_new_compare.html.haml
index be01905dd35..c6615b26bc0 100644
--- a/app/views/projects/merge_requests/creations/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/creations/_new_compare.html.haml
@@ -51,7 +51,7 @@
                   selected: f.object.target_project_id
           .merge-request-select.dropdown
             = f.hidden_field :target_branch
-            = dropdown_toggle f.object.target_branch, { toggle: "dropdown", 'field-name': "#{f.object_name}[target_branch]", 'refs-url': refs_project_path(f.object.target_project), selected: f.object.target_branch }, { toggle_class: "js-compare-dropdown js-target-branch monospace" }
+            = dropdown_toggle f.object.target_branch || _("Select target branch"), { toggle: "dropdown", 'field-name': "#{f.object_name}[target_branch]", 'refs-url': refs_project_path(f.object.target_project), selected: f.object.target_branch }, { toggle_class: "js-compare-dropdown js-target-branch monospace" }
             .dropdown-menu.dropdown-menu-selectable.js-target-branch-dropdown.git-revision-dropdown
               = dropdown_title(_("Select target branch"))
               = dropdown_filter(_("Search branches"))
diff --git a/changelogs/unreleased/fj-62807-not-prefill-target-branch.yml b/changelogs/unreleased/fj-62807-not-prefill-target-branch.yml
new file mode 100644
index 00000000000..f19634d80b2
--- /dev/null
+++ b/changelogs/unreleased/fj-62807-not-prefill-target-branch.yml
@@ -0,0 +1,5 @@
+---
+title: Avoid prefilling target branch when source branch is the default one
+merge_request: 32701
+author:
+type: other
diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb
index f18239f6d39..d546a092680 100644
--- a/spec/services/merge_requests/build_service_spec.rb
+++ b/spec/services/merge_requests/build_service_spec.rb
@@ -49,6 +49,22 @@ describe MergeRequests::BuildService do
     allow(project).to receive(:commit).and_return(commit_2)
   end
 
+  shared_examples 'allows the merge request to be created' do
+    it do
+      expect(merge_request.can_be_created).to eq(true)
+    end
+  end
+
+  shared_examples 'forbids the merge request from being created' do
+    it 'returns that the merge request cannot be created' do
+      expect(merge_request.can_be_created).to eq(false)
+    end
+
+    it 'adds an error message to the merge request' do
+      expect(merge_request.errors).to contain_exactly(*Array(error_message))
+    end
+  end
+
   describe '#execute' do
     it 'calls the compare service with the correct arguments' do
       allow_any_instance_of(described_class).to receive(:projects_and_branches_valid?).and_return(true)
@@ -79,12 +95,8 @@ describe MergeRequests::BuildService do
     context 'missing source branch' do
       let(:source_branch) { '' }
 
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds an error message to the merge request' do
-        expect(merge_request.errors).to contain_exactly('You must select source and target branch')
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) { 'You must select source and target branch' }
       end
     end
 
@@ -96,25 +108,44 @@ describe MergeRequests::BuildService do
         stub_compare
       end
 
-      it 'creates compare object with target branch as default branch' do
-        expect(merge_request.compare).to be_present
-        expect(merge_request.target_branch).to eq(project.default_branch)
-      end
+      context 'when source branch' do
+        context 'is not the repository default branch' do
+          it 'creates compare object with target branch as default branch' do
+            expect(merge_request.compare).to be_present
+            expect(merge_request.target_branch).to eq(project.default_branch)
+          end
+
+          it_behaves_like 'allows the merge request to be created'
+        end
+
+        context 'the repository default branch' do
+          let(:source_branch) { 'master' }
+
+          it_behaves_like 'forbids the merge request from being created' do
+            let(:error_message) { 'You must select source and target branch' }
+          end
 
-      it 'allows the merge request to be created' do
-        expect(merge_request.can_be_created).to eq(true)
+          context 'when source project is different from the target project' do
+            let(:target_project) { create(:project, :public, :repository) }
+            let!(:project) { fork_project(target_project, user, namespace: user.namespace, repository: true) }
+            let(:source_project) { project }
+
+            it 'creates compare object with target branch as default branch' do
+              expect(merge_request.compare).to be_present
+              expect(merge_request.target_branch).to eq(project.default_branch)
+            end
+
+            it_behaves_like 'allows the merge request to be created'
+          end
+        end
       end
     end
 
     context 'same source and target branch' do
       let(:source_branch) { 'master' }
 
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds an error message to the merge request' do
-        expect(merge_request.errors).to contain_exactly('You must select different branches')
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) { 'You must select different branches' }
       end
     end
 
@@ -125,9 +156,7 @@ describe MergeRequests::BuildService do
         stub_compare
       end
 
-      it 'allows the merge request to be created' do
-        expect(merge_request.can_be_created).to eq(true)
-      end
+      it_behaves_like 'allows the merge request to be created'
 
       it 'adds a WIP prefix to the merge request title' do
         expect(merge_request.title).to eq('WIP: Feature branch')
@@ -142,9 +171,7 @@ describe MergeRequests::BuildService do
         stub_compare
       end
 
-      it 'allows the merge request to be created' do
-        expect(merge_request.can_be_created).to eq(true)
-      end
+      it_behaves_like 'allows the merge request to be created'
 
       it 'uses the title of the commit as the title of the merge request' do
         expect(merge_request.title).to eq(commit_1.safe_message.split("\n").first)
@@ -254,9 +281,7 @@ describe MergeRequests::BuildService do
         stub_compare
       end
 
-      it 'allows the merge request to be created' do
-        expect(merge_request.can_be_created).to eq(true)
-      end
+      it_behaves_like 'allows the merge request to be created'
 
       it 'uses the title of the branch as the merge request title' do
         expect(merge_request.title).to eq('Feature branch')
@@ -340,12 +365,8 @@ describe MergeRequests::BuildService do
         allow(project).to receive(:commit).with(target_branch).and_return(commit_1)
       end
 
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds an error message to the merge request' do
-        expect(merge_request.errors).to contain_exactly('Source branch "feature-branch" does not exist')
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) { 'Source branch "feature-branch" does not exist' }
       end
     end
 
@@ -355,12 +376,8 @@ describe MergeRequests::BuildService do
         allow(project).to receive(:commit).with(target_branch).and_return(nil)
       end
 
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds an error message to the merge request' do
-        expect(merge_request.errors).to contain_exactly('Target branch "master" does not exist')
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) { 'Target branch "master" does not exist' }
       end
     end
 
@@ -369,15 +386,10 @@ describe MergeRequests::BuildService do
         allow(project).to receive(:commit).and_return(nil)
       end
 
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds both error messages to the merge request' do
-        expect(merge_request.errors).to contain_exactly(
-          'Source branch "feature-branch" does not exist',
-          'Target branch "master" does not exist'
-        )
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) do
+          ['Source branch "feature-branch" does not exist', 'Target branch "master" does not exist']
+        end
       end
     end