Commit f8b5429599254a21f88abb7541ff22b21833103e
Exists in
spb-stable
and in
3 other branches
Merge branch 'honor_notification_level' into 'master'
Honor notification levels Fixes #1093
Showing
2 changed files
with
88 additions
and
25 deletions
Show diff stats
app/services/notification_service.rb
| @@ -178,29 +178,29 @@ class NotificationService | @@ -178,29 +178,29 @@ class NotificationService | ||
| 178 | 178 | ||
| 179 | # Get project users with WATCH notification level | 179 | # Get project users with WATCH notification level |
| 180 | def project_watchers(project) | 180 | def project_watchers(project) |
| 181 | - # Gather all user ids that have WATCH notification setting for project | ||
| 182 | - project_notification_uids = project_notification_list(project, Notification::N_WATCH) | 181 | + project_members = users_project_notification(project) |
| 183 | 182 | ||
| 184 | - # Gather all user ids that have WATCH notification setting for group | ||
| 185 | - group_notification_uids = group_notification_list(project, Notification::N_WATCH) | 183 | + users_with_project_level_global = users_project_notification(project, Notification::N_GLOBAL) |
| 184 | + users_with_group_level_global = users_group_notification(project, Notification::N_GLOBAL) | ||
| 185 | + users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq) | ||
| 186 | 186 | ||
| 187 | - # Gather all user ids that have GLOBAL setting | ||
| 188 | - global_notification_uids = global_notification_list(project) | 187 | + users_with_project_setting = select_users_project_setting(project, users_with_project_level_global, users) |
| 188 | + users_with_group_setting = select_users_group_setting(project, project_members, users_with_group_level_global, users) | ||
| 189 | 189 | ||
| 190 | - project_and_group_uids = [project_notification_uids, group_notification_uids].flatten.uniq | ||
| 191 | - group_and_project_watchers = User.where(id: project_and_group_uids) | ||
| 192 | - | ||
| 193 | - # Find all users that have WATCH as their GLOBAL setting | ||
| 194 | - global_watchers = User.where(id: global_notification_uids, notification_level: Notification::N_WATCH) | ||
| 195 | - | ||
| 196 | - [group_and_project_watchers, global_watchers].flatten.uniq | 190 | + User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a |
| 197 | end | 191 | end |
| 198 | 192 | ||
| 199 | - def project_notification_list(project, notification_level) | ||
| 200 | - project.users_projects.where(notification_level: notification_level).pluck(:user_id) | 193 | + def users_project_notification(project, notification_level=nil) |
| 194 | + project_members = project.users_projects | ||
| 195 | + | ||
| 196 | + if notification_level | ||
| 197 | + project_members.where(notification_level: notification_level).pluck(:user_id) | ||
| 198 | + else | ||
| 199 | + project_members.pluck(:user_id) | ||
| 200 | + end | ||
| 201 | end | 201 | end |
| 202 | 202 | ||
| 203 | - def group_notification_list(project, notification_level) | 203 | + def users_group_notification(project, notification_level) |
| 204 | if project.group | 204 | if project.group |
| 205 | project.group.users_groups.where(notification_level: notification_level).pluck(:user_id) | 205 | project.group.users_groups.where(notification_level: notification_level).pluck(:user_id) |
| 206 | else | 206 | else |
| @@ -208,11 +208,47 @@ class NotificationService | @@ -208,11 +208,47 @@ class NotificationService | ||
| 208 | end | 208 | end |
| 209 | end | 209 | end |
| 210 | 210 | ||
| 211 | - def global_notification_list(project) | ||
| 212 | - [ | ||
| 213 | - project_notification_list(project, Notification::N_GLOBAL), | ||
| 214 | - group_notification_list(project, Notification::N_GLOBAL) | ||
| 215 | - ].flatten | 211 | + def users_with_global_level_watch(ids) |
| 212 | + User.where( | ||
| 213 | + id: ids, | ||
| 214 | + notification_level: Notification::N_WATCH | ||
| 215 | + ).pluck(:id) | ||
| 216 | + end | ||
| 217 | + | ||
| 218 | + # Build a list of users based on project notifcation settings | ||
| 219 | + def select_users_project_setting(project, global_setting, users_global_level_watch) | ||
| 220 | + users = users_project_notification(project, Notification::N_WATCH) | ||
| 221 | + | ||
| 222 | + # If project setting is global, add to watch list if global setting is watch | ||
| 223 | + global_setting.each do |user_id| | ||
| 224 | + if users_global_level_watch.include?(user_id) | ||
| 225 | + users << user_id | ||
| 226 | + end | ||
| 227 | + end | ||
| 228 | + | ||
| 229 | + users | ||
| 230 | + end | ||
| 231 | + | ||
| 232 | + # Build a list of users based on group notifcation settings | ||
| 233 | + def select_users_group_setting(project, project_members, global_setting, users_global_level_watch) | ||
| 234 | + uids = users_group_notification(project, Notification::N_WATCH) | ||
| 235 | + | ||
| 236 | + # Group setting is watch, add to users list if user is not project member | ||
| 237 | + users = [] | ||
| 238 | + uids.each do |user_id| | ||
| 239 | + if project_members.exclude?(user_id) | ||
| 240 | + users << user_id | ||
| 241 | + end | ||
| 242 | + end | ||
| 243 | + | ||
| 244 | + # Group setting is global, add to users list if global setting is watch | ||
| 245 | + global_setting.each do |user_id| | ||
| 246 | + if project_members.exclude?(user_id) && users_global_level_watch.include?(user_id) | ||
| 247 | + users << user_id | ||
| 248 | + end | ||
| 249 | + end | ||
| 250 | + | ||
| 251 | + users | ||
| 216 | end | 252 | end |
| 217 | 253 | ||
| 218 | # Remove users with disabled notifications from array | 254 | # Remove users with disabled notifications from array |
spec/services/notification_service_spec.rb
| @@ -57,15 +57,42 @@ describe NotificationService do | @@ -57,15 +57,42 @@ describe NotificationService do | ||
| 57 | Notify.should_not_receive(:note_issue_email) | 57 | Notify.should_not_receive(:note_issue_email) |
| 58 | notification.new_note(mentioned_note) | 58 | notification.new_note(mentioned_note) |
| 59 | end | 59 | end |
| 60 | + end | ||
| 60 | 61 | ||
| 61 | - def should_email(user_id) | ||
| 62 | - Notify.should_receive(:note_issue_email).with(user_id, note.id) | 62 | + describe 'new note on issue in project that belongs to a group' do |
| 63 | + let(:group) { create(:group) } | ||
| 64 | + | ||
| 65 | + before do | ||
| 66 | + note.project.namespace_id = group.id | ||
| 67 | + note.project.group.add_user(@u_watcher, UsersGroup::MASTER) | ||
| 68 | + note.project.save | ||
| 69 | + user_project = note.project.users_projects.find_by_user_id(@u_watcher.id) | ||
| 70 | + user_project.notification_level = Notification::N_PARTICIPATING | ||
| 71 | + user_project.save | ||
| 72 | + user_group = note.project.group.users_groups.find_by_user_id(@u_watcher.id) | ||
| 73 | + user_group.notification_level = Notification::N_GLOBAL | ||
| 74 | + user_group.save | ||
| 63 | end | 75 | end |
| 64 | 76 | ||
| 65 | - def should_not_email(user_id) | ||
| 66 | - Notify.should_not_receive(:note_issue_email).with(user_id, note.id) | 77 | + it do |
| 78 | + should_email(note.noteable.author_id) | ||
| 79 | + should_email(note.noteable.assignee_id) | ||
| 80 | + should_email(@u_mentioned.id) | ||
| 81 | + should_not_email(@u_watcher.id) | ||
| 82 | + should_not_email(note.author_id) | ||
| 83 | + should_not_email(@u_participating.id) | ||
| 84 | + should_not_email(@u_disabled.id) | ||
| 85 | + notification.new_note(note) | ||
| 67 | end | 86 | end |
| 68 | end | 87 | end |
| 88 | + | ||
| 89 | + def should_email(user_id) | ||
| 90 | + Notify.should_receive(:note_issue_email).with(user_id, note.id) | ||
| 91 | + end | ||
| 92 | + | ||
| 93 | + def should_not_email(user_id) | ||
| 94 | + Notify.should_not_receive(:note_issue_email).with(user_id, note.id) | ||
| 95 | + end | ||
| 69 | end | 96 | end |
| 70 | 97 | ||
| 71 | context 'commit note' do | 98 | context 'commit note' do |