[PATCH 3 of 8 v2] notification: allow comments in comment templates

Jan Heylen heyleke at gmail.com
Sat May 16 17:09:31 EDT 2015


On May 16, 2015 10:31 PM, "Thomas De Schampheleire" <
patrickdepinguin at gmail.com> wrote:
>
> On Sat, May 16, 2015 at 2:02 PM, Jan Heylen <heyleke at gmail.com> wrote:
> > # HG changeset patch
> > # User Jan Heylen <heyleke at gmail.com>
> > # Date 1431012349 -7200
> > #      Thu May 07 17:25:49 2015 +0200
> > # Node ID a761b2b50afdf387951bef282954a86650b6ee70
> > # Parent  dc6bb39a25a624034be795d083a118009f903d31
> > notification: allow comments in comment templates
> >
> > + create testcase for notifications with comments
> >
> > diff -r dc6bb39a25a6 -r a761b2b50afd kallithea/model/notification.py
> > --- a/kallithea/model/notification.py   Tue May 05 10:35:21 2015 +0200
> > +++ b/kallithea/model/notification.py   Thu May 07 17:25:49 2015 +0200
> > @@ -57,9 +57,9 @@
> >                  raise Exception('notification must be int, long or
Instance'
> >                                  ' of Notification got %s' %
type(notification))
> >
> > -    def create(self, created_by, subject, body, recipients=None,
> > +    def create(self, created_by, subject, recipients=None,
> >                 type_=Notification.TYPE_MESSAGE, with_email=True,
> > -               email_kwargs={}):
> > +               email_kwargs={}, **kwargs):
> >          """
> >
> >          Creates notification of given type
> > @@ -103,10 +103,23 @@
> >          #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 'body' not in kwargs:
> > +            body = ''
> > +        else:
> > +            body = kwargs['body']
> > +            notif = Notification.create(
> > +                created_by=created_by_obj, subject=subject,
> > +                body=body, recipients=recipients_objs, type_=type_)
>
> The case where body is not in kwargs, can it happen currently?
> In case it is, no notification would be created. Wouldn't it be more
> logical to move the Notification.create outside the else statement, so
> that is also done when body was not given (similar to how a
> notification is currently created even when body was None) ?

It actually was outside the else, but the comments tests were failing on
this, so I reason that norltifications with empty bodies are useless
anyway...
>
> > +
> > +        comments = []
> > +        comments_html = []
> > +        if 'bodies' in kwargs:
> > +            comments = kwargs['bodies']
> > +            for comment in comments:
> > +                notif = Notification.create(
> > +                    created_by=created_by_obj, subject=subject,
> > +                    body=comment, recipients=recipients_objs,
type_=type_)
> > +                comments_html.append(h.rst_w_mentions(comment))
> >
> >          if not with_email:
> >              return notif
> > @@ -123,13 +136,15 @@
> >              ## this is passed into template
> >              html_kwargs = {
> >                        'subject': subject,
> > -                      'body': h.rst_w_mentions(body),
> > +                      'comments': comments_html,
> > +                      'body': body,
>
> Here the rst_w_mentions wrap around body is removed, why is that?
I might have reintroduced an old issue again, I'll have a look at this...

>
> >                        'when': h.fmt_date(notif.created_on),
> >                        'user': notif.created_by_user.username,
> >                        }
> >
> >              txt_kwargs = {
> >                        'subject': subject,
> > +                      'comments': comments,
> >                        'body': body,
> >                        'when': h.fmt_date(notif.created_on),
> >                        'user': notif.created_by_user.username,
> > diff -r dc6bb39a25a6 -r a761b2b50afd
kallithea/templates/email_templates/changeset_comment.html
> > --- a/kallithea/templates/email_templates/changeset_comment.html
Tue May 05 10:35:21 2015 +0200
> > +++ b/kallithea/templates/email_templates/changeset_comment.html
Thu May 07 17:25:49 2015 +0200
> > @@ -7,7 +7,9 @@
> >  <p>${_('Comment from %s on %s changeset %s') % (cs_comment_user,
cs_target_repo, h.short_id(raw_id))}:</p>
> >  %endif
> >  <p>${body}</p>
> > -
> > +%for comment in comments:
> > +<p>${comment}</p>
> > +%endfor
> >  %if status_change:
> >      <p>${_('The changeset status was changed to')}:
<b>${status_change}</b></p>
> >  %endif
> > diff -r dc6bb39a25a6 -r a761b2b50afd
kallithea/templates/email_templates/changeset_comment.txt
> > --- a/kallithea/templates/email_templates/changeset_comment.txt Tue May
05 10:35:21 2015 +0200
> > +++ b/kallithea/templates/email_templates/changeset_comment.txt Thu May
07 17:25:49 2015 +0200
> > @@ -7,6 +7,9 @@
> >  ${_('Comment from %s on %s changeset %s') % (cs_comment_user,
cs_target_repo, h.short_id(raw_id))|n,unicode}:
> >  %endif
> >  ${body|n,unicode}
> > +%for comment in comments:
> > +${comment|n,unicode}
> > +%endfor
> >
> >  %if status_change:
> >  ${_('The changeset status was changed to')|n,unicode}:
${status_change|n,unicode}
> > diff -r dc6bb39a25a6 -r a761b2b50afd
kallithea/templates/email_templates/pull_request_comment.html
> > --- a/kallithea/templates/email_templates/pull_request_comment.html
 Tue May 05 10:35:21 2015 +0200
> > +++ b/kallithea/templates/email_templates/pull_request_comment.html
 Thu May 07 17:25:49 2015 +0200
> > @@ -3,6 +3,9 @@
> >
> >  <p>${_('Comment from %s on %s pull request "%s"') % (pr_comment_user,
repo_name, pr_title)}:</p>
> >  <p>${body}</p>
> > +%for comment in comments:
> > +<p>${comment}</p>
> > +%endfor
> >
> >  %if status_change:
> >      %if closing_pr:
> > diff -r dc6bb39a25a6 -r a761b2b50afd
kallithea/templates/email_templates/pull_request_comment.txt
> > --- a/kallithea/templates/email_templates/pull_request_comment.txt
Tue May 05 10:35:21 2015 +0200
> > +++ b/kallithea/templates/email_templates/pull_request_comment.txt
Thu May 07 17:25:49 2015 +0200
> > @@ -3,6 +3,9 @@
> >
> >  ${_('Comment from %s on %s pull request "%s"') % (pr_comment_user,
repo_name, pr_title)|n,unicode}:
> >  ${body|n,unicode}
> > +%for comment in comments:
> > +${comment|n,unicode}
> > +%endfor
> >
> >  %if status_change:
> >      %if closing_pr:
> > diff -r dc6bb39a25a6 -r a761b2b50afd
kallithea/tests/models/test_notifications.py
> > --- a/kallithea/tests/models/test_notifications.py      Tue May 05
10:35:21 2015 +0200
> > +++ b/kallithea/tests/models/test_notifications.py      Thu May 07
17:25:49 2015 +0200
> > @@ -70,6 +70,32 @@
> >          self.assertEqual(set([x.user.user_id for x in unotification]),
> >                           set(usrs))
> >
> > +    def test_create_notification_with_comments(self):
> > +        self.assertEqual([], Notification.query().all())
> > +        self.assertEqual([], UserNotification.query().all())
> > +
> > +        usrs = [self.u1, self.u2]
> > +        notification = NotificationModel().create(created_by=self.u1,
> > +                                           subject=u'subj',
bodies=['1','2','3'],
> > +                                           recipients=usrs)
> > +        Session().commit()
> > +        u1 = User.get(self.u1)
> > +        u2 = User.get(self.u2)
> > +        u3 = User.get(self.u3)
> > +        notifications = Notification.query().all()
> > +        self.assertEqual(len(notifications), 3)
> > +
> > +        self.assertEqual(notifications[0].recipients, [u1, u2])
> > +        self.assertEqual(notification.notification_id,
> > +                         notifications[2].notification_id)
> > +
> > +        unotification = UserNotification.query()\
> > +            .filter(UserNotification.notification ==
notification).all()
> > +
> > +        self.assertEqual(len(unotification), len(usrs))
> > +        self.assertEqual(set([x.user.user_id for x in unotification]),
> > +                         set(usrs))
> > +
> >      def test_user_notifications(self):
> >          self.assertEqual([], Notification.query().all())
> >          self.assertEqual([], UserNotification.query().all())
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sfconservancy.org/pipermail/kallithea-general/attachments/20150516/817e0467/attachment-0001.html>


More information about the kallithea-general mailing list