Merge branch 'only-create-unmergeable-todo-once' into 'master'

Only create unmergeable todos once

Closes #28555

See merge request !9513

5 files changed, 34 insertions, 11 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 7eb875f1ef5..d6e7ed87555 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -91,10 +91,6 @@ class MergeRequest < ActiveRecord::Base
     around_transition do |merge_request, transition, block|
       Gitlab::Timeless.timeless(merge_request, &block)
     end
-
-    after_transition unchecked: :cannot_be_merged do |merge_request, transition|
-      TodoService.new.merge_request_became_unmergeable(merge_request)
-    end
   end
 
   validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_without_fork?]
diff --git a/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb b/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
index 5616edf8b4a..5081dd5a0c4 100644
--- a/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
+++ b/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
@@ -24,7 +24,11 @@ module MergeRequests
 
       pipeline_merge_requests(pipeline) do |merge_request|
         next unless merge_request.merge_when_build_succeeds?
-        next unless merge_request.mergeable?
+
+        unless merge_request.mergeable?
+          todo_service.merge_request_became_unmergeable(merge_request)
+          next
+        end
 
         MergeWorker.perform_async(merge_request.id, merge_request.merge_user_id, merge_request.merge_params)
       end
diff --git a/changelogs/unreleased/only-create-unmergeable-todo-once.yml b/changelogs/unreleased/only-create-unmergeable-todo-once.yml
new file mode 100644
index 00000000000..e675ed945ad
--- /dev/null
+++ b/changelogs/unreleased/only-create-unmergeable-todo-once.yml
@@ -0,0 +1,4 @@
+---
+title: Only create unmergeable todos once when MR fails to merge
+merge_request:
+author:
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index fa1b0396bcf..9331dc41a5e 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -833,12 +833,6 @@ describe MergeRequest, models: true do
       it 'becomes unmergeable' do
         expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged')
       end
-
-      it 'creates Todo on unmergeability' do
-        expect_any_instance_of(TodoService).to receive(:merge_request_became_unmergeable).with(subject)
-
-        subject.check_if_can_be_merged
-      end
     end
 
     context 'when it has conflicts' do
diff --git a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
index f92978a33a3..0ff6e8fda16 100644
--- a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
+++ b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
@@ -111,6 +111,31 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
         service.trigger(unrelated_pipeline)
       end
     end
+
+    context 'when the merge request is not mergeable' do
+      let(:mr_conflict) do
+        create(:merge_request, merge_when_build_succeeds: true, merge_user: user,
+                               source_branch: 'master', target_branch: 'feature-conflict',
+                               source_project: project, target_project: project)
+      end
+
+      let(:conflict_pipeline) do
+        create(:ci_pipeline, project: project, ref: mr_conflict.source_branch,
+                             sha: mr_conflict.diff_head_sha, status: 'success')
+      end
+
+      it 'does not merge the merge request' do
+        expect(MergeWorker).not_to receive(:perform_async)
+
+        service.trigger(conflict_pipeline)
+      end
+
+      it 'creates todos for unmergeability' do
+        expect_any_instance_of(TodoService).to receive(:merge_request_became_unmergeable).with(mr_conflict)
+
+        service.trigger(conflict_pipeline)
+      end
+    end
   end
 
   describe "#cancel" do