Skip to content

Commit 724cdda

Browse files
Remove subscription if notifications.tasks.send_moderator_email_task fails with permission error
1 parent 7b94b12 commit 724cdda

5 files changed

Lines changed: 7 additions & 5 deletions

File tree

api/providers/serializers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ def update(self, instance, validated_data):
401401
return instance
402402

403403
try:
404-
provider.remove_from_group(instance, instance.permission_group, unsubscribe=False)
404+
provider.remove_from_group(instance, instance.permission_group, unsubscribe=True)
405405
except ValueError as e:
406406
raise ValidationError(str(e))
407407
provider.add_to_group(instance, perm_group)

api_tests/notifications/test_notifications_db_transaction.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
AuthUserFactory,
44
NotificationTypeFactory
55
)
6-
from datetime import datetime
76
from osf.models import Notification, NotificationType, NotificationSubscription
87
from tests.utils import capture_notifications
98
from django.db import reset_queries, connection
@@ -54,5 +53,5 @@ def test_emit_frequency_none(self, user_one, test_notification_type):
5453
)
5554
assert Notification.objects.filter(
5655
subscription__notification_type=test_notification_type,
57-
sent=datetime(1000, 1, 1)
56+
fake_sent=True
5857
).exists()

notifications/tasks.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,11 @@ def send_moderator_email_task(self, user_id, notification_ids, provider_content_
207207
if current_moderators is None or not current_moderators.user_set.filter(id=user.id).exists():
208208
current_admins = provider.get_group('admin')
209209
if current_admins is None or not current_admins.user_set.filter(id=user.id).exists():
210-
log_message(f"User is not a moderator for provider {provider._id} - skipping email")
211-
email_task.status = 'FAILURE'
210+
log_message(f"User is not a moderator for provider {provider._id} - notifications will not be sent.")
211+
email_task.status = 'AUTO_FIXED'
212212
email_task.error_message = f'User is not a moderator for provider {provider._id}'
213213
email_task.save()
214+
notifications_qs.update(sent=timezone.now(), fake_sent=True)
214215
return
215216

216217
additional_context = {}

osf/models/email_task.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ class EmailTask(models.Model):
1010
('FAILURE', 'Failure'),
1111
('RETRY', 'Retry'),
1212
('PARTIAL_SUCCESS', 'Partial Success'),
13+
('AUTO_FIXED', 'Auto Fixed'),
1314
)
1415

1516
task_id = models.CharField(max_length=255, unique=True)

osf/models/mixins.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,7 @@ def add_to_group(self, user, group):
10871087
content_type=ContentType.objects.get_for_model(self, for_concrete_model=False),
10881088
object_id=self.id,
10891089
notification_type=subscription.instance,
1090+
message_frequency='instantly',
10901091
_is_digest=True
10911092
)
10921093

0 commit comments

Comments
 (0)