[ENG-6445] [ENG-6457] Preprints Version Creation and API Implementation - Part 2 #10807
Conversation
cslzchen
changed the title
api/preprints/serializers.py
Outdated
| title = ser.CharField(required=False) | ||
|
|
||
| dupliate_from_guid = ser.CharField(required=True, write_only=True) |
There was a problem hiding this comment.
Let's use create_from_guid, which seems to describe version creation better than duplicate.
| try: | ||
| preprint, update_data = Preprint.create_version(dupliate_from_guid, auth) | ||
| except PendingVersionExists: | ||
| raise Conflict(detail='Before creating a new version, you must publish the latest version.') |
There was a problem hiding this comment.
Reminder for myself: need to check with FE if this detail is surfaced back directly to the user; if so, we need Product to provide the exact language; if not, we may not need to provide the detail.
| # These permissions are not checked for the list of preprints, permissions handled by the query | ||
| permission_classes = ( | ||
| drf_permissions.IsAuthenticatedOrReadOnly, | ||
| base_permissions.TokenHasScope, | ||
| ContributorOrPublic, | ||
| ) |
There was a problem hiding this comment.
Reminder for myself: it seems neither the create nor the list checks the permission ... if the comment is correct ... need to double check ... I think these are still checked but just not strict enough, which finer-grained access control are done in get_queryset().
framework/exceptions/__init__.py
Outdated
| class PendingVersionExists(FrameworkError): | ||
| pass |
There was a problem hiding this comment.
Add docstr to explain what this error is and when it is raised. PendingPreprintVersionExists is a better name too.
| except cls.MultipleObjectsReturned: | ||
| return None |
There was a problem hiding this comment.
Future improvements: let's return None for now, but we may need to add some sentry log to detect this and maybe re-raise the exception afterwards.
osf/models/preprint.py
Outdated
| def has_create_version_permission(self, user): | ||
| from guardian.shortcuts import get_group_perms | ||
| object_type = self.guardian_object_type | ||
|
|
||
| if not user or user.is_anonymous: | ||
| return False | ||
| perm = f'{ADMIN}_{object_type}' | ||
| # Using get_group_perms to get permissions that are inferred through | ||
| # group membership - not inherited from superuser status | ||
| has_permission = perm in get_group_perms(user, self) | ||
|
|
||
| return has_permission | ||
|
|
||
| @classmethod | ||
| def create_version(cls, dupliate_from_guid): | ||
| def create_version(cls, dupliate_from_guid, auth): | ||
| source_preprint = cls.load(dupliate_from_guid.split(cls.GUID_VERSION_DELIMITER)[0]) | ||
| if not source_preprint: | ||
| raise NotFound | ||
| if not source_preprint.has_create_version_permission(auth.user): |
There was a problem hiding this comment.
I think we can simply use source_preprint.has_permission(auth.user, ADMIN).
Purpose
Preprints Version Creation and API Implementation - Part 2
Changes
QA Notes
N/A
Documentation
N/A
Side Effects
N/A
Ticket
https://openscience.atlassian.net/browse/ENG-6457
https://openscience.atlassian.net/browse/ENG-6445