Feature/subscription improvements (#105)

* Update subscriptions email template

- Add helper script to create fake alerts
- Remove legacy templates
- Add helper dev views to update templates

* Fix email templates

* Fix email raw text templates

- Truncate alert admins to 40 chars

* Add error catch

- Update TODOs
- Upgrade pyright

---------

Co-authored-by: thenav56 <ayernavin@gmail.com>

Author: barshathakuri

.pre-commit-config.yaml

@@ -31,6 +31,6 @@ repos:
       - id: flake8
 
   - repo: https://github.com/RobertCraigie/pyright-python
-    rev: v1.1.389
+    rev: v1.1.390
     hooks:
       - id: pyright

apps/cap_feed/management/commands/create_fake_alerts.py

@@ -0,0 +1,128 @@
+import factory
+from django.conf import settings
+from django.core.management.base import BaseCommand
+from django.utils import timezone
+
+from apps.cap_feed.factories import (
+    Admin1Factory,
+    AlertFactory,
+    AlertInfoFactory,
+    CountryFactory,
+    FeedFactory,
+    RegionFactory,
+)
+from apps.cap_feed.models import Admin1, AlertInfo, Country, Feed, Region
+from apps.subscription.factories import UserAlertSubscriptionFactory
+from apps.subscription.models import UserAlertSubscription
+from apps.subscription.tasks import process_pending_subscription_alerts
+from apps.user.models import User
+
+SUBSCRIPTION_FILTERSET = [
+    (
+        "Sub-1",
+        dict(
+            filter_alert_urgencies=[AlertInfo.Urgency.IMMEDIATE],
+            filter_alert_severities=[],
+            filter_alert_certainties=[],
+            filter_alert_categories=[],
+        ),
+    ),
+    (
+        "Sub-1",
+        dict(
+            filter_alert_urgencies=[AlertInfo.Urgency.IMMEDIATE, AlertInfo.Urgency.EXPECTED],
+            filter_alert_severities=[AlertInfo.Severity.MODERATE],
+            filter_alert_certainties=[],
+            filter_alert_categories=[],
+        ),
+    ),
+    (
+        "Sub-3",
+        dict(
+            filter_alert_urgencies=[AlertInfo.Urgency.EXPECTED],
+            filter_alert_severities=[AlertInfo.Severity.MODERATE],
+            filter_alert_certainties=[AlertInfo.Certainty.LIKELY],
+            filter_alert_categories=[],
+        ),
+    ),
+    # Everything
+    (
+        "All",
+        dict(
+            filter_alert_urgencies=[],
+            filter_alert_severities=[],
+            filter_alert_certainties=[],
+            filter_alert_categories=[],
+        ),
+    ),
+]
+
+
+class Command(BaseCommand):
+    def add_arguments(self, parser):
+        parser.add_argument('--user-email', dest='user_email', required=True)
+
+    def handle(self, *_, **kwargs):
+        if not settings.ALLOW_FAKE_DATA:
+            self.stdout.write(
+                self.style.WARNING(
+                    "Add ALLOW_FAKE_DATA=true and DJANGO_DEBUG=true to environment variable to allow fake data generation"
+                )
+            )
+            return
+
+        user_email = kwargs['user_email']
+        user = User.objects.get(email=user_email)
+
+        FAKE_REGION_NAME = "[Fake] Asia"
+        FAKE_COUNTRY_NAME = "[Fake] Nepal"
+        FAKE_ADMIN1_NAME = "[Fake] Bagmati"
+        FAKE_FEED_URL = "https://fake-feed.com/123"
+
+        if (r_asia := Region.objects.filter(name=FAKE_REGION_NAME).first()) is None:
+            r_asia = RegionFactory.create(name=FAKE_REGION_NAME)
+        if (c_nepal := Country.objects.filter(name=FAKE_COUNTRY_NAME).first()) is None:
+            c_nepal = CountryFactory.create(name=FAKE_COUNTRY_NAME, region=r_asia)
+        if (ad1_bagmati := Admin1.objects.filter(name=FAKE_ADMIN1_NAME).first()) is None:
+            ad1_bagmati = Admin1Factory.create(name=FAKE_ADMIN1_NAME, country=c_nepal)
+
+        if (feed1 := Feed.objects.filter(url=FAKE_FEED_URL).first()) is None:
+            feed1 = FeedFactory.create(url=FAKE_FEED_URL, country=c_nepal, polling_interval=False)
+
+        random_id = timezone.now().isoformat()
+        alert_list = AlertFactory.create_batch(
+            50,
+            url=factory.Sequence(lambda n: f"https://alert-{random_id}-{n}.com/test"),
+            feed=feed1,
+            country=c_nepal,
+            admin1s=[ad1_bagmati],
+            sent=timezone.now().date(),
+        )
+
+        alert_info_iterator = dict(
+            category=factory.Iterator(AlertInfo.Category.choices, getter=lambda c: c[0]),
+            urgency=factory.Iterator(AlertInfo.Urgency.choices, getter=lambda c: c[0]),
+            severity=factory.Iterator(AlertInfo.Severity.choices, getter=lambda c: c[0]),
+            certainty=factory.Iterator(AlertInfo.Certainty.choices, getter=lambda c: c[0]),
+        )
+        for alert in alert_list:
+            AlertInfoFactory.create_batch(
+                2,
+                event=factory.Sequence(lambda n: f"Event-{n}"),
+                alert=alert,
+                **alert_info_iterator,
+            )
+
+        # Delete all existing fake subscriptions for this user
+        UserAlertSubscription.objects.filter(user=user, name__startswith="[Fake]").delete()
+        for name, filters in SUBSCRIPTION_FILTERSET:
+            UserAlertSubscriptionFactory.create(
+                name=f"[Fake] {name}",
+                email_frequency=UserAlertSubscription.EmailFrequency.DAILY,
+                user=user,
+                filter_alert_country=c_nepal,
+                filter_alert_admin1s=[ad1_bagmati.pk],
+                **filters,
+            )
+        # Tag new alerts to subscriptions
+        process_pending_subscription_alerts()

apps/subscription/emails.py

@@ -1,5 +1,7 @@
+import logging
 from datetime import timedelta
 
+from django.conf import settings
 from django.db import models
 from django.utils import timezone
 from django.utils.encoding import force_bytes
@@ -9,8 +11,11 @@
 from apps.user.models import EmailNotificationType, User
 from main.permalinks import Permalink
 from main.tokens import TokenManager
+from utils.common import logger_log_extra
 from utils.emails import send_email
 
+logger = logging.getLogger(__name__)
+
 
 def generate_unsubscribe_user_alert_subscription_url(subscription: UserAlertSubscription) -> str:
     uid = urlsafe_base64_encode(force_bytes(subscription.pk))
@@ -22,7 +27,7 @@ def generate_unsubscribe_user_alert_subscription_url(subscription: UserAlertSubs
 def generate_user_alert_subscription_email_context(
     user: User,
     email_frequency: UserAlertSubscription.EmailFrequency,
-) -> tuple[dict, models.QuerySet[UserAlertSubscription]]:
+) -> tuple[bool, dict, models.QuerySet[UserAlertSubscription]]:
     # NOTE: Number of subscription is static and less than UserAlertSubscription.LIMIT_PER_USER
     subscription_qs = UserAlertSubscription.objects.filter(user=user, email_frequency=email_frequency)
 
@@ -31,62 +36,90 @@ def generate_user_alert_subscription_email_context(
     elif email_frequency == UserAlertSubscription.EmailFrequency.WEEKLY:
         from_datetime_threshold = timezone.now() - timedelta(days=7)
     elif email_frequency == UserAlertSubscription.EmailFrequency.MONTHLY:
-        # TODO: Calculate days instead of using 30 days
+        # TODO: Calculate month days instead of using 30 days
         from_datetime_threshold = timezone.now() - timedelta(days=30)
 
-    subscription_data = [
-        {
-            'subscription': subscription,
-            'unsubscribe_url': generate_unsubscribe_user_alert_subscription_url(subscription),
-            'latest_alerts': [
-                subscription_alert.alert
-                # NOTE: N+1 query, but N < 10 for now
-                # TODO: Index/partition alert__sent column?
-                for subscription_alert in (
-                    SubscriptionAlert.objects.select_related('alert')
-                    .filter(
-                        subscription=subscription,
-                        alert__sent__gte=from_datetime_threshold,
-                    )
-                    .order_by('-alert__sent')[:5]
-                )
-            ],
+    def _alert_data(alert):
+        # TODO: Fix N+1 for alert.infos.first() and alert.admin1s
+        info = alert.infos.first()
+        return {
+            "url": Permalink.alert_detail(alert.pk),
+            "name": info and info.event or f"Alert #{alert.pk}",
+            "urgency": info and info.urgency or '-',
+            "severity": info and info.severity or '-',
+            "certainty": info and info.certainty or '-',
+            "admins": ",".join(list(alert.admin1s.values_list("name", flat=True))) or '-',
         }
-        for subscription in subscription_qs
-    ]
+
+    subscription_data = []
+    for subscription in subscription_qs.iterator():
+        latest_alerts = [
+            _alert_data(subscription_alert.alert)
+            # NOTE: N+1 query, but N < 10 for now
+            # TODO: Index/partition alert__sent column?
+            for subscription_alert in (
+                SubscriptionAlert.objects.select_related('alert')
+                .filter(
+                    subscription=subscription,
+                    alert__sent__gte=from_datetime_threshold,
+                )
+                .order_by('-alert__sent')[:5]
+            )
+        ]
+        if latest_alerts:
+            subscription_data.append(
+                {
+                    'subscription': subscription,
+                    'url': Permalink.subscription_detail(subscription.pk),
+                    'unsubscribe_url': generate_unsubscribe_user_alert_subscription_url(subscription),
+                    'latest_alerts': latest_alerts,
+                }
+            )
 
     context = {
-        'subscriptions': subscription_data,
+        'subscriptions_data': subscription_data,
     }
 
-    return context, subscription_qs
+    return len(context["subscriptions_data"]) > 0, context, subscription_qs
 
 
 def send_user_alert_subscription_email(user: User, email_frequency: UserAlertSubscription.EmailFrequency):
-    context, subscription_qs = generate_user_alert_subscription_email_context(user, email_frequency)
+    have_data, context, subscription_qs = generate_user_alert_subscription_email_context(user, email_frequency)
     sent_at = timezone.now()
 
-    send_email(
-        user=user,
-        email_type=EmailNotificationType.ALERT_SUBSCRIPTIONS,
-        subject="Daily Alerts",  # TODO: Is this fine?
-        email_html_template='emails/subscription/body.html',
-        email_text_template='emails/subscription/body.txt',
-        context=context,
-    )
+    if have_data:
+        send_email(
+            user=user,
+            email_type=EmailNotificationType.ALERT_SUBSCRIPTIONS,
+            subject=f"{settings.EMAIL_SUBJECT_PREFIX} {email_frequency.label}",
+            email_html_template='emails/subscription/body.html',
+            email_text_template='emails/subscription/body.txt',
+            context=context,
+        )
 
     # Post action
     subscription_qs.update(email_last_sent_at=sent_at)
 
 
 def send_user_alert_subscriptions_email(email_frequency: UserAlertSubscription.EmailFrequency):
-    # TODO: Send in parallel if email service supports it
+    # TODO: Send in parallel if email service supports it?
     users_qs = User.objects.filter(
         id__in=UserAlertSubscription.objects.filter(email_frequency=email_frequency).values('user'),
     )
 
-    # TODO: Handle failure
     for user in users_qs.iterator():
-        # TODO: Trigger this as cronjob
-        # TODO: Pass timezone.now for ref time
-        send_user_alert_subscription_email(user, email_frequency)
+        # TODO: Trigger this as cronjob?
+        # TODO: Pass timezone.now for ref time?
+        try:
+            send_user_alert_subscription_email(user, email_frequency)
+        except Exception:
+            logger.error(
+                "Subscription: Failed to send email to user",
+                exc_info=True,
+                extra=logger_log_extra(
+                    {
+                        'user_id': user.pk,
+                        'email_frequency': email_frequency,
+                    }
+                ),
+            )