Skip to content

Conversation

@antkryt
Copy link
Contributor

@antkryt antkryt commented Nov 4, 2025

Purpose

optimize batch create contributors

Changes

see ticket comment section

QA Notes

Documentation

Side Effects

Ticket

https://openscience.atlassian.net/browse/ENG-9596

@antkryt antkryt changed the base branch from feature/pbs-25-21 to feature/pbs-25-25 December 25, 2025 15:31
Copy link
Contributor

@ihorsokhanexoft ihorsokhanexoft left a comment

Choose a reason for hiding this comment

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

A few notes:

Comment on lines +1279 to +1300
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.')
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])
Copy link
Contributor

Choose a reason for hiding this comment

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

this part repeats above, so let's create a separate method for parsing data

Comment on lines +1459 to +1463
# 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)
Copy link
Contributor

Choose a reason for hiding this comment

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

And would be nice to have a few tests to check data parsing inside of NodeContributorsBulkCreateListSerializer as it has a lot of logic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants