From 25ce6c26c6ff34a40d72c6dd30f466fed3a44ff5 Mon Sep 17 00:00:00 2001 From: Ostap Zherebetskyi Date: Thu, 27 Nov 2025 13:26:39 +0200 Subject: [PATCH 1/2] fix: refactor notification subscriptions and digest handling --- api/subscriptions/views.py | 86 ++++++++++--- notifications/file_event_notifications.py | 1 + notifications/listeners.py | 58 +++++---- notifications/tasks.py | 149 ++++++++++++++++------ osf/models/collection_submission.py | 2 +- osf/models/mixins.py | 17 ++- osf/models/notification_subscription.py | 31 ++++- osf/models/notification_type.py | 119 +++++++++++++---- osf/utils/notifications.py | 2 +- website/reviews/listeners.py | 3 +- 10 files changed, 343 insertions(+), 125 deletions(-) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index c1cd36186e1..d7dfb944ecb 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -1,4 +1,4 @@ -from django.db.models import Value, When, Case, Q, OuterRef, Subquery +from django.db.models import Value, When, Case, OuterRef, Subquery from django.db.models.fields import CharField, IntegerField from django.db.models.functions import Concat, Cast from django.contrib.contenttypes.models import ContentType @@ -24,6 +24,7 @@ RegistrationProvider, AbstractProvider, AbstractNode, + Guid, ) from osf.models.notification_type import NotificationType from osf.models.notification_subscription import NotificationSubscription @@ -44,47 +45,72 @@ class SubscriptionList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin): def get_queryset(self): user_guid = self.request.user._id - provider_ct = ContentType.objects.get(app_label='osf', model='abstractprovider') node_subquery = AbstractNode.objects.filter( id=Cast(OuterRef('object_id'), IntegerField()), ).values('guids___id')[:1] + _global_file_updated = [ + NotificationType.Type.USER_FILE_UPDATED.value, + NotificationType.Type.FILE_ADDED.value, + NotificationType.Type.FILE_REMOVED.value, + NotificationType.Type.ADDON_FILE_COPIED.value, + NotificationType.Type.ADDON_FILE_RENAMED.value, + NotificationType.Type.ADDON_FILE_MOVED.value, + NotificationType.Type.ADDON_FILE_REMOVED.value, + NotificationType.Type.FOLDER_CREATED.value, + ] + _global_reviews = [ + 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, + NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value, + ] + _node_file_updated = [ + NotificationType.Type.NODE_FILE_UPDATED.value, + NotificationType.Type.FILE_ADDED.value, + NotificationType.Type.FILE_REMOVED.value, + NotificationType.Type.ADDON_FILE_COPIED.value, + NotificationType.Type.ADDON_FILE_RENAMED.value, + NotificationType.Type.ADDON_FILE_MOVED.value, + NotificationType.Type.ADDON_FILE_REMOVED.value, + NotificationType.Type.FOLDER_CREATED.value, + ] + qs = NotificationSubscription.objects.filter( - notification_type__in=[ - NotificationType.Type.USER_FILE_UPDATED.instance, - NotificationType.Type.NODE_FILE_UPDATED.instance, - NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, - ], + 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, user=self.request.user, ).annotate( event_name=Case( When( - notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance, + notification_type__name__in=_node_file_updated, then=Value('files_updated'), ), When( - notification_type=NotificationType.Type.USER_FILE_UPDATED.instance, + notification_type__name__in=_global_file_updated, then=Value('global_file_updated'), ), When( - Q(notification_type=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance) & - Q(content_type=provider_ct), + notification_type__name__in=_global_reviews, then=Value('global_reviews'), ), ), legacy_id=Case( When( - notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance, + notification_type__name__in=_node_file_updated, then=Concat(Subquery(node_subquery), Value('_file_updated')), ), When( - notification_type=NotificationType.Type.USER_FILE_UPDATED.instance, + notification_type__name__in=_global_file_updated, then=Value(f'{user_guid}_global_file_updated'), ), When( - Q(notification_type=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance) & - Q(content_type=provider_ct), + notification_type__name__in=_global_reviews, then=Value(f'{user_guid}_global_reviews'), ), ), @@ -182,7 +208,7 @@ def update(self, request, *args, **kwargs): NotificationType.Type.ADDON_FILE_REMOVED.value, NotificationType.Type.FOLDER_CREATED.value, ], - ) + ).exclude(content_type=ContentType.objects.get_for_model(AbstractNode)) if not qs.exists(): raise PermissionDenied @@ -211,6 +237,34 @@ def update(self, request, *args, **kwargs): serializer.is_valid(raise_exception=True) self.perform_update(serializer) return Response(serializer.data) + elif '_files_updated' in self.kwargs['subscription_id']: + # Copy _files_updated subscription changes to all node file subscriptions + node_id = Guid.load(self.kwargs['subscription_id'].split('_files_updated')[0]).object_id + + qs = NotificationSubscription.objects.filter( + user=self.request.user, + content_type=ContentType.objects.get_for_model(AbstractNode), + object_id=node_id, + notification_type__name__in=[ + NotificationType.Type.NODE_FILE_UPDATED.value, + NotificationType.Type.FILE_ADDED.value, + NotificationType.Type.FILE_REMOVED.value, + NotificationType.Type.ADDON_FILE_COPIED.value, + NotificationType.Type.ADDON_FILE_RENAMED.value, + NotificationType.Type.ADDON_FILE_MOVED.value, + NotificationType.Type.ADDON_FILE_REMOVED.value, + NotificationType.Type.FOLDER_CREATED.value, + ], + ) + if not qs.exists(): + raise PermissionDenied + + for instance in qs: + serializer = self.get_serializer(instance=instance, data=request.data, partial=True) + serializer.is_valid(raise_exception=True) + self.perform_update(serializer) + return Response(serializer.data) + else: return super().update(request, *args, **kwargs) diff --git a/notifications/file_event_notifications.py b/notifications/file_event_notifications.py index dd7457234ff..6d0fc3605bf 100644 --- a/notifications/file_event_notifications.py +++ b/notifications/file_event_notifications.py @@ -55,6 +55,7 @@ def perform(self): name=self.action ).emit( user=self.user, + subscribed_object=self.node, event_context={ 'user_fullname': self.user.fullname, 'profile_image_url': self.profile_image_url, diff --git a/notifications/listeners.py b/notifications/listeners.py index ea0d2069ec2..a96434c92e2 100644 --- a/notifications/listeners.py +++ b/notifications/listeners.py @@ -9,7 +9,7 @@ @project_created.connect def subscribe_creator(resource): - from osf.models import NotificationSubscription, NotificationType + from osf.models import NotificationSubscription, NotificationType, Preprint from django.contrib.contenttypes.models import ContentType @@ -30,24 +30,25 @@ def subscribe_creator(resource): ) except NotificationSubscription.MultipleObjectsReturned: pass - try: - NotificationSubscription.objects.get_or_create( - user=user, - notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance, - object_id=resource.id, - content_type=ContentType.objects.get_for_model(resource), - _is_digest=True, - defaults={ - 'message_frequency': 'instantly', - } - ) - except NotificationSubscription.MultipleObjectsReturned: - pass + if not isinstance(resource, Preprint): + try: + NotificationSubscription.objects.get_or_create( + user=user, + notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance, + object_id=resource.id, + content_type=ContentType.objects.get_for_model(resource), + _is_digest=True, + defaults={ + 'message_frequency': 'instantly', + } + ) + except NotificationSubscription.MultipleObjectsReturned: + pass @contributor_added.connect def subscribe_contributor(resource, contributor, auth=None, *args, **kwargs): from django.contrib.contenttypes.models import ContentType - from osf.models import NotificationSubscription, NotificationType + from osf.models import NotificationSubscription, NotificationType, Preprint from osf.models import Node if isinstance(resource, Node): @@ -67,19 +68,20 @@ def subscribe_contributor(resource, contributor, auth=None, *args, **kwargs): ) except NotificationSubscription.MultipleObjectsReturned: pass - try: - NotificationSubscription.objects.get_or_create( - user=contributor, - notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance, - object_id=resource.id, - content_type=ContentType.objects.get_for_model(resource), - _is_digest=True, - defaults={ - 'message_frequency': 'instantly', - } - ) - except NotificationSubscription.MultipleObjectsReturned: - pass + if not isinstance(resource, Preprint): + try: + NotificationSubscription.objects.get_or_create( + user=contributor, + notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance, + object_id=resource.id, + content_type=ContentType.objects.get_for_model(resource), + _is_digest=True, + defaults={ + 'message_frequency': 'instantly', + } + ) + except NotificationSubscription.MultipleObjectsReturned: + pass # Handle email notifications to notify moderators of new submissions. diff --git a/notifications/tasks.py b/notifications/tasks.py index dc24d904f2a..fbb1d0c59ed 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -1,7 +1,7 @@ import itertools from calendar import monthrange from datetime import date - +from django.contrib.contenttypes.models import ContentType from django.db import connection from django.utils import timezone @@ -10,7 +10,7 @@ from framework.postcommit_tasks.handlers import run_postcommit from osf.models import OSFUser, Notification, NotificationType, EmailTask, RegistrationProvider, \ - CollectionProvider + CollectionProvider, AbstractProvider from framework.sentry import log_message from osf.registrations.utils import get_registration_provider_submissions_url from osf.utils.permissions import ADMIN @@ -20,6 +20,24 @@ logger = get_task_logger(__name__) +def safe_render_notification(notifications, email_task): + """Helper to safely render notification, updating email_task on failure.""" + rendered_notifications = [] + failed_notifications = [] + for notification in notifications: + try: + rendered = notification.render() + except Exception as e: + email_task.error_message = f'Error rendering notification {notification.id}: {str(e)} \n' + email_task.save() + failed_notifications.append(notification.id) + continue + + rendered_notifications.append(rendered) + + return rendered_notifications, failed_notifications + + def get_user_and_email_task(task_id, user_id): """Helper to safely fetch user and initialize EmailTask.""" try: @@ -47,7 +65,7 @@ def get_user_and_email_task(task_id, user_id): return user, email_task -@celery_app.task(bind=True) +@celery_app.task(bind=True, max_retries=5) def send_user_email_task(self, user_id, notification_ids, **kwargs): user, email_task = get_user_and_email_task(self.request.id, user_id) if not user: @@ -55,9 +73,12 @@ def send_user_email_task(self, user_id, notification_ids, **kwargs): try: notifications_qs = Notification.objects.filter(id__in=notification_ids) - rendered_notifications = [n.render() for n in notifications_qs] + rendered_notifications, failed_notifications = safe_render_notification(notifications_qs, email_task) + notifications_qs = notifications_qs.exclude(id__in=failed_notifications) if not rendered_notifications: + if email_task.error_message: + logger.error(f'Partial success for send_user_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}') email_task.status = 'SUCCESS' email_task.save() return @@ -71,68 +92,83 @@ def send_user_email_task(self, user_id, notification_ids, **kwargs): NotificationType.Type.USER_DIGEST.instance.emit( user=user, event_context=event_context, + save=False ) notifications_qs.update(sent=timezone.now()) email_task.status = 'SUCCESS' email_task.save() + + if email_task.error_message: + logger.error(f'Partial success for send_user_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}') + except Exception as e: - try: - user = OSFUser.objects.get( - guids___id=user_id, - deleted__isnull=True - ) - except OSFUser.DoesNotExist: - logger.error(f'OSFUser with id {user_id} does not exist') - email_task, _ = EmailTask.objects.get_or_create(task_id=self.request.id) - email_task.status = 'NO_USER_FOUND' - email_task.error_message = 'User not found or disabled' + retry_count = self.request.retries + max_retries = self.max_retries + + if retry_count >= max_retries: + email_task.status = 'FAILURE' + email_task.error_message = email_task.error_message + f'Max retries reached: {str(e)} \n' email_task.save() + logger.error(f'Max retries reached for send_moderator_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}') return + email_task, _ = EmailTask.objects.get_or_create(task_id=self.request.id) email_task.user = user email_task.status = 'RETRY' - email_task.error_message = str(e) + email_task.error_message = f'{str(e)} \n' + email_task.error_message = email_task.error_message + f'Retry {retry_count}: {str(e)} \n' email_task.save() - logger.exception('Retrying send_user_email_task due to exception') raise self.retry(exc=e) -@celery_app.task(bind=True) -def send_moderator_email_task(self, user_id, notification_ids, **kwargs): +@celery_app.task(bind=True, max_retries=5) +def send_moderator_email_task(self, user_id, notification_ids, provider_content_type_id, provider_id, **kwargs): user, email_task = get_user_and_email_task(self.request.id, user_id) if not user: return - try: notifications_qs = Notification.objects.filter(id__in=notification_ids) - first_notification = notifications_qs.select_related('subscription').first() - subscription = first_notification.subscription if first_notification else None - subscribed_object = getattr(subscription, 'subscribed_object', None) if subscription else None - rendered_notifications = [notification.render() for notification in notifications_qs] + rendered_notifications, failed_notifications = safe_render_notification(notifications_qs, email_task) + notifications_qs = notifications_qs.exclude(id__in=failed_notifications) + if not rendered_notifications: - log_message(f"No notifications to send for moderator user {user._id}") + if email_task.error_message: + logger.error(f'Partial success for send_moderator_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}') email_task.status = 'SUCCESS' email_task.save() return - if subscribed_object is None: - log_message(f"subscribed_object fpr {subscribed_object} does not exist") - email_task.status = 'OBJECT NOT FOUND' + ProviderModel = ContentType.objects.get_for_id(provider_content_type_id).model_class() + try: + provider = ProviderModel.objects.get(id=provider_id) + except AbstractProvider.DoesNotExist: + log_message(f'Provider with id {provider_id} does not exist for model {ProviderModel.name}') + email_task.status = 'FAILURE' + email_task.error_message = f'Provider with id {provider_id} does not exist for model {ProviderModel.name}' + email_task.save() + return + except AttributeError as err: + log_message(f'Error retrieving provider with id {provider_id} for model {ProviderModel.name}: {err}') + email_task.status = 'FAILURE' + email_task.error_message = f'Error retrieving provider with id {provider_id} for model {ProviderModel.name}: {err}' email_task.save() return - provider = getattr(subscribed_object, 'provider', None) if provider is None: - log_message(f"subscribed_object fpr {subscribed_object} does not exist") - email_task.status = 'PROVIDER NOT FOUND' + log_message(f'Provider with id {provider_id} does not exist for model {ProviderModel.name}') + email_task.status = 'FAILURE' + email_task.error_message = f'Provider with id {provider_id} does not exist for model {ProviderModel.name}' 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.status = 'FAILURE' + email_task.error_message = f'User is not a moderator for provider {provider._id}' + email_task.save() + return additional_context = {} if isinstance(provider, RegistrationProvider): @@ -145,6 +181,8 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): 'logo_url': provider.brand.hero_logo_image, 'top_bar_color': provider.brand.primary_color } + else: + logo = settings.OSF_REGISTRIES_LOGO elif isinstance(provider, CollectionProvider): provider_type = 'collection' submissions_url = f'{settings.DOMAIN}collections/{provider._id}/moderation/' @@ -155,11 +193,14 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): 'logo_url': provider.brand.hero_logo_image, 'top_bar_color': provider.brand.primary_color } + else: + logo = settings.OSF_REGISTRIES_LOGO else: provider_type = 'preprint' submissions_url = f'{settings.DOMAIN}reviews/preprints/{provider._id}' withdrawals_url = '' notification_settings_url = f'{settings.DOMAIN}reviews/{provider_type}s/{provider._id}/notifications' + logo = provider._id if not provider.is_default else settings.OSF_PREPRINTS_LOGO event_context = { 'notifications': rendered_notifications, @@ -171,13 +212,14 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): 'provider_type': provider_type, 'additional_context': additional_context, 'is_admin': provider.get_group(ADMIN).user_set.filter(id=user.id).exists(), - 'logo': provider._id if not provider.is_default else settings.OSF_PREPRINTS_LOGO, + 'logo': logo, } NotificationType.Type.DIGEST_REVIEWS_MODERATORS.instance.emit( user=user, - subscribed_object=subscribed_object, + subscribed_object=user, event_context=event_context, + save=False ) notifications_qs.update(sent=timezone.now()) @@ -185,11 +227,23 @@ def send_moderator_email_task(self, user_id, notification_ids, **kwargs): email_task.status = 'SUCCESS' email_task.save() + if email_task.error_message: + logger.error(f'Partial success for send_moderator_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}') + except Exception as e: + retry_count = self.request.retries + max_retries = self.max_retries + + if retry_count >= max_retries: + email_task.status = 'FAILURE' + email_task.error_message = email_task.error_message + f'\nMax retries reached: {str(e)}' + email_task.save() + logger.error(f'Max retries reached for send_moderator_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}') + return + email_task.status = 'RETRY' - email_task.error_message = str(e) + email_task.error_message = email_task.error_message + f'Retry {retry_count}: {str(e)} \n' email_task.save() - logger.exception('Retrying send_moderator_email_task due to exception') raise self.retry(exc=e) @celery_app.task(name='notifications.tasks.send_users_digest_email') @@ -224,9 +278,11 @@ def send_moderators_digest_email(dry_run=False): grouped_emails = get_moderators_emails(freq) for group in grouped_emails: user_id = group['user_id'] + provider_id = group['provider_id'] + provider_content_type_id = group['provider_content_type_id'] notification_ids = [msg['notification_id'] for msg in group['info']] if not dry_run: - send_moderator_email_task.delay(user_id, notification_ids) + send_moderator_email_task.delay(user_id, notification_ids, provider_content_type_id, provider_id) def get_moderators_emails(message_freq: str): """Get all emails for reviews moderators that need to be sent, grouped by users AND providers. @@ -237,6 +293,8 @@ def get_moderators_emails(message_freq: str): SELECT json_build_object( 'user_id', osf_guid._id, + 'provider_id', ns.object_id, + 'provider_content_type_id', ns.content_type_id, 'info', json_agg( json_build_object( 'notification_id', n.id @@ -250,19 +308,22 @@ def get_moderators_emails(message_freq: str): WHERE n.sent IS NULL AND ns.message_frequency = %s AND nt.name IN (%s, %s) + AND nt.name NOT IN (%s, %s) AND osf_guid.content_type_id = ( SELECT id FROM django_content_type WHERE model = 'osfuser' ) - GROUP BY osf_guid._id, (n.event_context ->> 'provider_id') + GROUP BY osf_guid._id, ns.object_id, ns.content_type_id ORDER BY osf_guid._id ASC - """ + """ with connection.cursor() as cursor: cursor.execute(sql, [ message_freq, NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value, - NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value + NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value, + NotificationType.Type.DIGEST_REVIEWS_MODERATORS.value, + NotificationType.Type.USER_DIGEST.value, ] ) return itertools.chain.from_iterable(cursor.fetchall()) @@ -288,7 +349,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 NOT IN (%s, %s, %s, %s) AND osf_guid.content_type_id = ( SELECT id FROM django_content_type WHERE model = 'osfuser' ) @@ -301,7 +362,9 @@ def get_users_emails(message_freq): [ message_freq, NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value, - NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value + NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value, + NotificationType.Type.DIGEST_REVIEWS_MODERATORS.value, + NotificationType.Type.USER_DIGEST.value, ] ) return itertools.chain.from_iterable(cursor.fetchall()) @@ -349,6 +412,8 @@ def send_moderators_instant_digest_email(self, dry_run=False, **kwargs): grouped_emails = get_moderators_emails('instantly') for group in grouped_emails: user_id = group['user_id'] + provider_id = group['provider_id'] + provider_content_type_id = group['provider_content_type_id'] notification_ids = [msg['notification_id'] for msg in group['info']] if not dry_run: - send_moderator_email_task.delay(user_id, notification_ids) + send_moderator_email_task.delay(user_id, notification_ids, provider_content_type_id, provider_id) diff --git a/osf/models/collection_submission.py b/osf/models/collection_submission.py index 285a16acffc..1e646745dc4 100644 --- a/osf/models/collection_submission.py +++ b/osf/models/collection_submission.py @@ -137,7 +137,7 @@ def _notify_moderators_pending(self, event_data): user = event_data.kwargs.get('user', None) NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance.emit( user=user, - subscribed_object=self.guid.referent, + subscribed_object=self.collection.provider, event_context={ 'provider_id': self.collection.provider.id, 'submitter_fullname': self.creator.fullname, diff --git a/osf/models/mixins.py b/osf/models/mixins.py index f0c2cd1528b..8abb93c1dfe 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1081,13 +1081,14 @@ def add_to_group(self, user, group): else: raise TypeError(f"Unsupported group type: {type(group)}") - NotificationSubscription.objects.get_or_create( - user=user, - content_type=ContentType.objects.get_for_model(self), - object_id=self.id, - notification_type=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, - _is_digest=True - ) + for subscription in self.DEFAULT_SUBSCRIPTIONS: + NotificationSubscription.objects.get_or_create( + user=user, + content_type=ContentType.objects.get_for_model(self, for_concrete_model=False), + object_id=self.id, + notification_type=subscription.instance, + _is_digest=True + ) def remove_from_group(self, user, group, unsubscribe=True): _group = self.get_group(group) @@ -1100,6 +1101,8 @@ def remove_from_group(self, user, group, unsubscribe=True): NotificationSubscription.objects.filter( notification_type=subscription.instance, user=user, + content_type=ContentType.objects.get_for_model(self, for_concrete_model=False), + object_id=self.id, ).delete() return _group.user_set.remove(user) diff --git a/osf/models/notification_subscription.py b/osf/models/notification_subscription.py index 5b4dac5ba32..cd3707c074c 100644 --- a/osf/models/notification_subscription.py +++ b/osf/models/notification_subscription.py @@ -1,5 +1,5 @@ import logging - +from django.utils import timezone from django.db import models from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType @@ -32,13 +32,13 @@ class NotificationSubscription(BaseModel): object_id = models.CharField(max_length=255, null=True, blank=True) subscribed_object = GenericForeignKey('content_type', 'object_id') + # mark if subscription is for digest use only (instant subscriptions are sent every 5 minutes) _is_digest = models.BooleanField(default=False) def clean(self): ct = self.notification_type.object_content_type - if ct: - if self.content_type != ct: + if self.content_type != ct and 'provider' not in self.notification_type.name.lower(): raise ValidationError('Subscribed object must match type\'s content_type.') if not self.object_id: raise ValidationError('Subscribed object ID is required.') @@ -94,7 +94,14 @@ def emit( ) if save: notification.save() - if not self._is_digest: # instant digests are sent every 5 minutes. + else: + notification.send( + destination_address=destination_address, + email_context=email_context, + save=save, + ) + return + if not self._is_digest: notification.send( destination_address=destination_address, email_context=email_context, @@ -103,7 +110,8 @@ def emit( else: Notification.objects.create( subscription=self, - event_context=event_context + event_context=event_context, + sent=None if self.message_frequency != 'none' else timezone.now(), ) @property @@ -122,6 +130,7 @@ def _id(self): """ _global_file_updated = [ NotificationType.Type.USER_FILE_UPDATED.value, + NotificationType.Type.FILE_UPDATED.value, NotificationType.Type.FILE_ADDED.value, NotificationType.Type.FILE_REMOVED.value, NotificationType.Type.ADDON_FILE_COPIED.value, @@ -137,10 +146,20 @@ def _id(self): NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value, NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value, ] + _node_file_updated = [ + NotificationType.Type.NODE_FILE_UPDATED.value, + NotificationType.Type.FILE_ADDED.value, + NotificationType.Type.FILE_REMOVED.value, + NotificationType.Type.ADDON_FILE_COPIED.value, + NotificationType.Type.ADDON_FILE_RENAMED.value, + NotificationType.Type.ADDON_FILE_MOVED.value, + NotificationType.Type.ADDON_FILE_REMOVED.value, + NotificationType.Type.FOLDER_CREATED.value, + ] if self.notification_type.name in _global_file_updated: return f'{self.user._id}_file_updated' elif self.notification_type.name in _global_reviews: return f'{self.user._id}_global_reviews' - elif self.notification_type.name == NotificationType.Type.NODE_FILE_UPDATED.value: + elif self.notification_type.name in _node_file_updated: return f'{self.subscribed_object._id}_file_updated' raise NotImplementedError() diff --git a/osf/models/notification_type.py b/osf/models/notification_type.py index 4913e737282..5bb609e5a27 100644 --- a/osf/models/notification_type.py +++ b/osf/models/notification_type.py @@ -22,7 +22,7 @@ class Type(str, Enum): DESK_ARCHIVE_REGISTRATION_STUCK = 'desk_archive_registration_stuck' DESK_REQUEST_EXPORT = 'desk_request_export' DESK_REQUEST_DEACTIVATION = 'desk_request_deactivation' - DESK_OSF_SUPPORT_EMAIL = 'desk_osf_support_email' + DESK_OSF_SUPPORT_EMAIL = 'desk_osf_support_email' # unused same as DESK_CROSSREF_ERROR DESK_REGISTRATION_BULK_UPLOAD_PRODUCT_OWNER = 'desk_registration_bulk_upload_product_owner' DESK_USER_REGISTRATION_BULK_UPLOAD_UNEXPECTED_FAILURE = 'desk_user_registration_bulk_upload_unexpected_failure' DESK_ARCHIVE_JOB_EXCEEDED = 'desk_archive_job_exceeded' @@ -41,19 +41,19 @@ class Type(str, Enum): USER_INSTITUTION_DEACTIVATION = 'user_institution_deactivation' USER_FORGOT_PASSWORD = 'user_forgot_password' USER_FORGOT_PASSWORD_INSTITUTION = 'user_forgot_password_institution' - USER_REQUEST_EXPORT = 'user_request_export' + USER_REQUEST_EXPORT = 'user_request_export' # unused no template USER_DUPLICATE_ACCOUNTS_OSF4I = 'user_duplicate_accounts_osf4i' USER_EXTERNAL_LOGIN_LINK_SUCCESS = 'user_external_login_link_success' USER_REGISTRATION_BULK_UPLOAD_FAILURE_ALL = 'user_registration_bulk_upload_failure_all' USER_REGISTRATION_BULK_UPLOAD_SUCCESS_PARTIAL = 'user_registration_bulk_upload_success_partial' USER_REGISTRATION_BULK_UPLOAD_SUCCESS_ALL = 'user_registration_bulk_upload_success_all' - USER_ADD_SSO_EMAIL_OSF4I = 'user_add_sso_email_osf4i' + USER_ADD_SSO_EMAIL_OSF4I = 'user_add_sso_email_osf4i' # unused no template USER_WELCOME_OSF4I = 'user_welcome_osf4i' USER_ARCHIVE_JOB_EXCEEDED = 'user_archive_job_exceeded' USER_ARCHIVE_JOB_COPY_ERROR = 'user_archive_job_copy_error' USER_ARCHIVE_JOB_FILE_NOT_FOUND = 'user_archive_job_file_not_found' - USER_COMMENT_REPLIES = 'user_comment_replies' - USER_FILE_UPDATED = 'user_file_updated' + # USER_COMMENT_REPLIES = 'user_comment_replies' # unused + USER_FILE_UPDATED = 'user_file_updated' # unused USER_FILE_OPERATION_SUCCESS = 'user_file_operation_success' USER_FILE_OPERATION_FAILED = 'user_file_operation_failed' USER_PASSWORD_RESET = 'user_password_reset' @@ -63,15 +63,15 @@ class Type(str, Enum): USER_CONFIRM_EMAIL = 'user_confirm_email' USER_INITIAL_CONFIRM_EMAIL = 'user_initial_confirm_email' USER_INVITE_DEFAULT = 'user_invite_default' - USER_PENDING_INVITE = 'user_pending_invite' + USER_PENDING_INVITE = 'user_pending_invite' # unused USER_FORWARD_INVITE = 'user_forward_invite' USER_FORWARD_INVITE_REGISTERED = 'user_forward_invite_registered' - USER_INVITE_DRAFT_REGISTRATION = 'user_invite_draft_registration' + USER_INVITE_DRAFT_REGISTRATION = 'user_invite_draft_registration' # unused same as DRAFT_REGISTRATION_CONTRIBUTOR_ADDED_DEFAULT USER_INVITE_OSF_PREPRINT = 'user_invite_osf_preprint' - USER_CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF = 'user_contributor_added_preprint_node_from_osf' - USER_CONTRIBUTOR_ADDED_ACCESS_REQUEST = 'user_contributor_added_access_request' + USER_CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF = 'user_contributor_added_preprint_node_from_osf' # unused + USER_CONTRIBUTOR_ADDED_ACCESS_REQUEST = 'user_contributor_added_access_request' # unused USER_ARCHIVE_JOB_UNCAUGHT_ERROR = 'user_archive_job_uncaught_error' - USER_INSTITUTIONAL_ACCESS_REQUEST = 'user_institutional_access_request' + USER_INSTITUTIONAL_ACCESS_REQUEST = 'user_institutional_access_request' # confirm behavior USER_CAMPAIGN_CONFIRM_PREPRINTS_BRANDED = 'user_campaign_confirm_preprint_branded' USER_CAMPAIGN_CONFIRM_PREPRINTS_OSF = 'user_campaign_confirm_preprint_osf' USER_CAMPAIGN_CONFIRM_EMAIL_AGU_CONFERENCE = 'user_campaign_confirm_email_agu_conference' @@ -83,8 +83,8 @@ class Type(str, Enum): DIGEST_REVIEWS_MODERATORS = 'digest_reviews_moderators' # Node notifications - NODE_FILE_UPDATED = 'node_file_updated' - NODE_FILES_UPDATED = 'node_files_updated' + NODE_FILE_UPDATED = 'node_file_updated' # unused + NODE_FILES_UPDATED = 'node_files_updated' # unused NODE_AFFILIATION_CHANGED = 'node_affiliation_changed' NODE_REQUEST_ACCESS_SUBMITTED = 'node_request_access_submitted' NODE_REQUEST_ACCESS_DENIED = 'node_request_access_denied' @@ -115,22 +115,22 @@ class Type(str, Enum): ADDON_FILE_COPIED = 'addon_file_copied' ADDON_FILE_RENAMED = 'addon_file_renamed' ADDON_FILE_MOVED = 'addon_file_moved' - ADDON_FILE_REMOVED = 'addon_file_removed' + ADDON_FILE_REMOVED = 'addon_file_removed' # unused FOLDER_CREATED = 'folder_created' # Provider notifications PROVIDER_NEW_PENDING_SUBMISSIONS = 'provider_new_pending_submissions' PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS = 'provider_new_pending_withdraw_requests' PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION = 'provider_reviews_submission_confirmation' - PROVIDER_REVIEWS_MODERATOR_SUBMISSION_CONFIRMATION = 'provider_reviews_moderator_submission_confirmation' - PROVIDER_REVIEWS_REJECT_CONFIRMATION = 'provider_reviews_reject_confirmation' - PROVIDER_REVIEWS_ACCEPT_CONFIRMATION = 'provider_reviews_accept_confirmation' + PROVIDER_REVIEWS_MODERATOR_SUBMISSION_CONFIRMATION = 'provider_reviews_moderator_submission_confirmation' # unused + PROVIDER_REVIEWS_REJECT_CONFIRMATION = 'provider_reviews_reject_confirmation' # unused + PROVIDER_REVIEWS_ACCEPT_CONFIRMATION = 'provider_reviews_accept_confirmation' # unused PROVIDER_REVIEWS_RESUBMISSION_CONFIRMATION = 'provider_reviews_resubmission_confirmation' - PROVIDER_REVIEWS_COMMENT_EDITED = 'provider_reviews_comment_edited' - PROVIDER_CONTRIBUTOR_ADDED_PREPRINT = 'provider_contributor_added_preprint' + PROVIDER_REVIEWS_COMMENT_EDITED = 'provider_reviews_comment_edited' # unused + PROVIDER_CONTRIBUTOR_ADDED_PREPRINT = 'provider_contributor_added_preprint' # unused PROVIDER_CONFIRM_EMAIL_MODERATION = 'provider_confirm_email_moderation' PROVIDER_MODERATOR_ADDED = 'provider_moderator_added' - PROVIDER_CONFIRM_EMAIL_PREPRINTS = 'provider_confirm_email_preprints' + PROVIDER_CONFIRM_EMAIL_PREPRINTS = 'provider_confirm_email_preprints' # unused PROVIDER_USER_INVITE_PREPRINT = 'provider_user_invite_preprint' # Preprint notifications @@ -138,7 +138,7 @@ class Type(str, Enum): PREPRINT_REQUEST_WITHDRAWAL_DECLINED = 'preprint_request_withdrawal_declined' PREPRINT_CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF = 'preprint_contributor_added_preprint_node_from_osf' PREPRINT_CONTRIBUTOR_ADDED_DEFAULT = 'preprint_contributor_added_default' - PREPRINT_PENDING_RETRACTION_ADMIN = 'preprint_pending_retraction_admin' + PREPRINT_PENDING_RETRACTION_ADMIN = 'preprint_pending_retraction_admin' # unused # Collections Submission notifications COLLECTION_SUBMISSION_REMOVED_ADMIN = 'collection_submission_removed_admin' @@ -188,7 +188,7 @@ def emit( user=None, destination_address=None, subscribed_object=None, - message_frequency='instantly', + message_frequency=None, event_context=None, email_context=None, is_digest=False, @@ -209,11 +209,22 @@ def emit( used. """ from osf.models.notification_subscription import NotificationSubscription + from osf.models.provider import AbstractProvider + + # use concrete model for AbstractProvider to specifically get the provider content type + if isinstance(subscribed_object, AbstractProvider): + content_type = ContentType.objects.get_for_model(subscribed_object, for_concrete_model=False) if subscribed_object else None + else: + content_type = ContentType.objects.get_for_model(subscribed_object) if subscribed_object else None + + if message_frequency is None: + message_frequency = self.get_group_frequency_or_default(user, subscribed_object, content_type) + if not save: subscription = NotificationSubscription( notification_type=self, user=user, - content_type=ContentType.objects.get_for_model(subscribed_object) if subscribed_object else None, + content_type=content_type, object_id=subscribed_object.pk if subscribed_object else None, message_frequency=message_frequency, _is_digest=is_digest, @@ -222,7 +233,7 @@ def emit( subscription, created = NotificationSubscription.objects.get_or_create( notification_type=self, user=user, - content_type=ContentType.objects.get_for_model(subscribed_object) if subscribed_object else None, + content_type=content_type, object_id=subscribed_object.pk if subscribed_object else None, defaults={'message_frequency': message_frequency}, _is_digest=is_digest, @@ -240,3 +251,65 @@ def __str__(self) -> str: class Meta: verbose_name = 'Notification Type' verbose_name_plural = 'Notification Types' + + def get_group_frequency_or_default(self, user, subscribed_object, content_type): + from osf.models.notification_subscription import NotificationSubscription + + _global_file_updated = [ + NotificationType.Type.USER_FILE_UPDATED.value, + NotificationType.Type.FILE_UPDATED.value, + NotificationType.Type.FILE_ADDED.value, + NotificationType.Type.FILE_REMOVED.value, + NotificationType.Type.ADDON_FILE_COPIED.value, + NotificationType.Type.ADDON_FILE_RENAMED.value, + NotificationType.Type.ADDON_FILE_MOVED.value, + NotificationType.Type.ADDON_FILE_REMOVED.value, + NotificationType.Type.FOLDER_CREATED.value, + ] + _global_reviews = [ + 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, + NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value, + ] + _node_file_updated = [ + NotificationType.Type.NODE_FILE_UPDATED.value, + NotificationType.Type.FILE_ADDED.value, + NotificationType.Type.FILE_REMOVED.value, + NotificationType.Type.ADDON_FILE_COPIED.value, + NotificationType.Type.ADDON_FILE_RENAMED.value, + NotificationType.Type.ADDON_FILE_MOVED.value, + NotificationType.Type.ADDON_FILE_REMOVED.value, + NotificationType.Type.FOLDER_CREATED.value, + ] + + if self.name in _global_file_updated: + frequency_data = NotificationSubscription.objects.filter( + user=user, + content_type=content_type, + notification_type__name__in=_global_file_updated, + ).distinct('message_frequency').values_list('message_frequency', flat=True) + if frequency_data.exists() and len(frequency_data) == 1: + return frequency_data[0] + + elif self.name in _global_reviews: + frequency_data = NotificationSubscription.objects.filter( + user=user, + content_type=content_type, + notification_type__name__in=_global_reviews, + ).distinct('message_frequency').values_list('message_frequency', flat=True) + if frequency_data.exists() and len(frequency_data) == 1: + return frequency_data[0] + + elif self.name in _node_file_updated: + frequency_data = NotificationSubscription.objects.filter( + user=user, + content_type=content_type, + object_id=subscribed_object.id, + notification_type__name__in=_node_file_updated + ).distinct('message_frequency').values_list('message_frequency', flat=True) + if frequency_data.exists() and len(frequency_data) == 1: + return frequency_data[0] + + return 'instantly' diff --git a/osf/utils/notifications.py b/osf/utils/notifications.py index 9f5fa3d4c60..8808ef3b243 100644 --- a/osf/utils/notifications.py +++ b/osf/utils/notifications.py @@ -128,7 +128,7 @@ def notify_reject_withdraw_request(resource, action, *args, **kwargs): context['contributor_fullname'] = contributor.fullname context['is_requester'] = action.creator == contributor context['comment'] = action.comment - NotificationType.Type.NODE_WITHDRAWAl_REQUEST_APPROVED.instance.emit( + NotificationType.Type.NODE_WITHDRAWAl_REQUEST_REJECTED.instance.emit( user=contributor, event_context={ 'is_requester': contributor == action.creator, diff --git a/website/reviews/listeners.py b/website/reviews/listeners.py index caa88491db0..350564b15b0 100644 --- a/website/reviews/listeners.py +++ b/website/reviews/listeners.py @@ -18,8 +18,9 @@ def reviews_notification(self, creator, template, context, action): context['has_psyarxiv_chronos_text'] = action.target.has_permission(recipient, ADMIN) and 'psyarxiv' in action.target.provider.name.lower() template.instance.emit( user=recipient, - subscribed_object=action.target, + subscribed_object=recipient, event_context=context, + is_digest=True, ) @reviews_signals.reviews_withdraw_requests_notification_moderators.connect From aedf5a32bfacb9183db2a3e833d283856831062e Mon Sep 17 00:00:00 2001 From: Ostap Zherebetskyi Date: Thu, 27 Nov 2025 14:24:12 +0200 Subject: [PATCH 2/2] fix unit tests --- .../notifications/test_notification_digest.py | 36 +++++++++++-------- notifications.yaml | 7 ++++ ...t_registration_moderation_notifications.py | 3 +- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/api_tests/notifications/test_notification_digest.py b/api_tests/notifications/test_notification_digest.py index 7a5c8440807..6a8ef1c2ad2 100644 --- a/api_tests/notifications/test_notification_digest.py +++ b/api_tests/notifications/test_notification_digest.py @@ -18,7 +18,7 @@ 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 + from osf.models import NotificationSubscription, AbstractProvider kwargs = { 'user': user, 'notification_type': NotificationType.objects.get(name=notification_type), @@ -26,7 +26,10 @@ def add_notification_subscription(user, notification_type, frequency, subscribed } if subscribed_object is not None: kwargs['object_id'] = subscribed_object.id - kwargs['content_type'] = ContentType.objects.get_for_model(subscribed_object) + 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 if subscription is not None: kwargs['object_id'] = subscription.id kwargs['content_type'] = ContentType.objects.get_for_model(subscription) @@ -113,16 +116,17 @@ 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) - admin_group = reg_provider.get_group('admin') - admin_group.user_set.add(user) + reg_provider_content_type = ContentType.objects.get_for_model(reg_provider) + RegistrationFactory(provider=reg_provider) + moderator_group = reg_provider.get_group('moderator') + moderator_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 + subscribed_object=reg_provider ), event_context={ 'profile_image_url': 'http://example.com/profile.png', @@ -137,7 +141,7 @@ def test_send_moderator_email_task_registration_provider_admin(self): ) notification_ids = [notification.id] with capture_notifications() as notifications: - send_moderator_email_task.apply(args=(user._id, notification_ids)).get() + send_moderator_email_task.apply(args=(user._id, notification_ids, reg_provider_content_type.id, reg_provider.id)).get() assert len(notifications['emits']) == 1 assert notifications['emits'][0]['type'] == NotificationType.Type.DIGEST_REVIEWS_MODERATORS assert notifications['emits'][0]['kwargs']['user'] == user @@ -150,7 +154,8 @@ 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) + reg_provider_content_type = ContentType.objects.get_for_model(provider) + RegistrationFactory(provider=provider) notification_ids = [] notification_type = NotificationType.objects.get(name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS) @@ -158,22 +163,21 @@ def test_send_moderator_email_task_no_notifications(self): user, notification_type, 'daily', - subscribed_object=reg + subscribed_object=provider ) - - send_moderator_email_task.apply(args=(user._id, notification_ids)).get() + send_moderator_email_task.apply(args=(user._id, notification_ids, reg_provider_content_type.id, provider.id)).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() + send_moderator_email_task.apply(args=('nouser', [], 1, 1)).get() email_task = EmailTask.objects.filter() 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) + notification_type = NotificationType.objects.get(name=NotificationType.Type.USER_FILE_UPDATED) notification1 = Notification.objects.create( subscription=add_notification_subscription(user, notification_type, 'daily'), sent=None, @@ -249,10 +253,12 @@ def test_send_users_digest_email_end_to_end(self): def test_send_moderators_digest_email_end_to_end(self): user = AuthUserFactory() provider = RegistrationProviderFactory() - reg = RegistrationFactory(provider=provider) + RegistrationFactory(provider=provider) + moderator_group = provider.get_group('moderator') + moderator_group.user_set.add(user) 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), + subscription=add_notification_subscription(user, notification_type, 'daily', subscribed_object=provider), sent=None, event_context={ 'submitter_fullname': 'submitter_fullname', diff --git a/notifications.yaml b/notifications.yaml index 6b15d963789..fce989b0e02 100644 --- a/notifications.yaml +++ b/notifications.yaml @@ -884,3 +884,10 @@ notification_types: object_content_type_model_name: abstractnode template: 'website/templates/file_updated.html.mako' tests: ['tests/test_events.py'] + + - name: node_withdrawal_request_rejected + subject: 'Your withdrawal request has been declined' + __docs__: ... + object_content_type_model_name: abstractnode + template: 'website/templates/withdrawal_request_declined.html.mako' + tests: [] diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index 43d9b23802e..a18e5c97da0 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -140,8 +140,7 @@ def test_submit_notifications(self, registration, moderator, admin, contrib, pro 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 + assert NotificationSubscription.objects.count() == 6 digest = NotificationSubscription.objects.last() assert digest.user == moderator