From c2191f8f0b5821c799f6493d024e6d2dea9a14da Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Tue, 4 Nov 2025 02:52:14 +0200 Subject: [PATCH 1/2] add list_serializer_class for NodeContributorsCreateSerializer; minor optimization --- api/base/serializers.py | 5 ++ api/nodes/serializers.py | 119 +++++++++++++++++++++++++++++++++++++-- osf/models/mixins.py | 31 ++++++---- osf/utils/permissions.py | 4 ++ 4 files changed, 143 insertions(+), 16 deletions(-) diff --git a/api/base/serializers.py b/api/base/serializers.py index 5f430fbf7e6..a587f65ed3e 100644 --- a/api/base/serializers.py +++ b/api/base/serializers.py @@ -1456,6 +1456,11 @@ class JSONAPISerializer(BaseAPISerializer): @classmethod def many_init(cls, *args, **kwargs): kwargs['child'] = cls(*args, **kwargs) + # Use DRF list_serializer_class if it exists, otherwise use default JSONAPIListSerializer + meta = getattr(cls, 'Meta', None) + list_cls = getattr(meta, 'list_serializer_class', None) + if list_cls: + return list_cls(*args, **kwargs) return JSONAPIListSerializer(*args, **kwargs) def invalid_embeds(self, fields, embeds): diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 53b28d7b0ba..c63f76370e6 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1,4 +1,4 @@ -from django.db import connection +from django.db import connection, transaction from packaging.version import Version from api.base.exceptions import ( @@ -7,7 +7,7 @@ ) from api.base.serializers import ( VersionedDateTimeField, HideIfRegistration, IDField, - JSONAPISerializer, LinksField, + JSONAPISerializer, JSONAPIListSerializer, LinksField, NodeFileHyperLinkField, RelationshipField, ShowIfVersion, TargetTypeField, TypeField, WaterbutlerLink, BaseAPISerializer, @@ -38,7 +38,7 @@ Comment, DraftRegistration, ExternalAccount, RegistrationSchema, AbstractNode, PrivateLink, Preprint, RegistrationProvider, NodeLicense, DraftNode, - Registration, Node, + Registration, Node, OSFUser, ) from website.project import new_private_link from website.project.model import NodeUpdateError @@ -1219,6 +1219,113 @@ def get_unregistered_contributor(self, obj): return unclaimed_records.get('name', None) +class NodeContributorsBulkCreateListSerializer(JSONAPIListSerializer): + + def create(self, validated_data): + request = self.context['request'] + node = self.context['resource'] + auth = Auth(request.user) + + with transaction.atomic(): + results = [] + registered_items = [] + unregistered_items = [] + + for item in validated_data: + if item.get('_id') and not item.get('user', {}).get('email'): + registered_items.append(item) + else: + unregistered_items.append(item) + + def _perm(item): + return osf_permissions.get_contributor_proposed_permissions(item) + + if registered_items: + # Load users once and build a mapping to reuse + user_map = {} + for item in registered_items: + user_id = item.get('_id') + if user_id and user_id not in user_map: + user = OSFUser.load(user_id) + if not user: + raise exceptions.NotFound(detail=f"User {user_id} not found") + user_map[user_id] = user + + contrib_dicts = [] + for item in registered_items: + user = user_map[item.get('_id')] + contrib_dicts.append({ + 'user': user, + 'permissions': _perm(item), + 'visible': item.get('bibliographic'), + }) + contribs = node.add_contributors(contrib_dicts, auth=auth, log=True, save=True) + results.extend(contribs) + + child_to_items = {} + for item in registered_items: + child_nodes = item.get('child_nodes') + if child_nodes: + for child_id in child_nodes: + child_to_items.setdefault(child_id, []).append(item) + + for child_id, items in child_to_items.items(): + child = AbstractNode.load(child_id) + if not child: + continue + child_contribs = [] + for item in items: + child_contribs.append({ + 'user': user_map[item.get('_id')], + 'permissions': _perm(item), + 'visible': item.get('bibliographic'), + }) + child.add_contributors(child_contribs, auth=auth, log=True, save=True) + + for item in unregistered_items: + id = item.get('_id') + email = item.get('user', {}).get('email', None) + full_name = item.get('full_name') + bibliographic = item.get('bibliographic') + index = item.get('_order') if '_order' in item else None + send_email = request.GET.get('send_email') or self.context['default_email'] + permissions = _perm(item) + + try: + contributor_obj = node.add_contributor_registered_or_not( + auth=auth, + user_id=id, + email=email, + full_name=full_name, + send_email=send_email, + permissions=permissions, + bibliographic=bibliographic, + index=index, + save=True, + ) + results.append(contributor_obj) + child_nodes = item.get('child_nodes') + if child_nodes: + for child in AbstractNode.objects.filter(guids___id__in=child_nodes): + child.add_contributor_registered_or_not( + auth=auth, + user_id=id, + email=email, + full_name=full_name, + send_email=send_email, + permissions=permissions, + bibliographic=bibliographic, + index=index, + save=True, + ) + except ValidationError as e: + raise exceptions.ValidationError(detail=e.messages[0]) + except ValueError as e: + raise exceptions.NotFound(detail=e.args[0]) + + return results + + class NodeContributorsCreateSerializer(NodeContributorsSerializer): """ Overrides NodeContributorsSerializer to add email, full_name, send_email, and non-required index and users field. @@ -1239,8 +1346,8 @@ class NodeContributorsCreateSerializer(NodeContributorsSerializer): email_preferences = ['default', 'false'] - def get_proposed_permissions(self, validated_data): - return validated_data.get('permission') or osf_permissions.DEFAULT_CONTRIBUTOR_PERMISSIONS + class Meta(NodeContributorsSerializer.Meta): + list_serializer_class = NodeContributorsBulkCreateListSerializer def validate_data(self, node, user_id=None, full_name=None, email=None, index=None, child_nodes=None): if not user_id and not full_name: @@ -1264,7 +1371,7 @@ def create(self, validated_data): bibliographic = validated_data.get('bibliographic') send_email = self.context['request'].GET.get('send_email') or self.context['default_email'] child_nodes = validated_data.get('child_nodes') - permissions = self.get_proposed_permissions(validated_data) + permissions = osf_permissions.get_contributor_proposed_permissions(validated_data) self.validate_data(node, user_id=id, full_name=full_name, email=email, index=index, child_nodes=child_nodes) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 0e351edcd1b..f32874cdacb 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1432,7 +1432,7 @@ def add_contributor(self, contributor, permissions=None, visible=True, # Add default contributor permissions permissions = permissions or self.DEFAULT_CONTRIBUTOR_PERMISSIONS - self.add_permission(contrib_to_add, permissions, save=True) + self.add_permission(contrib_to_add, permissions, save=False) if make_curator: contributor_obj.is_curator = True contributor_obj.save() @@ -1482,11 +1482,13 @@ def add_contributors(self, contributors, auth=None, log=True, save=False): :param log: Add log to self :param save: Save after adding contributor """ + users = [] for contrib in contributors: self.add_contributor( contributor=contrib['user'], permissions=contrib['permissions'], visible=contrib['visible'], auth=auth, log=False, save=False, ) + users.append(contrib['user']) if log and contributors: params = self.log_params params['contributors'] = [ @@ -1502,8 +1504,10 @@ def add_contributors(self, contributors, auth=None, log=True, save=False): if save: self.save() + return self.contributor_set.filter(user__in=users) + def add_unregistered_contributor(self, fullname, email, auth, send_email=None, - visible=True, permissions=None, save=False, existing_user=None): + visible=True, permissions=None, save=True, existing_user=None): """Add a non-registered contributor to the project. :param str fullname: The full name of the person. @@ -1554,12 +1558,13 @@ def add_unregistered_contributor(self, fullname, email, auth, send_email=None, visible=visible, send_email=send_email, log=True, save=False ) self._add_related_source_tags(contributor) - self.save() + if save: + self.save() return contributor def add_contributor_registered_or_not(self, auth, user_id=None, full_name=None, email=None, send_email=None, - permissions=None, bibliographic=True, index=None, save=False): + permissions=None, bibliographic=True, index=None, save=True): OSFUser = apps.get_model('osf.OSFUser') send_email = send_email or self.contributor_email_template @@ -1573,7 +1578,7 @@ def add_contributor_registered_or_not(self, auth, user_id=None, if contributor.is_registered: contributor = self.add_contributor(contributor=contributor, auth=auth, visible=bibliographic, - permissions=permissions, send_email=send_email, save=True) + permissions=permissions, send_email=send_email, save=save) else: if not full_name: raise ValueError( @@ -1583,7 +1588,7 @@ def add_contributor_registered_or_not(self, auth, user_id=None, contributor = self.add_unregistered_contributor( fullname=full_name, email=contributor.username, auth=auth, send_email=send_email, permissions=permissions, - visible=bibliographic, existing_user=contributor, save=True + visible=bibliographic, existing_user=contributor, save=save ) else: @@ -1592,20 +1597,26 @@ def add_contributor_registered_or_not(self, auth, user_id=None, raise ValidationValueError(f'{contributor.fullname} is already a contributor.') if contributor and contributor.is_registered: - self.add_contributor(contributor=contributor, auth=auth, visible=bibliographic, - send_email=send_email, permissions=permissions, save=True) + self.add_contributor( + contributor=contributor, + auth=auth, + visible=bibliographic, + send_email=send_email, + permissions=permissions, + save=save, + ) else: contributor = self.add_unregistered_contributor( fullname=full_name, email=email, auth=auth, send_email=send_email, permissions=permissions, - visible=bibliographic, save=True + visible=bibliographic, save=save ) auth.user.email_last_sent = timezone.now() auth.user.save() if index is not None: - self.move_contributor(contributor=contributor, index=index, auth=auth, save=True) + self.move_contributor(contributor=contributor, index=index, auth=auth, save=save) contributor_obj = self.contributor_set.get(user=contributor) return contributor_obj diff --git a/osf/utils/permissions.py b/osf/utils/permissions.py index 76b656856fa..15ec2595a46 100644 --- a/osf/utils/permissions.py +++ b/osf/utils/permissions.py @@ -72,3 +72,7 @@ def check_private_key_for_anonymized_link(private_key): except PrivateLink.DoesNotExist: return False return link.anonymous + + +def get_contributor_proposed_permissions(validated_data): + return validated_data.get('permission') or DEFAULT_CONTRIBUTOR_PERMISSIONS From 8622ab5a4fe270b1c2e37d9d37d34c404fb4b086 Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Tue, 4 Nov 2025 16:30:41 +0200 Subject: [PATCH 2/2] add bulk add_contributor_registered_or_not method --- api/nodes/serializers.py | 170 +++++++++++++++++---------------------- osf/models/mixins.py | 66 +++++++++++++-- 2 files changed, 132 insertions(+), 104 deletions(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index c63f76370e6..ca90256ed41 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1,4 +1,4 @@ -from django.db import connection, transaction +from django.db import connection from packaging.version import Version from api.base.exceptions import ( @@ -1226,104 +1226,80 @@ def create(self, validated_data): node = self.context['resource'] auth = Auth(request.user) - with transaction.atomic(): - results = [] - registered_items = [] - unregistered_items = [] - - for item in validated_data: - if item.get('_id') and not item.get('user', {}).get('email'): - registered_items.append(item) - else: - unregistered_items.append(item) - - def _perm(item): - return osf_permissions.get_contributor_proposed_permissions(item) - - if registered_items: - # Load users once and build a mapping to reuse - user_map = {} - for item in registered_items: - user_id = item.get('_id') - if user_id and user_id not in user_map: - user = OSFUser.load(user_id) - if not user: - raise exceptions.NotFound(detail=f"User {user_id} not found") - user_map[user_id] = user - - contrib_dicts = [] - for item in registered_items: - user = user_map[item.get('_id')] - contrib_dicts.append({ - 'user': user, - 'permissions': _perm(item), - 'visible': item.get('bibliographic'), - }) - contribs = node.add_contributors(contrib_dicts, auth=auth, log=True, save=True) - results.extend(contribs) - - child_to_items = {} - for item in registered_items: - child_nodes = item.get('child_nodes') - if child_nodes: - for child_id in child_nodes: - child_to_items.setdefault(child_id, []).append(item) - - for child_id, items in child_to_items.items(): - child = AbstractNode.load(child_id) - if not child: - continue - child_contribs = [] - for item in items: - child_contribs.append({ - 'user': user_map[item.get('_id')], - 'permissions': _perm(item), - 'visible': item.get('bibliographic'), - }) - child.add_contributors(child_contribs, auth=auth, log=True, save=True) - - for item in unregistered_items: - id = item.get('_id') + def _perm(item): + return osf_permissions.get_contributor_proposed_permissions(item) + + send_email_default = request.GET.get('send_email') or self.context['default_email'] + # Preload users once and pass through to the bulk method (also reused for children) + user_ids = {item.get('_id') for item in validated_data if item.get('_id')} + user_map = {} + if user_ids: + for u in OSFUser.objects.filter(guids___id__in=user_ids): + user_map[u._id] = u + + payload = [] + for item in validated_data: + uid = item.get('_id') + user = user_map.get(uid) + email = item.get('user', {}).get('email', None) + full_name = item.get('full_name') or (user.fullname if user and not user.is_registered else None) + if not uid and not full_name: + raise exceptions.ValidationError(detail='A user ID or full name must be provided to add a contributor.') + payload.append({ + 'user_id': uid, + 'user': user, + 'email': email, + 'full_name': full_name, + 'send_email': send_email_default, + 'permissions': _perm(item), + 'bibliographic': item.get('bibliographic'), + 'index': item.get('_order') if '_order' in item else None, + }) + + try: + contribs = node.add_contributors_registered_or_not(payload, auth=auth, save=True) + except ValidationError as e: + raise exceptions.ValidationError(detail=e.messages[0]) + except ValueError as e: + raise exceptions.NotFound(detail=e.args[0]) + + child_to_items = {} + for item in validated_data: + child_nodes = item.get('child_nodes') + if child_nodes: + for child_id in child_nodes: + child_to_items.setdefault(child_id, []).append(item) + + for child_id, items in child_to_items.items(): + child = AbstractNode.load(child_id) + if not child: + continue + child_payload = [] + for item in items: + uid = item.get('_id') + user = user_map.get(uid) email = item.get('user', {}).get('email', None) - full_name = item.get('full_name') - bibliographic = item.get('bibliographic') - index = item.get('_order') if '_order' in item else None - send_email = request.GET.get('send_email') or self.context['default_email'] - permissions = _perm(item) + full_name = item.get('full_name') or (user.fullname if user and not user.is_registered else None) + if not uid and not full_name: + raise exceptions.ValidationError(detail='A user ID or full name must be provided to add a contributor.') + child_payload.append({ + 'user_id': uid, + 'user': user, + 'email': email, + 'full_name': full_name, + 'send_email': send_email_default, + 'permissions': _perm(item), + 'bibliographic': item.get('bibliographic'), + 'index': item.get('_order') if '_order' in item else None, + }) + try: + child.add_contributors_registered_or_not(child_payload, auth=auth, save=True) + except ValidationError as e: + raise exceptions.ValidationError(detail=e.messages[0]) + except ValueError as e: + raise exceptions.NotFound(detail=e.args[0]) - try: - contributor_obj = node.add_contributor_registered_or_not( - auth=auth, - user_id=id, - email=email, - full_name=full_name, - send_email=send_email, - permissions=permissions, - bibliographic=bibliographic, - index=index, - save=True, - ) - results.append(contributor_obj) - child_nodes = item.get('child_nodes') - if child_nodes: - for child in AbstractNode.objects.filter(guids___id__in=child_nodes): - child.add_contributor_registered_or_not( - auth=auth, - user_id=id, - email=email, - full_name=full_name, - send_email=send_email, - permissions=permissions, - bibliographic=bibliographic, - index=index, - save=True, - ) - except ValidationError as e: - raise exceptions.ValidationError(detail=e.messages[0]) - except ValueError as e: - raise exceptions.NotFound(detail=e.args[0]) - - return results + return contribs class NodeContributorsCreateSerializer(NodeContributorsSerializer): diff --git a/osf/models/mixins.py b/osf/models/mixins.py index f32874cdacb..65a72d15e17 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1507,7 +1507,8 @@ def add_contributors(self, contributors, auth=None, log=True, save=False): return self.contributor_set.filter(user__in=users) def add_unregistered_contributor(self, fullname, email, auth, send_email=None, - visible=True, permissions=None, save=True, existing_user=None): + visible=True, permissions=None, save=True, existing_user=None, + log=True): """Add a non-registered contributor to the project. :param str fullname: The full name of the person. @@ -1555,7 +1556,7 @@ def add_unregistered_contributor(self, fullname, email, auth, send_email=None, self.add_contributor( contributor, permissions=permissions, auth=auth, - visible=visible, send_email=send_email, log=True, save=False + visible=visible, send_email=send_email, log=log, save=False ) self._add_related_source_tags(contributor) if save: @@ -1564,12 +1565,13 @@ def add_unregistered_contributor(self, fullname, email, auth, send_email=None, def add_contributor_registered_or_not(self, auth, user_id=None, full_name=None, email=None, send_email=None, - permissions=None, bibliographic=True, index=None, save=True): + permissions=None, bibliographic=True, index=None, save=True, + user=None, log=True): OSFUser = apps.get_model('osf.OSFUser') send_email = send_email or self.contributor_email_template if user_id: - contributor = OSFUser.load(user_id) + contributor = user or OSFUser.load(user_id) if not contributor: raise ValueError(f'User with id {user_id} was not found.') @@ -1578,7 +1580,7 @@ def add_contributor_registered_or_not(self, auth, user_id=None, if contributor.is_registered: contributor = self.add_contributor(contributor=contributor, auth=auth, visible=bibliographic, - permissions=permissions, send_email=send_email, save=save) + permissions=permissions, send_email=send_email, save=save, log=log) else: if not full_name: raise ValueError( @@ -1588,7 +1590,7 @@ def add_contributor_registered_or_not(self, auth, user_id=None, contributor = self.add_unregistered_contributor( fullname=full_name, email=contributor.username, auth=auth, send_email=send_email, permissions=permissions, - visible=bibliographic, existing_user=contributor, save=save + visible=bibliographic, existing_user=contributor, save=save, log=log ) else: @@ -1604,12 +1606,13 @@ def add_contributor_registered_or_not(self, auth, user_id=None, send_email=send_email, permissions=permissions, save=save, + log=log, ) else: contributor = self.add_unregistered_contributor( fullname=full_name, email=email, auth=auth, send_email=send_email, permissions=permissions, - visible=bibliographic, save=save + visible=bibliographic, save=save, log=log ) auth.user.email_last_sent = timezone.now() @@ -1621,6 +1624,55 @@ def add_contributor_registered_or_not(self, auth, user_id=None, contributor_obj = self.contributor_set.get(user=contributor) return contributor_obj + def add_contributors_registered_or_not(self, contributors, auth=None, log=True, save=False): + """Add multiple contributors using the unified registered-or-not path. + + Each item should be a dictionary with keys compatible with + `add_contributor_registered_or_not`, e.g.: + { + 'user_id': '', + 'user': '' or None, + 'email': '' or None, + 'full_name': '' or None, + 'send_email': '' or None, + 'permissions': , + 'bibliographic': , + 'index': , + } + """ + results = [] + + for item in contributors: + contributor_obj = self.add_contributor_registered_or_not( + auth=auth, + user_id=item.get('user_id'), + user=item.get('user'), + full_name=item.get('full_name'), + email=item.get('email'), + send_email=item.get('send_email') or getattr(self, 'contributor_email_template', None), + permissions=item.get('permissions'), + bibliographic=item.get('bibliographic', True), + index=item.get('index'), + save=False, + log=False, + ) + results.append(contributor_obj) + + if log and results: + params = self.log_params + params['contributors'] = [c.user._id for c in results] + self.add_log( + action=self.log_class.CONTRIB_ADDED, + params=params, + auth=auth, + save=False, + ) + + if save: + self.save() + + return results + def replace_contributor(self, old, new): """ Replacing unregistered contributor with a verified user