From 62af77746a20cbf679a4626970b7e793e3531bf6 Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Mon, 19 Jan 2026 11:29:45 -0500 Subject: [PATCH 01/10] Create global_file_updated and global_reviews subscriptions if missing * Use USER_FILE_UPDATED for group global_file_updated * Use REVIEWS_SUBMISSION_STATUS for group global_reviews --- api/subscriptions/views.py | 56 +++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index 953e2a66aec..e1b7ea66e57 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -1,13 +1,17 @@ +from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.db.models import Value, When, Case, 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, PermissionDenied from rest_framework.response import Response + +from framework import sentry 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,6 +22,7 @@ RegistrationSubscriptionSerializer, ) from api.subscriptions.permissions import IsSubscriptionOwner + from osf.models import ( CollectionProvider, PreprintProvider, @@ -25,6 +30,7 @@ AbstractProvider, AbstractNode, Guid, + OSFUser, ) from osf.models.notification_type import NotificationType from osf.models.notification_subscription import NotificationSubscription @@ -156,14 +162,18 @@ 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') + user_file_updated_nt = NotificationType.Type.USER_FILE_UPDATED.instance + reviews_submission_status_nt = NotificationType.Type.REVIEWS_SUBMISSION_STATUS.instance + user_ct = ContentType.objects.get_for_model(OSFUser) + node_ct = ContentType.objects.get_for_model(AbstractNode) + provider_ct = ContentType.objects.get_for_model(AbstractProvider) node_subquery = AbstractNode.objects.filter( id=Cast(OuterRef('object_id'), IntegerField()), ).values('guids___id')[:1] + existing_subscriptions = None + missing_subscription_created = None try: annotated_obj_qs = NotificationSubscription.objects.filter(user=self.request.user).annotate( legacy_id=Case( @@ -185,17 +195,37 @@ def get_object(self): output_field=CharField(), ), ) - obj = annotated_obj_qs.filter(legacy_id=subscription_id) - + existing_subscriptions = annotated_obj_qs.filter(legacy_id=subscription_id) except ObjectDoesNotExist: - raise NotFound + # `global_file_updated` and `global_reviews` should exist by default for every user + # if not found, create them with `none` frequency as default. + if subscription_id == f'{user_guid}_global_file_updated': + notification_type = user_file_updated_nt + elif subscription_id == f'{user_guid}_global_reviews': + notification_type = reviews_submission_status_nt + else: + raise NotFound + sentry.log_message(f'Missing default subscription has been created: [user={user_guid}], type={notification_type}, id={subscription_id}') + missing_subscription_created = NotificationSubscription.objects.create( + notification_type=notification_type, + user=self.request.user, + content_type=user_ct, + object_id=self.request.user.id, + defaults={ + 'is_digest': True, + 'message_frequency': 'none', + }, + ) - obj = obj.filter(user=self.request.user).first() - if not obj: - raise PermissionDenied + if missing_subscription_created: + subscription = missing_subscription_created + else: + subscription = existing_subscriptions.filter(user=self.request.user).order_by('id').first() + 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): """ From 3093f88460c7df7a26a3fdee2b7ff25b6000eef2 Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Mon, 19 Jan 2026 20:08:49 -0500 Subject: [PATCH 02/10] Add missing `is_digest=True` for new OSF user subscriptions --- website/mailchimp_utils.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/website/mailchimp_utils.py b/website/mailchimp_utils.py index 7e92a59d275..3d44d489131 100644 --- a/website/mailchimp_utils.py +++ b/website/mailchimp_utils.py @@ -126,7 +126,10 @@ def subscribe_on_confirm(user): notification_type=NotificationType.Type.REVIEWS_SUBMISSION_STATUS.instance, content_type=ContentType.objects.get_for_model(user), object_id=user.id, - defaults={'message_frequency': 'instantly'}, + defaults={ + 'is_digest': True, + 'message_frequency': 'instantly', + }, ) NotificationSubscription.objects.get_or_create( @@ -134,5 +137,8 @@ def subscribe_on_confirm(user): notification_type=NotificationType.Type.USER_FILE_UPDATED.instance, content_type=ContentType.objects.get_for_model(user), object_id=user.id, - defaults={'message_frequency': 'instantly'}, + defaults={ + 'is_digest': True, + 'message_frequency': 'instantly', + }, ) From f23108ba825de6d80c410d95f6d421e0a36e705e Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Mon, 19 Jan 2026 20:29:57 -0500 Subject: [PATCH 03/10] Extend otf subscription creation to apply to _node_file_updated group --- api/subscriptions/views.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index e1b7ea66e57..23d9b128670 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -164,6 +164,7 @@ def get_object(self): user_guid = self.request.user._id 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 user_ct = ContentType.objects.get_for_model(OSFUser) node_ct = ContentType.objects.get_for_model(AbstractNode) provider_ct = ContentType.objects.get_for_model(AbstractProvider) @@ -172,6 +173,7 @@ def get_object(self): id=Cast(OuterRef('object_id'), IntegerField()), ).values('guids___id')[:1] + node_guid = 'n/a' existing_subscriptions = None missing_subscription_created = None try: @@ -200,17 +202,31 @@ def get_object(self): # `global_file_updated` and `global_reviews` should exist by default for every user # if not found, create them with `none` frequency as default. if subscription_id == f'{user_guid}_global_file_updated': + # TODO: should we verify user auth/permissions? notification_type = user_file_updated_nt + content_type = user_ct + object_id = self.request.user.id elif subscription_id == f'{user_guid}_global_reviews': + # TODO: should we verify user auth/permissions? notification_type = reviews_submission_status_nt + content_type = user_ct + object_id = self.request.user.id + elif subscription_id.endswith('_files_updated'): + node_guid = subscription_id[:-len('_files_updated')] + node = AbstractNode.objects.get(guid___id=node_guid) + # TODO: should we verify node contributorship and user auth/permissions? + notification_type = node_file_updated_nt + content_type = node_ct + object_id = node.id else: + # TODO: what is this `{user_guid}_global` legacy ID? Is this the same as not found? raise NotFound - sentry.log_message(f'Missing default subscription has been created: [user={user_guid}], type={notification_type}, id={subscription_id}') + sentry.log_message(f'Missing default subscription has been created: [user={user_guid}], node={node_guid} type={notification_type}, legacy_id={subscription_id}]') missing_subscription_created = NotificationSubscription.objects.create( notification_type=notification_type, user=self.request.user, - content_type=user_ct, - object_id=self.request.user.id, + content_type=content_type, + object_id=object_id, defaults={ 'is_digest': True, 'message_frequency': 'none', From abdfc1bbfd285ff2902da1bea38f8ccd98a95102 Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Mon, 19 Jan 2026 20:38:51 -0500 Subject: [PATCH 04/10] Fix typo for `_is_digest` --- api/subscriptions/views.py | 2 +- website/mailchimp_utils.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index 23d9b128670..2b7cf98e8f0 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -228,7 +228,7 @@ def get_object(self): content_type=content_type, object_id=object_id, defaults={ - 'is_digest': True, + '_is_digest': True, 'message_frequency': 'none', }, ) diff --git a/website/mailchimp_utils.py b/website/mailchimp_utils.py index 3d44d489131..e3d7a546d0f 100644 --- a/website/mailchimp_utils.py +++ b/website/mailchimp_utils.py @@ -127,7 +127,7 @@ def subscribe_on_confirm(user): content_type=ContentType.objects.get_for_model(user), object_id=user.id, defaults={ - 'is_digest': True, + '_is_digest': True, 'message_frequency': 'instantly', }, ) @@ -138,7 +138,7 @@ def subscribe_on_confirm(user): content_type=ContentType.objects.get_for_model(user), object_id=user.id, defaults={ - 'is_digest': True, + '_is_digest': True, 'message_frequency': 'instantly', }, ) From e7f7216bf97745528b293f02f15cbb144a5486f4 Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Wed, 21 Jan 2026 00:16:34 -0500 Subject: [PATCH 05/10] Move set-deafult-subscriptions out of non-effective try-except --- api/subscriptions/views.py | 60 ++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index 2b7cf98e8f0..64dfdcfc233 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -1,5 +1,5 @@ from django.contrib.contenttypes.models import ContentType -from django.core.exceptions import ObjectDoesNotExist, PermissionDenied +from django.core.exceptions import PermissionDenied from django.db.models import Value, When, Case, OuterRef, Subquery from django.db.models.fields import CharField, IntegerField from django.db.models.functions import Concat, Cast @@ -174,33 +174,31 @@ def get_object(self): ).values('guids___id')[:1] node_guid = 'n/a' - existing_subscriptions = None missing_subscription_created = None - 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(), + 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')), ), - ) - existing_subscriptions = annotated_obj_qs.filter(legacy_id=subscription_id) - except ObjectDoesNotExist: - # `global_file_updated` and `global_reviews` should exist by default for every user - # if not found, create them with `none` frequency as default. + 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(), + ), + ) + existing_subscriptions = annotated_obj_qs.filter(legacy_id=subscription_id) + if not existing_subscriptions: + # `global_file_updated` and `global_reviews` should exist by default for every user. + # If not found, create them with `none` frequency and `_is_digest=True` as default. if subscription_id == f'{user_guid}_global_file_updated': # TODO: should we verify user auth/permissions? notification_type = user_file_updated_nt @@ -213,13 +211,16 @@ def get_object(self): object_id = self.request.user.id elif subscription_id.endswith('_files_updated'): node_guid = subscription_id[:-len('_files_updated')] - node = AbstractNode.objects.get(guid___id=node_guid) + node = AbstractNode.objects.filter(guid___id=node_guid).first() + if not node: + sentry.log_message(f'Invalid node in legacy subscription ID: [user={user_guid}, legacy_id={subscription_id}]') + raise NotFound # TODO: should we verify node contributorship and user auth/permissions? notification_type = node_file_updated_nt content_type = node_ct object_id = node.id else: - # TODO: what is this `{user_guid}_global` legacy ID? Is this the same as not found? + sentry.log_message(f'Subscription not found: [user={user_guid}, legacy_id={subscription_id}]') raise NotFound sentry.log_message(f'Missing default subscription has been created: [user={user_guid}], node={node_guid} type={notification_type}, legacy_id={subscription_id}]') missing_subscription_created = NotificationSubscription.objects.create( @@ -236,7 +237,8 @@ def get_object(self): if missing_subscription_created: subscription = missing_subscription_created else: - subscription = existing_subscriptions.filter(user=self.request.user).order_by('id').first() + # TODO: should we only have one item in `existing_subscriptions` (assume there is no duplicates)? + subscription = existing_subscriptions.filter(user=self.request.user).order_by('id').last() if not subscription: raise PermissionDenied From b86793e7b059bb1cbedd705ecace004d623574f2 Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Thu, 22 Jan 2026 22:01:37 -0500 Subject: [PATCH 06/10] Enforce and improve permission check for subscriptions --- api/subscriptions/views.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index 64dfdcfc233..01b46ad7f7a 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -196,48 +196,51 @@ def get_object(self): ), ) existing_subscriptions = annotated_obj_qs.filter(legacy_id=subscription_id) - if not existing_subscriptions: + if not existing_subscriptions.exists(): # `global_file_updated` and `global_reviews` should exist by default for every user. # If not found, create them with `none` frequency and `_is_digest=True` as default. if subscription_id == f'{user_guid}_global_file_updated': - # TODO: should we verify user auth/permissions? notification_type = user_file_updated_nt content_type = user_ct object_id = self.request.user.id elif subscription_id == f'{user_guid}_global_reviews': - # TODO: should we verify user auth/permissions? notification_type = reviews_submission_status_nt content_type = user_ct object_id = self.request.user.id - elif subscription_id.endswith('_files_updated'): - node_guid = subscription_id[:-len('_files_updated')] - node = AbstractNode.objects.filter(guid___id=node_guid).first() - if not node: - sentry.log_message(f'Invalid node in legacy subscription ID: [user={user_guid}, legacy_id={subscription_id}]') - raise NotFound - # TODO: should we verify node contributorship and user auth/permissions? + elif subscription_id.endswith('_global_file_updated') or subscription_id.endswith('_global_reviews'): + # Mismatched request user and subscription user + sentry.log_message(f'Permission denied: [user={user_guid}, legacy_id={subscription_id}]') + raise PermissionDenied + elif subscription_id.endswith('_file_updated'): notification_type = node_file_updated_nt content_type = node_ct + node_guid = subscription_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_guid}, legacy_id={subscription_id}]') + raise NotFound + if not node.is_contributor(self.request.user): + # The request user is not a contributor of the node + sentry.log_message(f'Permission denied: [user={user_guid}], node={node_guid}, legacy_id={subscription_id}]') + raise PermissionDenied object_id = node.id else: sentry.log_message(f'Subscription not found: [user={user_guid}, legacy_id={subscription_id}]') raise NotFound - sentry.log_message(f'Missing default subscription has been created: [user={user_guid}], node={node_guid} type={notification_type}, legacy_id={subscription_id}]') missing_subscription_created = NotificationSubscription.objects.create( notification_type=notification_type, user=self.request.user, content_type=content_type, object_id=object_id, - defaults={ - '_is_digest': True, - 'message_frequency': 'none', - }, + _is_digest=True, + message_frequency='none', ) + sentry.log_message(f'Missing default subscription has been created: [user={user_guid}], node={node_guid} type={notification_type}, legacy_id={subscription_id}]') if missing_subscription_created: subscription = missing_subscription_created else: - # TODO: should we only have one item in `existing_subscriptions` (assume there is no duplicates)? subscription = existing_subscriptions.filter(user=self.request.user).order_by('id').last() if not subscription: raise PermissionDenied From 5d7849164b998499490603f91e472e80d5f5c9ea Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Thu, 22 Jan 2026 22:12:22 -0500 Subject: [PATCH 07/10] Fix typo in annotated_obj_qs for NODE_FILE_UPDATED --- api/subscriptions/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index 01b46ad7f7a..641490a5786 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -180,7 +180,7 @@ def get_object(self): When( notification_type__name=NotificationType.Type.NODE_FILE_UPDATED.value, content_type=node_ct, - then=Concat(Subquery(node_subquery), Value('_files_updated')), + then=Concat(Subquery(node_subquery), Value('_file_updated')), ), When( notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value, From 20d8cb67e8752b9aa1d1090c7e54b2a50d7f7fd1 Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Thu, 22 Jan 2026 22:19:31 -0500 Subject: [PATCH 08/10] Add unit tests for testing node_file_updated subscription detail --- .../views/test_subscriptions_detail.py | 71 +++++++++++++++++-- 1 file changed, 67 insertions(+), 4 deletions(-) diff --git a/api_tests/subscriptions/views/test_subscriptions_detail.py b/api_tests/subscriptions/views/test_subscriptions_detail.py index f14ca4e2522..765de7c5c56 100644 --- a/api_tests/subscriptions/views/test_subscriptions_detail.py +++ b/api_tests/subscriptions/views/test_subscriptions_detail.py @@ -2,10 +2,11 @@ from django.contrib.contenttypes.models import ContentType from api.base.settings.defaults import API_BASE -from osf.models import NotificationType +from osf.models import NotificationType, OSFUser, AbstractNode from osf_tests.factories import ( AuthUserFactory, - NotificationSubscriptionFactory + NodeFactory, + NotificationSubscriptionFactory, ) @pytest.mark.django_db @@ -15,6 +16,14 @@ class TestSubscriptionDetail: def user(self): return AuthUserFactory() + @pytest.fixture() + def node(self, user): + return NodeFactory(creator=user) + + @pytest.fixture() + def node_without_permission(self): + return NodeFactory() + @pytest.fixture() def user_no_auth(self): return AuthUserFactory() @@ -24,14 +33,54 @@ def notification(self, user): return NotificationSubscriptionFactory( notification_type=NotificationType.Type.USER_FILE_UPDATED.instance, object_id=user.id, - content_type_id=ContentType.objects.get_for_model(user).id, - user=user + content_type_id=ContentType.objects.get_for_model(OSFUser).id, + user=user, + _is_digest=True, + message_frequency='daily', + ) + + @pytest.fixture() + def notification_user_global_reviews(self, user): + return NotificationSubscriptionFactory( + notification_type=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance, + object_id=user.id, + content_type_id=ContentType.objects.get_for_model(OSFUser).id, + user=user, + _is_digest=True, + message_frequency='daily', + ) + + @pytest.fixture() + def notification_node_file_updated(self, node, user): + return NotificationSubscriptionFactory( + notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance, + object_id=node.id, + content_type_id=ContentType.objects.get_for_model(AbstractNode).id, + user=user, + _is_digest=True, + message_frequency='daily', ) @pytest.fixture() def url(self, user): return f'/{API_BASE}subscriptions/{user._id}_global_file_updated/' + @pytest.fixture() + def url_user_global_reviews(self, user): + return f'/{API_BASE}subscriptions/{user._id}_global_reviews/' + + @pytest.fixture() + def url_node_file_updated(self, node): + return f'/{API_BASE}subscriptions/{node._id}_file_updated/' + + @pytest.fixture() + def url_node_file_updated_not_found(self): + return f'/{API_BASE}subscriptions/12345_file_updated/' + + @pytest.fixture() + def url_node_file_updated_without_permission(self, node_without_permission): + return f'/{API_BASE}subscriptions/{node_without_permission._id}_file_updated/' + @pytest.fixture() def url_invalid(self): return f'/{API_BASE}subscriptions/invalid-notification-id/' @@ -84,6 +133,20 @@ def test_subscription_detail_valid_user( assert res.status_code == 200 assert notification_id == f'{user._id}_global_file_updated' + def test_node_file_updated_subscription_detail_success(self, app, user, node, notification_node_file_updated, url_node_file_updated): + res = app.get(url_node_file_updated, auth=user.auth) + notification_id = res.json['data']['id'] + assert res.status_code == 200 + assert notification_id == f'{node._id}_file_updated' + + def test_node_file_updated_subscription_detail_not_found(self, app, user, node, notification_node_file_updated, url_node_file_updated_not_found): + res = app.get(url_node_file_updated_not_found, auth=user.auth, expect_errors=True) + assert res.status_code == 404 + + def test_node_file_updated_subscription_detail_no_permission(self, app, user, node, notification_node_file_updated, url_node_file_updated_without_permission): + res = app.get(url_node_file_updated_without_permission, auth=user.auth, expect_errors=True) + assert res.status_code == 403 + def test_subscription_detail_invalid_notification_id_no_user( self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid ): From 1ede6f2fd154f824d1a315e2e70ccdd4ad547194 Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Fri, 23 Jan 2026 10:57:10 -0500 Subject: [PATCH 09/10] Fix legacy subscription ID for NODE_FILE_UPDATED: "guid_files_updated" --- api/subscriptions/views.py | 6 +++--- .../subscriptions/views/test_subscriptions_detail.py | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index 641490a5786..4938f0b948e 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -180,7 +180,7 @@ def get_object(self): When( notification_type__name=NotificationType.Type.NODE_FILE_UPDATED.value, content_type=node_ct, - then=Concat(Subquery(node_subquery), Value('_file_updated')), + then=Concat(Subquery(node_subquery), Value('_files_updated')), ), When( notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value, @@ -211,10 +211,10 @@ def get_object(self): # Mismatched request user and subscription user sentry.log_message(f'Permission denied: [user={user_guid}, legacy_id={subscription_id}]') raise PermissionDenied - elif subscription_id.endswith('_file_updated'): + elif subscription_id.endswith('_files_updated'): notification_type = node_file_updated_nt content_type = node_ct - node_guid = subscription_id[:-len('_file_updated')] + node_guid = subscription_id[:-len('_files_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 diff --git a/api_tests/subscriptions/views/test_subscriptions_detail.py b/api_tests/subscriptions/views/test_subscriptions_detail.py index 765de7c5c56..4b25b61ecb9 100644 --- a/api_tests/subscriptions/views/test_subscriptions_detail.py +++ b/api_tests/subscriptions/views/test_subscriptions_detail.py @@ -71,15 +71,15 @@ def url_user_global_reviews(self, user): @pytest.fixture() def url_node_file_updated(self, node): - return f'/{API_BASE}subscriptions/{node._id}_file_updated/' + return f'/{API_BASE}subscriptions/{node._id}_files_updated/' @pytest.fixture() def url_node_file_updated_not_found(self): - return f'/{API_BASE}subscriptions/12345_file_updated/' + return f'/{API_BASE}subscriptions/12345_files_updated/' @pytest.fixture() def url_node_file_updated_without_permission(self, node_without_permission): - return f'/{API_BASE}subscriptions/{node_without_permission._id}_file_updated/' + return f'/{API_BASE}subscriptions/{node_without_permission._id}_files_updated/' @pytest.fixture() def url_invalid(self): @@ -137,13 +137,13 @@ def test_node_file_updated_subscription_detail_success(self, app, user, node, no res = app.get(url_node_file_updated, auth=user.auth) notification_id = res.json['data']['id'] assert res.status_code == 200 - assert notification_id == f'{node._id}_file_updated' + assert notification_id == f'{node._id}_files_updated' def test_node_file_updated_subscription_detail_not_found(self, app, user, node, notification_node_file_updated, url_node_file_updated_not_found): res = app.get(url_node_file_updated_not_found, auth=user.auth, expect_errors=True) assert res.status_code == 404 - def test_node_file_updated_subscription_detail_no_permission(self, app, user, node, notification_node_file_updated, url_node_file_updated_without_permission): + def test_node_file_updated_subscription_detail_permission_denied(self, app, user, node, notification_node_file_updated, url_node_file_updated_without_permission): res = app.get(url_node_file_updated_without_permission, auth=user.auth, expect_errors=True) assert res.status_code == 403 From fa8ebfb23f44c2be39b18b3841377066f285dcee Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Fri, 23 Jan 2026 10:59:39 -0500 Subject: [PATCH 10/10] Fix duplicate and mismatched type NODE_FILE(S)_UPDATED --- notifications.yaml | 7 ------- osf/models/notification_type.py | 3 +-- tests/test_events.py | 4 ++-- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/notifications.yaml b/notifications.yaml index 29d896cdc89..6862927ec56 100644 --- a/notifications.yaml +++ b/notifications.yaml @@ -462,13 +462,6 @@ notification_types: template: 'website/templates/file_updated.html.mako' tests: ['tests/test_events.py'] - - name: node_file_updated - subject: 'File Updated' - __docs__: ... - object_content_type_model_name: abstractnode - template: 'website/templates/file_updated.html.mako' - tests: ['tests/test_events.py'] - - name: node_institutional_access_request subject: 'Institutional Access Request' __docs__: ... diff --git a/osf/models/notification_type.py b/osf/models/notification_type.py index fbbe2524f99..86407ef6e0f 100644 --- a/osf/models/notification_type.py +++ b/osf/models/notification_type.py @@ -83,8 +83,7 @@ class Type(str, Enum): USER_CROSSREF_DOI_PENDING = 'user_crossref_doi_pending' # Node notifications - NODE_FILE_UPDATED = 'node_file_updated' - NODE_FILES_UPDATED = 'node_files_updated' + NODE_FILE_UPDATED = 'node_files_updated' NODE_AFFILIATION_CHANGED = 'node_affiliation_changed' NODE_REQUEST_ACCESS_SUBMITTED = 'node_request_access_submitted' NODE_REQUEST_ACCESS_DENIED = 'node_request_access_denied' diff --git a/tests/test_events.py b/tests/test_events.py index fa79515e021..0d83bfdd072 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -309,7 +309,7 @@ def setUp(self): self.file_sub = factories.NotificationSubscriptionFactory( object_id=self.project.id, content_type=ContentType.objects.get_for_model(self.project), - notification_type=NotificationType.objects.get(name=NotificationType.Type.NODE_FILES_UPDATED) + notification_type=NotificationType.objects.get(name=NotificationType.Type.NODE_FILE_UPDATED) ) self.file_sub.save() @@ -407,7 +407,7 @@ def setUp(self): self.file_sub = factories.NotificationSubscriptionFactory( object_id=self.project.id, content_type=ContentType.objects.get_for_model(self.project), - notification_type=NotificationType.objects.get(name=NotificationType.Type.NODE_FILES_UPDATED) + notification_type=NotificationType.objects.get(name=NotificationType.Type.NODE_FILE_UPDATED) ) self.file_sub.save()