[PATCH 3 of 8 v2] notification: allow comments in comment templates
Thomas De Schampheleire
patrickdepinguin at gmail.com
Sat May 16 16:30:45 EDT 2015
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) ?
> +
> + 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?
> '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())
More information about the kallithea-general
mailing list