Skip to content

[ENG-8087] Upgrade User Confirmation Registrations #11220

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 35 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
35 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
afc3815
Merge branch 'refactor-notifications' of https://github.com/CenterFor…
Johnetordoff Jul 10, 2025
98071ba
update NotificationType filter
Ostap-Zherebetskyi Jul 11, 2025
3fe9ef7
Merge remote-tracking branch 'upstream/refactor-notifications' into f…
Ostap-Zherebetskyi Jul 11, 2025
f04cb3d
fix unit tests
Ostap-Zherebetskyi Jul 11, 2025
da51f08
fix unit tests
Ostap-Zherebetskyi Jul 11, 2025
f407da4
fix unit tests
Ostap-Zherebetskyi Jul 11, 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
2 changes: 1 addition & 1 deletion admin/base/settings/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
CSRF_COOKIE_HTTPONLY = False

ALLOWED_HOSTS = [
'.osf.io',
'.osf.io'
]

AUTH_PASSWORD_VALIDATORS = [
Expand Down
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
18 changes: 10 additions & 8 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,13 +334,15 @@ 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,
notification_type = NotificationType.objects.get(name=NotificationType.Type.USER_WELCOME_OSF4I.value)
notification_type.emit(
user=user,
domain=DOMAIN,
osf_support_email=OSF_SUPPORT_EMAIL,
storage_flag_is_active=flag_is_active(request, features.STORAGE_I18N),
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
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