Mercurial > kallithea

changeset 8276:fdde16d7cea0

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
celery: fix send_email to work with JSON encoding (Issue #363) Long time ago, c935bcaf7086 introduced an optional User object parameter to the send_email task and used the computed full_name_or_username property. Due to the magic of pickle, that also worked when using Celery to run the task async. Now, Celery 4 changed the default encoding from Pickle to JSON, which we anticipated in e539db6cc0da. That broke send_email in some cases, for example when a user comments on another user's changeset. Fixed by passing the "From" name as string instead of passing the whole User object. Thanks to vyom for reporting.
author Mads Kiilerich <mads@kiilerich.com>
date Sat, 29 Feb 2020 16:06:30 +0100
parents 169539b32b90
children afe7599f1d8b
files kallithea/lib/celerylib/tasks.py kallithea/model/notification.py kallithea/tests/functional/test_login.py kallithea/tests/models/test_dump_html_mails.ref.html kallithea/tests/models/test_notifications.py kallithea/tests/other/test_mail.py
diffstat 6 files changed, 34 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/lib/celerylib/tasks.py	Tue Feb 25 18:18:44 2020 +0100
+++ b/kallithea/lib/celerylib/tasks.py	Sat Feb 29 16:06:30 2020 +0100
@@ -233,7 +233,7 @@
 
 @celerylib.task
 @celerylib.dbsession
-def send_email(recipients, subject, body='', html_body='', headers=None, author=None):
+def send_email(recipients, subject, body='', html_body='', headers=None, from_name=None):
     """
     Sends an email with defined parameters from the .ini files.
 
@@ -243,7 +243,8 @@
     :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
+    :param from_name: full name to be used as sender of this mail - often a
+    .full_name_or_username value
     """
     assert isinstance(recipients, list), recipients
     if headers is None:
@@ -275,13 +276,13 @@
     # 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
+    if from_name is not None:
+        # set From header based on from_name 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" <%s>' % (
-            email.utils.quote('%s (no-reply)' % author.full_name_or_username),
+            email.utils.quote('%s (no-reply)' % from_name),
             envelope_addr)
 
     user = email_config.get('smtp_username')
--- a/kallithea/model/notification.py	Tue Feb 25 18:18:44 2020 +0100
+++ b/kallithea/model/notification.py	Sat Feb 29 16:06:30 2020 +0100
@@ -132,7 +132,8 @@
         # send email with notification to all other participants
         for rec in rec_objs:
             tasks.send_email([rec.email], email_subject, email_txt_body,
-                     email_html_body, headers, author=created_by_obj)
+                     email_html_body, headers,
+                     from_name=created_by_obj.full_name_or_username)
 
 
 class EmailNotificationModel(object):
--- a/kallithea/tests/functional/test_login.py	Tue Feb 25 18:18:44 2020 +0100
+++ b/kallithea/tests/functional/test_login.py	Sat Feb 29 16:06:30 2020 +0100
@@ -410,7 +410,7 @@
             User.get_by_username(username), timestamp, self.session_csrf_secret_token())
 
         collected = []
-        def mock_send_email(recipients, subject, body='', html_body='', headers=None, author=None):
+        def mock_send_email(recipients, subject, body='', html_body='', headers=None, from_name=None):
             collected.append((recipients, subject, body, html_body))
 
         with mock.patch.object(kallithea.lib.celerylib.tasks, 'send_email', mock_send_email), \
--- a/kallithea/tests/models/test_dump_html_mails.ref.html	Tue Feb 25 18:18:44 2020 +0100
+++ b/kallithea/tests/models/test_dump_html_mails.ref.html	Sat Feb 29 16:06:30 2020 +0100
@@ -5,7 +5,7 @@
 <hr/>
 <h1>cs_comment, is_mention=False, status_change=None</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Comment] repo/name changeset cafe1234 "This changeset did something cl..." on brunch
 </pre>
@@ -164,7 +164,7 @@
 <hr/>
 <h1>cs_comment, is_mention=True, status_change=None</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Comment] repo/name changeset cafe1234 "This changeset did something cl..." on brunch
 </pre>
@@ -323,7 +323,7 @@
 <hr/>
 <h1>cs_comment, is_mention=False, status_change='Approved'</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Approved: Comment] repo/name changeset cafe1234 "This changeset did something cl..." on brunch
 </pre>
@@ -500,7 +500,7 @@
 <hr/>
 <h1>cs_comment, is_mention=True, status_change='Approved'</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Approved: Comment] repo/name changeset cafe1234 "This changeset did something cl..." on brunch
 </pre>
@@ -677,7 +677,7 @@
 <hr/>
 <h1>message</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: Test Message
 </pre>
@@ -748,7 +748,7 @@
 <hr/>
 <h1>registration</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: New user newbie registered
 </pre>
@@ -881,7 +881,7 @@
 <hr/>
 <h1>pull_request, is_mention=False</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Review] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -1072,7 +1072,7 @@
 <hr/>
 <h1>pull_request, is_mention=True</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Review] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -1263,7 +1263,7 @@
 <hr/>
 <h1>pull_request_comment, is_mention=False, status_change=None, closing_pr=False</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Comment] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -1430,7 +1430,7 @@
 <hr/>
 <h1>pull_request_comment, is_mention=True, status_change=None, closing_pr=False</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Comment] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -1597,7 +1597,7 @@
 <hr/>
 <h1>pull_request_comment, is_mention=False, status_change='Under Review', closing_pr=False</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Under Review: Comment] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -1782,7 +1782,7 @@
 <hr/>
 <h1>pull_request_comment, is_mention=True, status_change='Under Review', closing_pr=False</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Under Review: Comment] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -1967,7 +1967,7 @@
 <hr/>
 <h1>pull_request_comment, is_mention=False, status_change=None, closing_pr=True</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Closing: Comment] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -2151,7 +2151,7 @@
 <hr/>
 <h1>pull_request_comment, is_mention=True, status_change=None, closing_pr=True</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Closing: Comment] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -2335,7 +2335,7 @@
 <hr/>
 <h1>pull_request_comment, is_mention=False, status_change='Under Review', closing_pr=True</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Under Review, Closing: Comment] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -2525,7 +2525,7 @@
 <hr/>
 <h1>pull_request_comment, is_mention=True, status_change='Under Review', closing_pr=True</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: u2@example.com
 Subject: [Under Review, Closing: Comment] repo/name PR #7 "The Title" from devbranch by u2
 </pre>
@@ -2715,7 +2715,7 @@
 <hr/>
 <h1>TYPE_PASSWORD_RESET</h1>
 <pre>
-From: u1
+From: u1 u1 <name@example.com>
 To: john@doe.com
 Subject: Password reset link
 </pre>
--- a/kallithea/tests/models/test_notifications.py	Tue Feb 25 18:18:44 2020 +0100
+++ b/kallithea/tests/models/test_notifications.py	Sat Feb 29 16:06:30 2020 +0100
@@ -43,12 +43,12 @@
         with test_context(self.app):
             usrs = [self.u1, self.u2]
 
-            def send_email(recipients, subject, body='', html_body='', headers=None, author=None):
+            def send_email(recipients, subject, body='', html_body='', headers=None, from_name=None):
                 assert recipients == ['u2@example.com']
                 assert subject == 'Test Message'
                 assert body == "hi there"
                 assert '>hi there<' in html_body
-                assert author.username == 'u1'
+                assert from_name == 'u1 u1'
             with mock.patch.object(kallithea.lib.celerylib.tasks, 'send_email', send_email):
                 NotificationModel().create(created_by=self.u1,
                                                    subject='subj', body='hi there',
@@ -59,11 +59,11 @@
         # Exercise all notification types and dump them to one big html file
         l = []
 
-        def send_email(recipients, subject, body='', html_body='', headers=None, author=None):
+        def send_email(recipients, subject, body='', html_body='', headers=None, from_name=None):
             l.append('<hr/>\n')
             l.append('<h1>%s</h1>\n' % desc) # desc is from outer scope
             l.append('<pre>\n')
-            l.append('From: %s\n' % author.username)
+            l.append('From: %s <name@example.com>\n' % from_name)
             l.append('To: %s\n' % ' '.join(recipients))
             l.append('Subject: %s\n' % subject)
             l.append('</pre>\n')
@@ -159,7 +159,7 @@
                     "Password reset link",
                     EmailNotificationModel().get_email_tmpl(EmailNotificationModel.TYPE_PASSWORD_RESET, 'txt', **kwargs),
                     EmailNotificationModel().get_email_tmpl(EmailNotificationModel.TYPE_PASSWORD_RESET, 'html', **kwargs),
-                    author=User.get(self.u1))
+                    from_name=User.get(self.u1).full_name_or_username)
 
         out = '<!doctype html>\n<html lang="en">\n<head><title>Notifications</title><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"></head>\n<body>\n%s\n</body>\n</html>\n' % \
             re.sub(r'<(/?(?:!doctype|html|head|title|meta|body)\b[^>]*)>', r'<!--\1-->', ''.join(l))
--- a/kallithea/tests/other/test_mail.py	Tue Feb 25 18:18:44 2020 +0100
+++ b/kallithea/tests/other/test_mail.py	Sat Feb 29 16:06:30 2020 +0100
@@ -135,7 +135,7 @@
             '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)
+            kallithea.lib.celerylib.tasks.send_email(recipients, subject, body, html_body, from_name=author.full_name_or_username)
 
         assert smtplib_mock.lastdest == set(recipients)
         assert smtplib_mock.lastsender == envelope_from
@@ -159,7 +159,7 @@
             '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)
+            kallithea.lib.celerylib.tasks.send_email(recipients, subject, body, html_body, from_name=author.full_name_or_username)
 
         assert smtplib_mock.lastdest == set(recipients)
         assert smtplib_mock.lastsender == envelope_from
@@ -184,7 +184,7 @@
         }
         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)
+                                                     from_name=author.full_name_or_username, headers=headers)
 
         assert smtplib_mock.lastdest == set(recipients)
         assert smtplib_mock.lastsender == envelope_from