Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/ENG-6134 #10951

Merged
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
29 changes: 29 additions & 0 deletions api/base/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.core.exceptions import ObjectDoesNotExist
from django.db.models import QuerySet
from rest_framework import fields
from rest_framework import exceptions
from rest_framework.exceptions import NotFound
from rest_framework.reverse import reverse

Expand Down Expand Up @@ -294,3 +295,31 @@ def _view_by_flag(request, *args, **kwargs):
# in `api_tests.base.test_views` and `api.base.serializers.RelationshipField`
_view_by_flag.view_class = new_view.view_class # type: ignore[attr-defined]
return _view_by_flag


def update_contributors_permissions_and_bibliographic_status(serializer, instance, validated_data):
'''
Helper function for serializers to update permissions of contributors and their bibliographic status
'''
index = None
if '_order' in validated_data:
index = validated_data.pop('_order')

auth = Auth(serializer.context['request'].user)
node = serializer.context['resource']

if 'bibliographic' in validated_data:
bibliographic = validated_data.get('bibliographic')
else:
bibliographic = node.get_visible(instance.user)
permission = validated_data.get('permission') or instance.permission
try:
if index is not None:
node.move_contributor(instance.user, auth, index, save=True)
node.update_contributor(instance.user, permission, bibliographic, auth, save=True)
except node.state_error as e:
raise exceptions.ValidationError(detail=str(e))
except ValueError as e:
raise exceptions.ValidationError(detail=str(e))
instance.refresh_from_db()
return instance
9 changes: 9 additions & 0 deletions api/nodes/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,15 @@ class ContributorDetailPermissions(permissions.BasePermission):
def load_resource(self, context, view):
return AbstractNode.load(context[view.node_lookup_url_kwarg])

def has_permission(self, request, view):
auth = get_user_auth(request)
context = request.parser_context['kwargs']
resource = self.load_resource(context, view)
if request.method == 'POST' and not resource.has_permission(auth.user, osf_permissions.ADMIN):
return False

return super().has_permission(request, view)

def has_object_permission(self, request, view, obj):
assert_resource_type(obj, self.acceptable_models)
auth = get_user_auth(request)
Expand Down
28 changes: 6 additions & 22 deletions api/nodes/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from api.base.utils import (
absolute_reverse, get_object_or_error,
get_user_auth, is_truthy,
update_contributors_permissions_and_bibliographic_status,
)
from api.base.versioning import get_kebab_snake_case_field
from api.institutions.utils import update_institutions
Expand Down Expand Up @@ -1268,28 +1269,11 @@ class NodeContributorDetailSerializer(NodeContributorsSerializer):
index = ser.IntegerField(required=False, read_only=False, source='_order')

def update(self, instance, validated_data):
index = None
if '_order' in validated_data:
index = validated_data.pop('_order')

auth = Auth(self.context['request'].user)
node = self.context['resource']

if 'bibliographic' in validated_data:
bibliographic = validated_data.get('bibliographic')
else:
bibliographic = node.get_visible(instance.user)
permission = validated_data.get('permission') or instance.permission
try:
if index is not None:
node.move_contributor(instance.user, auth, index, save=True)
node.update_contributor(instance.user, permission, bibliographic, auth, save=True)
except node.state_error as e:
raise exceptions.ValidationError(detail=str(e))
except ValueError as e:
raise exceptions.ValidationError(detail=str(e))
instance.refresh_from_db()
return instance
return update_contributors_permissions_and_bibliographic_status(
self,
instance,
validated_data,
)


class NodeLinksSerializer(JSONAPISerializer):
Expand Down
15 changes: 7 additions & 8 deletions api/nodes/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -521,22 +521,21 @@ class NodeContributorDetail(BaseContributorDetail, generics.RetrieveUpdateDestro
def get_resource(self):
return self.get_node()

# overrides DestroyAPIView
def get_serializer_context(self):
context = JSONAPIBaseView.get_serializer_context(self)
context['resource'] = self.get_resource()
context['default_email'] = 'default'
return context

def perform_destroy(self, instance):
Copy link
Contributor

Choose a reason for hiding this comment

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

As I said above don't bother using a mixin, it's okay to repeat yourself a little across views. It's just easier to see new behavior when you have a small number of methods for a view.

node = self.get_resource()
auth = get_user_auth(self.request)
if len(node.visible_contributors) == 1 and instance.visible:
if node.visible_contributors.count() == 1 and instance.visible:
raise ValidationError('Must have at least one visible contributor')
removed = node.remove_contributor(instance, auth)
if not removed:
raise ValidationError('Must have at least one registered admin contributor')

def get_serializer_context(self):
context = JSONAPIBaseView.get_serializer_context(self)
context['resource'] = self.get_resource()
context['default_email'] = 'default'
return context


class NodeImplicitContributorsList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin, NodeMixin):
permission_classes = (
Expand Down
21 changes: 20 additions & 1 deletion api/registrations/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
NodeLinksSerializer,
NodeLicenseSerializer,
NodeContributorsSerializer,
NodeContributorsCreateSerializer,
RegistrationProviderRelationshipField,
get_license_details,
)
Expand All @@ -30,6 +31,7 @@
ShowIfVersion, VersionedDateTimeField, ValuesListField,
HideIfWithdrawalOrWikiDisabled,
)
from api.base.utils import update_contributors_permissions_and_bibliographic_status
from api.institutions.utils import update_institutions
from framework.auth.core import Auth
from osf.exceptions import NodeStateError
Expand Down Expand Up @@ -83,7 +85,7 @@ class RegistrationSerializer(NodeSerializer):

ia_url = ser.URLField(read_only=True)
reviews_state = ser.CharField(source='moderation_state', read_only=True)
title = ser.CharField(read_only=True)
title = ser.CharField(required=False)
description = ser.CharField(required=False, allow_blank=True, allow_null=True)
category_choices = NodeSerializer.category_choices
category_choices_string = NodeSerializer.category_choices_string
Expand Down Expand Up @@ -917,6 +919,23 @@ def get_absolute_url(self, obj):
},
)

def update(self, instance, validated_data):
return update_contributors_permissions_and_bibliographic_status(
self,
instance,
validated_data,
)


class RegistrationContributorsCreateSerializer(NodeContributorsCreateSerializer, RegistrationContributorsSerializer):
"""
Overrides RegistrationContributorsSerializer to add email, full_name, send_email, and non-required index and users field.

id and index redefined because of the two serializers we've inherited
"""
id = IDField(source='_id', required=False, allow_null=True)
index = ser.IntegerField(required=False, source='_order')


class RegistrationFileSerializer(OsfStorageFileSerializer):

Expand Down
53 changes: 47 additions & 6 deletions api/registrations/views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from rest_framework import generics, permissions as drf_permissions
from rest_framework import generics, mixins, permissions as drf_permissions
from rest_framework.exceptions import ValidationError, NotFound, PermissionDenied
from framework.auth.oauth_scopes import CoreScopes

Expand Down Expand Up @@ -58,6 +58,7 @@
RegistrationSerializer,
RegistrationDetailSerializer,
RegistrationContributorsSerializer,
RegistrationContributorsCreateSerializer,
RegistrationCreateSerializer,
RegistrationStorageProviderSerializer,
)
Expand Down Expand Up @@ -267,7 +268,7 @@ def get_serializer_context(self):
return context


class RegistrationContributorsList(BaseContributorList, RegistrationMixin, UserMixin):
class RegistrationContributorsList(BaseContributorList, mixins.CreateModelMixin, RegistrationMixin, UserMixin):
"""The documentation for this endpoint can be found [here](https://developer.osf.io/#operation/registrations_contributors_list).
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good use of mixin. this class looks correct.

"""
view_category = 'registrations'
Expand All @@ -282,16 +283,33 @@ class RegistrationContributorsList(BaseContributorList, RegistrationMixin, UserM
permission_classes = (
ContributorDetailPermissions,
drf_permissions.IsAuthenticatedOrReadOnly,
ReadOnlyIfRegistration,
base_permissions.TokenHasScope,
)

def get_resource(self):
return self.get_node(check_object_permissions=False)

def get_serializer_class(self):
if self.request.method == 'POST':
return RegistrationContributorsCreateSerializer

return self.serializer_class

def post(self, request, *args, **kwargs):
return super().create(request, *args, **kwargs)

def get_default_queryset(self):
node = self.get_node(check_object_permissions=False)
node = self.get_resource()
return node.contributor_set.all().prefetch_related('user__guids')

def get_serializer_context(self):
context = super().get_serializer_context()
context['resource'] = self.get_resource()
context['default_email'] = 'default'
return context


class RegistrationContributorDetail(BaseContributorDetail, RegistrationMixin, UserMixin):
class RegistrationContributorDetail(BaseContributorDetail, mixins.UpdateModelMixin, mixins.DestroyModelMixin, RegistrationMixin, UserMixin):
"""The documentation for this endpoint can be found [here](https://developer.osf.io/#operation/registrations_contributors_read).
"""
view_category = 'registrations'
Expand All @@ -304,10 +322,33 @@ class RegistrationContributorDetail(BaseContributorDetail, RegistrationMixin, Us
permission_classes = (
ContributorDetailPermissions,
drf_permissions.IsAuthenticatedOrReadOnly,
ReadOnlyIfRegistration,
base_permissions.TokenHasScope,
)

def get_resource(self):
return self.get_node()

def patch(self, request, *args, **kwargs):
return super().partial_update(request, *args, **kwargs)

def delete(self, request, *args, **kwargs):
return super().destroy(request, *args, **kwargs)

def get_serializer_context(self):
context = super().get_serializer_context()
context['resource'] = self.get_resource()
context['default_email'] = 'default'
return context

def perform_destroy(self, instance):
node = self.get_resource()
auth = get_user_auth(self.request)
if node.visible_contributors.count() == 1 and instance.visible:
raise ValidationError('Must have at least one visible contributor')
removed = node.remove_contributor(instance, auth)
if not removed:
raise ValidationError('Must have at least one registered admin contributor')


class RegistrationBibliographicContributorsList(NodeBibliographicContributorsList, RegistrationMixin):

Expand Down
6 changes: 4 additions & 2 deletions api_tests/registrations/views/test_registration_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ def test_fields(
assert res.json['data']['attributes']['category'] == 'instrumentation'
assert res.json['data']['attributes']['description'] == description
assert res.json['data']['attributes']['tags'] == tags
assert res.json['data']['attributes']['title'] == private_registration.title
assert res.json['data']['attributes']['title'] == 'New title'
assert res.json['data']['attributes']['node_license']['copyright_holders'] == copyright_holders
assert res.json['data']['attributes']['node_license']['year'] == year
assert res.json['data']['attributes']['custom_citation'] == custom_citation
Expand Down Expand Up @@ -527,7 +527,7 @@ def test_fields(
assert res.json['data']['attributes']['category'] == ''
assert res.json['data']['attributes']['description'] == ''
assert res.json['data']['attributes']['tags'] == []
assert res.json['data']['attributes']['title'] == private_registration.title
assert res.json['data']['attributes']['title'] == 'New title'
assert res.json['data']['attributes']['node_license']['copyright_holders'] == []
assert res.json['data']['attributes']['node_license']['year'] == ''
assert res.json['data']['attributes']['custom_citation'] == ''
Expand Down Expand Up @@ -577,6 +577,7 @@ def test_turning_private_registrations_public(

def test_registration_fields_are_read_only(self):
writeable_fields = [
'title',
'type',
'public',
'draft_registration',
Expand All @@ -600,6 +601,7 @@ def test_registration_fields_are_read_only(self):

def test_registration_detail_fields_are_read_only(self):
writeable_fields = [
'title',
'type',
'public',
'draft_registration',
Expand Down
Loading
Loading