Mercurial > kallithea
changeset 8276:fdde16d7cea0
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