From f65db33f09bcaea56f4fef77727d66c2c6e57ab8 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 13:54:39 -0500 Subject: [PATCH 1/8] improve moderator and user files tests to properly reflect digest code also change listener code to reflect this --- notifications/tasks.py | 8 +- ...t_registration_moderation_notifications.py | 582 ++++++++++++------ website/reviews/listeners.py | 8 +- 3 files changed, 419 insertions(+), 179 deletions(-) diff --git a/notifications/tasks.py b/notifications/tasks.py index d8093d1211c..9dd64f6be74 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -124,7 +124,7 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): provider = getattr(subscribed_object, 'provider', None) if provider is None: - log_message(f"subscribed_object fpr {subscribed_object} does not exist") + log_message(f"provider for subscribed_object {subscribed_object} does not exist") email_task.status = 'PROVIDER NOT FOUND' email_task.save() return @@ -132,7 +132,9 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): current_moderators = provider.get_group('moderator') if current_moderators is None or not current_moderators.user_set.filter(id=user.id).exists(): log_message(f"User is not a moderator for provider {provider._id} - skipping email") - email_task.status = 'NOT MODERATOR' + email_task.status = 'NOT_MODERATOR' + email_task.save() + return additional_context = {} if isinstance(provider, RegistrationProvider): @@ -254,7 +256,7 @@ def get_moderators_emails(message_freq: str): ) GROUP BY osf_guid._id, (n.event_context ->> 'provider_id') ORDER BY osf_guid._id ASC - """ + """ with connection.cursor() as cursor: cursor.execute(sql, diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index 43d9b23802e..61dbd9f066d 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -1,198 +1,436 @@ import pytest -from unittest import mock - -from django.utils import timezone - -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 -from osf.models.action import RegistrationAction -from osf.utils.notifications import ( - notify_submit, - notify_moderator_registration_requests_withdrawal, - notify_reject_withdraw_request, - notify_withdraw_registration +from django.contrib.contenttypes.models import ContentType + +from osf.models import Notification, NotificationType, EmailTask +from notifications.tasks import ( + send_user_email_task, + send_moderator_email_task, + send_users_digest_email, + send_moderators_digest_email, + get_users_emails, + get_moderators_emails, ) -from osf.utils.workflows import RegistrationModerationTriggers, RegistrationModerationStates - from osf_tests.factories import ( - RegistrationFactory, AuthUserFactory, - RetractionFactory + RegistrationProviderFactory, + RegistrationFactory, ) from tests.utils import capture_notifications -def get_moderator(provider): - user = AuthUserFactory() - provider.add_to_group(user, 'moderator') - return user +def add_notification_subscription( + user, + notification_type, + frequency, + subscribed_object=None, +): + """ + Create a NotificationSubscription for a user. + + `notification_type` may be: + - a NotificationType instance + - a NotificationType.Type enum value + - a raw name string + """ + from osf.models import NotificationSubscription -def get_daily_moderator(provider): - user = AuthUserFactory() - provider.add_to_group(user, 'moderator') - for subscription_type in provider.DEFAULT_SUBSCRIPTIONS: - provider.notification_subscriptions.get(event_name=subscription_type) - return user + if isinstance(notification_type, NotificationType): + nt = notification_type + else: + # enum or name string + nt = NotificationType.objects.get(name=notification_type) + + kwargs = { + 'user': user, + 'notification_type': nt, + 'message_frequency': frequency, + } + + if subscribed_object is not None: + kwargs['object_id'] = subscribed_object.id + kwargs['content_type'] = ContentType.objects.get_for_model(subscribed_object) + + return NotificationSubscription.objects.create(**kwargs) -# Set USE_EMAIL to true and mock out the default mailer for consistency with other mocked settings @pytest.mark.django_db -class TestRegistrationMachineNotification: - - MOCK_NOW = timezone.now() - - @pytest.fixture(autouse=True) - def setup(self): - populate_notification_types() - with mock.patch('osf.utils.machines.timezone.now', return_value=self.MOCK_NOW): - yield - - @pytest.fixture() - def contrib(self): - return AuthUserFactory() - - @pytest.fixture() - def admin(self): - return AuthUserFactory() - - @pytest.fixture() - def registration(self, admin, contrib): - registration = RegistrationFactory(creator=admin) - registration.add_contributor(admin, 'admin') - registration.add_contributor(contrib, 'write') - update_provider_auth_groups() - return registration - - @pytest.fixture() - def registration_with_retraction(self, admin, contrib): - sanction = RetractionFactory(user=admin) - registration = sanction.target_registration - registration.update_moderation_state() - registration.add_contributor(admin, 'admin') - registration.add_contributor(contrib, 'write') - registration.save() - return registration - - @pytest.fixture() - def provider(self, registration): - return registration.provider - - @pytest.fixture() - def moderator(self, provider): +class TestNotificationDigestTasks: + + def test_send_user_email_task_success(self): user = AuthUserFactory() + notification_type = NotificationType.objects.get( + name=NotificationType.Type.USER_FILE_UPDATED + ) + add_notification_subscription( + user, + NotificationType.objects.get(name=NotificationType.Type.FILE_UPDATED), + 'daily', + ) + subscription_type = add_notification_subscription( + user, + notification_type, + 'daily', + subscribed_object=user + ) + + notification = Notification.objects.create( + subscription=subscription_type, + event_context={ + 'source_path': '/', + 'source_node_title': 'test title', + 'source_addon': 'test addon', + 'destination_addon': 'what?', + 'logo': 'test logo', + 'action': 'test action', + 'osf_logo': 'test logo', + 'osf_logo_list': 'osf_logo_list', + 'destination_node_parent_node_title': 'test parent node title', + 'destination_node_title': 'test node title', + }, + ) + user.save() + notification_ids = [notification.id] + with capture_notifications() as notifications: + send_user_email_task.apply(args=(user._id, notification_ids)).get() + assert len(notifications['emits']) == 1 + assert notifications['emits'][0]['type'] == NotificationType.Type.USER_DIGEST + assert notifications['emits'][0]['kwargs']['user'] == user + email_task = EmailTask.objects.get(user_id=user.id) + assert email_task.status == 'SUCCESS' + notification.refresh_from_db() + assert notification.sent + + def test_send_user_email_task_user_not_found(self): + non_existent_user_id = 'fakeuserid' + notification_ids = [] + send_user_email_task.apply(args=(non_existent_user_id, notification_ids)).get() + assert EmailTask.objects.all().exists() + email_task = EmailTask.objects.all().get() + assert email_task.status == 'NO_USER_FOUND' + assert email_task.error_message == 'User not found or disabled' + + def test_send_user_email_task_user_disabled(self): + user = AuthUserFactory() + user.deactivate_account() + user.save() + + # Any subscription is fine; we just need one + subscription = add_notification_subscription( + user, + NotificationType.Type.USER_FILE_UPDATED, + 'daily', + subscribed_object=user, + ) + notification = Notification.objects.create( + subscription=subscription, + sent=None, + event_context={}, + ) + notification_ids = [notification.id] + send_user_email_task.apply(args=(user._id, notification_ids)).get() + email_task = EmailTask.objects.filter(user_id=user.id).first() + assert email_task.status == 'USER_DISABLED' + assert email_task.error_message == 'User not found or disabled' + + def test_send_user_email_task_no_notifications(self): + user = AuthUserFactory() + notification_ids = [] + send_user_email_task.apply(args=(user._id, notification_ids)).get() + email_task = EmailTask.objects.filter(user_id=user.id).first() + assert email_task.status == 'SUCCESS' + + def test_send_moderator_email_task_registration_provider_admin(self): + user = AuthUserFactory(fullname='Admin User') + reg_provider = RegistrationProviderFactory(_id='abc123') + reg = RegistrationFactory(provider=reg_provider) + reg_provider.add_to_group(user, 'moderator') + reg_provider.add_to_group(user, 'admin') + + notification_type = NotificationType.objects.get( + name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS + ) + notification = Notification.objects.create( + subscription=add_notification_subscription( + user, + notification_type, + 'daily', + subscribed_object=reg, + ), + event_context={ + 'profile_image_url': 'http://example.com/profile.png', + 'is_request_email': False, + 'requester_contributor_names': [''], + 'reviews_submission_url': 'http://example.com/reviews_submission.png', + 'message': 'test message', + 'requester_fullname': '', + 'localized_timestamp': 'test timestamp', + }, + sent=None, + ) + notification_ids = [notification.id] + with capture_notifications() as notifications: + send_moderator_email_task.apply( + args=(user._id, notification_ids) + ).get() + assert len(notifications['emits']) == 1 + assert ( + notifications['emits'][0]['type'] + == NotificationType.Type.DIGEST_REVIEWS_MODERATORS + ) + assert notifications['emits'][0]['kwargs']['user'] == user + + email_task = EmailTask.objects.filter(user_id=user.id).first() + assert email_task.status == 'SUCCESS' + notification.refresh_from_db() + assert notification.sent + + def test_send_moderator_email_task_no_notifications(self): + user = AuthUserFactory(fullname='Admin User') + provider = RegistrationProviderFactory() + reg = RegistrationFactory(provider=provider) + + notification_ids = [] + notification_type = NotificationType.objects.get( + name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS + ) + add_notification_subscription( + user, + notification_type, + 'daily', + subscribed_object=reg, + ) + + send_moderator_email_task.apply(args=(user._id, notification_ids)).get() + email_task = EmailTask.objects.filter(user_id=user.id).first() + assert email_task.status == 'SUCCESS' + + def test_send_moderator_email_task_user_not_found(self): + send_moderator_email_task.apply(args=('nouser', [])).get() + email_task = EmailTask.objects.all() + assert email_task.exists() + assert email_task.first().status == 'NO_USER_FOUND' + + def test_get_users_emails(self): + user = AuthUserFactory() + notification_type = NotificationType.objects.get( + name=NotificationType.Type.USER_DIGEST + ) + notification1 = Notification.objects.create( + subscription=add_notification_subscription( + user, + notification_type, + 'daily', + subscribed_object=user, + ), + sent=None, + event_context={}, + ) + res = list(get_users_emails('daily')) + assert len(res) == 1 + user_info = res[0] + assert user_info['user_id'] == user._id + assert any(msg['notification_id'] == notification1.id for msg in user_info['info']) + + def test_get_moderators_emails(self): + user = AuthUserFactory() + provider = RegistrationProviderFactory() + reg = RegistrationFactory(provider=provider) + notification_type = NotificationType.objects.get( + name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS + ) + subscription = add_notification_subscription( + user, + notification_type, + 'daily', + subscribed_object=reg, + ) + Notification.objects.create( + subscription=subscription, + event_context={}, + sent=None, + ) provider.add_to_group(user, 'moderator') - return user - @pytest.fixture() - def daily_moderator(self, provider): + res = list(get_moderators_emails('daily')) + assert len(res) >= 1 + entry = [ + x + for x in res + if x['user_id'] == user._id + and subscription.subscribed_object.id == reg.id + ] + assert entry, 'Expected moderator digest group' + + def test_send_users_digest_email_end_to_end(self): user = AuthUserFactory() + notification_type = NotificationType.objects.get( + name=NotificationType.Type.USER_FILE_UPDATED + ) + add_notification_subscription( + user, + NotificationType.objects.get(name=NotificationType.Type.FILE_UPDATED), + 'daily', + ) + subscription_type = add_notification_subscription( + user, + notification_type, + 'daily', + subscribed_object=user, + ) + + Notification.objects.create( + subscription=subscription_type, + event_context={ + 'source_path': '/', + 'requester_fullname': '', + 'source_node_title': 'test title', + 'source_addon': 'test addon', + 'destination_addon': 'what?', + 'logo': 'test logo', + 'requester_contributor_names': [''], + 'action': 'test action', + 'osf_logo': 'test logo', + 'osf_logo_list': 'osf_logo_list', + 'profile_image_url': 'http://example.com/profile.png', + 'destination_node_parent_node_title': 'test parent node title', + 'destination_node_title': 'test node title', + 'nessage': 'test message', + 'localized_timestamp': 'test timestamp', + }, + ) + user.save() + with capture_notifications() as notifications: + # call task synchronously for tests + send_users_digest_email.apply(kwargs={'dry_run': False}).get() + assert len(notifications['emits']) == 1 + assert notifications['emits'][0]['type'] == NotificationType.Type.USER_DIGEST + email_task = EmailTask.objects.get(user_id=user.id) + assert email_task.status == 'SUCCESS' + + def test_send_moderators_digest_email_end_to_end(self): + user = AuthUserFactory() + provider = RegistrationProviderFactory() provider.add_to_group(user, 'moderator') - return user - - @pytest.fixture() - def accept_action(self, registration, admin): - registration_action = RegistrationAction.objects.create( - creator=admin, - target=registration, - trigger=RegistrationModerationTriggers.ACCEPT_SUBMISSION.db_name, - from_state=RegistrationModerationStates.INITIAL.db_name, - to_state=RegistrationModerationStates.ACCEPTED.db_name, - comment='yo' - ) - return registration_action - - @pytest.fixture() - def withdraw_request_action(self, registration, admin): - registration_action = RegistrationAction.objects.create( - creator=admin, - target=registration, - trigger=RegistrationModerationTriggers.REQUEST_WITHDRAWAL.db_name, - from_state=RegistrationModerationStates.ACCEPTED.db_name, - to_state=RegistrationModerationStates.PENDING_WITHDRAW.db_name, - comment='yo' - ) - return registration_action - - @pytest.fixture() - def withdraw_action(self, registration, admin): - registration_action = RegistrationAction.objects.create( - creator=admin, - target=registration, - trigger=RegistrationModerationTriggers.ACCEPT_WITHDRAWAL.db_name, - from_state=RegistrationModerationStates.PENDING_WITHDRAW.db_name, - to_state=RegistrationModerationStates.WITHDRAWN.db_name, - comment='yo' - ) - return registration_action - - def test_submit_notifications(self, registration, moderator, admin, contrib, provider): - """ - [REQS-96] "As moderator of branded registry, I receive email notification upon admin author(s) submission approval" - """ - with capture_notifications() as notification: - notify_submit(registration, admin) - - assert len(notification['emits']) == 3 - assert notification['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION - assert notification['emits'][0]['kwargs']['user'] == admin - assert notification['emits'][1]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION - assert notification['emits'][1]['kwargs']['user'] == contrib - assert notification['emits'][2]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS - - assert NotificationSubscription.objects.count() == 5 - digest = NotificationSubscription.objects.last() - assert digest.user == moderator - - def test_withdrawal_registration_accepted_notifications( - self, registration_with_retraction, contrib, admin, withdraw_action - ): - """ - [REQS-109] Authors receive notification when withdrawal is accepted. - Compare recipients by user objects via captured emits. - """ - with capture_notifications() as notification: - notify_withdraw_registration(registration_with_retraction, withdraw_action) - recipients = {rec['kwargs']['user'] for rec in notification['emits'] if 'user' in rec['kwargs']} - assert {admin, contrib}.issubset(recipients) + reg = RegistrationFactory(provider=provider) + notification_type = NotificationType.objects.get( + name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS + ) + Notification.objects.create( + subscription=add_notification_subscription( + user, + notification_type, + 'daily', + subscribed_object=reg, + ), + sent=None, + event_context={ + 'submitter_fullname': 'submitter_fullname', + 'requester_fullname': 'requester_fullname', + 'requester_contributor_names': 'requester_contributor_names', + 'localized_timestamp': '2024-01-01T00:00:00Z', + 'message': 'submitted title.', + 'reviews_submission_url': 'reviews_submission_url', + 'is_request_email': False, + 'is_initiator': False, + 'profile_image_url': 'profile_image_url', + }, + ) + with capture_notifications() as notifications: + send_moderators_digest_email.apply(kwargs={'dry_run': False}).get() + assert len(notifications['emits']) == 1 + assert ( + notifications['emits'][0]['type'] + == NotificationType.Type.DIGEST_REVIEWS_MODERATORS + ) + email_task = EmailTask.objects.filter(user_id=user.id).first() + assert email_task.status == 'SUCCESS' - def test_withdrawal_registration_rejected_notifications( - self, registration, contrib, admin, withdraw_request_action - ): + def test_send_users_digest_email_batches_multiple_notifications(self): """ - [REQS-109] Authors receive notification when withdrawal is rejected. - Compare recipients by user objects via captured emits. + Regression: multiple notifications for same user/frequency + MUST result in exactly one USER_DIGEST emit. """ - with capture_notifications() as notification: - notify_reject_withdraw_request(registration, withdraw_request_action) - - recipients = {rec['kwargs']['user'] for rec in notification['emits'] if 'user' in rec['kwargs']} - assert {admin, contrib}.issubset(recipients) + user = AuthUserFactory() + inner_sub = add_notification_subscription( + user, + NotificationType.Type.FILE_UPDATED.instance, + 'daily', + subscribed_object=user, + ) + add_notification_subscription( + user, + NotificationType.Type.USER_FILE_UPDATED.instance, + 'daily', + subscribed_object=user, + ) - def test_withdrawal_registration_force_notifications( - self, registration_with_retraction, contrib, admin, withdraw_action - ): + for i in range(5): + inner_sub.emit( + event_context={ + 'profile_image_url': 'http://example.com/profile.png', + 'localized_timestamp': 'time', + 'message': 'test message', + 'url': 'http://example.com', + 'user_fullname': '', + }, + ) + + user.save() + with capture_notifications() as notifications: + send_users_digest_email.apply(kwargs={'dry_run': False}).get() + + assert len(notifications['emits']) == 1 + emit = notifications['emits'][0] + assert emit['type'] == NotificationType.Type.USER_DIGEST + # Optional: make sure all 5 notifications were in the digest payload + event_context = emit['kwargs'].get('event_context') or {} + notifications_ctx = event_context.get('notifications') or [] + assert len(notifications_ctx) == 5 + + def test_send_moderators_digest_email_batches_multiple_notifications(self): """ - [REQS-109] Forced withdrawal route: compare recipients by user objects via captured emits. + Regression: multiple notifications for same moderator+provider + MUST result in exactly one DIGEST_REVIEWS_MODERATORS emit. """ - with capture_notifications() as notification: - notify_withdraw_registration(registration_with_retraction, withdraw_action) - - recipients = {rec['kwargs']['user'] for rec in notification['emits'] if 'user' in rec['kwargs']} - assert {admin, contrib}.issubset(recipients) - - def test_moderator_digest_emails_render(self, registration, admin, moderator): - with capture_notifications(): - notify_moderator_registration_requests_withdrawal(registration, admin) - send_users_digest_email() + user = AuthUserFactory() + provider = RegistrationProviderFactory() + reg = RegistrationFactory(provider=provider) + notification_type = NotificationType.objects.get( + name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS + ) + provider.add_to_group(user, 'moderator') - def test_branded_provider_notification_renders(self, registration, admin, moderator): - provider = registration.provider - provider.brand = Brand.objects.create(hero_logo_image='not-a-url', primary_color='#FFA500') - provider.name = 'Test Provider' - provider.save() + subscription = add_notification_subscription( + user, + notification_type, + 'daily', + subscribed_object=reg, + ) - with capture_notifications(): - notify_submit(registration, admin) + for i in range(4): + subscription.emit( + event_context={ + 'submitter_fullname': 'submitter_fullname', + 'requester_fullname': 'requester_fullname', + 'requester_contributor_names': 'requester_contributor_names', + 'localized_timestamp': '2024-01-01T00:00:00Z', + 'message': 'submitted title.', + 'reviews_submission_url': 'reviews_submission_url', + 'is_request_email': False, + 'is_initiator': False, + 'profile_image_url': 'profile_image_url', + }, + ) + + with capture_notifications() as notifications: + send_moderators_digest_email.apply(kwargs={'dry_run': False}).get() + + assert len(notifications['emits']) == 1 + emit = notifications['emits'][0] + assert ( + emit['type'] == NotificationType.Type.DIGEST_REVIEWS_MODERATORS + ) + event_context = emit['kwargs'].get('event_context') or {} + notifications_ctx = event_context.get('notifications') or [] + assert len(notifications_ctx) == 4 diff --git a/website/reviews/listeners.py b/website/reviews/listeners.py index caa88491db0..5a1c902dbbb 100644 --- a/website/reviews/listeners.py +++ b/website/reviews/listeners.py @@ -38,7 +38,7 @@ def reviews_withdraw_requests_notification_moderators(self, timestamp, context, NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.instance.emit( user=recipient, - subscribed_object=provider, + subscribed_object=resource, event_context=context, is_digest=True, ) @@ -63,7 +63,7 @@ def reviews_withdrawal_requests_notification(self, timestamp, context): NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.instance.emit( user=recipient, event_context=context, - subscribed_object=preprint.provider, + subscribed_object=preprint, is_digest=True, ) @@ -112,7 +112,7 @@ def reviews_submit_notification_moderators(self, timestamp, resource, context): NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance.emit( user=recipient, - subscribed_object=provider, + subscribed_object=resource, event_context=context, is_digest=True, ) @@ -147,6 +147,6 @@ def reviews_submit_notification(self, recipients, context, resource, notificatio context['user_fullname'] = recipient.username notification_type.instance.emit( user=recipient, - subscribed_object=provider, + subscribed_object=resource, event_context=context, ) From 51f427cd2de1bbd3342872e7ab8bba8fc5f6808e Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 14:07:43 -0500 Subject: [PATCH 2/8] add config changes for testing reasons --- website/settings/defaults.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/settings/defaults.py b/website/settings/defaults.py index 4de7bd67178..35147342ddf 100644 --- a/website/settings/defaults.py +++ b/website/settings/defaults.py @@ -619,7 +619,7 @@ class CeleryConfig: 'triggered_mails': { 'task': 'scripts.triggered_mails', 'schedule': crontab(minute=0, hour=5), # Daily 12 a.m - 'kwargs': {'dry_run': False}, + 'kwargs': {'dry_run': True}, # For no_login messages }, '5-minute-user-emails': { 'task': 'notifications.tasks.send_users_instant_digest_email', @@ -633,12 +633,12 @@ class CeleryConfig: }, 'send_moderators_digest_email': { 'task': 'notifications.tasks.send_moderators_digest_email', - 'schedule': crontab(minute=0, hour=5), # Daily 12 a.m + 'schedule': crontab(minute='*/10'), # Daily 12 a.m (ten minutes for testing purposes) 'kwargs': {'dry_run': False}, }, 'send_users_digest_email': { 'task': 'notifications.tasks.send_users_digest_email', - 'schedule': crontab(minute=0, hour=5), # Daily 12 a.m + 'schedule': crontab(minute='*/10'), # Daily 12 a.m (ten minutes for testing purposes) 'kwargs': {'dry_run': False}, }, 'clear_expired_sessions': { From 797505126d7bf7d48473bd4ebd100b31b329dac0 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 14:52:15 -0500 Subject: [PATCH 3/8] fix up tests and digest logic --- .../notifications/test_notification_digest.py | 33 +++++++++------- notifications/tasks.py | 2 + osf/models/notification_subscription.py | 11 +++--- ...t_registration_moderation_notifications.py | 39 ++++++++----------- 4 files changed, 41 insertions(+), 44 deletions(-) diff --git a/api_tests/notifications/test_notification_digest.py b/api_tests/notifications/test_notification_digest.py index 07863cad939..1bbc60c66c7 100644 --- a/api_tests/notifications/test_notification_digest.py +++ b/api_tests/notifications/test_notification_digest.py @@ -113,16 +113,14 @@ def test_send_moderator_email_task_registration_provider_admin(self): user = AuthUserFactory(fullname='Admin User') reg_provider = RegistrationProviderFactory(_id='abc123') reg = RegistrationFactory(provider=reg_provider) - admin_group = reg_provider.get_group('admin') - admin_group.user_set.add(user) - notification_type = NotificationType.objects.get(name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS) - notification = Notification.objects.create( - subscription=add_notification_subscription( - user, - notification_type, - 'daily', - subscribed_object=reg - ), + reg_provider.add_to_group(user, 'admin') + reg_provider.add_to_group(user, 'moderator') + add_notification_subscription( + user, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, + 'daily', + subscribed_object=reg + ).emit( event_context={ 'profile_image_url': 'http://example.com/profile.png', 'is_request_email': False, @@ -132,8 +130,8 @@ def test_send_moderator_email_task_registration_provider_admin(self): 'requester_fullname': '', 'localized_timestamp': 'test timestamp', }, - sent=None, ) + notification = Notification.objects.get() notification_ids = [notification.id] with capture_notifications() as notifications: send_moderator_email_task.apply(args=(user._id, notification_ids)).get() @@ -248,10 +246,15 @@ def test_send_moderators_digest_email_end_to_end(self): user = AuthUserFactory() provider = RegistrationProviderFactory() reg = RegistrationFactory(provider=provider) - notification_type = NotificationType.objects.get(name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS) - Notification.objects.create( - subscription=add_notification_subscription(user, notification_type, 'daily', subscribed_object=reg), - sent=None, + provider.add_to_group(user, 'admin') + provider.add_to_group(user, 'moderator') + + add_notification_subscription( + user, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, + 'daily', + subscribed_object=reg + ).emit( event_context={ 'submitter_fullname': 'submitter_fullname', 'requester_fullname': 'requester_fullname', diff --git a/notifications/tasks.py b/notifications/tasks.py index 9dd64f6be74..ad426fdaf38 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -71,6 +71,7 @@ def send_user_email_task(self, user_id, notification_ids, **kwargs): NotificationType.Type.USER_DIGEST.instance.emit( user=user, event_context=event_context, + message_frequency='instantly' ) notifications_qs.update(sent=timezone.now()) @@ -179,6 +180,7 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): user=user, subscribed_object=subscribed_object, event_context=event_context, + message_frequency='instantly' ) notifications_qs.update(sent=timezone.now()) diff --git a/osf/models/notification_subscription.py b/osf/models/notification_subscription.py index 25f14e8ec76..2a9c656ea8c 100644 --- a/osf/models/notification_subscription.py +++ b/osf/models/notification_subscription.py @@ -94,12 +94,11 @@ def emit( ) if save: notification.save() - if not self._is_digest: # instant digests are sent every 5 minutes. - notification.send( - destination_address=destination_address, - email_context=email_context, - save=save, - ) + notification.send( + destination_address=destination_address, + email_context=email_context, + save=save, + ) else: Notification.objects.create( subscription=self, diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index 61dbd9f066d..838829604cd 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -146,16 +146,14 @@ def test_send_moderator_email_task_registration_provider_admin(self): reg_provider.add_to_group(user, 'moderator') reg_provider.add_to_group(user, 'admin') - notification_type = NotificationType.objects.get( - name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS - ) - notification = Notification.objects.create( - subscription=add_notification_subscription( - user, - notification_type, - 'daily', - subscribed_object=reg, - ), + add_notification_subscription( + user, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, + 'daily', + subscribed_object=reg, + ).emit( + user, + subscribed_object=reg, event_context={ 'profile_image_url': 'http://example.com/profile.png', 'is_request_email': False, @@ -165,8 +163,8 @@ def test_send_moderator_email_task_registration_provider_admin(self): 'requester_fullname': '', 'localized_timestamp': 'test timestamp', }, - sent=None, ) + notification = Notification.objects.get() notification_ids = [notification.id] with capture_notifications() as notifications: send_moderator_email_task.apply( @@ -311,19 +309,14 @@ def test_send_moderators_digest_email_end_to_end(self): user = AuthUserFactory() provider = RegistrationProviderFactory() provider.add_to_group(user, 'moderator') - reg = RegistrationFactory(provider=provider) - notification_type = NotificationType.objects.get( - name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS - ) - Notification.objects.create( - subscription=add_notification_subscription( - user, - notification_type, - 'daily', - subscribed_object=reg, - ), - sent=None, + + add_notification_subscription( + user, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, + 'daily', + subscribed_object=reg, + ).emit( event_context={ 'submitter_fullname': 'submitter_fullname', 'requester_fullname': 'requester_fullname', From c71ee937f611e95df2dae1d9c7520a152026110d Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 15:14:59 -0500 Subject: [PATCH 4/8] fix tests and digest code queueing --- osf/models/notification_subscription.py | 11 ++++++----- .../test_registration_moderation_notifications.py | 1 - 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osf/models/notification_subscription.py b/osf/models/notification_subscription.py index 2a9c656ea8c..25f14e8ec76 100644 --- a/osf/models/notification_subscription.py +++ b/osf/models/notification_subscription.py @@ -94,11 +94,12 @@ def emit( ) if save: notification.save() - notification.send( - destination_address=destination_address, - email_context=email_context, - save=save, - ) + if not self._is_digest: # instant digests are sent every 5 minutes. + notification.send( + destination_address=destination_address, + email_context=email_context, + save=save, + ) else: Notification.objects.create( subscription=self, diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index 838829604cd..cac17f2a12a 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -153,7 +153,6 @@ def test_send_moderator_email_task_registration_provider_admin(self): subscribed_object=reg, ).emit( user, - subscribed_object=reg, event_context={ 'profile_image_url': 'http://example.com/profile.png', 'is_request_email': False, From ba7e6ae5f59a5d3bdc32a15a61ffafa4caf00152 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 16:37:27 -0500 Subject: [PATCH 5/8] fix more tests for notification digest --- osf_tests/test_registration_moderation_notifications.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index cac17f2a12a..0e7bbbbbdb1 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -73,8 +73,7 @@ def test_send_user_email_task_success(self): subscribed_object=user ) - notification = Notification.objects.create( - subscription=subscription_type, + subscription_type.emits( event_context={ 'source_path': '/', 'source_node_title': 'test title', @@ -89,6 +88,7 @@ def test_send_user_email_task_success(self): }, ) user.save() + notification = Notification.objects.get() notification_ids = [notification.id] with capture_notifications() as notifications: send_user_email_task.apply(args=(user._id, notification_ids)).get() @@ -152,7 +152,6 @@ def test_send_moderator_email_task_registration_provider_admin(self): 'daily', subscribed_object=reg, ).emit( - user, event_context={ 'profile_image_url': 'http://example.com/profile.png', 'is_request_email': False, From c4de04c73eaadbd553f404a9dbbbee6177c66032 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 16:38:15 -0500 Subject: [PATCH 6/8] fix another notification digest test --- osf_tests/test_registration_moderation_notifications.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index 0e7bbbbbdb1..ac00f81f73b 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -73,7 +73,7 @@ def test_send_user_email_task_success(self): subscribed_object=user ) - subscription_type.emits( + subscription_type.emit( event_context={ 'source_path': '/', 'source_node_title': 'test title', From a40430d66e9eeccc672ba3e80d9d21bba938d7cb Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Fri, 14 Nov 2025 08:26:53 -0500 Subject: [PATCH 7/8] change status to error message to preserve RETRY status --- notifications/tasks.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/notifications/tasks.py b/notifications/tasks.py index ad426fdaf38..0d3f8a6ff8c 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -119,21 +119,25 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): if subscribed_object is None: log_message(f"subscribed_object fpr {subscribed_object} does not exist") - email_task.status = 'OBJECT NOT FOUND' + email_task.error_message = 'subscribed object not found' + email_task.status = 'FAILURE' email_task.save() return provider = getattr(subscribed_object, 'provider', None) if provider is None: log_message(f"provider for subscribed_object {subscribed_object} does not exist") - email_task.status = 'PROVIDER NOT FOUND' + email_task.error_message = 'provider not found' + email_task.status = 'FAILURE' + email_task.save() return current_moderators = provider.get_group('moderator') if current_moderators is None or not current_moderators.user_set.filter(id=user.id).exists(): log_message(f"User is not a moderator for provider {provider._id} - skipping email") - email_task.status = 'NOT_MODERATOR' + email_task.error_message = 'NOT MODERATOR' + email_task.status = 'FAILURE' email_task.save() return From fd2e388c046034cebfb24bad6f1ca15833a56f38 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Fri, 14 Nov 2025 10:04:46 -0500 Subject: [PATCH 8/8] update code to work better with subscription detail and FE --- api/subscriptions/serializers.py | 42 ++++++- .../notifications/test_notification_digest.py | 60 +++++----- notifications/tasks.py | 12 +- ...t_registration_moderation_notifications.py | 110 +++++++----------- website/reviews/listeners.py | 7 ++ 5 files changed, 126 insertions(+), 105 deletions(-) diff --git a/api/subscriptions/serializers.py b/api/subscriptions/serializers.py index 43efa38f261..9626f33a7ea 100644 --- a/api/subscriptions/serializers.py +++ b/api/subscriptions/serializers.py @@ -2,6 +2,7 @@ from api.nodes.serializers import RegistrationProviderRelationshipField from api.collections_providers.fields import CollectionProviderRelationshipField from api.preprints.serializers import PreprintProviderRelationshipField +from osf.models import NotificationType, NotificationSubscription from website.util import api_v2_url @@ -38,10 +39,43 @@ def get_absolute_url(self, obj): def update(self, instance, validated_data): freq = validated_data.get('message_frequency') - if freq is None: - freq = validated_data.get('frequency') - instance.message_frequency = freq - instance.save() + if instance.name in ( + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS, + NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS, + ): + NotificationSubscription.get_or_create( # Legacy subscription keeps global settings + user=instance.user, + notification_type=NotificationType.Type.REVIEWS_SUBMISSION_STATUS.instance, + defaults={ + 'message_frequency': freq, + }, + ) + NotificationSubscription.objects.filter( + user=instance.user, + notification_type__in=[ + NotificationType.Type.REVIEWS_SUBMISSION_STATUS.instance, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, + ], + ).update(frequency=freq) + elif instance.name == NotificationType.Type.USER_FILE_UPDATED: + NotificationSubscription.objects.filter( + user=instance.user, + notification_type__in=[ + NotificationType.Type.USER_FILE_UPDATED, + NotificationType.Type.ADDON_FILE_RENAMED.instance, + NotificationType.Type.ADDON_FILE_COPIED.instance, + NotificationType.Type.FILE_ADDED.instance, + NotificationType.Type.ADDON_FILE_MOVED.instance, + NotificationType.Type.FILE_REMOVED.instance, + NotificationType.Type.FILE_UPDATED.instance, + NotificationType.Type.FOLDER_CREATED.instance, + ], + ).update(frequency=freq) + else: + if freq is None: + freq = validated_data.get('frequency') + instance.message_frequency = freq + instance.save() return instance diff --git a/api_tests/notifications/test_notification_digest.py b/api_tests/notifications/test_notification_digest.py index 1bbc60c66c7..9c5738dcf5c 100644 --- a/api_tests/notifications/test_notification_digest.py +++ b/api_tests/notifications/test_notification_digest.py @@ -170,28 +170,45 @@ def test_send_moderator_email_task_user_not_found(self): def test_get_users_emails(self): user = AuthUserFactory() - notification_type = NotificationType.objects.get(name=NotificationType.Type.USER_DIGEST) - notification1 = Notification.objects.create( - subscription=add_notification_subscription(user, notification_type, 'daily'), - sent=None, - event_context={}, + add_notification_subscription( + user, + NotificationType.Type.FILE_UPDATED, + 'daily' + ).emit( + event_context={ + 'source_path': '/', + 'requester_fullname': '', + 'source_node_title': 'test title', + 'source_addon': 'test addon', + 'destination_addon': 'what?', + 'logo': 'test logo', + 'requester_contributor_names': [''], + 'action': 'test action', + 'osf_logo': 'test logo', + 'osf_logo_list': 'osf_logo_list', + 'profile_image_url': 'http://example.com/profile.png', + 'destination_node_parent_node_title': 'test parent node title', + 'destination_node_title': 'test node title', + 'nessage': 'test message', + 'localized_timestamp': 'test timestamp', + }, ) res = list(get_users_emails('daily')) assert len(res) == 1 user_info = res[0] assert user_info['user_id'] == user._id - assert any(msg['notification_id'] == notification1.id for msg in user_info['info']) + notification = Notification.objects.get() + assert any(msg['notification_id'] == notification.id for msg in user_info['info']) def test_get_moderators_emails(self): user = AuthUserFactory() provider = RegistrationProviderFactory() reg = RegistrationFactory(provider=provider) - notification_type = NotificationType.objects.get(name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS) - subscription = add_notification_subscription(user, notification_type, 'daily', subscribed_object=reg) - Notification.objects.create( - subscription=subscription, - event_context={}, - sent=None + subscription = add_notification_subscription( + user, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, + 'daily', + subscribed_object=reg ) res = list(get_moderators_emails('daily')) assert len(res) >= 1 @@ -202,20 +219,11 @@ def test_get_moderators_emails(self): def test_send_users_digest_email_end_to_end(self): user = AuthUserFactory() - notification_type = NotificationType.objects.get(name=NotificationType.Type.USER_FILE_UPDATED) - subscription_type = add_notification_subscription( + add_notification_subscription( user, - notification_type, + NotificationType.Type.FILE_UPDATED.instance, 'daily', - subscription=add_notification_subscription( - user, - NotificationType.objects.get(name=NotificationType.Type.FILE_UPDATED), - 'daily' - ) - ) - - Notification.objects.create( - subscription=subscription_type, + ).emit( event_context={ 'source_path': '/', 'requester_fullname': '', @@ -230,13 +238,13 @@ def test_send_users_digest_email_end_to_end(self): 'profile_image_url': 'http://example.com/profile.png', 'destination_node_parent_node_title': 'test parent node title', 'destination_node_title': 'test node title', - 'nessage': 'test message', + 'message': 'test message', 'localized_timestamp': 'test timestamp', }, ) user.save() with capture_notifications() as notifications: - send_users_digest_email.delay() + send_users_digest_email.apply(kwargs={'dry_run': False}).get() assert len(notifications['emits']) == 1 assert notifications['emits'][0]['type'] == NotificationType.Type.USER_DIGEST email_task = EmailTask.objects.get(user_id=user.id) diff --git a/notifications/tasks.py b/notifications/tasks.py index 0d3f8a6ff8c..9d55b454f30 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -129,7 +129,6 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): log_message(f"provider for subscribed_object {subscribed_object} does not exist") email_task.error_message = 'provider not found' email_task.status = 'FAILURE' - email_task.save() return @@ -295,7 +294,7 @@ def get_users_emails(message_freq): LEFT JOIN osf_guid ON ns.user_id = osf_guid.object_id WHERE n.sent IS NULL AND ns.message_frequency = %s - AND nt.name NOT IN (%s, %s) + AND nt.name IN (%s, %s, %s, %s, %s, %s, %s) AND osf_guid.content_type_id = ( SELECT id FROM django_content_type WHERE model = 'osfuser' ) @@ -307,8 +306,13 @@ def get_users_emails(message_freq): cursor.execute(sql, [ message_freq, - NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value, - NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value + NotificationType.Type.ADDON_FILE_RENAMED, + NotificationType.Type.ADDON_FILE_COPIED, + NotificationType.Type.FILE_ADDED, + NotificationType.Type.ADDON_FILE_MOVED, + NotificationType.Type.FILE_REMOVED, + NotificationType.Type.FILE_UPDATED, + NotificationType.Type.FOLDER_CREATED, ] ) return itertools.chain.from_iterable(cursor.fetchall()) diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index ac00f81f73b..6e368ba8939 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -12,8 +12,8 @@ ) from osf_tests.factories import ( AuthUserFactory, - RegistrationProviderFactory, - RegistrationFactory, + PreprintProviderFactory, + PreprintFactory, ) from tests.utils import capture_notifications @@ -58,21 +58,11 @@ class TestNotificationDigestTasks: def test_send_user_email_task_success(self): user = AuthUserFactory() - notification_type = NotificationType.objects.get( - name=NotificationType.Type.USER_FILE_UPDATED - ) - add_notification_subscription( - user, - NotificationType.objects.get(name=NotificationType.Type.FILE_UPDATED), - 'daily', - ) subscription_type = add_notification_subscription( user, - notification_type, + NotificationType.Type.USER_FILE_UPDATED.instance, 'daily', - subscribed_object=user ) - subscription_type.emit( event_context={ 'source_path': '/', @@ -141,8 +131,8 @@ def test_send_user_email_task_no_notifications(self): def test_send_moderator_email_task_registration_provider_admin(self): user = AuthUserFactory(fullname='Admin User') - reg_provider = RegistrationProviderFactory(_id='abc123') - reg = RegistrationFactory(provider=reg_provider) + reg_provider = PreprintProviderFactory(_id='abc123') + preprint = PreprintFactory(provider=reg_provider) reg_provider.add_to_group(user, 'moderator') reg_provider.add_to_group(user, 'admin') @@ -150,7 +140,7 @@ def test_send_moderator_email_task_registration_provider_admin(self): user, NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, 'daily', - subscribed_object=reg, + subscribed_object=preprint, ).emit( event_context={ 'profile_image_url': 'http://example.com/profile.png', @@ -182,18 +172,15 @@ def test_send_moderator_email_task_registration_provider_admin(self): def test_send_moderator_email_task_no_notifications(self): user = AuthUserFactory(fullname='Admin User') - provider = RegistrationProviderFactory() - reg = RegistrationFactory(provider=provider) + provider = PreprintProviderFactory() + preprint = PreprintFactory(provider=provider) notification_ids = [] - notification_type = NotificationType.objects.get( - name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS - ) add_notification_subscription( user, - notification_type, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, 'daily', - subscribed_object=reg, + subscribed_object=preprint, ) send_moderator_email_task.apply(args=(user._id, notification_ids)).get() @@ -208,29 +195,26 @@ def test_send_moderator_email_task_user_not_found(self): def test_get_users_emails(self): user = AuthUserFactory() - notification_type = NotificationType.objects.get( - name=NotificationType.Type.USER_DIGEST - ) - notification1 = Notification.objects.create( - subscription=add_notification_subscription( - user, - notification_type, - 'daily', - subscribed_object=user, - ), - sent=None, - event_context={}, + add_notification_subscription( + user, + NotificationType.Type.FILE_REMOVED.instance, + 'daily', + subscribed_object=user, + ).emit( + event_context={} ) res = list(get_users_emails('daily')) assert len(res) == 1 user_info = res[0] assert user_info['user_id'] == user._id - assert any(msg['notification_id'] == notification1.id for msg in user_info['info']) + notification = Notification.objects.get() + assert any(msg['notification_id'] == notification.id for msg in user_info['info']) def test_get_moderators_emails(self): user = AuthUserFactory() - provider = RegistrationProviderFactory() - reg = RegistrationFactory(provider=provider) + provider = PreprintProviderFactory() + preprint = PreprintFactory(provider=provider) + provider.add_to_group(user, 'moderator') notification_type = NotificationType.objects.get( name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS ) @@ -238,14 +222,12 @@ def test_get_moderators_emails(self): user, notification_type, 'daily', - subscribed_object=reg, + subscribed_object=preprint, ) - Notification.objects.create( - subscription=subscription, - event_context={}, - sent=None, + + subscription.emit( + event_context={} ) - provider.add_to_group(user, 'moderator') res = list(get_moderators_emails('daily')) assert len(res) >= 1 @@ -253,29 +235,17 @@ def test_get_moderators_emails(self): x for x in res if x['user_id'] == user._id - and subscription.subscribed_object.id == reg.id + and subscription.subscribed_object.id == preprint.id ] assert entry, 'Expected moderator digest group' def test_send_users_digest_email_end_to_end(self): user = AuthUserFactory() - notification_type = NotificationType.objects.get( - name=NotificationType.Type.USER_FILE_UPDATED - ) add_notification_subscription( user, - NotificationType.objects.get(name=NotificationType.Type.FILE_UPDATED), - 'daily', - ) - subscription_type = add_notification_subscription( - user, - notification_type, + NotificationType.Type.FILE_ADDED.instance, 'daily', - subscribed_object=user, - ) - - Notification.objects.create( - subscription=subscription_type, + ).emit( event_context={ 'source_path': '/', 'requester_fullname': '', @@ -290,8 +260,10 @@ def test_send_users_digest_email_end_to_end(self): 'profile_image_url': 'http://example.com/profile.png', 'destination_node_parent_node_title': 'test parent node title', 'destination_node_title': 'test node title', - 'nessage': 'test message', + 'message': 'test message', + 'user_fullname': 'user fullname', 'localized_timestamp': 'test timestamp', + 'url': 'test url', }, ) user.save() @@ -305,15 +277,14 @@ def test_send_users_digest_email_end_to_end(self): def test_send_moderators_digest_email_end_to_end(self): user = AuthUserFactory() - provider = RegistrationProviderFactory() + provider = PreprintProviderFactory() provider.add_to_group(user, 'moderator') - reg = RegistrationFactory(provider=provider) - + preprint = PreprintFactory(provider=provider) add_notification_subscription( user, NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, 'daily', - subscribed_object=reg, + subscribed_object=preprint, ).emit( event_context={ 'submitter_fullname': 'submitter_fullname', @@ -385,18 +356,15 @@ def test_send_moderators_digest_email_batches_multiple_notifications(self): MUST result in exactly one DIGEST_REVIEWS_MODERATORS emit. """ user = AuthUserFactory() - provider = RegistrationProviderFactory() - reg = RegistrationFactory(provider=provider) - notification_type = NotificationType.objects.get( - name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS - ) + provider = PreprintProviderFactory() + preprint = PreprintFactory(provider=provider) provider.add_to_group(user, 'moderator') subscription = add_notification_subscription( user, - notification_type, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, 'daily', - subscribed_object=reg, + subscribed_object=preprint, ) for i in range(4): diff --git a/website/reviews/listeners.py b/website/reviews/listeners.py index 5a1c902dbbb..1500cd94094 100644 --- a/website/reviews/listeners.py +++ b/website/reviews/listeners.py @@ -1,5 +1,6 @@ from django.utils import timezone +from osf.models import NotificationSubscription from website.settings import DOMAIN, OSF_PREPRINTS_LOGO, OSF_REGISTRIES_LOGO from osf.utils.permissions import ADMIN from website.reviews import signals as reviews_signals @@ -110,11 +111,17 @@ def reviews_submit_notification_moderators(self, timestamp, resource, context): context['requester_fullname'] = recipient.fullname context['is_request_email'] = False + freq_setting, created = NotificationSubscription.instance.get_or_create( + user=recipient, + notification_type=NotificationType.Type.REVIEWS_SUBMISSION_STATUS.instance, + ) + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance.emit( user=recipient, subscribed_object=resource, event_context=context, is_digest=True, + message_frequency=freq_setting.frequency, )