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.
This commit is contained in:
parent
12ef494db8
commit
045c671533
5 changed files with 23 additions and 22 deletions
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
Loading…
Reference in a new issue