Skip to content

Commit ad18e9d

Browse files
JohnetordoffJohnetordoff
committed
fix backward compat issues and remove old tests
1 parent bb145f4 commit ad18e9d

File tree

7 files changed

+166
-1692
lines changed

7 files changed

+166
-1692
lines changed

‎api/subscriptions/serializers.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ class SubscriptionSerializer(JSONAPISerializer):
1717
'frequency',
1818
])
1919

20-
id = ser.CharField(read_only=True)
20+
id = ser.CharField(
21+
read_only=True,
22+
source='legacy_id',
23+
help_text='The id of the subscription fixed for backward compatibility',
24+
)
2125
event_name = ser.CharField(read_only=True)
2226
frequency = FrequencyField(source='message_frequency', required=True)
2327

‎api/subscriptions/views.py

Lines changed: 110 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
from django.db.models import Value, When, Case, F, Q, OuterRef, Subquery
2+
from django.db.models.fields import CharField, IntegerField
3+
from django.db.models.functions import Concat, Cast
14
from pyasn1_modules.rfc5126 import ContentType
25
from rest_framework import generics
36
from rest_framework import permissions as drf_permissions
47
from rest_framework.exceptions import NotFound
5-
from django.core.exceptions import ObjectDoesNotExist
8+
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
69

710
from framework.auth.oauth_scopes import CoreScopes
811
from api.base.views import JSONAPIBaseView
@@ -19,9 +22,9 @@
1922
CollectionProvider,
2023
PreprintProvider,
2124
RegistrationProvider,
22-
AbstractProvider,
25+
AbstractProvider, AbstractNode, Preprint, OSFUser,
2326
)
24-
from osf.models.notification import NotificationSubscription
27+
from osf.models.notification import NotificationSubscription, NotificationType
2528

2629

2730
class SubscriptionList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin):
@@ -38,8 +41,48 @@ class SubscriptionList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin):
3841
required_write_scopes = [CoreScopes.NULL]
3942

4043
def get_queryset(self):
41-
return NotificationSubscription.objects.filter(
42-
user=self.request.user,
44+
user_guid = self.request.user._id
45+
from django.contrib.contenttypes.models import ContentType
46+
provider_ct = ContentType.objects.get(app_label='osf', model='abstractprovider')
47+
48+
provider_subquery = AbstractProvider.objects.filter(
49+
id=Cast(OuterRef('object_id'), IntegerField()),
50+
).values('_id')[:1]
51+
52+
node_subquery = AbstractNode.objects.filter(
53+
id=Cast(OuterRef('object_id'), IntegerField()),
54+
).values('guids___id')[:1]
55+
56+
return NotificationSubscription.objects.filter(user=self.request.user).annotate(
57+
event_name=Case(
58+
When(
59+
notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value,
60+
then=Value('files_updated'),
61+
),
62+
When(
63+
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
64+
then=Value('global_file_updated'),
65+
),
66+
default=F('notification_type__name'),
67+
output_field=CharField(),
68+
),
69+
legacy_id=Case(
70+
When(
71+
notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value,
72+
then=Concat(Subquery(node_subquery), Value('_file_updated')),
73+
),
74+
When(
75+
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
76+
then=Value(f'{user_guid}_global'),
77+
),
78+
When(
79+
Q(notification_type__name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value) &
80+
Q(content_type=provider_ct),
81+
then=Concat(Subquery(provider_subquery), Value('_new_pending_submissions')),
82+
),
83+
default=F('notification_type__name'),
84+
output_field=CharField(),
85+
),
4386
)
4487

4588

@@ -67,13 +110,72 @@ class SubscriptionDetail(JSONAPIBaseView, generics.RetrieveUpdateAPIView):
67110

68111
def get_object(self):
69112
subscription_id = self.kwargs['subscription_id']
113+
user = self.request.user
114+
115+
provider_ct = ContentType.objects.get(app_label='osf', model='abstractprovider')
116+
node_ct = ContentType.objects.get(app_label='osf', model='abstractnode')
117+
118+
provider_subquery = AbstractProvider.objects.filter(
119+
id=Cast(OuterRef('object_id'), IntegerField()),
120+
).values('_id')[:1]
121+
122+
node_subquery = AbstractNode.objects.filter(
123+
id=Cast(OuterRef('object_id'), IntegerField()),
124+
).values('guids___id')[:1]
125+
126+
guid_id, *event_parts = subscription_id.split('_')
127+
event = '_'.join(event_parts)
128+
129+
subscription_obj = AbstractNode.load(guid_id) or Preprint.load(guid_id) or OSFUser.load(guid_id)
130+
131+
if event == 'global':
132+
object_filter = Q(
133+
Q(content_type=ContentType.objects.get_for_model(OSFUser), object_id=user.id) |
134+
Q(content_type__isnull=True, object_id__isnull=True),
135+
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
136+
)
137+
else:
138+
if not subscription_obj:
139+
raise NotFound
140+
141+
object_filter = Q(
142+
object_id=subscription_obj.id,
143+
content_type=ContentType.objects.get_for_model(subscription_obj.__class__),
144+
notification_type__name__icontains=event,
145+
)
146+
147+
subscription_qs = NotificationSubscription.objects.annotate(
148+
legacy_id=Case(
149+
When(
150+
notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value,
151+
content_type=node_ct,
152+
then=Concat(Subquery(node_subquery), Value('_file_updated')),
153+
),
154+
When(
155+
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
156+
then=Value(f'{user._id}_global'),
157+
),
158+
When(
159+
notification_type__name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value,
160+
content_type=provider_ct,
161+
then=Concat(Subquery(provider_subquery), Value('_new_pending_submissions')),
162+
),
163+
default=F('notification_type__name'),
164+
output_field=CharField(),
165+
),
166+
).filter(object_filter)
167+
70168
try:
71-
obj = NotificationSubscription.objects.get(id=subscription_id)
169+
subscription = subscription_qs.get()
72170
except ObjectDoesNotExist:
73171
raise NotFound
74-
self.check_object_permissions(self.request, obj)
75-
return obj
76172

173+
if subscription.user != user:
174+
raise PermissionDenied
175+
176+
self.check_object_permissions(self.request, subscription)
177+
178+
return subscription
77179

78180
class AbstractProviderSubscriptionDetail(SubscriptionDetail):
79181
view_name = 'provider-notification-subscription-detail'

‎api_tests/subscriptions/views/test_subscriptions_detail.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ def user_no_auth(self):
1919

2020
@pytest.fixture()
2121
def notification(self, user):
22-
return NotificationSubscriptionFactory(
23-
user=user,
24-
)
22+
return NotificationSubscriptionFactory(user=user)
2523

2624
@pytest.fixture()
2725
def url(self, notification):
28-
return f'/{API_BASE}subscriptions/{notification.id}/'
26+
print('_id', notification._id)
27+
return f'/{API_BASE}subscriptions/{notification._id}/'
2928

3029
@pytest.fixture()
3130
def url_invalid(self):
@@ -53,9 +52,7 @@ def payload_invalid(self):
5352
}
5453
}
5554

56-
def test_subscription_detail_invalid_user(
57-
self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid
58-
):
55+
def test_subscription_detail_invalid_user(self, app, user, user_no_auth, notification, url, payload):
5956
res = app.get(
6057
url,
6158
auth=user_no_auth.auth,
@@ -79,7 +76,7 @@ def test_subscription_detail_valid_user(
7976
res = app.get(url, auth=user.auth)
8077
notification_id = res.json['data']['id']
8178
assert res.status_code == 200
82-
assert notification_id == str(notification.id)
79+
assert notification_id == f'{user._id}_global'
8380

8481
def test_subscription_detail_invalid_notification_id_no_user(
8582
self, app, user, user_no_auth, notification, url, url_invalid, payload, payload_invalid

‎api_tests/subscriptions/views/test_subscriptions_list.py

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
import pytest
22

33
from api.base.settings.defaults import API_BASE
4-
from osf_tests.factories import AuthUserFactory, PreprintProviderFactory, ProjectFactory, \
5-
NotificationSubscriptionLegacyFactory, NotificationSubscriptionFactory
4+
from osf.models import NotificationType
5+
from osf_tests.factories import (
6+
AuthUserFactory,
7+
PreprintProviderFactory,
8+
ProjectFactory,
9+
NotificationSubscriptionFactory
10+
)
611

712

813
@pytest.mark.django_db
@@ -24,25 +29,42 @@ def node(self, user):
2429

2530
@pytest.fixture()
2631
def global_user_notification(self, user):
27-
notification = NotificationSubscriptionLegacyFactory(_id=f'{user._id}_global', user=user, event_name='global')
28-
notification.add_user_to_subscription(user, 'email_transactional')
29-
return notification
32+
return NotificationSubscriptionFactory(
33+
notification_type=NotificationType.Type.USER_FILE_UPDATED.instance,
34+
user=user,
35+
)
3036

3137
@pytest.fixture()
3238
def file_updated_notification(self, node, user):
33-
notification = NotificationSubscriptionFactory(
34-
_id=node._id + 'file_updated',
35-
owner=node,
36-
event_name='file_updated',
39+
return NotificationSubscriptionFactory(
40+
notification_type=NotificationType.Type.NODE_FILES_UPDATED.instance,
41+
subscribed_object=node,
42+
user=user,
43+
)
44+
45+
@pytest.fixture()
46+
def provider_notification(self, provider, user):
47+
return NotificationSubscriptionFactory(
48+
notification_type=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance,
49+
subscribed_object=provider,
50+
user=user,
3751
)
38-
notification.add_user_to_subscription(user, 'email_transactional')
39-
return notification
4052

4153
@pytest.fixture()
4254
def url(self, user, node):
4355
return f'/{API_BASE}subscriptions/'
4456

45-
def test_list_complete(self, app, user, provider, node, global_user_notification, url):
57+
def test_list_complete(
58+
self,
59+
app,
60+
user,
61+
provider,
62+
node,
63+
global_user_notification,
64+
provider_notification,
65+
file_updated_notification,
66+
url
67+
):
4668
res = app.get(url, auth=user.auth)
4769
notification_ids = [item['id'] for item in res.json['data']]
4870
# There should only be 3 notifications: users' global, node's file updates and provider's preprint added.

‎osf/models/notification.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ class Type(str, Enum):
100100
NODE_PENDING_EMBARGO_TERMINATION_ADMIN = 'node_pending_embargo_termination_admin'
101101

102102
# Provider notifications
103+
PROVIDER_NEW_PENDING_SUBMISSIONS = 'provider_new_pending_submissions'
103104
PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION = 'provider_reviews_submission_confirmation'
104105
PROVIDER_REVIEWS_MODERATOR_SUBMISSION_CONFIRMATION = 'provider_reviews_moderator_submission_confirmation'
105106
PROVIDER_REVIEWS_WITHDRAWAL_REQUESTED = 'preprint_request_withdrawal_requested'
@@ -119,7 +120,6 @@ class Type(str, Enum):
119120
PREPRINT_CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF = 'preprint_contributor_added_preprint_node_from_osf'
120121

121122
# Collections Submission notifications
122-
NEW_PENDING_SUBMISSIONS = 'new_pending_submissions'
123123
COLLECTION_SUBMISSION_REMOVED_ADMIN = 'collection_submission_removed_admin'
124124
COLLECTION_SUBMISSION_REMOVED_MODERATOR = 'collection_submission_removed_moderator'
125125
COLLECTION_SUBMISSION_REMOVED_PRIVATE = 'collection_submission_removed_private'
@@ -136,6 +136,11 @@ class Type(str, Enum):
136136

137137
REGISTRATION_BULK_UPLOAD_FAILURE_DUPLICATES = 'registration_bulk_upload_failure_duplicates'
138138

139+
@property
140+
def instance(self):
141+
obj, created = NotificationType.objects.get_or_create(name=self.value)
142+
return obj
143+
139144
@classmethod
140145
def user_types(cls):
141146
return [member for member in cls if member.name.startswith('USER_')]
@@ -271,7 +276,7 @@ def clean(self):
271276
raise ValidationError(f'{self.message_frequency!r} is not allowed for {self.notification_type.name!r}.')
272277

273278
def __str__(self) -> str:
274-
return f'{self.user} subscribes to {self.notification_type.name} ({self.message_frequency})'
279+
return f'<{self.user} via {self.subscribed_object} subscribes to {self.notification_type.name} ({self.message_frequency})>'
275280

276281
class Meta:
277282
verbose_name = 'Notification Subscription'
@@ -321,9 +326,11 @@ def _id(self):
321326
case 'node' | 'collection' | 'preprint':
322327
# Node-like objects: use object_id (guid)
323328
return f'{self.subscribed_object._id}_{event}'
324-
case 'osfuser' | 'user', _:
329+
case 'osfuser' | 'user' | None:
325330
# Global: <user_id>_global
326-
return f'{self.subscribed_object._id}_global_{event}'
331+
return f'{self.user._id}_global'
332+
case _:
333+
raise NotImplementedError()
327334

328335

329336
class Notification(models.Model):

0 commit comments

Comments
 (0)