Notify with email when merge request became unmergeable

Display MR unmergeable reasons

12 files changed, 191 insertions, 0 deletions

diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index b3f2aeb08ca..c2333180663 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -56,6 +56,14 @@ module Emails
       mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
     end
 
+    def merge_request_unmergeable_email(recipient_id, merge_request_id, reason = nil)
+      setup_merge_request_mail(merge_request_id, recipient_id)
+
+      @merge_request_presenter = MergeRequestPresenter.new(@merge_request, current_user: current_user)
+
+      mail_answer_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id, reason))
+    end
+
     def resolved_all_discussions_email(recipient_id, merge_request_id, resolved_by_user_id, reason = nil)
       setup_merge_request_mail(merge_request_id, recipient_id)
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index dd4daf3711b..99687d305e7 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -125,6 +125,10 @@ class MergeRequest < ActiveRecord::Base
       Gitlab::Timeless.timeless(merge_request, &block)
     end
 
+    after_transition unchecked: :cannot_be_merged do |merge_request, transition|
+      NotificationService.new.merge_request_unmergeable(merge_request)
+    end
+
     def check_state?(merge_status)
       [:unchecked, :cannot_be_merged_recheck].include?(merge_status.to_sym)
     end
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 4b4132af2d0..ad839d9840a 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -20,6 +20,17 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
     end
   end
 
+  def unmergeable_reasons
+    strong_memoize(:unmergeable_reasons) do
+      reasons = []
+      reasons << "no commits" if merge_request.has_no_commits?
+      reasons << "source branch is missing" unless merge_request.source_branch_exists?
+      reasons << "target branch is missing" unless merge_request.target_branch_exists?
+      reasons << "has merge conflicts" unless merge_request.project.repository.can_be_merged?(merge_request.diff_head_sha, merge_request.target_branch)
+      reasons
+    end
+  end
+
   def cancel_merge_when_pipeline_succeeds_path
     if can_cancel_merge_when_pipeline_succeeds?(current_user)
       cancel_merge_when_pipeline_succeeds_project_merge_request_path(project, merge_request)
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 8b3ddab5e19..636cfbf5b45 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -149,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
@@ -485,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/views/notify/merge_request_unmergeable_email.html.haml b/app/views/notify/merge_request_unmergeable_email.html.haml
new file mode 100644
index 00000000000..f51a31c0eda
--- /dev/null
+++ b/app/views/notify/merge_request_unmergeable_email.html.haml
@@ -0,0 +1,6 @@
+%p
+  Merge Request #{link_to @merge_request.to_reference, project_merge_request_url(@merge_request.target_project, @merge_request)} can no longer be merged due to the following reason(s):
+
+  %ul
+    - @merge_request_presenter.unmergeable_reasons.each do |reason|
+      %li= reason
diff --git a/app/views/notify/merge_request_unmergeable_email.text.haml b/app/views/notify/merge_request_unmergeable_email.text.haml
new file mode 100644
index 00000000000..2acf0de34fc
--- /dev/null
+++ b/app/views/notify/merge_request_unmergeable_email.text.haml
@@ -0,0 +1,11 @@
+Merge Request #{@merge_request.to_reference} can no longer be merged due to the following reason(s):
+
+- @merge_request_presenter.unmergeable_reasons.each do |reason|
+  * #{reason}
+
+Merge Request url: #{project_merge_request_url(@merge_request.target_project, @merge_request)}
+
+= merge_path_description(@merge_request, 'to')
+
+Author: #{@merge_request.author_name}
+Assignee: #{@merge_request.assignee_name}
diff --git a/changelogs/unreleased/mr-conflict-notification.yml b/changelogs/unreleased/mr-conflict-notification.yml
new file mode 100644
index 00000000000..d3d5f1fc373
--- /dev/null
+++ b/changelogs/unreleased/mr-conflict-notification.yml
@@ -0,0 +1,5 @@
+---
+title: When MR becomes unmergeable, notify and create todo for author and merge user
+merge_request: 18042
+author:
+type: added
diff --git a/doc/workflow/notifications.md b/doc/workflow/notifications.md
index f1501c81b27..fc592b99860 100644
--- a/doc/workflow/notifications.md
+++ b/doc/workflow/notifications.md
@@ -106,6 +106,10 @@ by yourself (except when an issue is due). You will only receive automatic
 notifications when somebody else comments or adds changes to the ones that
 you've created or mentions you.
 
+If a merge request becomes unmergeable, its author will be notified about the cause.
+If a user has also set the merge request to automatically merge once pipeline succeeds,
+then that user will also be notified.
+
 ### Email Headers
 
 Notification emails include headers that provide extra content about the notification received:
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 84ddbbbf2ee..bed7675db4f 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -390,6 +390,45 @@ describe Notify do
         end
       end
 
+      describe 'that are unmergeable' do
+        set(:merge_request) do
+          create(:merge_request, :conflict,
+                 source_project: project,
+                 target_project: project,
+                 author: current_user,
+                 assignee: assignee,
+                 description: 'Awesome description')
+        end
+
+        subject { described_class.merge_request_unmergeable_email(recipient.id, merge_request.id) }
+
+        it_behaves_like 'a multiple recipients email'
+        it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+          let(:model) { merge_request }
+        end
+        it_behaves_like 'it should show Gmail Actions View Merge request link'
+        it_behaves_like 'an unsubscribeable thread'
+
+        it 'is sent as the merge request author' do
+          sender = subject.header[:from].addrs[0]
+          expect(sender.display_name).to eq(merge_request.author.name)
+          expect(sender.address).to eq(gitlab_sender)
+        end
+
+        it 'has the correct subject and body' do
+          reasons = %w[foo bar]
+
+          allow_any_instance_of(MergeRequestPresenter).to receive(:unmergeable_reasons).and_return(reasons)
+          aggregate_failures do
+            is_expected.to have_referable_subject(merge_request, reply: true)
+            is_expected.to have_body_text(project_merge_request_path(project, merge_request))
+            reasons.each do |reason|
+              is_expected.to have_body_text(reason)
+            end
+          end
+        end
+      end
+
       shared_examples 'a push to an existing merge request' do
         let(:push_user) { create(:user) }
 
diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb
index e3b37739e8e..d5fb4a7742c 100644
--- a/spec/presenters/merge_request_presenter_spec.rb
+++ b/spec/presenters/merge_request_presenter_spec.rb
@@ -70,6 +70,41 @@ describe MergeRequestPresenter do
     end
   end
 
+  describe "#unmergeable_reasons" do
+    let(:presenter) { described_class.new(resource, current_user: user) }
+
+    before do
+      # Mergeable base state
+      allow(resource).to receive(:has_no_commits?).and_return(false)
+      allow(resource).to receive(:source_branch_exists?).and_return(true)
+      allow(resource).to receive(:target_branch_exists?).and_return(true)
+      allow(resource.project.repository).to receive(:can_be_merged?).and_return(true)
+    end
+
+    it "handles mergeable request" do
+      expect(presenter.unmergeable_reasons).to eq([])
+    end
+
+    it "handles no commit" do
+      allow(resource).to receive(:has_no_commits?).and_return(true)
+
+      expect(presenter.unmergeable_reasons).to eq(["no commits"])
+    end
+
+    it "handles branches missing" do
+      allow(resource).to receive(:source_branch_exists?).and_return(false)
+      allow(resource).to receive(:target_branch_exists?).and_return(false)
+
+      expect(presenter.unmergeable_reasons).to eq(["source branch is missing", "target branch is missing"])
+    end
+
+    it "handles merge conflict" do
+      allow(resource.project.repository).to receive(:can_be_merged?).and_return(false)
+
+      expect(presenter.unmergeable_reasons).to eq(["has merge conflicts"])
+    end
+  end
+
   describe '#conflict_resolution_path' do
     let(:project) { create :project }
     let(:user) { create :user }
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 5f28bc123f3..831678b949d 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -1224,6 +1224,32 @@ describe NotificationService, :mailer do
       end
     end
 
+    describe '#merge_request_unmergeable' do
+      it "sends email to merge request author" do
+        notification.merge_request_unmergeable(merge_request)
+
+        should_email(merge_request.author)
+        expect(email_recipients.size).to eq(1)
+      end
+
+      describe 'when merge_when_pipeline_succeeds is true' do
+        before do
+          merge_request.update_attributes(
+            merge_when_pipeline_succeeds: true,
+            merge_user: create(:user)
+          )
+        end
+
+        it "sends email to merge request author and merge_user" do
+          notification.merge_request_unmergeable(merge_request)
+
+          should_email(merge_request.author)
+          should_email(merge_request.merge_user)
+          expect(email_recipients.size).to eq(2)
+        end
+      end
+    end
+
     describe '#closed_merge_request' do
       before do
         update_custom_notification(:close_merge_request, @u_guest_custom, resource: project)