Commit 2fd05aed authored by Frank Groeneveld's avatar Frank Groeneveld

Allow empty recipient list when pusher is added

Closes #13574
parent a872a29e
...@@ -60,6 +60,7 @@ v 8.7.0 (unreleased) ...@@ -60,6 +60,7 @@ v 8.7.0 (unreleased)
- API: Expose 'updated_at' for issue, snippet, and merge request notes (Robert Schilling) - API: Expose 'updated_at' for issue, snippet, and merge request notes (Robert Schilling)
- API: User can leave a project through the API when not master or owner. !3613 - API: User can leave a project through the API when not master or owner. !3613
- Fix repository cache invalidation issue when project is recreated with an empty repo (Stan Hu) - Fix repository cache invalidation issue when project is recreated with an empty repo (Stan Hu)
- Fix: Allow empty recipients list for builds emails service when pushed is added (Frank Groeneveld)
v 8.6.6 v 8.6.6
- Fix error on language detection when repository has no HEAD (e.g., master branch). !3654 (Jeroen Bobbeldijk) - Fix error on language detection when repository has no HEAD (e.g., master branch). !3654 (Jeroen Bobbeldijk)
......
...@@ -23,7 +23,7 @@ class BuildsEmailService < Service ...@@ -23,7 +23,7 @@ class BuildsEmailService < Service
prop_accessor :recipients prop_accessor :recipients
boolean_accessor :add_pusher boolean_accessor :add_pusher
boolean_accessor :notify_only_broken_builds boolean_accessor :notify_only_broken_builds
validates :recipients, presence: true, if: :activated? validates :recipients, presence: true, if: ->(s) { s.activated? && !s.add_pusher? }
def initialize_properties def initialize_properties
if properties.nil? if properties.nil?
...@@ -87,10 +87,14 @@ class BuildsEmailService < Service ...@@ -87,10 +87,14 @@ class BuildsEmailService < Service
end end
def all_recipients(data) def all_recipients(data)
all_recipients = recipients.split(',').compact.reject(&:blank?) all_recipients = []
unless recipients.blank?
all_recipients += recipients.split(',').compact.reject(&:blank?)
end
if add_pusher? && data[:user][:email] if add_pusher? && data[:user][:email]
all_recipients << "#{data[:user][:email]}" all_recipients << data[:user][:email]
end end
all_recipients all_recipients
......
...@@ -3,9 +3,10 @@ require 'spec_helper' ...@@ -3,9 +3,10 @@ require 'spec_helper'
describe BuildsEmailService do describe BuildsEmailService do
let(:build) { create(:ci_build) } let(:build) { create(:ci_build) }
let(:data) { Gitlab::BuildDataBuilder.build(build) } let(:data) { Gitlab::BuildDataBuilder.build(build) }
let(:service) { BuildsEmailService.new } let!(:project) { create(:project, :public, ci_id: 1) }
let(:service) { described_class.new(project: project, active: true) }
describe :execute do describe '#execute' do
it 'sends email' do it 'sends email' do
service.recipients = 'test@gitlab.com' service.recipients = 'test@gitlab.com'
data[:build_status] = 'failed' data[:build_status] = 'failed'
...@@ -40,4 +41,36 @@ describe BuildsEmailService do ...@@ -40,4 +41,36 @@ describe BuildsEmailService do
service.execute(data) service.execute(data)
end end
end end
describe 'validations' do
context 'when pusher is not added' do
before { service.add_pusher = false }
it 'does not allow empty recipient input' do
service.recipients = ''
expect(service.valid?).to be false
end
it 'does allow non-empty recipient input' do
service.recipients = 'test@example.com'
expect(service.valid?).to be true
end
end
context 'when pusher is added' do
before { service.add_pusher = true }
it 'does allow empty recipient input' do
service.recipients = ''
expect(service.valid?).to be true
end
it 'does allow non-empty recipient input' do
service.recipients = 'test@example.com'
expect(service.valid?).to be true
end
end
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