From 59bfa0809822c3dd257748197223809922ab5f80 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 12 Aug 2016 22:54:32 +0100 Subject: Send notification emails when users are newly mentioned in issue edits --- app/mailers/emails/issues.rb | 5 +++++ app/services/issuable_base_service.rb | 3 ++- app/services/issues/update_service.rb | 7 ++++++- app/services/notification_service.rb | 24 +++++++++++++++++++++- .../notify/new_mention_in_issue_email.html.haml | 9 ++++++++ .../notify/new_mention_in_issue_email.text.erb | 7 +++++++ 6 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 app/views/notify/new_mention_in_issue_email.html.haml create mode 100644 app/views/notify/new_mention_in_issue_email.text.erb (limited to 'app') diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb index 6f54c42146c..d64e48f774b 100644 --- a/app/mailers/emails/issues.rb +++ b/app/mailers/emails/issues.rb @@ -6,6 +6,11 @@ module Emails mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id)) 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) setup_issue_mail(issue_id, recipient_id) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 2d96efe1042..b0ea7c905f8 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -104,11 +104,12 @@ class IssuableBaseService < BaseService change_subscription(issuable) filter_params old_labels = issuable.labels.to_a + old_mentioned_users = issuable.mentioned_users.to_a if params.present? && update_issuable(issuable, params) issuable.reset_events_cache 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) execute_hooks(issuable, 'update') end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index c7d406cc331..a2111b3806b 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -4,7 +4,7 @@ module Issues update(issue) end - def handle_changes(issue, old_labels: []) + def handle_changes(issue, old_labels: [], old_mentioned_users: []) if has_changes?(issue, old_labels: old_labels) todo_service.mark_pending_todos_as_done(issue, current_user) end @@ -32,6 +32,11 @@ module Issues if added_labels.present? notification_service.relabeled_issue(issue, added_labels, current_user) 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 def reopen_service diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index ab6e51209ee..73df572514f 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -35,6 +35,20 @@ class NotificationService new_resource_email(issue, issue.project, :new_issue_email) 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: # # * issue author if their notification level is not Disabled @@ -177,7 +191,7 @@ class NotificationService # build notify method like 'note_commit_email' notify_method = "note_#{note.noteable_type.underscore}_email".to_sym - + recipients.each do |recipient| mailer.send(notify_method, recipient.id, note.id).deliver_later end @@ -471,6 +485,14 @@ class NotificationService 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) action = method == :merged_merge_request_email ? "merge" : "close" recipients = build_recipients(target, project, current_user, action: action) diff --git a/app/views/notify/new_mention_in_issue_email.html.haml b/app/views/notify/new_mention_in_issue_email.html.haml new file mode 100644 index 00000000000..f42b150c0d6 --- /dev/null +++ b/app/views/notify/new_mention_in_issue_email.html.haml @@ -0,0 +1,9 @@ +- 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} diff --git a/app/views/notify/new_mention_in_issue_email.text.erb b/app/views/notify/new_mention_in_issue_email.text.erb new file mode 100644 index 00000000000..457e94b4800 --- /dev/null +++ b/app/views/notify/new_mention_in_issue_email.text.erb @@ -0,0 +1,7 @@ +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 %> -- cgit v1.2.1 From e6f519c4a7efa6a865c7e8d2a62ed5c3db12b453 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 15 Aug 2016 12:05:08 +0100 Subject: Fix a 'missing keyword' error introduced in the last commit --- app/services/merge_requests/update_service.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 026a37997d4..0ad42b83fe2 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -16,7 +16,7 @@ module MergeRequests update(merge_request) end - def handle_changes(merge_request, old_labels: []) + def handle_changes(merge_request, old_labels: [], old_mentioned_users: []) if has_changes?(merge_request, old_labels: old_labels) todo_service.mark_pending_todos_as_done(merge_request, current_user) end @@ -55,6 +55,8 @@ module MergeRequests current_user ) end + + # TODO(nick): use old_mentioned_users to send notify for changed mentions end def reopen_service -- cgit v1.2.1 From 6642ae4579d6ceed6b26014aee4a22adb39fc43c Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 15 Aug 2016 14:47:16 +0100 Subject: Add notifications for new user mentions in merge requests --- app/mailers/emails/merge_requests.rb | 5 +++++ app/services/merge_requests/update_service.rb | 9 ++++++++- app/services/notification_service.rb | 14 ++++++++++++++ .../notify/new_mention_in_merge_request_email.html.haml | 12 ++++++++++++ .../notify/new_mention_in_merge_request_email.text.erb | 9 +++++++++ 5 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 app/views/notify/new_mention_in_merge_request_email.html.haml create mode 100644 app/views/notify/new_mention_in_merge_request_email.text.erb (limited to 'app') 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 %> -- cgit v1.2.1 From 10af11f4fe71e46d4decd2ab428e9b2b38e2d463 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 15 Aug 2016 15:15:20 +0100 Subject: Allow people to subscribe to mentions in updated MRs and Issues This slightly changes the semantics of the 'New Issue' and 'New MR' events to include new mentions in edited Mentionables. An alternative would be to introduce 'Issue updated' and 'MR updated' events, but that would lead to questions about why those events were only available to new mentions, and not existing mentions as well, so hold off for now. --- app/services/notification_service.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 01f95281170..2291bc0f127 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -500,7 +500,8 @@ class NotificationService 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 = build_recipients(target, project, current_user, action: "new") + recipients = recipients & new_mentioned_users recipients.each do |recipient| mailer.send(method, recipient.id, target.id, current_user.id).deliver_later -- cgit v1.2.1 From b62954db4c40df435363994ce4632632fde01455 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Tue, 16 Aug 2016 14:12:05 +0100 Subject: Include the reason for emailing in the new_mention emails --- app/views/notify/new_mention_in_issue_email.html.haml | 3 +++ app/views/notify/new_mention_in_merge_request_email.html.haml | 3 +++ 2 files changed, 6 insertions(+) (limited to 'app') diff --git a/app/views/notify/new_mention_in_issue_email.html.haml b/app/views/notify/new_mention_in_issue_email.html.haml index f42b150c0d6..4f3d36bd9ca 100644 --- a/app/views/notify/new_mention_in_issue_email.html.haml +++ b/app/views/notify/new_mention_in_issue_email.html.haml @@ -1,3 +1,6 @@ +%p + You have been mentioned in an issue. + - if current_application_settings.email_author_in_body %div #{link_to @issue.author_name, user_url(@issue.author)} wrote: 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 index 158404de396..32aedb9e6b9 100644 --- a/app/views/notify/new_mention_in_merge_request_email.html.haml +++ b/app/views/notify/new_mention_in_merge_request_email.html.haml @@ -1,3 +1,6 @@ +%p + You have been mentioned in Merge Request #{@merge_request.to_reference} + - if current_application_settings.email_author_in_body %div #{link_to @merge_request.author_name, user_url(@merge_request.author)} wrote: -- cgit v1.2.1