New Notification system proof of concept

Author: Johnetordoff

admin/notifications/views.py

@@ -1,4 +1,4 @@
-from osf.models.notifications import NotificationSubscription
+from osf.models.notification import NotificationSubscription
 from django.db.models import Count
 
 def delete_selected_notifications(selected_ids):

api/base/views.py

@@ -143,14 +143,14 @@ def get_serializer_context(self):
          multiple levels of nesting.
         """
         context = super().get_serializer_context()
-        if self.kwargs.get('is_embedded'):
+        if self.kwargs.get('is_embedded') or not self.request:
             embeds = []
         else:
             embeds = self.request.query_params.getlist('embed') or self.request.query_params.getlist('embed[]')
 
         fields_check = self.get_serializer_class()._declared_fields.copy()
         serializer_class_type = get_meta_type(self.serializer_class, self.request)
-        if f'fields[{serializer_class_type}]' in self.request.query_params:
+        if self.request and f'fields[{serializer_class_type}]' in self.request.query_params:
             # Check only requested and mandatory fields
             sparse_fields = self.request.query_params[f'fields[{serializer_class_type}]']
             for field in list(fields_check.copy().keys()):

api/subscriptions/fields.py

@@ -0,0 +1,12 @@
+from rest_framework import serializers as ser
+from osf.models import NotificationSubscription
+
+class FrequencyField(ser.ChoiceField):
+    def __init__(self, **kwargs):
+        super().__init__(choices=['none', 'instantly', 'daily', 'weekly', 'monthly'], **kwargs)
+
+    def to_representation(self, obj: NotificationSubscription):
+        return obj.message_frequency
+
+    def to_internal_value(self, freq):
+        return super().to_internal_value(freq)

api/subscriptions/permissions.py

@@ -1,13 +1,7 @@
 from rest_framework import permissions
-
-from osf.models.notifications import NotificationSubscription
-
+from osf.models 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()
+        assert isinstance(obj, NotificationSubscription), f'obj must be NotificationSubscription; got {obj}'
+        return obj.user == request.user

api/subscriptions/serializers.py

@@ -1,100 +1,69 @@
 from rest_framework import serializers as ser
-from rest_framework.exceptions import ValidationError
+from api.base.serializers import JSONAPISerializer, LinksField
+from website.util import api_v2_url
+from .fields import FrequencyField
 from api.nodes.serializers import RegistrationProviderRelationshipField
 from api.collections_providers.fields import CollectionProviderRelationshipField
 from api.preprints.serializers import PreprintProviderRelationshipField
-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}"')
-
 class SubscriptionSerializer(JSONAPISerializer):
-    filterable_fields = frozenset([
-        'id',
-        'event_name',
-    ])
-
-    id = ser.CharField(source='_id', read_only=True)
-    event_name = ser.CharField(read_only=True)
-    frequency = FrequencyField(source='*', required=True)
-    links = LinksField({
-        'self': 'get_absolute_url',
-    })
+    filterable_fields = frozenset(['id', 'type'])
+
+    id = ser.CharField(read_only=True)
+    type = ser.SerializerMethodField()
+    email_freq = FrequencyField(required=True)
+    links = LinksField({'self': 'get_absolute_url'})
 
     class Meta:
         type_ = 'subscription'
 
+    def get_type(self, obj):
+        return self.Meta.type_
+
     def get_absolute_url(self, obj):
-        return obj.absolute_api_v2_url
+        return api_v2_url(f'subscriptions/{obj.pk}')
 
     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)
+        instance.message_frequency = validated_data.get('message_frequency', instance.message_frequency)
+        instance.save(update_fields=['message_frequency'])
         return instance
 
-
 class RegistrationSubscriptionSerializer(SubscriptionSerializer):
     provider = RegistrationProviderRelationshipField(
         related_view='providers:registration-providers:registration-provider-detail',
-        related_view_kwargs={'provider_id': '<provider._id>'},
-        read_only=False,
-        required=False,
+        related_view_kwargs={'provider_id': '<subscribed_object._id>'},
+        read_only=True,  # We're not modifying the relationship from here
     )
 
     def get_absolute_url(self, obj):
-        return api_v2_url(f'registration_subscriptions/{obj._id}')
+        return api_v2_url(f'registration_subscriptions/{obj.pk}')
 
     class Meta:
         type_ = 'registration-subscription'
 
-
 class CollectionSubscriptionSerializer(SubscriptionSerializer):
     provider = CollectionProviderRelationshipField(
         related_view='providers:collection-providers:collection-provider-detail',
-        related_view_kwargs={'provider_id': '<provider._id>'},
-        read_only=False,
-        required=False,
+        related_view_kwargs={'provider_id': '<subscribed_object._id>'},
+        read_only=True,
     )
 
     def get_absolute_url(self, obj):
-        return api_v2_url(f'collection_subscriptions/{obj._id}')
+        return api_v2_url(f'collection_subscriptions/{obj.pk}')
 
     class Meta:
         type_ = 'collection-subscription'
 
-
 class PreprintSubscriptionSerializer(SubscriptionSerializer):
     provider = PreprintProviderRelationshipField(
         related_view='providers:preprint-providers:preprint-provider-detail',
-        related_view_kwargs={'provider_id': '<provider._id>'},
-        read_only=False,
+        related_view_kwargs={'provider_id': '<subscribed_object._id>'},
+        read_only=True,
     )
 
     def get_absolute_url(self, obj):
-        return api_v2_url(f'preprints_subscriptions/{obj._id}')
+        return api_v2_url(f'preprints_subscriptions/{obj.pk}')
 
     class Meta:
         type_ = 'preprint-subscription'

api/subscriptions/views.py

@@ -1,34 +1,27 @@
-from rest_framework import generics
-from rest_framework import permissions as drf_permissions
+from api.base.filters import ListFilterMixin
+from api.subscriptions.serializers import SubscriptionSerializer
+from osf.models.notification import NotificationSubscription
+from django.contrib.contenttypes.models import ContentType
+from rest_framework import generics, permissions as drf_permissions
 from rest_framework.exceptions import NotFound
-from django.core.exceptions import ObjectDoesNotExist
-from django.db.models import Q
+from django.shortcuts import get_object_or_404
+
+from osf.models import AbstractProvider, CollectionProvider, PreprintProvider, RegistrationProvider
 
-from framework.auth.oauth_scopes import CoreScopes
 from api.base.views import JSONAPIBaseView
-from api.base.filters import ListFilterMixin
 from api.base import permissions as base_permissions
+from api.subscriptions.permissions import IsSubscriptionOwner
 from api.subscriptions.serializers import (
-    SubscriptionSerializer,
     CollectionSubscriptionSerializer,
     PreprintSubscriptionSerializer,
     RegistrationSubscriptionSerializer,
 )
-from api.subscriptions.permissions import IsSubscriptionOwner
-from osf.models import (
-    NotificationSubscription,
-    CollectionProvider,
-    PreprintProvider,
-    RegistrationProvider,
-    AbstractProvider,
-)
-
+from framework.auth.oauth_scopes import CoreScopes
 
 class SubscriptionList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin):
     view_name = 'notification-subscription-list'
     view_category = 'notification-subscriptions'
     serializer_class = SubscriptionSerializer
-    model_class = NotificationSubscription
     permission_classes = (
         drf_permissions.IsAuthenticated,
         base_permissions.TokenHasScope,
@@ -37,31 +30,8 @@ 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()
-
-
-class AbstractProviderSubscriptionList(SubscriptionList):
-    def get_default_queryset(self):
-        user = self.request.user
-        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()
+        return NotificationSubscription.objects.filter(user=self.request.user)
 
 
 class SubscriptionDetail(JSONAPIBaseView, generics.RetrieveUpdateAPIView):
@@ -78,80 +48,102 @@ class SubscriptionDetail(JSONAPIBaseView, generics.RetrieveUpdateAPIView):
     required_write_scopes = [CoreScopes.SUBSCRIPTIONS_WRITE]
 
     def get_object(self):
-        subscription_id = self.kwargs['subscription_id']
         try:
-            obj = NotificationSubscription.objects.get(_id=subscription_id)
-        except ObjectDoesNotExist:
+            sub = NotificationSubscription.objects.get(pk=self.kwargs['pk'])
+        except NotificationSubscription.DoesNotExist:
             raise NotFound
-        self.check_object_permissions(self.request, obj)
-        return obj
+        self.check_object_permissions(self.request, sub)
+        return sub
 
 
-class AbstractProviderSubscriptionDetail(SubscriptionDetail):
+class AbstractProviderSubscriptionDetail(JSONAPIBaseView, generics.RetrieveUpdateAPIView):
     view_name = 'provider-notification-subscription-detail'
     view_category = 'notification-subscriptions'
     permission_classes = (
         drf_permissions.IsAuthenticated,
         base_permissions.TokenHasScope,
         IsSubscriptionOwner,
     )
-
     required_read_scopes = [CoreScopes.SUBSCRIPTIONS_READ]
     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)
+    provider_class = None  # Must be set in subclass
+    serializer_class = None  # Must be set in subclass
 
     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
+        assert issubclass(self.provider_class, AbstractProvider), 'Must set provider_class to an AbstractProvider subclass'
 
+        subscription_id = self.kwargs.get('pk')
+        provider_id = self.kwargs.get('provider_id')
+
+        # Get provider
+        provider = get_object_or_404(self.provider_class, _id=provider_id)
+        content_type = ContentType.objects.get_for_model(self.provider_class)
+
+        try:
+            sub = NotificationSubscription.objects.get(
+                pk=subscription_id,
+                content_type=content_type,
+                object_id=provider.id,
+            )
+        except NotificationSubscription.DoesNotExist:
+            raise NotFound
+
+        self.check_object_permissions(self.request, sub)
+        return sub
+
+
+class AbstractProviderSubscriptionList(JSONAPIBaseView, generics.ListAPIView):
+    permission_classes = (
+        drf_permissions.IsAuthenticated,
+        base_permissions.TokenHasScope,
+    )
+    required_read_scopes = [CoreScopes.SUBSCRIPTIONS_READ]
+    provider_class = None
+    serializer_class = None
+
+    def get_queryset(self):
+        assert issubclass(self.provider_class, AbstractProvider), 'Must set provider_class to an AbstractProvider subclass'
+        provider_id = self.kwargs.get('provider_id')
+        provider = get_object_or_404(self.provider_class, _id=provider_id)
+
+        return NotificationSubscription.objects.filter(
+            user=self.request.user,
+            content_type=ContentType.objects.get_for_model(self.provider_class),
+            object_id=provider.id,
+        )
 
 class CollectionProviderSubscriptionDetail(AbstractProviderSubscriptionDetail):
+    view_name = 'provider-notification-subscription-detail'
+    view_category = 'notification-subscriptions'
     provider_class = CollectionProvider
     serializer_class = CollectionSubscriptionSerializer
 
-
 class PreprintProviderSubscriptionDetail(AbstractProviderSubscriptionDetail):
+    view_name = 'provider-notification-subscription-detail'
+    view_category = 'notification-subscriptions'
     provider_class = PreprintProvider
     serializer_class = PreprintSubscriptionSerializer
 
-
 class RegistrationProviderSubscriptionDetail(AbstractProviderSubscriptionDetail):
+    view_name = 'provider-notification-subscription-detail'
+    view_category = 'notification-subscriptions'
     provider_class = RegistrationProvider
     serializer_class = RegistrationSubscriptionSerializer
 
-
 class CollectionProviderSubscriptionList(AbstractProviderSubscriptionList):
+    view_name = 'provider-notification-subscription-detail'
+    view_category = 'notification-subscriptions'
     provider_class = CollectionProvider
     serializer_class = CollectionSubscriptionSerializer
 
-
 class PreprintProviderSubscriptionList(AbstractProviderSubscriptionList):
+    view_name = 'provider-notification-subscription-detail'
+    view_category = 'notification-subscriptions'
     provider_class = PreprintProvider
     serializer_class = PreprintSubscriptionSerializer
 
-
 class RegistrationProviderSubscriptionList(AbstractProviderSubscriptionList):
+    view_name = 'provider-notification-subscription-detail'
+    view_category = 'notification-subscriptions'
     provider_class = RegistrationProvider
     serializer_class = RegistrationSubscriptionSerializer