DeprecatedWebhookEndpointNotification tests and improvements

readthedocs/projects/notifications.py

@@ -32,7 +32,7 @@ class DeprecatedWebhookEndpointNotification(Notification):
     ``.send`` is call and the ``email_period`` was reach from the
     ``Message.created`` time we mark ``send_email=True`` in this instance and
     call the super ``.send`` method that will effectively send the email and
-    mark the message as ``read=True``.
+    mark the message as ``extra_tags='email_sent'``.
     """
 
     name = 'deprecated_webhook_endpoint'
@@ -43,24 +43,27 @@ class DeprecatedWebhookEndpointNotification(Notification):
     level = REQUIREMENT
 
     def __init__(self, context_object, request, user=None):
-        # Each time this class is instantiated we create a new Message (it's
-        # de-duplicated by using the ``message``, ``user`` and ``read`` status)
-        self.message, created = Message.objects.get_or_create(
-            message='{}: {}'.format(self.name, self.get_subject()),
-            level=self.level,
-            user=user,
-            read=False,
+        super(DeprecatedWebhookEndpointNotification, self).__init__(
+            context_object,
+            request,
+            user,
         )
+        self.message, created = self._create_message()
 
         # Mark this notification to be sent as email the first time that it's
         # created (user hits this endpoint for the first time)
         if created:
             self.send_email = True
 
-        super(DeprecatedWebhookEndpointNotification, self).__init__(
-            context_object,
-            request,
-            user,
+    def _create_message(self):
+        # Each time this class is instantiated we create a new Message (it's
+        # de-duplicated by using the ``message``, ``user`` and ``extra_tags``
+        # status)
+        return Message.objects.get_or_create(
+            message='{}: {}'.format(self.name, self.get_subject()),
+            level=self.level,
+            user=self.user,
+            extra_tags='email_delayed',
         )
 
     def send(self, *args, **kwargs):
@@ -69,7 +72,12 @@ def send(self, *args, **kwargs):
             # EmailBackend to effectively send the email
             self.send_email = True
 
-            # Mark the message as read and send the email
-            self.message.read = True
+            # Mark the message as sent and send the email
+            self.message.extra_tags = 'email_sent'
             self.message.save()
+
+            # Create a new Message with ``email_delayed`` so we are prepared to
+            # de-duplicate the following one
+            self._create_message()
+
         super(DeprecatedWebhookEndpointNotification, self).send(*args, **kwargs)

readthedocs/rtd_tests/tests/test_notifications.py

@@ -2,16 +2,22 @@
 """Notification tests"""
 
 from __future__ import absolute_import
+from datetime import timedelta
 import mock
 import django_dynamic_fixture as fixture
+from django.http import HttpRequest
 from django.test import TestCase
 from django.test.utils import override_settings
 from django.contrib.auth.models import User, AnonymousUser
+from django.utils import timezone
+from freezegun import freeze_time
 from messages_extends.models import Message as PersistentMessage
 
 from readthedocs.notifications import Notification, SiteNotification
 from readthedocs.notifications.backends import EmailBackend, SiteBackend
 from readthedocs.notifications.constants import ERROR, INFO_NON_PERSISTENT, WARNING_NON_PERSISTENT
+from readthedocs.projects.models import Project
+from readthedocs.projects.notifications import DeprecatedWebhookEndpointNotification
 from readthedocs.builds.models import Build
 
 
@@ -222,3 +228,92 @@ def test_message(self):
         with mock.patch('readthedocs.notifications.notification.log') as mock_log:
             self.assertEqual(self.n.get_message(False), '')
             mock_log.error.assert_called_once()
+
+
+class DeprecatedWebhookEndpointNotificationTests(TestCase):
+
+    def setUp(self):
+        PersistentMessage.objects.all().delete()
+
+        self.project = fixture.get(Project)
+        self.user = fixture.get(User)
+        self.request = HttpRequest()
+
+        self.notification = DeprecatedWebhookEndpointNotification(
+            self.project,
+            self.request,
+            self.user,
+        )
+
+    def test_deduplication(self):
+        self.assertEqual(PersistentMessage.objects.filter(user=self.user).count(), 1)
+        for x in range(5):
+            DeprecatedWebhookEndpointNotification(
+                self.project,
+                self.request,
+                self.user,
+            )
+        self.assertEqual(PersistentMessage.objects.filter(user=self.user).count(), 1)
+        DeprecatedWebhookEndpointNotification(
+            self.project,
+            self.request,
+            fixture.get(User),
+        )
+        self.assertEqual(PersistentMessage.objects.count(), 2)
+        self.notification.message.extra_tags = 'email_sent'
+        self.notification.message.save()
+        DeprecatedWebhookEndpointNotification(
+            self.project,
+            self.request,
+            self.user,
+        )
+        self.assertEqual(PersistentMessage.objects.filter(user=self.user).count(), 2)
+        self.assertEqual(PersistentMessage.objects.count(), 3)
+
+    @mock.patch('readthedocs.notifications.backends.send_email')
+    def test_send_email(self, send_email):
+        # After creating the notification object, ``send_email=True``
+        self.assertTrue(self.notification.send_email)
+
+        # Calling ``.send`` on this instance will send the email and the
+        # ``Message`` will stay in ``email_delayed`` status.
+        self.notification.send()
+        self.notification.message.refresh_from_db()
+        self.assertEqual(self.notification.message.extra_tags, 'email_delayed')
+        self.assertTrue(send_email.called)
+        send_email.reset_mock()
+
+        # Hit the endpoint twice
+        for x in range(2):
+            notification = DeprecatedWebhookEndpointNotification(
+                self.project,
+                self.request,
+                self.user,
+            )
+        # A SiteNotification is created after calling ``.send`` so we filter for
+        # the message also to be sure that no new notification was created
+        self.assertEqual(PersistentMessage.objects.filter(
+            user=self.user,
+            message__startswith=DeprecatedWebhookEndpointNotification.name).count(),
+            1,
+        )
+        self.assertFalse(notification.send_email)  # second notification
+        notification.send()
+        self.assertFalse(send_email.called)
+        self.assertEqual(notification.message.extra_tags, 'email_delayed')
+
+        # In 7 days, when the ``.send`` method is called from a new created
+        # instance, this method will send the email and mark this ``Message`` as
+        # ``email_sent``
+        with freeze_time(timezone.now() + timedelta(days=7)):
+            notification.send()
+
+        self.assertTrue(send_email.called)
+        self.assertEqual(notification.message.extra_tags, 'email_sent')
+        # A new Message object is created
+        self.assertEqual(PersistentMessage.objects.filter(
+            user=self.user,
+            message__startswith=DeprecatedWebhookEndpointNotification.name).count(),
+            2,
+        )
+        self.assertEqual(PersistentMessage.objects.last().extra_tags, 'email_delayed')

requirements/testing.txt

@@ -19,6 +19,8 @@ Mercurial==4.4.2
 yamale==1.7.0
 pytest-mock==1.10.0
 
+freezegun==0.3.11
+
 # local debugging tools
 datadiff
-ipdb
+ipdb