Fix notification when global=disabled, group=watch395-fix-notification-when-group-set-to-watch

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 30 insertions, 6 deletions

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