diff options
| -rw-r--r-- | app/mailers/emails/pipelines.rb | 17 | ||||
| -rw-r--r-- | app/services/notification_service.rb | 4 | ||||
| -rw-r--r-- | spec/models/ci/pipeline_spec.rb | 2 | ||||
| -rw-r--r-- | spec/support/email_helpers.rb | 8 | ||||
| -rw-r--r-- | spec/workers/pipeline_notification_worker_spec.rb | 14 |
5 files changed, 23 insertions, 22 deletions
diff --git a/app/mailers/emails/pipelines.rb b/app/mailers/emails/pipelines.rb index 601c8b5cd62..9460a6cd2be 100644 --- a/app/mailers/emails/pipelines.rb +++ b/app/mailers/emails/pipelines.rb @@ -1,22 +1,27 @@ module Emails module Pipelines - def pipeline_success_email(pipeline, to) - pipeline_mail(pipeline, to, 'succeeded') + def pipeline_success_email(pipeline, recipients) + pipeline_mail(pipeline, recipients, 'succeeded') end - def pipeline_failed_email(pipeline, to) - pipeline_mail(pipeline, to, 'failed') + def pipeline_failed_email(pipeline, recipients) + pipeline_mail(pipeline, recipients, 'failed') end private - def pipeline_mail(pipeline, to, status) + def pipeline_mail(pipeline, recipients, status) @project = pipeline.project @pipeline = pipeline @merge_request = pipeline.merge_requests.first add_headers - mail(to: to, subject: pipeline_subject(status), skip_premailer: true) do |format| + # We use bcc here because we don't want to generate this emails for a + # thousand times. This could be potentially expensive in a loop, and + # recipients would contain all project watchers so it could be a lot. + mail(bcc: recipients, + subject: pipeline_subject(status), + skip_premailer: true) do |format| format.html { render layout: false } format.text end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index d4976aa8362..d5d69248af7 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -323,9 +323,7 @@ class NotificationService nil, # The acting user, who won't be added to recipients action: pipeline.status).map(&:email) - recipients.each do |to| - mailer.public_send(email_template, pipeline, to).deliver_later - end + mailer.public_send(email_template, pipeline, recipients).deliver_later end protected diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index caf191e11ae..4032029b80b 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -536,7 +536,7 @@ describe Ci::Pipeline, models: true do shared_examples 'sending a notification' do it 'sends an email' do - should_only_email(pipeline.user) + should_only_email(pipeline.user, kind: :bcc) end end diff --git a/spec/support/email_helpers.rb b/spec/support/email_helpers.rb index 36f5a2d5975..a946f10f71a 100644 --- a/spec/support/email_helpers.rb +++ b/spec/support/email_helpers.rb @@ -7,8 +7,8 @@ module EmailHelpers ActionMailer::Base.deliveries.clear end - def should_only_email(*users) - recipients = email_recipients + def should_only_email(*users, kind: :to) + recipients = email_recipients(kind: kind) users.each { |user| should_email(user, recipients) } @@ -27,7 +27,7 @@ module EmailHelpers expect(ActionMailer::Base.deliveries).to be_empty end - def email_recipients - ActionMailer::Base.deliveries.flat_map(&:to) + def email_recipients(kind: :to) + ActionMailer::Base.deliveries.flat_map(&kind) end end diff --git a/spec/workers/pipeline_notification_worker_spec.rb b/spec/workers/pipeline_notification_worker_spec.rb index c641d7a7801..c334b2057a6 100644 --- a/spec/workers/pipeline_notification_worker_spec.rb +++ b/spec/workers/pipeline_notification_worker_spec.rb @@ -26,15 +26,13 @@ describe PipelineNotificationWorker do subject.perform(pipeline.id) end - expected_receivers = [pusher, watcher].uniq.sort_by(&:email) - actual = ActionMailer::Base.deliveries.sort_by(&:to) + emails = ActionMailer::Base.deliveries + actual = emails.flat_map(&:bcc).sort + expected_receivers = [pusher, watcher].map(&:email).uniq.sort - expect(expected_receivers.size).to eq(actual.size) - - actual.zip(expected_receivers).each do |(email, receiver)| - expect(email.subject).to include(email_subject) - expect(email.to).to eq([receiver.email]) - end + expect(actual).to eq(expected_receivers) + expect(emails.size).to eq(1) + expect(emails.last.subject).to include(email_subject) end end |