<p dir="ltr"><br>
On May 16, 2015 10:31 PM, "Thomas De Schampheleire" <<a href="mailto:patrickdepinguin@gmail.com">patrickdepinguin@gmail.com</a>> wrote:<br>
><br>
> On Sat, May 16, 2015 at 2:02 PM, Jan Heylen <<a href="mailto:heyleke@gmail.com">heyleke@gmail.com</a>> wrote:<br>
> > # HG changeset patch<br>
> > # User Jan Heylen <<a href="mailto:heyleke@gmail.com">heyleke@gmail.com</a>><br>
> > # Date 1431012349 -7200<br>
> > # Thu May 07 17:25:49 2015 +0200<br>
> > # Node ID a761b2b50afdf387951bef282954a86650b6ee70<br>
> > # Parent dc6bb39a25a624034be795d083a118009f903d31<br>
> > notification: allow comments in comment templates<br>
> ><br>
> > + create testcase for notifications with comments<br>
> ><br>
> > diff -r dc6bb39a25a6 -r a761b2b50afd kallithea/model/notification.py<br>
> > --- a/kallithea/model/notification.py Tue May 05 10:35:21 2015 +0200<br>
> > +++ b/kallithea/model/notification.py Thu May 07 17:25:49 2015 +0200<br>
> > @@ -57,9 +57,9 @@<br>
> > raise Exception('notification must be int, long or Instance'<br>
> > ' of Notification got %s' % type(notification))<br>
> ><br>
> > - def create(self, created_by, subject, body, recipients=None,<br>
> > + def create(self, created_by, subject, recipients=None,<br>
> > type_=Notification.TYPE_MESSAGE, with_email=True,<br>
> > - email_kwargs={}):<br>
> > + email_kwargs={}, **kwargs):<br>
> > """<br>
> ><br>
> > Creates notification of given type<br>
> > @@ -103,10 +103,23 @@<br>
> > #else: silently skip notification mails?<br>
> ><br>
> > # TODO: inform user who are notified<br>
> > - notif = Notification.create(<br>
> > - created_by=created_by_obj, subject=subject,<br>
> > - body=body, recipients=recipients_objs, type_=type_<br>
> > - )<br>
> > + if 'body' not in kwargs:<br>
> > + body = ''<br>
> > + else:<br>
> > + body = kwargs['body']<br>
> > + notif = Notification.create(<br>
> > + created_by=created_by_obj, subject=subject,<br>
> > + body=body, recipients=recipients_objs, type_=type_)<br>
><br>
> The case where body is not in kwargs, can it happen currently?<br>
> In case it is, no notification would be created. Wouldn't it be more<br>
> logical to move the Notification.create outside the else statement, so<br>
> that is also done when body was not given (similar to how a<br>
> notification is currently created even when body was None) ?</p>
<p dir="ltr">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...<br>
><br>
> > +<br>
> > + comments = []<br>
> > + comments_html = []<br>
> > + if 'bodies' in kwargs:<br>
> > + comments = kwargs['bodies']<br>
> > + for comment in comments:<br>
> > + notif = Notification.create(<br>
> > + created_by=created_by_obj, subject=subject,<br>
> > + body=comment, recipients=recipients_objs, type_=type_)<br>
> > + comments_html.append(h.rst_w_mentions(comment))<br>
> ><br>
> > if not with_email:<br>
> > return notif<br>
> > @@ -123,13 +136,15 @@<br>
> > ## this is passed into template<br>
> > html_kwargs = {<br>
> > 'subject': subject,<br>
> > - 'body': h.rst_w_mentions(body),<br>
> > + 'comments': comments_html,<br>
> > + 'body': body,<br>
><br>
> Here the rst_w_mentions wrap around body is removed, why is that?<br>
I might have reintroduced an old issue again, I'll have a look at this...<br></p>
<p dir="ltr">><br>
> > 'when': h.fmt_date(notif.created_on),<br>
> > 'user': notif.created_by_user.username,<br>
> > }<br>
> ><br>
> > txt_kwargs = {<br>
> > 'subject': subject,<br>
> > + 'comments': comments,<br>
> > 'body': body,<br>
> > 'when': h.fmt_date(notif.created_on),<br>
> > 'user': notif.created_by_user.username,<br>
> > diff -r dc6bb39a25a6 -r a761b2b50afd kallithea/templates/email_templates/changeset_comment.html<br>
> > --- a/kallithea/templates/email_templates/changeset_comment.html Tue May 05 10:35:21 2015 +0200<br>
> > +++ b/kallithea/templates/email_templates/changeset_comment.html Thu May 07 17:25:49 2015 +0200<br>
> > @@ -7,7 +7,9 @@<br>
> > <p>${_('Comment from %s on %s changeset %s') % (cs_comment_user, cs_target_repo, h.short_id(raw_id))}:</p><br>
> > %endif<br>
> > <p>${body}</p><br>
> > -<br>
> > +%for comment in comments:<br>
> > +<p>${comment}</p><br>
> > +%endfor<br>
> > %if status_change:<br>
> > <p>${_('The changeset status was changed to')}: <b>${status_change}</b></p><br>
> > %endif<br>
> > diff -r dc6bb39a25a6 -r a761b2b50afd kallithea/templates/email_templates/changeset_comment.txt<br>
> > --- a/kallithea/templates/email_templates/changeset_comment.txt Tue May 05 10:35:21 2015 +0200<br>
> > +++ b/kallithea/templates/email_templates/changeset_comment.txt Thu May 07 17:25:49 2015 +0200<br>
> > @@ -7,6 +7,9 @@<br>
> > ${_('Comment from %s on %s changeset %s') % (cs_comment_user, cs_target_repo, h.short_id(raw_id))|n,unicode}:<br>
> > %endif<br>
> > ${body|n,unicode}<br>
> > +%for comment in comments:<br>
> > +${comment|n,unicode}<br>
> > +%endfor<br>
> ><br>
> > %if status_change:<br>
> > ${_('The changeset status was changed to')|n,unicode}: ${status_change|n,unicode}<br>
> > diff -r dc6bb39a25a6 -r a761b2b50afd kallithea/templates/email_templates/pull_request_comment.html<br>
> > --- a/kallithea/templates/email_templates/pull_request_comment.html Tue May 05 10:35:21 2015 +0200<br>
> > +++ b/kallithea/templates/email_templates/pull_request_comment.html Thu May 07 17:25:49 2015 +0200<br>
> > @@ -3,6 +3,9 @@<br>
> ><br>
> > <p>${_('Comment from %s on %s pull request "%s"') % (pr_comment_user, repo_name, pr_title)}:</p><br>
> > <p>${body}</p><br>
> > +%for comment in comments:<br>
> > +<p>${comment}</p><br>
> > +%endfor<br>
> ><br>
> > %if status_change:<br>
> > %if closing_pr:<br>
> > diff -r dc6bb39a25a6 -r a761b2b50afd kallithea/templates/email_templates/pull_request_comment.txt<br>
> > --- a/kallithea/templates/email_templates/pull_request_comment.txt Tue May 05 10:35:21 2015 +0200<br>
> > +++ b/kallithea/templates/email_templates/pull_request_comment.txt Thu May 07 17:25:49 2015 +0200<br>
> > @@ -3,6 +3,9 @@<br>
> ><br>
> > ${_('Comment from %s on %s pull request "%s"') % (pr_comment_user, repo_name, pr_title)|n,unicode}:<br>
> > ${body|n,unicode}<br>
> > +%for comment in comments:<br>
> > +${comment|n,unicode}<br>
> > +%endfor<br>
> ><br>
> > %if status_change:<br>
> > %if closing_pr:<br>
> > diff -r dc6bb39a25a6 -r a761b2b50afd kallithea/tests/models/test_notifications.py<br>
> > --- a/kallithea/tests/models/test_notifications.py Tue May 05 10:35:21 2015 +0200<br>
> > +++ b/kallithea/tests/models/test_notifications.py Thu May 07 17:25:49 2015 +0200<br>
> > @@ -70,6 +70,32 @@<br>
> > self.assertEqual(set([x.user.user_id for x in unotification]),<br>
> > set(usrs))<br>
> ><br>
> > + def test_create_notification_with_comments(self):<br>
> > + self.assertEqual([], Notification.query().all())<br>
> > + self.assertEqual([], UserNotification.query().all())<br>
> > +<br>
> > + usrs = [self.u1, self.u2]<br>
> > + notification = NotificationModel().create(created_by=self.u1,<br>
> > + subject=u'subj', bodies=['1','2','3'],<br>
> > + recipients=usrs)<br>
> > + Session().commit()<br>
> > + u1 = User.get(self.u1)<br>
> > + u2 = User.get(self.u2)<br>
> > + u3 = User.get(self.u3)<br>
> > + notifications = Notification.query().all()<br>
> > + self.assertEqual(len(notifications), 3)<br>
> > +<br>
> > + self.assertEqual(notifications[0].recipients, [u1, u2])<br>
> > + self.assertEqual(notification.notification_id,<br>
> > + notifications[2].notification_id)<br>
> > +<br>
> > + unotification = UserNotification.query()\<br>
> > + .filter(UserNotification.notification == notification).all()<br>
> > +<br>
> > + self.assertEqual(len(unotification), len(usrs))<br>
> > + self.assertEqual(set([x.user.user_id for x in unotification]),<br>
> > + set(usrs))<br>
> > +<br>
> > def test_user_notifications(self):<br>
> > self.assertEqual([], Notification.query().all())<br>
> > self.assertEqual([], UserNotification.query().all())<br>
</p>