Skip to content

Commit 300524c

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

File tree

7 files changed

+158
-1691
lines changed

7 files changed

+158
-1691
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: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
from pyasn1_modules.rfc5126 import ContentType
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
4+
from django.contrib.contenttypes.models 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,47 @@ 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+
provider_ct = ContentType.objects.get(app_label='osf', model='abstractprovider')
46+
47+
provider_subquery = AbstractProvider.objects.filter(
48+
id=Cast(OuterRef('object_id'), IntegerField()),
49+
).values('_id')[:1]
50+
51+
node_subquery = AbstractNode.objects.filter(
52+
id=Cast(OuterRef('object_id'), IntegerField()),
53+
).values('guids___id')[:1]
54+
55+
return NotificationSubscription.objects.filter(user=self.request.user).annotate(
56+
event_name=Case(
57+
When(
58+
notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value,
59+
then=Value('files_updated'),
60+
),
61+
When(
62+
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
63+
then=Value('global_file_updated'),
64+
),
65+
default=F('notification_type__name'),
66+
output_field=CharField(),
67+
),
68+
legacy_id=Case(
69+
When(
70+
notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value,
71+
then=Concat(Subquery(node_subquery), Value('_file_updated')),
72+
),
73+
When(
74+
notification_type__name=NotificationType.Type.USER_FILE_UPDATED.value,
75+
then=Value(f'{user_guid}_global'),
76+
),
77+
When(
78+
Q(notification_type__name=NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value) &
79+
Q(content_type=provider_ct),
80+
then=Concat(Subquery(provider_subquery), Value('_new_pending_submissions')),
81+
),
82+
default=F('notification_type__name'),
83+
output_field=CharField(),
84+
),
4385
)
4486

4587

@@ -67,10 +109,63 @@ class SubscriptionDetail(JSONAPIBaseView, generics.RetrieveUpdateAPIView):
67109

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

‎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)