Retarget MR only when source branch is removed

This ensures that we have an automation
in erroring condition

app/services/merge_requests/post_merge_service.rb

@@ -35,7 +35,8 @@ module MergeRequests
       return unless Feature.enabled?(:retarget_merge_requests, merge_request.target_project)
 
       # we can only retarget MRs that are targeting the same project
-      return unless merge_request.for_same_project?
+      # and have a remove source branch set
+      return unless merge_request.for_same_project? && merge_request.remove_source_branch?
 
       # find another merge requests that
       # - as a target have a current source project and branch
@@ -51,7 +52,9 @@ module MergeRequests
         next unless can?(current_user, :update_merge_request, other_merge_request.source_project)
 
         ::MergeRequests::UpdateService
-          .new(other_merge_request.source_project, current_user, target_branch: merge_request.target_branch)
+          .new(other_merge_request.source_project, current_user,
+            target_branch: merge_request.target_branch,
+            target_branch_was_deleted: true)
           .execute(other_merge_request)
       end
     end

app/services/merge_requests/update_service.rb

@@ -4,6 +4,12 @@ module MergeRequests
   class UpdateService < MergeRequests::BaseService
     extend ::Gitlab::Utils::Override
 
+    def initialize(project, user = nil, params = {})
+      super
+
+      @target_branch_was_deleted = @params.delete(:target_branch_was_deleted)
+    end
+
     def execute(merge_request)
       # We don't allow change of source/target projects and source branch
       # after merge request was created
@@ -36,7 +42,9 @@ module MergeRequests
       end
 
       if merge_request.previous_changes.include?('target_branch')
-        create_branch_change_note(merge_request, 'target',
+        create_branch_change_note(merge_request,
+                                  'target',
+                                  target_branch_was_deleted ? 'delete' : 'update',
                                   merge_request.previous_changes['target_branch'].first,
                                   merge_request.target_branch)
 
@@ -130,6 +138,8 @@ module MergeRequests
 
     private
 
+    attr_reader :target_branch_was_deleted
+
     def handle_milestone_change(merge_request)
       return if skip_milestone_email
 
@@ -162,9 +172,9 @@ module MergeRequests
       merge_request_activity_counter.track_users_review_requested(users: new_reviewers)
     end
 
-    def create_branch_change_note(issuable, branch_type, old_branch, new_branch)
+    def create_branch_change_note(issuable, branch_type, event_type, old_branch, new_branch)
       SystemNoteService.change_branch(
-        issuable, issuable.project, current_user, branch_type,
+        issuable, issuable.project, current_user, branch_type, event_type,
         old_branch, new_branch)
     end
 

app/services/system_note_service.rb

@@ -168,16 +168,19 @@ module SystemNoteService
   # project     - Project owning noteable
   # author      - User performing the change
   # branch_type - 'source' or 'target'
+  # event_type  - the source of event: 'update' or 'delete'
   # old_branch  - old branch name
   # new_branch  - new branch name
   #
-  # Example Note text:
+  # Example Note text is based on event_type:
   #
-  #   "changed target branch from `Old` to `New`"
+  #   update: "changed target branch from `Old` to `New`"
+  #   delete: "changed automatically target branch to `New` because `Old` was deleted"
   #
   # Returns the created Note object
-  def change_branch(noteable, project, author, branch_type, old_branch, new_branch)
-    ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).change_branch(branch_type, old_branch, new_branch)
+  def change_branch(noteable, project, author, branch_type, event_type, old_branch, new_branch)
+    ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author)
+      .change_branch(branch_type, event_type, old_branch, new_branch)
   end
 
   # Called when a branch in Noteable is added or deleted

app/services/system_notes/merge_requests_service.rb

@@ -83,16 +83,26 @@ module SystemNotes
     # Called when a branch in Noteable is changed
     #
     # branch_type - 'source' or 'target'
+    # event_type  - the source of event: 'update' or 'delete'
     # old_branch  - old branch name
     # new_branch  - new branch name
+
+    # Example Note text is based on event_type:
     #
-    # Example Note text:
-    #
-    #   "changed target branch from `Old` to `New`"
+    #   update: "changed target branch from `Old` to `New`"
+    #   delete: "changed automatically target branch to `New` because `Old` was deleted"
     #
     # Returns the created Note object
-    def change_branch(branch_type, old_branch, new_branch)
-      body = "changed #{branch_type} branch from `#{old_branch}` to `#{new_branch}`"
+    def change_branch(branch_type, event_type, old_branch, new_branch)
+      body =
+        case event_type.to_s
+        when 'delete'
+          "changed automatically #{branch_type} branch to `#{new_branch}` because `#{old_branch}` was deleted"
+        when 'update'
+          "changed #{branch_type} branch from `#{old_branch}` to `#{new_branch}`"
+        else
+          raise ArgumentError, "invalid value for event_type: #{event_type}"
+        end
 
       create_note(NoteSummary.new(noteable, project, author, body, action: 'branch'))
     end

doc/user/project/merge_requests/getting_started.md

@@ -208,7 +208,7 @@ open. Merge requests are often chained in this manner, with one merge request
 depending on another:
 
 - **Merge request 1**: merge `feature-alpha` into `master`.
-- **Merge Request 2**: merge `feature-beta` into `feature-alpha`.
+- **Merge request 2**: merge `feature-beta` into `feature-alpha`.
 
 These merge requests are usually handled in one of these ways:
 

ee/app/services/ee/merge_requests/update_service.rb

@@ -50,7 +50,7 @@ module EE
       end
 
       override :create_branch_change_note
-      def create_branch_change_note(merge_request, branch_type, old_branch, new_branch)
+      def create_branch_change_note(merge_request, branch_type, event_type, old_branch, new_branch)
         super
 
         reset_approvals(merge_request)

spec/services/merge_requests/post_merge_service_spec.rb

@@ -132,6 +132,12 @@ RSpec.describe MergeRequests::PostMergeService do
     end
 
     context 'for a merge request chain' do
+      before do
+        ::MergeRequests::UpdateService
+          .new(project, user, force_remove_source_branch: '1')
+          .execute(merge_request)
+      end
+
       context 'when there is another MR' do
         let!(:another_merge_request) do
           create(:merge_request,
@@ -142,11 +148,19 @@ RSpec.describe MergeRequests::PostMergeService do
           )
         end
 
+        shared_examples 'does not retarget merge request' do
+          it 'another merge request is unchanged' do
+            expect { subject }.not_to change { another_merge_request.reload.target_branch }
+              .from(merge_request.source_branch)
+          end
+        end
+
         shared_examples 'retargets merge request' do
           it 'another merge request is retargeted' do
             expect(SystemNoteService)
               .to receive(:change_branch).once
-              .with(another_merge_request, another_merge_request.project, user, 'target',
+              .with(another_merge_request, another_merge_request.project, user,
+                'target', 'delete',
                 merge_request.source_branch, merge_request.target_branch)
 
             expect { subject }.to change { another_merge_request.reload.target_branch }
@@ -159,17 +173,17 @@ RSpec.describe MergeRequests::PostMergeService do
               stub_feature_flags(retarget_merge_requests: false)
             end
 
-            it 'another merge request is unchanged' do
-              expect { subject }.not_to change { another_merge_request.reload.target_branch }
-                .from(merge_request.source_branch)
-            end
+            include_examples 'does not retarget merge request'
           end
-        end
 
-        shared_examples 'does not retarget merge request' do
-          it 'another merge request is unchanged' do
-            expect { subject }.not_to change { another_merge_request.reload.target_branch }
-              .from(merge_request.source_branch)
+          context 'when source branch is to be kept' do
+            before do
+              ::MergeRequests::UpdateService
+                .new(project, user, force_remove_source_branch: false)
+                .execute(merge_request)
+            end
+
+            include_examples 'does not retarget merge request'
           end
         end
 

spec/services/merge_requests/update_service_spec.rb

@@ -913,6 +913,33 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
       end
     end
 
+    context 'updating `target_branch`' do
+      let(:merge_request) do
+        create(:merge_request,
+               source_project: project,
+               source_branch: 'mr-b',
+               target_branch: 'mr-a')
+      end
+
+      it 'updates to master' do
+        expect(SystemNoteService).to receive(:change_branch).with(
+          merge_request, project, user, 'target', 'update', 'mr-a', 'master'
+        )
+
+        expect { update_merge_request(target_branch: 'master') }
+          .to change { merge_request.reload.target_branch }.from('mr-a').to('master')
+      end
+
+      it 'updates to master because of branch deletion' do
+        expect(SystemNoteService).to receive(:change_branch).with(
+          merge_request, project, user, 'target', 'delete', 'mr-a', 'master'
+        )
+
+        expect { update_merge_request(target_branch: 'master', target_branch_was_deleted: true) }
+          .to change { merge_request.reload.target_branch }.from('mr-a').to('master')
+      end
+    end
+
     it_behaves_like 'issuable record that supports quick actions' do
       let(:existing_merge_request) { create(:merge_request, source_project: project) }
       let(:issuable) { described_class.new(project, user, params).execute(existing_merge_request) }

spec/services/system_note_service_spec.rb

@@ -213,15 +213,16 @@ RSpec.describe SystemNoteService do
 
   describe '.change_branch' do
     it 'calls MergeRequestsService' do
-      old_branch = double
-      new_branch = double
-      branch_type = double
+      old_branch = double('old_branch')
+      new_branch = double('new_branch')
+      branch_type = double('branch_type')
+      event_type = double('event_type')
 
       expect_next_instance_of(::SystemNotes::MergeRequestsService) do |service|
-        expect(service).to receive(:change_branch).with(branch_type, old_branch, new_branch)
+        expect(service).to receive(:change_branch).with(branch_type, event_type, old_branch, new_branch)
       end
 
-      described_class.change_branch(noteable, project, author, branch_type, old_branch, new_branch)
+      described_class.change_branch(noteable, project, author, branch_type, event_type, old_branch, new_branch)
     end
   end
 

spec/services/system_notes/merge_requests_service_spec.rb

@@ -167,18 +167,38 @@ RSpec.describe ::SystemNotes::MergeRequestsService do
   end
 
   describe '.change_branch' do
-    subject { service.change_branch('target', old_branch, new_branch) }
-
     let(:old_branch) { 'old_branch'}
     let(:new_branch) { 'new_branch'}
 
     it_behaves_like 'a system note' do
       let(:action) { 'branch' }
+
+      subject { service.change_branch('target', 'update', old_branch, new_branch) }
     end
 
     context 'when target branch name changed' do
-      it 'sets the note text' do
-        expect(subject.note).to eq "changed target branch from `#{old_branch}` to `#{new_branch}`"
+      context 'on update' do
+        subject { service.change_branch('target', 'update', old_branch, new_branch) }
+
+        it 'sets the note text' do
+          expect(subject.note).to eq "changed target branch from `#{old_branch}` to `#{new_branch}`"
+        end
+      end
+
+      context 'on delete' do
+        subject { service.change_branch('target', 'delete', old_branch, new_branch) }
+
+        it 'sets the note text' do
+          expect(subject.note).to eq "changed automatically target branch to `#{new_branch}` because `#{old_branch}` was deleted"
+        end
+      end
+
+      context 'for invalid event_type' do
+        subject { service.change_branch('target', 'invalid', old_branch, new_branch) }
+
+        it 'raises exception' do
+          expect { subject }.to raise_error /invalid value for event_type/
+        end
       end
     end
   end