[PATCH 3 of 6] email: send comment and pullrequest mails with the author's name in 'From'

Thomas De Schampheleire patrickdepinguin at gmail.com
Tue Sep 1 16:28:35 UTC 2015


# HG changeset patch
# User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
# Date 1440706779 -7200
#      Thu Aug 27 22:19:39 2015 +0200
# Node ID 6f6acca6da4ad913f45eb18441575ea04d27eedd
# Parent  7117b34f08f32a9955c22845882364d1a55b841b
email: send comment and pullrequest mails with the author's name in 'From'

When emails are sent for comments and pullrequest invitations, set the From
header to:
    Author's Name (no-reply) <generic email address>

Using the name of the person that causes the email, makes the emails more
useful and interpretable for the recipient of the emails.
To avoid replies directly to the author, triggering an 'offline' email
discussion that is not visible in the Kallithea interface, a generic
'no-reply' email address is used instead of the author's email
address.
This approach is assumed to be accepted by spam filters, as several other
web services are using the same approach.

The sender used for other email types, e.g. password reset mails, is
untouched and remains the value configured in app_email_from.

The sender used for the SMTP envelope is untouched as well.

Based on code by Cedric De Herdt.

diff --git a/docs/usage/email.rst b/docs/usage/email.rst
--- a/docs/usage/email.rst
+++ b/docs/usage/email.rst
@@ -39,9 +39,14 @@ Recipients will see these emails origina
 ``app_email_from`` setting in the configuration file. This setting can either
 contain only an email address, like `kallithea-noreply at example.com`, or both
 a name and an address in the following format: `Kallithea
-<kallithea-noreply at example.com>`. The subject of these emails can
-optionally be prefixed with the value of ``email_prefix`` in the configuration
-file.
+<kallithea-noreply at example.com>`. However, if the email is sent due to an
+action of a particular user, for example when a comment is given or a pull
+request created, the name of that user will be combined with the email address
+specified in ``app_email_from`` to form the sender (and any name part in that
+configuration setting disregarded).
+
+The subject of these emails can optionally be prefixed with the value of
+``email_prefix`` in the configuration file.
 
 
 Error emails
diff --git a/kallithea/lib/celerylib/tasks.py b/kallithea/lib/celerylib/tasks.py
--- a/kallithea/lib/celerylib/tasks.py
+++ b/kallithea/lib/celerylib/tasks.py
@@ -45,6 +45,7 @@ from kallithea.lib.celerylib import run_
 from kallithea.lib.helpers import person
 from kallithea.lib.rcmail.smtp_mailer import SmtpMailer
 from kallithea.lib.utils import add_cache, action_logger
+from kallithea.lib.vcs.utils import author_email
 from kallithea.lib.compat import json, OrderedDict
 from kallithea.lib.hooks import log_create_repository
 
@@ -247,7 +248,7 @@ def get_commits_stats(repo_name, ts_min_
 
 @task(ignore_result=True)
 @dbsession
-def send_email(recipients, subject, body='', html_body='', headers=None):
+def send_email(recipients, subject, body='', html_body='', headers=None, author=None):
     """
     Sends an email with defined parameters from the .ini files.
 
@@ -256,9 +257,16 @@ def send_email(recipients, subject, body
     :param subject: subject of the mail
     :param body: body of the mail
     :param html_body: html version of body
+    :param headers: dictionary of prepopulated e-mail headers
+    :param author: User object of the author of this mail, if known and relevant
     """
     log = get_logger(send_email)
     assert isinstance(recipients, list), recipients
+    if headers is None:
+        headers = {}
+    else:
+        # do not modify the original headers object passed by the caller
+        headers = headers.copy()
 
     email_config = config
     email_prefix = email_config.get('email_prefix', '')
@@ -280,7 +288,16 @@ def send_email(recipients, subject, body
 
         log.warning("No recipients specified for '%s' - sending to admins %s", subject, ' '.join(recipients))
 
-    mail_from = email_config.get('app_email_from', 'Kallithea')
+    # SMTP sender
+    envelope_from = email_config.get('app_email_from', 'Kallithea')
+    # 'From' header
+    if author is not None:
+        # set From header based on author but with a generic e-mail address
+        # In case app_email_from is in "Some Name <e-mail>" format, we first
+        # extract the e-mail address.
+        envelope_addr = author_email(envelope_from)
+        headers['From'] = '%s (no-reply) <%s>' % (author.full_name_or_username, envelope_addr)
+
     user = email_config.get('smtp_username')
     passwd = email_config.get('smtp_password')
     mail_server = email_config.get('smtp_server')
@@ -306,7 +323,7 @@ def send_email(recipients, subject, body
         return False
 
     try:
-        m = SmtpMailer(mail_from, user, passwd, mail_server, smtp_auth,
+        m = SmtpMailer(envelope_from, user, passwd, mail_server, smtp_auth,
                        mail_port, ssl, tls, debug=debug)
         m.send(recipients, subject, body, html_body, headers=headers)
     except:
diff --git a/kallithea/model/notification.py b/kallithea/model/notification.py
--- a/kallithea/model/notification.py
+++ b/kallithea/model/notification.py
@@ -145,7 +145,7 @@ class NotificationModel(BaseModel):
                                 .get_email_tmpl(type_, 'html', **html_kwargs)
 
             run_task(tasks.send_email, [rec.email], email_subject, email_txt_body,
-                     email_html_body, headers)
+                     email_html_body, headers, author=created_by_obj)
 
         return notif
 
diff --git a/kallithea/tests/other/test_mail.py b/kallithea/tests/other/test_mail.py
--- a/kallithea/tests/other/test_mail.py
+++ b/kallithea/tests/other/test_mail.py
@@ -2,6 +2,7 @@ import mock
 
 import kallithea
 from kallithea.tests import *
+from kallithea.model.db import User
 
 class smtplib_mock(object):
 
@@ -89,3 +90,79 @@ class TestMail(BaseTestCase):
         self.assertIn('Subject: %s' % subject, smtplib_mock.lastmsg)
         self.assertIn(body, smtplib_mock.lastmsg)
         self.assertIn(html_body, smtplib_mock.lastmsg)
+
+    def test_send_mail_with_author(self):
+        mailserver = 'smtp.mailserver.org'
+        recipients = ['rcpt1', 'rcpt2']
+        envelope_from = 'noreply at mailserver.org'
+        subject = 'subject'
+        body = 'body'
+        html_body = 'html_body'
+        author = User.get_by_username(TEST_USER_REGULAR_LOGIN)
+
+        config_mock = {
+            'smtp_server': mailserver,
+            'app_email_from': envelope_from,
+        }
+        with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock):
+            kallithea.lib.celerylib.tasks.send_email(recipients, subject, body, html_body, author=author)
+
+        self.assertSetEqual(smtplib_mock.lastdest, set(recipients))
+        self.assertEqual(smtplib_mock.lastsender, envelope_from)
+        self.assertIn('From: Kallithea Admin (no-reply) <%s>' % envelope_from, smtplib_mock.lastmsg)
+        self.assertIn('Subject: %s' % subject, smtplib_mock.lastmsg)
+        self.assertIn(body, smtplib_mock.lastmsg)
+        self.assertIn(html_body, smtplib_mock.lastmsg)
+
+    def test_send_mail_with_author_full_mail_from(self):
+        mailserver = 'smtp.mailserver.org'
+        recipients = ['rcpt1', 'rcpt2']
+        envelope_addr = 'noreply at mailserver.org'
+        envelope_from = 'Some Name <%s>' % envelope_addr
+        subject = 'subject'
+        body = 'body'
+        html_body = 'html_body'
+        author = User.get_by_username(TEST_USER_REGULAR_LOGIN)
+
+        config_mock = {
+            'smtp_server': mailserver,
+            'app_email_from': envelope_from,
+        }
+        with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock):
+            kallithea.lib.celerylib.tasks.send_email(recipients, subject, body, html_body, author=author)
+
+        self.assertSetEqual(smtplib_mock.lastdest, set(recipients))
+        self.assertEqual(smtplib_mock.lastsender, envelope_from)
+        self.assertIn('From: Kallithea Admin (no-reply) <%s>' % envelope_addr, smtplib_mock.lastmsg)
+        self.assertIn('Subject: %s' % subject, smtplib_mock.lastmsg)
+        self.assertIn(body, smtplib_mock.lastmsg)
+        self.assertIn(html_body, smtplib_mock.lastmsg)
+
+    def test_send_mail_extra_headers(self):
+        mailserver = 'smtp.mailserver.org'
+        recipients = ['rcpt1', 'rcpt2']
+        envelope_from = 'noreply at mailserver.org'
+        subject = 'subject'
+        body = 'body'
+        html_body = 'html_body'
+        author = User.get_by_username(TEST_USER_REGULAR_LOGIN)
+        headers = {'extra': 'yes'}
+
+        config_mock = {
+            'smtp_server': mailserver,
+            'app_email_from': envelope_from,
+        }
+        with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock):
+            kallithea.lib.celerylib.tasks.send_email(recipients, subject, body, html_body,
+                                                     author=author, headers=headers)
+
+        self.assertSetEqual(smtplib_mock.lastdest, set(recipients))
+        self.assertEqual(smtplib_mock.lastsender, envelope_from)
+        self.assertIn('From: Kallithea Admin (no-reply) <%s>' % envelope_from, smtplib_mock.lastmsg)
+        self.assertIn('Subject: %s' % subject, smtplib_mock.lastmsg)
+        self.assertIn(body, smtplib_mock.lastmsg)
+        self.assertIn(html_body, smtplib_mock.lastmsg)
+        self.assertIn('Extra: yes', smtplib_mock.lastmsg)
+        # verify that headers dict hasn't mutated by send_email
+        self.assertDictEqual(headers, {'extra': 'yes'})
+


More information about the kallithea-general mailing list