Commit 02f38e80 authored by Georges Racinet's avatar Georges Racinet 🌳
Browse files

pipeline-emails: resist recipients being nil, RSpec tests and CI

In practice, the feature will work for the "Pipeline Emails"
integration, because these are triggered separately from
regular pipeline emails, with exactly the list as given in
the integration config.

Turns out that in all RSpec tests/examples about pipeline
emails, the list of recipients was actually `nil`, because
that is the email of the testing user. The case hence
seems articial, but let's not break because of that.

In the new test we don't use the `deliver_to` from `EmailSpec`
because it apparently actually reconstructs the envelop-to
from To and Bcc values.

Finally, the test in spec/models was explicitely looking
for recipients in the Bcc field, it had to be adapted after
CI complained.

branch : heptapod
parent c2db3539cc44
Pipeline #13847 skipped with stages
......@@ -33,7 +33,7 @@ def pipeline_mail(pipeline, recipients, status)
# recipients would contain all project watchers so it could be a lot.
mail_args = { bcc: recipients, subject: pipeline_subject(status) }
if recipients.length == 1
if !recipients.nil? && recipients.length == 1
# "Pipeline emails" integration can be used to send notifications of
# failure to mailing-lists. By default, GitLab sends those mails
# without a `To:` header, which trips anti-spam filters for mailing
......@@ -41,6 +41,7 @@ def pipeline_mail(pipeline, recipients, status)
# solution is to add a boolean to the UI to say "these emails should
# have a `To:` header".
mail_args[:to] = recipients[0]
mail_args.delete(:bcc) # avoid needless source of confusion
mail(**mail_args) do |format|
......@@ -6,6 +6,7 @@ bin/rspec \
spec/lib/gitlab/*_access*.rb \
spec/lib/gitlab/git \
spec/lib/gitlab/mercurial \
spec/mailers/emails/pipelines_spec.rb \
spec/models/*repository*.rb \
spec/models/user_spec.rb \
spec/models/merge_request_*_spec.rb \
......@@ -107,6 +107,21 @@
describe '#pipeline_failed_email_single_recipient' do
subject { Notify.pipeline_failed_email(pipeline, ['joe@heptapod.test']) }
let(:pipeline) { create(:ci_pipeline, project: project, ref: ref, sha: sha) }
let(:ref) { 'master' }
let(:sha) { project.commit(ref).sha }
it 'has a To field' do
# the `deliver_to` helper is not about the To header, whatever the
# doc says, but rather actual recipients (hence including the Bcc, which
# is not a header either).
expect( eq(['joe@heptapod.test'])
describe '#pipeline_fixed_email' do
subject { Notify.pipeline_fixed_email(pipeline, pipeline.user.try(:email)) }
......@@ -2884,7 +2884,7 @@ def create_build(name, stage_idx)
shared_examples 'sending a notification' do
it 'sends an email', :sidekiq_might_not_need_inline do
should_only_email(pipeline.user, kind: :bcc)
should_only_email(pipeline.user, kind: :to)
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment