diff options
author | George Thomas <iamgeorgethomas@gmail.com> | 2018-07-12 14:30:36 +0530 |
---|---|---|
committer | George Thomas <iamgeorgethomas@gmail.com> | 2018-07-19 12:37:59 +0530 |
commit | 32401535b9b043e258e925474dbd54adbf5a5056 (patch) | |
tree | a9705b5c7c7277d25c364573371de712af2eb27f | |
parent | 7f0431dd8550ac9d229d1383c03386c1634d015f (diff) | |
download | gitlab-ce-32401535b9b043e258e925474dbd54adbf5a5056.tar.gz |
Improve email address parsing
If you enter the following RFC 2822 compliant address:
`John Doe <john@doe.com>`
Gitlab will attempt to send three emails:
1) John
2) Doe
3) john@doe.com
With this change given the following:
`John Doe <johndoe@example.com>`
`Jane Doe <janedoe@example.com>`
Gitlab will send emails to `johndoe@example.com` and `janedoe@example.com`
-rw-r--r-- | app/workers/emails_on_push_worker.rb | 8 | ||||
-rw-r--r-- | changelogs/unreleased/accept-rf3-2822-compliant-addresses.yml | 5 | ||||
-rw-r--r-- | doc/user/project/integrations/emails_on_push.md | 2 | ||||
-rw-r--r-- | spec/workers/emails_on_push_worker_spec.rb | 61 |
4 files changed, 62 insertions, 14 deletions
diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index 8d0cfc73ccd..17ad1d5ab88 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -51,7 +51,7 @@ class EmailsOnPushWorker end end - recipients.split.each do |recipient| + valid_recipients(recipients).each do |recipient| begin send_email( recipient, @@ -89,4 +89,10 @@ class EmailsOnPushWorker email.header[:skip_premailer] = true if skip_premailer email.deliver_now end + + def valid_recipients(recipients) + recipients.split.select do |recipient| + recipient.include?('@') + end + end end diff --git a/changelogs/unreleased/accept-rf3-2822-compliant-addresses.yml b/changelogs/unreleased/accept-rf3-2822-compliant-addresses.yml new file mode 100644 index 00000000000..97d017613ba --- /dev/null +++ b/changelogs/unreleased/accept-rf3-2822-compliant-addresses.yml @@ -0,0 +1,5 @@ +--- +title: Emails on push recipients now accepts formats like John Doe <johndoe@example.com> +merge_request: +author: George Thomas +type: added diff --git a/doc/user/project/integrations/emails_on_push.md b/doc/user/project/integrations/emails_on_push.md index c01da883562..b050c83fb72 100644 --- a/doc/user/project/integrations/emails_on_push.md +++ b/doc/user/project/integrations/emails_on_push.md @@ -6,7 +6,7 @@ every change that is pushed to your project. Navigate to the [Integrations page](project_services.md#accessing-the-project-services) and select the **Emails on push** service to configure it. -In the _Recipients_ area, provide a list of emails separated by commas. +In the _Recipients_ area, provide a list of emails separated by spaces or newlines. You can configure any of the following settings depending on your preference. diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb index 318aad4bc1e..f17c5ac6aac 100644 --- a/spec/workers/emails_on_push_worker_spec.rb +++ b/spec/workers/emails_on_push_worker_spec.rb @@ -100,10 +100,6 @@ describe EmailsOnPushWorker, :mailer do end context "when there are multiple recipients" do - let(:recipients) do - 1.upto(5).map { |i| user.email.sub('@', "+#{i}@") }.join("\n") - end - before do # This is a hack because we modify the mail object before sending, for efficency, # but the TestMailer adapter just appends the objects to an array. To clone a mail @@ -114,16 +110,57 @@ describe EmailsOnPushWorker, :mailer do end end - it "sends the mail to each of the recipients" do - perform - expect(ActionMailer::Base.deliveries.count).to eq(5) - expect(ActionMailer::Base.deliveries.map(&:to).flatten).to contain_exactly(*recipients.split) + context "when the recipient addresses are a list of email addresses" do + let(:recipients) do + 1.upto(5).map { |i| user.email.sub('@', "+#{i}@") }.join("\n") + end + + it "sends the mail to each of the recipients" do + perform + + expect(ActionMailer::Base.deliveries.count).to eq(5) + expect(email_recipients).to contain_exactly(*recipients.split) + end + + it "only generates the mail once" do + expect(Notify).to receive(:repository_push_email).once.and_call_original + expect(Premailer::Rails::CustomizedPremailer).to receive(:new).once.and_call_original + + perform + end end - it "only generates the mail once" do - expect(Notify).to receive(:repository_push_email).once.and_call_original - expect(Premailer::Rails::CustomizedPremailer).to receive(:new).once.and_call_original - perform + context "when the recipient addresses contains angle brackets and are separated by spaces" do + let(:recipients) { "John Doe <johndoe@example.com> Jane Doe <janedoe@example.com>" } + + it "accepts emails separated by whitespace" do + perform + + expect(ActionMailer::Base.deliveries.count).to eq(2) + expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com") + end + end + + context "when the recipient addresses contain a mix of emails with and without angle brackets" do + let(:recipients) { "johndoe@example.com Jane Doe <janedoe@example.com>" } + + it "accepts both kind of emails" do + perform + + expect(ActionMailer::Base.deliveries.count).to eq(2) + expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com") + end + end + + context "when the recipient addresses contains angle brackets and are separated by newlines" do + let(:recipients) { "John Doe <johndoe@example.com>\nJane Doe <janedoe@example.com>" } + + it "accepts emails separated by newlines" do + perform + + expect(ActionMailer::Base.deliveries.count).to eq(2) + expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com") + end end end end |