diff --git a/api/providers/serializers.py b/api/providers/serializers.py index 81ba5f74b5c..ffc9097e2e3 100644 --- a/api/providers/serializers.py +++ b/api/providers/serializers.py @@ -401,7 +401,7 @@ def update(self, instance, validated_data): return instance try: - provider.remove_from_group(instance, instance.permission_group, unsubscribe=False) + provider.remove_from_group(instance, instance.permission_group, unsubscribe=True) except ValueError as e: raise ValidationError(str(e)) provider.add_to_group(instance, perm_group) diff --git a/notifications/tasks.py b/notifications/tasks.py index 0bf894be856..d5ab58e807b 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -1,6 +1,6 @@ import itertools from calendar import monthrange -from datetime import date +from datetime import date, datetime from django.contrib.contenttypes.models import ContentType from django.db import connection from django.utils import timezone @@ -205,10 +205,11 @@ def send_moderator_email_task(self, user_id, notification_ids, provider_content_ if current_moderators is None or not current_moderators.user_set.filter(id=user.id).exists(): current_admins = provider.get_group('admin') if current_admins is None or not current_admins.user_set.filter(id=user.id).exists(): - log_message(f"User is not a moderator for provider {provider._id} - skipping email") + log_message(f"User is not a moderator for provider {provider._id} - notifications will be marked as sent.") email_task.status = 'FAILURE' email_task.error_message = f'User is not a moderator for provider {provider._id}' email_task.save() + notifications_qs.update(sent=datetime(1000, 1, 1)) return additional_context = {} diff --git a/osf/management/commands/remove_duplicate_notification_subscriptions.py b/osf/management/commands/remove_duplicate_notification_subscriptions.py new file mode 100644 index 00000000000..1897ec65efe --- /dev/null +++ b/osf/management/commands/remove_duplicate_notification_subscriptions.py @@ -0,0 +1,55 @@ +from django.core.management.base import BaseCommand +from django.db import transaction +from django.db.models import OuterRef, Exists + +from osf.models import NotificationSubscription + + +class Command(BaseCommand): + help = ( + 'Remove duplicate NotificationSubscription records, keeping only ' + 'the highest-id record per (user, content_type, object_id, notification_type).' + ) + + def add_arguments(self, parser): + parser.add_argument( + '--dry', + action='store_true', + help='Show how many rows would be deleted without deleting anything.', + ) + + def handle(self, *args, **options): + self.stdout.write('Finding duplicate NotificationSubscription records…') + + to_remove = NotificationSubscription.objects.filter( + Exists( + NotificationSubscription.objects.filter( + user_id=OuterRef('user_id'), + content_type_id=OuterRef('content_type_id'), + object_id=OuterRef('object_id'), + notification_type_id=OuterRef('notification_type_id'), + _is_digest=OuterRef('_is_digest'), + id__gt=OuterRef('id'), # keep most recent record + ) + ) + ) + + count = to_remove.count() + self.stdout.write(f"Duplicates to remove: {count}") + + if options['dry']: + self.stdout.write( + self.style.WARNING('Dry run enabled — no records were deleted.') + ) + return + + if count == 0: + self.stdout.write(self.style.SUCCESS('No duplicates found.')) + return + + with transaction.atomic(): + deleted, _ = to_remove.delete() + + self.stdout.write( + self.style.SUCCESS(f"Successfully removed {deleted} duplicate records.") + ) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 035c0430b7c..2604b7d32cb 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1087,6 +1087,7 @@ def add_to_group(self, user, group): content_type=ContentType.objects.get_for_model(self, for_concrete_model=False), object_id=self.id, notification_type=subscription.instance, + message_frequency='instantly', _is_digest=True ) diff --git a/scripts/populate_notification_subscriptions.py b/scripts/populate_notification_subscriptions.py index f9da312f6e4..557b9f2a47d 100644 --- a/scripts/populate_notification_subscriptions.py +++ b/scripts/populate_notification_subscriptions.py @@ -76,8 +76,9 @@ def populate_notification_subscriptions(): content_type=user_ct, object_id=user.id, defaults={ - 'message_frequency': 'none', - }, + '_is_digest': True, + 'message_frequency': 'none', + }, ) if is_created: created += 1 @@ -96,6 +97,7 @@ def populate_notification_subscriptions(): content_type=node_ct, object_id=node.id, defaults={ + '_is_digest': True, 'message_frequency': 'none', }, )