[PATCH 2 of 7] notification: allow multiple body parts in templates

[Mads Kiilerich]

On 04/14/2015 06:03 PM, Jan Heylen wrote:
> # HG changeset patch
> # User Jan Heylen <heyleke at gmail.com>
> # Date 1427469226 -3600
> #      Fri Mar 27 16:13:46 2015 +0100
> # Node ID 55ba8e68269da8fb0c33c65711a57b4fd9e1b675
> # Parent  cb41b8211291b55bcd7c77919f714791927b8045
> notification: allow multiple body parts in templates
>
> diff -r cb41b8211291 -r 55ba8e68269d kallithea/model/notification.py
> --- a/kallithea/model/notification.py	Fri Mar 27 13:58:04 2015 +0100
> +++ b/kallithea/model/notification.py	Fri Mar 27 16:13:46 2015 +0100
> @@ -59,7 +59,7 @@
>   
>       def create(self, created_by, subject, body, recipients=None,
>                  type_=Notification.TYPE_MESSAGE, with_email=True,
> -               email_kwargs={}):
> +               email_kwargs={}, bodies=None):

I think it would be less ugly to allow body to be either a single 
instance or a list, perhaps depending on the type, perhaps with the 
single instance as a list with one element.

>           """
>   
>           Creates notification of given type
> @@ -103,10 +103,16 @@
>           #else: silently skip notification mails?
>   
>           # TODO: inform user who are notified
> -        notif = Notification.create(
> -            created_by=created_by_obj, subject=subject,
> -            body=body, recipients=recipients_objs, type_=type_
> -        )
> +        if not bodies:
> +            bodies = [body]
> +
> +        bodies_html = []
> +        for body in bodies:
> +            notif = Notification.create(
> +                created_by=created_by_obj, subject=subject,
> +                body=body, recipients=recipients_objs, type_=type_
> +            )
> +            bodies_html.append(h.rst_w_mentions(body))
>   
>           if not with_email:
>               return notif
> @@ -123,14 +129,14 @@
>               ## this is passed into template
>               html_kwargs = {
>                         'subject': subject,
> -                      'body': h.rst_w_mentions(body),
> +                      'bodies': bodies_html,
>                         'when': h.fmt_date(notif.created_on),
>                         'user': notif.created_by_user.username,
>                         }
>   
>               txt_kwargs = {
>                         'subject': subject,
> -                      'body': body,
> +                      'bodies': bodies,
>                         'when': h.fmt_date(notif.created_on),
>                         'user': notif.created_by_user.username,
>                         }
> diff -r cb41b8211291 -r 55ba8e68269d kallithea/templates/email_templates/changeset_comment.html
> --- a/kallithea/templates/email_templates/changeset_comment.html	Fri Mar 27 13:58:04 2015 +0100
> +++ b/kallithea/templates/email_templates/changeset_comment.html	Fri Mar 27 16:13:46 2015 +0100
> @@ -6,8 +6,9 @@
>   %else:
>   <p>${_('Comment from %s on %s changeset %s') % (cs_comment_user, cs_target_repo, h.short_id(raw_id))}:</p>
>   %endif
> +%for body in bodies:
>   <p>${body}</p>
> -
> +%endfor

I would like to let the mails include filename and linenumber (and 
ultimately also some lines of context). It seems like this change will 
make it harder to add that. Could it be done first?

And more important: There should be a cs_comment_url link for each comment.

>   %if status_change:
>       <p>${_('The changeset status was changed to')}: <b>${status_change}</b></p>
>   %endif
> diff -r cb41b8211291 -r 55ba8e68269d kallithea/templates/email_templates/changeset_comment.txt
> --- a/kallithea/templates/email_templates/changeset_comment.txt	Fri Mar 27 13:58:04 2015 +0100
> +++ b/kallithea/templates/email_templates/changeset_comment.txt	Fri Mar 27 16:13:46 2015 +0100
> @@ -6,7 +6,9 @@
>   %else:
>   ${_('Comment from %s on %s changeset %s') % (cs_comment_user, cs_target_repo, h.short_id(raw_id))|n,unicode}:
>   %endif
> +%for body in bodies:
>   ${body|n,unicode}
> +%endfor
>   
>   %if status_change:
>   ${_('The changeset status was changed to')|n,unicode}: ${status_change|n,unicode}
> diff -r cb41b8211291 -r 55ba8e68269d kallithea/templates/email_templates/default.html
> --- a/kallithea/templates/email_templates/default.html	Fri Mar 27 13:58:04 2015 +0100
> +++ b/kallithea/templates/email_templates/default.html	Fri Mar 27 16:13:46 2015 +0100
> @@ -1,4 +1,6 @@
>   ## -*- coding: utf-8 -*-
>   <%inherit file="main.html"/>
>   
> +%for body in bodies:
>   ${body}
> +%endfor

It doesn't make much sense to have multiple bodies if we only 
concatenate them. But ok, if it is simpler this way.

But I guess this really depends on how to solve the problem mentioned 
above: We do not just have a list of bodies; we have a list of entries 
that each have a file and line and url and body.

/Mads