diff options
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/project_services/builds_email_service.rb | 10 | ||||
-rw-r--r-- | spec/models/project_services/builds_email_service_spec.rb | 37 |
3 files changed, 43 insertions, 5 deletions
diff --git a/CHANGELOG b/CHANGELOG index 9baf6516ef6..ba98b3b100c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -60,6 +60,7 @@ v 8.7.0 (unreleased) - 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 - 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 - Fix error on language detection when repository has no HEAD (e.g., master branch). !3654 (Jeroen Bobbeldijk) diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb index f9f04838766..6ab6d7417b7 100644 --- a/app/models/project_services/builds_email_service.rb +++ b/app/models/project_services/builds_email_service.rb @@ -23,7 +23,7 @@ class BuildsEmailService < Service prop_accessor :recipients boolean_accessor :add_pusher 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 if properties.nil? @@ -87,10 +87,14 @@ class BuildsEmailService < Service end 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] - all_recipients << "#{data[:user][:email]}" + all_recipients << data[:user][:email] end all_recipients diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb index 2ccbff553f0..7c23c2efccd 100644 --- a/spec/models/project_services/builds_email_service_spec.rb +++ b/spec/models/project_services/builds_email_service_spec.rb @@ -3,9 +3,10 @@ require 'spec_helper' describe BuildsEmailService do let(:build) { create(:ci_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 service.recipients = 'test@gitlab.com' data[:build_status] = 'failed' @@ -40,4 +41,36 @@ describe BuildsEmailService do service.execute(data) 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 |