emails: set References header for threading in mail user agents even with different subjects

- drop template headers() function as it wasn't used, and actually had bad implementation

rhodecode/apps/admin/views/settings.py

@@ -578,8 +578,7 b' class AdminSettingsView(BaseAppView):'
             'user': self._rhodecode_db_user
         }
 
-        (subject, headers, email_body,
-         email_body_plaintext) = EmailNotificationModel().render_email(
+        (subject, email_body, email_body_plaintext) = EmailNotificationModel().render_email(
             EmailNotificationModel.TYPE_EMAIL_TEST, **email_kwargs)
 
         recipients = [test_email] if test_email else None

rhodecode/apps/debug_style/views.py

@@ -376,8 +376,7 b' users: description edit fixes'
         }
 
         template_type = email_id.split('+')[0]
-        (c.subject, c.headers, c.email_body,
-         c.email_body_plaintext) = EmailNotificationModel().render_email(
+        (c.subject, c.email_body, c.email_body_plaintext) = EmailNotificationModel().render_email(
             template_type, **email_kwargs.get(email_id, {}))
 
         test_email = self.request.GET.get('email')

rhodecode/lib/celerylib/tasks.py

@@ -29,6 +29,7 b' import time'
 from pyramid import compat
 from pyramid_mailer.mailer import Mailer
 from pyramid_mailer.message import Message
+from email.utils import formatdate
 
 import rhodecode
 from rhodecode.lib import audit_logger
@@ -40,7 +41,8 b' from rhodecode.model.db import ('
 
 
 @async_task(ignore_result=True, base=RequestContextTask)
-def send_email(recipients, subject, body='', html_body='', email_config=None):
+def send_email(recipients, subject, body='', html_body='', email_config=None,
+               extra_headers=None):
     """
     Sends an email with defined parameters from the .ini files.
 
@@ -50,6 +52,7 b' def send_email(recipients, subject, body'
     :param body: body of the mail
     :param html_body: html version of body
     :param email_config: specify custom configuration for mailer
+    :param extra_headers: specify custom headers
     """
     log = get_logger(send_email)
 
@@ -108,13 +111,23 b' def send_email(recipients, subject, body'
         # sendmail_template='',
     )
 
+    if extra_headers is None:
+        extra_headers = {}
+
+    extra_headers.setdefault('Date', formatdate(time.time()))
+
+    if 'thread_ids' in extra_headers:
+        thread_ids = extra_headers.pop('thread_ids')
+        extra_headers['References'] = ' '.join('<{}>'.format(t) for t in thread_ids)
+
     try:
         mailer = Mailer(**email_conf)
 
         message = Message(subject=subject,
                           sender=email_conf['default_sender'],
                           recipients=recipients,
-                          body=body, html=html_body)
+                          body=body, html=html_body,
+                          extra_headers=extra_headers)
         mailer.send_immediately(message)
 
     except Exception:

rhodecode/lib/exc_tracking.py

@@ -143,8 +143,7 b' def send_exc_email(request, exc_id, exc_'
         'exc_traceback': read_exception(exc_id, prefix=None),
     }
 
-    (subject, headers, email_body,
-     email_body_plaintext) = EmailNotificationModel().render_email(
+    (subject, email_body, email_body_plaintext) = EmailNotificationModel().render_email(
         EmailNotificationModel.TYPE_EMAIL_EXCEPTION, **email_kwargs)
 
     run_task(tasks.send_email, recipients, subject,

rhodecode/model/comment.py

@@ -370,13 +370,18 b' class CommentsModel(BaseModel):'
                 repo.repo_name,
                 h.route_url('repo_summary', repo_name=repo.repo_name))
 
+            commit_url = h.route_url('repo_commit', repo_name=repo.repo_name,
+                                     commit_id=commit_id)
+
             # commit specifics
             kwargs.update({
                 'commit': commit_obj,
                 'commit_message': commit_obj.message,
                 'commit_target_repo_url': target_repo_url,
                 'commit_comment_url': commit_comment_url,
-                'commit_comment_reply_url': commit_comment_reply_url
+                'commit_comment_reply_url': commit_comment_reply_url,
+                'commit_url': commit_url,
+                'thread_ids': [commit_url, commit_comment_url],
             })
 
         elif pull_request_obj:
@@ -421,15 +426,14 b' class CommentsModel(BaseModel):'
                 'pr_comment_url': pr_comment_url,
                 'pr_comment_reply_url': pr_comment_reply_url,
                 'pr_closing': closing_pr,
+                'thread_ids': [pr_url, pr_comment_url],
             })
 
         recipients += [self._get_user(u) for u in (extra_recipients or [])]
 
         if send_email:
             # pre-generate the subject for notification itself
-            (subject,
-             _h, _e,  # we don't care about those
-             body_plaintext) = EmailNotificationModel().render_email(
+            (subject, _e, body_plaintext) = EmailNotificationModel().render_email(
                 notification_type, **kwargs)
 
             mention_recipients = set(

rhodecode/model/notification.py

@@ -131,15 +131,17 b' class NotificationModel(BaseModel):'
             # inject current recipient
             email_kwargs['recipient'] = recipient
             email_kwargs['mention'] = recipient in mention_recipients
-            (subject, headers, email_body,
-             email_body_plaintext) = EmailNotificationModel().render_email(
+            (subject, email_body, email_body_plaintext) = EmailNotificationModel().render_email(
                 notification_type, **email_kwargs)
 
-            log.debug(
-                'Creating notification email task for user:`%s`', recipient)
+            extra_headers = None
+            if 'thread_ids' in email_kwargs:
+                extra_headers = {'thread_ids': email_kwargs.pop('thread_ids')}
+
+            log.debug('Creating notification email task for user:`%s`', recipient)
             task = run_task(
                 tasks.send_email, recipient.email, subject,
-                email_body_plaintext, email_body)
+                email_body_plaintext, email_body, extra_headers=extra_headers)
             log.debug('Created email task: %s', task)
 
         return notification
@@ -342,8 +344,7 b' class EmailNotificationModel(BaseModel):'
         """
         Example usage::
 
-            (subject, headers, email_body,
-             email_body_plaintext) = EmailNotificationModel().render_email(
+            (subject, email_body, email_body_plaintext) = EmailNotificationModel().render_email(
                 EmailNotificationModel.TYPE_TEST, **email_kwargs)
 
         """
@@ -387,12 +388,6 b' class EmailNotificationModel(BaseModel):'
         subject = email_template.render('subject', **_kwargs)
 
         try:
-            headers = email_template.render('headers', **_kwargs)
-        except AttributeError:
-            # it's not defined in template, ok we can skip it
-            headers = ''
-
-        try:
             body_plaintext = email_template.render('body_plaintext', **_kwargs)
         except AttributeError:
             # it's not defined in template, ok we can skip it
@@ -408,4 +403,4 b' class EmailNotificationModel(BaseModel):'
             log.exception('Failed to parse body with premailer')
             pass
 
-        return subject, headers, body, body_plaintext
+        return subject, body, body_plaintext

rhodecode/model/pull_request.py

@@ -1344,12 +1344,11 b' class PullRequestModel(BaseModel):'
             'pull_request_source_repo_url': pr_source_repo_url,
 
             'pull_request_url': pr_url,
+            'thread_ids': [pr_url],
         }
 
         # pre-generate the subject for notification itself
-        (subject,
-         _h, _e,  # we don't care about those
-         body_plaintext) = EmailNotificationModel().render_email(
+        (subject, _e, body_plaintext) = EmailNotificationModel().render_email(
             notification_type, **kwargs)
 
         # create notification objects, and emails
@@ -1414,11 +1413,10 b' class PullRequestModel(BaseModel):'
             'added_files': file_changes.added,
             'modified_files': file_changes.modified,
             'removed_files': file_changes.removed,
+            'thread_ids': [pr_url],
         }
 
-        (subject,
-         _h, _e,  # we don't care about those
-         body_plaintext) = EmailNotificationModel().render_email(
+        (subject, _e, body_plaintext) = EmailNotificationModel().render_email(
             EmailNotificationModel.TYPE_PULL_REQUEST_UPDATE, **email_kwargs)
 
         # create notification objects, and emails

rhodecode/model/user.py

@@ -422,9 +422,7 b' class UserModel(BaseModel):'
             }
             notification_type = EmailNotificationModel.TYPE_REGISTRATION
             # pre-generate the subject for notification itself
-            (subject,
-             _h, _e,  # we don't care about those
-             body_plaintext) = EmailNotificationModel().render_email(
+            (subject, _e, body_plaintext) = EmailNotificationModel().render_email(
                 notification_type, **kwargs)
 
             # create notification objects, and emails
@@ -659,8 +657,7 b' class UserModel(BaseModel):'
                     'first_admin_email': User.get_first_super_admin().email
                 }
 
-                (subject, headers, email_body,
-                 email_body_plaintext) = EmailNotificationModel().render_email(
+                (subject, email_body, email_body_plaintext) = EmailNotificationModel().render_email(
                     EmailNotificationModel.TYPE_PASSWORD_RESET, **email_kwargs)
 
                 recipients = [user_email]
@@ -718,8 +715,7 b' class UserModel(BaseModel):'
                 'first_admin_email': User.get_first_super_admin().email
             }
 
-            (subject, headers, email_body,
-             email_body_plaintext) = EmailNotificationModel().render_email(
+            (subject, email_body, email_body_plaintext) = EmailNotificationModel().render_email(
                 EmailNotificationModel.TYPE_PASSWORD_RESET_CONFIRMATION,
                 **email_kwargs)
 

rhodecode/templates/debug_style/email.mako

@@ -8,11 +8,6 b''
 SUBJECT:
 <pre>${c.subject}</pre>
 
-HEADERS:
-<pre>
-${c.headers}
-</pre>
-
 PLAINTEXT:
 <pre>
 ${c.email_body_plaintext|n}

rhodecode/templates/email_templates/base.mako

@@ -68,9 +68,6 b' text_monospace = "\'Menlo\', \'Liberation M'
 
 %>
 
-## headers we additionally can set for email
-<%def name="headers()" filter="n,trim"></%def>
-
 <%def name="plaintext_footer()" filter="trim">
 ${_('This is a notification from RhodeCode.')} ${instance_url}
 </%def>

rhodecode/templates/email_templates/commit_comment.mako

@@ -15,17 +15,18 b' data = {'
     'comment_id': comment_id,
 
     'commit_id': h.show_id(commit),
+    'mention_prefix': '[mention] ' if mention else '',
 }
 %>
 
 
 % if comment_file:
-    ${(_('[mention]') if mention else '')} ${_('{user} left a {comment_type} on file `{comment_file}` in commit `{commit_id}`').format(**data)} ${_('in the `{repo_name}` repository').format(**data) |n}
+    ${_('{mention_prefix}{user} left a {comment_type} on file `{comment_file}` in commit `{commit_id}`').format(**data)} ${_('in the `{repo_name}` repository').format(**data) |n}
 % else:
     % if status_change:
-    ${(_('[mention]') if mention else '')} ${_('[status: {status}] {user} left a {comment_type} on commit `{commit_id}`').format(**data) |n} ${_('in the `{repo_name}` repository').format(**data) |n}
+    ${_('{mention_prefix}[status: {status}] {user} left a {comment_type} on commit `{commit_id}`').format(**data) |n} ${_('in the `{repo_name}` repository').format(**data) |n}
     % else:
-    ${(_('[mention]') if mention else '')} ${_('{user} left a {comment_type} on commit `{commit_id}`').format(**data) |n} ${_('in the `{repo_name}` repository').format(**data) |n}
+    ${_('{mention_prefix}{user} left a {comment_type} on commit `{commit_id}`').format(**data) |n} ${_('in the `{repo_name}` repository').format(**data) |n}
     % endif
 % endif
 

rhodecode/templates/email_templates/pull_request_comment.mako

@@ -16,17 +16,18 b' data = {'
 
     'pr_title': pull_request.title,
     'pr_id': pull_request.pull_request_id,
+    'mention_prefix': '[mention] ' if mention else '',
 }
 %>
 
 
 % if comment_file:
-    ${(_('[mention]') if mention else '')} ${_('{user} left a {comment_type} on file `{comment_file}` in pull request !{pr_id}: "{pr_title}"').format(**data) |n}
+    ${_('{mention_prefix}{user} left a {comment_type} on file `{comment_file}` in pull request !{pr_id}: "{pr_title}"').format(**data) |n}
 % else:
     % if status_change:
-    ${(_('[mention]') if mention else '')} ${_('[status: {status}] {user} left a {comment_type} on pull request !{pr_id}: "{pr_title}"').format(**data) |n}
+    ${_('{mention_prefix}[status: {status}] {user} left a {comment_type} on pull request !{pr_id}: "{pr_title}"').format(**data) |n}
     % else:
-    ${(_('[mention]') if mention else '')} ${_('{user} left a {comment_type} on pull request !{pr_id}: "{pr_title}"').format(**data) |n}
+    ${_('{mention_prefix}{user} left a {comment_type} on pull request !{pr_id}: "{pr_title}"').format(**data) |n}
     % endif
 % endif
 

rhodecode/templates/email_templates/test.mako

@@ -5,10 +5,6 b''
 Test "Subject" ${_('hello "world"')|n}
 </%def>
 
-<%def name="headers()" filter="n,trim">
-X=Y
-</%def>
-
 ## plain text version of the email. Empty by default
 <%def name="body_plaintext()" filter="n,trim">
 Email Plaintext Body

rhodecode/tests/lib/test_mako_emails.py

@@ -35,15 +35,12 b' def test_get_template_obj(app, request_s'
 
 def test_render_email(app, http_host_only_stub):
     kwargs = {}
-    subject, headers, body, body_plaintext = EmailNotificationModel().render_email(
+    subject, body, body_plaintext = EmailNotificationModel().render_email(
         EmailNotificationModel.TYPE_TEST, **kwargs)
 
     # subject
     assert subject == 'Test "Subject" hello "world"'
 
-    # headers
-    assert headers == 'X=Y'
-
     # body plaintext
     assert body_plaintext == 'Email Plaintext Body'
 
@@ -80,7 +77,7 b' def test_render_pr_email(app, user_admin'
         'pull_request_url': 'http://localhost/pr1',
     }
 
-    subject, headers, body, body_plaintext = EmailNotificationModel().render_email(
+    subject, body, body_plaintext = EmailNotificationModel().render_email(
         EmailNotificationModel.TYPE_PULL_REQUEST, **kwargs)
 
     # subject
@@ -133,7 +130,7 b' def test_render_pr_update_email(app, use'
         'removed_files': file_changes.removed,
     }
 
-    subject, headers, body, body_plaintext = EmailNotificationModel().render_email(
+    subject, body, body_plaintext = EmailNotificationModel().render_email(
         EmailNotificationModel.TYPE_PULL_REQUEST_UPDATE, **kwargs)
 
     # subject
@@ -188,7 +185,6 b' def test_render_comment_subject_no_newli'
 
         'pull_request_url': 'http://code.rc.com/_pr/123'
     }
-    subject, headers, body, body_plaintext = EmailNotificationModel().render_email(
-        email_type, **kwargs)
+    subject, body, body_plaintext = EmailNotificationModel().render_email(email_type, **kwargs)
 
     assert '\n' not in subject