common/tests/factories/notifications.py

@@ -1,7 +1,10 @@
 from django.contrib.contenttypes.models import ContentType
 from factory.django import DjangoModelFactory
+import actstream.models
 import factory
 
+import notifications.models
+
 RELATION_ALLOWED_MODELS = []
 
 
@@ -17,3 +20,15 @@ def generic_foreign_key_id_for_type_factory(generic_relation_type_field):
 class ContentTypeFactory(DjangoModelFactory):
     class Meta:
         model = ContentType
+
+
+class ActionFactory(DjangoModelFactory):
+    class Meta:
+        model = actstream.models.Action
+
+
+class NotificationFactory(DjangoModelFactory):
+    class Meta:
+        model = notifications.models.Notification
+
+    action = factory.SubFactory(ActionFactory)

common/tests/factories/reviewers.py

@@ -0,0 +1,8 @@
+from factory.django import DjangoModelFactory
+
+from reviewers.models import ApprovalActivity
+
+
+class ApprovalActivityFactory(DjangoModelFactory):
+    class Meta:
+        model = ApprovalActivity

common/tests/factories/users.py

@@ -45,8 +45,8 @@ class UserFactory(DjangoModelFactory):
     oauth_info = factory.RelatedFactory(OAuthUserInfoFactory, factory_related_name='user')
 
 
-def create_moderator():
+def create_moderator(**kwargs):
-    user = UserFactory()
+    user = UserFactory(**kwargs)
     moderators = Group.objects.get(name='moderators')
     user.groups.add(moderators)
     return user

emails/admin.py

@@ -8,8 +8,14 @@ from django.utils.html import format_html
 from django.utils.safestring import mark_safe
 import django.core.mail
 
+from common.tests.factories.extensions import ExtensionFactory, RatingFactory
+from common.tests.factories.notifications import NotificationFactory
+from common.tests.factories.reviewers import ApprovalActivityFactory
+from common.tests.factories.users import UserFactory
+from constants.activity import Verb
 from emails.models import Email
-from emails.util import construct_email, get_template_context
+from emails.util import construct_email
+import reviewers.models
 
 logger = logging.getLogger(__name__)
 User = get_user_model()
@@ -92,16 +98,15 @@ class EmailPreviewAdmin(NoAddDeleteMixin, EmailAdmin):
     def _get_email_sent_message(self, obj):
         return f'Sent a test email "{obj.subject}" to {obj.to} from {obj.from_email}'
 
-    def get_object(self, request, object_id, from_field=None):
+    def get_object(self, request, object_id, from_field=None, fake_context=None):
-        """Construct the Email on th fly from known subscription email templates."""
+        """Construct the Email on th fly from known email templates."""
-        context = {
+        if not fake_context:
-            'user': User(),
+            fake_context = self._get_fake_context(request)
-            **get_template_context(),
+        context = fake_context[object_id]
-        }
+        mail_name = context.get('template', object_id)
-        mail_name = object_id
         email_body_html, email_body_txt, subject = construct_email(mail_name, context)
         return EmailPreview(
-            id=mail_name,
+            id=object_id,
             subject=subject,
             from_email=settings.DEFAULT_FROM_EMAIL,
             reply_to=settings.DEFAULT_REPLY_TO_EMAIL,
@@ -109,10 +114,59 @@ class EmailPreviewAdmin(NoAddDeleteMixin, EmailAdmin):
             message=email_body_txt,
         )
 
+    def _get_fake_context(self, request):
+        fake_context = {'feedback': {}}
+
+        # Make previews for all known notification types
+        fake_extension = ExtensionFactory.build(slug='test')
+        verb_to_action_object = {
+            Verb.APPROVED: ApprovalActivityFactory.build(
+                extension=fake_extension,
+                type=reviewers.models.ApprovalActivity.ActivityType.APPROVED,
+                message='Amazing add-on, approved!',
+            ),
+            Verb.COMMENTED: ApprovalActivityFactory.build(
+                extension=fake_extension,
+                type=reviewers.models.ApprovalActivity.ActivityType.COMMENT,
+                message='This is an important albeit somewhat inconclusive note',
+            ),
+            Verb.RATED_EXTENSION: RatingFactory.build(text='Amazing, 7/10!'),
+            Verb.REPORTED_EXTENSION: None,  # TODO: fake action_object
+            Verb.REPORTED_RATING: None,  # TODO: fake action_object
+            Verb.REQUESTED_CHANGES: ApprovalActivityFactory.build(
+                extension=fake_extension,
+                type=reviewers.models.ApprovalActivity.ActivityType.AWAITING_CHANGES,
+                message=(
+                    'Latest uploaded version does not appear to work, '
+                    'a change is necessary before this can be approved and publicly listed'
+                ),
+            ),
+            Verb.REQUESTED_REVIEW: ApprovalActivityFactory.build(
+                extension=fake_extension,
+                type=reviewers.models.ApprovalActivity.ActivityType.AWAITING_REVIEW,
+                message='This add-on is ready to be reviewed',
+            ),
+        }
+        for verb, action_object in verb_to_action_object.items():
+            fake_notification = NotificationFactory.build(
+                recipient=UserFactory.build(),
+                action__actor=UserFactory.build(),
+                action__target=fake_extension,
+                action__verb=verb,
+                action__action_object=action_object,
+            )
+            mail_name = fake_notification.full_template_name
+            fake_context[mail_name] = {
+                'template': fake_notification.template_name,
+                **fake_notification.get_template_context(),
+            }
+        return fake_context
+
     def _get_emails_list(self, request):
         emails = []
-        for mail_name in ('feedback',):
+        fake_context = self._get_fake_context(request)
-            emails.append(self.get_object(request, object_id=mail_name))
+        for mail_name in fake_context:
+            emails.append(self.get_object(request, object_id=mail_name, fake_context=fake_context))
         return emails
 
     def _changeform_view(self, request, object_id, form_url, extra_context):

emails/templates/emails/components/new_activity_action

@@ -0,0 +1,21 @@
+{% spaceless %}{% load i18n %}
+  {% with target_type=target.get_type_display what=target|safe someone=action.actor verb=action.verb %}
+    {% if verb == Verb.APPROVED %}
+      {% blocktrans %}{{ someone }} {{ verb }} {{ what }}{% endblocktrans %}
+    {% elif action.verb == Verb.COMMENTED %}
+      {% blocktrans %}{{ someone }} {{ verb }} on {{ what }}{% endblocktrans %}
+    {% elif verb == Verb.RATED_EXTENSION %}
+      {% blocktrans %}{{ someone }} {{ verb }} {{ what }}{% endblocktrans %}
+    {% elif verb == Verb.REPORTED_EXTENSION %}
+      {% blocktrans %}{{ someone }} reported {{ what }}{% endblocktrans %}
+    {% elif verb == Verb.REPORTED_RATING %}
+      {% blocktrans %}{{ someone }} {{ verb }} of {{ what }}{% endblocktrans %}
+    {% elif verb == Verb.REQUESTED_CHANGES %}
+      {% blocktrans %}{{ someone }} {{ verb }} on {{ what }}{% endblocktrans %}
+    {% elif verb == Verb.REQUESTED_REVIEW %}
+      {% blocktrans %}{{ someone }} {{ verb }} of {{ what }}{% endblocktrans %}
+    {% else %}
+      {% blocktrans %}{{ someone }} {{ verb }} {{ what }}{% endblocktrans %}
+    {% endif %}
+  {% endwith %}
+{% endspaceless %}

emails/templates/emails/new_activity.html

@@ -0,0 +1,26 @@
+{% extends "emails/base.html" %}
+{% block content %}
+  <div>
+    {% include "emails/components/new_activity_action" %}{% if quoted_message %}:{% endif %}
+    {% if quoted_message %}
+      <p>
+        <q>{{ quoted_message|truncatewords:15|truncatechars:140 }}</q>
+      </p>
+    {% endif %}
+  </div>
+  <p><a href="{{ url }}">{{ url }}</a></p>
+
+  <p>
+    Read all notifications at {{ site_url }}{% url "notifications:notifications" %}
+  </p>
+
+  {# TODO: store follow flags on Notifications, otherwise it's impossible to tell why this email is sent #}
+  {% comment %}
+    You are receiving this email because you are a moderator subscribed to notification emails.
+    You are receiving this email because you are subscribed to notifications on this extension.
+  {% endcomment %}
+{% endblock content %}
+
+{% block footer_links %}
+  Unsubscribe by adjusting your preferences at  {{ site_url }}{% url "users:my-profile" %}
+{% endblock footer_links %}

emails/templates/emails/new_activity_subject.txt

@@ -0,0 +1,21 @@
+{% spaceless %}{% load i18n %}
+{% with target_type=target.get_type_display what=target|safe name=target.name someone=action.actor verb=action.verb %}
+{% if verb == Verb.APPROVED %}
+    {% blocktrans %}{{ target_type }} approved: "{{ name }}"{% endblocktrans %}
+{% elif verb == Verb.COMMENTED %}
+    {% blocktrans %}New comment on {{ what }}{% endblocktrans %}
+{% elif verb == Verb.RATED_EXTENSION %}
+    {% blocktrans %}{{ target_type }} rated: "{{ name }}"{% endblocktrans %}
+{% elif verb == Verb.REPORTED_EXTENSION %}
+    {% blocktrans %}{{ target_type }} reported: "{{ name }}"{% endblocktrans %}
+{% elif verb == Verb.REPORTED_RATING %}
+    {% blocktrans %}{{ target_type }} rating reported: "{{ name }}"{% endblocktrans %}
+{% elif verb == Verb.REQUESTED_CHANGES %}
+    {% blocktrans %}{{ target_type }} changes requested: "{{ name }}"{% endblocktrans %}
+{% elif verb == Verb.REQUESTED_REVIEW %}
+    {% blocktrans %}{{ target_type }} review requested: "{{ name }}"{% endblocktrans %}
+{% else %}
+    {% blocktrans %}{{ someone }} {{ verb }} on {{ what }}{% endblocktrans %}
+{% endif %}
+{% endwith %}
+{% endspaceless %}

emails/util.py

@@ -4,9 +4,10 @@ import logging
 
 from django.conf import settings
 from django.contrib.sites.shortcuts import get_current_site
-from django.template import loader
 from django.core.mail import get_connection, EmailMultiAlternatives
+from django.template import loader, TemplateDoesNotExist
 from django.urls import reverse
+import html2text
 
 logger = logging.getLogger(__name__)
 logger.setLevel(logging.DEBUG)
@@ -47,8 +48,11 @@ def get_template_context() -> Dict[str, str]:
 def construct_email(email_name: str, context: Dict[str, Any]) -> Tuple[str, str, str]:
     """Construct an email message.
 
+    If plain text template is not found, text version will be generated from the HTML of the email.
+
     :return: tuple (html, text, subject)
     """
+    context = {**get_template_context(), **context}
     base_path = 'emails'
     subj_tmpl, html_tmpl, txt_tmpl = (
         f'{base_path}/{email_name}_subject.txt',
@@ -60,7 +64,11 @@ def construct_email(email_name: str, context: Dict[str, Any]) -> Tuple[str, str,
     context['subject'] = subject.strip()
 
     email_body_html = loader.render_to_string(html_tmpl, context)
-    email_body_txt = loader.render_to_string(txt_tmpl, context)
+    try:
+        email_body_txt = loader.render_to_string(txt_tmpl, context)
+    except TemplateDoesNotExist:
+        # Generate plain text content from the HTML one
+        email_body_txt = html2text.html2text(email_body_html)
     return email_body_html, email_body_txt, context['subject']
 
 

notifications/management/commands/send_notification_emails.py

@@ -1,11 +1,10 @@
 """Send user notifications as emails, at most once delivery."""
 import logging
 
-from django.conf import settings
-from django.core.mail import send_mail
 from django.core.management.base import BaseCommand
 from django.utils import timezone
 
+from emails.util import construct_and_send_email
 from notifications.models import Notification
 
 logger = logging.getLogger(__name__)
@@ -36,12 +35,6 @@ class Command(BaseCommand):
 
 
 def send_notification_email(notification):
-    # TODO construct a proper phrase, depending on the verb,
+    template_name = notification.template_name
-    # possibly share a template with NotificationsView
+    context = notification.get_template_context()
-    subject, message = notification.format_email()
+    construct_and_send_email(template_name, context, recipient_list=[notification.recipient.email])
-    send_mail(
-        subject,
-        message,
-        settings.DEFAULT_FROM_EMAIL,
-        [notification.recipient.email],
-    )

notifications/models.py

@@ -1,3 +1,5 @@
+import logging
+
 from actstream.models import Action
 from django.contrib.auth import get_user_model
 from django.db import models
@@ -6,6 +8,7 @@ from constants.activity import Verb
 from utils import absolutify
 
 User = get_user_model()
+logger = logging.getLogger(__name__)
 
 
 class Notification(models.Model):
@@ -30,12 +33,43 @@ class Notification(models.Model):
         ]
         unique_together = ['recipient', 'action']
 
-    def format_email(self):
+    @property
+    def full_template_name(self) -> str:
         action = self.action
-        subject = f'New Activity: {action.actor} {action.verb} {action.target}'
+        action_object = action.action_object
-        url = self.get_absolute_url()
+        target_name = action.target.__class__.__name__.lower()
-        mesage = f'{action.actor} {action.verb} {action.target}: {url}'
+        action_object_name = action_object.__class__.__name__.lower() if action_object else ''
-        return (subject, mesage)
+        return f"new_activity_{action_object_name}_{target_name}_{action.verb.replace(' ', '_')}"
+
+    @property
+    def template_name(self) -> str:
+        default_name = 'new_activity'
+        name = self.full_template_name
+        args = {'name': name, 'default_name': default_name}
+        from django.template import loader, TemplateDoesNotExist
+
+        try:
+            loader.get_template(f'emails/{name}.html')
+        except TemplateDoesNotExist:
+            logger.warning('Template %(name)s does not exist, using %(default_name)s', args)
+            return default_name
+        return name
+
+    def get_template_context(self):
+        action = self.action
+        action_object = action.action_object
+        quoted_message = getattr(action_object, 'message', getattr(action_object, 'text', ''))
+        context = {
+            'Verb': Verb,
+            'action': action,
+            'action_object': action_object,
+            'notification': self,
+            'quoted_message': quoted_message,
+            'target': action.target,
+            'url': self.get_absolute_url(),
+            'user': self.recipient,
+        }
+        return context
 
     def get_absolute_url(self):
         if self.action.verb == Verb.RATED_EXTENSION:
@@ -43,6 +77,9 @@ class Notification(models.Model):
         elif self.action.verb in [
             Verb.APPROVED,
             Verb.COMMENTED,
+            Verb.RATED_EXTENSION,
+            Verb.REPORTED_EXTENSION,
+            Verb.REPORTED_RATING,
             Verb.REQUESTED_CHANGES,
             Verb.REQUESTED_REVIEW,
         ]:

notifications/tests/test_send_emails.py

@@ -0,0 +1,161 @@
+from io import StringIO
+from pathlib import Path
+
+from django.core.management import call_command
+from django.test import TestCase
+from django.urls import reverse
+from django.utils import timezone
+
+from common.tests.factories.extensions import create_approved_version, create_version
+from common.tests.factories.files import FileFactory
+from common.tests.factories.users import UserFactory, create_moderator
+from files.models import File
+from notifications.models import Notification
+from reviewers.models import ApprovalActivity
+
+TEST_FILES_DIR = Path(__file__).resolve().parent / '../../extensions/tests/files'
+
+
+class TestSendNotificationEmails(TestCase):
+    fixtures = ['dev', 'licenses']
+
+    def test_ratings(self):
+        extension = create_approved_version(
+            ratings=[], file__user__confirmed_email_at=timezone.now()
+        ).extension
+        author = extension.authors.first()
+        notification_nr = Notification.objects.filter(recipient=author).count()
+        some_user = UserFactory()
+        self.client.force_login(some_user)
+        url = extension.get_rate_url()
+        response = self.client.post(url, {'score': 3, 'text': 'rating text'})
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(extension.ratings.count(), 1)
+        new_notification_nr = Notification.objects.filter(recipient=author).count()
+        self.assertEqual(new_notification_nr, notification_nr + 1)
+
+        out = StringIO()
+        call_command('send_notification_emails', stdout=out, stderr=out)
+
+    def test_abuse(self):
+        extension = create_approved_version(
+            ratings=[], file__user__confirmed_email_at=timezone.now()
+        ).extension
+        moderator = create_moderator(confirmed_email_at=timezone.now())
+        notification_nr = Notification.objects.filter(recipient=moderator).count()
+        some_user = UserFactory()
+        self.client.force_login(some_user)
+        url = extension.get_report_url()
+        self.client.post(
+            url,
+            {
+                'message': 'test message',
+                'reason': '127',
+                'version': '',
+            },
+        )
+        new_notification_nr = Notification.objects.filter(recipient=moderator).count()
+        self.assertEqual(new_notification_nr, notification_nr + 1)
+
+        out = StringIO()
+        call_command('send_notification_emails', stdout=out, stderr=out)
+
+    def test_new_extension_submitted(self):
+        moderator = create_moderator(confirmed_email_at=timezone.now())
+        notification_nr = Notification.objects.filter(recipient=moderator).count()
+        some_user = UserFactory()
+        file_data = {
+            'metadata': {
+                'tagline': 'Get insight on the complexity of an edit',
+                'id': 'edit_breakdown',
+                'name': 'Edit Breakdown',
+                'version': '0.1.0',
+                'blender_version_min': '4.2.0',
+                'type': 'add-on',
+                'schema_version': "1.0.0",
+            },
+            'file_hash': 'sha256:4f3664940fc41641c7136a909270a024bbcfb2f8523a06a0d22f85c459b0b1ae',
+            'size_bytes': 53959,
+            'tags': ['Sequencer'],
+            'version_str': '0.1.0',
+            'slug': 'edit-breakdown',
+        }
+        file = FileFactory(
+            type=File.TYPES.BPY,
+            user=some_user,
+            original_hash=file_data['file_hash'],
+            hash=file_data['file_hash'],
+            metadata=file_data['metadata'],
+        )
+        create_version(
+            file=file,
+            extension__name=file_data['metadata']['name'],
+            extension__slug=file_data['metadata']['id'].replace("_", "-"),
+            extension__website=None,
+            tagline=file_data['metadata']['tagline'],
+            version=file_data['metadata']['version'],
+            blender_version_min=file_data['metadata']['blender_version_min'],
+            schema_version=file_data['metadata']['schema_version'],
+        )
+        self.client.force_login(some_user)
+        data = {
+            # Most of these values should come from the form's initial values, set in the template
+            # Version fields
+            'release_notes': 'initial release',
+            # Extension fields
+            'description': 'Rather long and verbose description',
+            'support': 'https://example.com/issues',
+            # Previews
+            'form-TOTAL_FORMS': ['2'],
+            'form-INITIAL_FORMS': ['0'],
+            'form-MIN_NUM_FORMS': ['0'],
+            'form-MAX_NUM_FORMS': ['1000'],
+            'form-0-id': '',
+            'form-0-caption': ['First Preview Caption Text'],
+            'form-1-id': '',
+            'form-1-caption': ['Second Preview Caption Text'],
+            # Submit for Approval.
+            'submit_draft': '',
+        }
+        file_name1 = 'test_preview_image_0001.png'
+        file_name2 = 'test_preview_image_0002.png'
+        with open(TEST_FILES_DIR / file_name1, 'rb') as fp1, open(
+            TEST_FILES_DIR / file_name2, 'rb'
+        ) as fp2:
+            files = {
+                'form-0-source': fp1,
+                'form-1-source': fp2,
+            }
+            self.client.post(file.get_submit_url(), {**data, **files})
+        new_notification_nr = Notification.objects.filter(recipient=moderator).count()
+        self.assertEqual(new_notification_nr, notification_nr + 1)
+
+        out = StringIO()
+        call_command('send_notification_emails', stdout=out, stderr=out)
+
+    def test_approval_queue_activity(self):
+        extension = create_approved_version(ratings=[]).extension
+        author = extension.authors.first()
+        moderator = create_moderator()
+        some_user = UserFactory()
+        notification_nrs = {}
+        for user in [author, moderator, some_user]:
+            notification_nrs[user.pk] = Notification.objects.filter(recipient=user).count()
+        # both moderator and some_user start following only after their first comment
+        self._leave_a_comment(moderator, extension, 'need to check this')
+        self._leave_a_comment(some_user, extension, 'this is bad')
+        self._leave_a_comment(moderator, extension, 'thanks for the heads up')
+        new_notification_nrs = {}
+        for user in [author, moderator, some_user]:
+            new_notification_nrs[user.pk] = Notification.objects.filter(recipient=user).count()
+        self.assertEqual(new_notification_nrs[author.pk], notification_nrs[author.pk] + 3)
+        self.assertEqual(new_notification_nrs[moderator.pk], notification_nrs[moderator.pk] + 1)
+        self.assertEqual(new_notification_nrs[some_user.pk], notification_nrs[some_user.pk] + 1)
+
+        out = StringIO()
+        call_command('send_notification_emails', stdout=out, stderr=out)
+
+    def _leave_a_comment(self, user, extension, text):
+        self.client.force_login(user)
+        url = reverse('reviewers:approval-comment', args=[extension.slug])
+        self.client.post(url, {'type': ApprovalActivity.ActivityType.COMMENT, 'message': text})

requirements.txt

@@ -28,6 +28,7 @@ drf-spectacular-sidecar==2024.2.1
 frozenlist==1.3.0
 geoip2==4.6.0
 h11==0.13.0
+html2text==2024.2.26
 idna==3.3
 Jinja2==3.1.2
 jsmin==3.0.1