On 06/12/2015 09:09 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Cedric De Herdt <cedric.de_he...@alcatel-lucent.com>
# Date 1434135934 -7200
# Fri Jun 12 21:05:34 2015 +0200
# Node ID c9c5310da1771baed04e70fb45293b8d212e8bb9
# Parent 42feaacb78feecb2a07a3ca19db6cf7ace3fd4c1
notification: use Sender and From header to clarify comment and pull request
mails
Current e-mails are sent from the Kallithea-configured e-mail address. The
subject line then needs to refer to the user to be useful.
Instead, use the author of comments and pull requests as 'From', and make
the Kallithea-configured address the 'Sender' in accordance with RFC5322.
So this will not cause problems for domains using domain keys or other
anti spam "authenticated sender" schemes?
Additionally, add the changeset/pullrequest description in the subject.
Separate patch, please ... especially because these two aspects are
unrelated and have different concerns and review comments.
[Thomas De Schampheleire:
- extend commit message
- update after threading requirements described in commit 8d45a14d3191
]
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.
@@ -274,7 +274,17 @@ def send_email(recipients, subject, body
log.error("No recipients specified")
return False
- mail_from = email_config.get('app_email_from', 'Kallithea')
+ mail_sender = email_config.get('app_email_from', 'Kallithea')
+ if author:
+ # indicates that there is a real author which should be in the 'From'
field
+ # the agent will be put in 'Sender' field: see
http://tools.ietf.org/html/rfc5322#section-3.6.2
+ mail_from = author
+ try:
+ headers['Sender'] = mail_sender
+ except TypeError:
+ headers = {'Sender': mail_sender}
That is for the case where headers is None? Then please detect that
explicitly with "headers is None" or "not headers".
Or (assuming we have the default headers=None to avoid having mutable
objects as default values), have an early "it headers is None:
headers={}" in the function.
Anyway, how about a first patch that refactors and consistently renames
"mail_from" to "envelope_from" (which seems to be the right technical
term for what it is), and then another patch for setting "header_from"
(or reuse mail_from).
+ else:
+ mail_from = mail_sender
user = email_config.get('smtp_username')
passwd = email_config.get('smtp_password')
mail_server = email_config.get('smtp_server')
diff --git a/kallithea/model/comment.py b/kallithea/model/comment.py
--- a/kallithea/model/comment.py
+++ b/kallithea/model/comment.py
@@ -105,6 +105,7 @@ class ChangesetCommentsModel(BaseModel):
'cs_comment_url': comment_url,
'raw_id': revision,
'message': cs.message,
+ 'message_firstline': cs.message.splitlines()[0],
'repo_name': repo.repo_name,
'short_id': h.short_id(revision),
'branch': cs.branch,
diff --git a/kallithea/model/notification.py b/kallithea/model/notification.py
--- a/kallithea/model/notification.py
+++ b/kallithea/model/notification.py
@@ -117,6 +117,7 @@ class NotificationModel(BaseModel):
headers = None
if 'threading' in email_kwargs:
headers = {'References': ' '.join('<%s>' % x for x in
email_kwargs['threading'])}
+ author = created_by_obj.full_contact
# send email with notification to all other participants
for rec in rec_objs:
@@ -145,7 +146,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)
return notif
@@ -299,13 +300,13 @@ class EmailNotificationModel(BaseModel):
self.TYPE_PULL_REQUEST_COMMENT: 'pull_request_comment',
}
self._subj_map = {
- self.TYPE_CHANGESET_COMMENT: _('[Comment from
%(comment_username)s] %(repo_name)s changeset %(short_id)s on %(branch)s'),
+ self.TYPE_CHANGESET_COMMENT: _('[Comment] %(repo_name)s changeset
%(short_id)s: "%(message_firstline)s" on %(branch)s'),
Isn't it very useful to have in the subject _who_ the comment was from?
Anyway, that seems unrelated to adding the first line of the commit
message. Also, I guess it would make sense to restrict the commit
message to something like the first 50 characters.
And: wouldn't it be more useful to have the first line of the actual
comment in the mail subject instead of the first line of the PR description?
(Somewhat related: I have had a request to put the branch name "earlier"
in the subject line. This will go in the opposite direction. One thing
that could help could be to hide the branch name if it is default (hg)
or master (git).)
self.TYPE_MESSAGE: 'Test Message',
# self.TYPE_PASSWORD_RESET
self.TYPE_REGISTRATION: _('New user %(new_username)s registered'),
# self.TYPE_DEFAULT
- self.TYPE_PULL_REQUEST: _('[Added by %(pr_username)s]
%(repo_name)s pull request %(pr_nice_id)s from %(ref)s'),
- self.TYPE_PULL_REQUEST_COMMENT: _('[Comment from
%(comment_username)s] %(repo_name)s pull request %(pr_nice_id)s from %(ref)s'),
+ self.TYPE_PULL_REQUEST: _('[Added] %(repo_name)s pull request
%(pr_nice_id)s: "%(pr_title)s" from %(ref)s'),
+ self.TYPE_PULL_REQUEST_COMMENT: _('[Comment] %(repo_name)s pull request
%(pr_nice_id)s: "%(pr_title)s" from %(ref)s'),
}
def get_email_description(self, type_, **kwargs):
/Mads
_______________________________________________
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
