Fix subscription management and add command to remove duplicate notifications

Author: Ostap-Zherebetskyi

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)

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 = {}

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.")
+        )

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
             )
 

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',
                     },
                 )