[PATCH 2 of 5] e-mail: send comment and pullrequest mails with the author's name in From

[Thomas De Schampheleire]

# HG changeset patch
# User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
# Date 1436471455 -7200
#      Thu Jul 09 21:50:55 2015 +0200
# Node ID 95c18c5fdb59f2d985e2cd8ac6db093cb3e5f884
# Parent  c22a219636b830bac4b6125b306f6c8506f81d99
e-mail: send comment and pullrequest mails with the author's name in From

When e-mails are sent for comments and pullrequest invitations, set the From
header to:
    Author's Name <generic e-mail address>

The sender used for other e-mail 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/kallithea/lib/celerylib/tasks.py b/kallithea/lib/celerylib/tasks.py
--- a/kallithea/lib/celerylib/tasks.py
+++ b/kallithea/lib/celerylib/tasks.py
@@ -247,7 +247,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.
 
@@ -259,6 +259,8 @@ def send_email(recipients, subject, body
     """
     log = get_logger(send_email)
     assert isinstance(recipients, list), recipients
+    if headers is None:
+        headers = {}
 
     email_config = config
     email_prefix = email_config.get('email_prefix', '')
@@ -274,7 +276,23 @@ def send_email(recipients, subject, body
         log.error("No recipients specified")
         return False
 
-    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 None:
+        headers['From'] = envelope_from
+    else:
+        # 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.
+        import re
+        m = re.match('.*<(.*)>', envelope_from)
+        if m is not None:
+            envelope_addr = m.group(1)
+        else:
+            envelope_addr = envelope_from
+        headers['From'] = '%s <%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')
@@ -285,13 +303,18 @@ def send_email(recipients, subject, body
     smtp_auth = email_config.get('smtp_auth')
 
     if not mail_server:
-        log.error("SMTP mail server not configured - cannot send mail '%s' to %s", subject, ' '.join(recipients))
-        log.warning("body:\n%s", body)
-        log.warning("html:\n%s", html_body)
+        log.error("SMTP mail server not configured - cannot send mail to %s", ' '.join(recipients))
+        log.warning("Mail details:\n"
+                    "envelope_from: %s\n"
+                    "headers: %s\n"
+                    "subject: %s\n"
+                    "body:\n%s\n"
+                    "html:\n%s\n"
+                    % (envelope_from, headers, subject, body, html_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
new file mode 100644
--- /dev/null
+++ b/kallithea/tests/other/test_mail.py
@@ -0,0 +1,94 @@
+import mock
+
+from kallithea.tests import *
+from kallithea.model.db import User
+import kallithea.lib.celerylib.tasks as t
+
+class smtplib_mock(object):
+
+    @classmethod
+    def SMTP(cls, server, port, local_hostname):
+        return smtplib_mock()
+        print 'mocked'
+
+    def ehlo(self):
+        pass
+    def quit(self):
+        pass
+    def sendmail(self, sender, dest, msg):
+        smtplib_mock.lastsender = sender
+        smtplib_mock.lastdest = dest
+        smtplib_mock.lastmsg = msg
+        pass
+
+ at mock.patch('kallithea.lib.rcmail.smtp_mailer.smtplib', smtplib_mock)
+class TestMail(BaseTestCase):
+
+    def test_send_mail_trivial(self):
+        mailserver = 'smtp.mailserver.org'
+        recipients = ['rcpt1', 'rcpt2']
+        envelope_from = 'noreply at mailserver.org'
+        subject = 'subject'
+        body = 'body'
+        html_body = 'html_body'
+
+        config_mock = {
+                'smtp_server': mailserver,
+                'app_email_from': envelope_from,
+        }
+        with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock):
+            t.send_email(recipients, subject, body, html_body)
+
+        self.assertSetEqual(smtplib_mock.lastdest, set(recipients))
+        self.assertEqual(smtplib_mock.lastsender, envelope_from)
+        self.assertIn('From: %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(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):
+            t.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 <%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):
+            t.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 <%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)