Add notifications for new user mentions in merge requests

app/mailers/emails/merge_requests.rb

@@ -6,6 +6,11 @@ module Emails
       mail_new_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id))
     end
+    def new_mention_in_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
+      setup_merge_request_mail(merge_request_id, recipient_id)
+      mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id))
+    end
+
     def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id)
       setup_merge_request_mail(merge_request_id, recipient_id)

app/services/merge_requests/update_service.rb

@@ -56,7 +56,14 @@ module MergeRequests
         )
       end
-      # TODO(nick): use old_mentioned_users to send notify for changed mentions
+      added_mentions = merge_request.mentioned_users - old_mentioned_users
+      if added_mentions.present?
+        notification_service.new_mentions_in_merge_request(
+          merge_request,
+          added_mentions,
+          current_user
+        )
+      end
     end
     def reopen_service

app/services/notification_service.rb

@@ -89,6 +89,20 @@ class NotificationService
     new_resource_email(merge_request, merge_request.target_project, :new_merge_request_email)
   end
+  # When merge request text is updated, we should send an email to:
+  #
+  #  * newly mentioned project team members with notification level higher than Participating
+  #
+  def new_mentions_in_merge_request(merge_request, new_mentioned_users, current_user)
+    new_mentions_in_resource_email(
+      merge_request,
+      merge_request.target_project,
+      new_mentioned_users,
+      current_user,
+      :new_mention_in_merge_request_email
+    )
+  end
+
   # When we reassign a merge_request we should send an email to:
   #
   #  * merge_request old assignee if their notification level is not Disabled

app/views/notify/new_mention_in_merge_request_email.html.haml

+- if current_application_settings.email_author_in_body
+  %div
+    #{link_to @merge_request.author_name, user_url(@merge_request.author)} wrote:
+%p.details
+  != merge_path_description(@merge_request, '→')
+
+- if @merge_request.assignee_id.present?
+  %p
+    Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name}
+
+-if @merge_request.description
+  = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author)

app/views/notify/new_mention_in_merge_request_email.text.erb

+You have been mentioned in Merge Request <%= @merge_request.to_reference %>
+
+<%= url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)) %>
+
+<%= merge_path_description(@merge_request, 'to') %>
+Author:    <%= @merge_request.author_name %>
+Assignee:  <%= @merge_request.assignee_name %>
+
+<%= @merge_request.description %>

spec/services/merge_requests/update_service_spec.rb

@@ -226,6 +226,39 @@ describe MergeRequests::UpdateService, services: true do
       end
     end
+    context 'updated user mentions' do
+      let(:user4) { create(:user) }
+      before do
+        project.team << [user4, :developer]
+      end
+
+      context "in title" do
+        before do
+          perform_enqueued_jobs { update_merge_request(title: user4.to_reference) }
+        end
+
+        it "should email only the newly-mentioned user" do
+          should_not_email(user)
+          should_not_email(user2)
+          should_not_email(user3)
+          should_email(user4)
+        end
+      end
+
+      context "in description" do
+        before do
+          perform_enqueued_jobs { update_merge_request(description: user4.to_reference) }
+        end
+
+        it "should email only the newly-mentioned user" do
+          should_not_email(user)
+          should_not_email(user2)
+          should_not_email(user3)
+          should_email(user4)
+        end
+      end
+    end
+
     context 'when MergeRequest has tasks' do
       before { update_merge_request({ description: "- [ ] Task 1\n- [ ] Task 2" }) }

spec/services/notification_service_spec.rb

@@ -789,6 +789,32 @@ describe NotificationService, services: true do
       end
     end
+    describe '#new_mentions_in_merge_request' do
+      def send_notifications(*new_mentions)
+        ActionMailer::Base.deliveries.clear
+        notification.new_mentions_in_merge_request(merge_request, new_mentions, @u_disabled)
+      end
+
+      it "should not email anyone unless they are newly mentioned" do
+        send_notifications
+        expect(ActionMailer::Base.deliveries).to eq []
+      end
+
+      it "should email new mentions with a watch level higher than participant" do
+        send_notifications(@u_watcher, @u_participant_mentioned)
+
+        should_email(@u_watcher)
+        should_email(@u_participant_mentioned)
+
+        expect(ActionMailer::Base.deliveries.count).to eq 2
+      end
+
+      it "should not email new mentions with a watch level equal to or less than participant" do
+        send_notifications(@u_participating, @u_mentioned)
+        expect(ActionMailer::Base.deliveries).to eq []
+      end
+    end
+
     describe '#reassigned_merge_request' do
       before do
         update_custom_notification(:reassign_merge_request, @u_guest_custom, project)