diff --git a/app/assets/javascripts/notifications/constants.js b/app/assets/javascripts/notifications/constants.js index 690a24015a3e4b4de013e6b40853fd7ed0c801ee..27191ceab3693bb24286aba1a5ba58105eafa352 100644 --- a/app/assets/javascripts/notifications/constants.js +++ b/app/assets/javascripts/notifications/constants.js @@ -57,6 +57,11 @@ 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'), + service_account_success_pipeline: s__( + 'NotificationEvent|Pipeline by Service Account is successful', + ), success_pipeline: s__('NotificationEvent|Pipeline is successful'), }, }; diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb index 04563d180b58a9861f690144361d37faef4ed732..ef73ce3b1bc475e2717e98b2fe9dfd765e3cb707 100644 --- a/app/services/notification_recipients/build_service.rb +++ b/app/services/notification_recipients/build_service.rb @@ -38,3 +38,5 @@ def self.build_requested_review_recipients(...) end end end + +NotificationRecipients::BuildService.prepend_mod diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 6a70fca9de9ac3370b669bf09a81177efe3c0219..9c95a903b84438237079d584ca40d6a7496e4396 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -548,13 +548,18 @@ 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) 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 0000000000000000000000000000000000000000..51ba0d2f3f063740068db203ab5b5c7a41ec79d6 --- /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 '18.1' + + 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/20250328103435 b/db/schema_migrations/20250328103435 new file mode 100644 index 0000000000000000000000000000000000000000..1cede3c4e49634d940b6ea45801d51a5ad554232 --- /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 8acb0c9d1985e6c67318119dae52c7ac5eeaafcd..c1887476f66880a78c7f939ec96281557c8a62ff 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18155,7 +18155,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, + 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 396a51a78d65aa730b62f88d80c290969603582d..02bd0e9c2357cd6a6f8683ebdc275b76f4fb370d 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, + :service_account_failed_pipeline, + :service_account_success_pipeline, + :service_account_fixed_pipeline], ::User => [:approver], - ::Project => [:approver] + ::Project => [:approver, + :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/ee/notification_recipients/build_service.rb b/ee/app/services/ee/notification_recipients/build_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..589010675721af6447ef01bcff0dd5d4aba85da9 --- /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 # rubocop:disable Gitlab/BoundedContexts -- Existing module structure + module BuildService + def self.build_service_account_recipients(...) + ::NotificationRecipients::Builder::ServiceAccount.new(...).notification_recipients.map(&:user) + end + end + end +end diff --git a/ee/app/services/ee/notification_service.rb b/ee/app/services/ee/notification_service.rb index 9e69c29474741495070c59b993d884331a500f04..1bbe4ee6708b74221b22be8320b84246f176a239 100644 --- a/ee/app/services/ee/notification_service.rb +++ b/ee/app/services/ee/notification_service.rb @@ -104,6 +104,21 @@ 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? + + 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 # rubocop:disable GitlabSecurity/PublicSend -- not a security issue + end + end + private def oncall_user_removed_recipients(rotation, removed_user) 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 0000000000000000000000000000000000000000..144c157e2a2450bbc1a1ec7fba1beff3df82e32a --- /dev/null +++ b/ee/app/services/notification_recipients/builder/service_account.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module NotificationRecipients # rubocop:disable Gitlab/BoundedContexts -- Existing module structure + 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! + add_custom_notifications + end + + def target; end + + def custom_action + @custom_action ||= :"service_account_#{pipeline_status}_pipeline" + end + end + end +end diff --git a/ee/spec/models/ee/notification_setting_spec.rb b/ee/spec/models/ee/notification_setting_spec.rb index ff7e454523b324d891ea7cd239b9dd7b1959916c..ca507e4bda09151d69ae73c4f258a67d5e7d5237 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, + :service_account_failed_pipeline, + :service_account_success_pipeline, + :service_account_fixed_pipeline ] ) 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, + :service_account_failed_pipeline, + :service_account_success_pipeline, + :service_account_fixed_pipeline ] ) 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, + :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 new file mode 100644 index 0000000000000000000000000000000000000000..3c762810fe60553cc63d900fb54e602e9e829481 --- /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 service_account_failed_pipeline notification setting checked and pipeline failed' do + before do + notification_setting.update!(service_account_failed_pipeline: "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 diff --git a/ee/spec/services/ee/notification_service_spec.rb b/ee/spec/services/ee/notification_service_spec.rb index 573fb78d9f95af48c381627a76153b6906e9b8b6..5ec35957f444bce5ef9062e2fd90e1ffc09087b7 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 @@ -366,7 +386,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 } @@ -409,7 +431,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 @@ -431,7 +456,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 @@ -485,7 +514,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 @@ -530,7 +563,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) @@ -648,7 +684,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 @@ -712,7 +749,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) @@ -736,7 +774,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 @@ -768,7 +808,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! @@ -794,7 +836,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! @@ -839,7 +884,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 @@ -848,10 +894,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 @@ -898,14 +946,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 @@ -914,13 +966,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 @@ -946,11 +1003,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 @@ -981,6 +1041,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(: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(: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 + + 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) } @@ -991,7 +1097,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) @@ -1015,7 +1122,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) 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 0000000000000000000000000000000000000000..481ca9fe78bbd0fd422d0102cac8c43f971e7290 --- /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 service_account_failed_pipeline notification setting checked and pipeline failed' do + before do + notification_setting.update!(service_account_failed_pipeline: "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 service_account_failed_pipeline notification setting checked and pipeline succeeded' do + before do + notification_setting.update!(service_account_failed_pipeline: "true") + pipeline.succeed! + end + + it 'returns empty array' do + expect(build_service_method).to be_empty + end + end + + context 'with service_account_success_pipeline notification setting checked and pipeline succeeded' do + before do + notification_setting.update!(service_account_success_pipeline: "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 service_account_fixed_pipeline notification setting checked and pipeline fixed' do + let(:pipeline_status) { "fixed" } + + before do + notification_setting.update!(service_account_fixed_pipeline: "true") + end + + it 'returns an array with the subscribed user' do + expect(build_service_method).to match_array([user]) + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index be86a92c1753968aafb85163bb1996dc4869453d..430b81e704a4ba82df759f58db6def1cfe66a129 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -40791,6 +40791,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 ""