Skip to content

[ENG-8090] Upgrade Withdrawal/Retraction/Embargo Notifications #11212

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 26 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
26 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
2dee1b2
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf…
Johnetordoff Jul 9, 2025
56b1f75
Add notification type methods to EmailApprovableSanction and subclasses
Ostap-Zherebetskyi Jun 30, 2025
bb91b3c
Merge remote-tracking branch 'upstream/refactor-notifications' into f…
Ostap-Zherebetskyi Jul 10, 2025
23e6dc8
Merge remote-tracking branch 'upstream/refactor-notifications' into f…
Ostap-Zherebetskyi Jul 14, 2025
5786d90
fix unit tests
Ostap-Zherebetskyi Jul 14, 2025
04f2173
fix unit tests
Ostap-Zherebetskyi Jul 14, 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
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
163 changes: 110 additions & 53 deletions api/subscriptions/views.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from django.db.models import Value, When, Case, F, Q, OuterRef, Subquery
from django.db.models.fields import CharField, IntegerField
from django.db.models.functions import Concat, Cast
from django.contrib.contenttypes.models import ContentType
from rest_framework import generics
from rest_framework import permissions as drf_permissions
from rest_framework.exceptions import NotFound
from django.core.exceptions import ObjectDoesNotExist
from django.db.models import Q
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied

from framework.auth.oauth_scopes import CoreScopes
from api.base.views import JSONAPIBaseView
Expand All @@ -16,12 +19,13 @@
)
from api.subscriptions.permissions import IsSubscriptionOwner
from osf.models import (
NotificationSubscription,
CollectionProvider,
PreprintProvider,
RegistrationProvider,
AbstractProvider,
AbstractProvider, AbstractNode, Preprint, OSFUser,
)
from osf.models.notification_type import NotificationType
from osf.models.notification_subscription import NotificationSubscription


class SubscriptionList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin):
Expand All @@ -37,32 +41,59 @@ class SubscriptionList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin):
required_read_scopes = [CoreScopes.SUBSCRIPTIONS_READ]
required_write_scopes = [CoreScopes.NULL]

def get_default_queryset(self):
user = self.request.user
return NotificationSubscription.objects.filter(
Q(none=user) |
Q(email_digest=user) |
Q(
email_transactional=user,
),
).distinct()

def get_queryset(self):
return self.get_queryset_from_request()
user_guid = self.request.user._id
provider_ct = ContentType.objects.get(app_label='osf', model='abstractprovider')

provider_subquery = AbstractProvider.objects.filter(
id=Cast(OuterRef('object_id'), IntegerField()),
).values('_id')[:1]

node_subquery = AbstractNode.objects.filter(
id=Cast(OuterRef('object_id'), IntegerField()),
).values('guids___id')[:1]

return NotificationSubscription.objects.filter(user=self.request.user).annotate(
event_name=Case(
When(
notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value,
then=Value('files_updated'),
),
When(
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
then=Value('global_file_updated'),
),
default=F('notification_type__name'),
output_field=CharField(),
),
legacy_id=Case(
When(
notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value,
then=Concat(Subquery(node_subquery), Value('_file_updated')),
),
When(
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
then=Value(f'{user_guid}_global'),
),
When(
Q(notification_type__name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value) &
Q(content_type=provider_ct),
then=Concat(Subquery(provider_subquery), Value('_new_pending_submissions')),
),
default=F('notification_type__name'),
output_field=CharField(),
),
)


class AbstractProviderSubscriptionList(SubscriptionList):
def get_default_queryset(self):
user = self.request.user
def get_queryset(self):
provider = AbstractProvider.objects.get(_id=self.kwargs['provider_id'])
return NotificationSubscription.objects.filter(
provider___id=self.kwargs['provider_id'],
provider__type=self.provider_class._typedmodels_type,
).filter(
Q(none=user) |
Q(email_digest=user) |
Q(email_transactional=user),
).distinct()

object_id=provider,
provider__type=ContentType.objects.get_for_model(provider.__class__),
user=self.request.user,
)

class SubscriptionDetail(JSONAPIBaseView, generics.RetrieveUpdateAPIView):
view_name = 'notification-subscription-detail'
Expand All @@ -79,10 +110,63 @@ class SubscriptionDetail(JSONAPIBaseView, generics.RetrieveUpdateAPIView):

def get_object(self):
subscription_id = self.kwargs['subscription_id']
user_guid = self.request.user._id

provider_ct = ContentType.objects.get(app_label='osf', model='abstractprovider')
node_ct = ContentType.objects.get(app_label='osf', model='abstractnode')

provider_subquery = AbstractProvider.objects.filter(
id=Cast(OuterRef('object_id'), IntegerField()),
).values('_id')[:1]

node_subquery = AbstractNode.objects.filter(
id=Cast(OuterRef('object_id'), IntegerField()),
).values('guids___id')[:1]

guid_id, *event_parts = subscription_id.split('_')
event = '_'.join(event_parts) if event_parts else ''

subscription_obj = AbstractNode.load(guid_id) or Preprint.load(guid_id) or OSFUser.load(guid_id)

if event != 'global':
obj_filter = Q(
object_id=getattr(subscription_obj, 'id', None),
content_type=ContentType.objects.get_for_model(subscription_obj.__class__),
notification_type__name__icontains=event,
)
else:
obj_filter = Q()

try:
obj = NotificationSubscription.objects.get(_id=subscription_id)
obj = NotificationSubscription.objects.annotate(
legacy_id=Case(
When(
notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value,
content_type=node_ct,
then=Concat(Subquery(node_subquery), Value('_file_updated')),
),
When(
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
then=Value(f'{user_guid}_global'),
),
When(
notification_type__name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value,
content_type=provider_ct,
then=Concat(Subquery(provider_subquery), Value('_new_pending_submissions')),
),
default=Value(f'{user_guid}_global'),
output_field=CharField(),
),
).filter(obj_filter)

except ObjectDoesNotExist:
raise NotFound

try:
obj = obj.filter(user=self.request.user).get()
except ObjectDoesNotExist:
raise PermissionDenied

self.check_object_permissions(self.request, obj)
return obj

Expand All @@ -100,33 +184,6 @@ class AbstractProviderSubscriptionDetail(SubscriptionDetail):
required_write_scopes = [CoreScopes.SUBSCRIPTIONS_WRITE]
provider_class = None

def __init__(self, *args, **kwargs):
assert issubclass(self.provider_class, AbstractProvider), 'Class must be subclass of AbstractProvider'
super().__init__(*args, **kwargs)

def get_object(self):
subscription_id = self.kwargs['subscription_id']
if self.kwargs.get('provider_id'):
provider = self.provider_class.objects.get(_id=self.kwargs.get('provider_id'))
try:
obj = NotificationSubscription.objects.get(
_id=subscription_id,
provider_id=provider.id,
)
except ObjectDoesNotExist:
raise NotFound
else:
try:
obj = NotificationSubscription.objects.get(
_id=subscription_id,
provider__type=self.provider_class._typedmodels_type,
)
except ObjectDoesNotExist:
raise NotFound
self.check_object_permissions(self.request, obj)
return obj


class CollectionProviderSubscriptionDetail(AbstractProviderSubscriptionDetail):
provider_class = CollectionProvider
serializer_class = CollectionSubscriptionSerializer
Expand Down
Loading