Commit 59bfa080 authored by Nick Thomas's avatar Nick Thomas

Send notification emails when users are newly mentioned in issue edits

parent b2828d41
...@@ -6,6 +6,11 @@ module Emails ...@@ -6,6 +6,11 @@ module Emails
mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id)) mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id))
end end
def new_mention_in_issue_email(recipient_id, issue_id, updated_by_user_id)
setup_issue_mail(issue_id, recipient_id)
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
end
def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id) def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id)
setup_issue_mail(issue_id, recipient_id) setup_issue_mail(issue_id, recipient_id)
......
...@@ -104,11 +104,12 @@ class IssuableBaseService < BaseService ...@@ -104,11 +104,12 @@ class IssuableBaseService < BaseService
change_subscription(issuable) change_subscription(issuable)
filter_params filter_params
old_labels = issuable.labels.to_a old_labels = issuable.labels.to_a
old_mentioned_users = issuable.mentioned_users.to_a
if params.present? && update_issuable(issuable, params) if params.present? && update_issuable(issuable, params)
issuable.reset_events_cache issuable.reset_events_cache
handle_common_system_notes(issuable, old_labels: old_labels) handle_common_system_notes(issuable, old_labels: old_labels)
handle_changes(issuable, old_labels: old_labels) handle_changes(issuable, old_labels: old_labels, old_mentioned_users: old_mentioned_users)
issuable.create_new_cross_references!(current_user) issuable.create_new_cross_references!(current_user)
execute_hooks(issuable, 'update') execute_hooks(issuable, 'update')
end end
......
...@@ -4,7 +4,7 @@ module Issues ...@@ -4,7 +4,7 @@ module Issues
update(issue) update(issue)
end end
def handle_changes(issue, old_labels: []) def handle_changes(issue, old_labels: [], old_mentioned_users: [])
if has_changes?(issue, old_labels: old_labels) if has_changes?(issue, old_labels: old_labels)
todo_service.mark_pending_todos_as_done(issue, current_user) todo_service.mark_pending_todos_as_done(issue, current_user)
end end
...@@ -32,6 +32,11 @@ module Issues ...@@ -32,6 +32,11 @@ module Issues
if added_labels.present? if added_labels.present?
notification_service.relabeled_issue(issue, added_labels, current_user) notification_service.relabeled_issue(issue, added_labels, current_user)
end end
added_mentions = issue.mentioned_users - old_mentioned_users
if added_mentions.present?
notification_service.new_mentions_in_issue(issue, added_mentions, current_user)
end
end end
def reopen_service def reopen_service
......
...@@ -35,6 +35,20 @@ class NotificationService ...@@ -35,6 +35,20 @@ class NotificationService
new_resource_email(issue, issue.project, :new_issue_email) new_resource_email(issue, issue.project, :new_issue_email)
end end
# When issue text is updated, we should send an email to:
#
# * newly mentioned project team members with notification level higher than Participating
#
def new_mentions_in_issue(issue, new_mentioned_users, current_user)
new_mentions_in_resource_email(
issue,
issue.project,
new_mentioned_users,
current_user,
:new_mention_in_issue_email
)
end
# When we close an issue we should send an email to: # When we close an issue we should send an email to:
# #
# * issue author if their notification level is not Disabled # * issue author if their notification level is not Disabled
...@@ -177,7 +191,7 @@ class NotificationService ...@@ -177,7 +191,7 @@ class NotificationService
# build notify method like 'note_commit_email' # build notify method like 'note_commit_email'
notify_method = "note_#{note.noteable_type.underscore}_email".to_sym notify_method = "note_#{note.noteable_type.underscore}_email".to_sym
recipients.each do |recipient| recipients.each do |recipient|
mailer.send(notify_method, recipient.id, note.id).deliver_later mailer.send(notify_method, recipient.id, note.id).deliver_later
end end
...@@ -471,6 +485,14 @@ class NotificationService ...@@ -471,6 +485,14 @@ class NotificationService
end end
end end
def new_mentions_in_resource_email(target, project, new_mentioned_users, current_user, method)
recipients = build_recipients(target, project, current_user) & new_mentioned_users
recipients.each do |recipient|
mailer.send(method, recipient.id, target.id, current_user.id).deliver_later
end
end
def close_resource_email(target, project, current_user, method) def close_resource_email(target, project, current_user, method)
action = method == :merged_merge_request_email ? "merge" : "close" action = method == :merged_merge_request_email ? "merge" : "close"
recipients = build_recipients(target, project, current_user, action: action) recipients = build_recipients(target, project, current_user, action: action)
......
- if current_application_settings.email_author_in_body
%div
#{link_to @issue.author_name, user_url(@issue.author)} wrote:
-if @issue.description
= markdown(@issue.description, pipeline: :email, author: @issue.author)
- if @issue.assignee_id.present?
%p
Assignee: #{@issue.assignee_name}
You have been mentioned in an issue.
Issue <%= @issue.iid %>: <%= url_for(namespace_project_issue_url(@issue.project.namespace, @issue.project, @issue)) %>
Author: <%= @issue.author_name %>
Assignee: <%= @issue.assignee_name %>
<%= @issue.description %>
...@@ -319,5 +319,38 @@ describe Issues::UpdateService, services: true do ...@@ -319,5 +319,38 @@ describe Issues::UpdateService, services: true do
end end
end end
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_issue(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_issue(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
end end
end end
...@@ -399,6 +399,32 @@ describe NotificationService, services: true do ...@@ -399,6 +399,32 @@ describe NotificationService, services: true do
end end
end end
describe '#new_mentions_in_issue' do
def send_notifications(*new_mentions)
ActionMailer::Base.deliveries.clear
notification.new_mentions_in_issue(issue, 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_issue' do describe '#reassigned_issue' do
before do before do
update_custom_notification(:reassign_issue, @u_guest_custom, project) update_custom_notification(:reassign_issue, @u_guest_custom, project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment