Merge branch 'mr-conflict-notification' into 'master'

MR unmergeable notification

See merge request gitlab-org/gitlab-ce!18042

4 files changed, 60 insertions, 19 deletions

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 850deb0ac7a..9a4e6eb2e88 100644
--- a/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
+++ b/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
@@ -24,11 +24,7 @@ module MergeRequests
 
       pipeline_merge_requests(pipeline) do |merge_request|
         next unless merge_request.merge_when_pipeline_succeeds?
-
-        unless merge_request.mergeable?
-          todo_service.merge_request_became_unmergeable(merge_request)
-          next
-        end
+        next unless merge_request.mergeable?
 
         merge_request.merge_async(merge_request.merge_user_id, merge_request.merge_params)
       end
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 5658699664d..4fa38665abc 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -18,6 +18,10 @@ module NotificationRecipientService
     Builder::NewNote.new(*a).notification_recipients
   end
 
+  def self.build_merge_request_unmergeable_recipients(*a)
+    Builder::MergeRequestUnmergeable.new(*a).notification_recipients
+  end
+
   module Builder
     class Base
       def initialize(*)
@@ -330,5 +334,26 @@ module NotificationRecipientService
         note.author
       end
     end
+
+    class MergeRequestUnmergeable < Base
+      attr_reader :target
+      def initialize(merge_request)
+        @target = merge_request
+      end
+
+      def build!
+        target.merge_participants.each do |user|
+          add_recipients(user, :participating, nil)
+        end
+      end
+
+      def custom_action
+        :unmergeable_merge_request
+      end
+
+      def acting_user
+        nil
+      end
+    end
   end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 55a1735e54b..636cfbf5b45 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -129,6 +129,7 @@ class NotificationService
 
   # When create a merge request we should send an email to:
   #
+  #  * mr author
   #  * mr assignee if their notification level is not Disabled
   #  * project team members with notification level higher then Participating
   #  * watchers of the mr's labels
@@ -148,6 +149,15 @@ class NotificationService
     end
   end
 
+  # When a merge request is found to be unmergeable, we should send an email to:
+  #
+  #  * mr author
+  #  * mr merge user if set
+  #
+  def merge_request_unmergeable(merge_request)
+    merge_request_unmergeable_email(merge_request)
+  end
+
   # When merge request text is updated, we should send an email to:
   #
   #  * newly mentioned project team members with notification level higher than Participating
@@ -484,6 +494,14 @@ class NotificationService
     end
   end
 
+  def merge_request_unmergeable_email(merge_request)
+    recipients = NotificationRecipientService.build_merge_request_unmergeable_recipients(merge_request)
+
+    recipients.each do |recipient|
+      mailer.merge_request_unmergeable_email(recipient.user.id, merge_request.id).deliver_later
+    end
+  end
+
   def mailer
     Notify
   end
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index ffd48e842c2..f91cd03bf5c 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -98,12 +98,12 @@ class TodoService
 
   # When a build fails on the HEAD of a merge request we should:
   #
-  #  * create a todo for author of MR to fix it
-  #  * create a todo for merge_user to keep an eye on it
+  #  * create a todo for each merge participant
   #
   def merge_request_build_failed(merge_request)
-    create_build_failed_todo(merge_request, merge_request.author)
-    create_build_failed_todo(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds?
+    merge_request.merge_participants.each do |user|
+      create_build_failed_todo(merge_request, user)
+    end
   end
 
   # When a new commit is pushed to a merge request we should:
@@ -116,20 +116,22 @@ class TodoService
 
   # When a build is retried to a merge request we should:
   #
-  #  * mark all pending todos related to the merge request for the author as done
-  #  * mark all pending todos related to the merge request for the merge_user as done
+  #  * mark all pending todos related to the merge request as done for each merge participant
   #
   def merge_request_build_retried(merge_request)
-    mark_pending_todos_as_done(merge_request, merge_request.author)
-    mark_pending_todos_as_done(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds?
+    merge_request.merge_participants.each do |user|
+      mark_pending_todos_as_done(merge_request, user)
+    end
   end
 
-  # When a merge request could not be automatically merged due to its unmergeable state we should:
+  # When a merge request could not be merged due to its unmergeable state we should:
   #
-  #  * create a todo for a merge_user
+  #  * create a todo for each merge participant
   #
   def merge_request_became_unmergeable(merge_request)
-    create_unmergeable_todo(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds?
+    merge_request.merge_participants.each do |user|
+      create_unmergeable_todo(merge_request, user)
+    end
   end
 
   # When create a note we should:
@@ -275,9 +277,9 @@ class TodoService
     create_todos(todo_author, attributes)
   end
 
-  def create_unmergeable_todo(merge_request, merge_user)
-    attributes = attributes_for_todo(merge_request.project, merge_request, merge_user, Todo::UNMERGEABLE)
-    create_todos(merge_user, attributes)
+  def create_unmergeable_todo(merge_request, todo_author)
+    attributes = attributes_for_todo(merge_request.project, merge_request, todo_author, Todo::UNMERGEABLE)
+    create_todos(todo_author, attributes)
   end
 
   def attributes_for_target(target)