merge and fix unit tests

Ostap-Zherebetskyi · Johnetordoff · commit 4afeeeab1cd8 · 2025-09-17T13:35:45.000-04:00
diff --git a/addons/base/views.py b/addons/base/views.py
@@ -611,7 +611,6 @@ def create_waterbutler_log(payload, **kwargs):
     target_node = AbstractNode.load(metadata.get('nid'))
     if target_node and payload['action'] != 'download_file':
         update_storage_usage_with_size(payload)
-
     with transaction.atomic():
         file_signals.file_updated.send(target=node, user=user, event_type=action, payload=payload)
     return {'status': 'success'}
diff --git a/api/providers/serializers.py b/api/providers/serializers.py
@@ -364,7 +364,6 @@ def create(self, validated_data):
         context['provider__id'] = provider._id
         context['is_reviews_moderator_notification'] = True
         context['referrer_fullname'] = user.fullname
-        context['user_fullname'] = user.fullname
         context['is_reviews_moderator_notification'] = True
         context['is_admin'] = perm_group == ADMIN
 
diff --git a/api_tests/mailhog/provider/test_preprints.py b/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
+from osf.models import NotificationType, Notification
 from osf_tests.factories import (
     ProjectFactory,
     AuthUserFactory,
@@ -12,6 +12,7 @@
 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):
@@ -28,21 +29,31 @@ def setUp(self):
     @override_switch(features.ENABLE_MAILHOG, active=True)
     def test_creator_gets_email(self):
         delete_mailhog_messages()
+
         with capture_notifications(passthrough=True) as notifications:
             self.preprint.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
-        massages = get_mailhog_messages()
-        assert massages['count'] == len(notifications['emails'])
-        assert_emails(massages, notifications)
+        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'])
 
         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['emails'])
+        assert massages['count'] == len(notifications['emits'])
         assert_emails(massages, notifications)
 
         delete_mailhog_messages()
diff --git a/api_tests/mailhog/provider/test_reviewable.py b/api_tests/mailhog/provider/test_reviewable.py
@@ -6,7 +6,6 @@
 from osf_tests.factories import PreprintFactory, AuthUserFactory
 from tests.utils import get_mailhog_messages, delete_mailhog_messages, capture_notifications, assert_emails
 
-
 @pytest.mark.django_db
 class TestReviewable:
 
@@ -18,16 +17,11 @@ def test_reject_resubmission_sends_emails(self):
             is_published=False
         )
         assert preprint.machine_state == DefaultStates.INITIAL.value
-        delete_mailhog_messages()
         with capture_notifications(passthrough=True) as notifications:
             preprint.run_submit(user)
         assert len(notifications['emits']) == 1
         assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
         assert preprint.machine_state == DefaultStates.PENDING.value
-
-        massages = get_mailhog_messages()
-        assert massages['count'] == len(notifications['emails'])
-        assert_emails(massages, notifications)
         delete_mailhog_messages()
 
         assert not user.notification_subscriptions.exists()
@@ -49,8 +43,9 @@ def test_reject_resubmission_sends_emails(self):
         assert len(notifications['emits']) == 1
         assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_RESUBMISSION_CONFIRMATION
         assert preprint.machine_state == DefaultStates.PENDING.value
-        massages = get_mailhog_messages()
-        assert massages['count'] == len(notifications['emails'])
-        assert_emails(massages, notifications)
+
+        messages = get_mailhog_messages()
+        assert messages['count'] == len(notifications['emits'])
+        assert_emails(messages, notifications)
 
         delete_mailhog_messages()
diff --git a/api_tests/mailhog/provider/test_schema_responses.py b/api_tests/mailhog/provider/test_schema_responses.py
@@ -244,9 +244,9 @@ def test_moderator_accept_notification(
         with capture_notifications(passthrough=True) as notifications:
             revised_response.accept(user=moderator)
         assert len(notifications['emits']) == 3
-        massages = get_mailhog_messages()
-        assert massages['count'] == len(notifications['emails'])
-        assert_emails(massages, notifications)
+        messages = get_mailhog_messages()
+        assert messages['count'] == len(notifications['emails'])
+        assert_emails(messages, notifications)
 
         delete_mailhog_messages()
 
@@ -259,8 +259,8 @@ def test_moderator_reject_notification(
         with capture_notifications(passthrough=True) as notifications:
             revised_response.reject(user=moderator)
         assert len(notifications['emits']) == 3
-        massages = get_mailhog_messages()
-        assert massages['count'] == len(notifications['emails'])
-        assert_emails(massages, notifications)
+        messages = get_mailhog_messages()
+        assert messages['count'] == len(notifications['emails'])
+        assert_emails(messages, notifications)
 
         delete_mailhog_messages()
diff --git a/api_tests/mailhog/provider/test_submissions.py b/api_tests/mailhog/provider/test_submissions.py
@@ -1,5 +1,7 @@
 import pytest
 from waffle.testutils import override_switch
+
+from notifications.tasks import send_users_instant_digest_email
 from osf import features
 
 from api.base.settings.defaults import API_BASE
@@ -19,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, assert_emails
+from tests.utils import capture_notifications, get_mailhog_messages, delete_mailhog_messages
 
 
 @pytest.mark.django_db
@@ -82,9 +84,9 @@ def test_get_registration_actions(self, app, registration_actions_url, registrat
         assert len(notifications['emits']) == 2
         assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS
         assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS
-        massages = get_mailhog_messages()
-        assert massages['count'] == len(notifications['emails'])
-        assert_emails(massages, notifications)
+        messages = get_mailhog_messages()
+        assert messages['count'] == 1
+        assert messages['items'][0]['Content']['Headers']['To'][0] == registration.creator.username
 
         delete_mailhog_messages()
 
@@ -116,9 +118,10 @@ def test_get_provider_actions(self, app, provider_actions_url, registration, mod
         assert len(notifications['emits']) == 2
         assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
         assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
-        massages = get_mailhog_messages()
-        assert massages['count'] == len(notifications['emails'])
-        assert_emails(massages, notifications)
+        send_users_instant_digest_email.delay()
+        messages = get_mailhog_messages()
+        assert messages['count'] == 1
+        assert messages['items'][0]['Content']['Headers']['To'][0] == registration.creator.username
 
         delete_mailhog_messages()
 
diff --git a/api_tests/notifications/test_notification_digest.py b/api_tests/notifications/test_notification_digest.py
@@ -184,6 +184,6 @@ def test_send_moderators_digest_email_end_to_end(fake):
         sent=None,
         event_context={'provider_id': provider.id},
     )
-    send_moderators_digest_email()
+    send_moderators_digest_email.delay()
     email_task = EmailTask.objects.filter(user_id=user.id).first()
     assert email_task.status == 'SUCCESS'
diff --git a/notifications/tasks.py b/notifications/tasks.py
@@ -20,7 +20,7 @@
 logger = get_task_logger(__name__)
 
 @celery_app.task(bind=True)
-def send_user_email_task(self, user_id, notification_ids, message_freq):
+def send_user_email_task(self, user_id, notification_ids, **kwargs):
     try:
         user = OSFUser.objects.get(
             guids___id=user_id,
@@ -89,7 +89,7 @@ def send_user_email_task(self, user_id, notification_ids, message_freq):
         raise self.retry(exc=e)
 
 @celery_app.task(bind=True)
-def send_moderator_email_task(self, user_id, provider_id, notification_ids, message_freq):
+def send_moderator_email_task(self, user_id, provider_id, notification_ids, **kwargs):
     try:
         user = OSFUser.objects.get(
             guids___id=user_id,
@@ -165,7 +165,6 @@ def send_moderator_email_task(self, user_id, provider_id, notification_ids, mess
         NotificationType.Type.DIGEST_REVIEWS_MODERATORS.instance.emit(
             user=user,
             event_context=event_context,
-            is_digest=True
         )
 
         notifications_qs.update(sent=timezone.now())
@@ -196,7 +195,7 @@ def send_users_digest_email(dry_run=False):
             user_id = group['user_id']
             notification_ids = [msg['notification_id'] for msg in group['info']]
             if not dry_run:
-                send_user_email_task.delay(user_id, notification_ids, freq)
+                send_user_email_task.delay(user_id, notification_ids)
 
 @celery_app.task(name='notifications.tasks.send_moderators_digest_email')
 def send_moderators_digest_email(dry_run=False):
@@ -215,7 +214,7 @@ def send_moderators_digest_email(dry_run=False):
             provider_id = group['provider_id']
             notification_ids = [msg['notification_id'] for msg in group['info']]
             if not dry_run:
-                send_moderator_email_task.delay(user_id, provider_id, notification_ids, freq)
+                send_moderator_email_task.delay(user_id, provider_id, notification_ids)
 
 def get_moderators_emails(message_freq: str):
     """Get all emails for reviews moderators that need to be sent, grouped by users AND providers.
@@ -322,8 +321,8 @@ def remove_subscription_task(node_id):
     ).delete()
 
 
-@celery_app.task(name='notifications.tasks.send_users_instant_digest_email')
-def send_users_instant_digest_email(dry_run):
+@celery_app.task(bind=True, name='notifications.tasks.send_users_instant_digest_email')
+def send_users_instant_digest_email(self, dry_run=False, **kwargs):
     """Send pending "instant' digest emails.
     :return:
     """
@@ -332,10 +331,10 @@ def send_users_instant_digest_email(dry_run):
         user_id = group['user_id']
         notification_ids = [msg['notification_id'] for msg in group['info']]
         if not dry_run:
-            send_user_email_task.delay(user_id, notification_ids, 'instantly')
+            send_user_email_task.delay(user_id, notification_ids)
 
-@celery_app.task(name='notifications.tasks.send_moderators_instant_digest_email')
-def send_moderators_instant_digest_email(dry_run=False):
+@celery_app.task(bind=True, name='notifications.tasks.send_moderators_instant_digest_email')
+def send_moderators_instant_digest_email(self, dry_run=False, **kwargs):
     """Send pending "instant' digest emails.
     :return:
     """
@@ -345,4 +344,4 @@ def send_moderators_instant_digest_email(dry_run=False):
         provider_id = group['provider_id']
         notification_ids = [msg['notification_id'] for msg in group['info']]
         if not dry_run:
-            send_moderator_email_task.delay(user_id, provider_id, notification_ids, 'instantly')
+            send_moderator_email_task.delay(user_id, provider_id, notification_ids)
diff --git a/osf/migrations/0003_aggregated_runsql_calls.py b/osf/migrations/0003_aggregated_runsql_calls.py
@@ -11,6 +11,7 @@ class Migration(migrations.Migration):
         migrations.RunSQL(
             [
                 """
+                CREATE UNIQUE INDEX one_quickfiles_per_user ON public.osf_abstractnode USING btree (creator_id, type, is_deleted) WHERE (((type)::text = 'osf.quickfilesnode'::text) AND (is_deleted = false));
                 CREATE INDEX osf_abstractnode_collection_pub_del_type_index ON public.osf_abstractnode USING btree (is_public, is_deleted, type) WHERE ((is_public = true) AND (is_deleted = false) AND ((type)::text = 'osf.collection'::text));
                 CREATE INDEX osf_abstractnode_date_modified_ef1e2ad8 ON public.osf_abstractnode USING btree (last_logged);
                 CREATE INDEX osf_abstractnode_node_pub_del_type_index ON public.osf_abstractnode USING btree (is_public, is_deleted, type) WHERE ((is_public = true) AND (is_deleted = false) AND ((type)::text = 'osf.node'::text));
diff --git a/osf/migrations/0016_auto_20230828_1810.py b/osf/migrations/0016_auto_20230828_1810.py
@@ -23,6 +23,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='abstractnode',
             name='type',
-            field=models.CharField(choices=[('osf.node', 'node'), ('osf.draftnode', 'draft node'), ('osf.registration', 'registration')], db_index=True, max_length=255),
+            field=models.CharField(choices=[('osf.node', 'node'), ('osf.draftnode', 'draft node'), ('osf.quickfilesnode', 'quick files node'), ('osf.registration', 'registration')], db_index=True, max_length=255),
         ),
     ]
diff --git a/osf/migrations/0033_notification_system.py b/osf/migrations/0033_notification_system.py
@@ -17,6 +17,7 @@ class Migration(migrations.Migration):
     ]
 
     operations = [
+        # This renames the legacy data so we have overwrite it later with migrate notifications management command
         migrations.RunSQL(
             """
             DO $$
diff --git a/osf/models/mixins.py b/osf/models/mixins.py
@@ -1085,7 +1085,8 @@ def add_to_group(self, user, group):
             user=user,
             content_type=ContentType.objects.get_for_model(self),
             object_id=self.id,
-            notification_type=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance
+            notification_type=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance,
+            _is_digest=True
         )
 
     def remove_from_group(self, user, group, unsubscribe=True):
@@ -1096,21 +1097,13 @@ def remove_from_group(self, user, group, unsubscribe=True):
         if unsubscribe:
             # remove notification subscription
             for subscription in self.DEFAULT_SUBSCRIPTIONS:
-                self.remove_user_from_subscription(user, subscription)
+                NotificationSubscription.objects.filter(
+                    notification_type=subscription.instance,
+                    user=user,
+                ).delete()
 
         return _group.user_set.remove(user)
 
-    def remove_user_from_subscription(self, user, subscription):
-        notification_type = NotificationType.objects.get(
-            name=subscription,
-        )
-        subscriptions = NotificationSubscription.objects.filter(
-            notification_type=notification_type,
-            user=user
-        )
-        if subscriptions:
-            subscriptions.get().delete()
-
 
 class TaxonomizableMixin(models.Model):
     class Meta:
diff --git a/osf/models/notification_type.py b/osf/models/notification_type.py
@@ -229,15 +229,6 @@ def emit(
             save=save,
         )
 
-    def remove_user_from_subscription(self, user):
-        """
-        """
-        from osf.models.notification_subscription import NotificationSubscription
-        notification, _ = NotificationSubscription.objects.filter(
-            user=user,
-            notification_type=self,
-        ).delete()
-
     def __str__(self) -> str:
         return self.name
 
diff --git a/osf/models/preprint.py b/osf/models/preprint.py
@@ -1049,6 +1049,7 @@ def _send_preprint_confirmation(self, auth):
                 'document_type': self.provider.preprint_word,
                 'notify_comment': not self.provider.reviews_comments_private
             },
+            is_digest=True
         )
 
     # FOLLOWING BEHAVIOR NOT SPECIFIC TO PREPRINTS
diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py
@@ -3,7 +3,7 @@
 
 from django.utils import timezone
 
-from notifications.tasks import send_users_digest_email, send_moderators_digest_email
+from notifications.tasks import send_users_digest_email
 from osf.management.commands.populate_notification_types import populate_notification_types
 from osf.migrations import update_provider_auth_groups
 from osf.models import Brand, NotificationSubscription, NotificationType
@@ -196,4 +196,3 @@ def test_branded_provider_notification_renders(self, registration, admin, modera
 
         with capture_notifications():
             notify_submit(registration, admin)
-            send_moderators_digest_email()
diff --git a/tasks/__init__.py b/tasks/__init__.py
diff --git a/tests/test_addons.py b/tests/test_addons.py
diff --git a/tests/test_preprints.py b/tests/test_preprints.py
diff --git a/website/notifications/utils.py b/website/notifications/utils.py
diff --git a/website/notifications/views.py b/website/notifications/views.py

Original file line number	Diff line number	Diff line change
`@@ -184,6 +184,6 @@ def test_send_moderators_digest_email_end_to_end(fake):`
`184`	`184`	`sent=None,`
`185`	`185`	`event_context={'provider_id': provider.id},`
`186`	`186`	`)`
`187`		`- send_moderators_digest_email()`
	`187`	`+ send_moderators_digest_email.delay()`
`188`	`188`	`email_task = EmailTask.objects.filter(user_id=user.id).first()`
`189`	`189`	`assert email_task.status == 'SUCCESS'`
Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,7 @@ class Migration(migrations.Migration):`
`11`	`11`	`migrations.RunSQL(`
`12`	`12`	`[`
`13`	`13`	`"""`
	`14`	`+ CREATE UNIQUE INDEX one_quickfiles_per_user ON public.osf_abstractnode USING btree (creator_id, type, is_deleted) WHERE (((type)::text = 'osf.quickfilesnode'::text) AND (is_deleted = false));`
`14`	`15`	`CREATE INDEX osf_abstractnode_collection_pub_del_type_index ON public.osf_abstractnode USING btree (is_public, is_deleted, type) WHERE ((is_public = true) AND (is_deleted = false) AND ((type)::text = 'osf.collection'::text));`
`15`	`16`	`CREATE INDEX osf_abstractnode_date_modified_ef1e2ad8 ON public.osf_abstractnode USING btree (last_logged);`
`16`	`17`	`CREATE INDEX osf_abstractnode_node_pub_del_type_index ON public.osf_abstractnode USING btree (is_public, is_deleted, type) WHERE ((is_public = true) AND (is_deleted = false) AND ((type)::text = 'osf.node'::text));`
Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,6 @@ class Migration(migrations.Migration):`
`23`	`23`	`migrations.AlterField(`
`24`	`24`	`model_name='abstractnode',`
`25`	`25`	`name='type',`
`26`		`- field=models.CharField(choices=[('osf.node', 'node'), ('osf.draftnode', 'draft node'), ('osf.registration', 'registration')], db_index=True, max_length=255),`
	`26`	`+ field=models.CharField(choices=[('osf.node', 'node'), ('osf.draftnode', 'draft node'), ('osf.quickfilesnode', 'quick files node'), ('osf.registration', 'registration')], db_index=True, max_length=255),`
`27`	`27`	`),`
`28`	`28`	`]`