Commit f93c4dc0d83dd40452d40447a62cd68a08e24b09
1 parent
38ffb822
Exists in
master
and in
4 other branches
Use notificationService for MR reassigned
Showing
2 changed files
with
19 additions
and
14 deletions
Show diff stats
app/observers/merge_request_observer.rb
| @@ -6,29 +6,20 @@ class MergeRequestObserver < ActiveRecord::Observer | @@ -6,29 +6,20 @@ class MergeRequestObserver < ActiveRecord::Observer | ||
| 6 | end | 6 | end |
| 7 | 7 | ||
| 8 | def after_close(merge_request, transition) | 8 | def after_close(merge_request, transition) |
| 9 | - send_reassigned_email(merge_request) if merge_request.is_being_reassigned? | ||
| 10 | - | ||
| 11 | Note.create_status_change_note(merge_request, current_user, merge_request.state) | 9 | Note.create_status_change_note(merge_request, current_user, merge_request.state) |
| 12 | end | 10 | end |
| 13 | 11 | ||
| 14 | def after_reopen(merge_request, transition) | 12 | def after_reopen(merge_request, transition) |
| 15 | - send_reassigned_email(merge_request) if merge_request.is_being_reassigned? | ||
| 16 | - | ||
| 17 | Note.create_status_change_note(merge_request, current_user, merge_request.state) | 13 | Note.create_status_change_note(merge_request, current_user, merge_request.state) |
| 18 | end | 14 | end |
| 19 | 15 | ||
| 20 | def after_update(merge_request) | 16 | def after_update(merge_request) |
| 21 | - send_reassigned_email(merge_request) if merge_request.is_being_reassigned? | 17 | + notification.reassigned_merge_request(merge_request) if merge_request.is_being_reassigned? |
| 22 | end | 18 | end |
| 23 | 19 | ||
| 24 | protected | 20 | protected |
| 25 | 21 | ||
| 26 | - def send_reassigned_email(merge_request) | ||
| 27 | - recipients_ids = merge_request.assignee_id_was, merge_request.assignee_id | ||
| 28 | - recipients_ids.delete current_user.id | ||
| 29 | - | ||
| 30 | - recipients_ids.each do |recipient_id| | ||
| 31 | - Notify.delay.reassigned_merge_request_email(recipient_id, merge_request.id, merge_request.assignee_id_was) | ||
| 32 | - end | 22 | + def notification |
| 23 | + NotificationService.new | ||
| 33 | end | 24 | end |
| 34 | end | 25 | end |
app/services/notification_service.rb
| @@ -33,8 +33,8 @@ class NotificationService | @@ -33,8 +33,8 @@ class NotificationService | ||
| 33 | 33 | ||
| 34 | # When we reassign an issue we should send next emails: | 34 | # When we reassign an issue we should send next emails: |
| 35 | # | 35 | # |
| 36 | - # * issue author if his notification level is not Disabled | ||
| 37 | - # * issue assignee if his notification level is not Disabled | 36 | + # * issue old assignee if his notification level is not Disabled |
| 37 | + # * issue new assignee if his notification level is not Disabled | ||
| 38 | # | 38 | # |
| 39 | def reassigned_issue(issue, current_user) | 39 | def reassigned_issue(issue, current_user) |
| 40 | recipient_ids = [issue.assignee_id, issue.assignee_id_was].compact.uniq | 40 | recipient_ids = [issue.assignee_id, issue.assignee_id_was].compact.uniq |
| @@ -66,4 +66,18 @@ class NotificationService | @@ -66,4 +66,18 @@ class NotificationService | ||
| 66 | Notify.delay.new_merge_request_email(merge_request.id) | 66 | Notify.delay.new_merge_request_email(merge_request.id) |
| 67 | end | 67 | end |
| 68 | end | 68 | end |
| 69 | + | ||
| 70 | + # When we reassign a merge_request we should send next emails: | ||
| 71 | + # | ||
| 72 | + # * merge_request old assignee if his notification level is not Disabled | ||
| 73 | + # * merge_request assignee if his notification level is not Disabled | ||
| 74 | + # | ||
| 75 | + def reassigned_merge_request(merge_request, current_user) | ||
| 76 | + recipients_ids = merge_request.assignee_id_was, merge_request.assignee_id | ||
| 77 | + recipients_ids.delete current_user.id | ||
| 78 | + | ||
| 79 | + recipients_ids.each do |recipient_id| | ||
| 80 | + Notify.delay.reassigned_merge_request_email(recipient_id, merge_request.id, merge_request.assignee_id_was) | ||
| 81 | + end | ||
| 82 | + end | ||
| 69 | end | 83 | end |