Notification: cancel notifications automatically (#11048)

* Notification: cancel notifications automatically

Closes #10981

* Notifications: cancel `Project.skip` and `Organization.disabled`

Attach Django signals to `Organization` and `Project` to handle these two cases.

* Notifications: move check from each request to signal

* Notifications: use Django signals for `user_email_verified`

readthedocs/api/v3/tests/responses/notifications-list.json

@@ -5,18 +5,18 @@
   "results": [
     {
       "_links": {
-        "_self": "https://readthedocs.org/api/v3/users/testuser/notifications/4/"
+        "_self": "https://readthedocs.org/api/v3/organizations/organization/notifications/1/"
       },
-      "attached_to_content_type": "user",
+      "attached_to_content_type": "organization",
       "attached_to_id": 1,
       "dismissable": false,
-      "id": 4,
+      "id": 1,
       "message": {
-        "body": "Your primary email address is not verified.\nPlease <a href=\"\">verify it here</a>.",
-        "header": "Email address not verified",
-        "icon_classes": "fas fa-circle-exclamation",
-        "id": "core:email:validation-pending",
-        "type": "warning"
+        "body": "The organization \"organization\" is currently disabled. You need to <a href=\"\">renew your subscription</a> to keep using Read the Docs",
+        "header": "Your organization has been disabled",
+        "icon_classes": "fas fa-circle-info",
+        "id": "organization:disabled",
+        "type": "info"
       },
       "news": false,
       "state": "unread"
@@ -41,18 +41,18 @@
     },
     {
       "_links": {
-        "_self": "https://readthedocs.org/api/v3/organizations/organization/notifications/1/"
+        "_self": "https://readthedocs.org/api/v3/users/testuser/notifications/4/"
       },
-      "attached_to_content_type": "organization",
+      "attached_to_content_type": "user",
       "attached_to_id": 1,
       "dismissable": false,
-      "id": 1,
+      "id": 4,
       "message": {
-        "body": "The organization \"organization\" is currently disabled. You need to <a href=\"\">renew your subscription</a> to keep using Read the Docs",
-        "header": "Your organization has been disabled",
-        "icon_classes": "fas fa-circle-info",
-        "id": "organization:disabled",
-        "type": "info"
+        "body": "Your primary email address is not verified.\nPlease <a href=\"\">verify it here</a>.",
+        "header": "Email address not verified",
+        "icon_classes": "fas fa-circle-exclamation",
+        "id": "core:email:validation-pending",
+        "type": "warning"
       },
       "news": false,
       "state": "unread"

readthedocs/api/v3/tests/test_notifications.py

@@ -1,6 +1,11 @@
+import django_dynamic_fixture as fixture
+from django.contrib.contenttypes.models import ContentType
 from django.test import override_settings
 from django.urls import reverse
 
+from readthedocs.notifications.constants import CANCELLED, DISMISSED
+from readthedocs.notifications.models import Notification
+from readthedocs.projects.notifications import MESSAGE_PROJECT_SKIP_BUILDS
 from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS
 from readthedocs.subscriptions.products import RTDProductFeature
 
@@ -30,6 +35,30 @@ def test_notifications_list(self):
             self._get_response_dict("notifications-list"),
         )
 
+        # Adding a CANCELLED/DISMISSED notification won't be returned on this endpoint
+        fixture.get(
+            Notification,
+            attached_to_content_type=ContentType.objects.get_for_model(self.project),
+            attached_to_id=self.project.id,
+            message_id=MESSAGE_PROJECT_SKIP_BUILDS,
+            state=CANCELLED,
+        )
+
+        fixture.get(
+            Notification,
+            attached_to_content_type=ContentType.objects.get_for_model(self.project),
+            attached_to_id=self.project.id,
+            message_id=MESSAGE_PROJECT_SKIP_BUILDS,
+            state=DISMISSED,
+        )
+
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(
+            response.json(),
+            self._get_response_dict("notifications-list"),
+        )
+
     def test_notifications_list_post(self):
         url = reverse("notifications-list")
 

readthedocs/core/context_processors.py

@@ -1,7 +1,6 @@
 """Template context processors for core app."""
 
 from django.conf import settings
-from django.contrib.contenttypes.models import ContentType
 
 
 def readthedocs_processor(request):
@@ -45,11 +44,7 @@ def user_notifications(request):
 
     user_notifications = Notification.objects.none()
     if request.user.is_authenticated:
-        content_type = ContentType.objects.get_for_model(request.user)
-        user_notifications = Notification.objects.filter(
-            attached_to_content_type=content_type,
-            attached_to_id=request.user.pk,
-        )
+        user_notifications = Notification.objects.for_user(request.user)
 
     return {
         "user_notifications": user_notifications,

readthedocs/notifications/apps.py

@@ -5,3 +5,6 @@
 class NotificationsAppConfig(AppConfig):
     name = "readthedocs.notifications"
     verbose_name = "Notifications"
+
+    def ready(self):
+        import readthedocs.notifications.signals  # noqa

readthedocs/notifications/querysets.py

@@ -5,14 +5,16 @@
 
 from readthedocs.core.permissions import AdminPermission
 
+from .constants import CANCELLED, READ, UNREAD
+
 
 class NotificationQuerySet(models.QuerySet):
     def add(self, *args, **kwargs):
         """
         Create a notification without duplicating it.
 
         If a notification with the same ``message_id`` is already attached to the object,
-        its ``modified_at`` timestamp is updated.
+        its ``modified_at`` timestamp and ``state`` is updated.
 
         Otherwise, a new notification object is created for this object.
         """
@@ -25,17 +27,45 @@ def add(self, *args, **kwargs):
             attached_to_content_type=content_type,
             attached_to_id=attached_to.id,
             message_id=message_id,
+            # Update only ``READ`` and ``UNREAD`` notifications because we want
+            # to keep track of ``DISMISSED`` and ``CANCELLED`` ones.
+            state__in=(UNREAD, READ),
         ).first()
 
         if notification:
             self.filter(pk=notification.pk).update(
-                *args, modified=timezone.now(), **kwargs
+                *args,
+                modified=timezone.now(),
+                state=UNREAD,
+                **kwargs,
             )
             notification.refresh_from_db()
             return notification
 
         return super().create(*args, attached_to=attached_to, **kwargs)
 
+    def cancel(self, message_id, attached_to):
+        """
+        Cancel an on-going notification because the underlying state has changed.
+
+        When a notification is not valid anymore because the user has made the
+        required action (e.g. paid an unpaid subscription) we use this method
+        to mark those notifications as ``CANCELLED``.
+
+        It only cancels notifications that are ``UNREAD`` or ``READ``.
+        """
+        content_type = ContentType.objects.get_for_model(attached_to)
+
+        self.filter(
+            attached_to_content_type=content_type,
+            attached_to_id=attached_to.id,
+            message_id=message_id,
+            state__in=(UNREAD, READ),
+        ).update(
+            state=CANCELLED,
+            modified=timezone.now(),
+        )
+
     def for_user(self, user):
         """
         Retrieve all notifications related to a particular user.
@@ -45,6 +75,8 @@ def for_user(self, user):
          - are attached to an ``Organization`` where the user is owner
          - are attached to a ``Project`` where the user is admin
          - are attacehd to the ``User`` themselves
+
+        It only returns notifications that are ``READ`` or ``UNREAD``.
         """
         # Need to be here due to circular imports
         from readthedocs.organizations.models import Organization
@@ -76,4 +108,6 @@ def for_user(self, user):
 
         # Return all the notifications related to this user attached to:
         # User, Project and Organization models where the user is admin.
-        return user_notifications | project_notifications | organization_notifications
+        return (
+            user_notifications | project_notifications | organization_notifications
+        ).filter(state__in=(UNREAD, READ))

readthedocs/notifications/signals.py

@@ -0,0 +1,68 @@
+"""Custom Django signals related to notifications."""
+
+import structlog
+from allauth.account.models import EmailAddress
+from django.db.models.signals import post_save
+from django.dispatch import receiver
+from django.urls import reverse
+
+from readthedocs.core.notifications import MESSAGE_EMAIL_VALIDATION_PENDING
+from readthedocs.notifications.models import Notification
+from readthedocs.organizations.models import Organization
+from readthedocs.projects.models import Project
+from readthedocs.projects.notifications import MESSAGE_PROJECT_SKIP_BUILDS
+from readthedocs.subscriptions.notifications import MESSAGE_ORGANIZATION_DISABLED
+
+log = structlog.get_logger(__name__)
+
+
+@receiver(post_save, sender=Project)
+def project_skip_builds(instance, *args, **kwargs):
+    """Check if the project is ``skip`` and add/cancel the notification."""
+    if instance.skip:
+        Notification.objects.add(
+            message_id=MESSAGE_PROJECT_SKIP_BUILDS,
+            attached_to=instance,
+            dismissable=False,
+        )
+    else:
+        Notification.objects.cancel(
+            message_id=MESSAGE_PROJECT_SKIP_BUILDS,
+            attached_to=instance,
+        )
+
+
+@receiver(post_save, sender=Organization)
+def organization_disabled(instance, *args, **kwargs):
+    """Check if the organization is ``disabled`` and add/cancel the notification."""
+    if instance.disabled:
+        Notification.objects.add(
+            message_id=MESSAGE_ORGANIZATION_DISABLED,
+            attached_to=instance,
+            dismissable=False,
+        )
+    else:
+        Notification.objects.cancel(
+            message_id=MESSAGE_ORGANIZATION_DISABLED,
+            attached_to=instance,
+        )
+
+
+@receiver(post_save, sender=EmailAddress)
+def user_email_verified(instance, *args, **kwargs):
+    """Check if the primary email is validated and cancel the notification."""
+    if instance.primary:
+        if instance.verified:
+            Notification.objects.cancel(
+                attached_to=instance.user,
+                message_id=MESSAGE_EMAIL_VALIDATION_PENDING,
+            )
+        else:
+            Notification.objects.add(
+                attached_to=instance.user,
+                message_id=MESSAGE_EMAIL_VALIDATION_PENDING,
+                dismissable=True,
+                format_values={
+                    "account_email_url": reverse("account_email"),
+                },
+            )