Skip to content

[ENG-8085] Upgrade Provider Notifications #11196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 31 commits into
base: refactor-notifications
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
57b0bd1
add new data model for notifications
Johnetordoff May 20, 2025
929b9fd
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff May 22, 2025
69231e9
add new notificationsubscription class to views
Johnetordoff May 27, 2025
c324716
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff May 28, 2025
40b38f7
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff May 30, 2025
358091c
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff Jun 3, 2025
5165ab9
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jun 3, 2025
2ab3589
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff Jun 9, 2025
c449599
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jun 9, 2025
d83c2c3
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jun 13, 2025
f550c61
fix absolute url issue
Johnetordoff Jun 16, 2025
458fbfd
fix up unit test issues
Johnetordoff Jun 16, 2025
16c5409
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff Jun 25, 2025
5864187
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff Jun 30, 2025
bb145f4
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jun 30, 2025
300524c
fix backward compat issues and remove old tests
Johnetordoff Jul 2, 2025
3a682b3
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf…
Johnetordoff Jul 7, 2025
85e1342
split notification models into 3 files and improve interval choices
Johnetordoff Jul 7, 2025
f2e5309
clean-up tests and pass frequency data properly
Johnetordoff Jul 8, 2025
0471b76
update management commands and tests for notification migration
Johnetordoff Jul 8, 2025
9c3e02a
Update provider NotificationSubscription
Ostap-Zherebetskyi Jun 25, 2025
6b4ae9c
update notification subscription
Ostap-Zherebetskyi Jul 9, 2025
2dee1b2
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf…
Johnetordoff Jul 9, 2025
d11827d
Merge remote-tracking branch 'upstream/refactor-notifications' into f…
Ostap-Zherebetskyi Jul 10, 2025
52263a8
fix unit tests
Ostap-Zherebetskyi Jul 10, 2025
9465870
fix unit tests
Ostap-Zherebetskyi Jul 10, 2025
40378db
fix unit tests
Ostap-Zherebetskyi Jul 10, 2025
e255436
update RegisterUser notification_type
Ostap-Zherebetskyi Jul 16, 2025
ac039d9
Merge remote-tracking branch 'upstream/refactor-notifications' into f…
Ostap-Zherebetskyi Jul 16, 2025
6de0b63
update RegisterUser notification_type
Ostap-Zherebetskyi Jul 16, 2025
900f4b3
update notifications.yaml
Ostap-Zherebetskyi Jul 16, 2025
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
6 changes: 5 additions & 1 deletion admin/common_auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from django.contrib.auth import login, REDIRECT_FIELD_NAME, authenticate, logout

from osf.models.user import OSFUser
from osf.models import AdminProfile, AbstractProvider
from osf.models import AdminProfile, AbstractProvider, NotificationType
from admin.common_auth.forms import LoginForm, UserRegistrationForm, DeskUserForm


Expand Down Expand Up @@ -79,6 +79,10 @@ def form_valid(self, form):
provider = AbstractProvider.objects.get(id=provider_id)
provider.notification_subscriptions.get(event_name='new_pending_submissions').add_user_to_subscription(osf_user, 'email_transactional')

notification_type = NotificationType.objects.get(name=NotificationType.Type.USER_NEW_PENDING_SUBMISSIONS.value)

notification_type.emit(user=osf_user, message_frequency='instantly', event_context={'provider': provider._id})

osf_user.save()

if created:
Expand Down
8 changes: 4 additions & 4 deletions admin/notifications/views.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
from osf.models.notifications import NotificationSubscription
from osf.models.notifications import NotificationSubscriptionLegacy
from django.db.models import Count

def delete_selected_notifications(selected_ids):
NotificationSubscription.objects.filter(id__in=selected_ids).delete()
NotificationSubscriptionLegacy.objects.filter(id__in=selected_ids).delete()

def detect_duplicate_notifications(node_id=None):
query = NotificationSubscription.objects.values('_id').annotate(count=Count('_id')).filter(count__gt=1)
query = NotificationSubscriptionLegacy.objects.values('_id').annotate(count=Count('_id')).filter(count__gt=1)
if node_id:
query = query.filter(node_id=node_id)

detailed_duplicates = []
for dup in query:
notifications = NotificationSubscription.objects.filter(
notifications = NotificationSubscriptionLegacy.objects.filter(
_id=dup['_id']
).order_by('created')

Expand Down
19 changes: 10 additions & 9 deletions admin_tests/notifications/test_views.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import pytest
from django.test import RequestFactory
from osf.models import OSFUser, NotificationSubscription, Node
from osf.models import OSFUser, Node
from admin.notifications.views import (
delete_selected_notifications,
detect_duplicate_notifications,
)
from osf.models.notifications import NotificationSubscriptionLegacy
from tests.base import AdminTestCase

pytestmark = pytest.mark.django_db
Expand All @@ -18,19 +19,19 @@ def setUp(self):
self.request_factory = RequestFactory()

def test_delete_selected_notifications(self):
notification1 = NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event1')
notification2 = NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event2')
notification3 = NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event3')
notification1 = NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event1')
notification2 = NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event2')
notification3 = NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event3')

delete_selected_notifications([notification1.id, notification2.id])

assert not NotificationSubscription.objects.filter(id__in=[notification1.id, notification2.id]).exists()
assert NotificationSubscription.objects.filter(id=notification3.id).exists()
assert not NotificationSubscriptionLegacy.objects.filter(id__in=[notification1.id, notification2.id]).exists()
assert NotificationSubscriptionLegacy.objects.filter(id=notification3.id).exists()

def test_detect_duplicate_notifications(self):
NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event1')
NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event1')
NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event2')
NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event1')
NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event1')
NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event2')

duplicates = detect_duplicate_notifications()

Expand Down
11 changes: 11 additions & 0 deletions api/subscriptions/fields.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from rest_framework import serializers as ser

class FrequencyField(ser.ChoiceField):
def __init__(self, **kwargs):
super().__init__(choices=['none', 'instantly', 'daily', 'weekly', 'monthly'], **kwargs)

def to_representation(self, frequency: str):
return frequency or 'none'

def to_internal_value(self, freq):
return super().to_internal_value(freq)
7 changes: 2 additions & 5 deletions api/subscriptions/permissions.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
from rest_framework import permissions

from osf.models.notifications import NotificationSubscription
from osf.models.notification_subscription import NotificationSubscription


class IsSubscriptionOwner(permissions.BasePermission):

def has_object_permission(self, request, view, obj):
assert isinstance(obj, NotificationSubscription), f'obj must be a NotificationSubscription; got {obj}'
user_id = request.user.id
return obj.none.filter(id=user_id).exists() \
or obj.email_transactional.filter(id=user_id).exists() \
or obj.email_digest.filter(id=user_id).exists()
return obj.user == request.user
57 changes: 27 additions & 30 deletions api/subscriptions/serializers.py
Original file line number Diff line number Diff line change
@@ -1,58 +1,55 @@
from django.contrib.contenttypes.models import ContentType
from rest_framework import serializers as ser
from rest_framework.exceptions import ValidationError
from api.nodes.serializers import RegistrationProviderRelationshipField
from api.collections_providers.fields import CollectionProviderRelationshipField
from api.preprints.serializers import PreprintProviderRelationshipField
from osf.models import Node
from website.util import api_v2_url


from api.base.serializers import JSONAPISerializer, LinksField

NOTIFICATION_TYPES = {
'none': 'none',
'instant': 'email_transactional',
'daily': 'email_digest',
}


class FrequencyField(ser.Field):
def to_representation(self, obj):
user_id = self.context['request'].user.id
if obj.email_transactional.filter(id=user_id).exists():
return 'instant'
if obj.email_digest.filter(id=user_id).exists():
return 'daily'
return 'none'

def to_internal_value(self, frequency):
notification_type = NOTIFICATION_TYPES.get(frequency)
if notification_type:
return {'notification_type': notification_type}
raise ValidationError(f'Invalid frequency "{frequency}"')
from .fields import FrequencyField

class SubscriptionSerializer(JSONAPISerializer):
filterable_fields = frozenset([
'id',
'event_name',
'frequency',
])

id = ser.CharField(source='_id', read_only=True)
id = ser.CharField(
read_only=True,
source='legacy_id',
help_text='The id of the subscription fixed for backward compatibility',
)
event_name = ser.CharField(read_only=True)
frequency = FrequencyField(source='*', required=True)
links = LinksField({
'self': 'get_absolute_url',
})
frequency = FrequencyField(source='message_frequency', required=True)

class Meta:
type_ = 'subscription'

links = LinksField({
'self': 'get_absolute_url',
})

def get_absolute_url(self, obj):
return obj.absolute_api_v2_url

def update(self, instance, validated_data):
user = self.context['request'].user
notification_type = validated_data.get('notification_type')
instance.add_user_to_subscription(user, notification_type, save=True)
frequency = validated_data.get('frequency') or 'none'
instance.message_frequency = frequency

if frequency != 'none' and instance.content_type == ContentType.objects.get_for_model(Node):
node = Node.objects.get(
id=instance.id,
content_type=instance.content_type,
)
user_subs = node.parent_node.child_node_subscriptions
if node._id not in user_subs.setdefault(user._id, []):
user_subs[user._id].append(node._id)
node.parent_node.save()

return instance


Expand Down
Loading
Loading