Merge branch 'bugfix.notify-custom-participants' into 'master'

Bugfix.notify custom participants

Closes #36610

See merge request !13680

app/models/notification_recipient.rb

@@ -19,46 +19,45 @@ class NotificationRecipient
     @notification_setting ||= find_notification_setting
   end
 
-  def raw_notification_level
-    notification_setting&.level&.to_sym
-  end
-
   def notification_level
-    # custom is treated the same as watch if it's enabled - otherwise it's
-    # set to :custom, meaning to send exactly when our type is :participating
-    # or :mention.
-    @notification_level ||=
-      case raw_notification_level
-      when :custom
-        if @custom_action && notification_setting&.event_enabled?(@custom_action)
-          :watch
-        else
-          :custom
-        end
-      else
-        raw_notification_level
-      end
+    @notification_level ||= notification_setting&.level&.to_sym
   end
 
   def notifiable?
     return false unless has_access?
     return false if own_activity?
 
-    return true if @type == :subscription
-
-    return false if notification_level.nil? || notification_level == :disabled
-
-    return %i[participating mention].include?(@type) if notification_level == :custom
+    # even users with :disabled notifications receive manual subscriptions
+    return !unsubscribed? if @type == :subscription
 
-    return false if %i[watch participating].include?(notification_level) && excluded_watcher_action?
-
-    return false unless NotificationSetting.levels[notification_level] <= NotificationSetting.levels[@type]
+    return false unless suitable_notification_level?
 
+    # check this last because it's expensive
+    # nobody should receive notifications if they've specifically unsubscribed
     return false if unsubscribed?
 
     true
   end
 
+  def suitable_notification_level?
+    case notification_level
+    when :disabled, nil
+      false
+    when :custom
+      custom_enabled? || %i[participating mention].include?(@type)
+    when :watch, :participating
+      !excluded_watcher_action?
+    when :mention
+      @type == :mention
+    else
+      false
+    end
+  end
+
+  def custom_enabled?
+    @custom_action && notification_setting&.event_enabled?(@custom_action)
+  end
+
   def unsubscribed?
     return false unless @target
     return false unless @target.respond_to?(:subscriptions)
@@ -88,7 +87,7 @@ class NotificationRecipient
 
   def excluded_watcher_action?
     return false unless @custom_action
-    return false if raw_notification_level == :custom
+    return false if notification_level == :custom
 
     NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action)
   end

app/services/notification_recipient_service.rb

@@ -95,7 +95,7 @@ module NotificationRecipientService
       def add_participants(user)
         return unless target.respond_to?(:participants)
 
-        self << [target.participants(user), :watch]
+        self << [target.participants(user), :participating]
       end
 
       # Get project/group users with CUSTOM notification level

changelogs/unreleased/bugfix-notify-custom-participants.yml

+---
+title: Fixed: Notifications weren't sending to participating users with a `Custom` notification setting.
+merge_request: 13680
+author: jneen
+type: fixed

spec/services/notification_service_spec.rb

@@ -126,7 +126,18 @@ describe NotificationService, :mailer do
         project.add_master(issue.author)
         project.add_master(assignee)
         project.add_master(note.author)
-        create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy')
+
+        @u_custom_off = create_user_with_notification(:custom, 'custom_off')
+        project.add_guest(@u_custom_off)
+
+        create(
+          :note_on_issue,
+          author: @u_custom_off,
+          noteable: issue,
+          project_id: issue.project_id,
+          note: 'i think @subscribed_participant should see this'
+        )
+
         update_custom_notification(:new_note, @u_guest_custom, resource: project)
         update_custom_notification(:new_note, @u_custom_global)
       end
@@ -136,8 +147,7 @@ describe NotificationService, :mailer do
           add_users_with_subscription(note.project, issue)
           reset_delivered_emails!
 
-          # Ensure create SentNotification by noteable = issue 6 times, not noteable = note
-          expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times
+          expect(SentNotification).to receive(:record).with(issue, any_args).exactly(9).times
 
           notification.new_note(note)
 
@@ -149,6 +159,7 @@ describe NotificationService, :mailer do
           should_email(@subscriber)
           should_email(@watcher_and_subscriber)
           should_email(@subscribed_participant)
+          should_email(@u_custom_off)
           should_not_email(@u_guest_custom)
           should_not_email(@u_guest_watcher)
           should_not_email(note.author)