Skip to content

Commit 78eabe7

Browse files
cslzchencslzchen
committed
Merge tag '26.2.1' into develop
Notification refactor post-release hotfix
2 parents 8967a1c + c4b83ec commit 78eabe7

File tree

12 files changed

+194
-64
lines changed

12 files changed

+194
-64
lines changed

‎CHANGELOG‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,18 @@
22

33
We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO.
44

5+
26.2.1 (2026-02-09)
6+
===================
7+
8+
- Permanently update two notification types
9+
- Link digest to types and fix/log incorrect usage
10+
- Update notifiction dedupe command
11+
12+
26.2.0
13+
======
14+
15+
- TODO: add date and log
16+
517
26.1.6 (2026-01-14)
618
===================
719

‎api_tests/guids/views/test_guid_detail.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
PrivateLinkFactory,
1313
)
1414
from website.settings import API_DOMAIN
15+
from tests.utils import capture_notifications
1516

1617

1718
@pytest.mark.django_db
@@ -32,13 +33,14 @@ def registration(self):
3233
@pytest.fixture()
3334
def versioned_preprint(self, user):
3435
preprint = PreprintFactory(reviews_workflow='pre-moderation')
35-
PreprintFactory.create_version(
36-
create_from=preprint,
37-
creator=user,
38-
final_machine_state='accepted',
39-
is_published=True,
40-
set_doi=False
41-
)
36+
with capture_notifications():
37+
PreprintFactory.create_version(
38+
create_from=preprint,
39+
creator=user,
40+
final_machine_state='accepted',
41+
is_published=True,
42+
set_doi=False
43+
)
4244
return preprint
4345

4446
def test_redirects(self, app, project, registration, user):

‎api_tests/mailhog/provider/test_preprints.py‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from osf import features
33

44
from framework.auth.core import Auth
5-
from osf.models import NotificationType, Notification
5+
from osf.models import NotificationType
66
from osf_tests.factories import (
77
ProjectFactory,
88
AuthUserFactory,
@@ -12,7 +12,6 @@
1212
from osf.utils.permissions import WRITE
1313
from tests.base import OsfTestCase
1414
from tests.utils import get_mailhog_messages, delete_mailhog_messages, capture_notifications, assert_emails
15-
from notifications.tasks import send_users_instant_digest_email
1615

1716

1817
class TestPreprintConfirmationEmails(OsfTestCase):
@@ -35,25 +34,14 @@ def test_creator_gets_email(self):
3534
assert len(notifications['emits']) == 1
3635
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
3736
messages = get_mailhog_messages()
38-
assert not messages['items']
39-
assert Notification.objects.all()
40-
with capture_notifications(passthrough=True) as notifications:
41-
send_users_instant_digest_email.delay()
42-
43-
messages = get_mailhog_messages()
44-
assert messages['count'] == len(notifications['emits'])
37+
assert_emails(messages, notifications)
4538

4639
delete_mailhog_messages()
4740
with capture_notifications(passthrough=True) as notifications:
4841
self.preprint_branded.set_published(True, auth=Auth(self.user), save=True)
4942
assert len(notifications['emits']) == 1
5043
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
5144
messages = get_mailhog_messages()
52-
assert not messages['items']
53-
with capture_notifications(passthrough=True) as notifications:
54-
send_users_instant_digest_email.delay()
55-
massages = get_mailhog_messages()
56-
assert massages['count'] == len(notifications['emits'])
57-
assert_emails(massages, notifications)
45+
assert_emails(messages, notifications)
5846

5947
delete_mailhog_messages()

‎api_tests/mailhog/provider/test_submissions.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
from osf.models import NotificationType
2222

2323
from osf.migrations import update_provider_auth_groups
24-
from tests.utils import capture_notifications, get_mailhog_messages, delete_mailhog_messages
24+
from tests.utils import capture_notifications, get_mailhog_messages, delete_mailhog_messages, assert_emails
2525

2626

2727
@pytest.mark.django_db
@@ -85,8 +85,7 @@ def test_get_registration_actions(self, app, registration_actions_url, registrat
8585
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS
8686
assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS
8787
messages = get_mailhog_messages()
88-
assert messages['count'] == 1
89-
assert messages['items'][0]['Content']['Headers']['To'][0] == registration.creator.username
88+
assert_emails(messages, notifications)
9089

9190
delete_mailhog_messages()
9291

‎api_tests/share/test_share_preprint.py‎

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from website import settings
1919
from website.preprints.tasks import on_preprint_updated
2020
from ._utils import expect_preprint_ingest_request
21+
from tests.utils import capture_notifications
2122

2223

2324
@pytest.mark.django_db
@@ -72,54 +73,59 @@ def test_save_unpublished_not_called(self, mock_share_responses, preprint):
7273
preprint.save()
7374

7475
def test_save_published_called(self, mock_share_responses, preprint, user, auth):
75-
with expect_preprint_ingest_request(mock_share_responses, preprint):
76-
preprint.set_published(True, auth=auth, save=True)
76+
with capture_notifications():
77+
with expect_preprint_ingest_request(mock_share_responses, preprint):
78+
preprint.set_published(True, auth=auth, save=True)
7779

7880
# This covers an edge case where a preprint is forced back to unpublished
7981
# that it sends the information back to share
8082
def test_save_unpublished_called_forced(self, mock_share_responses, auth, preprint):
81-
with expect_preprint_ingest_request(mock_share_responses, preprint):
82-
preprint.set_published(True, auth=auth, save=True)
83-
with expect_preprint_ingest_request(mock_share_responses, preprint, delete=True):
84-
preprint.is_published = False
85-
preprint.save(**{'force_update': True})
83+
with capture_notifications():
84+
with expect_preprint_ingest_request(mock_share_responses, preprint):
85+
preprint.set_published(True, auth=auth, save=True)
86+
with expect_preprint_ingest_request(mock_share_responses, preprint, delete=True):
87+
preprint.is_published = False
88+
preprint.save(**{'force_update': True})
8689

8790
def test_save_published_subject_change_called(self, mock_share_responses, auth, preprint, subject, subject_two):
88-
preprint.set_published(True, auth=auth, save=True)
89-
with expect_preprint_ingest_request(mock_share_responses, preprint):
90-
preprint.set_subjects([[subject_two._id]], auth=auth)
91+
with capture_notifications():
92+
preprint.set_published(True, auth=auth, save=True)
93+
with expect_preprint_ingest_request(mock_share_responses, preprint):
94+
preprint.set_subjects([[subject_two._id]], auth=auth)
9195

9296
def test_save_unpublished_subject_change_not_called(self, mock_share_responses, auth, preprint, subject_two):
9397
with expect_preprint_ingest_request(mock_share_responses, preprint, delete=True):
9498
preprint.set_subjects([[subject_two._id]], auth=auth)
9599

96100
def test_send_to_share_is_true(self, mock_share_responses, auth, preprint):
97-
preprint.set_published(True, auth=auth, save=True)
98-
with expect_preprint_ingest_request(mock_share_responses, preprint):
99-
on_preprint_updated(preprint._id, saved_fields=['title'])
101+
with capture_notifications():
102+
preprint.set_published(True, auth=auth, save=True)
103+
with expect_preprint_ingest_request(mock_share_responses, preprint):
104+
on_preprint_updated(preprint._id, saved_fields=['title'])
100105

101106
def test_preprint_contributor_changes_updates_preprints_share(self, mock_share_responses, user, auth):
102-
preprint = PreprintFactory(is_published=True, creator=user)
103-
preprint.set_published(True, auth=auth, save=True)
104-
user2 = AuthUserFactory()
107+
with capture_notifications():
108+
preprint = PreprintFactory(is_published=True, creator=user)
109+
preprint.set_published(True, auth=auth, save=True)
110+
user2 = AuthUserFactory()
105111

106-
with expect_preprint_ingest_request(mock_share_responses, preprint):
107-
preprint.add_contributor(contributor=user2, auth=auth, save=True)
112+
with expect_preprint_ingest_request(mock_share_responses, preprint):
113+
preprint.add_contributor(contributor=user2, auth=auth, save=True)
108114

109-
with expect_preprint_ingest_request(mock_share_responses, preprint):
110-
preprint.move_contributor(contributor=user, index=0, auth=auth, save=True)
115+
with expect_preprint_ingest_request(mock_share_responses, preprint):
116+
preprint.move_contributor(contributor=user, index=0, auth=auth, save=True)
111117

112-
data = [{'id': user._id, 'permissions': ADMIN, 'visible': True},
113-
{'id': user2._id, 'permissions': WRITE, 'visible': False}]
118+
data = [{'id': user._id, 'permissions': ADMIN, 'visible': True},
119+
{'id': user2._id, 'permissions': WRITE, 'visible': False}]
114120

115-
with expect_preprint_ingest_request(mock_share_responses, preprint):
116-
preprint.manage_contributors(data, auth=auth, save=True)
121+
with expect_preprint_ingest_request(mock_share_responses, preprint):
122+
preprint.manage_contributors(data, auth=auth, save=True)
117123

118-
with expect_preprint_ingest_request(mock_share_responses, preprint):
119-
preprint.update_contributor(user2, READ, True, auth=auth, save=True)
124+
with expect_preprint_ingest_request(mock_share_responses, preprint):
125+
preprint.update_contributor(user2, READ, True, auth=auth, save=True)
120126

121-
with expect_preprint_ingest_request(mock_share_responses, preprint):
122-
preprint.remove_contributor(contributor=user2, auth=auth)
127+
with expect_preprint_ingest_request(mock_share_responses, preprint):
128+
preprint.remove_contributor(contributor=user2, auth=auth)
123129

124130
@pytest.mark.skip('Synchronous retries not supported if celery >=5.0')
125131
def test_call_async_update_on_500_failure(self, mock_share_responses, preprint, auth):
@@ -129,10 +135,11 @@ def test_call_async_update_on_500_failure(self, mock_share_responses, preprint,
129135
preprint.update_search()
130136

131137
def test_no_call_async_update_on_400_failure(self, mock_share_responses, preprint, auth):
132-
mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=400)
133-
preprint.set_published(True, auth=auth, save=True)
134-
with expect_preprint_ingest_request(mock_share_responses, preprint, count=1, error_response=True):
135-
preprint.update_search()
138+
with capture_notifications():
139+
mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=400)
140+
preprint.set_published(True, auth=auth, save=True)
141+
with expect_preprint_ingest_request(mock_share_responses, preprint, count=1, error_response=True):
142+
preprint.update_search()
136143

137144
def test_delete_from_share(self, mock_share_responses):
138145
preprint = PreprintFactory()

‎notifications/listeners.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ def reviews_withdraw_requests_notification_moderators(self, timestamp, context,
102102
NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.instance.emit(
103103
subscribed_object=provider,
104104
user=user,
105-
event_context=context
105+
event_context=context,
106+
is_digest=True,
106107
)
107108

108109

‎osf/management/commands/remove_duplicate_notification_subscriptions_v2.py‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
from django.core.management.base import BaseCommand
2+
from django.db import transaction
3+
from django.db.models import OuterRef, Exists, Q
4+
5+
from osf.models import NotificationSubscription, NotificationType
6+
7+
8+
class Command(BaseCommand):
9+
help = (
10+
'Remove duplicate NotificationSubscription records, keeping only the highest-id record: '
11+
'Default uniqueness: (user, content_type, object_id, notification_type, is_digest); '
12+
)
13+
14+
def add_arguments(self, parser):
15+
parser.add_argument(
16+
'--dry',
17+
action='store_true',
18+
help='Show how many rows would be deleted without deleting anything.',
19+
)
20+
21+
def handle(self, *args, **options):
22+
23+
self.stdout.write('Finding duplicate NotificationSubscription records…')
24+
digest_type_names = {
25+
# User types
26+
NotificationType.Type.USER_NO_ADDON.value,
27+
# File types
28+
NotificationType.Type.ADDON_FILE_COPIED.value,
29+
NotificationType.Type.ADDON_FILE_MOVED.value,
30+
NotificationType.Type.ADDON_FILE_RENAMED.value,
31+
NotificationType.Type.FILE_ADDED.value,
32+
NotificationType.Type.FILE_REMOVED.value,
33+
NotificationType.Type.FILE_UPDATED.value,
34+
NotificationType.Type.FOLDER_CREATED.value,
35+
NotificationType.Type.NODE_FILE_UPDATED.value,
36+
NotificationType.Type.USER_FILE_UPDATED.value,
37+
# Review types
38+
NotificationType.Type.COLLECTION_SUBMISSION_SUBMITTED.value,
39+
NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value,
40+
NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value,
41+
NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value,
42+
}
43+
44+
digest_type_ids = NotificationType.objects.filter(
45+
name__in=digest_type_names
46+
).values_list('id', flat=True)
47+
48+
invalid_non_digest = NotificationSubscription.objects.filter(
49+
notification_type_id__in=digest_type_ids,
50+
_is_digest=False,
51+
)
52+
invalid_non_digest_count = invalid_non_digest.count()
53+
54+
invalid_digest = NotificationSubscription.objects.filter(
55+
~Q(notification_type_id__in=digest_type_ids),
56+
_is_digest=True,
57+
)
58+
invalid_digest_count = invalid_digest.count()
59+
60+
if not options['dry']:
61+
invalid_non_digest.update(_is_digest=True)
62+
invalid_digest.update(_is_digest=False)
63+
64+
to_remove = NotificationSubscription.objects.filter(
65+
Exists(
66+
NotificationSubscription.objects.filter(
67+
user_id=OuterRef('user_id'),
68+
content_type_id=OuterRef('content_type_id'),
69+
object_id=OuterRef('object_id'),
70+
notification_type_id=OuterRef('notification_type_id'),
71+
_is_digest=OuterRef('_is_digest'),
72+
id__gt=OuterRef('id'), # keep most recent record
73+
)
74+
)
75+
)
76+
77+
count = to_remove.count()
78+
self.stdout.write(f"Duplicates to remove: {count}")
79+
self.stdout.write(f"Invalid non-digest records: {invalid_non_digest_count}")
80+
self.stdout.write(f"Invalid digest records: {invalid_digest_count}")
81+
82+
if count == 0 and invalid_non_digest_count == 0 and invalid_digest_count == 0:
83+
self.stdout.write(self.style.SUCCESS('No duplicates or invalid records found.'))
84+
return
85+
86+
if options['dry']:
87+
self.stdout.write(self.style.WARNING('Dry run enabled — no records were deleted.'))
88+
return
89+
90+
with transaction.atomic():
91+
deleted, _ = to_remove.delete()
92+
self.stdout.write(self.style.SUCCESS(f"Successfully removed {deleted} duplicate records."))
93+
self.stdout.write(self.style.SUCCESS(f"Successfully updated {invalid_non_digest_count} non-digest records."))
94+
self.stdout.write(self.style.SUCCESS(f"Successfully updated {invalid_digest_count} digest records."))

‎osf/models/notification_type.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,30 @@ def instance(self):
151151
obj, created = NotificationType.objects.get_or_create(name=self.value)
152152
return obj
153153

154+
@property
155+
def is_digest_type(self):
156+
digest_types = {
157+
# User types
158+
NotificationType.Type.USER_NO_ADDON.value,
159+
# File types
160+
NotificationType.Type.ADDON_FILE_COPIED.value,
161+
NotificationType.Type.ADDON_FILE_MOVED.value,
162+
NotificationType.Type.ADDON_FILE_RENAMED.value,
163+
NotificationType.Type.FILE_ADDED.value,
164+
NotificationType.Type.FILE_REMOVED.value,
165+
NotificationType.Type.FILE_UPDATED.value,
166+
NotificationType.Type.FOLDER_CREATED.value,
167+
NotificationType.Type.NODE_FILE_UPDATED.value,
168+
NotificationType.Type.USER_FILE_UPDATED.value,
169+
170+
# Review types
171+
NotificationType.Type.COLLECTION_SUBMISSION_SUBMITTED.value,
172+
NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value,
173+
NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value,
174+
NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value,
175+
}
176+
return self.name in digest_types
177+
154178
notification_interval_choices = ArrayField(
155179
base_field=models.CharField(max_length=32),
156180
default=get_default_frequency_choices,
@@ -204,6 +228,12 @@ def emit(
204228
from osf.models.notification_subscription import NotificationSubscription
205229
from osf.models.provider import AbstractProvider
206230

231+
if is_digest != self.is_digest_type:
232+
sentry.log_message(f'NotificationType.emit called with is_digest={is_digest} for '
233+
f'NotificationType {self.name} which has is_digest_type={self.is_digest_type}'
234+
'is_digest value will be overridden.')
235+
is_digest = self.is_digest_type
236+
207237
# use concrete model for AbstractProvider to specifically get the provider content type
208238
if isinstance(subscribed_object, AbstractProvider):
209239
content_type = ContentType.objects.get_for_model(subscribed_object, for_concrete_model=False) if subscribed_object else None

‎osf/models/preprint.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1094,7 +1094,6 @@ def _send_preprint_confirmation(self, auth):
10941094
'document_type': self.provider.preprint_word,
10951095
'notify_comment': not self.provider.reviews_comments_private
10961096
},
1097-
is_digest=True
10981097
)
10991098

11001099
# FOLLOWING BEHAVIOR NOT SPECIFIC TO PREPRINTS

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "OSF",
3-
"version": "26.1.6",
3+
"version": "26.2.1",
44
"description": "Facilitating Open Science",
55
"repository": "https://github.com/CenterForOpenScience/osf.io",
66
"author": "Center for Open Science",

0 commit comments

Comments
 (0)