Commit 26f51bb0 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'buildsemailservice-should-filter-out-blank-email-addresses-14883' into 'master'

Ensure empty recipients are rejected in BuildsEmailService

Fixes #14883.

See merge request !3543
parents d202c0b0 e79b867d
Pipeline #1496353 passed with stage
......@@ -14,6 +14,7 @@ v 8.7.0 (unreleased)
- Add links to CI setup documentation from project settings and builds pages
- Handle nil descriptions in Slack issue messages (Stan Hu)
- Add default scope to projects to exclude projects pending deletion
- Ensure empty recipients are rejected in BuildsEmailService
- Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.)
- Implement 'TODOs View' as an option for dashboard preferences !3379 (Elias W.)
- Gracefully handle notes on deleted commits in merge requests (Stan Hu)
......
......@@ -50,12 +50,15 @@ class BuildsEmailService < Service
def execute(push_data)
return unless supported_events.include?(push_data[:object_kind])
return unless should_build_be_notified?(push_data)
if should_build_be_notified?(push_data)
recipients = all_recipients(push_data)
if recipients.any?
BuildEmailWorker.perform_async(
push_data[:build_id],
all_recipients(push_data),
push_data,
recipients,
push_data
)
end
end
......@@ -84,7 +87,7 @@ class BuildsEmailService < Service
end
def all_recipients(data)
all_recipients = recipients.split(',')
all_recipients = recipients.split(',').compact.reject(&:blank?)
if add_pusher? && data[:user][:email]
all_recipients << "#{data[:user][:email]}"
......
......@@ -6,18 +6,38 @@ describe BuildsEmailService do
let(:service) { BuildsEmailService.new }
describe :execute do
it "sends email" do
it 'sends email' do
service.recipients = 'test@gitlab.com'
data[:build_status] = 'failed'
expect(BuildEmailWorker).to receive(:perform_async)
service.execute(data)
end
it "does not sends email with failed build and allowed_failure on" do
it 'does not send email with succeeded build and notify_only_broken_builds on' do
expect(service).to receive(:notify_only_broken_builds).and_return(true)
data[:build_status] = 'success'
expect(BuildEmailWorker).not_to receive(:perform_async)
service.execute(data)
end
it 'does not send email with failed build and build_allow_failure is true' do
data[:build_status] = 'failed'
data[:build_allow_failure] = true
expect(BuildEmailWorker).not_to receive(:perform_async)
service.execute(data)
end
it 'does not send email with unknown build status' do
data[:build_status] = 'foo'
expect(BuildEmailWorker).not_to receive(:perform_async)
service.execute(data)
end
it 'does not send email when recipients list is empty' do
service.recipients = ' ,, '
data[:build_status] = 'failed'
expect(BuildEmailWorker).not_to receive(:perform_async)
service.execute(data)
end
end
end
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