diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/services/notification_service.rb | 49 | ||||
| -rw-r--r-- | doc/workflow/notifications.md | 39 | ||||
| -rw-r--r-- | spec/services/notification_service_spec.rb | 7 |
4 files changed, 44 insertions, 52 deletions
diff --git a/CHANGELOG b/CHANGELOG index c5e3e4518b9..4cb80322846 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -32,6 +32,7 @@ v 7.14.0 (unreleased) - Fix bug causing Bitbucket importer to crash when OAuth application had been removed. - Add fetch command to the MR page. - Show who last edited a comment if it wasn't the original author + - Send notification to all participants when MR is merged. - Add ability to manage user email addresses via the API. - Show buttons to add license, changelog and contribution guide if they're missing. - Tweak project page buttons. diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 312b56eb87b..3735a136365 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -70,12 +70,6 @@ class NotificationService reassign_resource_email(merge_request, merge_request.target_project, current_user, 'reassigned_merge_request_email') end - # When we close a merge request we should send next emails: - # - # * merge_request author if their notification level is not Disabled - # * merge_request assignee if their notification level is not Disabled - # * project team members with notification level higher then Participating - # def close_mr(merge_request, current_user) close_resource_email(merge_request, merge_request.target_project, current_user, 'closed_merge_request_email') end @@ -84,26 +78,8 @@ class NotificationService reopen_resource_email(issue, issue.project, current_user, 'issue_status_changed_email', 'reopened') end - # When we merge a merge request we should send next emails: - # - # * merge_request author if their notification level is not Disabled - # * merge_request assignee if their notification level is not Disabled - # * project team members with notification level higher then Participating - # def merge_mr(merge_request, current_user) - recipients = [merge_request.author, merge_request.assignee] - - recipients = add_project_watchers(recipients, merge_request.target_project) - recipients = reject_muted_users(recipients, merge_request.target_project) - - recipients = add_subscribed_users(recipients, merge_request) - recipients = reject_unsubscribed_users(recipients, merge_request) - - recipients.delete(current_user) - - recipients.each do |recipient| - mailer.merged_merge_request_email(recipient.id, merge_request.id, current_user.id) - end + close_resource_email(merge_request, merge_request.target_project, current_user, 'merged_merge_request_email') end def reopen_mr(merge_request, current_user) @@ -364,8 +340,7 @@ class NotificationService end def new_resource_email(target, project, method) - recipients = build_recipients(target, project) - recipients.delete(target.author) + recipients = build_recipients(target, project, target.author) recipients.each do |recipient| mailer.send(method, recipient.id, target.id) @@ -373,8 +348,7 @@ class NotificationService end def close_resource_email(target, project, current_user, method) - recipients = build_recipients(target, project) - recipients.delete(current_user) + recipients = build_recipients(target, project, current_user) recipients.each do |recipient| mailer.send(method, recipient.id, target.id, current_user.id) @@ -383,8 +357,7 @@ class NotificationService def reassign_resource_email(target, project, current_user, method) assignee_id_was = previous_record(target, "assignee_id") - recipients = build_recipients(target, project) - recipients.delete(current_user) + recipients = build_recipients(target, project, current_user) recipients.each do |recipient| mailer.send(method, recipient.id, target.id, assignee_id_was, current_user.id) @@ -392,21 +365,15 @@ class NotificationService end def reopen_resource_email(target, project, current_user, method, status) - recipients = build_recipients(target, project) - recipients.delete(current_user) + recipients = build_recipients(target, project, current_user) recipients.each do |recipient| mailer.send(method, recipient.id, target.id, status, current_user.id) end end - def build_recipients(target, project) - recipients = - if target.respond_to?(:participants) - target.participants - else - [target.author, target.assignee] - end + def build_recipients(target, project, current_user) + recipients = target.participants(current_user) recipients = add_project_watchers(recipients, project) recipients = reject_mention_users(recipients, project) @@ -415,6 +382,8 @@ class NotificationService recipients = add_subscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target) + recipients.delete(current_user) + recipients end diff --git a/doc/workflow/notifications.md b/doc/workflow/notifications.md index 2b5f06dd1fa..025992deece 100644 --- a/doc/workflow/notifications.md +++ b/doc/workflow/notifications.md @@ -52,18 +52,35 @@ Below is the table of events users can be notified of: | New SSH key added | User | Security email, always sent. | | New email added | User | Security email, always sent. | | New user created | User | Sent on user creation, except for omniauth (LDAP)| -| New issue created | Issue assignee [1], project members [2] | [1] not disabled, [2] higher than participating | | User added to project | User | Sent when user is added to project | | Project access level changed | User | Sent when user project access level is changed | | User added to group | User | Sent when user is added to group | -| Project moved | Project members [1] | [1] not disabled | | Group access level changed | User | Sent when user group access level is changed | -| Close issue | Issue author [1], issue assignee [2], project members [3] | [1] [2] not disabled, [3] higher than participating | -| Reassign issue | New issue assignee [1], old issue assignee [2] | [1] [2] not disabled | -| Reopen issue | Project members [1] | [1] higher than participating | -| New merge request | MR assignee [1] | [1] not disabled | -| Reassign merge request | New MR assignee [1], old MR assignee [2] | [1] [2] not disabled | -| Close merge request | MR author [1], MR assignee [2], project members [3] | [1] [2] not disabled, [3] higher than participating | -| Reopen merge request | Project members [1] | [1] higher than participating | -| Merge merge request | MR author [1], MR assignee [2], project members [3] | [1] [2] not disabled, [3] higher than participating | -| New comment | Mentioned users [1], users participating [2], project members [3] | [1] [2] not disabled, [3] higher than participating | +| Project moved | Project members [1] | [1] not disabled | + +### Issue / Merge Request events + +In all of the below cases, the notification will be sent to: +- Participants: + - the author and assignee of the issue/merge request + - authors of comments on the issue/merge request + - anyone mentioned by `@username` in the issue/merge request description + - anyone mentioned by `@username` in any of the comments on the issue/merge request + + ...with notification level "Participating" or higher + +- Watchers: project members with notification level "Watch" +- Subscribers: anyone who manually subscribed to the issue/merge request + +| Event | Sent to | +|------------------------|---------| +| New issue | | +| Close issue | | +| Reassign issue | The above, plus the old assignee | +| Reopen issue | | +| New merge request | | +| Reassign merge request | The above, plus the old assignee | +| Close merge request | | +| Reopen merge request | | +| Merge merge request | | +| New comment | The above, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher | diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 253e5823499..9da6c9dc949 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -300,7 +300,7 @@ describe NotificationService do describe 'Merge Requests' do let(:project) { create(:project, :public) } - let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user) } + let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' } before do build_team(merge_request.target_project) @@ -311,6 +311,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) notification.new_merge_request(merge_request, @u_disabled) @@ -329,6 +330,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_email(@subscriber.id) should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) @@ -349,6 +351,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_email(@subscriber.id) should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) @@ -369,6 +372,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_email(@subscriber.id) should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) @@ -389,6 +393,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_email(@subscriber.id) should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) |