Adds tests to verify that notifications are bundled in the weekly digests

Author: benjaoming

django_nyt/management/commands/notifymail.py

@@ -241,12 +241,9 @@ def _send_with_retry(
             return
 
         while True:
+            notification_ids = [n.id for n in notifications]
             try:
-                self.logger.info(
-                    "Sending to notification ids {notification_ids}".format(
-                        notification_ids=", ".join(str(n.id) for n in notifications)
-                    )
-                )
+                self.logger.info(f"Sending to notification ids {notification_ids}")
                 self._render_and_send(
                     template_name, subject_template_name, context, connection
                 )
@@ -255,6 +252,9 @@ def _send_with_retry(
                     n.save()
                     n.subscription.last_sent = timezone.now()
                     n.subscription.save()
+                models.Subscription.objects.filter(
+                    notification__id__in=notification_ids
+                ).update(last_sent=timezone.now())
                 break
             except smtplib.SMTPSenderRefused:
                 self.logger.error(
@@ -372,18 +372,14 @@ def send_mails(  # noqa: max-complexity=12
                     (template_name, subject_template_name), []
                 )
 
-                # If there is an interval, we use the threshold.
-                if setting.interval:
-                    filter_kwargs = {"created__lte": threshold}
-                else:
-                    filter_kwargs = {}
+                # We assume that if we are sending a digest and we've missed sending it, we can
+                # still just summarize ALL notifications that haven't been emailed.
+                # Always, no matter what.
+                # This also means that if we are sending out emails every 5 minutes, and several
+                # notifications have been triggered meanwhile, they'll go into the same email.
                 emails_per_template[(template_name, subject_template_name)] += list(
-                    subscription.notification_set.filter(
-                        is_emailed=False, **filter_kwargs
-                    ),
+                    subscription.notification_set.filter(is_emailed=False),
                 )
-                print("found:")
-                print(emails_per_template[(template_name, subject_template_name)])
 
             # Send the prepared template names, subjects and context to the user
             for (

docs/release_notes.rst

@@ -17,6 +17,7 @@ Release Notes
 * Documentation reorganized with Diataxis structure and more how-to guides have been added. (Benjamin Balder Bach)
 * New shortcut function ``utils.unsubscribe()``. #137 (Benjamin Balder Bach)
 * Better logging for ``notifymail`` management command #141 (Benjamin Balder Bach)
+* Added fields ``created`` and ``modified`` on models ``Settings`` and ``Subscription``.
 
 **Changed**
 
@@ -28,6 +29,7 @@ Release Notes
 * Improvements to docstrings of main methods ``notify()`` and ``subscribe()`` #129 (Benjamin Balder Bach)
 * Return value of ``notify()`` was changed - it no longer returns an `int` (number of created notifications), instead it returns a list of created notifications.
   This is very useful, see :doc:`howto/object_relations` #134 (Benjamin Balder Bach)
+* The field ``Notification.url`` now accepts 2,000 characters rather than 200.
 
 **Fixed**
 

tests/core/test_email.py

@@ -250,13 +250,19 @@ def test_notify_weekly_nothing_sent_first_week(self):
         assert len(mail.outbox) == 1
         assert "weekly" in mail.outbox[0].subject
 
+        # Ensure nothing is sent by re-running the command
+        mail.outbox = []
+        call_command(
+            "notifymail",
+            "--cron",
+            "--no-sys-exit",
+            now=timezone.now() + timedelta(minutes=WEEKLY + 1),
+        )
+        assert len(mail.outbox) == 0
+
         # Now ensure that we receive an instant notification regardless of the previous weekly notification
         utils.subscribe(self.user1_settings, self.TEST_KEY, send_emails=True)
-        notifications = utils.notify("This is a second test", self.TEST_KEY)
-        print("hello")
-        print(notifications[0].subscription.notification_type)
-        print(notifications[0].subscription.settings.id)
-        print(self.user1_settings.id)
+        utils.notify("This is a second test", self.TEST_KEY)
         assert (
             models.Notification.objects.filter(
                 subscription__notification_type__key=self.TEST_KEY,
@@ -268,5 +274,46 @@ def test_notify_weekly_nothing_sent_first_week(self):
         call_command("notifymail", "--cron", "--no-sys-exit")
         assert len(mail.outbox) == 1
 
-        # Now fast-forward and ensure that the notification is found in the weekly digest
-        ...
+        # Unsubscribe the instant notification
+        mail.outbox = []
+        utils.unsubscribe(self.TEST_KEY, settings=self.user1_settings)
+
+        # Now create 2 notifications and test that it'll get sent when running again a week later
+        notifications = utils.notify("Test is a third test", self.TEST_KEY)
+        notifications += utils.notify("Test is a forth test", self.TEST_KEY)
+
+        call_command("notifymail", "--cron", "--no-sys-exit")
+
+        # Ensure nothing is sent, a week hasn't passed
+        assert len(mail.outbox) == 0
+
+        # Emulate that it was sent ½ week ago and try again
+        user1_weekly_setting.subscription_set.all().update(
+            last_sent=timezone.now() - timedelta(minutes=WEEKLY * 0.5)
+        )
+
+        call_command("notifymail", "--cron", "--no-sys-exit")
+
+        # Ensure nothing is sent, a week hasn't passed
+        assert len(mail.outbox) == 0
+
+        # Emulate that it was sent 1 week ago and try again
+        threshold = timezone.now() - timedelta(
+            minutes=user1_weekly_setting.interval * 1 + 1
+        )
+        user1_weekly_setting.subscription_set.all().update(last_sent=threshold)
+
+        assert user1_weekly_setting.subscription_set.all().exists()
+        assert models.Notification.objects.filter(
+            subscription__settings=user1_weekly_setting,
+            is_emailed=False,
+            subscription__latest__is_emailed=False,
+            subscription__last_sent__lte=threshold,
+        ).exists()
+
+        call_command("notifymail", "--cron", "--no-sys-exit")
+
+        # Ensure that exactly 1 digest is sent
+        assert len(mail.outbox) == 1
+        assert "weekly" in mail.outbox[0].subject
+        assert all(n.message in mail.outbox[0].body for n in notifications)