diff --git a/api/subscriptions/utils.py b/api/subscriptions/utils.py new file mode 100644 index 00000000000..d17d1ea2687 --- /dev/null +++ b/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 `_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 + # `_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('_files_updated'): + notification_type = node_file_updated_nt + content_type = node_ct + node_guid = legacy_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 + 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 diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index b23fccb2804..7e7ef9fd5ca 100644 --- a/api/subscriptions/views.py +++ b/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,6 +20,8 @@ RegistrationSubscriptionSerializer, ) from api.subscriptions.permissions import IsSubscriptionOwner +from api.subscriptions import utils + from osf.models import ( CollectionProvider, PreprintProvider, @@ -25,6 +29,7 @@ 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,9 +95,10 @@ 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( @@ -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_notification_from_legacy_id(filter_event_names, user) + + return return_qs - return qs class AbstractProviderSubscriptionList(SubscriptionList): def get_queryset(self): @@ -171,47 +196,53 @@ 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('_files_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): """ diff --git a/api_tests/subscriptions/views/test_subscriptions_detail.py b/api_tests/subscriptions/views/test_subscriptions_detail.py index f14ca4e2522..a11a89cce7b 100644 --- a/api_tests/subscriptions/views/test_subscriptions_detail.py +++ b/api_tests/subscriptions/views/test_subscriptions_detail.py @@ -1,11 +1,18 @@ import pytest + from django.contrib.contenttypes.models import ContentType from api.base.settings.defaults import API_BASE -from osf.models import NotificationType +from osf.models import ( + AbstractNode, + NotificationSubscription, + NotificationType, + OSFUser +) from osf_tests.factories import ( AuthUserFactory, - NotificationSubscriptionFactory + NodeFactory, + NotificationSubscriptionFactory, ) @pytest.mark.django_db @@ -16,22 +23,83 @@ def user(self): return AuthUserFactory() @pytest.fixture() - def user_no_auth(self): + def user_missing_subscriptions(self): + return AuthUserFactory() + + @pytest.fixture() + def user_no_permission(self): return AuthUserFactory() @pytest.fixture() - def notification(self, user): + def node(self, user): + return NodeFactory(creator=user) + + @pytest.fixture() + def node_missing_subscriptions(self, user_missing_subscriptions): + node = NodeFactory(creator=user_missing_subscriptions) + subscription = NotificationSubscription.objects.get( + user=user_missing_subscriptions, + notification_type__name=NotificationType.Type.NODE_FILE_UPDATED.value, + object_id=node.id, + content_type=ContentType.objects.get_for_model(AbstractNode) + ) + subscription.delete() + return node + + @pytest.fixture() + def notification_user_global_file_updated(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.REVIEWS_SUBMISSION_STATUS.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 url(self, user): + def url_user_global_file_updated(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_user_global_file_updated_missing(self, user_missing_subscriptions): + return f'/{API_BASE}subscriptions/{user_missing_subscriptions._id}_global_file_updated/' + + @pytest.fixture() + def url_user_global_reviews_missing(self, user_missing_subscriptions): + return f'/{API_BASE}subscriptions/{user_missing_subscriptions._id}_global_reviews/' + + @pytest.fixture() + def url_node_file_updated(self, node): + 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_files_updated/' + + @pytest.fixture() + def url_node_file_updated_without_permission(self, node_without_permission): + return f'/{API_BASE}subscriptions/{node_without_permission._id}_files_updated/' + + @pytest.fixture() + def url_node_file_updated_missing(self, node_missing_subscriptions): + return f'/{API_BASE}subscriptions/{node_missing_subscriptions._id}_files_updated/' + @pytest.fixture() def url_invalid(self): return f'/{API_BASE}subscriptions/invalid-notification-id/' @@ -58,40 +126,158 @@ def payload_invalid(self): } } - def test_subscription_detail_invalid_user(self, app, user, user_no_auth, notification, url, payload): - res = app.get( - url, - auth=user_no_auth.auth, - expect_errors=True - ) + def test_user_global_subscription_detail_permission_denied( + self, + app, + user, + user_no_permission, + notification_user_global_file_updated, + notification_user_global_reviews, + url_user_global_file_updated, + url_user_global_reviews + ): + res = app.get(url_user_global_file_updated, auth=user_no_permission.auth, expect_errors=True) + assert res.status_code == 403 + res = app.get(url_user_global_reviews, auth=user_no_permission.auth, expect_errors=True) assert res.status_code == 403 - def test_subscription_detail_no_user( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + def test_user_global_subscription_detail_forbidden( + self, + app, + user, + user_no_permission, + notification_user_global_file_updated, + notification_user_global_reviews, + url_user_global_file_updated, + url_user_global_reviews ): - res = app.get( - url, - expect_errors=True - ) + res = app.get(url_user_global_file_updated, expect_errors=True) + assert res.status_code == 401 + res = app.get(url_user_global_reviews, expect_errors=True) assert res.status_code == 401 - def test_subscription_detail_valid_user( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + def test_user_global_subscription_detail_success( + self, + app, + user, + user_no_permission, + notification_user_global_file_updated, + notification_user_global_reviews, + url_user_global_file_updated, + url_user_global_reviews ): - - res = app.get(url, auth=user.auth) + res = app.get(url_user_global_file_updated, auth=user.auth) notification_id = res.json['data']['id'] assert res.status_code == 200 assert notification_id == f'{user._id}_global_file_updated' + res = app.get(url_user_global_reviews, auth=user.auth) + notification_id = res.json['data']['id'] + assert res.status_code == 200 + assert notification_id == f'{user._id}_global_reviews' + + def test_user_global_file_updated_subscription_detail_missing_and_created( + self, + app, + user_missing_subscriptions, + url_user_global_file_updated_missing, + ): + assert not NotificationSubscription.objects.filter( + user=user_missing_subscriptions, + notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value, + object_id=user_missing_subscriptions.id, + content_type=ContentType.objects.get_for_model(OSFUser) + ).exists() + res = app.get(url_user_global_file_updated_missing, auth=user_missing_subscriptions.auth) + notification_id = res.json['data']['id'] + assert res.status_code == 200 + assert notification_id == f'{user_missing_subscriptions._id}_global_file_updated' + + def test_user_global_reviews_subscription_detail_missing_and_created( + self, + app, + user_missing_subscriptions, + url_user_global_reviews_missing, + ): + assert not NotificationSubscription.objects.filter( + user=user_missing_subscriptions, + notification_type__name=NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value, + object_id=user_missing_subscriptions.id, + content_type=ContentType.objects.get_for_model(OSFUser) + ).exists() + res = app.get(url_user_global_reviews_missing, auth=user_missing_subscriptions.auth) + notification_id = res.json['data']['id'] + assert res.status_code == 200 + assert notification_id == f'{user_missing_subscriptions._id}_global_reviews' + + def test_node_file_updated_subscription_detail_success( + self, + app, + user, + node, + 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}_files_updated' + + def test_node_file_updated_subscription_detail_missing_and_created( + self, + app, + user_missing_subscriptions, + node_missing_subscriptions, + url_node_file_updated_missing, + ): + assert not NotificationSubscription.objects.filter( + user=user_missing_subscriptions, + notification_type__name=NotificationType.Type.NODE_FILE_UPDATED.value, + object_id=node_missing_subscriptions.id, + content_type=ContentType.objects.get_for_model(AbstractNode) + ).exists() + res = app.get(url_node_file_updated_missing, auth=user_missing_subscriptions.auth) + notification_id = res.json['data']['id'] + assert res.status_code == 200 + assert notification_id == f'{node_missing_subscriptions._id}_files_updated' + + def test_node_file_updated_subscription_detail_not_found( + self, + app, + user, + node, + 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_permission_denied( + self, + app, + user, + user_no_permission, + node, + url_node_file_updated + ): + res = app.get(url_node_file_updated, auth=user_no_permission.auth, expect_errors=True) + assert res.status_code == 403 + + def test_node_file_updated_subscription_detail_forbidden( + self, + app, + user, + node, + url_node_file_updated + ): + res = app.get(url_node_file_updated, expect_errors=True) + assert res.status_code == 401 def test_subscription_detail_invalid_notification_id_no_user( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): res = app.get(url_invalid, expect_errors=True) assert res.status_code == 404 def test_subscription_detail_invalid_notification_id_existing_user( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): res = app.get( url_invalid, @@ -101,22 +287,22 @@ def test_subscription_detail_invalid_notification_id_existing_user( assert res.status_code == 404 def test_subscription_detail_invalid_payload_403( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): - res = app.patch_json_api(url, payload_invalid, auth=user_no_auth.auth, expect_errors=True) + res = app.patch_json_api(url_user_global_file_updated, payload_invalid, auth=user_no_permission.auth, expect_errors=True) assert res.status_code == 403 def test_subscription_detail_invalid_payload_401( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): - res = app.patch_json_api(url, payload_invalid, expect_errors=True) + res = app.patch_json_api(url_user_global_file_updated, payload_invalid, expect_errors=True) assert res.status_code == 401 def test_subscription_detail_invalid_payload_400( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): res = app.patch_json_api( - url, + url_user_global_file_updated, payload_invalid, auth=user.auth, expect_errors=True, @@ -126,33 +312,33 @@ def test_subscription_detail_invalid_payload_400( assert res.json['errors'][0]['detail'] == ('"invalid-frequency" is not a valid choice.') def test_subscription_detail_patch_invalid_notification_id_no_user( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): res = app.patch_json_api(url_invalid, payload, expect_errors=True) assert res.status_code == 404 def test_subscription_detail_patch_invalid_notification_id_existing_user( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): res = app.patch_json_api(url_invalid, payload, auth=user.auth, expect_errors=True) assert res.status_code == 404 def test_subscription_detail_patch_invalid_user( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): - res = app.patch_json_api(url, payload, auth=user_no_auth.auth, expect_errors=True) + res = app.patch_json_api(url_user_global_file_updated, payload, auth=user_no_permission.auth, expect_errors=True) assert res.status_code == 403 def test_subscription_detail_patch_no_user( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): - res = app.patch_json_api(url, payload, expect_errors=True) + res = app.patch_json_api(url_user_global_file_updated, payload, expect_errors=True) assert res.status_code == 401 def test_subscription_detail_patch( - self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid + self, app, user, user_no_permission, notification_user_global_file_updated, url_user_global_file_updated, url_invalid, payload, payload_invalid ): - res = app.patch_json_api(url, payload, auth=user.auth) + res = app.patch_json_api(url_user_global_file_updated, payload, auth=user.auth) assert res.status_code == 200 assert res.json['data']['attributes']['frequency'] == 'none' 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 109e758c8da..c6d67d3edbf 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() diff --git a/website/mailchimp_utils.py b/website/mailchimp_utils.py index 7e92a59d275..e3d7a546d0f 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', + }, )