summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2016-03-17 20:03:51 +0100
committerRémy Coutable <remy@rymai.me>2016-03-25 13:05:15 +0100
commit9f218fc184894d61c10f738c59bce97780f06e25 (patch)
treeedc94e1c3ac7bae80edb14f86810a12e98e13f5e
parent31e76baf610e1307090a6bac3a7b3d525bce057a (diff)
downloadgitlab-ce-2364-fallback-to-in-reply-to-header.tar.gz
Improve and finish the fallback to the In-Reply-To and References header for the reply-by-email feature2364-fallback-to-in-reply-to-header
A few things to note: - The IncomingEmail feature is now enabled even without a correctly-formatted sub-address - Message-ID for new thread mail are kept the same so that subsequent notifications to this thread are grouped in the thread by the email service that receives the notification (i.e. In-Reply-To of the answer == Message-ID of the first thread message) - To maximize our chance to be able to retrieve the reply key, we look for it in the In-Reply-To header and the References header - The pattern for the fallback reply message id is "reply-[key]@[gitlab_host]" - Improve docs thanks to Axil
-rw-r--r--CHANGELOG1
-rw-r--r--app/mailers/notify.rb12
-rw-r--r--config/gitlab.yml.example2
-rw-r--r--config/mail_room.yml2
-rw-r--r--doc/incoming_email/README.md108
-rw-r--r--lib/gitlab/email/receiver.rb10
-rw-r--r--lib/gitlab/incoming_email.rb16
-rw-r--r--lib/tasks/gitlab/check.rake15
-rw-r--r--spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml (renamed from spec/fixtures/emails/key_in_headers_reply.eml)4
-rw-r--r--spec/fixtures/emails/valid_reply.eml4
-rw-r--r--spec/lib/gitlab/email/receiver_spec.rb26
-rw-r--r--spec/lib/gitlab/incoming_email_spec.rb26
-rw-r--r--spec/mailers/notify_spec.rb70
-rw-r--r--spec/mailers/shared/notify.rb77
14 files changed, 248 insertions, 125 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 5d9f4961ef5..a9c38c16d12 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -46,6 +46,7 @@ v 8.6.0
- Fix wiki search results point to raw source (Hiroyuki Sato)
- Don't load all of GitLab in mail_room
- Add information about `image` and `services` field at `job` level in the `.gitlab-ci.yml` documentation (Pat Turner)
+ - Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla)
- HTTP error pages work independently from location and config (Artem Sidorenko)
- Update `omniauth-saml` to 1.5.0 to allow for custom response attributes to be set
- Memoize @group in Admin::GroupsController (Yatish Mehta)
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index e7331d88517..826e5f96fa1 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -110,6 +110,10 @@ class Notify < BaseMailer
headers['Reply-To'] = address
+ fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>".freeze
+ headers['References'] ||= ''
+ headers['References'] << ' ' << fallback_reply_message_id
+
@reply_by_email = true
end
@@ -121,17 +125,11 @@ class Notify < BaseMailer
#
# See: mail_answer_thread
def mail_new_thread(model, headers = {})
- headers['Message-ID'] = message_reply_id
- headers['In-Reply-To'] = message_id(model)
- headers['References'] = message_id(model)
+ headers['Message-ID'] = message_id(model)
mail_thread(model, headers)
end
- def message_reply_id
- Gitlab.config.incoming_email["address"].gsub("%{key}", reply_key)
- end
-
# Send an email that responds to an existing conversation thread,
# with headers suitable for grouping by thread in email clients.
#
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 500b745f55e..fb1c3476f65 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -106,7 +106,7 @@ production: &base
enabled: false
# The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
- # The `%{key}` placeholder is added after the user part, after a `+` character, before the `@`.
+ # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
address: "gitlab-incoming+%{key}@gmail.com"
# Email account username
diff --git a/config/mail_room.yml b/config/mail_room.yml
index aed55f74eab..60257329f3e 100644
--- a/config/mail_room.yml
+++ b/config/mail_room.yml
@@ -17,7 +17,7 @@ if File.exists?(config_file)
config['start_tls'] = false if config['start_tls'].nil?
config['mailbox'] = "inbox" if config['mailbox'].nil?
- if config['enabled'] && config['address'] && config['address'].include?('%{key}')
+ if config['enabled'] && config['address']
redis_url = Gitlab::RedisConfig.new(rails_env).url
%>
-
diff --git a/doc/incoming_email/README.md b/doc/incoming_email/README.md
index 4cfb8402943..5a9a1582877 100644
--- a/doc/incoming_email/README.md
+++ b/doc/incoming_email/README.md
@@ -1,36 +1,99 @@
# Reply by email
-GitLab can be set up to allow users to comment on issues and merge requests by replying to notification emails.
+GitLab can be set up to allow users to comment on issues and merge requests by
+replying to notification emails.
-## Get a mailbox
+## Requirement
-Reply by email requires an IMAP-enabled email account, with a provider or server that supports [email sub-addressing](https://en.wikipedia.org/wiki/Email_address#Sub-addressing). Sub-addressing is a feature where any email to `user+some_arbitrary_tag@example.com` will end up in the mailbox for `user@example.com`, and is supported by providers such as Gmail, Google Apps, Yahoo! Mail, Outlook.com and iCloud, as well as the Postfix mail server which you can run on-premises.
+Reply by email requires an IMAP-enabled email account. GitLab allows you to use
+three strategies for this feature:
+- using email sub-addressing
+- using a dedicated email address
+- using a catch-all mailbox
-If you want to use Gmail / Google Apps with Reply by email, make sure you have [IMAP access enabled](https://support.google.com/mail/troubleshooter/1668960?hl=en#ts=1665018) and [allow less secure apps to access the account](https://support.google.com/accounts/answer/6010255).
+### Email sub-addressing
-To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these instructions](./postfix.md).
+**If your provider or server supports email sub-addressing, we recommend using it.**
+
+[Sub-addressing](https://en.wikipedia.org/wiki/Email_address#Sub-addressing) is
+a feature where any email to `user+some_arbitrary_tag@example.com` will end up
+in the mailbox for `user@example.com`, and is supported by providers such as
+Gmail, Google Apps, Yahoo! Mail, Outlook.com and iCloud, as well as the Postfix
+mail server which you can run on-premises.
+
+### Dedicated email address
+
+This solution is really simple to set up: you just have to create an email
+address dedicated to receive your users' replies to GitLab notifications.
+
+### Catch-all mailbox
+
+A [catch-all mailbox](https://en.wikipedia.org/wiki/Catch-all) for a domain will
+"catch all" the emails addressed to the domain that do not exist in the mail
+server.
+
+## How it works?
+
+### 1. GitLab sends a notification email
+
+When GitLab sends a notification and Reply by email is enabled, the `Reply-To`
+header is set to the address defined in your GitLab configuration, with the
+`%{key}` placeholder (if present) replaced by a specific "reply key". In
+addition, this "reply key" is also added to the `References` header.
+
+### 2. You reply to the notification email
+
+When you reply to the notification email, your email client will:
+
+- send the email to the `Reply-To` address it got from the notification email
+- set the `In-Reply-To` header to the value of the `Message-ID` header from the
+ notification email
+- set the `References` header to the value of the `Message-ID` plus the value of
+ the notification email's `References` header.
+
+### 3. GitLab receives your reply to the notification email
+
+When GitLab receives your reply, it will look for the "reply key" in the
+following headers, in this order:
+
+1. the `To` header
+1. the `References` header
+
+If it finds a reply key, it will be able to leave your reply as a comment on
+the entity the notification was about (issue, merge request, commit...).
+
+For more details about the `Message-ID`, `In-Reply-To`, and `References headers`,
+please consult [RFC 5322](https://tools.ietf.org/html/rfc5322#section-3.6.4).
## Set it up
+If you want to use Gmail / Google Apps with Reply by email, make sure you have
+[IMAP access enabled](https://support.google.com/mail/troubleshooter/1668960?hl=en#ts=1665018)
+and [allowed less secure apps to access the account](https://support.google.com/accounts/answer/6010255).
+
+To set up a basic Postfix mail server with IMAP access on Ubuntu, follow
+[these instructions](./postfix.md).
+
### Omnibus package installations
-1. Find the `incoming_email` section in `/etc/gitlab/gitlab.rb`, enable the feature and fill in the details for your specific IMAP server and email account:
+1. Find the `incoming_email` section in `/etc/gitlab/gitlab.rb`, enable the
+ feature and fill in the details for your specific IMAP server and email account:
```ruby
# Configuration for Postfix mail server, assumes mailbox incoming@gitlab.example.com
gitlab_rails['incoming_email_enabled'] = true
-
- # The email address including a placeholder for the key that references the item being replied to.
- # The `%{key}` placeholder is added after the user part, before the `@`.
+
+ # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
+ # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
gitlab_rails['incoming_email_address'] = "incoming+%{key}@gitlab.example.com"
-
+
# Email account username
# With third party providers, this is usually the full email address.
# With self-hosted email servers, this is usually the user part of the email address.
gitlab_rails['incoming_email_email'] = "incoming"
# Email account password
gitlab_rails['incoming_email_password'] = "[REDACTED]"
-
+
# IMAP server host
gitlab_rails['incoming_email_host'] = "gitlab.example.com"
# IMAP server port
@@ -47,18 +110,18 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
```ruby
# Configuration for Gmail / Google Apps, assumes mailbox gitlab-incoming@gmail.com
gitlab_rails['incoming_email_enabled'] = true
-
+
# The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
- # The `%{key}` placeholder is added after the user part, after a `+` character, before the `@`.
+ # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
gitlab_rails['incoming_email_address'] = "gitlab-incoming+%{key}@gmail.com"
-
+
# Email account username
# With third party providers, this is usually the full email address.
# With self-hosted email servers, this is usually the user part of the email address.
gitlab_rails['incoming_email_email'] = "gitlab-incoming@gmail.com"
# Email account password
gitlab_rails['incoming_email_password'] = "[REDACTED]"
-
+
# IMAP server host
gitlab_rails['incoming_email_host'] = "imap.gmail.com"
# IMAP server port
@@ -72,8 +135,6 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
gitlab_rails['incoming_email_mailbox_name'] = "inbox"
```
- As mentioned, the part after `+` in the address is ignored, and any email sent here will end up in the mailbox for `incoming@gitlab.example.com`/`gitlab-incoming@gmail.com`.
-
1. Reconfigure GitLab and restart mailroom for the changes to take effect:
```sh
@@ -97,7 +158,8 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
cd /home/git/gitlab
```
-1. Find the `incoming_email` section in `config/gitlab.yml`, enable the feature and fill in the details for your specific IMAP server and email account:
+1. Find the `incoming_email` section in `config/gitlab.yml`, enable the feature
+ and fill in the details for your specific IMAP server and email account:
```sh
sudo editor config/gitlab.yml
@@ -109,7 +171,7 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
enabled: true
# The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
- # The `%{key}` placeholder is added after the user part, after a `+` character, before the `@`.
+ # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
address: "incoming+%{key}@gitlab.example.com"
# Email account username
@@ -138,7 +200,7 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
enabled: true
# The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
- # The `%{key}` placeholder is added after the user part, after a `+` character, before the `@`.
+ # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
address: "gitlab-incoming+%{key}@gmail.com"
# Email account username
@@ -161,8 +223,6 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
mailbox: "inbox"
```
- As mentioned, the part after `+` in the address is ignored, and any email sent here will end up in the mailbox for `incoming@gitlab.example.com`/`gitlab-incoming@gmail.com`.
-
1. Enable `mail_room` in the init script at `/etc/default/gitlab`:
```sh
@@ -195,8 +255,8 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
incoming_email:
enabled: true
- # The email address including a placeholder for the key that references the item being replied to.
- # The `%{key}` placeholder is added after the user part, before the `@`.
+ # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
+ # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
address: "gitlab-incoming+%{key}@gmail.com"
# Email account username
diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb
index d55bacde5b0..97ef9851d71 100644
--- a/lib/gitlab/email/receiver.rb
+++ b/lib/gitlab/email/receiver.rb
@@ -63,10 +63,10 @@ module Gitlab
end
def reply_key
- key_from_to_address || key_from_in_reply_to_header
+ key_from_to_header || key_from_additional_headers
end
- def key_from_to_address
+ def key_from_to_header
key = nil
message.to.each do |address|
key = Gitlab::IncomingEmail.key_from_address(address)
@@ -76,11 +76,11 @@ module Gitlab
key
end
- def key_from_in_reply_to_header
+ def key_from_additional_headers
reply_key = nil
- message[:in_reply_to].message_ids.each do |message_id|
- reply_key = Gitlab::IncomingEmail.key_from_address(message_id)
+ Array(message.references).each do |message_id|
+ reply_key = Gitlab::IncomingEmail.key_from_fallback_reply_message_id(message_id)
break if reply_key
end
diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb
index 9068d79c95e..8ce9d32abe0 100644
--- a/lib/gitlab/incoming_email.rb
+++ b/lib/gitlab/incoming_email.rb
@@ -1,13 +1,10 @@
module Gitlab
module IncomingEmail
class << self
- def enabled?
- config.enabled && address_formatted_correctly?
- end
+ FALLBACK_REPLY_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze
- def address_formatted_correctly?
- config.address &&
- config.address.include?("%{key}")
+ def enabled?
+ config.enabled && config.address
end
def reply_address(key)
@@ -24,6 +21,13 @@ module Gitlab
match[1]
end
+ def key_from_fallback_reply_message_id(message_id)
+ match = message_id.match(FALLBACK_REPLY_MESSAGE_ID_REGEX)
+ return unless match
+
+ match[1]
+ end
+
def config
Gitlab.config.incoming_email
end
diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake
index 27ed57efe55..effb8eb6001 100644
--- a/lib/tasks/gitlab/check.rake
+++ b/lib/tasks/gitlab/check.rake
@@ -623,7 +623,6 @@ namespace :gitlab do
start_checking "Reply by email"
if Gitlab.config.incoming_email.enabled
- check_address_formatted_correctly
check_imap_authentication
if Rails.env.production?
@@ -643,20 +642,6 @@ namespace :gitlab do
# Checks
########################
- def check_address_formatted_correctly
- print "Address formatted correctly? ... "
-
- if Gitlab::IncomingEmail.address_formatted_correctly?
- puts "yes".green
- else
- puts "no".red
- try_fixing_it(
- "Make sure that the address in config/gitlab.yml includes the '%{key}' placeholder."
- )
- fix_and_rerun
- end
- end
-
def check_initd_configured_correctly
print "Init.d configured correctly? ... "
diff --git a/spec/fixtures/emails/key_in_headers_reply.eml b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml
index 8f288775d0a..39d5cefbc2a 100644
--- a/spec/fixtures/emails/key_in_headers_reply.eml
+++ b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml
@@ -3,12 +3,12 @@ Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.
Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700
-In-Reply-To: <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>
-References: <issue_1@localhost> <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>
Date: Thu, 13 Jun 2013 17:03:48 -0400
From: Jake the Dog <jake@adventuretime.ooo>
To: reply@appmail.adventuretime.ooo
Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+In-Reply-To: <issue_1@localhost>
+References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
Mime-Version: 1.0
Content-Type: text/plain;
diff --git a/spec/fixtures/emails/valid_reply.eml b/spec/fixtures/emails/valid_reply.eml
index e2b974174bd..980e10a8812 100644
--- a/spec/fixtures/emails/valid_reply.eml
+++ b/spec/fixtures/emails/valid_reply.eml
@@ -3,12 +3,12 @@ Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.
Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700
-In-Reply-To: <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>
-References: <issue_1@localhost> <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>
Date: Thu, 13 Jun 2013 17:03:48 -0400
From: Jake the Dog <jake@adventuretime.ooo>
To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+In-Reply-To: <issue_1@localhost>
+References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
Mime-Version: 1.0
Content-Type: text/plain;
diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb
index 13fd83ec7ba..36267faeb93 100644
--- a/spec/lib/gitlab/email/receiver_spec.rb
+++ b/spec/lib/gitlab/email/receiver_spec.rb
@@ -3,6 +3,7 @@ require "spec_helper"
describe Gitlab::Email::Receiver, lib: true do
before do
stub_incoming_email_setting(enabled: true, address: "reply+%{key}@appmail.adventuretime.ooo")
+ stub_config_setting(host: 'localhost')
end
let(:reply_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" }
@@ -138,17 +139,26 @@ describe Gitlab::Email::Receiver, lib: true do
expect(note.note).to include(markdown)
end
- context "when the reply key is in the In-Reply-To header" do
- let(:email_raw) { fixture_file("emails/key_in_headers_reply.eml") }
+ context 'when sub-addressing is not supported' do
+ before do
+ stub_incoming_email_setting(enabled: true, address: nil)
+ end
- it "creates a comment" do
- expect { receiver.execute }.to change { noteable.notes.count }.by(1)
- note = noteable.notes.last
+ shared_examples 'an email that contains a reply key' do |header|
+ it "fetches the reply key from the #{header} header and creates a comment" do
+ expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+ note = noteable.notes.last
- expect(note.author).to eq(sent_notification.recipient)
- expect(note.note).to include("I could not disagree more.")
+ expect(note.author).to eq(sent_notification.recipient)
+ expect(note.note).to include('I could not disagree more.')
+ end
+ end
+
+ context 'reply key is in the References header' do
+ let(:email_raw) { fixture_file('emails/reply_without_subaddressing_and_key_inside_references.eml') }
+
+ it_behaves_like 'an email that contains a reply key', 'References'
end
end
end
end
-
diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/incoming_email_spec.rb
index bcdba8d4c12..afb3e26f8fb 100644
--- a/spec/lib/gitlab/incoming_email_spec.rb
+++ b/spec/lib/gitlab/incoming_email_spec.rb
@@ -7,24 +7,8 @@ describe Gitlab::IncomingEmail, lib: true do
stub_incoming_email_setting(enabled: true)
end
- context "when the address is valid" do
- before do
- stub_incoming_email_setting(address: "replies+%{key}@example.com")
- end
-
- it "returns true" do
- expect(described_class.enabled?).to be_truthy
- end
- end
-
- context "when the address is invalid" do
- before do
- stub_incoming_email_setting(address: "replies@example.com")
- end
-
- it "returns false" do
- expect(described_class.enabled?).to be_falsey
- end
+ it 'returns true' do
+ expect(described_class.enabled?).to be_truthy
end
end
@@ -58,4 +42,10 @@ describe Gitlab::IncomingEmail, lib: true do
expect(described_class.key_from_address("replies+key@example.com")).to eq("key")
end
end
+
+ context 'self.key_from_fallback_reply_message_id' do
+ it 'returns reply key' do
+ expect(described_class.key_from_fallback_reply_message_id('reply-key@localhost')).to eq('key')
+ end
+ end
end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 0f3de33f361..631b5094f42 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -35,7 +35,9 @@ describe Notify do
subject { Notify.new_issue_email(issue.assignee_id, issue.id) }
it_behaves_like 'an assignee email'
- it_behaves_like 'an email starting a new thread', 'issue'
+ it_behaves_like 'an email starting a new thread with reply-by-email enabled' do
+ let(:model) { issue }
+ end
it_behaves_like 'it should show Gmail Actions View Issue link'
it_behaves_like 'an unsubscribeable thread'
@@ -73,9 +75,11 @@ describe Notify do
subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user.id) }
it_behaves_like 'a multiple recipients email'
- it_behaves_like 'an answer to an existing thread', 'issue'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { issue }
+ end
it_behaves_like 'it should show Gmail Actions View Issue link'
- it_behaves_like "an unsubscribeable thread"
+ it_behaves_like 'an unsubscribeable thread'
it 'is sent as the author' do
sender = subject.header[:from].addrs[0]
@@ -104,7 +108,9 @@ describe Notify do
subject { Notify.relabeled_issue_email(recipient.id, issue.id, %w[foo bar baz], current_user.id) }
it_behaves_like 'a multiple recipients email'
- it_behaves_like 'an answer to an existing thread', 'issue'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { issue }
+ end
it_behaves_like 'it should show Gmail Actions View Issue link'
it_behaves_like 'a user cannot unsubscribe through footer link'
it_behaves_like 'an email with a labels subscriptions link in its footer'
@@ -132,7 +138,9 @@ describe Notify do
let(:status) { 'closed' }
subject { Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user.id) }
- it_behaves_like 'an answer to an existing thread', 'issue'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { issue }
+ end
it_behaves_like 'it should show Gmail Actions View Issue link'
it_behaves_like 'an unsubscribeable thread'
@@ -163,7 +171,9 @@ describe Notify do
let(:new_issue) { create(:issue) }
subject { Notify.issue_moved_email(recipient, issue, new_issue, current_user) }
- it_behaves_like 'an answer to an existing thread', 'issue'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { issue }
+ end
it_behaves_like 'it should show Gmail Actions View Issue link'
it_behaves_like 'an unsubscribeable thread'
@@ -196,9 +206,11 @@ describe Notify do
subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) }
it_behaves_like 'an assignee email'
- it_behaves_like 'an email starting a new thread', 'merge_request'
+ it_behaves_like 'an email starting a new thread with reply-by-email enabled' do
+ let(:model) { merge_request }
+ end
it_behaves_like 'it should show Gmail Actions View Merge request link'
- it_behaves_like "an unsubscribeable thread"
+ it_behaves_like 'an unsubscribeable thread'
it 'has the correct subject' do
is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
@@ -216,14 +228,6 @@ describe Notify do
is_expected.to have_body_text /#{merge_request.target_branch}/
end
- it 'has the correct message-id set' do
- is_expected.to have_header 'Message-ID', /<reply\+(.*)@#{Gitlab.config.gitlab.host}>/
- end
-
- it 'has the correct references set' do
- is_expected.to have_header 'References', "<merge_request_#{merge_request.id}@#{Gitlab.config.gitlab.host}>"
- end
-
context 'when enabled email_author_in_body' do
before do
allow(current_application_settings).to receive(:email_author_in_body).and_return(true)
@@ -251,7 +255,9 @@ describe Notify do
subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) }
it_behaves_like 'a multiple recipients email'
- it_behaves_like 'an answer to an existing thread', 'merge_request'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { merge_request }
+ end
it_behaves_like 'it should show Gmail Actions View Merge request link'
it_behaves_like "an unsubscribeable thread"
@@ -282,7 +288,9 @@ describe Notify do
subject { Notify.relabeled_merge_request_email(recipient.id, merge_request.id, %w[foo bar baz], current_user.id) }
it_behaves_like 'a multiple recipients email'
- it_behaves_like 'an answer to an existing thread', 'merge_request'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { merge_request }
+ end
it_behaves_like 'it should show Gmail Actions View Merge request link'
it_behaves_like 'a user cannot unsubscribe through footer link'
it_behaves_like 'an email with a labels subscriptions link in its footer'
@@ -310,9 +318,11 @@ describe Notify do
let(:status) { 'reopened' }
subject { Notify.merge_request_status_email(recipient.id, merge_request.id, status, current_user.id) }
- it_behaves_like 'an answer to an existing thread', 'merge_request'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { merge_request }
+ end
it_behaves_like 'it should show Gmail Actions View Merge request link'
- it_behaves_like "an unsubscribeable thread"
+ it_behaves_like 'an unsubscribeable thread'
it 'is sent as the author' do
sender = subject.header[:from].addrs[0]
@@ -341,9 +351,11 @@ describe Notify do
subject { Notify.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) }
it_behaves_like 'a multiple recipients email'
- it_behaves_like 'an answer to an existing thread', 'merge_request'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { merge_request }
+ end
it_behaves_like 'it should show Gmail Actions View Merge request link'
- it_behaves_like "an unsubscribeable thread"
+ it_behaves_like 'an unsubscribeable thread'
it 'is sent as the merge author' do
sender = subject.header[:from].addrs[0]
@@ -460,9 +472,11 @@ describe Notify do
subject { Notify.note_commit_email(recipient.id, note.id) }
it_behaves_like 'a note email'
- it_behaves_like 'an answer to an existing thread', 'commit'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { commit }
+ end
it_behaves_like 'it should show Gmail Actions View Commit link'
- it_behaves_like "a user cannot unsubscribe through footer link"
+ it_behaves_like 'a user cannot unsubscribe through footer link'
it 'has the correct subject' do
is_expected.to have_subject /#{commit.title} \(#{commit.short_id}\)/
@@ -481,7 +495,9 @@ describe Notify do
subject { Notify.note_merge_request_email(recipient.id, note.id) }
it_behaves_like 'a note email'
- it_behaves_like 'an answer to an existing thread', 'merge_request'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { merge_request }
+ end
it_behaves_like 'it should show Gmail Actions View Merge request link'
it_behaves_like 'an unsubscribeable thread'
@@ -502,7 +518,9 @@ describe Notify do
subject { Notify.note_issue_email(recipient.id, note.id) }
it_behaves_like 'a note email'
- it_behaves_like 'an answer to an existing thread', 'issue'
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { issue }
+ end
it_behaves_like 'it should show Gmail Actions View Issue link'
it_behaves_like 'an unsubscribeable thread'
diff --git a/spec/mailers/shared/notify.rb b/spec/mailers/shared/notify.rb
index eafc5e5044b..56a6dbf96f9 100644
--- a/spec/mailers/shared/notify.rb
+++ b/spec/mailers/shared/notify.rb
@@ -10,6 +10,13 @@ shared_context 'gitlab email notification' do
ActionMailer::Base.deliveries.clear
email = recipient.emails.create(email: "notifications@example.com")
recipient.update_attribute(:notification_email, email.email)
+ stub_incoming_email_setting(enabled: true, address: "reply+%{key}@#{Gitlab.config.gitlab.host}")
+ end
+end
+
+shared_context 'reply-by-email is enabled with incoming address without %{key}' do
+ before do
+ stub_incoming_email_setting(enabled: true, address: "reply@#{Gitlab.config.gitlab.host}")
end
end
@@ -46,26 +53,76 @@ shared_examples 'an email with X-GitLab headers containing project details' do
end
end
-shared_examples 'an email starting a new thread' do |message_id_prefix|
- include_examples 'an email with X-GitLab headers containing project details'
+shared_examples 'a new thread email with reply-by-email enabled' do
+ let(:regex) { /\A<reply\-(.*)@#{Gitlab.config.gitlab.host}>\Z/ }
+
+ it 'has a Message-ID header' do
+ is_expected.to have_header 'Message-ID', "<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}>"
+ end
- it 'has a discussion identifier' do
- is_expected.to have_header 'Message-ID', /<reply\+(.*)@#{Gitlab.config.gitlab.host}>/
- is_expected.to have_header 'References', /<#{message_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/
+ it 'has a References header' do
+ is_expected.to have_header 'References', regex
end
end
-shared_examples 'an answer to an existing thread' do |thread_id_prefix|
+shared_examples 'a thread answer email with reply-by-email enabled' do
include_examples 'an email with X-GitLab headers containing project details'
+ let(:regex) { /\A<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}> <reply\-(.*)@#{Gitlab.config.gitlab.host}>\Z/ }
+
+ it 'has a Message-ID header' do
+ is_expected.to have_header 'Message-ID', /\A<(.*)@#{Gitlab.config.gitlab.host}>\Z/
+ end
+
+ it 'has a In-Reply-To header' do
+ is_expected.to have_header 'In-Reply-To', "<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}>"
+ end
+
+ it 'has a References header' do
+ is_expected.to have_header 'References', regex
+ end
it 'has a subject that begins with Re: ' do
is_expected.to have_subject /^Re: /
end
+end
+
+shared_examples 'an email starting a new thread with reply-by-email enabled' do
+ include_examples 'an email with X-GitLab headers containing project details'
+ include_examples 'a new thread email with reply-by-email enabled'
+
+ context 'when reply-by-email is enabled with incoming address with %{key}' do
+ it 'has a Reply-To header' do
+ is_expected.to have_header 'Reply-To', /<reply+(.*)@#{Gitlab.config.gitlab.host}>\Z/
+ end
+ end
+
+ context 'when reply-by-email is enabled with incoming address without %{key}' do
+ include_context 'reply-by-email is enabled with incoming address without %{key}'
+ include_examples 'a new thread email with reply-by-email enabled'
+
+ it 'has a Reply-To header' do
+ is_expected.to have_header 'Reply-To', /<reply@#{Gitlab.config.gitlab.host}>\Z/
+ end
+ end
+end
+
+shared_examples 'an answer to an existing thread with reply-by-email enabled' do
+ include_examples 'an email with X-GitLab headers containing project details'
+ include_examples 'a thread answer email with reply-by-email enabled'
+
+ context 'when reply-by-email is enabled with incoming address with %{key}' do
+ it 'has a Reply-To header' do
+ is_expected.to have_header 'Reply-To', /<reply+(.*)@#{Gitlab.config.gitlab.host}>\Z/
+ end
+ end
+
+ context 'when reply-by-email is enabled with incoming address without %{key}' do
+ include_context 'reply-by-email is enabled with incoming address without %{key}'
+ include_examples 'a thread answer email with reply-by-email enabled'
- it 'has headers that reference an existing thread' do
- is_expected.to have_header 'Message-ID', /<(.*)@#{Gitlab.config.gitlab.host}>/
- is_expected.to have_header 'References', /<#{thread_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/
- is_expected.to have_header 'In-Reply-To', /<#{thread_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/
+ it 'has a Reply-To header' do
+ is_expected.to have_header 'Reply-To', /<reply@#{Gitlab.config.gitlab.host}>\Z/
+ end
end
end