Skip to content

Commit c57313a

Browse files
authored
Merge pull request #11308 from Ostap-Zherebetskyi/fix/submission_notifications
[ENG-8848] Preprint pending submission notifications: Content of email is not relevant when new preprint is submitted to OSFPreprints
2 parents 7d7578a + 98579bf commit c57313a

File tree

6 files changed

+15
-75
lines changed

6 files changed

+15
-75
lines changed

api_tests/mailhog/provider/test_schema_responses.py

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,11 @@ def test_accept_notification_sent_on_admin_approval(self, revised_response, admi
205205
delete_mailhog_messages()
206206
with capture_notifications(passthrough=True) as notifications:
207207
revised_response.approve(user=admin_user)
208-
assert len(notifications['emits']) == 3
208+
assert len(notifications['emits']) == 2
209209
assert notifications['emits'][0]['kwargs']['user'] == moderator
210210
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
211-
assert notifications['emits'][1]['kwargs']['user'] == moderator
212-
assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
213-
assert notifications['emits'][2]['kwargs']['user'] == admin_user
214-
assert notifications['emits'][2]['type'] == NotificationType.Type.NODE_SCHEMA_RESPONSE_APPROVED
211+
assert notifications['emits'][1]['kwargs']['user'] == admin_user
212+
assert notifications['emits'][1]['type'] == NotificationType.Type.NODE_SCHEMA_RESPONSE_APPROVED
215213
massages = get_mailhog_messages()
216214
assert massages['count'] == len(notifications['emails'])
217215
assert_emails(massages, notifications)
@@ -226,13 +224,11 @@ def test_moderators_notified_on_admin_approval(self, revised_response, admin_use
226224
delete_mailhog_messages()
227225
with capture_notifications(passthrough=True) as notifications:
228226
revised_response.approve(user=admin_user)
229-
assert len(notifications['emits']) == 3
227+
assert len(notifications['emits']) == 2
230228
assert notifications['emits'][0]['kwargs']['user'] == moderator
231229
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
232-
assert notifications['emits'][1]['kwargs']['user'] == moderator
233-
assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
234-
assert notifications['emits'][2]['kwargs']['user'] == admin_user
235-
assert notifications['emits'][2]['type'] == NotificationType.Type.NODE_SCHEMA_RESPONSE_APPROVED
230+
assert notifications['emits'][1]['kwargs']['user'] == admin_user
231+
assert notifications['emits'][1]['type'] == NotificationType.Type.NODE_SCHEMA_RESPONSE_APPROVED
236232
massages = get_mailhog_messages()
237233
assert massages['count'] == len(notifications['emails'])
238234
assert_emails(massages, notifications)

api_tests/mailhog/provider/test_submissions.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,9 @@ def test_get_provider_actions(self, app, provider_actions_url, registration, mod
113113

114114
resp = app.get(provider_actions_url, auth=moderator.auth)
115115

116-
assert len(notifications['emits']) == 3
116+
assert len(notifications['emits']) == 2
117117
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
118118
assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
119-
assert notifications['emits'][2]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
120119
massages = get_mailhog_messages()
121120
assert massages['count'] == len(notifications['emails'])
122121
assert_emails(massages, notifications)

notifications/listeners.py

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import logging
22

33
from django.apps import apps
4-
from django.utils import timezone
54

65
from website.project.signals import contributor_added, project_created, node_deleted, contributor_removed
76
from website.reviews import signals as reviews_signals
@@ -61,55 +60,6 @@ def subscribe_contributor(resource, contributor, auth=None, *args, **kwargs):
6160
)
6261

6362

64-
@reviews_signals.reviews_email_submit_moderators_notifications.connect
65-
def reviews_submit_notification_moderators(self, timestamp, context, resource):
66-
"""
67-
Handle email notifications to notify moderators of new submissions or resubmission.
68-
"""
69-
70-
# imports moved here to avoid AppRegistryNotReady error
71-
from osf.models import NotificationType
72-
from website.settings import DOMAIN
73-
74-
provider = resource.provider
75-
context['domain'] = DOMAIN
76-
context['requester_contributor_names'] = ''.join(resource.contributors.values_list('fullname', flat=True))
77-
context['localized_timestamp'] = str(timezone.now()),
78-
79-
# Set submission url
80-
if provider.type == 'osf.preprintprovider':
81-
context['reviews_submission_url'] = (
82-
f'{DOMAIN}reviews/preprints/{provider._id}/{resource._id}'
83-
)
84-
elif provider.type == 'osf.registrationprovider':
85-
context['reviews_submission_url'] = f'{DOMAIN}{resource._id}?mode=moderator'
86-
else:
87-
raise NotImplementedError(f'unsupported provider type {provider.type}')
88-
89-
# Set message
90-
revision_id = context.get('revision_id')
91-
if revision_id:
92-
context['message'] = f'submitted updates to "{resource.title}".'
93-
context['reviews_submission_url'] += f'&revisionId={revision_id}'
94-
else:
95-
if context.get('resubmission'):
96-
context['message'] = f'resubmitted "{resource.title}".'
97-
else:
98-
context['message'] = f'submitted "{resource.title}".'
99-
100-
for recipient in resource.provider.get_group('moderator').user_set.all():
101-
context['user_fullname'] = recipient.fullname
102-
context['recipient_fullname'] = recipient.fullname
103-
context['requester_fullname'] = recipient.fullname
104-
context['is_request_email'] = False
105-
106-
NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.instance.emit(
107-
user=recipient,
108-
subscribed_object=provider,
109-
event_context=context,
110-
is_digest=True,
111-
)
112-
11363
# Handle email notifications to notify moderators of new submissions.
11464
@reviews_signals.reviews_withdraw_requests_notification_moderators.connect
11565
def reviews_withdraw_requests_notification_moderators(self, timestamp, context, user, resource):

osf/email/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ def _read_lookup_uri(uri: str) -> str:
119119

120120
NOTIFY_BASE_DEFAULTS = {
121121
'logo': settings.OSF_LOGO, # matches default in notify_base.mako
122-
'logo_url': settings.OSF_LOGO,
122+
'logo_url': None,
123123
'node_url': '',
124124
'ns_url': '',
125125
'osf_contact_email': settings.OSF_CONTACT_EMAIL,

osf_tests/test_registration_moderation_notifications.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,12 @@ def test_submit_notifications(self, registration, moderator, admin, contrib, pro
134134
with capture_notifications() as notification:
135135
notify_submit(registration, admin)
136136

137-
assert len(notification['emits']) == 4
137+
assert len(notification['emits']) == 3
138138
assert notification['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
139139
assert notification['emits'][0]['kwargs']['user'] == admin
140140
assert notification['emits'][1]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
141141
assert notification['emits'][1]['kwargs']['user'] == contrib
142142
assert notification['emits'][2]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
143-
assert notification['emits'][3]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
144143

145144
assert NotificationSubscription.objects.count() == 5
146145
digest = NotificationSubscription.objects.last()

osf_tests/test_schema_responses.py

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -859,13 +859,11 @@ def test_accept_notification_sent_on_admin_approval(self, revised_response, admi
859859

860860
with capture_notifications() as notifications:
861861
revised_response.approve(user=admin_user)
862-
assert len(notifications['emits']) == 3
862+
assert len(notifications['emits']) == 2
863863
assert notifications['emits'][0]['kwargs']['user'] == moderator
864864
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
865-
assert notifications['emits'][1]['kwargs']['user'] == moderator
866-
assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
867-
assert notifications['emits'][2]['kwargs']['user'] == admin_user
868-
assert notifications['emits'][2]['type'] == NotificationType.Type.NODE_SCHEMA_RESPONSE_APPROVED
865+
assert notifications['emits'][1]['kwargs']['user'] == admin_user
866+
assert notifications['emits'][1]['type'] == NotificationType.Type.NODE_SCHEMA_RESPONSE_APPROVED
869867

870868
def test_moderators_notified_on_admin_approval(self, revised_response, admin_user, moderator):
871869
revised_response.approvals_state_machine.set_state(ApprovalStates.UNAPPROVED)
@@ -874,13 +872,11 @@ def test_moderators_notified_on_admin_approval(self, revised_response, admin_use
874872

875873
with capture_notifications() as notifications:
876874
revised_response.approve(user=admin_user)
877-
assert len(notifications['emits']) == 3
875+
assert len(notifications['emits']) == 2
878876
assert notifications['emits'][0]['kwargs']['user'] == moderator
879877
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
880-
assert notifications['emits'][1]['kwargs']['user'] == moderator
881-
assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS
882-
assert notifications['emits'][2]['kwargs']['user'] == admin_user
883-
assert notifications['emits'][2]['type'] == NotificationType.Type.NODE_SCHEMA_RESPONSE_APPROVED
878+
assert notifications['emits'][1]['kwargs']['user'] == admin_user
879+
assert notifications['emits'][1]['type'] == NotificationType.Type.NODE_SCHEMA_RESPONSE_APPROVED
884880

885881
def test_no_moderator_notification_on_admin_approval_of_initial_response(
886882
self, initial_response, admin_user):

0 commit comments

Comments
 (0)