Skip to content

[ENG-8084] Feature/upgrade adding contributor notifications #11189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 50 commits into
base: refactor-notifications
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
57b0bd1
add new data model for notifications
Johnetordoff May 20, 2025
929b9fd
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff May 22, 2025
69231e9
add new notificationsubscription class to views
Johnetordoff May 27, 2025
c324716
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff May 28, 2025
40b38f7
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff May 30, 2025
358091c
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff Jun 3, 2025
5165ab9
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jun 3, 2025
2ab3589
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff Jun 9, 2025
c449599
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jun 9, 2025
d83c2c3
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jun 13, 2025
f550c61
fix absolute url issue
Johnetordoff Jun 16, 2025
458fbfd
fix up unit test issues
Johnetordoff Jun 16, 2025
16c5409
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff Jun 25, 2025
5864187
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
Johnetordoff Jun 30, 2025
bb145f4
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jun 30, 2025
300524c
fix backward compat issues and remove old tests
Johnetordoff Jul 2, 2025
3a682b3
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf…
Johnetordoff Jul 7, 2025
85e1342
split notification models into 3 files and improve interval choices
Johnetordoff Jul 7, 2025
f2e5309
clean-up tests and pass frequency data properly
Johnetordoff Jul 8, 2025
0471b76
update management commands and tests for notification migration
Johnetordoff Jul 8, 2025
78e968c
Upgrade User Confirmation Registrations
Ostap-Zherebetskyi Jul 7, 2025
1affb5e
fix unit tests
Ostap-Zherebetskyi Jul 9, 2025
2dee1b2
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf…
Johnetordoff Jul 9, 2025
72623cc
fix unit tests
Ostap-Zherebetskyi Jul 9, 2025
2da68ad
Merge remote-tracking branch 'upstream/refactor-notifications' into f…
Ostap-Zherebetskyi Jul 9, 2025
a02352c
fix unit tests
Ostap-Zherebetskyi Jul 9, 2025
ff9743b
fix unit tests
Ostap-Zherebetskyi Jul 9, 2025
feba563
fix unit tests
Ostap-Zherebetskyi Jul 10, 2025
90ef652
fix unit tests
Ostap-Zherebetskyi Jul 10, 2025
0c003f5
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf…
Johnetordoff Jul 10, 2025
8743277
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jul 10, 2025
e3bc742
remove old management commands and add new ones to population notific…
Johnetordoff Jul 10, 2025
cd0bc26
Merge branch 'add-new-notifications-data-model' into feature/user_con…
Johnetordoff Jul 10, 2025
b0d8bdc
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jul 10, 2025
37b419a
fix issues with migrate schema response task deletion
Johnetordoff Jul 10, 2025
0657c03
Merge branch 'add-new-notifications-data-model' into feature/user_con…
Johnetordoff Jul 11, 2025
03cad99
fix issues with migrate schema response task deletion
Johnetordoff Jul 11, 2025
4a4167f
Merge pull request #249 from Johnetordoff/feature/user_confirmation
Johnetordoff Jul 11, 2025
c37366c
Merge branch 'add-new-notifications-data-model' of github.com:johneto…
Johnetordoff Jul 11, 2025
058f03b
Merge branch 'refactor-notifications' into add-new-notifications-data…
Johnetordoff Jul 11, 2025
cd68988
refactored notifications created with model use
bodintsov Jun 17, 2025
f3713fc
flake8
bodintsov Jun 18, 2025
9d87d84
updated event_context
bodintsov Jun 18, 2025
a9241eb
refactored to NotificationType
bodintsov Jul 15, 2025
da143e4
added to notifications.yaml
bodintsov Jul 15, 2025
84b85b0
refactored to use NotificationType
bodintsov Jul 16, 2025
703030a
fix test
bodintsov Jul 16, 2025
f7ab4ca
fix test
bodintsov Jul 16, 2025
1bf2a71
fix test
bodintsov Jul 16, 2025
6c194b1
fix notifications.yaml
bodintsov Jul 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions admin/notifications/views.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
from osf.models.notifications import NotificationSubscription
from osf.models.notifications import NotificationSubscriptionLegacy
from django.db.models import Count

def delete_selected_notifications(selected_ids):
NotificationSubscription.objects.filter(id__in=selected_ids).delete()
NotificationSubscriptionLegacy.objects.filter(id__in=selected_ids).delete()

def detect_duplicate_notifications(node_id=None):
query = NotificationSubscription.objects.values('_id').annotate(count=Count('_id')).filter(count__gt=1)
query = NotificationSubscriptionLegacy.objects.values('_id').annotate(count=Count('_id')).filter(count__gt=1)
if node_id:
query = query.filter(node_id=node_id)

detailed_duplicates = []
for dup in query:
notifications = NotificationSubscription.objects.filter(
notifications = NotificationSubscriptionLegacy.objects.filter(
_id=dup['_id']
).order_by('created')

Expand Down
19 changes: 10 additions & 9 deletions admin_tests/notifications/test_views.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import pytest
from django.test import RequestFactory
from osf.models import OSFUser, NotificationSubscription, Node
from osf.models import OSFUser, Node
from admin.notifications.views import (
delete_selected_notifications,
detect_duplicate_notifications,
)
from osf.models.notifications import NotificationSubscriptionLegacy
from tests.base import AdminTestCase

pytestmark = pytest.mark.django_db
Expand All @@ -18,19 +19,19 @@ def setUp(self):
self.request_factory = RequestFactory()

def test_delete_selected_notifications(self):
notification1 = NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event1')
notification2 = NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event2')
notification3 = NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event3')
notification1 = NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event1')
notification2 = NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event2')
notification3 = NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event3')

delete_selected_notifications([notification1.id, notification2.id])

assert not NotificationSubscription.objects.filter(id__in=[notification1.id, notification2.id]).exists()
assert NotificationSubscription.objects.filter(id=notification3.id).exists()
assert not NotificationSubscriptionLegacy.objects.filter(id__in=[notification1.id, notification2.id]).exists()
assert NotificationSubscriptionLegacy.objects.filter(id=notification3.id).exists()

def test_detect_duplicate_notifications(self):
NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event1')
NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event1')
NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event2')
NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event1')
NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event1')
NotificationSubscriptionLegacy.objects.create(user=self.user, node=self.node, event_name='event2')

duplicates = detect_duplicate_notifications()

Expand Down
19 changes: 9 additions & 10 deletions api/institutions/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@

from osf import features
from osf.exceptions import InstitutionAffiliationStateError
from osf.models import Institution
from osf.models import Institution, NotificationType
from osf.models.institution import SsoFilterCriteriaAction

from website.mails import send_mail, WELCOME_OSF4I, DUPLICATE_ACCOUNTS_OSF4I, ADD_SSO_EMAIL_OSF4I
from website.mails import send_mail, DUPLICATE_ACCOUNTS_OSF4I, ADD_SSO_EMAIL_OSF4I
from website.settings import OSF_SUPPORT_EMAIL, DOMAIN
from website.util.metrics import institution_source_tag

Expand Down Expand Up @@ -334,14 +334,13 @@ def authenticate(self, request):
user.save()

# Send confirmation email for all three: created, confirmed and claimed
send_mail(
to_addr=user.username,
mail=WELCOME_OSF4I,
user=user,
domain=DOMAIN,
osf_support_email=OSF_SUPPORT_EMAIL,
storage_flag_is_active=flag_is_active(request, features.STORAGE_I18N),
)
notification_type = NotificationType.objects.filter(name='welcome_osf4i')
if not notification_type.exists():
raise NotificationType.DoesNotExist(
'NotificationType with name welcome_osf4i does not exist.',
)
notification_type = notification_type.first()
notification_type.emit(user=user, message_frequency='instantly', event_context={'domain': DOMAIN, 'osf_support_email': OSF_SUPPORT_EMAIL, 'storage_flag_is_active': flag_is_active(request, features.STORAGE_I18N)})

# Add the email to the user's account if it is identified by the eppn
if email_to_add:
Expand Down
11 changes: 11 additions & 0 deletions api/subscriptions/fields.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from rest_framework import serializers as ser

class FrequencyField(ser.ChoiceField):
def __init__(self, **kwargs):
super().__init__(choices=['none', 'instantly', 'daily', 'weekly', 'monthly'], **kwargs)

def to_representation(self, frequency: str):
return frequency or 'none'

def to_internal_value(self, freq):
return super().to_internal_value(freq)
7 changes: 2 additions & 5 deletions api/subscriptions/permissions.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
from rest_framework import permissions

from osf.models.notifications import NotificationSubscription
from osf.models.notification_subscription import NotificationSubscription


class IsSubscriptionOwner(permissions.BasePermission):

def has_object_permission(self, request, view, obj):
assert isinstance(obj, NotificationSubscription), f'obj must be a NotificationSubscription; got {obj}'
user_id = request.user.id
return obj.none.filter(id=user_id).exists() \
or obj.email_transactional.filter(id=user_id).exists() \
or obj.email_digest.filter(id=user_id).exists()
return obj.user == request.user
57 changes: 27 additions & 30 deletions api/subscriptions/serializers.py
Original file line number Diff line number Diff line change
@@ -1,58 +1,55 @@
from django.contrib.contenttypes.models import ContentType
from rest_framework import serializers as ser
from rest_framework.exceptions import ValidationError
from api.nodes.serializers import RegistrationProviderRelationshipField
from api.collections_providers.fields import CollectionProviderRelationshipField
from api.preprints.serializers import PreprintProviderRelationshipField
from osf.models import Node
from website.util import api_v2_url


from api.base.serializers import JSONAPISerializer, LinksField

NOTIFICATION_TYPES = {
'none': 'none',
'instant': 'email_transactional',
'daily': 'email_digest',
}


class FrequencyField(ser.Field):
def to_representation(self, obj):
user_id = self.context['request'].user.id
if obj.email_transactional.filter(id=user_id).exists():
return 'instant'
if obj.email_digest.filter(id=user_id).exists():
return 'daily'
return 'none'

def to_internal_value(self, frequency):
notification_type = NOTIFICATION_TYPES.get(frequency)
if notification_type:
return {'notification_type': notification_type}
raise ValidationError(f'Invalid frequency "{frequency}"')
from .fields import FrequencyField

class SubscriptionSerializer(JSONAPISerializer):
filterable_fields = frozenset([
'id',
'event_name',
'frequency',
])

id = ser.CharField(source='_id', read_only=True)
id = ser.CharField(
read_only=True,
source='legacy_id',
help_text='The id of the subscription fixed for backward compatibility',
)
event_name = ser.CharField(read_only=True)
frequency = FrequencyField(source='*', required=True)
links = LinksField({
'self': 'get_absolute_url',
})
frequency = FrequencyField(source='message_frequency', required=True)

class Meta:
type_ = 'subscription'

links = LinksField({
'self': 'get_absolute_url',
})

def get_absolute_url(self, obj):
return obj.absolute_api_v2_url

def update(self, instance, validated_data):
user = self.context['request'].user
notification_type = validated_data.get('notification_type')
instance.add_user_to_subscription(user, notification_type, save=True)
frequency = validated_data.get('frequency') or 'none'
instance.message_frequency = frequency

if frequency != 'none' and instance.content_type == ContentType.objects.get_for_model(Node):
node = Node.objects.get(
id=instance.id,
content_type=instance.content_type,
)
user_subs = node.parent_node.child_node_subscriptions
if node._id not in user_subs.setdefault(user._id, []):
user_subs[user._id].append(node._id)
node.parent_node.save()

return instance


Expand Down
Loading
Loading