diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index f74e6cac174..b2cc39763f3 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -365,7 +365,7 @@ class NotificationService users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq) users_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users) - users_with_group_setting = select_group_member_setting(project, project_members, users_with_group_level_global, users) + users_with_group_setting = select_group_member_setting(project.group, project_members, users_with_group_level_global, users) User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a end @@ -415,8 +415,8 @@ class NotificationService end # Build a list of users based on group notification settings - def select_group_member_setting(project, project_members, global_setting, users_global_level_watch) - uids = notification_settings_for(project, :watch) + def select_group_member_setting(group, project_members, global_setting, users_global_level_watch) + uids = notification_settings_for(group, :watch) # Group setting is watch, add to users list if user is not project member users = [] @@ -473,7 +473,7 @@ class NotificationService setting = user.notification_settings_for(project) - if !setting && project.group + if project.group && (setting.nil? || setting.global?) setting = user.notification_settings_for(project.group) end diff --git a/changelogs/unreleased/395-fix-notification-when-group-set-to-watch.yml b/changelogs/unreleased/395-fix-notification-when-group-set-to-watch.yml new file mode 100644 index 00000000000..11d1f55172b --- /dev/null +++ b/changelogs/unreleased/395-fix-notification-when-group-set-to-watch.yml @@ -0,0 +1,4 @@ +--- +title: Fix notifications when set at group level +merge_request: 6813 +author: Alexandre Maia diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 309985a5d90..7cf2cd9968f 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -443,6 +443,8 @@ describe NotificationService, services: true do before do build_team(issue.project) + build_group(issue.project) + add_users_with_subscription(issue.project, issue) reset_delivered_emails! update_custom_notification(:new_issue, @u_guest_custom, project) @@ -459,6 +461,8 @@ describe NotificationService, services: true do should_email(@u_guest_custom) should_email(@u_custom_global) should_email(@u_participant_mentioned) + should_email(@g_global_watcher) + should_email(@g_watcher) should_not_email(@u_mentioned) should_not_email(@u_participating) should_not_email(@u_disabled) @@ -1218,6 +1222,22 @@ describe NotificationService, services: true do project.add_master(@u_custom_global) end + # Users in the project's group but not part of project's team + # with different notification settings + def build_group(project) + group = create(:group, :public) + project.group = group + + # Group member: global=disabled, group=watch + @g_watcher = create_user_with_notification(:watch, 'group_watcher', project.group) + @g_watcher.notification_settings_for(nil).disabled! + + # Group member: global=watch, group=global + @g_global_watcher = create_global_setting_for(create(:user), :watch) + group.add_users([@g_watcher, @g_global_watcher], :master) + group + end + def create_global_setting_for(user, level) setting = user.global_notification_setting setting.level = level @@ -1226,9 +1246,9 @@ describe NotificationService, services: true do user end - def create_user_with_notification(level, username) + def create_user_with_notification(level, username, resource = project) user = create(:user, username: username) - setting = user.notification_settings_for(project) + setting = user.notification_settings_for(resource) setting.level = level setting.save