diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 9dd11d20ea6..95810b0ac6e 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/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) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 0ad42b83fe2..30c5f24988c 100644 --- a/app/services/merge_requests/update_service.rb +++ b/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 diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 73df572514f..01f95281170 100644 --- a/app/services/notification_service.rb +++ b/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 diff --git a/app/views/notify/new_mention_in_merge_request_email.html.haml b/app/views/notify/new_mention_in_merge_request_email.html.haml new file mode 100644 index 00000000000..158404de396 --- /dev/null +++ b/app/views/notify/new_mention_in_merge_request_email.html.haml @@ -0,0 +1,12 @@ +- 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) diff --git a/app/views/notify/new_mention_in_merge_request_email.text.erb b/app/views/notify/new_mention_in_merge_request_email.text.erb new file mode 100644 index 00000000000..5bf0282e097 --- /dev/null +++ b/app/views/notify/new_mention_in_merge_request_email.text.erb @@ -0,0 +1,9 @@ +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 %> diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 283a336afd9..e26e925cd81 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/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" }) } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index d61de6c15d1..2fa0d9f1ac2 100644 --- a/spec/services/notification_service_spec.rb +++ b/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)