From 556bb46298f6035de001951fac3fe186b23e69fe Mon Sep 17 00:00:00 2001
From: Densett <kevin.morisseau@protonmail.com>
Date: Wed, 22 Jan 2025 13:46:43 +0000
Subject: [PATCH 01/21] Add pipeline events notifications for service accounts

---
 ..._add_new_event_to_notification_settings.rb | 36 +++++++++++++++++++
 db/schema_migrations/20250122103435           |  1 +
 db/structure.sql                              |  5 ++-
 ee/app/models/ee/notification_setting.rb      | 10 ++++--
 4 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 db/migrate/20250122103435_add_new_event_to_notification_settings.rb
 create mode 100644 db/schema_migrations/20250122103435

diff --git a/db/migrate/20250122103435_add_new_event_to_notification_settings.rb b/db/migrate/20250122103435_add_new_event_to_notification_settings.rb
new file mode 100644
index 00000000000000..4b292481970ac4
--- /dev/null
+++ b/db/migrate/20250122103435_add_new_event_to_notification_settings.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddNewEventToNotificationSettings < Gitlab::Database::Migration[2.2]
+  # When using the methods "add_concurrent_index" or "remove_concurrent_index"
+  # you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+  #
+  # Configure the `gitlab_schema` to perform data manipulation (DML).
+  # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html
+  # restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+  # Add dependent 'batched_background_migrations.queued_migration_version' values.
+  # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = []
+  milestone '17.9'
+
+  def change
+    add_column :notification_settings, :failed_pipeline_for_service_account, :boolean, null: false, default: false,
+      if_not_exists: true
+    add_column :notification_settings, :success_pipeline_for_service_account, :boolean, null: false, default: false,
+      if_not_exists: true
+    add_column :notification_settings, :fixed_pipeline_for_service_account, :boolean, null: false, default: false,
+      if_not_exists: true
+  end
+end
diff --git a/db/schema_migrations/20250122103435 b/db/schema_migrations/20250122103435
new file mode 100644
index 00000000000000..100b27a3c57f83
--- /dev/null
+++ b/db/schema_migrations/20250122103435
@@ -0,0 +1 @@
+d1cb98ab058fbaffe8bcd5ad66a3c7f8729c3b861999ad3411597545749c9051
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index bada3ef3aef231..1132a9a69f925e 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -18247,7 +18247,10 @@ CREATE TABLE notification_settings (
     moved_project boolean DEFAULT true NOT NULL,
     change_reviewer_merge_request boolean,
     merge_when_pipeline_succeeds boolean DEFAULT false NOT NULL,
-    approver boolean DEFAULT false NOT NULL
+    approver boolean DEFAULT false NOT NULL,
+    failed_pipeline_for_service_account boolean DEFAULT false NOT NULL,
+    success_pipeline_for_service_account boolean DEFAULT false NOT NULL,
+    fixed_pipeline_for_service_account boolean DEFAULT false NOT NULL
 );
 
 CREATE SEQUENCE notification_settings_id_seq
diff --git a/ee/app/models/ee/notification_setting.rb b/ee/app/models/ee/notification_setting.rb
index 396a51a78d65aa..1e1f69b8764848 100644
--- a/ee/app/models/ee/notification_setting.rb
+++ b/ee/app/models/ee/notification_setting.rb
@@ -5,9 +5,15 @@ module NotificationSetting
     extend ActiveSupport::Concern
 
     EMAIL_EVENTS_MAPPING = {
-      ::Group => [:new_epic],
+      ::Group => [:new_epic,
+        :failed_pipeline_for_service_account,
+        :success_pipeline_for_service_account,
+        :fixed_pipeline_for_service_account],
       ::User => [:approver],
-      ::Project => [:approver]
+      ::Project => [:approver,
+        :failed_pipeline_for_service_account,
+        :success_pipeline_for_service_account,
+        :fixed_pipeline_for_service_account]
     }.freeze
     FULL_EMAIL_EVENTS = EMAIL_EVENTS_MAPPING.values.flatten.freeze
 
-- 
GitLab


From 07d811f1afa2b838bed640470f1f71a37fdf0a0e Mon Sep 17 00:00:00 2001
From: Geoffrey McQuat <gmcq@protonmail.com>
Date: Wed, 22 Jan 2025 15:38:45 +0000
Subject: [PATCH 02/21] Service Account pipelines notification events UI text

---
 app/assets/javascripts/notifications/constants.js | 10 +++++++++-
 locale/gitlab.pot                                 |  9 +++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/notifications/constants.js b/app/assets/javascripts/notifications/constants.js
index 690a24015a3e4b..aafbc28dd43c22 100644
--- a/app/assets/javascripts/notifications/constants.js
+++ b/app/assets/javascripts/notifications/constants.js
@@ -42,8 +42,12 @@ export const i18n = {
     close_issue: s__('NotificationEvent|Issue is closed'),
     close_merge_request: s__('NotificationEvent|Merge request is closed'),
     failed_pipeline: s__('NotificationEvent|Pipeline fails'),
+    failed_pipeline_for_service_account: s__('NotificationEvent|Pipeline by Service Account fails'),
     fixed_pipeline: s__('NotificationEvent|Pipeline is fixed'),
-    issue_due: s__('NotificationEvent|Issue is due tomorrow'),
+    fixed_pipeline_for_service_account: s__(
+      'NotificationEvent|Pipeline by Service Account is fixed',
+    ),
+    issue_due: s__('NotificationEvent|Issue is due soon'),
     merge_merge_request: s__('NotificationEvent|Merge request is merged'),
     merge_when_pipeline_succeeds: s__('NotificationEvent|Merge request is set to auto-merge'),
     moved_project: s__('NotificationEvent|Project is moved'),
@@ -58,5 +62,9 @@ export const i18n = {
     reopen_issue: s__('NotificationEvent|Issue is reopened'),
     reopen_merge_request: s__('NotificationEvent|Merge request is reopened'),
     success_pipeline: s__('NotificationEvent|Pipeline is successful'),
+    success_pipeline_for_service_account: s__(
+      'NotificationEvent|Pipeline by Service Account is successful',
+    ),
+    approver: s__('NotificationEvent|You are added as an approver on a merge request'),
   },
 };
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 6a469c542dab48..1878113a40b75a 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -40671,6 +40671,15 @@ msgstr ""
 msgid "NotificationEvent|New release"
 msgstr ""
 
+msgid "NotificationEvent|Pipeline by Service Account fails"
+msgstr ""
+
+msgid "NotificationEvent|Pipeline by Service Account is fixed"
+msgstr ""
+
+msgid "NotificationEvent|Pipeline by Service Account is successful"
+msgstr ""
+
 msgid "NotificationEvent|Pipeline fails"
 msgstr ""
 
-- 
GitLab


From 3b8f2f30cf5fd9885d487bb6432cdc30c0c2d625 Mon Sep 17 00:00:00 2001
From: Densett <kevin.morisseau@protonmail.com>
Date: Wed, 22 Jan 2025 15:53:36 +0000
Subject: [PATCH 03/21] Looking for notifiable users

---
 app/services/notification_service.rb | 46 +++++++++++++++++++---------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 5c7f8853e99887..8d676f4f1a2037 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -60,9 +60,9 @@ def disabled_two_factor(user)
   # it won't be sent to internal users like the
   # ghost user or the EE support bot.
   def new_key(key)
-    if key.user&.can?(:receive_notifications)
-      mailer.new_ssh_key_email(key.id).deliver_later
-    end
+    return unless key.user&.can?(:receive_notifications)
+
+    mailer.new_ssh_key_email(key.id).deliver_later
   end
 
   # Always notify the user about gpg key added
@@ -70,9 +70,9 @@ def new_key(key)
   # This is a security email so it will be sent even if the user disabled
   # notifications
   def new_gpg_key(gpg_key)
-    if gpg_key.user&.can?(:receive_notifications)
-      mailer.new_gpg_key_email(gpg_key.id).deliver_later
-    end
+    return unless gpg_key.user&.can?(:receive_notifications)
+
+    mailer.new_gpg_key_email(gpg_key.id).deliver_later
   end
 
   def bot_resource_access_token_about_to_expire(bot_user, token_name, params = {})
@@ -272,7 +272,8 @@ def push_to_merge_request(merge_request, current_user, new_commits: [], existing
   end
 
   def change_in_merge_request_draft_status(merge_request, current_user)
-    recipients = NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: "draft_status_change")
+    recipients = NotificationRecipients::BuildService.build_recipients(merge_request, current_user,
+      action: "draft_status_change")
 
     recipients.each do |recipient|
       mailer.send(
@@ -362,7 +363,8 @@ def changed_reviewer_of_merge_request(merge_request, current_user, previous_revi
   end
 
   def review_requested_of_merge_request(merge_request, current_user, reviewer)
-    recipients = NotificationRecipients::BuildService.build_requested_review_recipients(merge_request, current_user, reviewer)
+    recipients = NotificationRecipients::BuildService.build_requested_review_recipients(merge_request, current_user,
+      reviewer)
 
     deliver_option = review_request_deliver_options(merge_request.project)
 
@@ -414,7 +416,8 @@ def resolve_all_discussions(merge_request, current_user)
       action: "resolve_all_discussions")
 
     recipients.each do |recipient|
-      mailer.resolved_all_discussions_email(recipient.user.id, merge_request.id, current_user.id, recipient.reason).deliver_later
+      mailer.resolved_all_discussions_email(recipient.user.id, merge_request.id, current_user.id,
+        recipient.reason).deliver_later
     end
   end
 
@@ -438,7 +441,7 @@ def new_note(note)
   end
 
   def send_new_note_notifications(note)
-    notify_method = "note_#{note.noteable_ability_name}_email".to_sym
+    notify_method = :"note_#{note.noteable_ability_name}_email"
 
     recipients = NotificationRecipients::BuildService.build_new_note_recipients(note)
     recipients.each do |recipient|
@@ -594,7 +597,19 @@ def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
       [pipeline.user], :watch,
       custom_action: :"#{status}_pipeline",
       target: pipeline
-    ).map do |user|
+    )
+
+    binding.pry_shell # rubocop:disable Lint/Debugger
+
+    if pipeline.user.service_account?
+
+      finder = MembersFinder.new(project)
+      finder.execute.preload_user_and_notification_settings.map(&:user)
+      recipients += NotificationRecipients::Builder::Base.add_recipients(notifiable_users(finder), nil, nil)
+
+    end
+
+    recipients.map do |user|
       user.notification_email_for(pipeline.project.group)
     end
 
@@ -854,7 +869,8 @@ def close_resource_email(target, current_user, method, skip_current_user: true,
     )
 
     recipients.each do |recipient|
-      mailer.send(method, recipient.user.id, target.id, current_user.id, reason: recipient.reason, closed_via: closed_via).deliver_later
+      mailer.send(method, recipient.user.id, target.id, current_user.id, reason: recipient.reason,
+        closed_via: closed_via).deliver_later
     end
   end
 
@@ -922,7 +938,8 @@ def approve_mr_email(merge_request, project, current_user)
   end
 
   def unapprove_mr_email(merge_request, project, current_user)
-    recipients = ::NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: 'unapprove')
+    recipients = ::NotificationRecipients::BuildService.build_recipients(merge_request, current_user,
+      action: 'unapprove')
 
     recipients.each do |recipient|
       mailer.unapproved_merge_request_email(recipient.user.id, merge_request.id, current_user.id).deliver_later
@@ -1021,7 +1038,8 @@ def notifiable_users(...)
   end
 
   def warn_skipping_notifications(user, object)
-    Gitlab::AppLogger.warn(message: "Skipping sending notifications", user: user.id, klass: object.class.to_s, object_id: object.id)
+    Gitlab::AppLogger.warn(message: "Skipping sending notifications", user: user.id, klass: object.class.to_s,
+      object_id: object.id)
   end
 
   def new_review_deliver_options(review)
-- 
GitLab


From fc1ef9e3ddf12281e0bff7531b599c02b169653b Mon Sep 17 00:00:00 2001
From: gilles_dehaudt <tonton1728@gmail.com>
Date: Thu, 23 Jan 2025 09:08:54 +0000
Subject: [PATCH 04/21] work in progress

---
 .../builder/service_account.rb                | 20 +++++++++++++++++++
 app/services/notification_service.rb          |  9 +++++----
 2 files changed, 25 insertions(+), 4 deletions(-)
 create mode 100644 app/services/notification_recipients/builder/service_account.rb

diff --git a/app/services/notification_recipients/builder/service_account.rb b/app/services/notification_recipients/builder/service_account.rb
new file mode 100644
index 00000000000000..7fc5561a090a66
--- /dev/null
+++ b/app/services/notification_recipients/builder/service_account.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module NotificationRecipients
+  module Builder
+    class ServiceAccount < Base
+      attr_reader :project, :current_user
+
+      def initialize(project, current_user)
+        @project = project
+        @current_user = current_user
+      end
+
+      def build!
+      end
+
+      def target
+      end
+    end
+  end
+end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 8d676f4f1a2037..279f3fb1cb70cc 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -599,13 +599,14 @@ def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
       target: pipeline
     )
 
-    binding.pry_shell # rubocop:disable Lint/Debugger
+    
 
     if pipeline.user.service_account?
 
-      finder = MembersFinder.new(project)
-      finder.execute.preload_user_and_notification_settings.map(&:user)
-      recipients += NotificationRecipients::Builder::Base.add_recipients(notifiable_users(finder), nil, nil)
+      finder = MembersFinder.new(pipeline.project, nil)
+      users = finder.execute.preload_user_and_notification_settings.map(&:user)
+      binding.pry_shell # rubocop:disable Lint/Debugger
+      recipients += NotificationRecipients::Builder::ServiceAccount.new(pipeline.project, pipeline.user).add_recipients(users, :custom, nil)
 
     end
 
-- 
GitLab


From 2a3bedcce6a229aede205e4dd6fb2c5e4e9335dc Mon Sep 17 00:00:00 2001
From: gilles_dehaudt <tonton1728@gmail.com>
Date: Thu, 23 Jan 2025 10:55:14 +0000
Subject: [PATCH 05/21] work in progress

---
 .../notification_recipients/build_service.rb     |  4 ++++
 .../builder/service_account.rb                   | 16 ++++++++++++++--
 app/services/notification_service.rb             | 11 ++---------
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb
index 04563d180b58a9..506e55973df719 100644
--- a/app/services/notification_recipients/build_service.rb
+++ b/app/services/notification_recipients/build_service.rb
@@ -36,5 +36,9 @@ def self.build_new_review_recipients(...)
     def self.build_requested_review_recipients(...)
       ::NotificationRecipients::Builder::RequestReview.new(...).notification_recipients
     end
+
+    def self.build_service_account_recipients(...)
+      ::NotificationRecipients::Builder::ServiceAccount.new(...).build!.map(&:user)
+    end
   end
 end
diff --git a/app/services/notification_recipients/builder/service_account.rb b/app/services/notification_recipients/builder/service_account.rb
index 7fc5561a090a66..b92dd2ea381d4e 100644
--- a/app/services/notification_recipients/builder/service_account.rb
+++ b/app/services/notification_recipients/builder/service_account.rb
@@ -3,17 +3,29 @@
 module NotificationRecipients
   module Builder
     class ServiceAccount < Base
-      attr_reader :project, :current_user
+      attr_reader :project, :current_user, :pipeline_status
 
-      def initialize(project, current_user)
+      def initialize(project, current_user, pipeline_status)
         @project = project
         @current_user = current_user
+        @pipeline_status = pipeline_status
       end
 
       def build!
+        notification_by_sources = related_notification_settings_sources(:custom)
+
+        return if notification_by_sources.blank?
+
+        user_ids = NotificationSetting.from_union(notification_by_sources).select(:user_id)
+        add_recipients(user_scope.where(id: user_ids), :custom, nil)
       end
 
       def target
+        
+      end
+
+      def custom_action
+        @custom_action ||= "#{pipeline_status}_pipeline_for_service_account".to_sym
       end
     end
   end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 279f3fb1cb70cc..7f06ced69239f5 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -599,18 +599,11 @@ def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
       target: pipeline
     )
 
-    
-
     if pipeline.user.service_account?
-
-      finder = MembersFinder.new(pipeline.project, nil)
-      users = finder.execute.preload_user_and_notification_settings.map(&:user)
-      binding.pry_shell # rubocop:disable Lint/Debugger
-      recipients += NotificationRecipients::Builder::ServiceAccount.new(pipeline.project, pipeline.user).add_recipients(users, :custom, nil)
+      recipients += NotificationRecipients::BuildService.build_service_account_recipients(pipeline.project, pipeline.user, status)
 
     end
-
-    recipients.map do |user|
+    recipients = recipients.uniq.map do |user|
       user.notification_email_for(pipeline.project.group)
     end
 
-- 
GitLab


From 43f9fcbd2ae23c4849ee90478afdea641d65ddd6 Mon Sep 17 00:00:00 2001
From: gilles_dehaudt <tonton1728@gmail.com>
Date: Thu, 23 Jan 2025 14:35:43 +0000
Subject: [PATCH 06/21] Seems to work with code added in EE section

---
 .../notification_recipients/build_service.rb  |  6 ++--
 app/services/notification_service.rb          | 28 +++++++++++--------
 .../notification_recipients/build_service.rb  | 14 ++++++++++
 ee/app/services/ee/notification_service.rb    | 16 +++++++++++
 4 files changed, 48 insertions(+), 16 deletions(-)
 create mode 100644 ee/app/services/ee/notification_recipients/build_service.rb

diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb
index 506e55973df719..7e570373f35e19 100644
--- a/app/services/notification_recipients/build_service.rb
+++ b/app/services/notification_recipients/build_service.rb
@@ -36,9 +36,7 @@ def self.build_new_review_recipients(...)
     def self.build_requested_review_recipients(...)
       ::NotificationRecipients::Builder::RequestReview.new(...).notification_recipients
     end
-
-    def self.build_service_account_recipients(...)
-      ::NotificationRecipients::Builder::ServiceAccount.new(...).build!.map(&:user)
-    end
   end
 end
+
+NotificationRecipients::BuildService.prepend_mod
\ No newline at end of file
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 7f06ced69239f5..dfad1dcf2f3791 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -583,30 +583,34 @@ def project_not_exported(project, current_user, errors)
     mailer.project_was_not_exported_email(current_user, project, errors).deliver_later
   end
 
+  def email_template_name(status)
+    "pipeline_#{status}_email"
+  end
+
   def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
     # Must always check project configuration since recipients could be a list of emails
     # from the PipelinesEmailService integration.
     return if pipeline.project.emails_disabled?
 
+    # If changing the next line don't forget to do the same in EE section
     status = pipeline_notification_status(ref_status, pipeline)
-    email_template = "pipeline_#{status}_email"
+    email_template = email_template_name(status)
 
     return unless mailer.respond_to?(email_template)
 
-    recipients ||= notifiable_users(
-      [pipeline.user], :watch,
-      custom_action: :"#{status}_pipeline",
-      target: pipeline
-    )
+    unless recipients
+      recipients = notifiable_users(
+        [pipeline.user], :watch,
+        custom_action: :"#{status}_pipeline",
+        target: pipeline
+      )
 
-    if pipeline.user.service_account?
-      recipients += NotificationRecipients::BuildService.build_service_account_recipients(pipeline.project, pipeline.user, status)
 
+      recipients = recipients.uniq.map do |user|
+        user.notification_email_for(pipeline.project.group)
+      end
     end
-    recipients = recipients.uniq.map do |user|
-      user.notification_email_for(pipeline.project.group)
-    end
-
+    binding.pry_shell
     recipients.each do |recipient|
       mailer.public_send(email_template, pipeline, recipient).deliver_later
     end
diff --git a/ee/app/services/ee/notification_recipients/build_service.rb b/ee/app/services/ee/notification_recipients/build_service.rb
new file mode 100644
index 00000000000000..f37fb35270ef81
--- /dev/null
+++ b/ee/app/services/ee/notification_recipients/build_service.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+#
+# Used by NotificationService to determine who should receive notification
+#
+module EE
+  module NotificationRecipients
+    module BuildService
+      def self.build_service_account_recipients(...)
+        ::NotificationRecipients::Builder::ServiceAccount.new(...).build!.map(&:user)
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/ee/app/services/ee/notification_service.rb b/ee/app/services/ee/notification_service.rb
index c18d5cd1e3ef43..f8e57501f71614 100644
--- a/ee/app/services/ee/notification_service.rb
+++ b/ee/app/services/ee/notification_service.rb
@@ -110,6 +110,22 @@ def no_more_seats(namespace, recipients, user, requested_member_list)
       end
     end
 
+    def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
+      return if super.nil? || !pipeline.user.service_account? || recipients.present?
+
+      binding.pry_shell
+      status = pipeline_notification_status(ref_status, pipeline)
+      email_template = email_template_name(status)
+
+      recipients = NotificationRecipients::BuildService.build_service_account_recipients(pipeline.project, pipeline.user, status)
+
+      recipients.uniq.map do |user|
+        recipient = user.notification_email_for(pipeline.project.group)
+        mailer.public_send(email_template, pipeline, recipient).deliver_later
+      end
+
+    end
+
     private
 
     def oncall_user_removed_recipients(rotation, removed_user)
-- 
GitLab


From 2fc5d3f881c18f5dd68233ddab261f5b70b1a972 Mon Sep 17 00:00:00 2001
From: gilles_dehaudt <tonton1728@gmail.com>
Date: Thu, 23 Jan 2025 14:36:03 +0000
Subject: [PATCH 07/21] Seems to work with code added in EE section

---
 .../builder/service_account.rb                | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 ee/app/services/notification_recipients/builder/service_account.rb

diff --git a/ee/app/services/notification_recipients/builder/service_account.rb b/ee/app/services/notification_recipients/builder/service_account.rb
new file mode 100644
index 00000000000000..b92dd2ea381d4e
--- /dev/null
+++ b/ee/app/services/notification_recipients/builder/service_account.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module NotificationRecipients
+  module Builder
+    class ServiceAccount < Base
+      attr_reader :project, :current_user, :pipeline_status
+
+      def initialize(project, current_user, pipeline_status)
+        @project = project
+        @current_user = current_user
+        @pipeline_status = pipeline_status
+      end
+
+      def build!
+        notification_by_sources = related_notification_settings_sources(:custom)
+
+        return if notification_by_sources.blank?
+
+        user_ids = NotificationSetting.from_union(notification_by_sources).select(:user_id)
+        add_recipients(user_scope.where(id: user_ids), :custom, nil)
+      end
+
+      def target
+        
+      end
+
+      def custom_action
+        @custom_action ||= "#{pipeline_status}_pipeline_for_service_account".to_sym
+      end
+    end
+  end
+end
-- 
GitLab


From 1c631ebf60924d89edf7dfb5addc82384c20837e Mon Sep 17 00:00:00 2001
From: gilles_dehaudt <tonton1728@gmail.com>
Date: Thu, 23 Jan 2025 14:36:46 +0000
Subject: [PATCH 08/21] Seems to work with code added in EE section

---
 .../builder/service_account.rb                | 32 -------------------
 1 file changed, 32 deletions(-)
 delete mode 100644 app/services/notification_recipients/builder/service_account.rb

diff --git a/app/services/notification_recipients/builder/service_account.rb b/app/services/notification_recipients/builder/service_account.rb
deleted file mode 100644
index b92dd2ea381d4e..00000000000000
--- a/app/services/notification_recipients/builder/service_account.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-# frozen_string_literal: true
-
-module NotificationRecipients
-  module Builder
-    class ServiceAccount < Base
-      attr_reader :project, :current_user, :pipeline_status
-
-      def initialize(project, current_user, pipeline_status)
-        @project = project
-        @current_user = current_user
-        @pipeline_status = pipeline_status
-      end
-
-      def build!
-        notification_by_sources = related_notification_settings_sources(:custom)
-
-        return if notification_by_sources.blank?
-
-        user_ids = NotificationSetting.from_union(notification_by_sources).select(:user_id)
-        add_recipients(user_scope.where(id: user_ids), :custom, nil)
-      end
-
-      def target
-        
-      end
-
-      def custom_action
-        @custom_action ||= "#{pipeline_status}_pipeline_for_service_account".to_sym
-      end
-    end
-  end
-end
-- 
GitLab


From ca2575e5b94228406b37cc3492aa223002d0551b Mon Sep 17 00:00:00 2001
From: gilles_dehaudt <tonton1728@gmail.com>
Date: Thu, 23 Jan 2025 14:52:43 +0000
Subject: [PATCH 09/21] remove unwanted modifications

---
 app/services/notification_service.rb       | 51 +++++++++-------------
 ee/app/services/ee/notification_service.rb |  1 -
 2 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index dfad1dcf2f3791..fb73b0c5bdda9d 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -60,9 +60,9 @@ def disabled_two_factor(user)
   # it won't be sent to internal users like the
   # ghost user or the EE support bot.
   def new_key(key)
-    return unless key.user&.can?(:receive_notifications)
-
-    mailer.new_ssh_key_email(key.id).deliver_later
+    if key.user&.can?(:receive_notifications)
+      mailer.new_ssh_key_email(key.id).deliver_later
+    end
   end
 
   # Always notify the user about gpg key added
@@ -70,9 +70,9 @@ def new_key(key)
   # This is a security email so it will be sent even if the user disabled
   # notifications
   def new_gpg_key(gpg_key)
-    return unless gpg_key.user&.can?(:receive_notifications)
-
-    mailer.new_gpg_key_email(gpg_key.id).deliver_later
+    if gpg_key.user&.can?(:receive_notifications)
+      mailer.new_gpg_key_email(gpg_key.id).deliver_later
+    end
   end
 
   def bot_resource_access_token_about_to_expire(bot_user, token_name, params = {})
@@ -272,8 +272,7 @@ def push_to_merge_request(merge_request, current_user, new_commits: [], existing
   end
 
   def change_in_merge_request_draft_status(merge_request, current_user)
-    recipients = NotificationRecipients::BuildService.build_recipients(merge_request, current_user,
-      action: "draft_status_change")
+    recipients = NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: "draft_status_change")
 
     recipients.each do |recipient|
       mailer.send(
@@ -363,8 +362,7 @@ def changed_reviewer_of_merge_request(merge_request, current_user, previous_revi
   end
 
   def review_requested_of_merge_request(merge_request, current_user, reviewer)
-    recipients = NotificationRecipients::BuildService.build_requested_review_recipients(merge_request, current_user,
-      reviewer)
+    recipients = NotificationRecipients::BuildService.build_requested_review_recipients(merge_request, current_user, reviewer)
 
     deliver_option = review_request_deliver_options(merge_request.project)
 
@@ -416,8 +414,7 @@ def resolve_all_discussions(merge_request, current_user)
       action: "resolve_all_discussions")
 
     recipients.each do |recipient|
-      mailer.resolved_all_discussions_email(recipient.user.id, merge_request.id, current_user.id,
-        recipient.reason).deliver_later
+      mailer.resolved_all_discussions_email(recipient.user.id, merge_request.id, current_user.id, recipient.reason).deliver_later
     end
   end
 
@@ -441,7 +438,7 @@ def new_note(note)
   end
 
   def send_new_note_notifications(note)
-    notify_method = :"note_#{note.noteable_ability_name}_email"
+    notify_method = "note_#{note.noteable_ability_name}_email".to_sym
 
     recipients = NotificationRecipients::BuildService.build_new_note_recipients(note)
     recipients.each do |recipient|
@@ -598,19 +595,14 @@ def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
 
     return unless mailer.respond_to?(email_template)
 
-    unless recipients
-      recipients = notifiable_users(
-        [pipeline.user], :watch,
-        custom_action: :"#{status}_pipeline",
-        target: pipeline
-      )
-
-
-      recipients = recipients.uniq.map do |user|
-        user.notification_email_for(pipeline.project.group)
-      end
+    recipients ||= notifiable_users(
+      [pipeline.user], :watch,
+      custom_action: :"#{status}_pipeline",
+      target: pipeline
+    ).map do |user|
+      user.notification_email_for(pipeline.project.group)
     end
-    binding.pry_shell
+
     recipients.each do |recipient|
       mailer.public_send(email_template, pipeline, recipient).deliver_later
     end
@@ -867,8 +859,7 @@ def close_resource_email(target, current_user, method, skip_current_user: true,
     )
 
     recipients.each do |recipient|
-      mailer.send(method, recipient.user.id, target.id, current_user.id, reason: recipient.reason,
-        closed_via: closed_via).deliver_later
+      mailer.send(method, recipient.user.id, target.id, current_user.id, reason: recipient.reason, closed_via: closed_via).deliver_later
     end
   end
 
@@ -936,8 +927,7 @@ def approve_mr_email(merge_request, project, current_user)
   end
 
   def unapprove_mr_email(merge_request, project, current_user)
-    recipients = ::NotificationRecipients::BuildService.build_recipients(merge_request, current_user,
-      action: 'unapprove')
+    recipients = ::NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: 'unapprove')
 
     recipients.each do |recipient|
       mailer.unapproved_merge_request_email(recipient.user.id, merge_request.id, current_user.id).deliver_later
@@ -1036,8 +1026,7 @@ def notifiable_users(...)
   end
 
   def warn_skipping_notifications(user, object)
-    Gitlab::AppLogger.warn(message: "Skipping sending notifications", user: user.id, klass: object.class.to_s,
-      object_id: object.id)
+    Gitlab::AppLogger.warn(message: "Skipping sending notifications", user: user.id, klass: object.class.to_s, object_id: object.id)
   end
 
   def new_review_deliver_options(review)
diff --git a/ee/app/services/ee/notification_service.rb b/ee/app/services/ee/notification_service.rb
index f8e57501f71614..9612805d653a20 100644
--- a/ee/app/services/ee/notification_service.rb
+++ b/ee/app/services/ee/notification_service.rb
@@ -113,7 +113,6 @@ def no_more_seats(namespace, recipients, user, requested_member_list)
     def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
       return if super.nil? || !pipeline.user.service_account? || recipients.present?
 
-      binding.pry_shell
       status = pipeline_notification_status(ref_status, pipeline)
       email_template = email_template_name(status)
 
-- 
GitLab


From ac34706becf3a8a51ffc52e96ae17c00af10f7a6 Mon Sep 17 00:00:00 2001
From: gilles_dehaudt <tonton1728@gmail.com>
Date: Fri, 24 Jan 2025 10:37:43 +0000
Subject: [PATCH 10/21] Testing and fixing

---
 .../notification_recipients/build_service.rb  |  6 +-
 .../builder/service_account.rb                | 10 +--
 .../builder/service_account_spec.rb           | 82 +++++++++++++++++++
 3 files changed, 89 insertions(+), 9 deletions(-)
 create mode 100644 ee/spec/services/notification_recipients/builder/service_account_spec.rb

diff --git a/ee/app/services/ee/notification_recipients/build_service.rb b/ee/app/services/ee/notification_recipients/build_service.rb
index f37fb35270ef81..589010675721af 100644
--- a/ee/app/services/ee/notification_recipients/build_service.rb
+++ b/ee/app/services/ee/notification_recipients/build_service.rb
@@ -4,11 +4,11 @@
 # Used by NotificationService to determine who should receive notification
 #
 module EE
-  module NotificationRecipients
+  module NotificationRecipients # rubocop:disable Gitlab/BoundedContexts -- Existing module structure
     module BuildService
       def self.build_service_account_recipients(...)
-        ::NotificationRecipients::Builder::ServiceAccount.new(...).build!.map(&:user)
+        ::NotificationRecipients::Builder::ServiceAccount.new(...).notification_recipients.map(&:user)
       end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/ee/app/services/notification_recipients/builder/service_account.rb b/ee/app/services/notification_recipients/builder/service_account.rb
index b92dd2ea381d4e..aa2b595e862b59 100644
--- a/ee/app/services/notification_recipients/builder/service_account.rb
+++ b/ee/app/services/notification_recipients/builder/service_account.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module NotificationRecipients
+module NotificationRecipients # rubocop:disable Gitlab/BoundedContexts -- Existing module structure
   module Builder
     class ServiceAccount < Base
       attr_reader :project, :current_user, :pipeline_status
@@ -17,15 +17,13 @@ def build!
         return if notification_by_sources.blank?
 
         user_ids = NotificationSetting.from_union(notification_by_sources).select(:user_id)
-        add_recipients(user_scope.where(id: user_ids), :custom, nil)
+        add_recipients(user_scope.by_ids(user_ids), :custom, nil)
       end
 
-      def target
-        
-      end
+      def target; end
 
       def custom_action
-        @custom_action ||= "#{pipeline_status}_pipeline_for_service_account".to_sym
+        @custom_action ||= :"#{pipeline_status}_pipeline_for_service_account"
       end
     end
   end
diff --git a/ee/spec/services/notification_recipients/builder/service_account_spec.rb b/ee/spec/services/notification_recipients/builder/service_account_spec.rb
new file mode 100644
index 00000000000000..b510dcd0329efb
--- /dev/null
+++ b/ee/spec/services/notification_recipients/builder/service_account_spec.rb
@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe NotificationRecipients::Builder::ServiceAccount, feature_category: :user_management do
+  describe '#notification_recipients' do
+    let(:service_account) { create(:user, :service_account) }
+    let(:pipeline) { create(:ci_pipeline, user: service_account) }
+    let(:user) { create(:user, developer_of: pipeline.project) }
+    let(:pipeline_status) { pipeline.status }
+
+    subject(:build_service_method) do
+      ::NotificationRecipients::Builder::ServiceAccount
+        .new(pipeline.project, pipeline.user, pipeline_status).notification_recipients.map(&:user)
+    end
+
+    context 'when no user subscribed for service account notifications' do
+      it 'returns empty array' do
+        expect(build_service_method).to be_empty
+      end
+    end
+
+    context 'when there is one subscriber' do
+      let(:notification_setting) { user.notification_settings_for(pipeline.project) }
+
+      before do
+        notification_setting.update!(level: "custom")
+      end
+
+      context 'with no settings set' do
+        it 'returns empty array' do
+          expect(build_service_method).to be_empty
+        end
+      end
+
+      context 'with failed_pipeline_for_service_account notification setting checked and pipeline failed' do
+        before do
+          notification_setting.update!(failed_pipeline_for_service_account: "true")
+          pipeline.drop!
+        end
+
+        it 'returns an array with the subscribed user' do
+          expect(build_service_method).to match_array([user])
+        end
+      end
+
+      context 'with failed_pipeline_for_service_account notification setting checked and pipeline succeeded' do
+        before do
+          notification_setting.update!(failed_pipeline_for_service_account: "true")
+          pipeline.succeed!
+        end
+
+        it 'returns empty array' do
+          expect(build_service_method).to be_empty
+        end
+      end
+
+      context 'with success_pipeline_for_service_account notification setting checked and pipeline succeeded' do
+        before do
+          notification_setting.update!(success_pipeline_for_service_account: "true")
+          pipeline.succeed!
+        end
+
+        it 'returns an array with the subscribed user' do
+          expect(build_service_method).to match_array([user])
+        end
+      end
+
+      context 'with fixed_pipeline_for_service_account notification setting checked and pipeline fixed' do
+        let(:pipeline_status) { "fixed" }
+
+        before do
+          notification_setting.update!(fixed_pipeline_for_service_account: "true")
+        end
+
+        it 'returns an array with the subscribed user' do
+          expect(build_service_method).to match_array([user])
+        end
+      end
+    end
+  end
+end
-- 
GitLab


From f0d105856c7d2cf3e95f7f414f154b10e6c7e06b Mon Sep 17 00:00:00 2001
From: gilles_dehaudt <tonton1728@gmail.com>
Date: Fri, 24 Jan 2025 10:41:41 +0000
Subject: [PATCH 11/21] Rubocop

---
 app/services/notification_recipients/build_service.rb | 2 +-
 ee/app/services/ee/notification_service.rb            | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb
index 7e570373f35e19..ef73ce3b1bc475 100644
--- a/app/services/notification_recipients/build_service.rb
+++ b/app/services/notification_recipients/build_service.rb
@@ -39,4 +39,4 @@ def self.build_requested_review_recipients(...)
   end
 end
 
-NotificationRecipients::BuildService.prepend_mod
\ No newline at end of file
+NotificationRecipients::BuildService.prepend_mod
diff --git a/ee/app/services/ee/notification_service.rb b/ee/app/services/ee/notification_service.rb
index 9612805d653a20..0d7e63b2e4ef66 100644
--- a/ee/app/services/ee/notification_service.rb
+++ b/ee/app/services/ee/notification_service.rb
@@ -116,13 +116,13 @@ def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
       status = pipeline_notification_status(ref_status, pipeline)
       email_template = email_template_name(status)
 
-      recipients = NotificationRecipients::BuildService.build_service_account_recipients(pipeline.project, pipeline.user, status)
+      recipients = NotificationRecipients::BuildService
+        .build_service_account_recipients(pipeline.project, pipeline.user, status)
 
       recipients.uniq.map do |user|
         recipient = user.notification_email_for(pipeline.project.group)
-        mailer.public_send(email_template, pipeline, recipient).deliver_later
+        mailer.public_send(email_template, pipeline, recipient).deliver_later # rubocop:disable GitlabSecurity/PublicSend -- not a security issue
       end
-
     end
 
     private
-- 
GitLab


From 5a95c38d2ed73e9bb777cf3161a0be85016eccc2 Mon Sep 17 00:00:00 2001
From: Densett <kevin.morisseau@protonmail.com>
Date: Fri, 24 Jan 2025 14:58:41 +0000
Subject: [PATCH 12/21] Tests for notification service

---
 .../models/ee/notification_setting_spec.rb    |  21 +-
 .../services/ee/notification_service_spec.rb  | 191 ++++++++++++++----
 2 files changed, 165 insertions(+), 47 deletions(-)

diff --git a/ee/spec/models/ee/notification_setting_spec.rb b/ee/spec/models/ee/notification_setting_spec.rb
index ff7e454523b324..6bfc92ebb0cf18 100644
--- a/ee/spec/models/ee/notification_setting_spec.rb
+++ b/ee/spec/models/ee/notification_setting_spec.rb
@@ -10,7 +10,7 @@
       let(:target) { build_stubbed(:group) }
 
       it 'appends EE specific events' do
-        expect(subject).to eq(
+        expect(subject).to match_array(
           [
             :new_release,
             :new_note,
@@ -31,7 +31,10 @@
             :success_pipeline,
             :moved_project,
             :merge_when_pipeline_succeeds,
-            :new_epic
+            :new_epic,
+            :failed_pipeline_for_service_account,
+            :success_pipeline_for_service_account,
+            :fixed_pipeline_for_service_account
           ]
         )
       end
@@ -41,7 +44,7 @@
       let(:target) { build_stubbed(:project) }
 
       it 'returns CE list' do
-        expect(subject).to eq(
+        expect(subject).to match_array(
           [
             :new_release,
             :new_note,
@@ -62,7 +65,10 @@
             :success_pipeline,
             :moved_project,
             :merge_when_pipeline_succeeds,
-            :approver
+            :approver,
+            :failed_pipeline_for_service_account,
+            :success_pipeline_for_service_account,
+            :fixed_pipeline_for_service_account
           ]
         )
       end
@@ -72,7 +78,7 @@
       let(:target) { nil }
 
       it 'appends EE specific events' do
-        expect(subject).to eq(
+        expect(subject).to match_array(
           [
             :new_release,
             :new_note,
@@ -94,7 +100,10 @@
             :moved_project,
             :merge_when_pipeline_succeeds,
             :new_epic,
-            :approver
+            :approver,
+            :failed_pipeline_for_service_account,
+            :success_pipeline_for_service_account,
+            :fixed_pipeline_for_service_account
           ]
         )
       end
diff --git a/ee/spec/services/ee/notification_service_spec.rb b/ee/spec/services/ee/notification_service_spec.rb
index 88e390e32a3bb6..bcbf72304e5065 100644
--- a/ee/spec/services/ee/notification_service_spec.rb
+++ b/ee/spec/services/ee/notification_service_spec.rb
@@ -25,9 +25,14 @@
     let(:user) { create(:user) }
     let(:user2) { create(:user) }
     let(:reviewer) { create(:user) }
-    let(:merge_request) { create(:merge_request, source_project: project, assignees: [user, user2], author: create(:user)) }
+    let(:merge_request) do
+      create(:merge_request, source_project: project, assignees: [user, user2], author: create(:user))
+    end
+
     let(:review) { create(:review, merge_request: merge_request, project: project, author: reviewer) }
-    let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, author: reviewer, review: review) }
+    let(:note) do
+      create(:diff_note_on_merge_request, project: project, noteable: merge_request, author: reviewer, review: review)
+    end
 
     before do
       build_team(review.project, merge_request)
@@ -85,7 +90,8 @@ def build_team(project, merge_request)
       @subscriber = create :user
       @unsubscriber = create :user
       @unsubscribed_mentioned = create(:user, username: 'unsubscribed_mentioned')
-      @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
+      @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'),
+        :participating)
       @watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
 
       # User to be participant by default
@@ -127,7 +133,8 @@ def build_team(project, merge_request)
       let!(:project_member) { create(:project_member, :invited, project: project) }
 
       it 'sends email' do
-        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, project.first_owner.id).and_call_original
+        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id,
+          project.first_owner.id).and_call_original
 
         subject.mirror_was_hard_failed(project)
       end
@@ -146,7 +153,8 @@ def build_team(project, merge_request)
       let(:project) { create(:project, :mirror, :import_hard_failed) }
 
       it 'sends email' do
-        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, project.first_owner.id).and_call_original
+        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id,
+          project.first_owner.id).and_call_original
 
         subject.mirror_was_hard_failed(project)
       end
@@ -179,7 +187,8 @@ def build_team(project, merge_request)
 
             project = create(:project, :mirror, :import_hard_failed, namespace: group)
 
-            expect(Notify).not_to receive(:mirror_was_hard_failed_email).with(project.id, blocked_user.id).and_call_original
+            expect(Notify).not_to receive(:mirror_was_hard_failed_email).with(project.id,
+              blocked_user.id).and_call_original
             expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, user.id).and_call_original
 
             subject.mirror_was_hard_failed(project)
@@ -193,7 +202,8 @@ def build_team(project, merge_request)
         project = create(:project, :mirror, :import_hard_failed)
         project.add_maintainer(user)
 
-        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, project.first_owner.id).and_call_original
+        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id,
+          project.first_owner.id).and_call_original
         expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, user.id).and_call_original
 
         subject.mirror_was_hard_failed(project)
@@ -251,7 +261,8 @@ def build_team(project, merge_request)
       let!(:project_member) { create(:project_member, :invited, project: project) }
 
       it 'sends email' do
-        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.first_owner.id, deleted_username).and_call_original
+        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.first_owner.id,
+          deleted_username).and_call_original
 
         subject.mirror_was_disabled(project, deleted_username)
       end
@@ -268,7 +279,8 @@ def build_team(project, merge_request)
 
     context 'when user is owner' do
       it 'sends email' do
-        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.first_owner.id, deleted_username).and_call_original
+        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.first_owner.id,
+          deleted_username).and_call_original
 
         subject.mirror_was_disabled(project, deleted_username)
       end
@@ -301,8 +313,10 @@ def build_team(project, merge_request)
 
             project = create(:project, namespace: group)
 
-            expect(Notify).not_to receive(:mirror_was_disabled_email).with(project.id, blocked_user.id, deleted_username).and_call_original
-            expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
+            expect(Notify).not_to receive(:mirror_was_disabled_email).with(project.id, blocked_user.id,
+              deleted_username).and_call_original
+            expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id,
+              deleted_username).and_call_original
 
             subject.mirror_was_disabled(project, deleted_username)
           end
@@ -314,8 +328,10 @@ def build_team(project, merge_request)
       it 'sends email' do
         project.add_maintainer(user)
 
-        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.first_owner.id, deleted_username).and_call_original
-        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
+        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.first_owner.id,
+          deleted_username).and_call_original
+        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id,
+          deleted_username).and_call_original
 
         subject.mirror_was_disabled(project, deleted_username)
       end
@@ -325,8 +341,10 @@ def build_team(project, merge_request)
       it 'does not send email' do
         project.add_developer(user)
 
-        expect(Notify).not_to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
-        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.creator.id, deleted_username).and_call_original
+        expect(Notify).not_to receive(:mirror_was_disabled_email).with(project.id, user.id,
+          deleted_username).and_call_original
+        expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.creator.id,
+          deleted_username).and_call_original
 
         subject.mirror_was_disabled(project, deleted_username)
       end
@@ -339,7 +357,8 @@ def build_team(project, merge_request)
 
           project = create(:project, namespace: group)
 
-          expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
+          expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id,
+            deleted_username).and_call_original
 
           subject.mirror_was_disabled(project, deleted_username)
         end
@@ -353,7 +372,8 @@ def build_team(project, merge_request)
 
           project = create(:project, namespace: group)
 
-          expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
+          expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id,
+            deleted_username).and_call_original
 
           subject.mirror_was_disabled(project, deleted_username)
         end
@@ -367,7 +387,8 @@ def build_team(project, merge_request)
     let(:new_mirror_user) { project.team.owners.first }
 
     it 'sends email' do
-      expect(Notify).to receive(:project_mirror_user_changed_email).with(new_mirror_user.id, mirror_user.name, project.id).and_call_original
+      expect(Notify).to receive(:project_mirror_user_changed_email).with(new_mirror_user.id, mirror_user.name,
+        project.id).and_call_original
 
       subject.project_mirror_user_changed(new_mirror_user, mirror_user.name, project)
     end
@@ -387,7 +408,9 @@ def build_team(project, merge_request)
     let(:project) { create(:project, :public, namespace: group) }
     let(:assignee) { create(:user) }
 
-    let(:issue) { create :issue, project: project, assignees: [assignee], description: 'cc @participant @unsubscribed_mentioned' }
+    let(:issue) do
+      create :issue, project: project, assignees: [assignee], description: 'cc @participant @unsubscribed_mentioned'
+    end
 
     let(:notification) { NotificationService.new }
 
@@ -430,7 +453,10 @@ def build_team(project, merge_request)
       let(:mailer_method) { :removed_iteration_issue_email }
 
       context do
-        let(:iteration) { create(:iteration, iterations_cadence: create(:iterations_cadence, group: group), issues: [issue]) }
+        let(:iteration) do
+          create(:iteration, iterations_cadence: create(:iterations_cadence, group: group), issues: [issue])
+        end
+
         let!(:subscriber_to_new_iteration) { create(:user) { |u| issue.toggle_subscription(u, project) } }
 
         it_behaves_like 'altered iteration notification on issue' do
@@ -452,7 +478,11 @@ def build_team(project, merge_request)
         let(:member) { create(:user) }
         let(:guest) { create(:user) }
         let(:admin) { create(:admin) }
-        let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) }
+        let(:confidential_issue) do
+          create(:issue, :confidential, project: project, title: 'Confidential issue', author: author,
+            assignees: [assignee])
+        end
+
         let(:iteration) { create(:iteration, issues: [confidential_issue]) }
 
         it "emails subscribers of the issue's iteration that can read the issue" do
@@ -506,7 +536,11 @@ def build_team(project, merge_request)
         let(:member) { create(:user) }
         let(:guest) { create(:user) }
         let(:admin) { create(:admin) }
-        let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) }
+        let(:confidential_issue) do
+          create(:issue, :confidential, project: project, title: 'Confidential issue', author: author,
+            assignees: [assignee])
+        end
+
         let(:new_iteration) { create(:iteration, issues: [confidential_issue]) }
 
         it "emails subscribers of the issue's iteration that can read the issue" do
@@ -551,7 +585,10 @@ def build_team(project, merge_request)
     end
 
     context 'epic notes' do
-      let(:note) { create(:note, project: nil, noteable: epic, note: '@mention referenced, @unsubscribed_mentioned and @outsider also') }
+      let(:note) do
+        create(:note, project: nil, noteable: epic,
+          note: '@mention referenced, @unsubscribed_mentioned and @outsider also')
+      end
 
       before do
         build_group_members(group)
@@ -669,7 +706,8 @@ def build_team(project, merge_request)
 
       shared_examples 'is not able to send notifications' do
         it 'does not send any notification' do
-          expect(Gitlab::AppLogger).to receive(:warn).with(message: 'Skipping sending notifications', user: current_user.id, klass: epic.class.to_s, object_id: epic.id)
+          expect(Gitlab::AppLogger).to receive(:warn).with(message: 'Skipping sending notifications',
+            user: current_user.id, klass: epic.class.to_s, object_id: epic.id)
 
           execute
 
@@ -733,7 +771,8 @@ def add_users_with_subscription(group, issuable)
     @subscriber = create :user
     @unsubscriber = create :user
     @unsubscribed_mentioned = create :user, username: 'unsubscribed_mentioned'
-    @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
+    @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'),
+      :participating)
     @watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
 
     create(:group_member, group: group, user: @subscribed_participant)
@@ -757,7 +796,9 @@ def add_users_with_subscription(group, issuable)
     let(:group) { create(:group) }
     let(:project) { create(:project, :public, :repository, namespace: group) }
     let(:another_project) { create(:project, :public, namespace: group) }
-    let(:merge_request) { create :merge_request, source_project: project, assignees: [assignee, assignee2], description: 'cc @participant' }
+    let(:merge_request) do
+      create :merge_request, source_project: project, assignees: [assignee, assignee2], description: 'cc @participant'
+    end
 
     around do |example|
       perform_enqueued_jobs do
@@ -789,7 +830,9 @@ def add_users_with_subscription(group, issuable)
 
       context 'when the target project has approvers set' do
         let(:project_approvers) { create_list(:user, 3) }
-        let!(:rule) { create(:approval_project_rule, project: project, users: project_approvers, approvals_required: 1) }
+        let!(:rule) do
+          create(:approval_project_rule, project: project, users: project_approvers, approvals_required: 1)
+        end
 
         before do
           reset_delivered_emails!
@@ -815,7 +858,10 @@ def add_users_with_subscription(group, issuable)
 
         context 'when the merge request has approvers set' do
           let(:mr_approvers) { create_list(:user, 3) }
-          let!(:mr_rule) { create(:approval_merge_request_rule, merge_request: merge_request, users: mr_approvers, approvals_required: 1) }
+          let!(:mr_rule) do
+            create(:approval_merge_request_rule, merge_request: merge_request, users: mr_approvers,
+              approvals_required: 1)
+          end
 
           before do
             reset_delivered_emails!
@@ -860,7 +906,8 @@ def build_team(project)
       @u_guest_watcher = create_user_with_notification(:watch, 'guest_watching')
       @u_guest_custom = create_user_with_notification(:custom, 'guest_custom')
 
-      [@u_watcher, @u_participating, @u_participant_mentioned, @u_disabled, @u_mentioned, @u_committer, @u_not_mentioned, @u_lazy_participant, @u_custom_global].each do |user|
+      [@u_watcher, @u_participating, @u_participant_mentioned, @u_disabled, @u_mentioned, @u_committer,
+        @u_not_mentioned, @u_lazy_participant, @u_custom_global].each do |user|
         project.add_maintainer(user)
       end
     end
@@ -869,10 +916,12 @@ def add_users_with_subscription(project, issuable)
       @subscriber = create :user
       @unsubscriber = create :user
       @unsubscribed_mentioned = create :user, username: 'unsubscribed_mentioned'
-      @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
+      @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'),
+        :participating)
       @watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
 
-      [@subscribed_participant, @subscriber, @unsubscriber, @watcher_and_subscriber, @unsubscribed_mentioned].each do |user|
+      [@subscribed_participant, @subscriber, @unsubscriber, @watcher_and_subscriber,
+        @unsubscribed_mentioned].each do |user|
         project.add_maintainer(user)
       end
 
@@ -919,14 +968,18 @@ def add_users_with_subscription(project, issuable)
       let_it_be(:participant) { create(:incident_management_oncall_participant, rotation: rotation) }
 
       it 'sends an email to the owner and participants' do
-        expect(Notify).to receive(:user_removed_from_rotation_email).with(user, rotation, [schedule.project.first_owner]).once.and_call_original
-        expect(Notify).to receive(:user_removed_from_rotation_email).with(user, rotation, [participant.user]).once.and_call_original
+        expect(Notify).to receive(:user_removed_from_rotation_email).with(user, rotation,
+          [schedule.project.first_owner]).once.and_call_original
+        expect(Notify).to receive(:user_removed_from_rotation_email).with(user, rotation,
+          [participant.user]).once.and_call_original
 
         subject.oncall_user_removed(rotation, user)
       end
 
       it 'sends the email inline when async = false' do
-        expect { subject.oncall_user_removed(rotation, user, false) }.to change(ActionMailer::Base.deliveries, :size).by(2)
+        expect do
+          subject.oncall_user_removed(rotation, user, false)
+        end.to change(ActionMailer::Base.deliveries, :size).by(2)
       end
     end
 
@@ -935,13 +988,18 @@ def add_users_with_subscription(project, issuable)
       let(:user) { create(:user) }
       let(:rules) { [rule_1, rule_2] }
       let!(:rule_1) { create(:incident_management_escalation_rule, :with_user, project: project, user: user) }
-      let!(:rule_2) { create(:incident_management_escalation_rule, :with_user, :resolved, project: project, user: user) }
+      let!(:rule_2) do
+        create(:incident_management_escalation_rule, :with_user, :resolved, project: project, user: user)
+      end
 
       it 'immediately sends an email to the project owner' do
-        expect(Notify).to receive(:user_escalation_rule_deleted_email).with(user, project, rules, project.first_owner).once.and_call_original
+        expect(Notify).to receive(:user_escalation_rule_deleted_email).with(user, project, rules,
+          project.first_owner).once.and_call_original
         expect(Notify).not_to receive(:user_escalation_rule_deleted_email).with(user, project, rules, user)
 
-        expect { subject.user_escalation_rule_deleted(project, user, rules) }.to change(ActionMailer::Base.deliveries, :size).by(1)
+        expect do
+          subject.user_escalation_rule_deleted(project, user, rules)
+        end.to change(ActionMailer::Base.deliveries, :size).by(1)
       end
 
       context 'when project owner is the removed user' do
@@ -967,11 +1025,14 @@ def add_users_with_subscription(project, issuable)
         end
 
         it 'immediately sends an email to the eligable project owners' do
-          expect(Notify).to receive(:user_escalation_rule_deleted_email).with(user, project, rules, owner).once.and_call_original
+          expect(Notify).to receive(:user_escalation_rule_deleted_email).with(user, project, rules,
+            owner).once.and_call_original
           expect(Notify).not_to receive(:user_escalation_rule_deleted_email).with(user, project, rules, blocked_owner)
           expect(Notify).not_to receive(:user_escalation_rule_deleted_email).with(user, project, rules, user)
 
-          expect { subject.user_escalation_rule_deleted(project, user, rules) }.to change(ActionMailer::Base.deliveries, :size).by(1)
+          expect do
+            subject.user_escalation_rule_deleted(project, user, rules)
+          end.to change(ActionMailer::Base.deliveries, :size).by(1)
         end
       end
     end
@@ -1002,6 +1063,52 @@ def add_users_with_subscription(project, issuable)
     end
   end
 
+  context 'when user subscribed to service account pipeline notifications' do
+    around do |example|
+      perform_enqueued_jobs { example.run }
+    end
+
+    let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:service_account) { create(:user, :service_account) }
+    let_it_be(:u_custom_notification_enabled) do
+      user = create_user_with_notification(:custom, 'custom_enabled')
+      update_custom_notification(:success_pipeline_for_service_account, user, resource: project)
+      update_custom_notification(:failed_pipeline_for_service_account, user, resource: project)
+      update_custom_notification(:fixed_pipeline_for_service_account, user, resource: project)
+      user
+    end
+
+    let_it_be(:u_custom_notification_disabled) do
+      user = create_user_with_notification(:custom, 'custom_disabled')
+      update_custom_notification(:success_pipeline_for_service_account, user, resource: project, value: false)
+      update_custom_notification(:failed_pipeline_for_service_account, user, resource: project, value: false)
+      update_custom_notification(:fixed_pipeline_for_service_account, user, resource: project, value: false)
+      user
+    end
+
+    def create_pipeline(service_account, status)
+      create(
+        :ci_pipeline, status,
+        project: project,
+        user: service_account
+      )
+    end
+
+    before do
+      project.add_developer(u_custom_notification_enabled)
+      project.add_developer(u_custom_notification_disabled)
+      reset_delivered_emails!
+      project.update!(service_desk_enabled: false)
+      pipeline = create_pipeline(service_account, :failed)
+      subject.pipeline_finished(pipeline, ref_status: 'failed')
+    end
+
+    it 'emails the user' do
+      should_email(u_custom_notification_enabled)
+      should_not_email(u_custom_notification_disabled)
+    end
+  end
+
   describe '#no_more_seats' do
     let_it_be(:namespace) { create(:namespace) }
     let_it_be(:namespace_limit) { create(:namespace_limit, namespace: namespace) }
@@ -1012,7 +1119,8 @@ def add_users_with_subscription(project, issuable)
     subject(:execute) { NotificationService.new.no_more_seats(namespace, [recipient], user, required_members) }
 
     it 'sends emails and update the last_at value' do
-      expect(Notify).to receive(:no_more_seats).with(recipient.id, user.id, namespace, required_members).and_return(mailer)
+      expect(Notify).to receive(:no_more_seats).with(recipient.id, user.id, namespace,
+        required_members).and_return(mailer)
 
       expect(mailer).to receive(:deliver_later)
 
@@ -1036,7 +1144,8 @@ def add_users_with_subscription(project, issuable)
         let_it_be(:notification_at) { 2.days.ago }
 
         it 'sends emails and update the last_at value' do
-          expect(Notify).to receive(:no_more_seats).with(recipient.id, user.id, namespace, required_members).and_return(mailer)
+          expect(Notify).to receive(:no_more_seats).with(recipient.id, user.id, namespace,
+            required_members).and_return(mailer)
 
           expect(mailer).to receive(:deliver_later)
 
-- 
GitLab


From eda2e1a83262f4c3c5faa7c8a2ca6d60fc5d7d28 Mon Sep 17 00:00:00 2001
From: Densett <kevin.morisseau@protonmail.com>
Date: Fri, 21 Mar 2025 16:06:10 +0000
Subject: [PATCH 13/21] Add tests for build service account recipients and fix
 attempt for notification service

---
 ee/app/services/ee/notification_service.rb    |  2 +-
 .../build_service_account_recipients_spec.rb  | 48 +++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 ee/spec/services/ee/notification_recipients/build_service_account_recipients_spec.rb

diff --git a/ee/app/services/ee/notification_service.rb b/ee/app/services/ee/notification_service.rb
index 0d7e63b2e4ef66..acf3f8bc9b5c7f 100644
--- a/ee/app/services/ee/notification_service.rb
+++ b/ee/app/services/ee/notification_service.rb
@@ -111,7 +111,7 @@ def no_more_seats(namespace, recipients, user, requested_member_list)
     end
 
     def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
-      return if super.nil? || !pipeline.user.service_account? || recipients.present?
+      return if super.nil? || !pipeline.user&.service_account? || recipients.present?
 
       status = pipeline_notification_status(ref_status, pipeline)
       email_template = email_template_name(status)
diff --git a/ee/spec/services/ee/notification_recipients/build_service_account_recipients_spec.rb b/ee/spec/services/ee/notification_recipients/build_service_account_recipients_spec.rb
new file mode 100644
index 00000000000000..a992fedfdf9e67
--- /dev/null
+++ b/ee/spec/services/ee/notification_recipients/build_service_account_recipients_spec.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe EE::NotificationRecipients::BuildService, feature_category: :team_planning do
+  let(:service) { described_class }
+
+  describe '#build_service_account_recipients' do
+    let(:service_account) { create(:user, :service_account) }
+    let(:pipeline) { create(:ci_pipeline, user: service_account) }
+    let(:user) { create(:user, developer_of: pipeline.project) }
+    let(:pipeline_status) { pipeline.status }
+
+    context 'when no user subscribed for service account notifications' do
+      it 'does not add recipient' do
+        recipients = service.build_service_account_recipients(pipeline.project, pipeline.user, pipeline_status)
+        expect(recipients).to be_empty
+      end
+    end
+
+    context 'when there is one subscriber' do
+      let(:notification_setting) { user.notification_settings_for(pipeline.project) }
+
+      before do
+        notification_setting.update!(level: "custom")
+      end
+
+      context 'with no settings set' do
+        it 'does not add recipient' do
+          recipients = service.build_service_account_recipients(pipeline.project, pipeline.user, pipeline_status)
+          expect(recipients).to be_empty
+        end
+      end
+
+      context 'with failed_pipeline_for_service_account notification setting checked and pipeline failed' do
+        before do
+          notification_setting.update!(failed_pipeline_for_service_account: "true")
+          pipeline.drop!
+        end
+
+        it 'adds subscribed user to recipients' do
+          recipients = service.build_service_account_recipients(pipeline.project, pipeline.user, pipeline_status)
+          expect(recipients).to match_array([user])
+        end
+      end
+    end
+  end
+end
-- 
GitLab


From 878295465d009dcbda97b30bc4f4b89a69d9ffda Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@gitlab.com>
Date: Fri, 28 Mar 2025 10:40:51 -0500
Subject: [PATCH 14/21] Refactor new notification names

---
 .../javascripts/notifications/constants.js    | 14 ++++----
 ..._add_new_event_to_notification_settings.rb | 36 -------------------
 ..._add_new_event_to_notification_settings.rb | 14 ++++++++
 db/schema_migrations/20250122103435           |  1 -
 db/schema_migrations/20250328103435           |  1 +
 db/structure.sql                              |  6 ++--
 ee/app/models/ee/notification_setting.rb      | 12 +++----
 .../builder/service_account.rb                |  2 +-
 .../models/ee/notification_setting_spec.rb    | 18 +++++-----
 .../build_service_account_recipients_spec.rb  |  4 +--
 .../builder/service_account_spec.rb           | 16 ++++-----
 11 files changed, 52 insertions(+), 72 deletions(-)
 delete mode 100644 db/migrate/20250122103435_add_new_event_to_notification_settings.rb
 create mode 100644 db/migrate/20250328103435_add_new_event_to_notification_settings.rb
 delete mode 100644 db/schema_migrations/20250122103435
 create mode 100644 db/schema_migrations/20250328103435

diff --git a/app/assets/javascripts/notifications/constants.js b/app/assets/javascripts/notifications/constants.js
index aafbc28dd43c22..3a5227df8d59f0 100644
--- a/app/assets/javascripts/notifications/constants.js
+++ b/app/assets/javascripts/notifications/constants.js
@@ -42,11 +42,7 @@ export const i18n = {
     close_issue: s__('NotificationEvent|Issue is closed'),
     close_merge_request: s__('NotificationEvent|Merge request is closed'),
     failed_pipeline: s__('NotificationEvent|Pipeline fails'),
-    failed_pipeline_for_service_account: s__('NotificationEvent|Pipeline by Service Account fails'),
     fixed_pipeline: s__('NotificationEvent|Pipeline is fixed'),
-    fixed_pipeline_for_service_account: s__(
-      'NotificationEvent|Pipeline by Service Account is fixed',
-    ),
     issue_due: s__('NotificationEvent|Issue is due soon'),
     merge_merge_request: s__('NotificationEvent|Merge request is merged'),
     merge_when_pipeline_succeeds: s__('NotificationEvent|Merge request is set to auto-merge'),
@@ -61,10 +57,16 @@ export const i18n = {
     reassign_merge_request: s__('NotificationEvent|Merge request is reassigned'),
     reopen_issue: s__('NotificationEvent|Issue is reopened'),
     reopen_merge_request: s__('NotificationEvent|Merge request is reopened'),
-    success_pipeline: s__('NotificationEvent|Pipeline is successful'),
-    success_pipeline_for_service_account: s__(
+    merge_when_pipeline_succeeds: s__('NotificationEvent|Merge request is set to auto-merge'),
+
+    service_account_failed_pipeline: s__('NotificationEvent|Pipeline by Service Account fails'),
+    service_account_fixed_pipeline: s__(
+      'NotificationEvent|Pipeline by Service Account is fixed',
+    ),
+    service_account_success_pipeline: s__(
       'NotificationEvent|Pipeline by Service Account is successful',
     ),
+    success_pipeline: s__('NotificationEvent|Pipeline is successful'),
     approver: s__('NotificationEvent|You are added as an approver on a merge request'),
   },
 };
diff --git a/db/migrate/20250122103435_add_new_event_to_notification_settings.rb b/db/migrate/20250122103435_add_new_event_to_notification_settings.rb
deleted file mode 100644
index 4b292481970ac4..00000000000000
--- a/db/migrate/20250122103435_add_new_event_to_notification_settings.rb
+++ /dev/null
@@ -1,36 +0,0 @@
-# frozen_string_literal: true
-
-# See https://docs.gitlab.com/ee/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
-class AddNewEventToNotificationSettings < Gitlab::Database::Migration[2.2]
-  # When using the methods "add_concurrent_index" or "remove_concurrent_index"
-  # you must disable the use of transactions
-  # as these methods can not run in an existing transaction.
-  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
-  # that either of them is the _only_ method called in the migration,
-  # any other changes should go in a separate migration.
-  # This ensures that upon failure _only_ the index creation or removing fails
-  # and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  # disable_ddl_transaction!
-  #
-  # Configure the `gitlab_schema` to perform data manipulation (DML).
-  # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html
-  # restrict_gitlab_migration gitlab_schema: :gitlab_main
-
-  # Add dependent 'batched_background_migrations.queued_migration_version' values.
-  # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = []
-  milestone '17.9'
-
-  def change
-    add_column :notification_settings, :failed_pipeline_for_service_account, :boolean, null: false, default: false,
-      if_not_exists: true
-    add_column :notification_settings, :success_pipeline_for_service_account, :boolean, null: false, default: false,
-      if_not_exists: true
-    add_column :notification_settings, :fixed_pipeline_for_service_account, :boolean, null: false, default: false,
-      if_not_exists: true
-  end
-end
diff --git a/db/migrate/20250328103435_add_new_event_to_notification_settings.rb b/db/migrate/20250328103435_add_new_event_to_notification_settings.rb
new file mode 100644
index 00000000000000..6ceccdb6791475
--- /dev/null
+++ b/db/migrate/20250328103435_add_new_event_to_notification_settings.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class AddNewEventToNotificationSettings < Gitlab::Database::Migration[2.2]
+  milestone '17.11'
+
+  def change
+    add_column :notification_settings, :service_account_failed_pipeline, :boolean, null: false, default: false,
+      if_not_exists: true
+    add_column :notification_settings, :service_account_success_pipeline, :boolean, null: false, default: false,
+      if_not_exists: true
+    add_column :notification_settings, :service_account_fixed_pipeline, :boolean, null: false, default: false,
+      if_not_exists: true
+  end
+end
diff --git a/db/schema_migrations/20250122103435 b/db/schema_migrations/20250122103435
deleted file mode 100644
index 100b27a3c57f83..00000000000000
--- a/db/schema_migrations/20250122103435
+++ /dev/null
@@ -1 +0,0 @@
-d1cb98ab058fbaffe8bcd5ad66a3c7f8729c3b861999ad3411597545749c9051
\ No newline at end of file
diff --git a/db/schema_migrations/20250328103435 b/db/schema_migrations/20250328103435
new file mode 100644
index 00000000000000..1cede3c4e49634
--- /dev/null
+++ b/db/schema_migrations/20250328103435
@@ -0,0 +1 @@
+f9150b1a364daf372ea9d07bc46e8fea194800bd31a0d4e4b2640e5bb6c53f86
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 1132a9a69f925e..e2775820caed03 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -18248,9 +18248,9 @@ CREATE TABLE notification_settings (
     change_reviewer_merge_request boolean,
     merge_when_pipeline_succeeds boolean DEFAULT false NOT NULL,
     approver boolean DEFAULT false NOT NULL,
-    failed_pipeline_for_service_account boolean DEFAULT false NOT NULL,
-    success_pipeline_for_service_account boolean DEFAULT false NOT NULL,
-    fixed_pipeline_for_service_account boolean DEFAULT false NOT NULL
+    service_account_failed_pipeline boolean DEFAULT false NOT NULL,
+    service_account_success_pipeline boolean DEFAULT false NOT NULL,
+    service_account_fixed_pipeline boolean DEFAULT false NOT NULL
 );
 
 CREATE SEQUENCE notification_settings_id_seq
diff --git a/ee/app/models/ee/notification_setting.rb b/ee/app/models/ee/notification_setting.rb
index 1e1f69b8764848..02bd0e9c2357cd 100644
--- a/ee/app/models/ee/notification_setting.rb
+++ b/ee/app/models/ee/notification_setting.rb
@@ -6,14 +6,14 @@ module NotificationSetting
 
     EMAIL_EVENTS_MAPPING = {
       ::Group => [:new_epic,
-        :failed_pipeline_for_service_account,
-        :success_pipeline_for_service_account,
-        :fixed_pipeline_for_service_account],
+        :service_account_failed_pipeline,
+        :service_account_success_pipeline,
+        :service_account_fixed_pipeline],
       ::User => [:approver],
       ::Project => [:approver,
-        :failed_pipeline_for_service_account,
-        :success_pipeline_for_service_account,
-        :fixed_pipeline_for_service_account]
+        :service_account_failed_pipeline,
+        :service_account_success_pipeline,
+        :service_account_fixed_pipeline]
     }.freeze
     FULL_EMAIL_EVENTS = EMAIL_EVENTS_MAPPING.values.flatten.freeze
 
diff --git a/ee/app/services/notification_recipients/builder/service_account.rb b/ee/app/services/notification_recipients/builder/service_account.rb
index aa2b595e862b59..3f15ecbd52eee6 100644
--- a/ee/app/services/notification_recipients/builder/service_account.rb
+++ b/ee/app/services/notification_recipients/builder/service_account.rb
@@ -23,7 +23,7 @@ def build!
       def target; end
 
       def custom_action
-        @custom_action ||= :"#{pipeline_status}_pipeline_for_service_account"
+        @custom_action ||= :"service_account_#{pipeline_status}_pipeline"
       end
     end
   end
diff --git a/ee/spec/models/ee/notification_setting_spec.rb b/ee/spec/models/ee/notification_setting_spec.rb
index 6bfc92ebb0cf18..ca507e4bda0915 100644
--- a/ee/spec/models/ee/notification_setting_spec.rb
+++ b/ee/spec/models/ee/notification_setting_spec.rb
@@ -32,9 +32,9 @@
             :moved_project,
             :merge_when_pipeline_succeeds,
             :new_epic,
-            :failed_pipeline_for_service_account,
-            :success_pipeline_for_service_account,
-            :fixed_pipeline_for_service_account
+            :service_account_failed_pipeline,
+            :service_account_success_pipeline,
+            :service_account_fixed_pipeline
           ]
         )
       end
@@ -66,9 +66,9 @@
             :moved_project,
             :merge_when_pipeline_succeeds,
             :approver,
-            :failed_pipeline_for_service_account,
-            :success_pipeline_for_service_account,
-            :fixed_pipeline_for_service_account
+            :service_account_failed_pipeline,
+            :service_account_success_pipeline,
+            :service_account_fixed_pipeline
           ]
         )
       end
@@ -101,9 +101,9 @@
             :merge_when_pipeline_succeeds,
             :new_epic,
             :approver,
-            :failed_pipeline_for_service_account,
-            :success_pipeline_for_service_account,
-            :fixed_pipeline_for_service_account
+            :service_account_failed_pipeline,
+            :service_account_success_pipeline,
+            :service_account_fixed_pipeline
           ]
         )
       end
diff --git a/ee/spec/services/ee/notification_recipients/build_service_account_recipients_spec.rb b/ee/spec/services/ee/notification_recipients/build_service_account_recipients_spec.rb
index a992fedfdf9e67..3c762810fe6055 100644
--- a/ee/spec/services/ee/notification_recipients/build_service_account_recipients_spec.rb
+++ b/ee/spec/services/ee/notification_recipients/build_service_account_recipients_spec.rb
@@ -32,9 +32,9 @@
         end
       end
 
-      context 'with failed_pipeline_for_service_account notification setting checked and pipeline failed' do
+      context 'with service_account_failed_pipeline notification setting checked and pipeline failed' do
         before do
-          notification_setting.update!(failed_pipeline_for_service_account: "true")
+          notification_setting.update!(service_account_failed_pipeline: "true")
           pipeline.drop!
         end
 
diff --git a/ee/spec/services/notification_recipients/builder/service_account_spec.rb b/ee/spec/services/notification_recipients/builder/service_account_spec.rb
index b510dcd0329efb..481ca9fe78bbd0 100644
--- a/ee/spec/services/notification_recipients/builder/service_account_spec.rb
+++ b/ee/spec/services/notification_recipients/builder/service_account_spec.rb
@@ -33,9 +33,9 @@
         end
       end
 
-      context 'with failed_pipeline_for_service_account notification setting checked and pipeline failed' do
+      context 'with service_account_failed_pipeline notification setting checked and pipeline failed' do
         before do
-          notification_setting.update!(failed_pipeline_for_service_account: "true")
+          notification_setting.update!(service_account_failed_pipeline: "true")
           pipeline.drop!
         end
 
@@ -44,9 +44,9 @@
         end
       end
 
-      context 'with failed_pipeline_for_service_account notification setting checked and pipeline succeeded' do
+      context 'with service_account_failed_pipeline notification setting checked and pipeline succeeded' do
         before do
-          notification_setting.update!(failed_pipeline_for_service_account: "true")
+          notification_setting.update!(service_account_failed_pipeline: "true")
           pipeline.succeed!
         end
 
@@ -55,9 +55,9 @@
         end
       end
 
-      context 'with success_pipeline_for_service_account notification setting checked and pipeline succeeded' do
+      context 'with service_account_success_pipeline notification setting checked and pipeline succeeded' do
         before do
-          notification_setting.update!(success_pipeline_for_service_account: "true")
+          notification_setting.update!(service_account_success_pipeline: "true")
           pipeline.succeed!
         end
 
@@ -66,11 +66,11 @@
         end
       end
 
-      context 'with fixed_pipeline_for_service_account notification setting checked and pipeline fixed' do
+      context 'with service_account_fixed_pipeline notification setting checked and pipeline fixed' do
         let(:pipeline_status) { "fixed" }
 
         before do
-          notification_setting.update!(fixed_pipeline_for_service_account: "true")
+          notification_setting.update!(service_account_fixed_pipeline: "true")
         end
 
         it 'returns an array with the subscribed user' do
-- 
GitLab


From bdcbded1c40865ff510e6a70f76f16f5d69a1d73 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@blessing.io>
Date: Thu, 1 May 2025 09:48:55 -0500
Subject: [PATCH 15/21] Fix duplicate constant

---
 app/assets/javascripts/notifications/constants.js | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/assets/javascripts/notifications/constants.js b/app/assets/javascripts/notifications/constants.js
index 3a5227df8d59f0..3bb706f7e70698 100644
--- a/app/assets/javascripts/notifications/constants.js
+++ b/app/assets/javascripts/notifications/constants.js
@@ -43,7 +43,7 @@ export const i18n = {
     close_merge_request: s__('NotificationEvent|Merge request is closed'),
     failed_pipeline: s__('NotificationEvent|Pipeline fails'),
     fixed_pipeline: s__('NotificationEvent|Pipeline is fixed'),
-    issue_due: s__('NotificationEvent|Issue is due soon'),
+    issue_due: s__('NotificationEvent|Issue is due tomorrow'),
     merge_merge_request: s__('NotificationEvent|Merge request is merged'),
     merge_when_pipeline_succeeds: s__('NotificationEvent|Merge request is set to auto-merge'),
     moved_project: s__('NotificationEvent|Project is moved'),
@@ -57,7 +57,6 @@ export const i18n = {
     reassign_merge_request: s__('NotificationEvent|Merge request is reassigned'),
     reopen_issue: s__('NotificationEvent|Issue is reopened'),
     reopen_merge_request: s__('NotificationEvent|Merge request is reopened'),
-    merge_when_pipeline_succeeds: s__('NotificationEvent|Merge request is set to auto-merge'),
 
     service_account_failed_pipeline: s__('NotificationEvent|Pipeline by Service Account fails'),
     service_account_fixed_pipeline: s__(
@@ -67,6 +66,5 @@ export const i18n = {
       'NotificationEvent|Pipeline by Service Account is successful',
     ),
     success_pipeline: s__('NotificationEvent|Pipeline is successful'),
-    approver: s__('NotificationEvent|You are added as an approver on a merge request'),
   },
 };
-- 
GitLab


From 3162396495be6e433d69ee4f7eb9b63b33e4dd43 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@blessing.io>
Date: Thu, 1 May 2025 09:51:12 -0500
Subject: [PATCH 16/21] Fix conflicts

---
 app/assets/javascripts/notifications/constants.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/assets/javascripts/notifications/constants.js b/app/assets/javascripts/notifications/constants.js
index 3bb706f7e70698..4b6c0db0c15e16 100644
--- a/app/assets/javascripts/notifications/constants.js
+++ b/app/assets/javascripts/notifications/constants.js
@@ -57,7 +57,6 @@ export const i18n = {
     reassign_merge_request: s__('NotificationEvent|Merge request is reassigned'),
     reopen_issue: s__('NotificationEvent|Issue is reopened'),
     reopen_merge_request: s__('NotificationEvent|Merge request is reopened'),
-
     service_account_failed_pipeline: s__('NotificationEvent|Pipeline by Service Account fails'),
     service_account_fixed_pipeline: s__(
       'NotificationEvent|Pipeline by Service Account is fixed',
-- 
GitLab


From 4b4187cea16c92ee66a52bb1e3bf93d9643b6ff4 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@blessing.io>
Date: Thu, 1 May 2025 10:54:29 -0500
Subject: [PATCH 17/21] Fix specs

---
 ee/spec/services/ee/notification_service_spec.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ee/spec/services/ee/notification_service_spec.rb b/ee/spec/services/ee/notification_service_spec.rb
index bcbf72304e5065..2c658f18230ed0 100644
--- a/ee/spec/services/ee/notification_service_spec.rb
+++ b/ee/spec/services/ee/notification_service_spec.rb
@@ -1072,17 +1072,17 @@ def add_users_with_subscription(project, issuable)
     let_it_be(:service_account) { create(:user, :service_account) }
     let_it_be(:u_custom_notification_enabled) do
       user = create_user_with_notification(:custom, 'custom_enabled')
-      update_custom_notification(:success_pipeline_for_service_account, user, resource: project)
-      update_custom_notification(:failed_pipeline_for_service_account, user, resource: project)
-      update_custom_notification(:fixed_pipeline_for_service_account, user, resource: project)
+      update_custom_notification(:service_account_success_pipeline, user, resource: project)
+      update_custom_notification(:service_account_failed_pipeline, user, resource: project)
+      update_custom_notification(:service_account_fixed_pipeline, user, resource: project)
       user
     end
 
     let_it_be(:u_custom_notification_disabled) do
       user = create_user_with_notification(:custom, 'custom_disabled')
-      update_custom_notification(:success_pipeline_for_service_account, user, resource: project, value: false)
-      update_custom_notification(:failed_pipeline_for_service_account, user, resource: project, value: false)
-      update_custom_notification(:fixed_pipeline_for_service_account, user, resource: project, value: false)
+      update_custom_notification(:service_account_success_pipeline, user, resource: project, value: false)
+      update_custom_notification(:service_account_failed_pipeline, user, resource: project, value: false)
+      update_custom_notification(:service_account_fixed_pipeline, user, resource: project, value: false)
       user
     end
 
-- 
GitLab


From bea07ae14add6d3d3678db14c3cafb6bcd869087 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@blessing.io>
Date: Thu, 1 May 2025 11:19:56 -0500
Subject: [PATCH 18/21] Fix style

---
 app/assets/javascripts/notifications/constants.js | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/assets/javascripts/notifications/constants.js b/app/assets/javascripts/notifications/constants.js
index 4b6c0db0c15e16..27191ceab3693b 100644
--- a/app/assets/javascripts/notifications/constants.js
+++ b/app/assets/javascripts/notifications/constants.js
@@ -58,9 +58,7 @@ export const i18n = {
     reopen_issue: s__('NotificationEvent|Issue is reopened'),
     reopen_merge_request: s__('NotificationEvent|Merge request is reopened'),
     service_account_failed_pipeline: s__('NotificationEvent|Pipeline by Service Account fails'),
-    service_account_fixed_pipeline: s__(
-      'NotificationEvent|Pipeline by Service Account is fixed',
-    ),
+    service_account_fixed_pipeline: s__('NotificationEvent|Pipeline by Service Account is fixed'),
     service_account_success_pipeline: s__(
       'NotificationEvent|Pipeline by Service Account is successful',
     ),
-- 
GitLab


From 94a58c24d0ffa6bdf25f20c97932dee0373ca4e8 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@blessing.io>
Date: Mon, 5 May 2025 13:44:02 -0500
Subject: [PATCH 19/21] Fix merge conflict incorrect resolution

---
 .../services/ee/notification_service_spec.rb  | 22 -------------------
 1 file changed, 22 deletions(-)

diff --git a/ee/spec/services/ee/notification_service_spec.rb b/ee/spec/services/ee/notification_service_spec.rb
index 2c658f18230ed0..5ec35957f444bc 100644
--- a/ee/spec/services/ee/notification_service_spec.rb
+++ b/ee/spec/services/ee/notification_service_spec.rb
@@ -381,28 +381,6 @@ def build_team(project, merge_request)
     end
   end
 
-  context 'mirror user changed' do
-    let(:mirror_user) { create(:user) }
-    let(:project) { create(:project, :mirror, mirror_user_id: mirror_user.id) }
-    let(:new_mirror_user) { project.team.owners.first }
-
-    it 'sends email' do
-      expect(Notify).to receive(:project_mirror_user_changed_email).with(new_mirror_user.id, mirror_user.name,
-        project.id).and_call_original
-
-      subject.project_mirror_user_changed(new_mirror_user, mirror_user.name, project)
-    end
-
-    it_behaves_like 'project emails are disabled' do
-      let(:notification_target)  { project }
-      let(:notification_trigger) { subject.project_mirror_user_changed(new_mirror_user, mirror_user.name, project) }
-
-      around do |example|
-        perform_enqueued_jobs { example.run }
-      end
-    end
-  end
-
   describe 'issues' do
     let(:group) { create(:group) }
     let(:project) { create(:project, :public, namespace: group) }
-- 
GitLab


From 58218caae48b4f41c4f35d03b497f7400457bdf0 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@gitlab.com>
Date: Thu, 15 May 2025 14:09:00 -0500
Subject: [PATCH 20/21] Apply 1 suggestion(s) to 1 file(s)

---
 .../20250328103435_add_new_event_to_notification_settings.rb    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/db/migrate/20250328103435_add_new_event_to_notification_settings.rb b/db/migrate/20250328103435_add_new_event_to_notification_settings.rb
index 6ceccdb6791475..51ba0d2f3f0637 100644
--- a/db/migrate/20250328103435_add_new_event_to_notification_settings.rb
+++ b/db/migrate/20250328103435_add_new_event_to_notification_settings.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddNewEventToNotificationSettings < Gitlab::Database::Migration[2.2]
-  milestone '17.11'
+  milestone '18.1'
 
   def change
     add_column :notification_settings, :service_account_failed_pipeline, :boolean, null: false, default: false,
-- 
GitLab


From d99ae231007ee3bd4b0a83a4e77f867b9a0e462d Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@blessing.io>
Date: Tue, 27 May 2025 11:20:55 -0500
Subject: [PATCH 21/21] Use existing method instead of custom logic

---
 .../notification_recipients/builder/service_account.rb     | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/ee/app/services/notification_recipients/builder/service_account.rb b/ee/app/services/notification_recipients/builder/service_account.rb
index 3f15ecbd52eee6..144c157e2a2450 100644
--- a/ee/app/services/notification_recipients/builder/service_account.rb
+++ b/ee/app/services/notification_recipients/builder/service_account.rb
@@ -12,12 +12,7 @@ def initialize(project, current_user, pipeline_status)
       end
 
       def build!
-        notification_by_sources = related_notification_settings_sources(:custom)
-
-        return if notification_by_sources.blank?
-
-        user_ids = NotificationSetting.from_union(notification_by_sources).select(:user_id)
-        add_recipients(user_scope.by_ids(user_ids), :custom, nil)
+        add_custom_notifications
       end
 
       def target; end
-- 
GitLab