Skip to content

Commit 7309cda

Browse files
Ostap-ZherebetskyiOstap-Zherebetskyi
committed
fix: update unregistered contributor notifications and email templates
1 parent 1e812ee commit 7309cda

20 files changed

+104
-122
lines changed

‎api/nodes/serializers.py‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
from django.core.exceptions import ValidationError
2929
from framework.auth.core import Auth
3030
from framework.exceptions import PermissionsError
31-
from osf.models import Tag, CollectionSubmission, NotificationType, OSFUser
31+
from osf.models import Tag, CollectionSubmission
3232
from rest_framework import serializers as ser
3333
from rest_framework import exceptions
3434
from addons.base.exceptions import InvalidAuthError, InvalidFolderError
@@ -1269,19 +1269,9 @@ def create(self, validated_data):
12691269
if email_pref not in self.email_preferences:
12701270
raise exceptions.ValidationError(f'{email_pref} is not a valid email preference.')
12711271

1272-
is_published = getattr(resource, 'is_published', False)
1273-
notification_type = {
1274-
'false': False,
1275-
'default': NotificationType.Type.NODE_CONTRIBUTOR_ADDED_DEFAULT,
1276-
'draft_registration': NotificationType.Type.DRAFT_REGISTRATION_CONTRIBUTOR_ADDED_DEFAULT,
1277-
'preprint': NotificationType.Type.PREPRINT_CONTRIBUTOR_ADDED_DEFAULT if is_published else False,
1278-
}.get(email_pref, False)
1279-
contributor = OSFUser.load(user_id)
1280-
notification_type = notification_type if email or (contributor and contributor.is_registered) else False
1281-
12821272
try:
12831273
contributor_dict = {
1284-
'auth': auth, 'user_id': user_id, 'email': email, 'full_name': full_name, 'notification_type': notification_type,
1274+
'auth': auth, 'user_id': user_id, 'email': email, 'full_name': full_name, 'notification_type': False if email_pref == 'false' else None,
12851275
'bibliographic': bibliographic, 'index': index, 'permissions': permissions,
12861276
}
12871277
contributor_obj = resource.add_contributor_registered_or_not(**contributor_dict)

‎api_tests/draft_registrations/views/test_draft_registration_contributor_list.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ def test_add_unregistered_contributor_sends_email(self, app, user, url_project_c
281281
)
282282
assert res.status_code == 201
283283
assert len(notifications['emits']) == 1
284-
assert notifications['emits'][0]['type'] == NotificationType.Type.DRAFT_REGISTRATION_CONTRIBUTOR_ADDED_DEFAULT
284+
assert notifications['emits'][0]['type'] == NotificationType.Type.USER_INVITE_DRAFT_REGISTRATION
285285

286286
# Overrides TestNodeContributorCreateEmail
287287
def test_add_unregistered_contributor_signal_if_default(self, app, user, url_project_contribs):
@@ -300,7 +300,7 @@ def test_add_unregistered_contributor_signal_if_default(self, app, user, url_pro
300300
)
301301
assert res.status_code == 201
302302
assert len(notifications['emits']) == 1
303-
assert notifications['emits'][0]['type'] == NotificationType.Type.DRAFT_REGISTRATION_CONTRIBUTOR_ADDED_DEFAULT
303+
assert notifications['emits'][0]['type'] == NotificationType.Type.USER_INVITE_DRAFT_REGISTRATION
304304

305305
# Overrides TestNodeContributorCreateEmail
306306
def test_add_unregistered_contributor_without_email_no_email(self, app, user, url_project_contribs):

‎api_tests/nodes/views/test_node_contributors_list.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,7 +1311,7 @@ def test_add_unregistered_contributor_sends_email(
13111311
)
13121312
assert res.status_code == 201
13131313
assert len(notifications['emits']) == 1
1314-
assert notifications['emits'][0]['type'] == NotificationType.Type.NODE_CONTRIBUTOR_ADDED_DEFAULT
1314+
assert notifications['emits'][0]['type'] == NotificationType.Type.USER_INVITE_DEFAULT
13151315

13161316
@mock.patch('website.project.signals.unreg_contributor_added.send')
13171317
def test_add_unregistered_contributor_signal_if_default(
@@ -1333,7 +1333,7 @@ def test_add_unregistered_contributor_signal_if_default(
13331333
)
13341334
assert res.status_code == 201
13351335
assert len(notifications['emits']) == 1
1336-
assert notifications['emits'][0]['type'] == NotificationType.Type.NODE_CONTRIBUTOR_ADDED_DEFAULT
1336+
assert notifications['emits'][0]['type'] == NotificationType.Type.USER_INVITE_DEFAULT
13371337

13381338
def test_add_unregistered_contributor_signal_preprint_email_disallowed(
13391339
self, app, user, url_project_contribs

‎api_tests/nodes/views/test_node_list.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,8 +1660,7 @@ def test_create_component_inherit_contributors(
16601660
}
16611661
}
16621662
}
1663-
with capture_notifications():
1664-
res = app.post_json_api(url, component_data, auth=user_one.auth)
1663+
res = app.post_json_api(url, component_data, auth=user_one.auth)
16651664
assert res.status_code == 201
16661665
json_data = res.json['data']
16671666

‎api_tests/preprints/views/test_preprint_contributors_list.py‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,7 +1463,7 @@ def test_add_unregistered_contributor_sends_email(
14631463
auth=user.auth
14641464
)
14651465
assert len(notifications['emits']) == 1
1466-
assert notifications['emits'][0]['type'] == NotificationType.Type.PREPRINT_CONTRIBUTOR_ADDED_DEFAULT
1466+
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_USER_INVITE_PREPRINT
14671467
assert res.status_code == 201
14681468

14691469
def test_add_unregistered_contributor_signal_if_preprint(self, app, user, url_preprint_contribs):
@@ -1483,7 +1483,7 @@ def test_add_unregistered_contributor_signal_if_preprint(self, app, user, url_pr
14831483
)
14841484
assert res.status_code == 201
14851485
assert len(notifications['emits']) == 1
1486-
assert notifications['emits'][0]['type'] == NotificationType.Type.PREPRINT_CONTRIBUTOR_ADDED_DEFAULT
1486+
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_USER_INVITE_PREPRINT
14871487

14881488
def test_add_contributor_invalid_send_email_param(self, app, user, url_preprint_contribs):
14891489
url = f'{url_preprint_contribs}?send_email=true'
@@ -1564,23 +1564,7 @@ def test_contributor_added_signal_not_specified(self, app, user, url_preprint_co
15641564
)
15651565
assert res.status_code == 201
15661566
assert len(notifications['emits']) == 1
1567-
assert notifications['emits'][0]['type'] == NotificationType.Type.PREPRINT_CONTRIBUTOR_ADDED_DEFAULT
1568-
1569-
def test_contributor_added_not_sent_if_unpublished(self, app, user, preprint_unpublished):
1570-
res = app.post_json_api(
1571-
f'/{API_BASE}preprints/{preprint_unpublished._id}/contributors/?send_email=preprint',
1572-
{
1573-
'data': {
1574-
'type': 'contributors',
1575-
'attributes': {
1576-
'full_name': 'Jalen Hurt',
1577-
'email': '[email protected]'
1578-
}
1579-
}
1580-
},
1581-
auth=user.auth
1582-
)
1583-
assert res.status_code == 201
1567+
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_USER_INVITE_PREPRINT
15841568

15851569
@pytest.mark.django_db
15861570
class TestPreprintContributorBulkCreate(NodeCRUDTestCase):

‎notifications.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,14 @@ notification_types:
109109
tests: ['osf_tests/test_institution.py']
110110
template: 'website/templates/institution_deactivation.html.mako'
111111

112-
- name: user_invite_preprints_osf
112+
- name: user_invite_osf_preprint
113113
subject: 'You have been added as a contributor to an OSF preprint.'
114114
__docs__: ...
115115
object_content_type_model_name: osfuser
116116
tests: []
117117
template: 'website/templates/invite_preprints_osf.html.mako'
118118

119-
- name: user_invite_preprints
119+
- name: provider_user_invite_preprint
120120
subject: 'You have been invited to contribute to a preprint'
121121
__docs__: ...
122122
object_content_type_model_name: osfuser

‎osf/models/mixins.py‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,7 +1499,7 @@ def add_contributors(
14991499
auth=None,
15001500
log=True,
15011501
save=False,
1502-
notification_type=NotificationType.Type.NODE_CONTRIBUTOR_ADDED_DEFAULT
1502+
notification_type=False,
15031503
):
15041504
"""Add multiple contributors
15051505
@@ -1592,6 +1592,19 @@ def add_unregistered_contributor(
15921592
else:
15931593
raise e
15941594

1595+
if notification_type is None:
1596+
from osf.models import AbstractNode, Preprint, DraftRegistration
1597+
1598+
if isinstance(self, AbstractNode):
1599+
notification_type = NotificationType.Type.USER_INVITE_DEFAULT
1600+
elif isinstance(self, Preprint):
1601+
if self.provider.is_default:
1602+
notification_type = NotificationType.Type.USER_INVITE_OSF_PREPRINT
1603+
else:
1604+
notification_type = NotificationType.Type.PROVIDER_USER_INVITE_PREPRINT
1605+
elif isinstance(self, DraftRegistration):
1606+
notification_type = NotificationType.Type.USER_INVITE_DRAFT_REGISTRATION
1607+
15951608
self.add_contributor(
15961609
contributor,
15971610
permissions=permissions,
@@ -1610,7 +1623,7 @@ def add_contributor_registered_or_not(self,
16101623
user_id=None,
16111624
full_name=None,
16121625
email=None,
1613-
notification_type=None,
1626+
notification_type=False,
16141627
permissions=None,
16151628
bibliographic=True,
16161629
index=None):

‎osf/models/notification_type.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class Type(str, Enum):
6666
USER_PENDING_INVITE = 'user_pending_invite' # unused
6767
USER_FORWARD_INVITE = 'user_forward_invite'
6868
USER_FORWARD_INVITE_REGISTERED = 'user_forward_invite_registered'
69-
USER_INVITE_DRAFT_REGISTRATION = 'user_invite_draft_registration' # unused same as DRAFT_REGISTRATION_CONTRIBUTOR_ADDED_DEFAULT
69+
USER_INVITE_DRAFT_REGISTRATION = 'user_invite_draft_registration'
7070
USER_INVITE_OSF_PREPRINT = 'user_invite_osf_preprint'
7171
USER_CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF = 'user_contributor_added_preprint_node_from_osf' # unused
7272
USER_CONTRIBUTOR_ADDED_ACCESS_REQUEST = 'user_contributor_added_access_request' # unused

‎osf_tests/test_draft_registration.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,8 @@ def test_add_contributors(self, draft_registration, auth):
348348
{'user': user1, 'permissions': ADMIN, 'visible': True},
349349
{'user': user2, 'permissions': WRITE, 'visible': False}
350350
],
351-
auth=auth
351+
auth=auth,
352+
notification_type=None
352353
)
353354
last_log = draft_registration.logs.all().order_by('-created')[0]
354355
assert (
@@ -510,7 +511,8 @@ def test_remove_contributors(self, draft_registration, auth):
510511
{'user': user1, 'permissions': WRITE, 'visible': True},
511512
{'user': user2, 'permissions': WRITE, 'visible': True}
512513
],
513-
auth=auth
514+
auth=auth,
515+
notification_type=None
514516
)
515517
assert user1 in draft_registration.contributors
516518
assert user2 in draft_registration.contributors

‎osf_tests/test_node.py‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,8 @@ def test_add_contributors(self, node, auth):
916916
{'user': user1, 'permissions': ADMIN, 'visible': True},
917917
{'user': user2, 'permissions': WRITE, 'visible': False}
918918
],
919-
auth=auth
919+
auth=auth,
920+
notification_type=None
920921
)
921922
last_log = node.logs.all().order_by('-date')[0]
922923
assert (
@@ -1114,7 +1115,8 @@ def test_remove_contributors(self, node, auth):
11141115
{'user': user1, 'permissions': permissions.WRITE, 'visible': True},
11151116
{'user': user2, 'permissions': permissions.WRITE, 'visible': True}
11161117
],
1117-
auth=auth
1118+
auth=auth,
1119+
notification_type=None
11181120
)
11191121
assert user1 in node.contributors
11201122
assert user2 in node.contributors
@@ -1232,7 +1234,7 @@ class TestNodeAddContributorRegisteredOrNot:
12321234
def test_add_contributor_user_id(self, user, node):
12331235
registered_user = UserFactory()
12341236
with capture_notifications():
1235-
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), user_id=registered_user._id)
1237+
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), user_id=registered_user._id, notification_type=None)
12361238
contributor = contributor_obj.user
12371239
assert contributor in node.contributors
12381240
assert contributor.is_registered is True
@@ -1241,7 +1243,7 @@ def test_add_contributor_registered_or_not_unreg_user_without_unclaimed_records(
12411243
unregistered_user = UnregUserFactory()
12421244
unregistered_user.save()
12431245
with capture_notifications():
1244-
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), email=unregistered_user.email, full_name=unregistered_user.fullname)
1246+
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), email=unregistered_user.email, full_name=unregistered_user.fullname, notification_type=None)
12451247

12461248
contributor = contributor_obj.user
12471249
assert contributor in node.contributors
@@ -1260,22 +1262,22 @@ def test_add_contributor_invalid_user_id(self, user, node):
12601262

12611263
def test_add_contributor_fullname_email(self, user, node):
12621264
with capture_notifications():
1263-
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='Jane Doe', email='[email protected]')
1265+
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='Jane Doe', email='[email protected]', notification_type=None)
12641266
contributor = contributor_obj.user
12651267
assert contributor in node.contributors
12661268
assert contributor.is_registered is False
12671269

12681270
def test_add_contributor_fullname(self, user, node):
1269-
with capture_notifications():
1270-
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='Jane Doe')
1271+
with capture_notifications(expect_none=True):
1272+
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='Jane Doe', notification_type=None)
12711273
contributor = contributor_obj.user
12721274
assert contributor in node.contributors
12731275
assert contributor.is_registered is False
12741276

12751277
def test_add_contributor_fullname_email_already_exists(self, user, node):
12761278
registered_user = UserFactory()
12771279
with capture_notifications():
1278-
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='F Mercury', email=registered_user.username)
1280+
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='F Mercury', email=registered_user.username, notification_type=None)
12791281
contributor = contributor_obj.user
12801282
assert contributor in node.contributors
12811283
assert contributor.is_registered is True
@@ -1284,7 +1286,7 @@ def test_add_contributor_fullname_email_exists_as_secondary(self, user, node):
12841286
registered_user = UserFactory()
12851287
secondary_email = '[email protected]'
12861288
Email.objects.create(address=secondary_email, user=registered_user)
1287-
with capture_notifications():
1289+
with capture_notifications(expect_none=True):
12881290
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='F Mercury', email=secondary_email)
12891291
contributor = contributor_obj.user
12901292
assert contributor == registered_user
@@ -1295,7 +1297,7 @@ def test_add_contributor_unregistered(self, user, node):
12951297
unregistered_user = UnregUserFactory()
12961298
unregistered_user.save()
12971299
with capture_notifications():
1298-
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name=unregistered_user.fullname, email=unregistered_user.email)
1300+
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name=unregistered_user.fullname, email=unregistered_user.email, notification_type=None)
12991301
contributor = contributor_obj.user
13001302
assert contributor == unregistered_user
13011303
assert contributor in node.contributors
@@ -1345,8 +1347,7 @@ def test_add_contributors_sends_contributor_added_signal(self, node, auth):
13451347
'permissions': permissions.WRITE
13461348
}]
13471349
with capture_signals() as mock_signals:
1348-
with capture_notifications():
1349-
node.add_contributors(contributors=contributors, auth=auth)
1350+
node.add_contributors(contributors=contributors, auth=auth)
13501351
node.save()
13511352
assert node.is_contributor(user)
13521353
assert mock_signals.signals_sent() == {contributor_added}
@@ -2612,7 +2613,8 @@ def test_contributor_set_visibility_validation(self, node, user, auth):
26122613
[
26132614
{'user': reg_user1, 'permissions': ADMIN, 'visible': True},
26142615
{'user': reg_user2, 'permissions': ADMIN, 'visible': False},
2615-
]
2616+
],
2617+
notification_type=None
26162618
)
26172619
with pytest.raises(ValueError) as e:
26182620
node.set_visible(user=reg_user1, visible=False, auth=None)
@@ -3364,7 +3366,8 @@ def test_move_contributor(self, user, node, auth):
33643366
{'user': user1, 'permissions': WRITE, 'visible': True},
33653367
{'user': user2, 'permissions': WRITE, 'visible': True}
33663368
],
3367-
auth=auth
3369+
auth=auth,
3370+
notification_type=None
33683371
)
33693372

33703373
user_contrib_id = node.contributor_set.get(user=user).id

0 commit comments

Comments
 (0)