[ENG-9005] Fix/update v2 subscriptions #11329
Conversation
|
|
||
| return NotificationSubscription.objects.filter(user=self.request.user).annotate( | ||
| return NotificationSubscription.objects.filter( | ||
| notification_type__in=[ |
There was a problem hiding this comment.
Main issue was we were pulling from types of subscription that weren't coded for legacy susbscriptions, this filters the types of new notification that are editable now. Only those 3 now that comments and mentions aren't subscription types.
| event_name=Case( | ||
| When( | ||
| notification_type__name=NotificationType.Type.NODE_FILES_UPDATED.value, | ||
| notification_type=NotificationType.Type.USER_FILE_UPDATED.instance, |
There was a problem hiding this comment.
Improvement performance and Value( for correctness.
| def notification(self, user): | ||
| return NotificationSubscriptionFactory(user=user) | ||
| return NotificationSubscriptionFactory( | ||
| notification_type=NotificationType.Type.USER_FILE_UPDATED.instance, |
There was a problem hiding this comment.
These tests weren't quite right because they didn't use the subscribed object attribute, only type and user.
| return NotificationSubscriptionFactory( | ||
| notification_type=NotificationType.Type.NODE_FILES_UPDATED.instance, | ||
| subscribed_object=node, | ||
| notification_type=NotificationType.Type.NODE_FILE_UPDATED.instance, |
There was a problem hiding this comment.
typo in FILE(S) and same as above.
Johnetordoff
force-pushed
the
fix/update-v2-subscriptions
branch
from
43f53d6 to
caca24c
Compare
| match getattr(ct, 'model', None): | ||
| case 'preprintprovider' | 'collectionprovider' | 'registrationprovider': | ||
| # Providers: use subscribed_object._id (which is the provider short name, e.g. 'mindrxiv') | ||
| match self.notification_type.name: |
There was a problem hiding this comment.
This is just changed for clarity and correctness.
| return obj.absolute_api_v2_url | ||
|
|
||
| def update(self, instance, validated_data): | ||
| instance.message_frequency = validated_data.get['frequency'] |
There was a problem hiding this comment.
Strange this wasn't effected by tests, but now it works in test_subscription_detail_patch
| @property | ||
| def absolute_api_v2_url(self): | ||
| from api.base.utils import absolute_reverse | ||
| return absolute_reverse('institutions:institution-detail', kwargs={'institution_id': self._id, 'version': 'v2'}) |
There was a problem hiding this comment.
This is another thing that should have broken tests, but didn't.
Johnetordoff
changed the title
Johnetordoff
merged commit 0b7fe79
into
CenterForOpenScience:feature/notification-refactor-p2-s
2 participants
Purpose
Fix subscription legacy compatibility issues with v2 API
Changes
QA Notes
Please make verification statements inspired by your code and what your code touches.
What are the areas of risk?
Any concerns/considerations/questions that development raised?
Documentation
Side Effects
Ticket
https://openscience.atlassian.net/browse/ENG-9005