Merge pull request #11556 from cslzchen/feature/create-missing-subscriptions-otf

[ENG-10080] Create global_file_updated, global_reviews and node_file_updated subscriptions if missing

Author: cslzchen

api/subscriptions/utils.py

@@ -0,0 +1,82 @@
+from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import PermissionDenied
+
+from rest_framework.exceptions import NotFound
+
+from framework import sentry
+
+from osf.models import AbstractNode, OSFUser
+from osf.models.notification_type import NotificationType
+from osf.models.notification_subscription import NotificationSubscription
+
+
+def create_missing_notification_from_legacy_id(legacy_id, user):
+    """
+    `global_file_updated` and `global_reviews` should exist by default for every user, and `<node_id>_files_update`
+    should exist by default if user is a contributor of the node. If not found, create them with `none` frequency
+    and `_is_digest=True` as default. Raise error if not found, not authorized or permission denied.
+    """
+
+    node_ct = ContentType.objects.get_for_model(AbstractNode)
+    user_ct = ContentType.objects.get_for_model(OSFUser)
+
+    user_file_updated_nt = NotificationType.Type.USER_FILE_UPDATED.instance
+    reviews_submission_status_nt = NotificationType.Type.REVIEWS_SUBMISSION_STATUS.instance
+    node_file_updated_nt = NotificationType.Type.NODE_FILE_UPDATED.instance
+
+    node_guid = 'n/a'
+
+    if legacy_id == f'{user._id}_global_file_updated':
+        notification_type = user_file_updated_nt
+        content_type = user_ct
+        object_id = user.id
+    elif legacy_id == f'{user._id}_global_reviews':
+        notification_type = reviews_submission_status_nt
+        content_type = user_ct
+        object_id = user.id
+    elif legacy_id.endswith('_global_file_updated') or legacy_id.endswith('_global_reviews'):
+        # Mismatched request user and subscription user
+        sentry.log_message(f'Permission denied: [user={user._id}, legacy_id={legacy_id}]')
+        raise PermissionDenied
+    # `<node_id>_files_update` should exist by default if user is a contributor of the node.
+    # If not found, create them with `none` frequency and `_is_digest=True` as default.
+    elif legacy_id.endswith('_file_updated'):
+        notification_type = node_file_updated_nt
+        content_type = node_ct
+        node_guid = legacy_id[:-len('_file_updated')]
+        node = AbstractNode.objects.filter(guids___id=node_guid, is_deleted=False, type='osf.node').first()
+        if not node:
+            # The node in the legacy subscription ID does not exist or is invalid
+            sentry.log_message(
+                f'Node not found in legacy subscription ID: [user={user._id}, legacy_id={legacy_id}]',
+            )
+            raise NotFound
+        if not node.is_contributor(user):
+            # The request user is not a contributor of the node
+            sentry.log_message(
+                f'Permission denied: [user={user._id}], node={node_guid}, legacy_id={legacy_id}]',
+            )
+            raise PermissionDenied
+        object_id = node.id
+    else:
+        sentry.log_message(f'Subscription not found: [user={user._id}, legacy_id={legacy_id}]')
+        raise NotFound
+    missing_subscription_created = NotificationSubscription.objects.create(
+        notification_type=notification_type,
+        user=user,
+        content_type=content_type,
+        object_id=object_id,
+        _is_digest=True,
+        message_frequency='none',
+    )
+    sentry.log_message(
+        f'Missing default subscription has been created: [user={user._id}], node={node_guid} type={notification_type}, legacy_id={legacy_id}]',
+    )
+    return missing_subscription_created
+
+def create_missing_notifications_from_event_name(filter_event_names, user):
+    # Note: this may not be needed since 1) missing node subscriptions are created in the LIST view when filter by
+    # legacy ID, and 2) missing user global subscriptions are created in DETAILS view with legacy ID. However, log
+    # this message to sentry for tracking how often this happens.
+    sentry.log_message(f'Detected empty subscription list when filter by event names: [event={filter_event_names}, user={user._id}]')
+    return None

api/subscriptions/views.py

@@ -1,13 +1,15 @@
+from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import PermissionDenied
 from django.db.models import Value, When, Case, OuterRef, Subquery, F
 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, PermissionDenied
 from rest_framework.response import Response
+
 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
@@ -18,13 +20,16 @@
     RegistrationSubscriptionSerializer,
 )
 from api.subscriptions.permissions import IsSubscriptionOwner
+from api.subscriptions import utils
+
 from osf.models import (
     CollectionProvider,
     PreprintProvider,
     RegistrationProvider,
     AbstractProvider,
     AbstractNode,
     Guid,
+    OSFUser,
 )
 from osf.models.notification_type import NotificationType
 from osf.models.notification_subscription import NotificationSubscription
@@ -44,11 +49,16 @@ class SubscriptionList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin):
     required_write_scopes = [CoreScopes.NULL]
 
     def get_queryset(self):
+
+        user = self.request.user
         user_guid = self.request.user._id
 
-        provider_ct = ContentType.objects.get_by_natural_key(app_label='osf', model='abstractprovider')
-        node_ct = ContentType.objects.get_by_natural_key(app_label='osf', model='abstractnode')
-        user_ct = ContentType.objects.get_by_natural_key(app_label='osf', model='osfuser')
+        filter_id = self.request.query_params.get('filter[id]')
+        filter_event_name = self.request.query_params.get('filter[event_name]')
+
+        provider_ct = ContentType.objects.get_for_model(AbstractProvider)
+        node_ct = ContentType.objects.get_for_model(AbstractNode)
+        user_ct = ContentType.objects.get_for_model(OSFUser)
 
         node_subquery = AbstractNode.objects.filter(
             id=Cast(OuterRef('object_id'), IntegerField()),
@@ -85,15 +95,16 @@ def get_queryset(self):
             NotificationType.Type.FILE_UPDATED.value,
         ]
 
-        qs = NotificationSubscription.objects.filter(
-            notification_type__name__in=_global_reviews_provider + _global_reviews_user + _global_file_updated + _node_file_updated,
-            user=self.request.user,
+        full_set_of_types = _global_reviews_provider + _global_reviews_user + _global_file_updated + _node_file_updated
+        annotated_qs = NotificationSubscription.objects.filter(
+            notification_type__name__in=full_set_of_types,
+            user=user,
         ).annotate(
             event_name=Case(
                 When(
                     notification_type__name__in=_node_file_updated,
                     content_type=node_ct,
-                    then=Value('files_updated'),
+                    then=Value('file_updated'),
                 ),
                 When(
                     notification_type__name__in=_global_file_updated,
@@ -135,17 +146,31 @@ def get_queryset(self):
             ),
         ).distinct('legacy_id')
 
+        return_qs = annotated_qs
+
         # Apply manual filter for legacy_id if requested
-        filter_id = self.request.query_params.get('filter[id]')
         if filter_id:
-            qs = qs.filter(legacy_id=filter_id)
-            # convert to list comprehension because legacy_id is an annotation, not in DB
+            return_qs = annotated_qs.filter(legacy_id=filter_id)
+            # TODO: Rework missing subscription fix after fully populating the OSF DB with all missing notifications
+            # NOTE: `.exists()` errors for unknown reason, possibly due to complex annotation with `.distinct()`
+            if return_qs.count() == 0:
+                missing_subscription_created = utils.create_missing_notification_from_legacy_id(filter_id, user)
+                if missing_subscription_created:
+                    return_qs = annotated_qs.filter(legacy_id=filter_id)
+            # `filter_id` takes priority over `filter_event_name`
+            return return_qs
+
         # Apply manual filter for event_name if requested
-        filter_event_name = self.request.query_params.get('filter[event_name]')
         if filter_event_name:
-            qs = qs.filter(event_name__in=filter_event_name.split(','))
+            filter_event_names = filter_event_name.split(',')
+            return_qs = annotated_qs.filter(event_name__in=filter_event_names)
+            # TODO: Rework missing subscription fix after fully populating the OSF DB with all missing notifications
+            # NOTE: `.exists()` errors for unknown reason, possibly due to complex annotation with `.distinct()`
+            if return_qs.count() == 0:
+                utils.create_missing_notifications_from_event_name(filter_event_names, user)
+
+        return return_qs
 
-        return qs
 
 class AbstractProviderSubscriptionList(SubscriptionList):
     def get_queryset(self):
@@ -171,52 +196,60 @@ class SubscriptionDetail(JSONAPIBaseView, generics.RetrieveUpdateAPIView):
 
     def get_object(self):
         subscription_id = self.kwargs['subscription_id']
+        user = self.request.user
         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')
+        user_ct = ContentType.objects.get_for_model(OSFUser)
+        node_ct = ContentType.objects.get_for_model(AbstractNode)
 
         node_subquery = AbstractNode.objects.filter(
             id=Cast(OuterRef('object_id'), IntegerField()),
         ).values('guids___id')[:1]
 
-        try:
-            annotated_obj_qs = NotificationSubscription.objects.filter(user=self.request.user).annotate(
-                legacy_id=Case(
-                    When(
-                        notification_type__name=NotificationType.Type.NODE_FILE_UPDATED.value,
-                        content_type=node_ct,
-                        then=Concat(Subquery(node_subquery), Value('_files_updated')),
-                    ),
-                    When(
-                        notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
-                        then=Value(f'{user_guid}_global_file_updated'),
-                    ),
-                    When(
-                        notification_type__name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value,
-                        content_type=provider_ct,
-                        then=Value(f'{user_guid}_global_reviews'),
-                    ),
-                    default=Value(f'{user_guid}_global'),
-                    output_field=CharField(),
+        missing_subscription_created = None
+        annotated_obj_qs = NotificationSubscription.objects.filter(user=user).annotate(
+            legacy_id=Case(
+                When(
+                    notification_type__name=NotificationType.Type.NODE_FILE_UPDATED.value,
+                    content_type=node_ct,
+                    then=Concat(Subquery(node_subquery), Value('_file_updated')),
                 ),
-            )
-            obj = annotated_obj_qs.filter(legacy_id=subscription_id)
-
-        except ObjectDoesNotExist:
-            raise NotFound
-
-        obj = obj.filter(user=self.request.user).first()
-        if not obj:
+                When(
+                    notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
+                    then=Value(f'{user_guid}_global_file_updated'),
+                ),
+                When(
+                    notification_type__name=NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value,
+                    content_type=user_ct,
+                    then=Value(f'{user_guid}_global_reviews'),
+                ),
+                default=Value(f'{user_guid}_global'),
+                output_field=CharField(),
+            ),
+        )
+        existing_subscriptions = annotated_obj_qs.filter(legacy_id=subscription_id)
+
+        # TODO: Rework missing subscription fix after fully populating the OSF DB with all missing notifications
+        if not existing_subscriptions.exists():
+            missing_subscription_created = utils.create_missing_notification_from_legacy_id(subscription_id, user)
+        if missing_subscription_created:
+            # Note: must use `annotated_obj_qs` to insert `legacy_id` so that `SubscriptionSerializer` can build data
+            # properly; in addition, there should be only one result
+            subscription = annotated_obj_qs.get(legacy_id=subscription_id)
+        else:
+            # TODO: Use `get()` and fails/warns on multiple objects after fully de-duplicating the OSF DB
+            subscription = existing_subscriptions.order_by('id').last()
+        if not subscription:
             raise PermissionDenied
 
-        self.check_object_permissions(self.request, obj)
-        return obj
+        self.check_object_permissions(self.request, subscription)
+        return subscription
 
     def update(self, request, *args, **kwargs):
         """
         Update a notification subscription
         """
+        self.get_object()
+
         if '_global_file_updated' in self.kwargs['subscription_id']:
             # Copy _global_file_updated subscription changes to all file subscriptions
             qs = NotificationSubscription.objects.filter(
@@ -261,9 +294,9 @@ def update(self, request, *args, **kwargs):
                 serializer.is_valid(raise_exception=True)
                 self.perform_update(serializer)
             return Response(serializer.data)
-        elif '_files_updated' in self.kwargs['subscription_id']:
-            # Copy _files_updated subscription changes to all node file subscriptions
-            node_id = Guid.load(self.kwargs['subscription_id'].split('_files_updated')[0]).object_id
+        elif '_file_updated' in self.kwargs['subscription_id']:
+            # Copy _file_updated subscription changes to all node file subscriptions
+            node_id = Guid.load(self.kwargs['subscription_id'].split('_file_updated')[0]).object_id
 
             qs = NotificationSubscription.objects.filter(
                 user=self.request.user,