Apply notification settings level of groups to all child objectsissue_44230

6 files changed, 214 insertions, 27 deletions

diff --git a/app/models/group.rb b/app/models/group.rb
index 8fb77a7869d..fbe4e9f5cc6 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -26,7 +26,11 @@ class Group < Namespace
   has_many :milestones
   has_many :project_group_links, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :shared_projects, through: :project_group_links, source: :project
+
+  # Overriden on another method
+  # Left here just to be dependent: :destroy
   has_many :notification_settings, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
+
   has_many :labels, class_name: 'GroupLabel'
   has_many :variables, class_name: 'Ci::GroupVariable'
   has_many :custom_attributes, class_name: 'GroupCustomAttribute'
@@ -88,6 +92,15 @@ class Group < Namespace
     end
   end
 
+  # Overrides notification_settings has_many association
+  # This allows to apply notification settings from parent groups
+  # to child groups and projects.
+  def notification_settings
+    source_type = self.class.base_class.name
+
+    NotificationSetting.where(source_type: source_type, source_id: self_and_ancestors.select(:id))
+  end
+
   def to_reference(_from = nil, full: nil)
     "#{self.class.reference_prefix}#{full_path}"
   end
diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb
index 2c3580bbdc6..5ab6ca031a5 100644
--- a/app/models/notification_recipient.rb
+++ b/app/models/notification_recipient.rb
@@ -142,10 +142,20 @@ class NotificationRecipient
 
     return project_setting unless project_setting.nil? || project_setting.global?
 
-    group_setting = @group && user.notification_settings_for(@group)
+    group_setting = notification_setting_for(@group)
 
-    return group_setting unless group_setting.nil? || group_setting.global?
+    return group_setting unless group_setting.nil?
 
     user.global_notification_setting
   end
+
+  # Searchs for notification settings with level different from global on group and ancestors
+  def notification_setting_for(group)
+    return unless group
+
+    group.notification_settings.where(user_id: user.id)
+      .where.not(level: NotificationSetting.levels[:global])
+      .order('source_id ASC')
+      .first
+  end
 end
diff --git a/changelogs/unreleased/issue_44230.yml b/changelogs/unreleased/issue_44230.yml
new file mode 100644
index 00000000000..2c6dba6c0fb
--- /dev/null
+++ b/changelogs/unreleased/issue_44230.yml
@@ -0,0 +1,5 @@
+---
+title: Apply notification settings level of groups to all child objects
+merge_request:
+author:
+type: changed
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index f83b52e8975..0f46f01f7f7 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -67,6 +67,30 @@ describe Group do
       end
     end
 
+    describe '#notification_settings', :nested_groups do
+      let(:user) { create (:user) }
+      let(:group) { create(:group) }
+      let(:sub_group) { create(:group, parent_id: group.id) }
+
+      before do
+        group.add_developer(user)
+        sub_group.add_developer(user)
+      end
+
+      it 'also gets notification settings from parent groups' do
+        expect(sub_group.notification_settings.size).to eq(2)
+        expect(sub_group.notification_settings).to include(group.notification_settings.first)
+      end
+
+      context 'when sub group is deleted' do
+        it 'does not delete parent notification settings' do
+          expect do
+            sub_group.destroy
+          end.to change { NotificationSetting.count }.by(-1)
+        end
+      end
+    end
+
     describe '#visibility_level_allowed_by_parent' do
       let(:parent) { create(:group, :internal) }
       let(:sub_group) { build(:group, parent_id: parent.id) }
diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb
index eda0e1da835..c8ac6fd9fd1 100644
--- a/spec/models/notification_recipient_spec.rb
+++ b/spec/models/notification_recipient_spec.rb
@@ -13,4 +13,35 @@ describe NotificationRecipient do
 
     expect(recipient.has_access?).to be_falsy
   end
+
+  context '#notification_setting' do
+    context 'for child groups', :nested_groups do
+      let(:group) { create(:group) }
+      let(:sub_group_1) { create(:group, parent: group) }
+      let(:sub_group_2) { create(:group, parent: sub_group_1) }
+      let(:project) { create(:project, namespace: sub_group_2) }
+
+      context 'when notification setting is global' do
+        before do
+          user.notification_settings_for(group).global!
+          user.notification_settings_for(sub_group_1).mention!
+          user.notification_settings_for(sub_group_2).global!
+        end
+
+        it 'considers notification setting from the first parent without global setting' do
+          expect(subject.notification_setting.source).to eq(sub_group_1)
+        end
+      end
+
+      context 'when notification setting is not global' do
+        before do
+          user.notification_settings_for(sub_group_2).watch!
+        end
+
+        it 'considers notification setting from the current group' do
+          expect(subject.notification_setting.source).to eq(sub_group_2)
+        end
+      end
+    end
+  end
 end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 831678b949d..8e702e32274 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -239,34 +239,56 @@ describe NotificationService, :mailer do
       end
 
       describe 'new note on issue in project that belongs to a group' do
-        let(:group) { create(:group) }
-
         before do
           note.project.namespace_id = group.id
-          note.project.group.add_user(@u_watcher, GroupMember::MASTER)
-          note.project.group.add_user(@u_custom_global, GroupMember::MASTER)
+          group.add_user(@u_watcher, GroupMember::MASTER)
+          group.add_user(@u_custom_global, GroupMember::MASTER)
           note.project.save
 
           @u_watcher.notification_settings_for(note.project).participating!
-          @u_watcher.notification_settings_for(note.project.group).global!
+          @u_watcher.notification_settings_for(group).global!
           update_custom_notification(:new_note, @u_custom_global)
           reset_delivered_emails!
         end
 
-        it do
-          notification.new_note(note)
+        shared_examples 'new note notifications' do
+          it do
+            notification.new_note(note)
+
+            should_email(note.noteable.author)
+            should_email(note.noteable.assignees.first)
+            should_email(@u_mentioned)
+            should_email(@u_custom_global)
+            should_not_email(@u_guest_custom)
+            should_not_email(@u_guest_watcher)
+            should_not_email(@u_watcher)
+            should_not_email(note.author)
+            should_not_email(@u_participating)
+            should_not_email(@u_disabled)
+            should_not_email(@u_lazy_participant)
+          end
+        end
 
-          should_email(note.noteable.author)
-          should_email(note.noteable.assignees.first)
-          should_email(@u_mentioned)
-          should_email(@u_custom_global)
-          should_not_email(@u_guest_custom)
-          should_not_email(@u_guest_watcher)
-          should_not_email(@u_watcher)
-          should_not_email(note.author)
-          should_not_email(@u_participating)
-          should_not_email(@u_disabled)
-          should_not_email(@u_lazy_participant)
+        let(:group) { create(:group) }
+
+        it_behaves_like 'new note notifications'
+
+        context 'which is a subgroup', :nested_groups do
+          let!(:parent) { create(:group) }
+          let!(:group) { create(:group, parent: parent) }
+
+          it_behaves_like 'new note notifications'
+
+          it 'overrides child objects with global level' do
+            user = create(:user)
+            parent.add_developer(user)
+            user.notification_settings_for(parent).watch!
+            reset_delivered_emails!
+
+            notification.new_note(note)
+
+            should_email(user)
+          end
         end
       end
     end
@@ -301,6 +323,31 @@ describe NotificationService, :mailer do
         should_email(member)
         should_email(admin)
       end
+
+      context 'on project that belongs to subgroup', :nested_groups do
+        let(:group_reporter) { create(:user) }
+        let(:group_guest) { create(:user) }
+        let(:parent_group) { create(:group) }
+        let(:child_group) { create(:group, parent: parent_group) }
+        let(:project) { create(:project, namespace: child_group) }
+
+        context 'when user is group guest member' do
+          before do
+            parent_group.add_reporter(group_reporter)
+            parent_group.add_guest(group_guest)
+            group_guest.notification_settings_for(parent_group).watch!
+            group_reporter.notification_settings_for(parent_group).watch!
+            reset_delivered_emails!
+          end
+
+          it 'does not email guest user' do
+            notification.new_note(note)
+
+            should_email(group_reporter)
+            should_not_email(group_guest)
+          end
+        end
+      end
     end
 
     context 'issue note mention' do
@@ -311,6 +358,7 @@ describe NotificationService, :mailer do
 
       before do
         build_team(note.project)
+        build_group(note.project)
         note.project.add_master(note.author)
         add_users_with_subscription(note.project, issue)
         reset_delivered_emails!
@@ -336,12 +384,22 @@ describe NotificationService, :mailer do
           should_email(@u_guest_watcher)
           should_email(note.noteable.author)
           should_email(note.noteable.assignees.first)
+          should_email_nested_group_user(@pg_watcher)
           should_not_email(note.author)
           should_email(@u_mentioned)
           should_not_email(@u_disabled)
+          should_not_email_nested_group_user(@pg_disabled)
           should_email(@u_not_mentioned)
         end
 
+        it 'notifies parent group members with mention level', :nested_groups do
+          note = create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: "@#{@pg_mention.username}")
+
+          notification.new_note(note)
+
+          should_email_nested_group_user(@pg_mention)
+        end
+
         it 'filters out "mentioned in" notes' do
           mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author)
 
@@ -352,17 +410,18 @@ describe NotificationService, :mailer do
     end
 
     context 'project snippet note' do
-      let(:project) { create(:project, :public) }
+      let!(:project) { create(:project, :public) }
       let(:snippet) { create(:project_snippet, project: project, author: create(:user)) }
-      let(:note) { create(:note_on_project_snippet, noteable: snippet, project_id: snippet.project.id, note: '@all mentioned') }
+      let(:note) { create(:note_on_project_snippet, noteable: snippet, project_id: project.id, note: '@all mentioned') }
 
       before do
-        build_team(note.project)
+        build_team(project)
+        build_group(project)
 
         # make sure these users can read the project snippet!
         project.add_guest(@u_guest_watcher)
         project.add_guest(@u_guest_custom)
-
+        add_member_for_parent_group(@pg_watcher, project)
         note.project.add_master(note.author)
         reset_delivered_emails!
       end
@@ -370,7 +429,6 @@ describe NotificationService, :mailer do
       describe '#new_note' do
         it 'notifies the team members' do
           notification.new_note(note)
-
           # Notify all team members
           note.project.team.members.each do |member|
             # User with disabled notification should not be notified
@@ -547,7 +605,9 @@ describe NotificationService, :mailer do
         should_email(@u_participant_mentioned)
         should_email(@g_global_watcher)
         should_email(@g_watcher)
+        should_email_nested_group_user(@pg_watcher)
         should_email(@unsubscribed_mentioned)
+        should_not_email_nested_group_user(@pg_disabled)
         should_not_email(@u_mentioned)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -1922,8 +1982,8 @@ describe NotificationService, :mailer do
   # 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 = create_nested_group
+    project.update(namespace_id: group.id)
 
     # Group member: global=disabled, group=watch
     @g_watcher = create_user_with_notification(:watch, 'group_watcher', project.group)
@@ -1932,9 +1992,53 @@ describe NotificationService, :mailer do
     # 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
 
+  # Creates a nested group only if supported
+  # to avoid errors on MySQL
+  def create_nested_group
+    if Group.supports_nested_groups?
+      parent_group = create(:group, :public)
+      child_group = create(:group, :public, parent: parent_group)
+
+      # Parent group member: global=disabled, parent_group=watch, child_group=global
+      @pg_watcher = create_user_with_notification(:watch, 'parent_group_watcher', parent_group)
+      @pg_watcher.notification_settings_for(nil).disabled!
+
+      # Parent group member: global=global, parent_group=disabled, child_group=global
+      @pg_disabled = create_user_with_notification(:disabled, 'parent_group_disabled', parent_group)
+      @pg_disabled.notification_settings_for(nil).disabled!
+
+      # Parent group member: global=global, parent_group=mention, child_group=global
+      @pg_mention = create_user_with_notification(:disabled, 'parent_group_mention', parent_group)
+      @pg_mention.notification_settings_for(nil).mention!
+
+      child_group
+    else
+      create(:group, :public)
+    end
+  end
+
+  def add_member_for_parent_group(user, project)
+    return unless Group.supports_nested_groups?
+
+    project.group.parent.add_master(user)
+  end
+
+  def should_email_nested_group_user(user, times: 1, recipients: email_recipients)
+    return unless Group.supports_nested_groups?
+
+    should_email(user, times: 1, recipients: email_recipients)
+  end
+
+  def should_not_email_nested_group_user(user, recipients: email_recipients)
+    return unless Group.supports_nested_groups?
+
+    should_not_email(user, recipients: email_recipients)
+  end
+
   def add_users_with_subscription(project, issuable)
     @subscriber = create :user
     @unsubscriber = create :user