Skip to content

Commit 88103af

Browse files
cslzchenOstap-Zherebetskyi
cslzchen
and
Ostap-Zherebetskyi
authored
[ENG-10083] Add type<-->is_digest "relation" and update subscription (#11575)
* Add is_digest_type property to NotificationType and log message in emit method * Fix is_digest handling in NotificationType.emit method * Log message update in NotificationType.emit method for is_digest handling * fix unit tests --------- Co-authored-by: Ostap Zherebetskyi <ozherebetskyi@exoft.net>
1 parent e0edf59 commit 88103af

7 files changed

Lines changed: 85 additions & 59 deletions

File tree

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

0 commit comments

Comments
 (0)