summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlex Kalderimis <akalderimis@gitlab.com>2019-08-21 19:23:27 +0000
committerMayra Cabrera <mcabrera@gitlab.com>2019-08-21 19:23:27 +0000
commitac94033b2db012f6d9e2dcca2bf48f7ac94f38a0 (patch)
treeb8971803961b85ab06c293b5fbfdff0a398b582e
parent2ef59c7e4bf133b6d92277b081fb1eb76bc3d0d9 (diff)
downloadgitlab-ce-ac94033b2db012f6d9e2dcca2bf48f7ac94f38a0.tar.gz
Handle namespaced models
We encountered issues with setting module headers for namespaced models. These changes address this. We retain the namespacing, but transform the classnames to make them into safe email headers.
-rw-r--r--app/mailers/notify.rb15
-rw-r--r--spec/mailers/notify_spec.rb67
2 files changed, 80 insertions, 2 deletions
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 5d292094a05..3683f2ea9a9 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -125,9 +125,8 @@ class Notify < BaseMailer
def mail_thread(model, headers = {})
add_project_headers
add_unsubscription_headers_and_links
+ add_model_headers(model)
- headers["X-GitLab-#{model.class.name}-ID"] = model.id
- headers["X-GitLab-#{model.class.name}-IID"] = model.iid if model.respond_to?(:iid)
headers['X-GitLab-Reply-Key'] = reply_key
@reason = headers['X-GitLab-NotificationReason']
@@ -196,6 +195,18 @@ class Notify < BaseMailer
@reply_key ||= SentNotification.reply_key
end
+ # This method applies threading headers to the email to identify
+ # the instance we are discussing.
+ #
+ # All model instances must have `#id`, and may implement `#iid`.
+ def add_model_headers(object)
+ # Use replacement so we don't strip the module.
+ prefix = "X-GitLab-#{object.class.name.gsub(/::/, '-')}"
+
+ headers["#{prefix}-ID"] = object.id
+ headers["#{prefix}-IID"] = object.iid if object.respond_to?(:iid)
+ end
+
def add_project_headers
return unless @project
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index dcc4b70a382..6cba7df114c 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -543,6 +543,73 @@ describe Notify do
end
end
+ describe '#mail_thread' do
+ set(:mail_thread_note) { create(:note) }
+
+ let(:headers) do
+ {
+ from: 'someone@test.com',
+ to: 'someone-else@test.com',
+ subject: 'something',
+ template_name: '_note_email' # re-use this for testing
+ }
+ end
+
+ let(:mailer) do
+ mailer = described_class.new
+ mailer.instance_variable_set(:@note, mail_thread_note)
+ mailer
+ end
+
+ context 'the model has no namespace' do
+ class TopLevelThing
+ include Referable
+ include Noteable
+
+ def to_reference(*_args)
+ 'tlt-ref'
+ end
+
+ def id
+ 'tlt-id'
+ end
+ end
+
+ subject do
+ mailer.send(:mail_thread, TopLevelThing.new, headers)
+ end
+
+ it 'has X-GitLab-Namespaced-Thing-ID header' do
+ expect(subject.header['X-GitLab-TopLevelThing-ID'].value).to eq('tlt-id')
+ end
+ end
+
+ context 'the model has a namespace' do
+ module Namespaced
+ class Thing
+ include Referable
+ include Noteable
+
+ def to_reference(*_args)
+ 'some-reference'
+ end
+
+ def id
+ 'some-id'
+ end
+ end
+ end
+
+ subject do
+ mailer.send(:mail_thread, Namespaced::Thing.new, headers)
+ end
+
+ it 'has X-GitLab-Namespaced-Thing-ID header' do
+ expect(subject.header['X-GitLab-Namespaced-Thing-ID'].value).to eq('some-id')
+ end
+ end
+ end
+
context 'for issue notes' do
let(:host) { Gitlab.config.gitlab.host }