[Reference PR] Post-NR with Migration

api/subscriptions/views.py

@@ -1,4 +1,4 @@
-from django.db.models import Value, When, Case, OuterRef, Subquery
+from django.db.models import Value, When, Case, OuterRef, Subquery, F
 from django.db.models.fields import CharField, IntegerField
 from django.db.models.functions import Concat, Cast
 from django.contrib.contenttypes.models import ContentType
@@ -46,6 +46,10 @@ class SubscriptionList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin):
     def get_queryset(self):
         user_guid = self.request.user._id
 
+        provider_ct = ContentType.objects.get_by_natural_key(app_label='osf', model='abstractprovider')
+        node_ct = ContentType.objects.get_by_natural_key(app_label='osf', model='abstractnode')
+        user_ct = ContentType.objects.get_by_natural_key(app_label='osf', model='osfuser')
+
         node_subquery = AbstractNode.objects.filter(
             id=Cast(OuterRef('object_id'), IntegerField()),
         ).values('guids___id')[:1]
@@ -60,11 +64,13 @@ def get_queryset(self):
             NotificationType.Type.ADDON_FILE_REMOVED.value,
             NotificationType.Type.FOLDER_CREATED.value,
         ]
-        _global_reviews = [
+        _global_reviews_provider = [
             NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value,
             NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION.value,
             NotificationType.Type.PROVIDER_REVIEWS_RESUBMISSION_CONFIRMATION.value,
             NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value,
+        ]
+        _global_reviews_user = [
             NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value,
         ]
         _node_file_updated = [
@@ -80,27 +86,31 @@ def get_queryset(self):
         ]
 
         qs = NotificationSubscription.objects.filter(
-            notification_type__name__in=[
-                NotificationType.Type.USER_FILE_UPDATED.value,
-                NotificationType.Type.NODE_FILE_UPDATED.value,
-                NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value,
-            ] + _global_reviews + _global_file_updated + _node_file_updated,
+            notification_type__name__in=_global_reviews_provider + _global_reviews_user + _global_file_updated + _node_file_updated,
             user=self.request.user,
         ).annotate(
             event_name=Case(
                 When(
                     notification_type__name__in=_node_file_updated,
+                    content_type=node_ct,
                     then=Value('files_updated'),
                 ),
                 When(
                     notification_type__name__in=_global_file_updated,
+                    content_type=user_ct,
                     then=Value('global_file_updated'),
                 ),
                 When(
-                    notification_type__name__in=_global_reviews,
+                    notification_type__name__in=_global_reviews_provider,
+                    content_type=provider_ct,
+                    then=Value('global_reviews'),
+                ),
+                When(
+                    notification_type__name__in=_global_reviews_user,
+                    content_type=user_ct,
                     then=Value('global_reviews'),
                 ),
-                default=Value('notification_type__name'),
+                default=F('notification_type__name'),
             ),
             legacy_id=Case(
                 When(
@@ -112,10 +122,16 @@ def get_queryset(self):
                     then=Value(f'{user_guid}_global_file_updated'),
                 ),
                 When(
-                    notification_type__name__in=_global_reviews,
+                    notification_type__name__in=_global_reviews_provider,
+                    content_type=provider_ct,
+                    then=Value(f'{user_guid}_global_reviews'),
+                ),
+                When(
+                    notification_type__name__in=_global_reviews_user,
+                    content_type=user_ct,
                     then=Value(f'{user_guid}_global_reviews'),
                 ),
-                default=Value('notification_type__name'),
+                default=F('notification_type__name'),
             ),
         ).distinct('legacy_id')
 

api_tests/guids/views/test_guid_detail.py

@@ -12,6 +12,7 @@
     PrivateLinkFactory,
 )
 from website.settings import API_DOMAIN
+from tests.utils import capture_notifications
 
 
 @pytest.mark.django_db
@@ -32,13 +33,14 @@ def registration(self):
     @pytest.fixture()
     def versioned_preprint(self, user):
         preprint = PreprintFactory(reviews_workflow='pre-moderation')
-        PreprintFactory.create_version(
-            create_from=preprint,
-            creator=user,
-            final_machine_state='accepted',
-            is_published=True,
-            set_doi=False
-        )
+        with capture_notifications():
+            PreprintFactory.create_version(
+                create_from=preprint,
+                creator=user,
+                final_machine_state='accepted',
+                is_published=True,
+                set_doi=False
+            )
         return preprint
 
     def test_redirects(self, app, project, registration, user):

api_tests/mailhog/provider/test_preprints.py

@@ -2,7 +2,7 @@
 from osf import features
 
 from framework.auth.core import Auth
-from osf.models import NotificationType, Notification
+from osf.models import NotificationType
 from osf_tests.factories import (
     ProjectFactory,
     AuthUserFactory,
@@ -12,7 +12,6 @@
 from osf.utils.permissions import WRITE
 from tests.base import OsfTestCase
 from tests.utils import get_mailhog_messages, delete_mailhog_messages, capture_notifications, assert_emails
-from notifications.tasks import send_users_instant_digest_email
 
 
 class TestPreprintConfirmationEmails(OsfTestCase):
@@ -35,25 +34,14 @@ def test_creator_gets_email(self):
         assert len(notifications['emits']) == 1
         assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
         messages = get_mailhog_messages()
-        assert not messages['items']
-        assert Notification.objects.all()
-        with capture_notifications(passthrough=True) as notifications:
-            send_users_instant_digest_email.delay()
-
-        messages = get_mailhog_messages()
-        assert messages['count'] == len(notifications['emits'])
+        assert_emails(messages, notifications)
 
         delete_mailhog_messages()
         with capture_notifications(passthrough=True) as notifications:
             self.preprint_branded.set_published(True, auth=Auth(self.user), save=True)
         assert len(notifications['emits']) == 1
         assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
         messages = get_mailhog_messages()
-        assert not messages['items']
-        with capture_notifications(passthrough=True) as notifications:
-            send_users_instant_digest_email.delay()
-        massages = get_mailhog_messages()
-        assert massages['count'] == len(notifications['emits'])
-        assert_emails(massages, notifications)
+        assert_emails(messages, notifications)
 
         delete_mailhog_messages()

api_tests/mailhog/provider/test_submissions.py

@@ -21,7 +21,7 @@
 from osf.models import NotificationType
 
 from osf.migrations import update_provider_auth_groups
-from tests.utils import capture_notifications, get_mailhog_messages, delete_mailhog_messages
+from tests.utils import capture_notifications, get_mailhog_messages, delete_mailhog_messages, assert_emails
 
 
 @pytest.mark.django_db
@@ -85,8 +85,7 @@ def test_get_registration_actions(self, app, registration_actions_url, registrat
         assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS
         assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS
         messages = get_mailhog_messages()
-        assert messages['count'] == 1
-        assert messages['items'][0]['Content']['Headers']['To'][0] == registration.creator.username
+        assert_emails(messages, notifications)
 
         delete_mailhog_messages()
 

api_tests/notifications/test_notification_digest.py

@@ -18,18 +18,15 @@ def add_notification_subscription(user, notification_type, frequency, subscribed
     Create a NotificationSubscription for a user.
     If the notification type corresponds to a subscribed_object, set subscribed_object to get the provider.
     """
-    from osf.models import NotificationSubscription, AbstractProvider
+    from osf.models import NotificationSubscription
     kwargs = {
         'user': user,
         'notification_type': NotificationType.objects.get(name=notification_type),
         'message_frequency': frequency,
     }
     if subscribed_object is not None:
         kwargs['object_id'] = subscribed_object.id
-        if isinstance(subscribed_object, AbstractProvider):
-            kwargs['content_type'] = ContentType.objects.get_for_model(subscribed_object, for_concrete_model=False) if subscribed_object else None
-        else:
-            kwargs['content_type'] = ContentType.objects.get_for_model(subscribed_object) if subscribed_object else None
+        kwargs['content_type'] = ContentType.objects.get_for_model(subscribed_object)
     if subscription is not None:
         kwargs['object_id'] = subscription.id
         kwargs['content_type'] = ContentType.objects.get_for_model(subscription)

api_tests/notifications/test_notifications_cleanup.py

@@ -0,0 +1,189 @@
+import pytest
+from osf.models import Notification, NotificationType, EmailTask, NotificationSubscription
+from notifications.tasks import (
+    notifications_cleanup_task
+)
+from osf_tests.factories import AuthUserFactory
+from website.settings import NOTIFICATIONS_CLEANUP_AGE
+from django.utils import timezone
+from datetime import timedelta
+
+def create_notification(subscription, sent_date=None):
+    return Notification.objects.create(
+        subscription=subscription,
+        event_context={},
+        sent=sent_date
+    )
+
+def create_email_task(user, created_date):
+    et = EmailTask.objects.create(
+        task_id=f'test-{created_date.timestamp()}',
+        user=user,
+        status='SUCCESS',
+    )
+    et.created_at = created_date
+    et.save()
+    return et
+
+@pytest.mark.django_db
+class TestNotificationCleanUpTask:
+
+    @pytest.fixture()
+    def user(self):
+        return AuthUserFactory()
+
+    @pytest.fixture()
+    def notification_type(self):
+        return NotificationType.objects.get_or_create(
+            name='Test Notification',
+            subject='Hello',
+            template='Sample Template',
+        )[0]
+
+    @pytest.fixture()
+    def subscription(self, user, notification_type):
+        return NotificationSubscription.objects.get_or_create(
+            user=user,
+            notification_type=notification_type,
+            message_frequency='daily',
+        )[0]
+
+    def test_dry_run_does_not_delete_records(self, user, subscription):
+        now = timezone.now()
+
+        old_notification = create_notification(
+            subscription,
+            sent_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+        old_email_task = create_email_task(
+            user,
+            created_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+
+        notifications_cleanup_task(dry_run=True)
+
+        assert Notification.objects.filter(id=old_notification.id).exists()
+        assert EmailTask.objects.filter(id=old_email_task.id).exists()
+
+    def test_deletes_old_notifications_and_email_tasks(self, user, subscription):
+        now = timezone.now()
+
+        old_notification = create_notification(
+            subscription,
+            sent_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+        new_notification = create_notification(
+            subscription,
+            sent_date=now - timedelta(days=10),
+        )
+
+        old_email_task = create_email_task(
+            user,
+            created_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+        new_email_task = create_email_task(
+            user,
+            created_date=now - timedelta(days=10),
+        )
+
+        notifications_cleanup_task()
+
+        assert not Notification.objects.filter(id=old_notification.id).exists()
+        assert Notification.objects.filter(id=new_notification.id).exists()
+
+        assert not EmailTask.objects.filter(id=old_email_task.id).exists()
+        assert EmailTask.objects.filter(id=new_email_task.id).exists()
+
+    def test_records_at_cutoff_are_not_deleted(self, user, subscription):
+        now = timezone.now()
+        cutoff = now - NOTIFICATIONS_CLEANUP_AGE + timedelta(hours=1)
+
+        notification = create_notification(
+            subscription,
+            sent_date=cutoff,
+        )
+        email_task = create_email_task(
+            user,
+            created_date=cutoff,
+        )
+
+        notifications_cleanup_task()
+
+        assert Notification.objects.filter(id=notification.id).exists()
+        assert EmailTask.objects.filter(id=email_task.id).exists()
+
+    def test_cleanup_when_only_notifications_exist(self, user, subscription):
+        now = timezone.now()
+
+        notification = create_notification(
+            subscription,
+            sent_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+
+        notifications_cleanup_task()
+
+        assert not Notification.objects.filter(id=notification.id).exists()
+
+    def test_cleanup_when_only_email_tasks_exist(self, user, subscription):
+        now = timezone.now()
+
+        email_task = create_email_task(
+            user,
+            created_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+
+        notifications_cleanup_task()
+
+        assert not EmailTask.objects.filter(id=email_task.id).exists()
+
+    def test_task_is_idempotent(self, user, subscription):
+        now = timezone.now()
+
+        create_notification(
+            subscription,
+            sent_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+        create_email_task(
+            user,
+            created_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+
+        notifications_cleanup_task()
+        notifications_cleanup_task()
+
+        assert Notification.objects.count() == 0
+        assert EmailTask.objects.count() == 0
+
+    def test_recent_records_are_not_deleted(self, user, subscription):
+        now = timezone.now()
+
+        create_notification(
+            subscription,
+            sent_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+        create_email_task(
+            user,
+            created_date=now - NOTIFICATIONS_CLEANUP_AGE - timedelta(days=1),
+        )
+        create_notification(
+            subscription,
+            sent_date=now,
+        )
+        create_email_task(
+            user,
+            created_date=now,
+        )
+
+        notifications_cleanup_task()
+
+        assert Notification.objects.count() == 1
+        assert EmailTask.objects.count() == 1
+
+    def test_not_sent_notifications_are_not_deleted(self, user, subscription):
+        create_notification(subscription)
+        create_notification(subscription)
+        create_notification(subscription)
+
+        notifications_cleanup_task()
+
+        assert Notification.objects.count() == 3