Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/providers/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions notifications/tasks.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 = {}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.")
)
1 change: 1 addition & 0 deletions osf/models/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
6 changes: 4 additions & 2 deletions scripts/populate_notification_subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -96,6 +97,7 @@ def populate_notification_subscriptions():
content_type=node_ct,
object_id=node.id,
defaults={
'_is_digest': True,
'message_frequency': 'none',
},
)
Expand Down
Loading