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

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?]

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

changelogs/unreleased/only-create-unmergeable-todo-once.yml

+---
+title: Only create unmergeable todos once when MR fails to merge
+merge_request:
+author:

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

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