Commit 0a044c7318ada48999bcf61cc95ab65b0d1435ca

Authored by Dmitriy Zaporozhets
1 parent ba599120

NotificationService respects disabled notifications now

Showing 1 changed file with 43 additions and 19 deletions   Show diff stats
app/services/notification_service.rb
@@ -8,6 +8,9 @@ @@ -8,6 +8,9 @@
8 class NotificationService 8 class NotificationService
9 # Always notify user about ssh key added 9 # Always notify user about ssh key added
10 # only if ssh key is not deploy key 10 # only if ssh key is not deploy key
  11 + #
  12 + # This is security email so it will be sent
  13 + # even if user disabled notifications
11 def new_key(key) 14 def new_key(key)
12 if key.user 15 if key.user
13 Notify.delay.new_ssh_key_email(key.id) 16 Notify.delay.new_ssh_key_email(key.id)
@@ -21,10 +24,10 @@ class NotificationService @@ -21,10 +24,10 @@ class NotificationService
21 # * project team members with notification level higher then Participating 24 # * project team members with notification level higher then Participating
22 # 25 #
23 def close_issue(issue, current_user) 26 def close_issue(issue, current_user)
24 - recipients = [issue.author, issue.assignee].compact.uniq 27 + recipients = reject_muted_users([issue.author, issue.assignee])
25 28
26 # Dont send email to me when I close an issue 29 # Dont send email to me when I close an issue
27 - recipients.reject! { |u| u == current_user } 30 + recipients.delete(current_user)
28 31
29 recipients.each do |recipient| 32 recipients.each do |recipient|
30 Notify.delay.issue_status_changed_email(recipient.id, issue.id, issue.state, current_user.id) 33 Notify.delay.issue_status_changed_email(recipient.id, issue.id, issue.state, current_user.id)
@@ -37,14 +40,7 @@ class NotificationService @@ -37,14 +40,7 @@ class NotificationService
37 # * issue new assignee if his notification level is not Disabled 40 # * issue new assignee if his notification level is not Disabled
38 # 41 #
39 def reassigned_issue(issue, current_user) 42 def reassigned_issue(issue, current_user)
40 - recipient_ids = [issue.assignee_id, issue.assignee_id_was].compact.uniq  
41 -  
42 - # Reject me from recipients if I reassign an issue  
43 - recipient_ids.reject! { |id| id == current_user.id }  
44 -  
45 - recipient_ids.each do |recipient_id|  
46 - Notify.delay.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was)  
47 - end 43 + reassign_email(merge_request, current_user, 'reassigned_issue_email')
48 end 44 end
49 45
50 # When create an issue we should send next emails: 46 # When create an issue we should send next emails:
@@ -52,7 +48,11 @@ class NotificationService @@ -52,7 +48,11 @@ class NotificationService
52 # * issue assignee if his notification level is not Disabled 48 # * issue assignee if his notification level is not Disabled
53 # 49 #
54 def new_issue(issue, current_user) 50 def new_issue(issue, current_user)
  51 +
55 if issue.assignee && issue.assignee != current_user 52 if issue.assignee && issue.assignee != current_user
  53 + # skip if assignee notification disabled
  54 + return true if issue.assignee.notification.disabled?
  55 +
56 Notify.delay.new_issue_email(issue.id) 56 Notify.delay.new_issue_email(issue.id)
57 end 57 end
58 end 58 end
@@ -63,6 +63,9 @@ class NotificationService @@ -63,6 +63,9 @@ class NotificationService
63 # 63 #
64 def new_merge_request(merge_request, current_user) 64 def new_merge_request(merge_request, current_user)
65 if merge_request.assignee && merge_request.assignee != current_user 65 if merge_request.assignee && merge_request.assignee != current_user
  66 + # skip if assignee notification disabled
  67 + return true if merge_request.assignee.notification.disabled?
  68 +
66 Notify.delay.new_merge_request_email(merge_request.id) 69 Notify.delay.new_merge_request_email(merge_request.id)
67 end 70 end
68 end 71 end
@@ -73,12 +76,7 @@ class NotificationService @@ -73,12 +76,7 @@ class NotificationService
73 # * merge_request assignee if his notification level is not Disabled 76 # * merge_request assignee if his notification level is not Disabled
74 # 77 #
75 def reassigned_merge_request(merge_request, current_user) 78 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 79 + reassign_email(merge_request, current_user, 'reassigned_merge_request_email')
82 end 80 end
83 81
84 # Notify new user with email after creation 82 # Notify new user with email after creation
@@ -93,15 +91,17 @@ class NotificationService @@ -93,15 +91,17 @@ class NotificationService
93 # 91 #
94 def new_note(note) 92 def new_note(note)
95 if note.notify 93 if note.notify
96 - users = note.project.users.reject { |u| u.id == note.author.id } 94 + users = note.project.users
  95 + users = reject_muted_users(users)
  96 + users.delete(note.author)
97 97
98 # Note: wall posts are not "attached" to anything, so fall back to "Wall" 98 # Note: wall posts are not "attached" to anything, so fall back to "Wall"
99 noteable_type = note.noteable_type.presence || "Wall" 99 noteable_type = note.noteable_type.presence || "Wall"
100 notify_method = "note_#{noteable_type.underscore}_email".to_sym 100 notify_method = "note_#{noteable_type.underscore}_email".to_sym
101 101
102 if Notify.respond_to? notify_method 102 if Notify.respond_to? notify_method
103 - team_without_note_author(note).map do |u|  
104 - Notify.delay.send(notify_method, u.id, note.id) 103 + users.each do |user|
  104 + Notify.delay.send(notify_method, user.id, note.id)
105 end 105 end
106 end 106 end
107 elsif note.notify_author && note.commit_author 107 elsif note.notify_author && note.commit_author
@@ -116,4 +116,28 @@ class NotificationService @@ -116,4 +116,28 @@ class NotificationService
116 def update_team_member(users_project) 116 def update_team_member(users_project)
117 Notify.delay.project_access_granted_email(users_project.id) 117 Notify.delay.project_access_granted_email(users_project.id)
118 end 118 end
  119 +
  120 + protected
  121 +
  122 + # Remove users with disabled notifications from array
  123 + # Also remove duplications and nil recipients
  124 + def reject_muted_users(users)
  125 + users.compact.uniq.reject do |user|
  126 + user.notification.disabled?
  127 + end
  128 + end
  129 +
  130 + def reassign_email(target, current_user, entity_sym)
  131 + recipients = User.where(id: [target.assignee_id, target.assignee_id_was])
  132 +
  133 + # reject users with disabled notifications
  134 + recipients = reject_muted_users(recipients)
  135 +
  136 + # Reject me from recipients if I reassign an item
  137 + recipients.delete(current_user)
  138 +
  139 + recipients.each do |recipient_id|
  140 + Notify.delay.send(method, recipient.id, target.id, target.assignee_id_was)
  141 + end
  142 + end
119 end 143 end