diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index 54fcb07e30b..1a1450a412e 100644 --- a/api/subscriptions/views.py +++ b/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): diff --git a/api_tests/subscriptions/views/test_subscriptions_list.py b/api_tests/subscriptions/views/test_subscriptions_list.py index 101e84fc0a3..cd5e1699614 100644 --- a/api_tests/subscriptions/views/test_subscriptions_list.py +++ b/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'] diff --git a/notifications/listeners.py b/notifications/listeners.py index 8cbc43f1eca..7fd96aba506 100644 --- a/notifications/listeners.py +++ b/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, diff --git a/osf_tests/management_commands/test_migrate_notifications.py b/osf_tests/management_commands/test_migrate_notifications.py index ae223337f29..306f1c7130d 100644 --- a/osf_tests/management_commands/test_migrate_notifications.py +++ b/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)