diff --git a/reviewboard/admin/forms/email_settings.py b/reviewboard/admin/forms/email_settings.py index 9674739857a231e82c98c061519b712f0b588729..1b91d984e546b2831c2bc851882f30b8d5f99233 100644 --- a/reviewboard/admin/forms/email_settings.py +++ b/reviewboard/admin/forms/email_settings.py @@ -44,6 +44,10 @@ class EMailSettingsForm(SiteSettingsForm): mail_send_password_changed_mail = forms.BooleanField( label=_('Send e-mails to users when they change their password'), required=False) + mail_send_shipit_revocation_mail = forms.BooleanField( + label=_('Send e-mails when a review ship-it status is revoked'), + required=False + ) mail_enable_autogenerated_header = forms.BooleanField( label=_('Enable "Auto-Submitted: auto-generated" header'), help_text=_('Marks outgoing e-mails as "auto-generated" to avoid ' @@ -173,7 +177,8 @@ class EMailSettingsForm(SiteSettingsForm): 'fields': ('mail_send_review_mail', 'mail_send_review_close_mail', 'mail_send_new_user_mail', - 'mail_send_password_changed_mail'), + 'mail_send_password_changed_mail', + 'mail_send_shipit_revocation_mail'), }, { 'title': _('E-Mail Delivery Settings'), diff --git a/reviewboard/admin/siteconfig.py b/reviewboard/admin/siteconfig.py index 6e7b9fc39feaef3bade9f9486500617a7438d5ac..de043b4f556d0500769a48beec7ae20ee98d9b1a 100644 --- a/reviewboard/admin/siteconfig.py +++ b/reviewboard/admin/siteconfig.py @@ -156,6 +156,7 @@ defaults.update({ 'diffviewer_show_trailing_whitespace': True, 'mail_send_review_mail': False, 'mail_send_new_user_mail': False, + 'mail_send_shipit_revocation_mail': False, 'mail_send_password_changed_mail': False, 'mail_enable_autogenerated_header': True, 'mail_from_spoofing': EmailMessage.FROM_SPOOFING_SMART, diff --git a/reviewboard/admin/tests/test_email_settings_form.py b/reviewboard/admin/tests/test_email_settings_form.py index fd90e983f117fe264b4ae1d31b95ace70b5f105d..e36f90b4894f737762833dc28ad14c55ab849327 100644 --- a/reviewboard/admin/tests/test_email_settings_form.py +++ b/reviewboard/admin/tests/test_email_settings_form.py @@ -25,6 +25,7 @@ class EMailSettingsFormTestCase(kgb.SpyAgency, TestCase): siteconfig.set('mail_send_password_changed_mail', False) siteconfig.set('mail_send_review_close_mail', False) siteconfig.set('mail_send_review_mail', False) + siteconfig.set('mail_send_shipit_revocation_mail', False) siteconfig.set('mail_use_tls', False) siteconfig.set('mail_default_from', 'noreply@example.com') siteconfig.set('mail_from_spoofing', @@ -49,6 +50,7 @@ class EMailSettingsFormTestCase(kgb.SpyAgency, TestCase): 'mail_send_password_changed_mail': True, 'mail_send_review_close_mail': True, 'mail_send_review_mail': True, + 'mail_send_shipit_revocation_mail': True, 'mail_use_tls': True, }) @@ -66,6 +68,7 @@ class EMailSettingsFormTestCase(kgb.SpyAgency, TestCase): self.assertTrue(siteconfig.get('mail_send_password_changed_mail')) self.assertTrue(siteconfig.get('mail_send_review_close_mail')) self.assertTrue(siteconfig.get('mail_send_review_mail')) + self.assertTrue(siteconfig.get('mail_send_shipit_revocation_mail')) self.assertTrue(siteconfig.get('mail_use_tls')) self.assertEqual(siteconfig.get('mail_default_from'), 'no-reply@mail.example.com') diff --git a/reviewboard/extensions/hooks.py b/reviewboard/extensions/hooks.py index f1ce8f13c84bedd6f861bd1034aa58db8b09bfeb..48b2ca0f6df50e687588fdcc223e31595e6eef42 100644 --- a/reviewboard/extensions/hooks.py +++ b/reviewboard/extensions/hooks.py @@ -39,8 +39,10 @@ from reviewboard.reviews.fields import (get_review_request_fieldset, register_review_request_fieldset, unregister_review_request_fieldset) from reviewboard.reviews.signals import (review_request_published, - review_published, reply_published, - review_request_closed) + review_published, + reply_published, + review_request_closed, + review_ship_it_revoked) from reviewboard.reviews.ui.base import register_ui, unregister_ui from reviewboard.urls import (diffviewer_url_names, main_review_request_url_name) @@ -1780,6 +1782,69 @@ class ReviewRequestPublishedEmailHook(EmailHook): return cc_field +class ReviewShipItRevokedEmailHook(EmailHook): + """A hook for changing the recipients of review ship-it revoked + publishing e-mails. + + This hook must be subclassed. The caller is expected to override + :py:meth:`get_to_field` and/or :py:meth:`get_cc_field`. + """ + def initialize(self): + """Initialize the hook. """ + super(ReviewShipItRevokedEmailHook, self).initialize( + signals=[review_ship_it_revoked]) + + def get_to_field(self, to_field, review_request, user, **kwargs): + """Return the To field for the e-mail. + + Args: + to_field (set): + A set of :py:class:`Users ` + and :py:class:`Groups ` that + will receive the e-mail. + + review_request (reviewboard.reviews.models.ReviewRequest): + The review request that was published. + + user (django.contrib.auth.models.User): + The user who published the review request. + + **kwargs (dict): + Additional keyword arguments, since the signature may change in + the future. + + Returns: + set: + The desired To field. + """ + return to_field + + def get_cc_field(self, cc_field, review_request, user, **kwargs): + """Return the CC field for the e-mail. + + Args: + to_field (set): + A set of :py:class:`Users ` + and :py:class:`Groups ` that + will receive a carbon copy of the e-mail. + + review_request (reviewboard.reviews.models.ReviewRequest): + The review request that was published. + + user (django.contrib.auth.models.User): + The user who published the review request. + + **kwargs (dict): + Additional keyword arguments, since the signature may change in + the future. + + Returns: + set: + The desired CC field. + """ + return cc_field + + class APIExtraDataAccessHook(ExtensionHook, metaclass=ExtensionHookPoint): """A hook for setting access states to extra data fields. diff --git a/reviewboard/notifications/email/__init__.py b/reviewboard/notifications/email/__init__.py index 39ed4c64a68f0c59ce84df6654f40ae7f8dff3b5..c40432e5f164e4b6715b557d11683b6489633e4a 100644 --- a/reviewboard/notifications/email/__init__.py +++ b/reviewboard/notifications/email/__init__.py @@ -10,6 +10,7 @@ from reviewboard.notifications.email.signal_handlers import ( send_review_published_mail, send_review_request_closed_mail, send_review_request_published_mail, + send_review_ship_it_revoked_mail, send_user_registered_mail, send_webapi_token_created_mail, send_webapi_token_deleted_mail, @@ -19,7 +20,8 @@ from reviewboard.notifications.email.hooks import (register_email_hook, from reviewboard.reviews.models import ReviewRequest, Review from reviewboard.reviews.signals import (review_request_published, review_published, reply_published, - review_request_closed) + review_request_closed, + review_ship_it_revoked) from reviewboard.webapi.models import WebAPIToken from djblets.webapi.signals import webapi_token_created, webapi_token_updated @@ -32,6 +34,7 @@ def connect_signals(): ReviewRequest), (review_request_published, send_review_request_published_mail, ReviewRequest), + (review_ship_it_revoked, send_review_ship_it_revoked_mail, Review), (user_registered, send_user_registered_mail, None), (webapi_token_created, send_webapi_token_created_mail, WebAPIToken), (webapi_token_updated, send_webapi_token_updated_mail, WebAPIToken), diff --git a/reviewboard/notifications/email/message.py b/reviewboard/notifications/email/message.py index 477ba4ff5dd4da53410d87b604e6f0bb40dd43be..9a27a44bccd15bcc7fdab3515a7f28cd2a4603ec 100644 --- a/reviewboard/notifications/email/message.py +++ b/reviewboard/notifications/email/message.py @@ -510,6 +510,69 @@ def prepare_review_request_mail(user, review_request, changedesc=None, cc_field, 'notifications/review_request_email', extra_context) +def prepare_review_ship_it_revoked_mail(user, review, review_request): + """Return an e-mail notifying the user that the ship-it has been revoked. + + Args: + user (django.contrib.auth.models.User): + The user who triggered the e-mail (i.e., they revoked the Ship It). + + review (reviewboard.reviews.models.review.Review): + The review to send an e-mail about. + + review_request (reviewboard.reviews.models.ReviewRequest): + The review request that was reviewed. + + Returns: + EmailMessage: + The e-mail message showing the ship-it revocation + as well as details regarding the review. + """ + from reviewboard.reviews.views import build_diff_comment_fragments + + review.ordered_comments = review.comments.order_by('filediff', + 'first_line') + + to_field, cc_field = build_recipients(review.user, review_request, + limit_recipients_to=None) + + to_field, cc_field = filter_email_recipients_from_hooks( + to_field, cc_field, review_published, + user=user, + review=review, + to_owner_only=False, + review_request=review_request) + + summary = _ensure_unicode(review_request.summary) + subject = 'Review Request %d: %s' % (review_request.display_id, summary) + reply_message_id = review_request.email_message_id + + if reply_message_id: + # Fancy quoted "replies". + subject = 'Re: %s' % subject + + extra_context = { + 'user': review.user, + 'review': review, + 'has_issues': True, + 'request': None, + 'site_url': 'baseurl' + } + + extra_headers = {} + + extra_context['comment_entries'] = build_diff_comment_fragments( + review.ordered_comments, extra_context, + 'notifications/email_diff_comment_fragment.html')[1] + + return prepare_base_review_request_mail( + review.user, review_request, + subject, + reply_message_id, to_field, cc_field, + 'notifications/review_ship_it_revoked_email', + extra_context, + extra_headers=extra_headers) + def prepare_user_registered_mail(user): """Prepare an e-mail to the administrators notifying of a new user. diff --git a/reviewboard/notifications/email/signal_handlers.py b/reviewboard/notifications/email/signal_handlers.py index 0f1d5308cbe74632cf6e606198fdccf87619eed3..33ac8b0c50491f14687b6c170d8e9f2c83277e26 100644 --- a/reviewboard/notifications/email/signal_handlers.py +++ b/reviewboard/notifications/email/signal_handlers.py @@ -10,6 +10,7 @@ from reviewboard.notifications.email.message import ( prepare_reply_published_mail, prepare_review_published_mail, prepare_review_request_mail, + prepare_review_ship_it_revoked_mail, prepare_user_registered_mail, prepare_webapi_token_mail) from reviewboard.notifications.email.utils import send_email @@ -224,6 +225,43 @@ def send_review_request_published_mail(user, review_request, trivial, _update_email_info(review_request, message.message_id) +def send_review_ship_it_revoked_mail(user, review, **kwargs): + """Send e-mail when a review has it's 'Ship It' status revoked. + + Listens to the + :py:data:`~reviewboard.reviews.signals.review_ship_it_revoked` + signal and sends e-mail if this type of notification is enabled (through + the ``mail_send_shipit_revocation_mail`` site configuration setting). + + Args: + user (django.contrib.auth.models.User): + The user that published the review. + + review (reviewboard.reviews.models.review.Review): + The review that had it's 'Ship It' status revoked. + + **kwargs (dict): + Additional keyword arguments. + """ + siteconfig = SiteConfiguration.objects.get_current() + + if not siteconfig.get('mail_send_shipit_revocation_mail'): + return + + review_request = review.review_request + + if not review_request.public: + return + + message, sent = send_email(prepare_review_ship_it_revoked_mail, + user=user, + review=review, + review_request=review_request) + + if sent: + _update_email_info(review, message.message_id) + + def send_user_registered_mail(user, **kwargs): """Send e-mail when a user is registered. diff --git a/reviewboard/notifications/tests/test_email_sending.py b/reviewboard/notifications/tests/test_email_sending.py index 1a4584de28e9ddba2ec47661d12c1191b064f67b..9bb0de29ef24859344b9616ebb1459f126b9f22a 100644 --- a/reviewboard/notifications/tests/test_email_sending.py +++ b/reviewboard/notifications/tests/test_email_sending.py @@ -1753,6 +1753,33 @@ class ReviewRequestEmailTests(ReviewRequestEmailTestsMixin, DmarcDnsTestsMixin, self.assertTrue(Review.FIX_IT_THEN_SHIP_IT_TEXT in message.message().as_string()) + def test_review_shipit_revoked_email(self): + """Testing sending an email when a review marked with 'Ship It' + has been revoked + """ + settings = { + 'mail_send_shipit_revocation_mail': True, + } + + review_request = self.create_review_request(public=True, + publish=True) + + review = self.create_review(review_request, + ship_it=True, + publish=True) + + self.assertEqual(review_request.shipit_count, 1) + + mail.outbox = [] + with self.siteconfig_settings(settings): + review.revoke_ship_it(review.user) + + self.assertEqual(len(mail.outbox), 1) + message = mail.outbox[0].message().as_string() + + self.assertTrue(Review.REVOKED_SHIP_IT_TEXT in message) + self.assertFalse(Review.FIX_IT_THEN_SHIP_IT_TEXT in message) + def test_change_ownership_email(self): """Testing sending a review request e-mail when the owner is being changed diff --git a/reviewboard/templates/notifications/review_ship_it_revoked_email.html b/reviewboard/templates/notifications/review_ship_it_revoked_email.html new file mode 100644 index 0000000000000000000000000000000000000000..c4e9de1450b4cb97f42d3b9d7a0debfb55ddaf43 --- /dev/null +++ b/reviewboard/templates/notifications/review_ship_it_revoked_email.html @@ -0,0 +1,88 @@ +{% extends "notifications/email_base.html" %} +{% load djblets_email djblets_extensions djblets_utils %} +{% load markdown_email rb_extensions %} + +{% block content %} +

+{{review.SHIP_IT_TEXT}} +

+ +{% template_hook_point "review-email-html-summary" %} +{% if review.body_top %} + 
{{review.body_top|markdown_email_html:review.body_top_rich_text}}
+
+{% endif %} + +{% for comment in review.screenshot_comments.all %} + + + + + + + +
{{comment.screenshot.image.name|basename}}
{{comment.screenshot.caption}}
+
+{% comment_detail_display_hook comment "html-email" %} + 
{{comment.text|markdown_email_html:comment.rich_text}}
+
+
+{% endfor %} + +{% for comment in review.file_attachment_comments.all %} + + + + +
+ {{comment.get_link_text}} +{% if comment.file_attachment.caption %} +

{{comment.file_attachment.caption}}

+{% endif %} +{% with comment.thumbnail as thumbnail %} +{% if thumbnail %} +
{{thumbnail|default:''|safe}}
+{% endif %} +{% endwith %} +
+
+{% comment_detail_display_hook comment "html-email" %} + 
{{comment.text|markdown_email_html:comment.rich_text}}
+
+
+{% endfor %} + +{% for comment in review.general_comments.all %} +
+{% comment_detail_display_hook comment "html-email" %} + 
{{comment.text|markdown_email_html:comment.rich_text}}
+
+
+{% endfor %} + +{% for entry in comment_entries %} +
+{{entry.html|safe}} +
+{% comment_detail_display_hook entry.comment "html-email" %} + 
{{entry.comment.text|markdown_email_html:entry.comment.rich_text}}
+
+
+
+{% endfor %} + +{% if review.body_bottom %} + 
{{review.body_bottom|markdown_email_html:review.body_bottom_rich_text}}
+{% endif %} +

- {{review.user|user_displayname}}

+{% endblock %} + +{% block footer %} +
+

On {{review_request.time_emailed|date:"F jS, Y, P T"}}, {{review_request.submitter|user_displayname}} wrote:

+ +{% with 1 as embedded %} +{% include "notifications/review_request_email.html" %} +{% endwith %} +{% endblock %} + diff --git a/reviewboard/templates/notifications/review_ship_it_revoked_email.txt b/reviewboard/templates/notifications/review_ship_it_revoked_email.txt new file mode 100644 index 0000000000000000000000000000000000000000..36d01d948bd603487c0a86dfcd22bbf24f076942 --- /dev/null +++ b/reviewboard/templates/notifications/review_ship_it_revoked_email.txt @@ -0,0 +1,62 @@ +{% autoescape off %}{% load djblets_email djblets_extensions djblets_utils markdown_email rb_extensions reviewtags %} +----------------------------------------------------------- +This is an automatically generated e-mail. To reply, visit: +{{site_url}}{{review.get_absolute_url}} +----------------------------------------------------------- +{{ review.REVOKED_SHIP_IT_TEXT }} +{% template_hook_point "review-email-text-summary" %} +{% if review.body_top %} +{{review.body_top|markdown_email_text:review.body_top_rich_text}} +{% endif %}{% for comment in review.file_attachment_comments.all %} + +File Attachment: {% if comment.file_attachment.caption %}{{comment.file_attachment.caption}} - {% endif %}{{comment.get_link_text}} +<{{site_url}}{{comment.get_review_url}}> + +{% filter indent %}{% condense 2 %} +{% comment_detail_display_hook comment "text-email" %} + +{{comment.text|markdown_email_text:comment.rich_text}} +{% endcondense %}{% endfilter %} + +{% endfor %}{% for comment in review.general_comments.all %} + +{% filter indent %}{% condense 2 %} +{% comment_detail_display_hook comment "text-email" %} + +{{comment.text|markdown_email_text:comment.rich_text}} +{% endcondense %}{% endfilter %} + +{% endfor %}{% for comment in review.screenshot_comments.all %} + +Screenshot: {{ comment.screenshot.caption }} +<{{site_url}}{{comment.get_review_url}}> + +{% filter indent %}{% condense 2 %} +{% comment_detail_display_hook comment "text-email" %} + +{{comment.text|markdown_email_text:comment.rich_text}} +{% endcondense %}{% endfilter %} + +{% endfor %}{% for entry in comment_entries %} + +{% condense 1 %} +{{entry.comment.filediff.source_file_display}} +{% diff_comment_line_numbers entry.chunks entry.comment %} +<{{site_url}}{{entry.comment.get_review_url}}> +{% endcondense %} + +{% filter indent %}{% condense 2 %} +{% comment_detail_display_hook entry.comment "text-email" %} + +{{entry.comment.text|markdown_email_text:entry.comment.rich_text}} +{% endcondense %}{% endfilter %} + +{% endfor %}{% if review.body_bottom %} +{{review.body_bottom|markdown_email_text:review.body_bottom_rich_text}} +{% endif %} +- {{review.user|user_displayname}} + + +On {{review_request.time_emailed}}, {{review_request.submitter|user_displayname}} wrote: +{% quoted_email "notifications/review_request_email.txt" %} +{% endautoescape %}