Fix unexpected extra notification mails

Using custom_action and recipient filtering

Add more generic filtering to user_ids_notifiable_on

Add changelog entry

Remove commented class

Method is no longer needed

Overloading no longer required

Filter by action just in case of custom notification level

Add participant check

Fix unexpected extra notification mails

Using custom_action and recipient filtering

Add more generic filtering to user_ids_notifiable_on

Add changelog entry

Remove commented class

Method is no longer needed

Overloading no longer required

Filter by action just in case of custom notification level

Fix comment

Add repond_to? checks

Reverted custom_action filtering

Enhanced output of should_email helper

Changed :watch to :participating for custom notifiable users

Change spec variable name

Enhanced participating check

These conditions are no longer needed

Fix custom notification handling for participating type

Participating level should include maintainers

Fixed add_guest notification

Fix successful pipeline notification

Refactoring: Use maintainer? method on team instead

Add spec for new_issue: true for a custom group setting
which should have lower prio than an available project setting

Clean up specs

6 files changed, 84 insertions, 19 deletions

diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb
index 793cce191fa..377ac3febb6 100644
--- a/app/models/notification_recipient.rb
+++ b/app/models/notification_recipient.rb
@@ -47,14 +47,14 @@ class NotificationRecipient
 
   def suitable_notification_level?
     case notification_level
-    when :disabled, nil
-      false
-    when :custom
-      custom_enabled? || %i[participating mention].include?(@type)
-    when :watch, :participating
-      !action_excluded?
     when :mention
       @type == :mention
+    when :participating
+      !excluded_participating_action? && %i[participating mention watch].include?(@type)
+    when :custom
+      custom_enabled? || %i[participating mention].include?(@type)
+    when :watch
+      !excluded_watcher_action?
     else
       false
     end
@@ -100,18 +100,14 @@ class NotificationRecipient
     end
   end
 
-  def action_excluded?
-    excluded_watcher_action? || excluded_participating_action?
-  end
-
   def excluded_watcher_action?
-    return false unless @custom_action && notification_level == :watch
+    return false unless @custom_action
 
     NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action)
   end
 
   def excluded_participating_action?
-    return false unless @custom_action && notification_level == :participating
+    return false unless @custom_action
 
     NotificationSetting::EXCLUDED_PARTICIPATING_EVENTS.include?(@custom_action)
   end
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 760962346fb..ca3f0b73096 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -135,7 +135,7 @@ module NotificationRecipientService
         global_users_ids = user_ids_with_project_level_global.concat(user_ids_with_group_level_global)
         user_ids += user_ids_with_global_level_custom(global_users_ids, custom_action)
 
-        add_recipients(user_scope.where(id: user_ids), :watch, nil)
+        add_recipients(user_scope.where(id: user_ids), :custom, nil)
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
@@ -391,7 +391,7 @@ module NotificationRecipientService
       def build!
         return [] unless project
 
-        add_recipients(project.team.maintainers, :watch, nil)
+        add_recipients(project.team.maintainers, :mention, nil)
       end
 
       def acting_user
diff --git a/changelogs/unreleased/fix-extra-emails-for-custom-notifications.yml b/changelogs/unreleased/fix-extra-emails-for-custom-notifications.yml
new file mode 100644
index 00000000000..6eb3225e4a1
--- /dev/null
+++ b/changelogs/unreleased/fix-extra-emails-for-custom-notifications.yml
@@ -0,0 +1,5 @@
+---
+title: Fix extra emails for custom notifications
+merge_request: 25607
+author:
+type: fixed
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 8e34406133a..4b40c86574f 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -643,6 +643,64 @@ describe NotificationService, :mailer do
     end
   end
 
+  describe 'Participating project notification settings have priority over group and global settings if available' do
+    let!(:group) { create(:group) }
+    let!(:maintainer) { group.add_owner(create(:user, username: 'maintainer')).user }
+    let!(:user1) { group.add_developer(create(:user, username: 'user_with_project_and_custom_setting')).user }
+
+    let(:project) { create(:project, :public, namespace: group) }
+    let(:issue) { create :issue, project: project, assignees: [assignee], description: '' }
+
+    before do
+      reset_delivered_emails!
+
+      create_notification_setting(user1, project, :participating)
+    end
+
+    context 'custom on group' do
+      [nil, true].each do |new_issue_value|
+        value_caption = new_issue_value || 'nil'
+        it "does not send an email to user1 when a new issue is created and new_issue is set to #{value_caption}" do
+          update_custom_notification(:new_issue, user1, resource: group, value: new_issue_value)
+
+          notification.new_issue(issue, maintainer)
+          should_not_email(user1)
+        end
+      end
+    end
+
+    context 'watch on group' do
+      it 'does not send an email' do
+        user1.notification_settings_for(group).update!(level: :watch)
+
+        notification.new_issue(issue, maintainer)
+        should_not_email(user1)
+      end
+    end
+
+    context 'custom on global, global on group' do
+      it 'does not send an email' do
+        user1.notification_settings_for(nil).update!(level: :custom)
+
+        user1.notification_settings_for(group).update!(level: :global)
+
+        notification.new_issue(issue, maintainer)
+        should_not_email(user1)
+      end
+    end
+
+    context 'watch on global, global on group' do
+      it 'does not send an email' do
+        user1.notification_settings_for(nil).update!(level: :watch)
+
+        user1.notification_settings_for(group).update!(level: :global)
+
+        notification.new_issue(issue, maintainer)
+        should_not_email(user1)
+      end
+    end
+  end
+
   describe 'Issues' do
     let(:group) { create(:group) }
     let(:project) { create(:project, :public, namespace: group) }
@@ -660,7 +718,7 @@ describe NotificationService, :mailer do
     end
 
     describe '#new_issue' do
-      it do
+      it 'notifies the expected users' do
         notification.new_issue(issue, @u_disabled)
 
         should_email(assignee)
@@ -1639,7 +1697,7 @@ describe NotificationService, :mailer do
     end
 
     describe '#project_was_moved' do
-      it do
+      it 'notifies the expected users' do
         notification.project_was_moved(project, "gitlab/gitlab")
 
         should_email(@u_watcher)
diff --git a/spec/support/helpers/email_helpers.rb b/spec/support/helpers/email_helpers.rb
index ad6e1064499..ed049daba80 100644
--- a/spec/support/helpers/email_helpers.rb
+++ b/spec/support/helpers/email_helpers.rb
@@ -16,7 +16,9 @@ module EmailHelpers
   end
 
   def should_email(user, times: 1, recipients: email_recipients)
-    expect(sent_to_user(user, recipients: recipients)).to eq(times)
+    amount = sent_to_user(user, recipients: recipients)
+    failed_message = lambda { "User #{user.username} (#{user.id}): email test failed (expected #{times}, got #{amount})" }
+    expect(amount).to eq(times), failed_message
   end
 
   def should_not_email(user, recipients: email_recipients)
diff --git a/spec/support/helpers/notification_helpers.rb b/spec/support/helpers/notification_helpers.rb
index 8d84510fb73..44c2051598c 100644
--- a/spec/support/helpers/notification_helpers.rb
+++ b/spec/support/helpers/notification_helpers.rb
@@ -17,11 +17,15 @@ module NotificationHelpers
 
   def create_user_with_notification(level, username, resource = project)
     user = create(:user, username: username)
+    create_notification_setting(user, resource, level)
+
+    user
+  end
+
+  def create_notification_setting(user, resource, level)
     setting = user.notification_settings_for(resource)
     setting.level = level
     setting.save
-
-    user
   end
 
   # Create custom notifications