Merge branch '37759-also-treat-newlines-as-separator' into 'master'

Treat newlines as separators for pipeline emails service

Closes #37759

See merge request gitlab-org/gitlab-ce!14250

3 files changed, 30 insertions, 14 deletions

diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb
index 9d37184be2c..6a3118a11b8 100644
--- a/app/models/project_services/pipelines_email_service.rb
+++ b/app/models/project_services/pipelines_email_service.rb
@@ -80,6 +80,6 @@ class PipelinesEmailService < Service
   end
 
   def retrieve_recipients(data)
-    recipients.to_s.split(',').reject(&:blank?)
+    recipients.to_s.split(/[,(?:\r?\n) ]+/).reject(&:empty?)
   end
 end
diff --git a/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml b/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml
new file mode 100644
index 00000000000..6894e650c11
--- /dev/null
+++ b/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml
@@ -0,0 +1,5 @@
+---
+title: Allow using newlines in pipeline email service recipients
+merge_request: 14250
+author:
+type: fixed
diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb
index 5faab9ba38b..be07ca2d945 100644
--- a/spec/models/project_services/pipelines_email_service_spec.rb
+++ b/spec/models/project_services/pipelines_email_service_spec.rb
@@ -6,7 +6,8 @@ describe PipelinesEmailService, :mailer do
   end
 
   let(:project) { create(:project, :repository) }
-  let(:recipient) { 'test@gitlab.com' }
+  let(:recipients) { 'test@gitlab.com' }
+  let(:receivers) { [recipients] }
 
   let(:data) do
     Gitlab::DataBuilder::Pipeline.build(pipeline)
@@ -48,18 +49,24 @@ describe PipelinesEmailService, :mailer do
 
   shared_examples 'sending email' do
     before do
+      subject.recipients = recipients
+
       perform_enqueued_jobs do
         run
       end
     end
 
     it 'sends email' do
-      should_only_email(double(notification_email: recipient), kind: :bcc)
+      emails = receivers.map { |r| double(notification_email: r) }
+
+      should_only_email(*emails, kind: :bcc)
     end
   end
 
   shared_examples 'not sending email' do
     before do
+      subject.recipients = recipients
+
       perform_enqueued_jobs do
         run
       end
@@ -75,10 +82,6 @@ describe PipelinesEmailService, :mailer do
       subject.test(data)
     end
 
-    before do
-      subject.recipients = recipient
-    end
-
     context 'when pipeline is failed' do
       before do
         data[:object_attributes][:status] = 'failed'
@@ -104,10 +107,6 @@ describe PipelinesEmailService, :mailer do
     end
 
     context 'with recipients' do
-      before do
-        subject.recipients = recipient
-      end
-
       context 'with failed pipeline' do
         before do
           data[:object_attributes][:status] = 'failed'
@@ -152,9 +151,7 @@ describe PipelinesEmailService, :mailer do
     end
 
     context 'with empty recipients list' do
-      before do
-        subject.recipients = ' ,, '
-      end
+      let(:recipients) { ' ,, ' }
 
       context 'with failed pipeline' do
         before do
@@ -165,5 +162,19 @@ describe PipelinesEmailService, :mailer do
         it_behaves_like 'not sending email'
       end
     end
+
+    context 'with recipients list separating with newlines' do
+      let(:recipients) { "\ntest@gitlab.com,  \r\nexample@gitlab.com" }
+      let(:receivers) { %w[test@gitlab.com example@gitlab.com] }
+
+      context 'with failed pipeline' do
+        before do
+          data[:object_attributes][:status] = 'failed'
+          pipeline.update(status: 'failed')
+        end
+
+        it_behaves_like 'sending email'
+      end
+    end
   end
 end