Use bcc for pipeline emails because:

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.

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