Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 2 additions & 12 deletions api/nodes/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from django.core.exceptions import ValidationError
from framework.auth.core import Auth
from framework.exceptions import PermissionsError
from osf.models import Tag, CollectionSubmission, NotificationType, OSFUser
from osf.models import Tag, CollectionSubmission
from rest_framework import serializers as ser
from rest_framework import exceptions
from addons.base.exceptions import InvalidAuthError, InvalidFolderError
Expand Down Expand Up @@ -1269,19 +1269,9 @@ def create(self, validated_data):
if email_pref not in self.email_preferences:
raise exceptions.ValidationError(f'{email_pref} is not a valid email preference.')

is_published = getattr(resource, 'is_published', False)
notification_type = {
'false': False,
'default': NotificationType.Type.NODE_CONTRIBUTOR_ADDED_DEFAULT,
'draft_registration': NotificationType.Type.DRAFT_REGISTRATION_CONTRIBUTOR_ADDED_DEFAULT,
'preprint': NotificationType.Type.PREPRINT_CONTRIBUTOR_ADDED_DEFAULT if is_published else False,
}.get(email_pref, False)
contributor = OSFUser.load(user_id)
notification_type = notification_type if email or (contributor and contributor.is_registered) else False

try:
contributor_dict = {
'auth': auth, 'user_id': user_id, 'email': email, 'full_name': full_name, 'notification_type': notification_type,
'auth': auth, 'user_id': user_id, 'email': email, 'full_name': full_name, 'notification_type': False if email_pref == 'false' else None,
'bibliographic': bibliographic, 'index': index, 'permissions': permissions,
}
contributor_obj = resource.add_contributor_registered_or_not(**contributor_dict)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def test_add_unregistered_contributor_sends_email(self, app, user, url_project_c
)
assert res.status_code == 201
assert len(notifications['emits']) == 1
assert notifications['emits'][0]['type'] == NotificationType.Type.DRAFT_REGISTRATION_CONTRIBUTOR_ADDED_DEFAULT
assert notifications['emits'][0]['type'] == NotificationType.Type.USER_INVITE_DRAFT_REGISTRATION

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

# Overrides TestNodeContributorCreateEmail
def test_add_unregistered_contributor_without_email_no_email(self, app, user, url_project_contribs):
Expand Down
4 changes: 2 additions & 2 deletions api_tests/nodes/views/test_node_contributors_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,7 @@ def test_add_unregistered_contributor_sends_email(
)
assert res.status_code == 201
assert len(notifications['emits']) == 1
assert notifications['emits'][0]['type'] == NotificationType.Type.NODE_CONTRIBUTOR_ADDED_DEFAULT
assert notifications['emits'][0]['type'] == NotificationType.Type.USER_INVITE_DEFAULT

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

def test_add_unregistered_contributor_signal_preprint_email_disallowed(
self, app, user, url_project_contribs
Expand Down
3 changes: 1 addition & 2 deletions api_tests/nodes/views/test_node_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -1660,8 +1660,7 @@ def test_create_component_inherit_contributors(
}
}
}
with capture_notifications():
res = app.post_json_api(url, component_data, auth=user_one.auth)
res = app.post_json_api(url, component_data, auth=user_one.auth)
assert res.status_code == 201
json_data = res.json['data']

Expand Down
22 changes: 3 additions & 19 deletions api_tests/preprints/views/test_preprint_contributors_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -1463,7 +1463,7 @@ def test_add_unregistered_contributor_sends_email(
auth=user.auth
)
assert len(notifications['emits']) == 1
assert notifications['emits'][0]['type'] == NotificationType.Type.PREPRINT_CONTRIBUTOR_ADDED_DEFAULT
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_USER_INVITE_PREPRINT
assert res.status_code == 201

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

def test_add_contributor_invalid_send_email_param(self, app, user, url_preprint_contribs):
url = f'{url_preprint_contribs}?send_email=true'
Expand Down Expand Up @@ -1564,23 +1564,7 @@ def test_contributor_added_signal_not_specified(self, app, user, url_preprint_co
)
assert res.status_code == 201
assert len(notifications['emits']) == 1
assert notifications['emits'][0]['type'] == NotificationType.Type.PREPRINT_CONTRIBUTOR_ADDED_DEFAULT

def test_contributor_added_not_sent_if_unpublished(self, app, user, preprint_unpublished):
res = app.post_json_api(
f'/{API_BASE}preprints/{preprint_unpublished._id}/contributors/?send_email=preprint',
{
'data': {
'type': 'contributors',
'attributes': {
'full_name': 'Jalen Hurt',
'email': '[email protected]'
}
}
},
auth=user.auth
)
assert res.status_code == 201
assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_USER_INVITE_PREPRINT

@pytest.mark.django_db
class TestPreprintContributorBulkCreate(NodeCRUDTestCase):
Expand Down
4 changes: 2 additions & 2 deletions notifications.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,14 @@ notification_types:
tests: ['osf_tests/test_institution.py']
template: 'website/templates/institution_deactivation.html.mako'

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

- name: user_invite_preprints
- name: provider_user_invite_preprint
subject: 'You have been invited to contribute to a preprint'
__docs__: ...
object_content_type_model_name: osfuser
Expand Down
17 changes: 15 additions & 2 deletions osf/models/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -1499,7 +1499,7 @@ def add_contributors(
auth=None,
log=True,
save=False,
notification_type=NotificationType.Type.NODE_CONTRIBUTOR_ADDED_DEFAULT
notification_type=False,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

False: we don't want user to receive any emails
None: we will just wait for the caller to provide the template

):
"""Add multiple contributors

Expand Down Expand Up @@ -1592,6 +1592,19 @@ def add_unregistered_contributor(
else:
raise e

if notification_type is None:
from osf.models import AbstractNode, Preprint, DraftRegistration

if isinstance(self, AbstractNode):
notification_type = NotificationType.Type.USER_INVITE_DEFAULT
elif isinstance(self, Preprint):
if self.provider.is_default:
notification_type = NotificationType.Type.USER_INVITE_OSF_PREPRINT
else:
notification_type = NotificationType.Type.PROVIDER_USER_INVITE_PREPRINT
elif isinstance(self, DraftRegistration):
notification_type = NotificationType.Type.USER_INVITE_DRAFT_REGISTRATION
Comment on lines +1598 to +1606
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove "unused" comments in the types definition for these ones.


self.add_contributor(
contributor,
permissions=permissions,
Expand All @@ -1610,7 +1623,7 @@ def add_contributor_registered_or_not(self,
user_id=None,
full_name=None,
email=None,
notification_type=None,
notification_type=False,
permissions=None,
bibliographic=True,
index=None):
Expand Down
2 changes: 1 addition & 1 deletion osf/models/notification_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class Type(str, Enum):
USER_PENDING_INVITE = 'user_pending_invite' # unused
USER_FORWARD_INVITE = 'user_forward_invite'
USER_FORWARD_INVITE_REGISTERED = 'user_forward_invite_registered'
USER_INVITE_DRAFT_REGISTRATION = 'user_invite_draft_registration' # unused same as DRAFT_REGISTRATION_CONTRIBUTOR_ADDED_DEFAULT
USER_INVITE_DRAFT_REGISTRATION = 'user_invite_draft_registration'
USER_INVITE_OSF_PREPRINT = 'user_invite_osf_preprint'
USER_CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF = 'user_contributor_added_preprint_node_from_osf' # unused
USER_CONTRIBUTOR_ADDED_ACCESS_REQUEST = 'user_contributor_added_access_request' # unused
Expand Down
6 changes: 4 additions & 2 deletions osf_tests/test_draft_registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ def test_add_contributors(self, draft_registration, auth):
{'user': user1, 'permissions': ADMIN, 'visible': True},
{'user': user2, 'permissions': WRITE, 'visible': False}
],
auth=auth
auth=auth,
notification_type=None
)
last_log = draft_registration.logs.all().order_by('-created')[0]
assert (
Expand Down Expand Up @@ -510,7 +511,8 @@ def test_remove_contributors(self, draft_registration, auth):
{'user': user1, 'permissions': WRITE, 'visible': True},
{'user': user2, 'permissions': WRITE, 'visible': True}
],
auth=auth
auth=auth,
notification_type=None
)
assert user1 in draft_registration.contributors
assert user2 in draft_registration.contributors
Expand Down
31 changes: 17 additions & 14 deletions osf_tests/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,8 @@ def test_add_contributors(self, node, auth):
{'user': user1, 'permissions': ADMIN, 'visible': True},
{'user': user2, 'permissions': WRITE, 'visible': False}
],
auth=auth
auth=auth,
notification_type=None
)
last_log = node.logs.all().order_by('-date')[0]
assert (
Expand Down Expand Up @@ -1114,7 +1115,8 @@ def test_remove_contributors(self, node, auth):
{'user': user1, 'permissions': permissions.WRITE, 'visible': True},
{'user': user2, 'permissions': permissions.WRITE, 'visible': True}
],
auth=auth
auth=auth,
notification_type=None
)
assert user1 in node.contributors
assert user2 in node.contributors
Expand Down Expand Up @@ -1232,7 +1234,7 @@ class TestNodeAddContributorRegisteredOrNot:
def test_add_contributor_user_id(self, user, node):
registered_user = UserFactory()
with capture_notifications():
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), user_id=registered_user._id)
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), user_id=registered_user._id, notification_type=None)
contributor = contributor_obj.user
assert contributor in node.contributors
assert contributor.is_registered is True
Expand All @@ -1241,7 +1243,7 @@ def test_add_contributor_registered_or_not_unreg_user_without_unclaimed_records(
unregistered_user = UnregUserFactory()
unregistered_user.save()
with capture_notifications():
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), email=unregistered_user.email, full_name=unregistered_user.fullname)
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), email=unregistered_user.email, full_name=unregistered_user.fullname, notification_type=None)

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

def test_add_contributor_fullname_email(self, user, node):
with capture_notifications():
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='Jane Doe', email='[email protected]')
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='Jane Doe', email='[email protected]', notification_type=None)
contributor = contributor_obj.user
assert contributor in node.contributors
assert contributor.is_registered is False

def test_add_contributor_fullname(self, user, node):
with capture_notifications():
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='Jane Doe')
with capture_notifications(expect_none=True):
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='Jane Doe', notification_type=None)
contributor = contributor_obj.user
assert contributor in node.contributors
assert contributor.is_registered is False

def test_add_contributor_fullname_email_already_exists(self, user, node):
registered_user = UserFactory()
with capture_notifications():
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='F Mercury', email=registered_user.username)
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='F Mercury', email=registered_user.username, notification_type=None)
contributor = contributor_obj.user
assert contributor in node.contributors
assert contributor.is_registered is True
Expand All @@ -1284,7 +1286,7 @@ def test_add_contributor_fullname_email_exists_as_secondary(self, user, node):
registered_user = UserFactory()
secondary_email = '[email protected]'
Email.objects.create(address=secondary_email, user=registered_user)
with capture_notifications():
with capture_notifications(expect_none=True):
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name='F Mercury', email=secondary_email)
contributor = contributor_obj.user
assert contributor == registered_user
Expand All @@ -1295,7 +1297,7 @@ def test_add_contributor_unregistered(self, user, node):
unregistered_user = UnregUserFactory()
unregistered_user.save()
with capture_notifications():
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name=unregistered_user.fullname, email=unregistered_user.email)
contributor_obj = node.add_contributor_registered_or_not(auth=Auth(user), full_name=unregistered_user.fullname, email=unregistered_user.email, notification_type=None)
contributor = contributor_obj.user
assert contributor == unregistered_user
assert contributor in node.contributors
Expand Down Expand Up @@ -1345,8 +1347,7 @@ def test_add_contributors_sends_contributor_added_signal(self, node, auth):
'permissions': permissions.WRITE
}]
with capture_signals() as mock_signals:
with capture_notifications():
node.add_contributors(contributors=contributors, auth=auth)
node.add_contributors(contributors=contributors, auth=auth)
node.save()
assert node.is_contributor(user)
assert mock_signals.signals_sent() == {contributor_added}
Expand Down Expand Up @@ -2612,7 +2613,8 @@ def test_contributor_set_visibility_validation(self, node, user, auth):
[
{'user': reg_user1, 'permissions': ADMIN, 'visible': True},
{'user': reg_user2, 'permissions': ADMIN, 'visible': False},
]
],
notification_type=None
)
with pytest.raises(ValueError) as e:
node.set_visible(user=reg_user1, visible=False, auth=None)
Expand Down Expand Up @@ -3364,7 +3366,8 @@ def test_move_contributor(self, user, node, auth):
{'user': user1, 'permissions': WRITE, 'visible': True},
{'user': user2, 'permissions': WRITE, 'visible': True}
],
auth=auth
auth=auth,
notification_type=None
)

user_contrib_id = node.contributor_set.get(user=user).id
Expand Down
Loading
Loading