Merge pull request #11405 from Johnetordoff/fix-node-subscriptions-on-demand

[ENG-9664] Fix node subscriptions API filtering

Author: Johnetordoff

api/subscriptions/views.py

@@ -56,7 +56,7 @@ def get_queryset(self):
             id=Cast(OuterRef('object_id'), IntegerField()),
         ).values('guids___id')[:1]
 
-        return NotificationSubscription.objects.filter(
+        qs = NotificationSubscription.objects.filter(
             notification_type__in=[
                 NotificationType.Type.USER_FILE_UPDATED.instance,
                 NotificationType.Type.NODE_FILE_UPDATED.instance,
@@ -91,6 +91,13 @@ def get_queryset(self):
             ),
         )
 
+        # 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
 
 class AbstractProviderSubscriptionList(SubscriptionList):
     def get_queryset(self):

api_tests/subscriptions/views/test_subscriptions_list.py

@@ -66,7 +66,6 @@ def test_list_complete(
             user,
             provider,
             node,
-            file_updated_notification,
             url
     ):
         res = app.get(url, auth=user.auth)
@@ -91,8 +90,37 @@ def test_cannot_post_patch_put_or_delete(self, app, url, user):
         assert put_res.status_code == 405
         assert delete_res.status_code == 405
 
-    def test_multiple_values_filter(self, app, url, file_updated_notification, user):
+    def test_multiple_values_filter(self, app, url, user):
         res = app.get(url + '?filter[event_name]=comments,file_updated', auth=user.auth)
         assert len(res.json['data']) == 2
         for subscription in res.json['data']:
             subscription['attributes']['event_name'] in ['global', 'comments']
+
+    def test_value_filter_id(
+        self,
+        app,
+        url,
+        user,
+        node,
+    ):
+        # Request all subscriptions first, to confirm all are visible
+        res = app.get(url, auth=user.auth)
+        all_ids = [sub['id'] for sub in res.json['data']]
+        assert len(all_ids) == 2
+        assert f'{node._id}_file_updated' in all_ids
+        assert f'{user._id}_global_file_updated' in all_ids
+
+        # Now filter by a specific annotated legacy_id (the node file_updated one)
+        target_id = f'{node._id}_file_updated'
+        filtered_res = app.get(f'{url}?filter[id]={target_id}', auth=user.auth)
+
+        # Response should contain exactly one record matching that legacy_id
+        assert filtered_res.status_code == 200
+        data = filtered_res.json['data']
+        assert len(data) == 1
+        assert data[0]['id'] == target_id
+
+        # Confirm it’s the expected subscription object
+        attributes = data[0]['attributes']
+        assert attributes['event_name'] is None  # event names are legacy
+        assert attributes['frequency'] in ['instantly', 'daily', 'none']

notifications/listeners.py

@@ -21,17 +21,24 @@ def subscribe_creator(resource):
             NotificationSubscription.objects.get_or_create(
                 user=user,
                 notification_type=NotificationType.Type.USER_FILE_UPDATED.instance,
+                object_id=user.id,
+                content_type=ContentType.objects.get_for_model(user),
                 _is_digest=True,
+                message_frequency='instantly',
+            )
+        except NotificationSubscription.MultipleObjectsReturned:
+            pass
+        try:
+            NotificationSubscription.objects.get_or_create(
+                user=user,
+                notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance,
+                object_id=resource.id,
+                content_type=ContentType.objects.get_for_model(resource),
+                _is_digest=True,
+                message_frequency='instantly',
             )
         except NotificationSubscription.MultipleObjectsReturned:
             pass
-        NotificationSubscription.objects.get_or_create(
-            user=user,
-            notification_type=NotificationType.Type.FILE_UPDATED.instance,
-            object_id=resource.id,
-            content_type=ContentType.objects.get_for_model(resource),
-            _is_digest=True,
-        )
 
 @contributor_added.connect
 def subscribe_contributor(resource, contributor, auth=None, *args, **kwargs):
@@ -54,7 +61,7 @@ def subscribe_contributor(resource, contributor, auth=None, *args, **kwargs):
             ),
             NotificationSubscription(
                 user=contributor,
-                notification_type=NotificationType.Type.FILE_UPDATED.instance,
+                notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance,
                 object_id=resource.id,
                 content_type=ContentType.objects.get_for_model(resource),
                 _is_digest=True,

osf_tests/management_commands/test_migrate_notifications.py

@@ -105,7 +105,6 @@ def test_migrate_provider_subscription(self, users, provider, provider2):
             assert subs.filter(object_id=obj.id, content_type=content_type).exists()
 
     def test_migrate_node_subscription(self, users, user, node):
-        self.create_legacy_sub('file_updated', users, user=user, node=node)
         migrate_legacy_notification_subscriptions()
         nt = NotificationType.objects.get(name=NotificationType.Type.NODE_FILE_UPDATED)
         assert nt.object_content_type == ContentType.objects.get_for_model(Node)